Hi, Commenting myself, sorry.
On Tuesday, 27 January 2026 10:54:20 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. > > v2: Drop unclear GET_REG_MASK macro (Sebastian), > - reuse no longer needed variable containing PCI_HEADER_TYPE for storing > PCI_EXP_FLAGS_TYPE, > - maintain a single instance of struct pci_access throughout processing > of the whole udev device list (Sebastian). > > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10753 > Cc: Sebastian Brzezinka <[email protected]> > Signed-off-by: Janusz Krzysztofik <[email protected]> > --- > lib/igt_device_scan.c | 80 +++++++++++++++++++++++++++++++++++++++++-- > lib/meson.build | 2 ++ > meson.build | 1 + > 3 files changed, 80 insertions(+), 3 deletions(-) > > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c > index 6a907a4ebb..ec6cc38953 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> > @@ -913,6 +914,26 @@ 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; > + > + 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; > + > + type = pci_read_word(dev, pcie->addr + PCI_EXP_FLAGS); > + type &= PCI_EXP_FLAGS_TYPE; > + type >>= ffs(PCI_EXP_FLAGS_TYPE); That shoud read: type >== ffs(PCI_EXP_FLAGS_TYPE) - 1; (ffs() counts bit positions starting from 1, not 0). I missed to apply that last minute correction before sending, sorry. Thanks, Janusz > + > + return type == PCI_EXP_TYPE_UPSTREAM; > +} > + > #define RETRIES_GET_PARENT 5 > > static struct igt_device *find_or_add_igt_device(struct udev *udev, > @@ -952,18 +973,52 @@ 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) > +{ > + igt_assert(pacc); > + > + for (dev = udev_device_get_parent(dev); dev; dev = > udev_device_get_parent(dev)) { > + struct pci_filter filter; > + struct pci_dev *pci_dev; > + const char *slot; > + > + slot = udev_device_get_property_value(dev, "PCI_SLOT_NAME"); > + if (igt_debug_on(!slot)) > + continue; > + > + pci_filter_init(pacc, &filter); > + if (igt_debug_on(pci_filter_parse_slot(&filter, (char *)slot))) > + continue; > + > + pci_dev = pci_get_dev(pacc, filter.domain, filter.bus, > filter.slot, filter.func); > + if (igt_debug_on(!pci_dev)) > + continue; > + > + if (is_pcie_upstream_bridge(pci_dev)) > + break; > + } > + > + return dev; > +} > + > /* > * For each drm igt_device add or update its parent igt_device to the array. > * As card/render drm devices mostly have same parent (vkms is an exception) > * link to it and update corresponding drm_card / drm_render fields. > + * > + * If collecting all attributes and the parent is a discrete GPU then also > + * add or update its bridge's upstream port. > */ > static void update_or_add_parent(struct udev *udev, > struct udev_device *dev, > struct igt_device *idev, > + struct pci_access *pacc, > bool limit_attrs) > { > - struct udev_device *parent_dev; > - struct igt_device *parent_idev; > + struct igt_device *parent_idev, *bridge_idev; > + struct udev_device *parent_dev, *bridge_dev; > const char *devname; > > /* > @@ -983,6 +1038,16 @@ static void update_or_add_parent(struct udev *udev, > parent_idev->drm_render = strdup(devname); > > idev->parent = parent_idev; > + > + if (!pacc || parent_idev->dev_type != DEVTYPE_DISCRETE) > + return; > + > + bridge_dev = get_pcie_upstream_bridge(udev, parent_dev, pacc); > + if (!bridge_dev) > + return; > + > + bridge_idev = find_or_add_igt_device(udev, bridge_dev, limit_attrs); > + igt_assert(bridge_idev); > } > > static struct igt_device *duplicate_device(struct igt_device *dev) { > @@ -1072,6 +1137,7 @@ static void scan_drm_devices(bool limit_attrs) > struct udev *udev; > struct udev_enumerate *enumerate; > struct udev_list_entry *devices, *dev_list_entry; > + struct pci_access *pacc = NULL; > struct igt_device *dev; > int ret; > > @@ -1095,6 +1161,12 @@ static void scan_drm_devices(bool limit_attrs) > if (!devices) > return; > > + /* prepare for upstream bridge port scan if called from lsgpu */ > + if (!limit_attrs) { > + pacc = pci_alloc(); > + pci_init(pacc); > + } > + > udev_list_entry_foreach(dev_list_entry, devices) { > const char *path; > struct udev_device *udev_dev; > @@ -1104,10 +1176,12 @@ static void scan_drm_devices(bool limit_attrs) > udev_dev = udev_device_new_from_syspath(udev, path); > idev = igt_device_new_from_udev(udev_dev, limit_attrs); > igt_list_add_tail(&idev->link, &igt_devs.all); > - update_or_add_parent(udev, udev_dev, idev, limit_attrs); > + update_or_add_parent(udev, udev_dev, idev, pacc, limit_attrs); > > udev_device_unref(udev_dev); > } > + if (pacc) > + pci_cleanup(pacc); > udev_enumerate_unref(enumerate); > udev_unref(udev); > > diff --git a/lib/meson.build b/lib/meson.build > index 1a569ba52a..d10e1405ac 100644 > --- a/lib/meson.build > +++ b/lib/meson.build > @@ -139,6 +139,7 @@ lib_deps = [ > libdrm, > libdw, > libkmod, > + libpci, > libudev, > math, > pciaccess, > @@ -332,6 +333,7 @@ lib_igt_perf = declare_dependency(link_with : > lib_igt_perf_build, > > scan_dep = [ > glib, > + libpci, > libudev, > ] > > diff --git a/meson.build b/meson.build > index 4b2496c016..57849648a3 100644 > --- a/meson.build > +++ b/meson.build > @@ -162,6 +162,7 @@ endif > build_info += 'Valgrind annotations: @0@'.format(valgrind.found()) > > cairo = dependency('cairo', version : '>1.12.0', required : true) > +libpci = dependency('libpci', required : true) > libudev = dependency('libudev', required : true) > glib = dependency('glib-2.0', required : true) > >
