Hi Sebastian,
On Friday, 23 January 2026 12:02:30 CET Sebastian Brzezinka wrote:
> 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 <[email protected]>
> > ---
> > 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.
I guess it took you a bit of time to understand how that macro works, so it
did to me :-). Then OK, let's use the more clear but correct looking version
you propose, even if it's not clear to me why they did it that way, less
clear, more complicated.
>
> > +
> > + 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.
OK, you've got it.
Thanks,
Janusz
