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

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 tools/mk_detect_intel_gpu/Makefile  |  57 ++++++++
 tools/mk_detect_intel_gpu/get_gpu.c | 216 ++++++++++++++++++++++++++++
 2 files changed, 273 insertions(+)
 create mode 100644 tools/mk_detect_intel_gpu/Makefile
 create mode 100644 tools/mk_detect_intel_gpu/get_gpu.c

diff --git a/tools/mk_detect_intel_gpu/Makefile b/tools/mk_detect_intel_gpu/Makefile
new file mode 100644
index 000000000000..ea24307ce19c
--- /dev/null
+++ b/tools/mk_detect_intel_gpu/Makefile
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: MIT
+#
+# Copyright © 2022-2024 Intel Corporation
+# Author: Mauro Carvalho Chehab <mchehab@kernel.org>
+#
+# Usage: specify either:
+#  make KERNEL_DIR=<directory to copy files for both i915 and Xe Intel drivers>
+# or:
+#  make XE_DIR=<directory to copy files for Xe Intel driver> I915_DIR=<directory to copy files for i915 Intel driver>
+
+XE_DIR :=
+I915_DIR :=
+
+all: intel_detect_gpu.pl
+
+clean:
+	rm i915_pciids.h i915_id.h xe_pciids.h xe_pci.c get_gpu intel_detect_gpu.pl
+
+ifneq ($(KERNEL_DIR),)
+  XE_DIR := $(KERNEL_DIR)
+  I915_DIR := $(KERNEL_DIR)
+endif
+
+ifeq ($(XE_DIR),)
+  $(error Need a directory with the XE driver)
+endif
+
+ifeq ($(I915_DIR),)
+  $(error Need a directory with the i915 driver)
+endif
+
+intel_detect_gpu.pl: get_gpu
+	./get_gpu >intel_detect_gpu.pl
+	chmod 755 intel_detect_gpu.pl
+
+i915_pciids.h:
+	cp ${I915_DIR}/include/drm/i915_pciids.h .
+	sed s,"(unsigned long) info }","info }", -i i915_pciids.h
+
+xe_pci_ids.h:
+	cp ${XE_DIR}/include/drm/xe_pciids.h .
+
+xe_pci.c:
+	cp ${XE_DIR}/drivers/gpu/drm/xe/xe_pci.c .
+
+i915_id.h: i915_pciids.h xe_pci.c
+	echo "static struct pci_device_id pci_ids[] = {" > i915_id.h
+	tac i915_pciids.h |grep define|perl -ne 'if (/define\s+INTEL_(\w+)_IDS/) { print "\tINTEL_$$1_IDS(\"$$1\"),\n" }' >> i915_id.h
+
+	cat xe_pci.c|perl -ne 'if (m/(XE_)([\w_]+)(_IDS\(INTEL_VGA_DEVICE)/) { print "\t$$1$$2$$3, \"$$2\"),\n"}' >> i915_id.h
+
+	echo "};" >> i915_id.h
+
+get_gpu: get_gpu.c i915_id.h i915_pciids.h xe_pci_ids.h
+	gcc -g -Wall get_gpu.c -o get_gpu
+
+.PHONY: i915_id.h i915_pciids.h
diff --git a/tools/mk_detect_intel_gpu/get_gpu.c b/tools/mk_detect_intel_gpu/get_gpu.c
new file mode 100644
index 000000000000..44fee08d398b
--- /dev/null
+++ b/tools/mk_detect_intel_gpu/get_gpu.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022-2024 Intel Corporation
+ * Author: Mauro Carvalho Chehab <mchehab@kernel.org>
+ */
+
+
+#include "i915_pciids.h"
+#include "xe_pciids.h"
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+struct pci_device_id {
+	unsigned int vendor, device;
+	unsigned int subvendor, subdevice;
+	unsigned int class, class_mask;
+	char *name;
+};
+
+#include "i915_id.h"
+
+#define ARRAY_SIZE(arr) sizeof(arr)/sizeof(*arr)
+
+#define TGL "TGL_12"
+
+int cmpfunc (const void *_a, const void *_b) {
+        int ret;
+
+        const struct pci_device_id *a = _a;
+        const struct pci_device_id *b = _b;
+
+        ret = strcmp(a->name, b->name);
+        if (ret)
+                return ret;
+
+        ret = a->vendor - b->vendor;
+        if (ret)
+                return ret;
+
+        ret = a->device - b->device;
+        if (ret)
+                return ret;
+
+        ret = a->subvendor - b->subvendor;
+        if (ret)
+                return ret;
+
+        ret = a->subdevice - b->subdevice;
+        if (ret)
+                return ret;
+
+        ret = a->class - b->class;
+        if (ret)
+                return ret;
+
+        ret = a->class_mask - b->class_mask;
+
+        return ret;
+}
+
+
+int main(void)
+{
+	int i;
+	int shown[0x10000] = { [0 ... 0xffff] = -1 };
+	int ignore[ARRAY_SIZE(pci_ids)] = { 0 };
+	int other[ARRAY_SIZE(pci_ids)] = { [0 ... ARRAY_SIZE(pci_ids) - 1] = -1 };
+	char last[80] = "";
+
+	printf("#!/usr/bin/perl\n\n");
+	printf("$ENV{'LC_ALL'} = 'C';\n\n");
+
+        qsort(pci_ids, ARRAY_SIZE(pci_ids), sizeof(*pci_ids), cmpfunc);
+
+	/*
+	 * Step 1: Handle non-subvendor specific devices, discovering duplicates
+	 */
+	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
+		if (pci_ids[i].subvendor != (unsigned int)-1)
+			continue;
+
+		if (!strncmp(pci_ids[i].name, TGL, strlen(TGL))) {
+			char *tmp = strdup(pci_ids[i].name);
+			pci_ids[i].name = tmp;
+			strcpy(pci_ids[i].name + 3, tmp + strlen(TGL));
+		}
+
+		if (shown[pci_ids[i].device] >= 0) {
+			ignore[i] = i;
+
+                        // If it is duplicated, just ignore it
+                        if (!cmpfunc(&pci_ids[i], &pci_ids[shown[pci_ids[i].device]]))
+                                continue;
+
+			fprintf(stderr, "PCI ID: %04x:%04x %s is a subtype of %s\n",
+			        pci_ids[i].vendor, pci_ids[i].device,
+			        pci_ids[i].name,
+				pci_ids[shown[pci_ids[i].device]].name);
+
+			other[shown[pci_ids[i].device]] = i;
+			continue;
+		}
+		shown[pci_ids[i].device] = i;
+
+		if (pci_ids[i].device == 0x56c0)
+			pci_ids[i].name = "ATS_M1";
+
+		if (pci_ids[i].device == 0x56c1)
+			pci_ids[i].name = "ATS_M3";
+
+		fprintf(stderr, "Adding: %s: vendor: %04x:%04x\n",
+		        pci_ids[i].name,
+		        pci_ids[i].vendor, pci_ids[i].device);
+	}
+
+	/*
+	 * Step 2: output array with all other devices
+	 */
+	printf("my %%intel_pci_id = (");
+
+	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
+		if (ignore[i] > 0)
+			continue;
+
+		if (strcmp(last, pci_ids[i].name)) {
+			strcpy(last, pci_ids[i].name);
+			printf("\n");
+		}
+
+		printf("\t\"%04x:%04x\" => \"%s\",\n",
+		       pci_ids[i].vendor, pci_ids[i].device,
+		       pci_ids[i].name
+		);
+	}
+	printf(");\n\n");
+
+	/*
+	 * Step 3: output array with all other devices
+	 */
+	printf("my %%intel_pci_id_alt_name = (");
+
+	last[0] = '\0';
+	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
+		if (ignore[i] > 0)
+			continue;
+
+		if (other[i] < 0)
+			continue;
+
+		if (strcmp(last, pci_ids[i].name)) {
+			strcpy(last, pci_ids[i].name);
+			printf("\n\t# %s alternative names\n", pci_ids[i].name);
+		}
+
+		printf("\t\"%04x:%04x\" => \"%s\",\n",
+		       pci_ids[i].vendor, pci_ids[i].device,
+		       pci_ids[other[i]].name
+		);
+	}
+	printf(");\n\n");
+
+	/*
+	 * Step 4: output array for devices with subvendor/subdevice
+	 */
+	printf("my %%intel_pci_subvendor_id = (\n");
+
+	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
+		if (pci_ids[i].subvendor == (unsigned int) -1)
+			continue;
+
+		fprintf(stderr, "Adding: %s: vendor: %04x:%04x, subvendor: %04x:%04x\n",
+		        pci_ids[i].name,
+		        pci_ids[i].vendor, pci_ids[i].device,
+		        pci_ids[i].subvendor, pci_ids[i].subdevice);
+
+		printf("\t\"%04x:%04x %04x:%04x\" => \"%s\",\n",
+		       pci_ids[i].vendor, pci_ids[i].device,
+		       pci_ids[i].subvendor, pci_ids[i].subdevice,
+		       pci_ids[i].name
+		);
+
+		ignore[i] = 1;
+	}
+	printf(");\n\n");
+
+	printf("sub fix_id($) {\n");
+	printf("\tmy $id = shift;\n");
+	printf("\t$id = $intel_pci_id{$id} if defined($intel_pci_id{$id});\n");
+	printf("\treturn $id;\n}\n\n");
+	printf("my @gpus;\n");
+	printf("my $id;\n\n");
+	printf("open IN, \"lspci -nm|\";\n");
+	printf("while (<IN>) {\n");
+	printf("\tif (m/\"(8086)\"\\s\"([\\da-f]+)\".*\"([^\\\"]*)\"\\s\"([^\\\"]*\")/) {\n");
+	printf("\t\tmy $pci_id_sub = \"$1:$2 $3:$4\";\n");
+	printf("\t\tmy $pci_id = \"$1:$2\";\n");
+	printf("\t\tif (defined($intel_pci_subvendor_id{$pci_id_sub})) {\n");
+	printf("\t\t\t$id = $intel_pci_subvendor_id{$pci_id};\n");
+	printf("\t\t} elsif (defined($intel_pci_id{$pci_id})) {\n");
+	printf("\t\t\t$id = $intel_pci_id{$pci_id};\n");
+	printf("\t\t}\n");
+	printf("\t\tif ($id) {\n");
+	printf("\t\t\tmy $subtype = \"\";\n");
+	printf("\t\t\t$subtype = \" (\" . $intel_pci_id_alt_name{$pci_id} .\")\" if defined($intel_pci_id_alt_name{$pci_id});\n");
+	printf("\t\t\tprint \"Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\\n\";\n");
+	printf("\t\t\tundef($id);\n");
+	printf("\t\t\tpush @gpus, $id;\n\t\t}\n\t}\n}\n");
+	printf("close IN;\n\n");
+	printf("if (@gpus == 0) {\n");
+	printf("\tprint STDERR \"Warning: No Intel GPUs detected.\\n\";\n");
+	printf("} elsif (@gpus > 1) {\n");
+	printf("\tprint STDERR \"Warning: More than one Intel GPUs detected.\\n\";\n");
+	printf("}\n");
+}
-- 
2.44.0

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

