Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: igt-dev@lists.freedesktop.org
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	Kamil Konieczny <kamil.konieczny@linux.intel.com>
Subject: [PATCH i-g-t 2/2] intel_reg: Move decoding behind an option
Date: Fri, 26 Apr 2024 08:33:52 -0700	[thread overview]
Message-ID: <20240426153352.3209972-2-lucas.demarchi@intel.com> (raw)
In-Reply-To: <20240426153352.3209972-1-lucas.demarchi@intel.com>

Decoding the register can be only as good as the reg_spec being used.
The current builtin register spec is not that. Move that decoding behind
an option to make it better for the normal case when running from the
repo checkout where the external reg spec is not available. Passing any
of --decode, --all, --spec brings the old behavior back:

	$ sudo ./build/tools/intel_reg --decode read 0x2358
	Warning: stat '/usr/local/share/igt-gpu-tools/registers' failed: No such file or directory. Using builtin register spec.
					    (0x00002358): 0x00000000

vs the new behavior:

	$ sudo ./build/tools/intel_reg --decode read 0x2358
					    (0x00002358): 0x00000000

We could probably reduce the leading space since we won't have any name,
but that can be improved later.

v2: Instead of removing the warning, move the whole decoding logic
    behind a command line option
v3: Some commands also imply --decode

Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 tools/intel_reg.c | 41 ++++++++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 8f585e4bd..c5800cf05 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -68,6 +68,9 @@ struct config {
 	/* write: do a posting read */
 	bool post;
 
+	/* decode registers, otherwise use just raw values */
+	bool decode;
+
 	/* decode register for all platforms */
 	bool all_platforms;
 
@@ -195,7 +198,7 @@ static bool port_is_mmio(enum port_addr port)
 	}
 }
 
-static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
+static void dump_regval(struct config *config, struct reg *reg, uint32_t val)
 {
 	char decode[1300];
 	char tmp[1024];
@@ -206,8 +209,11 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 	else
 		*bin = '\0';
 
-	intel_reg_spec_decode(tmp, sizeof(tmp), reg, val,
-			      config->all_platforms ? 0 : config->devid);
+	if (config->decode)
+		intel_reg_spec_decode(tmp, sizeof(tmp), reg, val,
+				      config->all_platforms ? 0 : config->devid);
+	else
+		*tmp = '\0';
 
 	if (*tmp) {
 		/* We have a decode result, and maybe binary decode. */
@@ -573,7 +579,7 @@ static void dump_register(struct config *config, struct reg *reg)
 	uint32_t val;
 
 	if (read_register(config, reg, &val) == 0)
-		dump_decode(config, reg, val);
+		dump_regval(config, reg, val);
 }
 
 static int write_register(struct config *config, struct reg *reg, uint32_t val)
@@ -944,7 +950,7 @@ static int intel_reg_decode(struct config *config, int argc, char *argv[])
 			continue;
 		}
 
-		dump_decode(config, &reg, val);
+		dump_regval(config, &reg, val);
 	}
 
 	return EXIT_SUCCESS;
@@ -1044,10 +1050,11 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 	printf("\n\n");
 
 	printf("OPTIONS common to most COMMANDS:\n");
-	printf(" --spec=PATH    Read register spec from directory or file\n");
+	printf(" --spec=PATH    Read register spec from directory or file. Implies --decode\n");
 	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(" --decode       Decode registers. Implied by commands that require it\n");
+	printf(" --all          Decode registers for all known platforms. Implies --decode\n");
 	printf(" --binary       Binary dump registers\n");
 	printf(" --verbose      Increase verbosity\n");
 	printf(" --quiet        Reduce verbosity\n");
@@ -1113,6 +1120,9 @@ static int read_reg_spec(struct config *config)
 	struct stat st;
 	int r;
 
+	if (!config->decode)
+		return 0;
+
 	path = config->specfile;
 	if (!path)
 		path = getenv("INTEL_REG_SPEC");
@@ -1161,6 +1171,7 @@ enum opt {
 	OPT_DEVID,
 	OPT_COUNT,
 	OPT_POST,
+	OPT_DECODE,
 	OPT_ALL,
 	OPT_BINARY,
 	OPT_SPEC,
@@ -1195,6 +1206,7 @@ int main(int argc, char *argv[])
 		/* options specific to write */
 		{ "post",	no_argument,		NULL,	OPT_POST },
 		/* options specific to read, dump and decode */
+		{ "decode",	no_argument,		NULL,	OPT_DECODE },
 		{ "all",	no_argument,		NULL,	OPT_ALL },
 		{ "binary",	no_argument,		NULL,	OPT_BINARY },
 		{ 0 }
@@ -1230,6 +1242,7 @@ int main(int argc, char *argv[])
 			config.post = true;
 			break;
 		case OPT_SPEC:
+			config.decode = true;
 			config.specfile = strdup(optarg);
 			if (!config.specfile) {
 				fprintf(stderr, "strdup: %s\n",
@@ -1239,6 +1252,10 @@ int main(int argc, char *argv[])
 			break;
 		case OPT_ALL:
 			config.all_platforms = true;
+			config.decode = true;
+			break;
+		case OPT_DECODE:
+			config.decode = true;
 			break;
 		case OPT_BINARY:
 			config.binary = true;
@@ -1285,10 +1302,6 @@ int main(int argc, char *argv[])
 		config.devid = config.pci_dev->device_id;
 	}
 
-	if (read_reg_spec(&config) < 0) {
-		return EXIT_FAILURE;
-	}
-
 	for (i = 0; i < ARRAY_SIZE(commands); i++) {
 		if (strcmp(argv[0], commands[i].name) == 0) {
 			command = &commands[i];
@@ -1301,6 +1314,12 @@ int main(int argc, char *argv[])
 		return EXIT_FAILURE;
 	}
 
+	if (command->decode)
+		config.decode = true;
+
+	if (read_reg_spec(&config) < 0)
+		return EXIT_FAILURE;
+
 	ret = command->function(&config, argc, argv);
 
 	free(config.mmiofile);
-- 
2.43.0


  reply	other threads:[~2024-04-26 15:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-26 15:33 [PATCH i-g-t 1/2] intel_reg: Reorder commands and annotate ones needing reg spec Lucas De Marchi
2024-04-26 15:33 ` Lucas De Marchi [this message]
2024-04-26 17:40 ` ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/2] " Patchwork
2024-04-26 17:57 ` ✓ CI.xeBAT: " Patchwork
2024-04-26 22:47 ` ✗ CI.xeFULL: failure " Patchwork
2024-04-26 23:23 ` ✗ Fi.CI.IGT: " Patchwork
2024-04-29  9:06 ` [PATCH i-g-t 1/2] " Jani Nikula
2024-04-29 13:31   ` Lucas De Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240426153352.3209972-2-lucas.demarchi@intel.com \
    --to=lucas.demarchi@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=kamil.konieczny@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox