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: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.

-- 
Best regards,
Sebastian