[Jani Nikula]

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.

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.


BR,
Jani.


>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> ---
>  tools/mk_detect_intel_gpu/Makefile  |  57 ++++++++
>  tools/mk_detect_intel_gpu/get_gpu.c | 216 ++++++++++++++++++++++++++++
>  2 files changed, 273 insertions(+)
>  create mode 100644 tools/mk_detect_intel_gpu/Makefile
>  create mode 100644 tools/mk_detect_intel_gpu/get_gpu.c
>
> diff --git a/tools/mk_detect_intel_gpu/Makefile b/tools/mk_detect_intel_gpu/Makefile
> new file mode 100644
> index 000000000000..ea24307ce19c
> --- /dev/null
> +++ b/tools/mk_detect_intel_gpu/Makefile
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: MIT
> +#
> +# Copyright © 2022-2024 Intel Corporation
> +# Author: Mauro Carvalho Chehab <mchehab@kernel.org>
> +#
> +# Usage: specify either:
> +#  make KERNEL_DIR=<directory to copy files for both i915 and Xe Intel drivers>
> +# or:
> +#  make XE_DIR=<directory to copy files for Xe Intel driver> I915_DIR=<directory to copy files for i915 Intel driver>
> +
> +XE_DIR :=
> +I915_DIR :=
> +
> +all: intel_detect_gpu.pl
> +
> +clean:
> +	rm i915_pciids.h i915_id.h xe_pciids.h xe_pci.c get_gpu intel_detect_gpu.pl
> +
> +ifneq ($(KERNEL_DIR),)
> +  XE_DIR := $(KERNEL_DIR)
> +  I915_DIR := $(KERNEL_DIR)
> +endif
> +
> +ifeq ($(XE_DIR),)
> +  $(error Need a directory with the XE driver)
> +endif
> +
> +ifeq ($(I915_DIR),)
> +  $(error Need a directory with the i915 driver)
> +endif
> +
> +intel_detect_gpu.pl: get_gpu
> +	./get_gpu >intel_detect_gpu.pl
> +	chmod 755 intel_detect_gpu.pl
> +
> +i915_pciids.h:
> +	cp ${I915_DIR}/include/drm/i915_pciids.h .
> +	sed s,"(unsigned long) info }","info }", -i i915_pciids.h
> +
> +xe_pci_ids.h:
> +	cp ${XE_DIR}/include/drm/xe_pciids.h .
> +
> +xe_pci.c:
> +	cp ${XE_DIR}/drivers/gpu/drm/xe/xe_pci.c .
> +
> +i915_id.h: i915_pciids.h xe_pci.c
> +	echo "static struct pci_device_id pci_ids[] = {" > i915_id.h
> +	tac i915_pciids.h |grep define|perl -ne 'if (/define\s+INTEL_(\w+)_IDS/) { print "\tINTEL_$$1_IDS(\"$$1\"),\n" }' >> i915_id.h
> +
> +	cat xe_pci.c|perl -ne 'if (m/(XE_)([\w_]+)(_IDS\(INTEL_VGA_DEVICE)/) { print "\t$$1$$2$$3, \"$$2\"),\n"}' >> i915_id.h
> +
> +	echo "};" >> i915_id.h
> +
> +get_gpu: get_gpu.c i915_id.h i915_pciids.h xe_pci_ids.h
> +	gcc -g -Wall get_gpu.c -o get_gpu
> +
> +.PHONY: i915_id.h i915_pciids.h
> diff --git a/tools/mk_detect_intel_gpu/get_gpu.c b/tools/mk_detect_intel_gpu/get_gpu.c
> new file mode 100644
> index 000000000000..44fee08d398b
> --- /dev/null
> +++ b/tools/mk_detect_intel_gpu/get_gpu.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022-2024 Intel Corporation
> + * Author: Mauro Carvalho Chehab <mchehab@kernel.org>
> + */
> +
> +
> +#include "i915_pciids.h"
> +#include "xe_pciids.h"
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +struct pci_device_id {
> +	unsigned int vendor, device;
> +	unsigned int subvendor, subdevice;
> +	unsigned int class, class_mask;
> +	char *name;
> +};
> +
> +#include "i915_id.h"
> +
> +#define ARRAY_SIZE(arr) sizeof(arr)/sizeof(*arr)
> +
> +#define TGL "TGL_12"
> +
> +int cmpfunc (const void *_a, const void *_b) {
> +        int ret;
> +
> +        const struct pci_device_id *a = _a;
> +        const struct pci_device_id *b = _b;
> +
> +        ret = strcmp(a->name, b->name);
> +        if (ret)
> +                return ret;
> +
> +        ret = a->vendor - b->vendor;
> +        if (ret)
> +                return ret;
> +
> +        ret = a->device - b->device;
> +        if (ret)
> +                return ret;
> +
> +        ret = a->subvendor - b->subvendor;
> +        if (ret)
> +                return ret;
> +
> +        ret = a->subdevice - b->subdevice;
> +        if (ret)
> +                return ret;
> +
> +        ret = a->class - b->class;
> +        if (ret)
> +                return ret;
> +
> +        ret = a->class_mask - b->class_mask;
> +
> +        return ret;
> +}
> +
> +
> +int main(void)
> +{
> +	int i;
> +	int shown[0x10000] = { [0 ... 0xffff] = -1 };
> +	int ignore[ARRAY_SIZE(pci_ids)] = { 0 };
> +	int other[ARRAY_SIZE(pci_ids)] = { [0 ... ARRAY_SIZE(pci_ids) - 1] = -1 };
> +	char last[80] = "";
> +
> +	printf("#!/usr/bin/perl\n\n");
> +	printf("$ENV{'LC_ALL'} = 'C';\n\n");
> +
> +        qsort(pci_ids, ARRAY_SIZE(pci_ids), sizeof(*pci_ids), cmpfunc);
> +
> +	/*
> +	 * Step 1: Handle non-subvendor specific devices, discovering duplicates
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
> +		if (pci_ids[i].subvendor != (unsigned int)-1)
> +			continue;
> +
> +		if (!strncmp(pci_ids[i].name, TGL, strlen(TGL))) {
> +			char *tmp = strdup(pci_ids[i].name);
> +			pci_ids[i].name = tmp;
> +			strcpy(pci_ids[i].name + 3, tmp + strlen(TGL));
> +		}
> +
> +		if (shown[pci_ids[i].device] >= 0) {
> +			ignore[i] = i;
> +
> +                        // If it is duplicated, just ignore it
> +                        if (!cmpfunc(&pci_ids[i], &pci_ids[shown[pci_ids[i].device]]))
> +                                continue;
> +
> +			fprintf(stderr, "PCI ID: %04x:%04x %s is a subtype of %s\n",
> +			        pci_ids[i].vendor, pci_ids[i].device,
> +			        pci_ids[i].name,
> +				pci_ids[shown[pci_ids[i].device]].name);
> +
> +			other[shown[pci_ids[i].device]] = i;
> +			continue;
> +		}
> +		shown[pci_ids[i].device] = i;
> +
> +		if (pci_ids[i].device == 0x56c0)
> +			pci_ids[i].name = "ATS_M1";
> +
> +		if (pci_ids[i].device == 0x56c1)
> +			pci_ids[i].name = "ATS_M3";
> +
> +		fprintf(stderr, "Adding: %s: vendor: %04x:%04x\n",
> +		        pci_ids[i].name,
> +		        pci_ids[i].vendor, pci_ids[i].device);
> +	}
> +
> +	/*
> +	 * Step 2: output array with all other devices
> +	 */
> +	printf("my %%intel_pci_id = (");
> +
> +	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
> +		if (ignore[i] > 0)
> +			continue;
> +
> +		if (strcmp(last, pci_ids[i].name)) {
> +			strcpy(last, pci_ids[i].name);
> +			printf("\n");
> +		}
> +
> +		printf("\t\"%04x:%04x\" => \"%s\",\n",
> +		       pci_ids[i].vendor, pci_ids[i].device,
> +		       pci_ids[i].name
> +		);
> +	}
> +	printf(");\n\n");
> +
> +	/*
> +	 * Step 3: output array with all other devices
> +	 */
> +	printf("my %%intel_pci_id_alt_name = (");
> +
> +	last[0] = '\0';
> +	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
> +		if (ignore[i] > 0)
> +			continue;
> +
> +		if (other[i] < 0)
> +			continue;
> +
> +		if (strcmp(last, pci_ids[i].name)) {
> +			strcpy(last, pci_ids[i].name);
> +			printf("\n\t# %s alternative names\n", pci_ids[i].name);
> +		}
> +
> +		printf("\t\"%04x:%04x\" => \"%s\",\n",
> +		       pci_ids[i].vendor, pci_ids[i].device,
> +		       pci_ids[other[i]].name
> +		);
> +	}
> +	printf(");\n\n");
> +
> +	/*
> +	 * Step 4: output array for devices with subvendor/subdevice
> +	 */
> +	printf("my %%intel_pci_subvendor_id = (\n");
> +
> +	for (i = 0; i < ARRAY_SIZE(pci_ids); i++) {
> +		if (pci_ids[i].subvendor == (unsigned int) -1)
> +			continue;
> +
> +		fprintf(stderr, "Adding: %s: vendor: %04x:%04x, subvendor: %04x:%04x\n",
> +		        pci_ids[i].name,
> +		        pci_ids[i].vendor, pci_ids[i].device,
> +		        pci_ids[i].subvendor, pci_ids[i].subdevice);
> +
> +		printf("\t\"%04x:%04x %04x:%04x\" => \"%s\",\n",
> +		       pci_ids[i].vendor, pci_ids[i].device,
> +		       pci_ids[i].subvendor, pci_ids[i].subdevice,
> +		       pci_ids[i].name
> +		);
> +
> +		ignore[i] = 1;
> +	}
> +	printf(");\n\n");
> +
> +	printf("sub fix_id($) {\n");
> +	printf("\tmy $id = shift;\n");
> +	printf("\t$id = $intel_pci_id{$id} if defined($intel_pci_id{$id});\n");
> +	printf("\treturn $id;\n}\n\n");
> +	printf("my @gpus;\n");
> +	printf("my $id;\n\n");
> +	printf("open IN, \"lspci -nm|\";\n");
> +	printf("while (<IN>) {\n");
> +	printf("\tif (m/\"(8086)\"\\s\"([\\da-f]+)\".*\"([^\\\"]*)\"\\s\"([^\\\"]*\")/) {\n");
> +	printf("\t\tmy $pci_id_sub = \"$1:$2 $3:$4\";\n");
> +	printf("\t\tmy $pci_id = \"$1:$2\";\n");
> +	printf("\t\tif (defined($intel_pci_subvendor_id{$pci_id_sub})) {\n");
> +	printf("\t\t\t$id = $intel_pci_subvendor_id{$pci_id};\n");
> +	printf("\t\t} elsif (defined($intel_pci_id{$pci_id})) {\n");
> +	printf("\t\t\t$id = $intel_pci_id{$pci_id};\n");
> +	printf("\t\t}\n");
> +	printf("\t\tif ($id) {\n");
> +	printf("\t\t\tmy $subtype = \"\";\n");
> +	printf("\t\t\t$subtype = \" (\" . $intel_pci_id_alt_name{$pci_id} .\")\" if defined($intel_pci_id_alt_name{$pci_id});\n");
> +	printf("\t\t\tprint \"Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\\n\";\n");
> +	printf("\t\t\tundef($id);\n");
> +	printf("\t\t\tpush @gpus, $id;\n\t\t}\n\t}\n}\n");
> +	printf("close IN;\n\n");
> +	printf("if (@gpus == 0) {\n");
> +	printf("\tprint STDERR \"Warning: No Intel GPUs detected.\\n\";\n");
> +	printf("} elsif (@gpus > 1) {\n");
> +	printf("\tprint STDERR \"Warning: More than one Intel GPUs detected.\\n\";\n");
> +	printf("}\n");
> +}

