Re: [PATCH i-g-t v4] tools/mk_detect_intel_gpu: add a tool to detect Intel GPUs from their PCI IDs

[Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>]

On Wed, 22 May 2024 16:46:27 +0200
Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:

> Hi Jani,
> On 2024-05-22 at 16:26:44 +0300, Jani Nikula wrote:
> > On Wed, 22 May 2024, Jani Nikula <jani.nikula@intel.com> wrote:  
> > > On Wed, 22 May 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:  
> > >> From: Mauro Carvalho Chehab <mchehab@kernel.org>
> > >>
> > >> Such tool parses the Kernel drivers for both i915 and Xe and
> > >> generates a script that helps detecting Intel GPU models.  
> > >
> > > I acknowledge the usefulness of such a tool, but to be brutally honest,
> > > this implementation is horrible in so many levels.
> > >
> > > IGT uses meson and avoids perl.  
> 
> imho this could also be a python script.

Converting the output to a python script should be trivial and can be done
later on. The generated script has a a simple hash/dict lookup logic.

> > >
> > > This has a makefile with perl to generate a header that's included in a
> > > C program that is built and executed to generate a perl script that's
> > > then the tool, and the generated perl script runs lspci(8).
> > >
> > > I think there's really only one reasonable way to implement this in the
> > > IGT context, and that's pure C, leveraging all the stuff in IGT, using
> > > the PCI IDs listed in the IGT repo.
> > >
> > > Or you can turn this into a separate pet project, because what you have
> > > here does not fit IGT.
> > >
> > > The i915_pciids.h parsing below is already stale, and you really can't
> > > expect to get this merged and be kept up-to-date by folks updating
> > > i915_pciids.h. IMO it's not maintainable.  
> 
> This is for making a new script, after generation there will be
> standalone program: intel_detect_gpu.pl and imho igt is missing
> a tool like lsgpu which will work _without_ kmd gpu driver loaded,
> simple enough so one can just run it.
> 
> > 
> > An alternative is a pure bash or Python script to run something like:
> > 
> > 	lspci -d 8086::0300 -nn
> >   
> 
> Looks like a good idea but this is also dependancy, btw
> from man:
> -nn    Show PCI vendor and device codes as both numbers and names.

The names from lspci database aren't read from ACPI. They come from
a pci.ids table that it is updated by this script:

	https://github.com/pciutils/pciutils/blob/master/update-pciids.sh

that is executed at lspci build time[1]. 

From an user-maintained database at:

	https://pci-ids.ucw.cz/

According to pci-ids:

	"Submit new data

	 The database is maintained by volunteers like you, so if you have
	 any devices which are not identified properly, please help us by 
	 adding them to the database or by fixing the existing entry."

So, the names reported by lspci depend on:
	1. someone sending updates to such database;
	2. having the contribution updated by pci-ids maintainers;
	3. having the pci id dataset updated at the running system.

It works fine when one is using a brand new distro, with an older
hardware, but on LTS distros and new hardware, it is unlikely that
lspci will use a pci.ids file with the vendor ID for new GPUs.

On the other hand, the driver source code needs to maintain a list of
the supported PCI IDs, together with some names associated to it.

Based on the information inside the driver, the generated script
will associate a vendor ID to its GPU model as written inside the
driver:

	$ ./intel_detect_gpu.pl 
	Detected GPU PCI device: CFL (CML_U_GT2), PCI ID: 8086:9b41

This works not only for old enough GPUs to be there at the pci.ids
filled shipped at the distro, but also for brand new ones - and even
for not-yet-released ones that might be there at drm-next or on 
internal development trees.

[1] Ok, one might run update-pciids later on to update it manually, 
    but most people aren't aware of that - and it will still require
    someone to manually add the new PCI IDs to the project. 

> 
> imho we should not depend on names from lspci.
> More from man:
> 
> -n     Show PCI vendor and device codes as numbers instead of looking them up in the PCI ID list.
> -m     Dump PCI device data in a backward-compatible machine readable form.
> 
> > and annotate/filter the results based on the PCI IDs from:
> > 
> > 	modinfo -F alias i915
> > 	modinfo -F alias xe  
> 
> But this one is creating one more dependancy, one should have
> modules compiled in current running kernel.
> 
> Regards,
> Kamil
> 
> > 
> > You get the fancy marketing names directly from lspci, supported kernel
> > modules from modinfo, available on the system, with zero dependency on
> > PCI ID macros or names in source code.
> > 
> > That's like a 100 lines of script that should need virtually zero
> > maintenance.
> > 
> > Additionally, the script could match the modinfo PCI IDs against
> > /usr/share/misc/pci.ids (see man pci.ds) to show the marketing names.
> > 
> > 
> > BR,
> > Jani.
> > 
> > -- 
> > Jani Nikula, Intel