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

[Sebastian Brzezinka <sebastian.brzezinka@intel.com>]

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.

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

-- 
Best regards,
Sebastian