Re: [PATCH i-g-t 3/7] lib/igt_device_scan: Include PCIe bridge upstream port if available

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

Hi Janusz,

On Wed Jan 21, 2026 at 12:42 PM CET, Janusz Krzysztofik wrote:
> Users of Intel discrete graphics adapters are confused with fake
> information on PCIe link bandwidth (speed and size) of their GPU devices
> reported by sysfs and userspace tools, including our lsgpu utility.  In
> order for the lsgpu to show correct link bandwidth information, we need to
> identify an upstream port of a PCIe bridge that sits on the GPU card and
> get that information from that port.
>
> Since the tool uses our udev based igt_device_scan library for identifying
> GPU devices and printing their properties and attributes, modifications
> that we need apply to that library.
>
> When scanning for DRM devices and their PCI parents, the lsgpu utility
> requests collection of all their attributes.  When running in this mode,
> also try to collect information about upstream ports of PCIe bridges of
> discrete GPU devices.  Once collected, the lsgpu utility will show that
> information automatically while listing the devices.
>
> While IGT tests are using pciaccess library for processing PCI devices,
> that library requires careful handling in order to avoid collisions among
> multiple processes or threads potentially using it.  That protection is
> implemented in igt_device with help of IGT exit handlers. That requires
> linking with full igt_core library code, while the lsgpu tool now depends
> neither on igt_device nor on igt_core.  To keep that independence,
> implement the new code around libpci.  With that approach, refactoring of
> IGT use of pciaccess is avoided.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10753
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  lib/igt_device_scan.c | 73 +++++++++++++++++++++++++++++++++++++++++--
>  lib/meson.build       |  2 ++
>  meson.build           |  1 +
>  3 files changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
> index d3a2ebe8d2..34c7a8131b 100644
> --- a/lib/igt_device_scan.c
> +++ b/lib/igt_device_scan.c
> @@ -36,6 +36,7 @@
>  #ifdef __linux__
>  #include <linux/limits.h>
>  #endif
> +#include <pci/pci.h>
>  #include <sys/stat.h>
>  #include <sys/time.h>
>  #include <sys/types.h>
> @@ -909,6 +910,27 @@ static struct igt_device *igt_device_from_syspath(const char *syspath)
>  	return NULL;
>  }
>  
> +static bool is_pcie_upstream_bridge(struct pci_dev *dev)
> +{
> +	struct pci_cap *pcie;
> +	uint8_t type, dir;
> +
> +	type = pci_read_byte(dev, PCI_HEADER_TYPE) & 0x7f;
> +	if (type != PCI_HEADER_TYPE_BRIDGE)
> +		return false;
> +
> +	pcie = pci_find_cap(dev, PCI_CAP_ID_EXP, PCI_CAP_NORMAL);
> +	if (!pcie)
> +		return false;
> +
> +	/* GET_REG_MASK macro borrowed from pciutils' internal bitops.h */
> +#define GET_REG_MASK(reg, mask) (((reg) & (mask)) / ((mask) & ~((mask) << 1)))
> +	dir = GET_REG_MASK(pci_read_word(dev, pcie->addr + PCI_EXP_FLAGS), PCI_EXP_FLAGS_TYPE);
> +#undef GET_REG_MASK
Instead of copying the macro, we could just use: 
	type = ( pci_read_word... & PCI_EXP_FLAGS_TYPE) >> 4. This seems cleaner.

> +
> +	return dir == PCI_EXP_TYPE_UPSTREAM;
> +}
> +
>  #define RETRIES_GET_PARENT 5
>  
>  static struct igt_device *find_or_add_igt_device(struct udev *udev,
> @@ -948,18 +970,55 @@ static struct igt_device *find_or_add_igt_device(struct udev *udev,
>  	return idev;
>  }
>  
> +static struct udev_device *get_pcie_upstream_bridge(struct udev *udev,
> +						    struct udev_device *dev)
> +{
> +	struct pci_access *pacc;
> +
> +	pacc = pci_alloc();
> +	pci_init(pacc);
> +
I'm not entirely familiar with this pci library, but is it necessary to initialize it
for every device? It might be more efficient to do it once in scan_drm_devices.

-- 
Best regards,
Sebastian