-- 
Jani Nikula, Intel

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

[Jani Nikula]

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

An alternative is a pure bash or Python script to run something like:

	lspci -d 8086::0300 -nn

and annotate/filter the results based on the PCI IDs from:

	modinfo -F alias i915
	modinfo -F alias xe

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

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

[Kamil Konieczny]

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.

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

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

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]

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  

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

[Jani Nikula]

On Thu, 23 May 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> 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.

It should not be a generated script in the first place. It should not
grep the kernel or igt sources for certain patterns which will
change. It should not have to be this complicated.

If it's C, it should be a regular build (via meson). If it's Python, it
should do all it needs to do when running it.


BR,
Jani.


-- 
Jani Nikula, Intel

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

[Jani Nikula]

On Wed, 22 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> Looks like a good idea but this is also dependancy, btw

[snip]

> But this one is creating one more dependancy, one should have
> modules compiled in current running kernel.

Reading the comments about dependencies, I think this needs a different
perspective.

What is the target audience of the tool? What are they expected to have
around? What is the easiest for them to install?

lspci and modinfo are trivial to install, and most people have them
installed already. modinfo does not require the modules to be probed,
you can also point it at the .ko under /usr/lib/modules. For a user,
this gives information about the modules they actually have on their
system, which may be different from kernel or igt sources.

OTOH most people won't have kernel or igt sources available. (Let alone
specific versions of the source, which match the expectations of the
patch at hand.) Indeed, you can install igt via the distro package
manager, with no need to check out the sources.


BR,
Jani.


-- 
Jani Nikula, Intel

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]

On Thu, 23 May 2024 10:30:37 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> On Wed, 22 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> > Looks like a good idea but this is also dependancy, btw  
> 
> [snip]
> 
> > But this one is creating one more dependancy, one should have
> > modules compiled in current running kernel.  
> 
> Reading the comments about dependencies, I think this needs a different
> perspective.
> 
> What is the target audience of the tool? What are they expected to have
> around? What is the easiest for them to install?

The target audience for the submitted changeset is to provide a way for
IGT developers to generate a script to detect the presence and the GPU
model for Device Under Test (DUT) machines.

It was written to be generic enough to use different directories for Xe
and for i915 (or the same one). So, it should work properly when different
versions of the drivers would be used, during driver's development.

On other words, the way it is, this is not focused on the end user. 

The way it works is that a C file read the i915 and Xe macros, creating
a PCI ID database, stored as hash(dict):

	my %intel_pci_id = (
		"8086:46d0" => "ADLN",
		"8086:46d1" => "ADLN",
		"8086:46d2" => "ADLN",
		...
	)
	...
	my %intel_pci_id_alt_name = (
		# AML_CFL_GT2 alternative names
		"8086:87ca" => "CFL",

		# AML_KBL_GT2 alternative names
		"8086:591c" => "KBL",
		"8086:87c0" => "KBL",
		...
	)

	Typically, most GPUs have two entries:

	$ grep 8086:9b41 ./intel_detect_gpu.pl
		"8086:9b41" => "CFL",
		"8086:9b41" => "CML_U_GT2",

	The first one is the generic name, the second one contains
	an alternate name, with has additional GPU version details
	(in this case, CML-U GT2). All of that obtained from the
	driver's source code.

