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 libpciaccess library for processing PCI devices,
that library requires careful handling in order to avoid collisions among
multiple call sites 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 either
libpciaccess or igt_device_scan is avoided.  As an additional benefit,
there is no need to re-implement some functionality, already provided by
libpci function pci_find_cap(), which has no equivalent in libpciaccess.

v3: Fix incorrect use of ffs(),
  - fix bridge link attribute printing suppressed with DEVTYPE_DISCRETE,
  - in commit description, elaborate more on reasons for using libpci.
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 | 83 +++++++++++++++++++++++++++++++++++++++++--
 lib/meson.build       |  2 ++
 meson.build           |  1 +
 3 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
index 6a907a4ebb..bd88e61b10 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) - 1;
+
+       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,19 @@ 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);
+
+       /* override DEVTYPE_INTEGRATED so link attributes won't be omitted */
+       bridge_idev->dev_type = DEVTYPE_ALL;
 }
 
 static struct igt_device *duplicate_device(struct igt_device *dev) {
@@ -1072,6 +1140,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 +1164,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 +1179,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)
 
-- 
2.52.0

Reply via email to