On Thursday, 29 January 2026 12:49:39 CET Sebastian Brzezinka wrote:
> Hi Janusz,
>
> On Wed Jan 28, 2026 at 5:09 PM CET, Janusz Krzysztofik wrote:
> > In a short listing, lsgpu prints a sysfs path of a PCI GPU parent as a
> > local attribute of a DRM device. However, if that's a discrete GPU and
> > its associated PCIe upstream bridge port has been identified, no
> > information on that bridge is listed among the GPU attributes. Follow the
> > pattern used with DRM devices and also show a PCI slot of the bridge port
> > as a local attribute of the discrete GPU device.
> >
> > Moreover, in both short and detailed listings, local attributes intended
> > for providing device names of GPU associated DRM devices and the GPU
> > codename are also printed as attributes of related PCIe upstream bridge
> > port, however, the DRM device names are shown as (null), and the codename
> > attribute provides raw vendor:device codes of the bridge itself. Replace
> > those with PCI slot and codename of the GPU device.
> >
> > v2: Allocate memory to local attributes of a bridge for safety
(Sebastian),
> > - merge with a formerly separate patch "lib/igt_device_scan: Don't print
> > bridge not applicable attributes" (Sebastian),
> > - no need for DEVTYPE_BRIDGE, just skip attributes if NULL.
> >
> > Cc: Sebastian Brzezinka <[email protected]>
> > Signed-off-by: Janusz Krzysztofik <[email protected]>
> > ---
> > lib/igt_device_scan.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> > index f4d2eb6568..96bf0e359d 100644
> > --- a/lib/igt_device_scan.c
> > +++ b/lib/igt_device_scan.c
> > @@ -249,6 +249,8 @@ struct igt_device {
> > char *codename; /* For grouping by codename */
> > enum dev_type dev_type; /* For grouping by integrated/discrete */
> >
> > + char *pci_gpu; /* Filled for upstream bridge ports */
> > +
> > struct igt_list_head link;
> > };
> >
> > @@ -1063,6 +1065,9 @@ static void update_or_add_parent(struct udev *udev,
> >
> > /* override DEVTYPE_INTEGRATED so link attributes won't be omitted
*/
> > bridge_idev->dev_type = DEVTYPE_ALL;
> > + bridge_idev->pci_gpu = strdup(parent_idev->pci_slot_name);
> > + bridge_idev->codename = strdup(parent_idev->codename);
> Releasing memory here is safer, but we must ensure
> igt_device_new_from_udev hasn't already filled the codename otherwise,
> the original pointer will be lost.
Indeed, it comes already populated, then I should release the old string
before overwriting.
>
> I’m thinking about how to refactor these functions to make them
> cleaner. They’re a bit cluttered right now since the 'find' and
> 'update' logic are merged together. This might be outside the scope
> of your current patches, but the memory management is becoming quite
> confusing. Unfortunately, there isn't an easy way to move this logic
> into igt_device_new_from_udev right now.
>
> > + parent_idev->parent = bridge_idev;
> > }
> >
> > static struct igt_device *duplicate_device(struct igt_device *dev) {
> > @@ -1234,6 +1239,7 @@ static void igt_device_free(struct igt_device *dev)
> > free(dev->device);
> > free(dev->driver);
> > free(dev->pci_slot_name);
> > + free(dev->pci_gpu);
> It could be unalocated memory.
Since igt_device_new() allocates memory with calloc(), it may be NULL, and
free(NULL) is safe, I believe.
Thanks,
Janusz
>
>