The generated script itself doesn't require anything other than lspci to
run, as what the script does, on its 35 lines of code (not including the
PCI ID database), is:

	1. run lspci -nm

	2. If the PCI ID device is at the database, it prints:

		print "Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\n";

	   For the above example, the output is:

		Detected GPU PCI device: CFL (CML_U_GT2), PCI ID: 8086:9b41

	3. If multiple Intel GPUs are found, it prints a warning, as DUT
	   tests may require an extra parameter to use the right GPU:

		Warning: More than one Intel GPUs detected

	4. if no Intel GPU is found, it will print:

		Warning: No Intel GPUs detected

> lspci and modinfo are trivial to install, and most people have them
> installed already. modinfo does not require the modules to be probed,
> you can also point it at the .ko under /usr/lib/modules. For a user,
> this gives information about the modules they actually have on their
> system, which may be different from kernel or igt sources.

Once generated, intel_detect_gpu.pl would be trivial to install and
to be used, as the only requirements are to have perl and pci-tools
(due to lspci dependency) installed.

> OTOH most people won't have kernel or igt sources available. (Let alone
> specific versions of the source, which match the expectations of the
> patch at hand.) Indeed, you can install igt via the distro package
> manager, with no need to check out the sources.

If IGT maintainers think it is worth targeting end users in the future,
the best is for them to periodically generate the script (for example,
when releasing a new IGT version), adding it to scripts/ and adding a
target at Meson to install it under ${installprefix}/bin directory.

