Re: [PATCH i-g-t 1/7] lib/igt_device_scan: Don't print fake link bandwidth attributes

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

On Fri Jan 23, 2026 at 3:42 PM CET, Janusz Krzysztofik wrote:
> On Friday, 23 January 2026 15:20:16 CET Sebastian Brzezinka wrote:
>> On Fri Jan 23, 2026 at 3:10 PM CET, Janusz Krzysztofik wrote:
>> > Hi Sebastian,
>> >
>> > Thanks for looking at this.
>> >
>> > On Friday, 23 January 2026 12:01:54 CET Sebastian Brzezinka wrote:
>> >> 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 tools like lspci or lsgpu.  That fake information is
>> >> > unfortunately provided by hardware, Linux PCI subsystem just exposes it
>> >> > untouched to upper layers, including userspace via sysfs, and userspace
>> >> > tools just report those fake values.
>> >> >
>> >> > While we can't do much about the kernel side or general purpose userspace
>> >> > tools like lspci, we can try to address the issue with our lsgpu utility.
>> >> >
>> >> > Correct link bandwidth attributes of a discrete GPU card can be obtained
>> >> > from the kernel by looking not at the PCI device of the GPU itself, only
>> >> > at a PCIe upstream port of the card's PCI bridge.  For integrity with
>> >> > content of the sysfs and with output from the other tools, we are not
>> >> > going to replace the fake information with that from the bridge upstream
>> >> > port, only show that port and its attributes themselves while listing
>> >> > devices.
>> >> >
>> >> > 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.
>> >> >
>> >> > As a first step, exclude the fake data from being printed.
>> >> >
>> >> > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10753
>> >> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
>> >> > ---
>> >> >  lib/igt_device_scan.c | 8 ++++++++
>> >> >  1 file changed, 8 insertions(+)
>> >> >
>> >> > diff --git a/lib/igt_device_scan.c b/lib/igt_device_scan.c
>> >> > index abd8ca209e..7753262a53 100644
>> >> > --- a/lib/igt_device_scan.c
>> >> > +++ b/lib/igt_device_scan.c
>> >> > @@ -613,6 +613,14 @@ static void dump_props_and_attrs(const struct igt_device *dev)
>> >> >  
>> >> >  	printf("\n[attributes]\n");
>> >> >  	igt_map_foreach(dev->attrs_map, entry) {
>> >> > +		/* omit fake link bandwidth attributes */
>> >> > +		if (dev->dev_type == DEVTYPE_DISCRETE &&
>> >> > +		    (!strcmp(entry->key, "max_link_speed") ||
>> >> > +		     !strcmp(entry->key, "max_link_width") ||
>> >> > +		     !strcmp(entry->key, "current_link_speed") ||
>> >> > +		     !strcmp(entry->key, "current_link_width")))
>> >> > +			continue;
>> >> > +
>> >> Nit: This might be a bit confusing now that the return value depends on DEVTYPE_DISCRETE,
>> >> especially for a library. I know it’s extra work to keep it generic, but maybe we could
>> >> move the check to its own function just to clean things up a bit?
>> >> 
>> >> 
>> >
>> > OK, so you say it's not clear for someone reading this why the exclusion of 
>> > the fake data from print output is limited to discrete graphics adapter.  
>> > Simply because integrated graphics devices don't provide any fake values, they 
>> > respond with "unknown" which I see no reason to also remove from the output.
>> >
>> > Since I don't understand how moving that piece of code to a separate function 
>> > could make things more clear, I think I'll better provide the missing details 
>> > about acceptable behavior of integrated devices to my commit description and, 
>> > still better, extend the in-line comment above that piece of code with that 
>> > information.  What do you think?
>> Thanks for the clarification. I left it as a nit since I’m fine with
>> the change overall. My concern is that this is a library function, and
>> the update makes it a bit less generic. Changes like this can accumulate
>> over time, but in this case I might be overthinking it.
>
> OK, now I understand better what you could mean by "move the check to its own 
> function".  To keep dumps_props_and_attrs() as generic as possible, I can pass 
> a flag to it which says "skip link bandwitdh attributes", and hand over the 
> decision whether to print them or not to the caller, OK?
I’m okay with it. Looking at the rest of the library, I may have been
overthinking it. This library was never meant to be fully generic, so
if no one else has an issue with this approach, neither do I. Sorry for
the confusion.

-- 
Best regards,
Sebastian