All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] tools/intel_reg: add possibility to select device
@ 2024-03-14 22:58 Ashutosh Dixit
  2024-03-14 23:01 ` Dixit, Ashutosh
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Ashutosh Dixit @ 2024-03-14 22:58 UTC (permalink / raw
  To: igt-dev; +Cc: Kamil Konieczny, Zbigniew Kempczynski

From: Łukasz Łaguna <lukasz.laguna@intel.com>

Device can be selected by slot or using filter. Example:

intel_reg dump --slot "0000:01:00.0"
intel_reg dump --filter "pci:vendor=8086"

Signed-off-by: Łukasz Łaguna <lukasz.laguna@intel.com>
---
 tools/intel_reg.c | 128 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 126 insertions(+), 2 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 6c37e14d12..6b903f3f87 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -39,6 +39,7 @@
 #include "intel_chipset.h"
 
 #include "intel_reg_spec.h"
+#include "igt_device_scan.h"
 
 
 #ifdef HAVE_SYS_IO_H
@@ -86,6 +87,13 @@ struct config {
 	int verbosity;
 };
 
+struct pci_slot {
+	int domain;
+	int bus;
+	int dev;
+	int func;
+};
+
 /* port desc must have been set */
 static int set_reg_by_addr(struct config *config, struct reg *reg,
 			   uint32_t addr)
@@ -958,6 +966,12 @@ static int intel_reg_list(struct config *config, int argc, char *argv[])
 	return EXIT_SUCCESS;
 }
 
+static int intel_reg_list_filters(struct config *config, int argc, char *argv[])
+{
+	igt_device_print_filter_types();
+	return EXIT_SUCCESS;
+}
+
 static int intel_reg_help(struct config *config, int argc, char *argv[]);
 
 struct command {
@@ -1001,6 +1015,11 @@ static const struct command commands[] = {
 		.function = intel_reg_list,
 		.description = "list all known register names",
 	},
+	{
+		.name = "list_filters",
+		.function = intel_reg_list_filters,
+		.description = "list registered device filters types",
+	},
 	{
 		.name = "help",
 		.function = intel_reg_help,
@@ -1041,6 +1060,8 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 	printf(" --mmio=FILE    Use an MMIO snapshot\n");
 	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
 	printf(" --all          Decode registers for all known platforms\n");
+	printf(" --filter       Decode registers for platform described by filter\n");
+	printf(" --slot         Decode registers for platform described by slot\n");
 	printf(" --binary       Binary dump registers\n");
 	printf(" --verbose      Increase verbosity\n");
 	printf(" --quiet        Reduce verbosity\n");
@@ -1147,6 +1168,52 @@ builtin:
 	return config->regcount;
 }
 
+static void parse_pci_slot_name(struct pci_slot *pci_slot, const char *pci_slot_name)
+{
+	igt_assert_eq(sscanf(pci_slot_name, "%x:%x:%x.%x",
+		&pci_slot->domain, &pci_slot->bus, &pci_slot->dev, &pci_slot->func), 4);
+}
+
+static struct pci_device *find_intel_graphics_card(void)
+{
+	struct pci_device *pci_dev;
+	struct pci_device_iterator *iter;
+	struct pci_id_match match;
+
+	match.vendor_id = 0x8086; /* Intel */
+	match.device_id = PCI_MATCH_ANY;
+	match.subvendor_id = PCI_MATCH_ANY;
+	match.subdevice_id = PCI_MATCH_ANY;
+
+	match.device_class = 0x3 << 16;
+	match.device_class_mask = 0xff << 16;
+
+	match.match_data = 0;
+
+	iter = pci_id_match_iterator_create(&match);
+	pci_dev = pci_device_next(iter);
+	pci_iterator_destroy(iter);
+
+	return pci_dev;
+}
+
+static bool is_graphics_card_valid(struct pci_device *pci_dev)
+{
+	if (!pci_dev) {
+		fprintf(stderr, "Graphics card not found\n");
+		return false;
+	}
+	if (pci_device_probe(pci_dev) != 0) {
+		fprintf(stderr, "Couldn't probe graphics card\n");
+		return false;
+	}
+	if (pci_dev->vendor_id != 0x8086) {
+		fprintf(stderr, "Graphics card is non-intel\n");
+		return false;
+	}
+	return true;
+}
+
 enum opt {
 	OPT_UNKNOWN = '?',
 	OPT_END = -1,
@@ -1155,6 +1222,8 @@ enum opt {
 	OPT_COUNT,
 	OPT_POST,
 	OPT_ALL,
+	OPT_FILTER,
+	OPT_SLOT,
 	OPT_BINARY,
 	OPT_SPEC,
 	OPT_VERBOSE,
@@ -1164,8 +1233,12 @@ enum opt {
 
 int main(int argc, char *argv[])
 {
-	int ret, i, index;
+	int ret, i, index, len;
 	char *endp;
+	char *opt_filter = NULL;
+	struct pci_slot bdf;
+	struct pci_device *pci_dev = NULL;
+	struct igt_device_card card;
 	enum opt opt;
 	const struct command *command = NULL;
 	struct config config = {
@@ -1189,6 +1262,8 @@ int main(int argc, char *argv[])
 		{ "post",	no_argument,		NULL,	OPT_POST },
 		/* options specific to read, dump and decode */
 		{ "all",	no_argument,		NULL,	OPT_ALL },
+		{ "filter",	required_argument,	NULL,	OPT_FILTER },
+		{ "slot",	required_argument,	NULL,	OPT_SLOT },
 		{ "binary",	no_argument,		NULL,	OPT_BINARY },
 		{ 0 }
 	};
@@ -1233,6 +1308,24 @@ int main(int argc, char *argv[])
 		case OPT_ALL:
 			config.all_platforms = true;
 			break;
+		case OPT_FILTER:
+			opt_filter = strdup(optarg);
+			if (!opt_filter) {
+				fprintf(stderr, "strdup: %s\n",
+					strerror(errno));
+				return EXIT_FAILURE;
+			}
+			break;
+		case OPT_SLOT:
+			len = strlen("pci:slot=") + strlen(optarg) + 1;
+			opt_filter = malloc(len);
+			snprintf(opt_filter, len, "%s%s", "pci:slot=", optarg);
+			if (!opt_filter) {
+				fprintf(stderr, "opt_filter: %s\n",
+					strerror(errno));
+				return EXIT_FAILURE;
+			}
+			break;
 		case OPT_BINARY:
 			config.binary = true;
 			break;
@@ -1258,6 +1351,9 @@ int main(int argc, char *argv[])
 	if (help || (argc > 0 && strcmp(argv[0], "help") == 0))
 		return intel_reg_help(&config, argc, argv);
 
+	if (argc > 0 && strcmp(argv[0], "list_filters") == 0)
+		return intel_reg_list_filters(&config, argc, argv);
+
 	if (argc == 0) {
 		fprintf(stderr, "Command missing. Try intel_reg help.\n");
 		return EXIT_FAILURE;
@@ -1274,7 +1370,32 @@ int main(int argc, char *argv[])
 			fprintf(stderr, "--devid without --mmio\n");
 			return EXIT_FAILURE;
 		}
-		config.pci_dev = intel_get_pci_device();
+
+		if (pci_system_init() != 0) {
+			fprintf(stderr, "Couldn't initialize PCI system\n");
+			return EXIT_FAILURE;
+		}
+
+		igt_devices_scan(false);
+
+		if (opt_filter) {
+			if (!igt_device_card_match(opt_filter, &card)) {
+				fprintf(stderr, "Requested device %s not found!\n", opt_filter);
+				ret = EXIT_FAILURE;
+				goto exit;
+			}
+			parse_pci_slot_name(&bdf, card.pci_slot_name);
+			pci_dev = pci_device_find_by_slot(bdf.domain, bdf.bus, bdf.dev, bdf.func);
+		} else {
+			pci_dev = find_intel_graphics_card();
+		}
+
+		if (!is_graphics_card_valid(pci_dev)) {
+			ret = EXIT_FAILURE;
+			goto exit;
+		}
+
+		config.pci_dev = pci_dev;
 		config.devid = config.pci_dev->device_id;
 	}
 
@@ -1301,5 +1422,8 @@ int main(int argc, char *argv[])
 	if (config.fd >= 0)
 		close(config.fd);
 
+exit:
+	igt_devices_free();
+	free(opt_filter);
 	return ret;
 }
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tools/intel_reg: add possibility to select device
  2024-03-14 22:58 [PATCH i-g-t] tools/intel_reg: add possibility to select device Ashutosh Dixit
@ 2024-03-14 23:01 ` Dixit, Ashutosh
  2024-03-15  8:44   ` Ville Syrjälä
  2024-03-15  1:14 ` ✓ CI.xeBAT: success for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Dixit, Ashutosh @ 2024-03-14 23:01 UTC (permalink / raw
  To: igt-dev; +Cc: Kamil Konieczny, Zbigniew Kempczynski, lukasz.laguna

On Thu, 14 Mar 2024 15:58:39 -0700, Ashutosh Dixit wrote:
>
> From: Łukasz Łaguna <lukasz.laguna@intel.com>
>
> Device can be selected by slot or using filter. Example:
>
> intel_reg dump --slot "0000:01:00.0"
> intel_reg dump --filter "pci:vendor=8086"
>
> Signed-off-by: Łukasz Łaguna <lukasz.laguna@intel.com>

Is it possible to merge this old patch upstream? Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ CI.xeBAT: success for tools/intel_reg: add possibility to select device
  2024-03-14 22:58 [PATCH i-g-t] tools/intel_reg: add possibility to select device Ashutosh Dixit
  2024-03-14 23:01 ` Dixit, Ashutosh
@ 2024-03-15  1:14 ` Patchwork
  2024-03-15  1:35 ` ✗ Fi.CI.BAT: failure " Patchwork
  2024-03-15  9:13 ` [PATCH i-g-t] " Jani Nikula
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2024-03-15  1:14 UTC (permalink / raw
  To: Dixit, Ashutosh; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

== Series Details ==

Series: tools/intel_reg: add possibility to select device
URL   : https://patchwork.freedesktop.org/series/131155/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_7766_BAT -> XEIGTPW_10839_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (4 -> 4)
------------------------------

  No changes in participating hosts

Known issues
------------

  Here are the changes found in XEIGTPW_10839_BAT that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@xe_exec_threads@threads-mixed-shared-vm-basic:
    - bat-pvc-2:          [PASS][1] -> [DMESG-WARN][2] ([Intel XE#1238])
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_7766/bat-pvc-2/igt@xe_exec_threads@threads-mixed-shared-vm-basic.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10839/bat-pvc-2/igt@xe_exec_threads@threads-mixed-shared-vm-basic.html

  
  [Intel XE#1238]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1238


Build changes
-------------

  * IGT: IGT_7766 -> IGTPW_10839
  * Linux: xe-943-c44e29a7b62293355744fd857aad93cbf43e5126 -> xe-946-b7ead5c90db25002638773b1a9289220e6a36b4d

  IGTPW_10839: 10839
  IGT_7766: 08cf1b2fa6b2f422f417ea74f41b12b93e91156f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  xe-943-c44e29a7b62293355744fd857aad93cbf43e5126: c44e29a7b62293355744fd857aad93cbf43e5126
  xe-946-b7ead5c90db25002638773b1a9289220e6a36b4d: b7ead5c90db25002638773b1a9289220e6a36b4d

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_10839/index.html

[-- Attachment #2: Type: text/html, Size: 2217 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✗ Fi.CI.BAT: failure for tools/intel_reg: add possibility to select device
  2024-03-14 22:58 [PATCH i-g-t] tools/intel_reg: add possibility to select device Ashutosh Dixit
  2024-03-14 23:01 ` Dixit, Ashutosh
  2024-03-15  1:14 ` ✓ CI.xeBAT: success for " Patchwork
@ 2024-03-15  1:35 ` Patchwork
  2024-03-15  9:13 ` [PATCH i-g-t] " Jani Nikula
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2024-03-15  1:35 UTC (permalink / raw
  To: Ashutosh Dixit; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 3961 bytes --]

== Series Details ==

Series: tools/intel_reg: add possibility to select device
URL   : https://patchwork.freedesktop.org/series/131155/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14437 -> IGTPW_10839
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10839 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10839, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/index.html

Participating hosts (35 -> 33)
------------------------------

  Missing    (2): bat-jsl-1 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_10839:

### IGT changes ###

#### Possible regressions ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-kbl-8809g:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14437/fi-kbl-8809g/igt@dmabuf@all-tests@dma_fence_chain.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/fi-kbl-8809g/igt@dmabuf@all-tests@dma_fence_chain.html

  
Known issues
------------

  Here are the changes found in IGTPW_10839 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][3] ([i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/fi-cfl-8109u/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][4] ([i915#4613]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/fi-cfl-8109u/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_selftest@live@gt_lrc:
    - bat-dg2-9:          [PASS][5] -> [ABORT][6] ([i915#10366])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14437/bat-dg2-9/igt@i915_selftest@live@gt_lrc.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/bat-dg2-9/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@hangcheck:
    - bat-rpls-3:         [PASS][7] -> [DMESG-WARN][8] ([i915#5591])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14437/bat-rpls-3/igt@i915_selftest@live@hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/bat-rpls-3/igt@i915_selftest@live@hangcheck.html

  * igt@kms_pm_backlight@basic-brightness:
    - fi-cfl-8109u:       NOTRUN -> [SKIP][9] +11 other tests skip
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/fi-cfl-8109u/igt@kms_pm_backlight@basic-brightness.html

  
#### Possible fixes ####

  * igt@gem_lmem_swapping@basic@lmem0:
    - bat-dg2-14:         [FAIL][10] ([i915#10378]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14437/bat-dg2-14/igt@gem_lmem_swapping@basic@lmem0.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/bat-dg2-14/igt@gem_lmem_swapping@basic@lmem0.html

  
  [i915#10366]: https://gitlab.freedesktop.org/drm/intel/issues/10366
  [i915#10378]: https://gitlab.freedesktop.org/drm/intel/issues/10378
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7766 -> IGTPW_10839

  CI-20190529: 20190529
  CI_DRM_14437: b7ead5c90db25002638773b1a9289220e6a36b4d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10839: 10839
  IGT_7766: 08cf1b2fa6b2f422f417ea74f41b12b93e91156f @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10839/index.html

[-- Attachment #2: Type: text/html, Size: 4734 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tools/intel_reg: add possibility to select device
  2024-03-14 23:01 ` Dixit, Ashutosh
@ 2024-03-15  8:44   ` Ville Syrjälä
  0 siblings, 0 replies; 6+ messages in thread
From: Ville Syrjälä @ 2024-03-15  8:44 UTC (permalink / raw
  To: Dixit, Ashutosh
  Cc: igt-dev, Kamil Konieczny, Zbigniew Kempczynski, lukasz.laguna

On Thu, Mar 14, 2024 at 04:01:55PM -0700, Dixit, Ashutosh wrote:
> On Thu, 14 Mar 2024 15:58:39 -0700, Ashutosh Dixit wrote:
> >
> > From: Łukasz Łaguna <lukasz.laguna@intel.com>
> >
> > Device can be selected by slot or using filter. Example:
> >
> > intel_reg dump --slot "0000:01:00.0"
> > intel_reg dump --filter "pci:vendor=8086"
> >
> > Signed-off-by: Łukasz Łaguna <lukasz.laguna@intel.com>
> 
> Is it possible to merge this old patch upstream? Thanks.

Looks like two patches squashed to one. Someone should split
it up properly.

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t] tools/intel_reg: add possibility to select device
  2024-03-14 22:58 [PATCH i-g-t] tools/intel_reg: add possibility to select device Ashutosh Dixit
                   ` (2 preceding siblings ...)
  2024-03-15  1:35 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2024-03-15  9:13 ` Jani Nikula
  3 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2024-03-15  9:13 UTC (permalink / raw
  To: Ashutosh Dixit, igt-dev; +Cc: Kamil Konieczny, Zbigniew Kempczynski

On Thu, 14 Mar 2024, Ashutosh Dixit <ashutosh.dixit@intel.com> wrote:
> From: Łukasz Łaguna <lukasz.laguna@intel.com>
>
> Device can be selected by slot or using filter. Example:
>
> intel_reg dump --slot "0000:01:00.0"
> intel_reg dump --filter "pci:vendor=8086"

Really, if you want to add device filtering, the sensible thing to do is
to mimic lspci -s or -d options instead of NIH'ing your own. Maybe add a
subset of the functionality first, but please don't deviate from the
format.

Other comments inline.

> Signed-off-by: Łukasz Łaguna <lukasz.laguna@intel.com>
> ---
>  tools/intel_reg.c | 128 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 126 insertions(+), 2 deletions(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 6c37e14d12..6b903f3f87 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -39,6 +39,7 @@
>  #include "intel_chipset.h"
>  
>  #include "intel_reg_spec.h"
> +#include "igt_device_scan.h"
>  
>  
>  #ifdef HAVE_SYS_IO_H
> @@ -86,6 +87,13 @@ struct config {
>  	int verbosity;
>  };
>  
> +struct pci_slot {

The name has clash potential with pci libs.

> +	int domain;
> +	int bus;
> +	int dev;
> +	int func;
> +};
> +
>  /* port desc must have been set */
>  static int set_reg_by_addr(struct config *config, struct reg *reg,
>  			   uint32_t addr)
> @@ -958,6 +966,12 @@ static int intel_reg_list(struct config *config, int argc, char *argv[])
>  	return EXIT_SUCCESS;
>  }
>  
> +static int intel_reg_list_filters(struct config *config, int argc, char *argv[])
> +{
> +	igt_device_print_filter_types();
> +	return EXIT_SUCCESS;
> +}
> +
>  static int intel_reg_help(struct config *config, int argc, char *argv[]);
>  
>  struct command {
> @@ -1001,6 +1015,11 @@ static const struct command commands[] = {
>  		.function = intel_reg_list,
>  		.description = "list all known register names",
>  	},
> +	{
> +		.name = "list_filters",

Please do not use underscore in subcommands. list-filters.

When you update the command-line interface, a man page update is
required as well: man/intel_reg.rst.

> +		.function = intel_reg_list_filters,
> +		.description = "list registered device filters types",
> +	},
>  	{
>  		.name = "help",
>  		.function = intel_reg_help,
> @@ -1041,6 +1060,8 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>  	printf(" --mmio=FILE    Use an MMIO snapshot\n");
>  	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
>  	printf(" --all          Decode registers for all known platforms\n");
> +	printf(" --filter       Decode registers for platform described by filter\n");
> +	printf(" --slot         Decode registers for platform described by slot\n");
>  	printf(" --binary       Binary dump registers\n");
>  	printf(" --verbose      Increase verbosity\n");
>  	printf(" --quiet        Reduce verbosity\n");
> @@ -1147,6 +1168,52 @@ builtin:
>  	return config->regcount;
>  }
>  
> +static void parse_pci_slot_name(struct pci_slot *pci_slot, const char *pci_slot_name)
> +{
> +	igt_assert_eq(sscanf(pci_slot_name, "%x:%x:%x.%x",
> +		&pci_slot->domain, &pci_slot->bus, &pci_slot->dev, &pci_slot->func), 4);

This is a tool, not a test. Please do not use igt_assert or anything
like that. Handle the errors and fprintf a message to stderr.

> +}
> +
> +static struct pci_device *find_intel_graphics_card(void)
> +{
> +	struct pci_device *pci_dev;
> +	struct pci_device_iterator *iter;
> +	struct pci_id_match match;
> +
> +	match.vendor_id = 0x8086; /* Intel */
> +	match.device_id = PCI_MATCH_ANY;
> +	match.subvendor_id = PCI_MATCH_ANY;
> +	match.subdevice_id = PCI_MATCH_ANY;
> +
> +	match.device_class = 0x3 << 16;
> +	match.device_class_mask = 0xff << 16;
> +
> +	match.match_data = 0;
> +
> +	iter = pci_id_match_iterator_create(&match);
> +	pci_dev = pci_device_next(iter);
> +	pci_iterator_destroy(iter);
> +
> +	return pci_dev;
> +}
> +
> +static bool is_graphics_card_valid(struct pci_device *pci_dev)
> +{
> +	if (!pci_dev) {
> +		fprintf(stderr, "Graphics card not found\n");
> +		return false;
> +	}
> +	if (pci_device_probe(pci_dev) != 0) {
> +		fprintf(stderr, "Couldn't probe graphics card\n");
> +		return false;
> +	}
> +	if (pci_dev->vendor_id != 0x8086) {
> +		fprintf(stderr, "Graphics card is non-intel\n");
> +		return false;
> +	}
> +	return true;
> +}
> +
>  enum opt {
>  	OPT_UNKNOWN = '?',
>  	OPT_END = -1,
> @@ -1155,6 +1222,8 @@ enum opt {
>  	OPT_COUNT,
>  	OPT_POST,
>  	OPT_ALL,
> +	OPT_FILTER,
> +	OPT_SLOT,
>  	OPT_BINARY,
>  	OPT_SPEC,
>  	OPT_VERBOSE,
> @@ -1164,8 +1233,12 @@ enum opt {
>  
>  int main(int argc, char *argv[])
>  {
> -	int ret, i, index;
> +	int ret, i, index, len;
>  	char *endp;
> +	char *opt_filter = NULL;
> +	struct pci_slot bdf;
> +	struct pci_device *pci_dev = NULL;
> +	struct igt_device_card card;
>  	enum opt opt;
>  	const struct command *command = NULL;
>  	struct config config = {
> @@ -1189,6 +1262,8 @@ int main(int argc, char *argv[])
>  		{ "post",	no_argument,		NULL,	OPT_POST },
>  		/* options specific to read, dump and decode */
>  		{ "all",	no_argument,		NULL,	OPT_ALL },
> +		{ "filter",	required_argument,	NULL,	OPT_FILTER },
> +		{ "slot",	required_argument,	NULL,	OPT_SLOT },
>  		{ "binary",	no_argument,		NULL,	OPT_BINARY },
>  		{ 0 }
>  	};
> @@ -1233,6 +1308,24 @@ int main(int argc, char *argv[])
>  		case OPT_ALL:
>  			config.all_platforms = true;
>  			break;
> +		case OPT_FILTER:
> +			opt_filter = strdup(optarg);
> +			if (!opt_filter) {
> +				fprintf(stderr, "strdup: %s\n",
> +					strerror(errno));
> +				return EXIT_FAILURE;
> +			}
> +			break;
> +		case OPT_SLOT:
> +			len = strlen("pci:slot=") + strlen(optarg) + 1;
> +			opt_filter = malloc(len);
> +			snprintf(opt_filter, len, "%s%s", "pci:slot=", optarg);
> +			if (!opt_filter) {

snprintf() would've crashed by now if !opt_filter.

But maybe you're looking for asprintf() instead.

> +				fprintf(stderr, "opt_filter: %s\n",
> +					strerror(errno));
> +				return EXIT_FAILURE;
> +			}
> +			break;

Adding both --filter and --slot options leaks opt_filter. It's benign
for a small tool like this, but still wrong.

>  		case OPT_BINARY:
>  			config.binary = true;
>  			break;
> @@ -1258,6 +1351,9 @@ int main(int argc, char *argv[])
>  	if (help || (argc > 0 && strcmp(argv[0], "help") == 0))
>  		return intel_reg_help(&config, argc, argv);
>  
> +	if (argc > 0 && strcmp(argv[0], "list_filters") == 0)
> +		return intel_reg_list_filters(&config, argc, argv);
> +
>  	if (argc == 0) {
>  		fprintf(stderr, "Command missing. Try intel_reg help.\n");
>  		return EXIT_FAILURE;
> @@ -1274,7 +1370,32 @@ int main(int argc, char *argv[])
>  			fprintf(stderr, "--devid without --mmio\n");
>  			return EXIT_FAILURE;
>  		}
> -		config.pci_dev = intel_get_pci_device();
> +
> +		if (pci_system_init() != 0) {
> +			fprintf(stderr, "Couldn't initialize PCI system\n");
> +			return EXIT_FAILURE;
> +		}
> +
> +		igt_devices_scan(false);
> +
> +		if (opt_filter) {
> +			if (!igt_device_card_match(opt_filter, &card)) {
> +				fprintf(stderr, "Requested device %s not found!\n", opt_filter);
> +				ret = EXIT_FAILURE;
> +				goto exit;
> +			}
> +			parse_pci_slot_name(&bdf, card.pci_slot_name);
> +			pci_dev = pci_device_find_by_slot(bdf.domain, bdf.bus, bdf.dev, bdf.func);
> +		} else {
> +			pci_dev = find_intel_graphics_card();
> +		}
> +
> +		if (!is_graphics_card_valid(pci_dev)) {
> +			ret = EXIT_FAILURE;
> +			goto exit;
> +		}
> +

All of the above should be abstracted to a function instead of crammed
into main().

> +		config.pci_dev = pci_dev;
>  		config.devid = config.pci_dev->device_id;
>  	}
>  
> @@ -1301,5 +1422,8 @@ int main(int argc, char *argv[])
>  	if (config.fd >= 0)
>  		close(config.fd);
>  
> +exit:
> +	igt_devices_free();
> +	free(opt_filter);
>  	return ret;
>  }

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-15  9:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-14 22:58 [PATCH i-g-t] tools/intel_reg: add possibility to select device Ashutosh Dixit
2024-03-14 23:01 ` Dixit, Ashutosh
2024-03-15  8:44   ` Ville Syrjälä
2024-03-15  1:14 ` ✓ CI.xeBAT: success for " Patchwork
2024-03-15  1:35 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-03-15  9:13 ` [PATCH i-g-t] " Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.