Regards,
Mauro

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

[Jani Nikula]

On Thu, 23 May 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> On Thu, 23 May 2024 10:30:37 +0300
> Jani Nikula <jani.nikula@intel.com> wrote:
>
>> On Wed, 22 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
>> > Looks like a good idea but this is also dependancy, btw  
>> 
>> [snip]
>> 
>> > But this one is creating one more dependancy, one should have
>> > modules compiled in current running kernel.  
>> 
>> Reading the comments about dependencies, I think this needs a different
>> perspective.
>> 
>> What is the target audience of the tool? What are they expected to have
>> around? What is the easiest for them to install?
>
> The target audience for the submitted changeset is to provide a way for
> IGT developers to generate a script to detect the presence and the GPU
> model for Device Under Test (DUT) machines.
>
> It was written to be generic enough to use different directories for Xe
> and for i915 (or the same one). So, it should work properly when different
> versions of the drivers would be used, during driver's development.
>
> On other words, the way it is, this is not focused on the end user. 
>
> The way it works is that a C file read the i915 and Xe macros, creating
> a PCI ID database, stored as hash(dict):
>
> 	my %intel_pci_id = (
> 		"8086:46d0" => "ADLN",
> 		"8086:46d1" => "ADLN",
> 		"8086:46d2" => "ADLN",
> 		...
> 	)
> 	...
> 	my %intel_pci_id_alt_name = (
> 		# AML_CFL_GT2 alternative names
> 		"8086:87ca" => "CFL",
>
> 		# AML_KBL_GT2 alternative names
> 		"8086:591c" => "KBL",
> 		"8086:87c0" => "KBL",
> 		...
> 	)
>
> 	Typically, most GPUs have two entries:
>
> 	$ grep 8086:9b41 ./intel_detect_gpu.pl
> 		"8086:9b41" => "CFL",
> 		"8086:9b41" => "CML_U_GT2",
>
> 	The first one is the generic name, the second one contains
> 	an alternate name, with has additional GPU version details
> 	(in this case, CML-U GT2). All of that obtained from the
> 	driver's source code.
>
> The generated script itself doesn't require anything other than lspci to
> run, as what the script does, on its 35 lines of code (not including the
> PCI ID database), is:
>
> 	1. run lspci -nm
>
> 	2. If the PCI ID device is at the database, it prints:
>
> 		print "Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\n";
>
> 	   For the above example, the output is:
>
> 		Detected GPU PCI device: CFL (CML_U_GT2), PCI ID: 8086:9b41
>
> 	3. If multiple Intel GPUs are found, it prints a warning, as DUT
> 	   tests may require an extra parameter to use the right GPU:
>
> 		Warning: More than one Intel GPUs detected
>
> 	4. if no Intel GPU is found, it will print:
>
> 		Warning: No Intel GPUs detected

The way I see it, intel_device_info.c and igt_device_scan.c have all the
building blocks necessary to accomplish this. As long as
intel_device_match[] is kept up-to-date, which naturally will be, it
needs no further maintenance. It could be added to lsgpu as an option.

>> lspci and modinfo are trivial to install, and most people have them
>> installed already. modinfo does not require the modules to be probed,
>> you can also point it at the .ko under /usr/lib/modules. For a user,
>> this gives information about the modules they actually have on their
>> system, which may be different from kernel or igt sources.
>
> Once generated, intel_detect_gpu.pl would be trivial to install and
> to be used, as the only requirements are to have perl and pci-tools
> (due to lspci dependency) installed.
>
>> OTOH most people won't have kernel or igt sources available. (Let alone
>> specific versions of the source, which match the expectations of the
>> patch at hand.) Indeed, you can install igt via the distro package
>> manager, with no need to check out the sources.
>
> If IGT maintainers think it is worth targeting end users in the future,
> the best is for them to periodically generate the script (for example,
> when releasing a new IGT version), adding it to scripts/ and adding a
> target at Meson to install it under ${installprefix}/bin directory.

