Re: [PATCH i-g-t v3 6/6] lib/igt_device_scan: Print GPU upstream port parent/child relations

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

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 <sebastian.brzezinka@intel.com>
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  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.
> 
> 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.

I've had another look at it.  We could pass a flag that says that's a bridge, 
not a GPU, but that's not sufficient.  It could be used for selecting correct 
dev_type for the bridge, but if we also wanted igt_device_new_from_udev() to 
populated codename with a copy of that of the GPU then also that data would 
have to be passed.  OTOH, update_or_add_parent() never fully relied on 
igt_device_new_from_udev() populating all attributes of a GPU parent and 
handled some, e.g. drm_dev and drm_render, locally, then I think we can take 
a similar approach to a bridge sub-parent.  As I stated before, the only non-
trivial case is the already populated codename, but I'll fix its handling.

Thanks,
Janusz

> 
> > +	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.
> 
>