[PATCH i-g-t v2] tools/lsgpu: Add switch to display gpu pci devices

[Zbigniew Kempczyński]

On Fri, Jun 14, 2024 at 05:16:28PM +0200, Kamil Konieczny wrote:
> Hi Zbigniew,
> On 2024-06-05 at 06:53:08 +0200, Zbigniew Kempczyński wrote:
> > Device scanning in IGT is based on iterating over udev drm devices. This
> > limits to display only to devices which have driver loaded.
> > 
> > To remove this limitation add dedicated udev pci scanning in lsgpu which
> > displays all gpu devices (pci class 0x30000).
> 
> This or class 0x38000. 0x30000 catches aspeed display card and others
> but I do not see ATS-M cards, after loading Xe driver:
> 
> sudo build/tools/lsgpu
> card0                    Intel Ats_m (Gen12)               drm:/dev/dri/card0
> └─renderD128                                               drm:/dev/dri/renderD128
> 
> sudo build/tools/lsgpu -pn |grep -i class
> 
> ID_PCI_CLASS_FROM_DATABASE      : Display controller
> ID_PCI_SUBCLASS_FROM_DATABASE   : Display controller
> PCI_CLASS                       : 38000
> class                           : 0x038000
> 
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Jani Nikula <jani.nikula at intel.com>
> > 
> > ---
> > v2: Remove unnecessary variable
> >     Add pci-id and use codename only when is pretty
> > ---
> >  tools/lsgpu.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 87 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/lsgpu.c b/tools/lsgpu.c
> > index da84e20505..23ea8ccdd7 100644
> > --- a/tools/lsgpu.c
> > +++ b/tools/lsgpu.c
> > @@ -30,6 +30,7 @@
> >  #include <string.h>
> >  #include <signal.h>
> >  #include <glib.h>
> > +#include <libudev.h>
> >  
> >  /**
> >   * SECTION:lsgpu
> > @@ -77,12 +78,14 @@ enum {
> >  	OPT_LIST_VENDORS   = 'v',
> >  	OPT_LIST_FILTERS   = 'l',
> >  	OPT_DEVICE         = 'd',
> > -	OPT_HELP           = 'h'
> > +	OPT_HELP           = 'h',
> > +	OPT_PCISCAN        = 'P',
> >  };
> 
> I do not see description of 'P' option after --help?

Ok, I will add it.

> 
> >  
> >  static bool g_show_vendors;
> >  static bool g_list_filters;
> >  static bool g_help;
> > +static bool g_pciscan;
> >  static char *igt_device;
> >  
> >  static const char *usage_str =
> > @@ -158,6 +161,82 @@ static char *get_device_from_rc(void)
> >  	return rc_device;
> >  }
> >  
> > +static int pciscan(void)
> > +{
> > +	struct udev *udev;
> > +	struct udev_enumerate *enumerate;
> > +	struct udev_list_entry *devices, *dev_list_entry;
> > +	struct igt_device_card card;
> > +	char pcistr[10];
> > +	int ret;
> > +
> > +	udev = udev_new();
> > +	igt_assert(udev);
> > +
> > +	enumerate = udev_enumerate_new(udev);
> > +	igt_assert(enumerate);
> > +
> > +	printf("Scanning pci subsystem\n");
> > +	printf("----------------------\n");
> > +	ret = udev_enumerate_add_match_subsystem(enumerate, "pci");
> > +	igt_assert(!ret);
> > +
> > +	ret = udev_enumerate_add_match_property(enumerate, "PCI_CLASS", "30000");
> > +	igt_assert(!ret);
> 
> I am not sure if assert is good here, why not gracefull return?
> Print info and return.

Assert is correct here. That's expected value here on successful call. 

> 
> > +
> > +	ret = udev_enumerate_scan_devices(enumerate);
> > +	igt_assert(!ret);
> 
> Same here.
> 
> > +
> > +	devices = udev_enumerate_get_list_entry(enumerate);
> > +	if (!devices) {
> > +		printf("No pci devices with class 0x30000 found\n");
> 
> Why not make class a param?

According to pci spec we're interested in 0x30000 and 0x38000 at
the moment. I think most users even are not aware of those pci
classes, so I see no reason to add some parameter. If any other
class will be used in the future we can simply add another property
match call.

--
Zbigniew

> 
> Regards,
> Kamil
> 
> > +		return 0;
> > +	}
> > +
> > +	udev_list_entry_foreach(dev_list_entry, devices) {
> > +		const char *path;
> > +		struct udev_device *udev_dev;
> > +		struct udev_list_entry *entry;
> > +		char *codename;
> > +
> > +		path = udev_list_entry_get_name(dev_list_entry);
> > +		udev_dev = udev_device_new_from_syspath(udev, path);
> > +		printf("[%s]\n", path);
> > +
> > +		strcpy(card.pci_slot_name, "-");
> > +		entry = udev_device_get_properties_list_entry(udev_dev);
> > +		while (entry) {
> > +			const char *name = udev_list_entry_get_name(entry);
> > +			const char *value = udev_list_entry_get_value(entry);
> > +
> > +			entry = udev_list_entry_get_next(entry);
> > +			if (!strcmp(name, "ID_VENDOR_FROM_DATABASE"))
> > +				printf("  vendor [db]: %s\n", value);
> > +			else if (!strcmp(name, "ID_MODEL_FROM_DATABASE"))
> > +				printf("  model  [db]: %s\n", value);
> > +			else if (!strcmp(name, "DRIVER"))
> > +				printf("  driver     : %s\n", value);
> > +			else if (!strcmp(name, "PCI_ID"))
> > +				igt_assert_eq(sscanf(value, "%hx:%hx",
> > +						     &card.pci_vendor, &card.pci_device), 2);
> > +		}
> > +		snprintf(pcistr, sizeof(pcistr), "%04x:%04x",
> > +			 card.pci_vendor, card.pci_device);
> > +		printf("  pci id     : %s\n", pcistr);
> > +		codename = igt_device_get_pretty_name(&card, false);
> > +		if (strcmp(pcistr, codename))
> > +			printf("  codename   : %s\n", codename);
> > +		free(codename);
> > +
> > +		udev_device_unref(udev_dev);
> > +	}
> > +
> > +	udev_enumerate_unref(enumerate);
> > +	udev_unref(udev);
> > +
> > +	return 0;
> > +}
> > +
> >  int main(int argc, char *argv[])
> >  {
> >  	static struct option long_options[] = {
> > @@ -180,7 +259,7 @@ int main(int argc, char *argv[])
> >  			.type = IGT_PRINT_USER,
> >  	};
> >  
> > -	while ((c = getopt_long(argc, argv, "ncspvld:h",
> > +	while ((c = getopt_long(argc, argv, "ncspvld:hP",
> >  				long_options, &index)) != -1) {
> >  		switch(c) {
> >  
> > @@ -208,6 +287,9 @@ int main(int argc, char *argv[])
> >  		case OPT_HELP:
> >  			g_help = true;
> >  			break;
> > +		case OPT_PCISCAN:
> > +			g_pciscan = true;
> > +			break;
> >  		case 0:
> >  			fmt.option = IGT_PRINT_DRM;
> >  			break;
> > @@ -220,6 +302,9 @@ int main(int argc, char *argv[])
> >  		}
> >  	}
> >  
> > +	if (g_pciscan)
> > +		return pciscan();
> > +
> >  	if (g_help) {
> >  		printf("%s\n", usage_str);
> >  		exit(0);
> > -- 
> > 2.34.1
> >