The best is what I described above, not adding Rube Goldberg machines
that require constant attention.

BR,
Jani.


>
> Regards,
> Mauro

-- 
Jani Nikula, Intel

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

[Kamil Konieczny]

Hi Jani,
On 2024-05-23 at 12:10:05 +0300, Jani Nikula wrote:
> On Thu, 23 May 2024, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> > On Thu, 23 May 2024 10:30:37 +0300
> > Jani Nikula <jani.nikula@intel.com> wrote:
> >
> >> On Wed, 22 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> >> > Looks like a good idea but this is also dependancy, btw  
> >> 
> >> [snip]
> >> 
> >> > But this one is creating one more dependancy, one should have
> >> > modules compiled in current running kernel.  
> >> 
> >> Reading the comments about dependencies, I think this needs a different
> >> perspective.
> >> 
> >> What is the target audience of the tool? What are they expected to have
> >> around? What is the easiest for them to install?
> >
> > The target audience for the submitted changeset is to provide a way for
> > IGT developers to generate a script to detect the presence and the GPU
> > model for Device Under Test (DUT) machines.
> >
> > It was written to be generic enough to use different directories for Xe
> > and for i915 (or the same one). So, it should work properly when different
> > versions of the drivers would be used, during driver's development.
> >
> > On other words, the way it is, this is not focused on the end user. 
> >
> > The way it works is that a C file read the i915 and Xe macros, creating
> > a PCI ID database, stored as hash(dict):
> >
> > 	my %intel_pci_id = (
> > 		"8086:46d0" => "ADLN",
> > 		"8086:46d1" => "ADLN",
> > 		"8086:46d2" => "ADLN",
> > 		...
> > 	)
> > 	...
> > 	my %intel_pci_id_alt_name = (
> > 		# AML_CFL_GT2 alternative names
> > 		"8086:87ca" => "CFL",
> >
> > 		# AML_KBL_GT2 alternative names
> > 		"8086:591c" => "KBL",
> > 		"8086:87c0" => "KBL",
> > 		...
> > 	)
> >
> > 	Typically, most GPUs have two entries:
> >
> > 	$ grep 8086:9b41 ./intel_detect_gpu.pl
> > 		"8086:9b41" => "CFL",
> > 		"8086:9b41" => "CML_U_GT2",
> >
> > 	The first one is the generic name, the second one contains
> > 	an alternate name, with has additional GPU version details
> > 	(in this case, CML-U GT2). All of that obtained from the
> > 	driver's source code.
> >
> > The generated script itself doesn't require anything other than lspci to
> > run, as what the script does, on its 35 lines of code (not including the
> > PCI ID database), is:
> >
> > 	1. run lspci -nm
> >
> > 	2. If the PCI ID device is at the database, it prints:
> >
> > 		print "Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\n";
> >
> > 	   For the above example, the output is:
> >
> > 		Detected GPU PCI device: CFL (CML_U_GT2), PCI ID: 8086:9b41
> >
> > 	3. If multiple Intel GPUs are found, it prints a warning, as DUT
> > 	   tests may require an extra parameter to use the right GPU:
> >
> > 		Warning: More than one Intel GPUs detected
> >
> > 	4. if no Intel GPU is found, it will print:
> >
> > 		Warning: No Intel GPUs detected
> 
> The way I see it, intel_device_info.c and igt_device_scan.c have all the
> building blocks necessary to accomplish this. As long as
> intel_device_match[] is kept up-to-date, which naturally will be, it
> needs no further maintenance. It could be added to lsgpu as an option.
> 

Yes it would be helpfull.

> >> lspci and modinfo are trivial to install, and most people have them
> >> installed already. modinfo does not require the modules to be probed,
> >> you can also point it at the .ko under /usr/lib/modules. For a user,
> >> this gives information about the modules they actually have on their
> >> system, which may be different from kernel or igt sources.
> >
> > Once generated, intel_detect_gpu.pl would be trivial to install and
> > to be used, as the only requirements are to have perl and pci-tools
> > (due to lspci dependency) installed.
> >
> >> OTOH most people won't have kernel or igt sources available. (Let alone
> >> specific versions of the source, which match the expectations of the
> >> patch at hand.) Indeed, you can install igt via the distro package
> >> manager, with no need to check out the sources.
> >
> > If IGT maintainers think it is worth targeting end users in the future,
> > the best is for them to periodically generate the script (for example,
> > when releasing a new IGT version), adding it to scripts/ and adding a
> > target at Meson to install it under ${installprefix}/bin directory.
> 
> The best is what I described above, not adding Rube Goldberg machines
> that require constant attention.
> 
> BR,
> Jani.

Maybe this should/may be added to scripts?
I do find it usefull to have a little script checks,
without much dependancy on other libs/tools. I would
also want to have a way to semi-automatically update
it later on, where new devices will be added.

Regards,
Kamil

> 
> 
> >
> > Regards,
> > Mauro
> 
> -- 
> Jani Nikula, Intel

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

[Jani Nikula]

On Thu, 23 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> Maybe this should/may be added to scripts?
> I do find it usefull to have a little script checks,
> without much dependancy on other libs/tools. I would
> also want to have a way to semi-automatically update
> it later on, where new devices will be added.

I never questioned the usefulness of such a tool. Only the
implementation, and how perl/make are not aligned with the IGT project,
and the constant maintenance burden all of this brings.

Besides that, I think I've pretty much said everything I want to say on
the matter.

Thanks,
Jani.

-- 
Jani Nikula, Intel

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]

On Fri, 24 May 2024 11:49:36 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> On Thu, 23 May 2024, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> > Maybe this should/may be added to scripts?
> > I do find it usefull to have a little script checks,
> > without much dependancy on other libs/tools. I would
> > also want to have a way to semi-automatically update
> > it later on, where new devices will be added.  
> 
> I never questioned the usefulness of such a tool. Only the
> implementation, and how perl/make are not aligned with the IGT project,
> and the constant maintenance burden all of this brings.

Maintenance is required only when the macros to define PCI IDs at the
i915 driver changes - or if Xe driver stops using INTEL_VGA_MACRO.
This is not frequent. The only changes such approach had since when it
was written, back in 2022 is when we added support for Xe driver, and
now that the PCI ID macros at drm-tip for i915 changed.

With regards to the latest change, the v5 of this patch solves it.
The only change to make it compatible with the new way is to add a
single line:

	if [ "$$(grep MACRO__ i915_pciids.h)" != "" ]; then sed -E "s/(INTEL_\S+_IDS\()/\1INTEL_VGA_DEVICE, /" -i i915_id.h; fi

As, up to the current upstream Kernel, the macro to define a PCI ID
inside i915_pciids.h were:

	#define INTEL_I815_IDS(info)					\
		INTEL_VGA_DEVICE(0x1132, info)  /* I815*/
	...

And, on current upstream, it now requires INTEL_VGA_DEVICE to be
passed on its first argument:

	#define INTEL_I815_IDS(MACRO__, ...) \
		MACRO__(0x1132, ## __VA_ARGS__)  /* I815*/
	...

So, the generated list of PCI IDs should be changed from:

	static struct pci_device_id pci_ids[] = {
		INTEL_PVC_IDS("PVC"),
		INTEL_MTL_IDS("MTL"),
		INTEL_MTL_P_IDS("MTL_P"),
		INTEL_MTL_M_IDS("MTL_M"),
		...

to:

	static struct pci_device_id pci_ids[] = {
		INTEL_PVC_IDS(INTEL_VGA_DEVICE, "PVC"),
		INTEL_MTL_IDS(INTEL_VGA_DEVICE, "MTL"),
		INTEL_MTL_P_IDS(INTEL_VGA_DEVICE, "MTL_P"),
		INTEL_MTL_M_IDS(INTEL_VGA_DEVICE, "MTL_M"),
		...

Changing PCI ID macros is something that it is not frequent.

Regards,
Mauro

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

[Zbigniew Kempczyński]

On Thu, May 23, 2024 at 12:10:05PM +0300, Jani Nikula wrote:
<cut>

> >
> > The generated script itself doesn't require anything other than lspci to
> > run, as what the script does, on its 35 lines of code (not including the
> > PCI ID database), is:
> >
> > 	1. run lspci -nm
> >
> > 	2. If the PCI ID device is at the database, it prints:
> >
> > 		print "Detected GPU PCI device: $id$subtype, PCI ID: $pci_id\n";
> >
> > 	   For the above example, the output is:
> >
> > 		Detected GPU PCI device: CFL (CML_U_GT2), PCI ID: 8086:9b41
> >
> > 	3. If multiple Intel GPUs are found, it prints a warning, as DUT
> > 	   tests may require an extra parameter to use the right GPU:
> >
> > 		Warning: More than one Intel GPUs detected
> >
> > 	4. if no Intel GPU is found, it will print:
> >
> > 		Warning: No Intel GPUs detected
> 
> The way I see it, intel_device_info.c and igt_device_scan.c have all the
> building blocks necessary to accomplish this. As long as
> intel_device_match[] is kept up-to-date, which naturally will be, it
> needs no further maintenance. It could be added to lsgpu as an option.
> 

Currently igt_device_scan.c is limited to devices which have drm driver
loaded (that was danvet requirement of udev scanning of drm devices, not
pci). But if I'm not wrong it would be relatively simple to add
scan_pci_devices() in which we would iterate over udev pci devices and
filter out 300 class then add some switch to display it lsgpu. We would
at least keep this simply in igt, without any additional scripting/tools
needed.

--
Zbigniew

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

[Jani Nikula]

On Mon, 03 Jun 2024, Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> wrote:
> Currently igt_device_scan.c is limited to devices which have drm driver
> loaded (that was danvet requirement of udev scanning of drm devices, not
> pci). But if I'm not wrong it would be relatively simple to add
> scan_pci_devices() in which we would iterate over udev pci devices and
> filter out 300 class then add some switch to display it lsgpu. We would
> at least keep this simply in igt, without any additional scripting/tools
> needed.

Yes.

-- 
Jani Nikula, Intel