[PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Mika Kuoppala]

Add option to specify engine for register read/write operation.
If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
to write and read register using a batch targeted at that engine.

v2: no MI_NOOP after BBE (Chris)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 tools/intel_reg.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 137 insertions(+), 2 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 00d2a4a1..ec45c2c9 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -33,6 +33,7 @@
 #include <unistd.h>
 
 #include "igt.h"
+#include "igt_gt.h"
 #include "intel_io.h"
 #include "intel_chipset.h"
 
@@ -73,6 +74,12 @@ struct config {
 
 	/* register spec */
 	char *specfile;
+
+	/* engine to use for lri (write) and srm (read) */
+	char *engine;
+	/* use secure batch */
+	bool engine_secure_batch;
+
 	struct reg *regs;
 	ssize_t regcount;
 
@@ -236,13 +243,116 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 	}
 }
 
+static const struct intel_execution_engine *find_engine(const char *name)
+{
+	const struct intel_execution_engine *e;
+
+	for (e = intel_execution_engines; e->name; e++) {
+		if (!strcmp(e->name, name))
+			return e;
+	}
+
+	fprintf(stderr, "no such engine as '%s'\n", name);
+
+	fprintf(stderr, "valid engines:");
+	for (e = intel_execution_engines; e->name; e++)
+		fprintf(stderr, " %s", e->name);
+
+	fprintf(stderr, "\n");
+
+	exit(EXIT_FAILURE);
+}
+
+static int register_srm(struct config *config, struct reg *reg,
+			uint32_t *val_in)
+{
+	const int gen = intel_gen(intel_get_drm_devid(config->drm_fd));
+	const bool r64b = gen >= 8;
+	const uint32_t ctx = 0;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry reloc[1];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	uint32_t *batch, *r;
+	const struct intel_execution_engine *engine;
+	int i915, i;
+	uint32_t val;
+	i915 = config->drm_fd;
+
+	engine = find_engine(config->engine);
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(i915, 4096);
+	obj[1].handle = gem_create(i915, 4096);
+	obj[1].relocs_ptr = to_user_pointer(reloc);
+	obj[1].relocation_count = 1;
+
+	batch = gem_mmap__cpu(i915, obj[1].handle, 0, 4096, PROT_WRITE);
+	gem_set_domain(i915, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	i = 0;
+	if (val_in) {
+		batch[i++] = MI_NOOP;
+		batch[i++] = MI_NOOP;
+
+		batch[i++] = MI_LOAD_REGISTER_IMM;
+		batch[i++] = reg->addr;
+		batch[i++] = *val_in;
+		batch[i++] = MI_NOOP;
+	}
+
+	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
+	batch[i++] = reg->addr;
+	reloc[0].target_handle = obj[0].handle;
+	reloc[0].presumed_offset = obj[0].offset;
+	reloc[0].offset = i * sizeof(uint32_t);
+	reloc[0].delta = 0;
+	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
+	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
+	batch[i++] = reloc[0].delta;
+	if (r64b)
+		batch[i++] = 0;
+
+	batch[i++] = MI_BATCH_BUFFER_END;
+	munmap(batch, 4096);
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(obj);
+	execbuf.buffer_count = 2;
+	execbuf.flags = engine->exec_id;
+
+	if (config->engine_secure_batch) {
+		execbuf.flags |= I915_EXEC_SECURE;
+
+		if (config->verbosity > 0)
+			printf("%s: using priviledged (secure) batch\n",
+			       engine->name);
+	}
+
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(i915, &execbuf);
+	gem_close(i915, obj[1].handle);
+
+	r = gem_mmap__cpu(i915, obj[0].handle, 0, 4096, PROT_READ);
+	gem_set_domain(i915, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
+
+	val = r[0];
+
+	gem_close(i915, obj[0].handle);
+
+	return val;
+}
+
 static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 {
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		val = INREG(reg->mmio_offset + reg->addr);
+		if (config->engine)
+			val = register_srm(config, reg, NULL);
+		else
+			val = INREG(reg->mmio_offset + reg->addr);
 		break;
 	case PORT_PORTIO_VGA:
 		iopl(3);
@@ -299,7 +409,15 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		OUTREG(reg->mmio_offset + reg->addr, val);
+		if (config->engine) {
+			ret = register_srm(config, reg, &val);
+			if (config->verbosity > 0 && ret != val)
+				fprintf(stderr,
+					"value readback 0x%08x != 0x%08x\n",
+					ret, val);
+		} else {
+			OUTREG(reg->mmio_offset + reg->addr, val);
+		}
 		break;
 	case PORT_PORTIO_VGA:
 		if (val > 0xff) {
@@ -641,6 +759,8 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 	printf(" --spec=PATH    Read register spec from directory or file\n");
 	printf(" --mmio=FILE    Use an MMIO snapshot\n");
 	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
+	printf(" --engine=ENGINE Use a specific engine to read/write\n");
+	printf(" --secure       Use secure batch to do engine read/write\n");
 	printf(" --all          Decode registers for all known platforms\n");
 	printf(" --binary       Binary dump registers\n");
 	printf(" --verbose      Increase verbosity\n");
@@ -758,6 +878,8 @@ enum opt {
 	OPT_ALL,
 	OPT_BINARY,
 	OPT_SPEC,
+	OPT_ENGINE,
+	OPT_SECURE,
 	OPT_VERBOSE,
 	OPT_QUIET,
 	OPT_HELP,
@@ -776,6 +898,8 @@ int main(int argc, char *argv[])
 
 	static struct option options[] = {
 		/* global options */
+		{ "engine",	required_argument,	NULL,	OPT_ENGINE },
+		{ "secure",     no_argument,            NULL,   OPT_SECURE},
 		{ "spec",	required_argument,	NULL,	OPT_SPEC },
 		{ "verbose",	no_argument,		NULL,	OPT_VERBOSE },
 		{ "quiet",	no_argument,		NULL,	OPT_QUIET },
@@ -830,6 +954,17 @@ int main(int argc, char *argv[])
 				return EXIT_FAILURE;
 			}
 			break;
+		case OPT_ENGINE:
+			config.engine = strdup(optarg);
+			if (!config.engine) {
+				fprintf(stderr, "strdup: %s\n",
+					strerror(errno));
+				return EXIT_FAILURE;
+			}
+			break;
+		case OPT_SECURE:
+			config.engine_secure_batch = true;
+			break;
 		case OPT_ALL:
 			config.all_platforms = true;
 			break;
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for tools/intel_reg: Add reading and writing registers through engine

[Patchwork]

== Series Details ==

Series: tools/intel_reg: Add reading and writing registers through engine
URL   : https://patchwork.freedesktop.org/series/34755/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
476c4b462e0453c70ee81664c0227fdddc26cbd0 igt/gem_eio: Increase wakeup delay for in-flight-suspend

make  all-recursive
Making all in lib
make  all-recursive
Making all in .
Making all in tests
make[4]: Nothing to be done for 'all'.
Making all in man
make[2]: Nothing to be done for 'all'.
Making all in tools
Making all in null_state_gen
make[3]: Nothing to be done for 'all'.
Making all in registers
make[3]: Nothing to be done for 'all'.
  CCLD     intel_aubdump.la
  CCLD     intel_audio_dump
  CC       intel_reg.o
intel_reg.c: In function ‘register_srm’:
intel_reg.c:269:54: error: ‘struct config’ has no member named ‘drm_fd’
  const int gen = intel_gen(intel_get_drm_devid(config->drm_fd));
                                                      ^~
intel_reg.c:279:15: error: ‘struct config’ has no member named ‘drm_fd’
  i915 = config->drm_fd;
               ^~
Makefile:1145: recipe for target 'intel_reg.o' failed
make[3]: *** [intel_reg.o] Error 1
Makefile:1209: recipe for target 'all-recursive' failed
make[2]: *** [all-recursive] Error 1
Makefile:531: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
Makefile:463: recipe for target 'all' failed
make: *** [all] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Chris Wilson]

Quoting Mika Kuoppala (2017-12-01 14:16:39)
> Add option to specify engine for register read/write operation.
> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
> to write and read register using a batch targeted at that engine.
> 
> v2: no MI_NOOP after BBE (Chris)
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  tools/intel_reg.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 137 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 00d2a4a1..ec45c2c9 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -33,6 +33,7 @@
>  #include <unistd.h>
>  
>  #include "igt.h"
> +#include "igt_gt.h"
>  #include "intel_io.h"
>  #include "intel_chipset.h"
>  
> @@ -73,6 +74,12 @@ struct config {
>  
>         /* register spec */
>         char *specfile;
> +
> +       /* engine to use for lri (write) and srm (read) */
> +       char *engine;
> +       /* use secure batch */
> +       bool engine_secure_batch;
> +
>         struct reg *regs;
>         ssize_t regcount;
>  
> @@ -236,13 +243,116 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>         }
>  }
>  
> +static const struct intel_execution_engine *find_engine(const char *name)
> +{

Being modern, we should be using the "$class$instance" pattern (e.g. vcs1).

> +       const struct intel_execution_engine *e;
> +
> +       for (e = intel_execution_engines; e->name; e++) {
> +               if (!strcmp(e->name, name))
> +                       return e;
> +       }
> +
> +       fprintf(stderr, "no such engine as '%s'\n", name);
> +
> +       fprintf(stderr, "valid engines:");
> +       for (e = intel_execution_engines; e->name; e++)
> +               fprintf(stderr, " %s", e->name);
> +
> +       fprintf(stderr, "\n");
> +
> +       exit(EXIT_FAILURE);
> +}
> +
> +static int register_srm(struct config *config, struct reg *reg,
> +                       uint32_t *val_in)
> +{
> +       const int gen = intel_gen(intel_get_drm_devid(config->drm_fd));
> +       const bool r64b = gen >= 8;
> +       const uint32_t ctx = 0;
> +       struct drm_i915_gem_exec_object2 obj[2];
> +       struct drm_i915_gem_relocation_entry reloc[1];
> +       struct drm_i915_gem_execbuffer2 execbuf;
> +       uint32_t *batch, *r;
> +       const struct intel_execution_engine *engine;
> +       int i915, i;
> +       uint32_t val;
> +       i915 = config->drm_fd;
> +
> +       engine = find_engine(config->engine);
> +
> +       memset(obj, 0, sizeof(obj));
> +       obj[0].handle = gem_create(i915, 4096);
> +       obj[1].handle = gem_create(i915, 4096);
> +       obj[1].relocs_ptr = to_user_pointer(reloc);
> +       obj[1].relocation_count = 1;
> +
> +       batch = gem_mmap__cpu(i915, obj[1].handle, 0, 4096, PROT_WRITE);
> +       gem_set_domain(i915, obj[1].handle,
> +                      I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> +
> +       i = 0;
> +       if (val_in) {
> +               batch[i++] = MI_NOOP;
> +               batch[i++] = MI_NOOP;
> +
> +               batch[i++] = MI_LOAD_REGISTER_IMM;
> +               batch[i++] = reg->addr;
> +               batch[i++] = *val_in;
> +               batch[i++] = MI_NOOP;
> +       }
> +
> +       batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
> +       batch[i++] = reg->addr;
> +       reloc[0].target_handle = obj[0].handle;
> +       reloc[0].presumed_offset = obj[0].offset;
> +       reloc[0].offset = i * sizeof(uint32_t);
> +       reloc[0].delta = 0;
> +       reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> +       reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> +       batch[i++] = reloc[0].delta;
> +       if (r64b)
> +               batch[i++] = 0;
> +
> +       batch[i++] = MI_BATCH_BUFFER_END;
> +       munmap(batch, 4096);
> +
> +       memset(&execbuf, 0, sizeof(execbuf));
> +       execbuf.buffers_ptr = to_user_pointer(obj);
> +       execbuf.buffer_count = 2;
> +       execbuf.flags = engine->exec_id;
> +
> +       if (config->engine_secure_batch) {
> +               execbuf.flags |= I915_EXEC_SECURE;
> +
> +               if (config->verbosity > 0)
> +                       printf("%s: using priviledged (secure) batch\n",
privileged
Scrap saying (secure) to the users. It's blatantly not at this point if
we allow any old (root) user to write any old register.

> +                              engine->name);
> +       }
> +
> +       execbuf.rsvd1 = ctx;
> +       gem_execbuf(i915, &execbuf);
> +       gem_close(i915, obj[1].handle);
> +
> +       r = gem_mmap__cpu(i915, obj[0].handle, 0, 4096, PROT_READ);
> +       gem_set_domain(i915, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> +
> +       val = r[0];
> +
> +       gem_close(i915, obj[0].handle);
> +
> +       return val;
> +}
> +
>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>  {
>         uint32_t val = 0;
>  
>         switch (reg->port_desc.port) {
>         case PORT_MMIO:
> -               val = INREG(reg->mmio_offset + reg->addr);
> +               if (config->engine)
> +                       val = register_srm(config, reg, NULL);
> +               else
> +                       val = INREG(reg->mmio_offset + reg->addr);
>                 break;
>         case PORT_PORTIO_VGA:
>                 iopl(3);
> @@ -299,7 +409,15 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>  
>         switch (reg->port_desc.port) {
>         case PORT_MMIO:
> -               OUTREG(reg->mmio_offset + reg->addr, val);
> +               if (config->engine) {
> +                       ret = register_srm(config, reg, &val);
> +                       if (config->verbosity > 0 && ret != val)
> +                               fprintf(stderr,
> +                                       "value readback 0x%08x != 0x%08x\n",
> +                                       ret, val);

Not reading back the same value is quite common. Don't we normally print
out the value after writing anyway? i.e. I don't see why the LRI/SRM path
is special in this regard.

> +               } else {
> +                       OUTREG(reg->mmio_offset + reg->addr, val);
> +               }
>                 break;
>         case PORT_PORTIO_VGA:
>                 if (val > 0xff) {
> @@ -641,6 +759,8 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>         printf(" --spec=PATH    Read register spec from directory or file\n");
>         printf(" --mmio=FILE    Use an MMIO snapshot\n");
>         printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
> +       printf(" --engine=ENGINE Use a specific engine to read/write\n");
> +       printf(" --secure       Use secure batch to do engine read/write\n");
>         printf(" --all          Decode registers for all known platforms\n");
>         printf(" --binary       Binary dump registers\n");
>         printf(" --verbose      Increase verbosity\n");
> @@ -758,6 +878,8 @@ enum opt {
>         OPT_ALL,
>         OPT_BINARY,
>         OPT_SPEC,
> +       OPT_ENGINE,
> +       OPT_SECURE,
>         OPT_VERBOSE,
>         OPT_QUIET,
>         OPT_HELP,
> @@ -776,6 +898,8 @@ int main(int argc, char *argv[])
>  
>         static struct option options[] = {
>                 /* global options */
> +               { "engine",     required_argument,      NULL,   OPT_ENGINE },
> +               { "secure",     no_argument,            NULL,   OPT_SECURE},

Ah, I see. I would probably just encode it into the engine name, +rcs0
I'm failing to think of a switch name that makes sense to me. We're
being run as root with the express intent of writing a register, it
seems to me that by default we should be trying to write the register
(and hence use privileged instructions). So maybe it should be
--non-privileged, or -rcs0 to drop the privileges.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Mika Kuoppala]

Add option to specify engine for register read/write operation.
If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
to write and read register using a batch targeted at that engine.

v2: no MI_NOOP after BBE (Chris)
v3: use modern engine names (Chris), use global fd

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 tools/intel_reg.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 154 insertions(+), 2 deletions(-)

diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 00d2a4a1..7f3494ef 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -33,6 +33,7 @@
 #include <unistd.h>
 
 #include "igt.h"
+#include "igt_gt.h"
 #include "intel_io.h"
 #include "intel_chipset.h"
 
@@ -73,6 +74,11 @@ struct config {
 
 	/* register spec */
 	char *specfile;
+
+	/* engine to use for lri (write) and srm (read) */
+	char *engine;
+	int fd;
+
 	struct reg *regs;
 	ssize_t regcount;
 
@@ -236,13 +242,140 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 	}
 }
 
+static const struct intel_execution_engine2 *find_engine(const char *name,
+							 bool *secure)
+{
+	const struct intel_execution_engine2 *e;
+
+	if (strlen(name) < 2)
+		goto out;
+
+	if (name[0] == '-') {
+		*secure = false;
+		name++;
+	} else {
+		*secure = true;
+	}
+
+	for (e = intel_execution_engines2; e->name; e++) {
+		if (!strcmp(e->name, name))
+			return e;
+	}
+
+out:
+	fprintf(stderr, "no such engine as '%s'\n", name);
+
+	fprintf(stderr, "valid engines:");
+	for (e = intel_execution_engines2; e->name; e++)
+		fprintf(stderr, " %s", e->name);
+
+	fprintf(stderr, "\n");
+
+	exit(EXIT_FAILURE);
+}
+
+static int register_srm(struct config *config, struct reg *reg,
+			uint32_t *val_in)
+{
+	const int gen = intel_gen(config->devid);
+	const bool r64b = gen >= 8;
+	const uint32_t ctx = 0;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry reloc[1];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	uint32_t *batch, *r;
+	const struct intel_execution_engine2 *engine;
+	bool secure;
+	int fd, i;
+	uint32_t val;
+
+	if (config->fd == -1) {
+		config->fd = __drm_open_driver(DRIVER_INTEL);
+		if (config->fd == -1) {
+			fprintf(stderr, "Error opening driver: %s",
+				strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	fd = config->fd;
+	engine = find_engine(config->engine, &secure);
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, 4096);
+	obj[1].handle = gem_create(fd, 4096);
+	obj[1].relocs_ptr = to_user_pointer(reloc);
+	obj[1].relocation_count = 1;
+
+	batch = gem_mmap__cpu(fd, obj[1].handle, 0, 4096, PROT_WRITE);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	i = 0;
+	if (val_in) {
+		batch[i++] = MI_NOOP;
+		batch[i++] = MI_NOOP;
+
+		batch[i++] = MI_LOAD_REGISTER_IMM;
+		batch[i++] = reg->addr;
+		batch[i++] = *val_in;
+		batch[i++] = MI_NOOP;
+	}
+
+	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
+	batch[i++] = reg->addr;
+	reloc[0].target_handle = obj[0].handle;
+	reloc[0].presumed_offset = obj[0].offset;
+	reloc[0].offset = i * sizeof(uint32_t);
+	reloc[0].delta = 0;
+	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
+	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
+	batch[i++] = reloc[0].delta;
+	if (r64b)
+		batch[i++] = 0;
+
+	batch[i++] = MI_BATCH_BUFFER_END;
+	munmap(batch, 4096);
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(obj);
+	execbuf.buffer_count = 2;
+	execbuf.flags = gem_class_instance_to_eb_flags(fd,
+						       engine->class,
+						       engine->instance);
+	if (secure)
+		execbuf.flags |= I915_EXEC_SECURE;
+
+	if (config->verbosity > 0)
+		printf("%s: using %sprivileged batch\n",
+		       engine->name,
+		       secure ? "" : "non-");
+
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+
+	r = gem_mmap__cpu(fd, obj[0].handle, 0, 4096, PROT_READ);
+	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
+
+	val = r[0];
+	munmap(r, 4096);
+
+	gem_close(fd, obj[0].handle);
+
+	return val;
+}
+
 static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 {
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		val = INREG(reg->mmio_offset + reg->addr);
+		if (config->engine)
+			val = register_srm(config, reg, NULL);
+		else
+			val = INREG(reg->mmio_offset + reg->addr);
 		break;
 	case PORT_PORTIO_VGA:
 		iopl(3);
@@ -299,7 +432,11 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		OUTREG(reg->mmio_offset + reg->addr, val);
+		if (config->engine) {
+			register_srm(config, reg, &val);
+		} else {
+			OUTREG(reg->mmio_offset + reg->addr, val);
+		}
 		break;
 	case PORT_PORTIO_VGA:
 		if (val > 0xff) {
@@ -641,6 +778,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 	printf(" --spec=PATH    Read register spec from directory or file\n");
 	printf(" --mmio=FILE    Use an MMIO snapshot\n");
 	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
+	printf(" --engine=[-]ENGINE Use a specific engine to read/write\n");
 	printf(" --all          Decode registers for all known platforms\n");
 	printf(" --binary       Binary dump registers\n");
 	printf(" --verbose      Increase verbosity\n");
@@ -758,6 +896,7 @@ enum opt {
 	OPT_ALL,
 	OPT_BINARY,
 	OPT_SPEC,
+	OPT_ENGINE,
 	OPT_VERBOSE,
 	OPT_QUIET,
 	OPT_HELP,
@@ -771,11 +910,13 @@ int main(int argc, char *argv[])
 	const struct command *command = NULL;
 	struct config config = {
 		.count = 1,
+		.fd = -1,
 	};
 	bool help = false;
 
 	static struct option options[] = {
 		/* global options */
+		{ "engine",	required_argument,	NULL,	OPT_ENGINE },
 		{ "spec",	required_argument,	NULL,	OPT_SPEC },
 		{ "verbose",	no_argument,		NULL,	OPT_VERBOSE },
 		{ "quiet",	no_argument,		NULL,	OPT_QUIET },
@@ -830,6 +971,14 @@ int main(int argc, char *argv[])
 				return EXIT_FAILURE;
 			}
 			break;
+		case OPT_ENGINE:
+			config.engine = strdup(optarg);
+			if (!config.engine) {
+				fprintf(stderr, "strdup: %s\n",
+					strerror(errno));
+				return EXIT_FAILURE;
+			}
+			break;
 		case OPT_ALL:
 			config.all_platforms = true;
 			break;
@@ -898,5 +1047,8 @@ int main(int argc, char *argv[])
 
 	free(config.mmiofile);
 
+	if (config.fd >= 0)
+		close(config.fd);
+
 	return ret;
 }
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Chris Wilson]

Quoting Mika Kuoppala (2018-01-08 14:12:02)
> Add option to specify engine for register read/write operation.
> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
> to write and read register using a batch targeted at that engine.
> 
> v2: no MI_NOOP after BBE (Chris)
> v3: use modern engine names (Chris), use global fd
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
> +static const struct intel_execution_engine2 *find_engine(const char *name,
> +                                                        bool *secure)
> +{
> +       const struct intel_execution_engine2 *e;
> +
> +       if (strlen(name) < 2)
> +               goto out;
> +
> +       if (name[0] == '-') {
> +               *secure = false;
> +               name++;
> +       } else {
> +               *secure = true;
> +       }
> +
> +       for (e = intel_execution_engines2; e->name; e++) {
> +               if (!strcmp(e->name, name))

strcasecmp() just for ease of use.

Lgtm, 
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Jani Nikula]

On Mon, 08 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Add option to specify engine for register read/write operation.
> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
> to write and read register using a batch targeted at that engine.

Copy-pasting from the man page, we already have the notation:

"Registers are defined as [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR)."

Why don't we add this as ENGINE:REGNAME or something instead of an extra
--engine parameter? As a "port". Sure, it's more work, but I really like
the current possibility of reading all types of registers at once. Now
you prevent dumps that would contain both mmio and batch based reads.

BR,
Jani.


>
> v2: no MI_NOOP after BBE (Chris)
> v3: use modern engine names (Chris), use global fd
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  tools/intel_reg.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 154 insertions(+), 2 deletions(-)
>
> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
> index 00d2a4a1..7f3494ef 100644
> --- a/tools/intel_reg.c
> +++ b/tools/intel_reg.c
> @@ -33,6 +33,7 @@
>  #include <unistd.h>
>  
>  #include "igt.h"
> +#include "igt_gt.h"
>  #include "intel_io.h"
>  #include "intel_chipset.h"
>  
> @@ -73,6 +74,11 @@ struct config {
>  
>  	/* register spec */
>  	char *specfile;
> +
> +	/* engine to use for lri (write) and srm (read) */
> +	char *engine;
> +	int fd;
> +
>  	struct reg *regs;
>  	ssize_t regcount;
>  
> @@ -236,13 +242,140 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>  	}
>  }
>  
> +static const struct intel_execution_engine2 *find_engine(const char *name,
> +							 bool *secure)
> +{
> +	const struct intel_execution_engine2 *e;
> +
> +	if (strlen(name) < 2)
> +		goto out;
> +
> +	if (name[0] == '-') {
> +		*secure = false;
> +		name++;
> +	} else {
> +		*secure = true;
> +	}
> +
> +	for (e = intel_execution_engines2; e->name; e++) {
> +		if (!strcmp(e->name, name))
> +			return e;
> +	}
> +
> +out:
> +	fprintf(stderr, "no such engine as '%s'\n", name);
> +
> +	fprintf(stderr, "valid engines:");
> +	for (e = intel_execution_engines2; e->name; e++)
> +		fprintf(stderr, " %s", e->name);
> +
> +	fprintf(stderr, "\n");
> +
> +	exit(EXIT_FAILURE);
> +}
> +
> +static int register_srm(struct config *config, struct reg *reg,
> +			uint32_t *val_in)
> +{
> +	const int gen = intel_gen(config->devid);
> +	const bool r64b = gen >= 8;
> +	const uint32_t ctx = 0;
> +	struct drm_i915_gem_exec_object2 obj[2];
> +	struct drm_i915_gem_relocation_entry reloc[1];
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	uint32_t *batch, *r;
> +	const struct intel_execution_engine2 *engine;
> +	bool secure;
> +	int fd, i;
> +	uint32_t val;
> +
> +	if (config->fd == -1) {
> +		config->fd = __drm_open_driver(DRIVER_INTEL);
> +		if (config->fd == -1) {
> +			fprintf(stderr, "Error opening driver: %s",
> +				strerror(errno));
> +			exit(EXIT_FAILURE);
> +		}
> +	}
> +
> +	fd = config->fd;
> +	engine = find_engine(config->engine, &secure);
> +
> +	memset(obj, 0, sizeof(obj));
> +	obj[0].handle = gem_create(fd, 4096);
> +	obj[1].handle = gem_create(fd, 4096);
> +	obj[1].relocs_ptr = to_user_pointer(reloc);
> +	obj[1].relocation_count = 1;
> +
> +	batch = gem_mmap__cpu(fd, obj[1].handle, 0, 4096, PROT_WRITE);
> +	gem_set_domain(fd, obj[1].handle,
> +		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> +
> +	i = 0;
> +	if (val_in) {
> +		batch[i++] = MI_NOOP;
> +		batch[i++] = MI_NOOP;
> +
> +		batch[i++] = MI_LOAD_REGISTER_IMM;
> +		batch[i++] = reg->addr;
> +		batch[i++] = *val_in;
> +		batch[i++] = MI_NOOP;
> +	}
> +
> +	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
> +	batch[i++] = reg->addr;
> +	reloc[0].target_handle = obj[0].handle;
> +	reloc[0].presumed_offset = obj[0].offset;
> +	reloc[0].offset = i * sizeof(uint32_t);
> +	reloc[0].delta = 0;
> +	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
> +	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
> +	batch[i++] = reloc[0].delta;
> +	if (r64b)
> +		batch[i++] = 0;
> +
> +	batch[i++] = MI_BATCH_BUFFER_END;
> +	munmap(batch, 4096);
> +
> +	memset(&execbuf, 0, sizeof(execbuf));
> +	execbuf.buffers_ptr = to_user_pointer(obj);
> +	execbuf.buffer_count = 2;
> +	execbuf.flags = gem_class_instance_to_eb_flags(fd,
> +						       engine->class,
> +						       engine->instance);
> +	if (secure)
> +		execbuf.flags |= I915_EXEC_SECURE;
> +
> +	if (config->verbosity > 0)
> +		printf("%s: using %sprivileged batch\n",
> +		       engine->name,
> +		       secure ? "" : "non-");
> +
> +	execbuf.rsvd1 = ctx;
> +	gem_execbuf(fd, &execbuf);
> +	gem_close(fd, obj[1].handle);
> +
> +	r = gem_mmap__cpu(fd, obj[0].handle, 0, 4096, PROT_READ);
> +	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
> +
> +	val = r[0];
> +	munmap(r, 4096);
> +
> +	gem_close(fd, obj[0].handle);
> +
> +	return val;
> +}
> +
>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>  {
>  	uint32_t val = 0;
>  
>  	switch (reg->port_desc.port) {
>  	case PORT_MMIO:
> -		val = INREG(reg->mmio_offset + reg->addr);
> +		if (config->engine)
> +			val = register_srm(config, reg, NULL);
> +		else
> +			val = INREG(reg->mmio_offset + reg->addr);
>  		break;
>  	case PORT_PORTIO_VGA:
>  		iopl(3);
> @@ -299,7 +432,11 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>  
>  	switch (reg->port_desc.port) {
>  	case PORT_MMIO:
> -		OUTREG(reg->mmio_offset + reg->addr, val);
> +		if (config->engine) {
> +			register_srm(config, reg, &val);
> +		} else {
> +			OUTREG(reg->mmio_offset + reg->addr, val);
> +		}
>  		break;
>  	case PORT_PORTIO_VGA:
>  		if (val > 0xff) {
> @@ -641,6 +778,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>  	printf(" --spec=PATH    Read register spec from directory or file\n");
>  	printf(" --mmio=FILE    Use an MMIO snapshot\n");
>  	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
> +	printf(" --engine=[-]ENGINE Use a specific engine to read/write\n");
>  	printf(" --all          Decode registers for all known platforms\n");
>  	printf(" --binary       Binary dump registers\n");
>  	printf(" --verbose      Increase verbosity\n");
> @@ -758,6 +896,7 @@ enum opt {
>  	OPT_ALL,
>  	OPT_BINARY,
>  	OPT_SPEC,
> +	OPT_ENGINE,
>  	OPT_VERBOSE,
>  	OPT_QUIET,
>  	OPT_HELP,
> @@ -771,11 +910,13 @@ int main(int argc, char *argv[])
>  	const struct command *command = NULL;
>  	struct config config = {
>  		.count = 1,
> +		.fd = -1,
>  	};
>  	bool help = false;
>  
>  	static struct option options[] = {
>  		/* global options */
> +		{ "engine",	required_argument,	NULL,	OPT_ENGINE },
>  		{ "spec",	required_argument,	NULL,	OPT_SPEC },
>  		{ "verbose",	no_argument,		NULL,	OPT_VERBOSE },
>  		{ "quiet",	no_argument,		NULL,	OPT_QUIET },
> @@ -830,6 +971,14 @@ int main(int argc, char *argv[])
>  				return EXIT_FAILURE;
>  			}
>  			break;
> +		case OPT_ENGINE:
> +			config.engine = strdup(optarg);
> +			if (!config.engine) {
> +				fprintf(stderr, "strdup: %s\n",
> +					strerror(errno));
> +				return EXIT_FAILURE;
> +			}
> +			break;
>  		case OPT_ALL:
>  			config.all_platforms = true;
>  			break;
> @@ -898,5 +1047,8 @@ int main(int argc, char *argv[])
>  
>  	free(config.mmiofile);
>  
> +	if (config.fd >= 0)
> +		close(config.fd);
> +
>  	return ret;
>  }

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Mika Kuoppala]

Add option to specify engine for register read/write operation.
If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
to write and read register using a batch targeted at that engine.

v2: no MI_NOOP after BBE (Chris)
v3: use modern engine names (Chris), use global fd
v4: strcasecmp (Chris)
v5: use register definition format for engine (Jani)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v4)
---
 man/intel_reg.rst      |   9 ++-
 tools/intel_reg.c      | 169 +++++++++++++++++++++++++++++++++++++++++++++++--
 tools/intel_reg_spec.h |   1 +
 3 files changed, 171 insertions(+), 8 deletions(-)

diff --git a/man/intel_reg.rst b/man/intel_reg.rst
index d1af178a..4ec24c4c 100644
--- a/man/intel_reg.rst
+++ b/man/intel_reg.rst
@@ -103,7 +103,7 @@ Display brief help.
 REGISTER REFERENCES
 ===================
 
-Registers are defined as [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR).
+Registers are defined as [(PORTNAME|PORTNUM|ENGINE|MMIO-OFFSET):](REGNAME|REGADDR).
 
 PORTNAME
     The register access method, most often MMIO, which is the default. The
@@ -120,6 +120,13 @@ PORTNUM
     Numbers above 0xff are automatically interpreted as MMIO offsets, not port
     numbers.
 
+ENGINE
+    Instead of cpu based MMIO, specified engine can be used for access method.
+    Batchbuffer will be targeted for the engine to do read/write. The list of
+    available engines is architecture specific and can be found with
+    "intel_reg help". Prefixing engine name with '-' uses non-privileged
+    batchbuffer for access.
+
 MMIO-OFFSET
     Use MMIO, and add this offset to the register address.
 
diff --git a/tools/intel_reg.c b/tools/intel_reg.c
index 00d2a4a1..ddff2794 100644
--- a/tools/intel_reg.c
+++ b/tools/intel_reg.c
@@ -33,6 +33,7 @@
 #include <unistd.h>
 
 #include "igt.h"
+#include "igt_gt.h"
 #include "intel_io.h"
 #include "intel_chipset.h"
 
@@ -73,6 +74,10 @@ struct config {
 
 	/* register spec */
 	char *specfile;
+
+	/* fd for engine access avoiding reopens */
+	int fd;
+
 	struct reg *regs;
 	ssize_t regcount;
 
@@ -236,13 +241,130 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
 	}
 }
 
+static const struct intel_execution_engine2 *find_engine(const char *name)
+{
+	const struct intel_execution_engine2 *e;
+
+	if (strlen(name) < 2)
+		return NULL;
+
+	if (name[0] == '-')
+		name++;
+
+	for (e = intel_execution_engines2; e->name; e++) {
+		if (!strcasecmp(e->name, name))
+			return e;
+	}
+
+	return NULL;
+}
+
+static int register_srm(struct config *config, struct reg *reg,
+			uint32_t *val_in)
+{
+	const int gen = intel_gen(config->devid);
+	const bool r64b = gen >= 8;
+	const uint32_t ctx = 0;
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry reloc[1];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	uint32_t *batch, *r;
+	const struct intel_execution_engine2 *engine;
+	bool secure;
+	int fd, i;
+	uint32_t val;
+
+	if (config->fd == -1) {
+		config->fd = __drm_open_driver(DRIVER_INTEL);
+		if (config->fd == -1) {
+			fprintf(stderr, "Error opening driver: %s",
+				strerror(errno));
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	fd = config->fd;
+	engine = find_engine(reg->engine);
+	if (engine == NULL)
+		exit(EXIT_FAILURE);
+
+	secure = reg->engine[0] != '-';
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, 4096);
+	obj[1].handle = gem_create(fd, 4096);
+	obj[1].relocs_ptr = to_user_pointer(reloc);
+	obj[1].relocation_count = 1;
+
+	batch = gem_mmap__cpu(fd, obj[1].handle, 0, 4096, PROT_WRITE);
+	gem_set_domain(fd, obj[1].handle,
+		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
+
+	i = 0;
+	if (val_in) {
+		batch[i++] = MI_NOOP;
+		batch[i++] = MI_NOOP;
+
+		batch[i++] = MI_LOAD_REGISTER_IMM;
+		batch[i++] = reg->addr;
+		batch[i++] = *val_in;
+		batch[i++] = MI_NOOP;
+	}
+
+	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
+	batch[i++] = reg->addr;
+	reloc[0].target_handle = obj[0].handle;
+	reloc[0].presumed_offset = obj[0].offset;
+	reloc[0].offset = i * sizeof(uint32_t);
+	reloc[0].delta = 0;
+	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
+	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
+	batch[i++] = reloc[0].delta;
+	if (r64b)
+		batch[i++] = 0;
+
+	batch[i++] = MI_BATCH_BUFFER_END;
+	munmap(batch, 4096);
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = to_user_pointer(obj);
+	execbuf.buffer_count = 2;
+	execbuf.flags = gem_class_instance_to_eb_flags(fd,
+						       engine->class,
+						       engine->instance);
+	if (secure)
+		execbuf.flags |= I915_EXEC_SECURE;
+
+	if (config->verbosity > 0)
+		printf("%s: using %sprivileged batch\n",
+		       engine->name,
+		       secure ? "" : "non-");
+
+	execbuf.rsvd1 = ctx;
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+
+	r = gem_mmap__cpu(fd, obj[0].handle, 0, 4096, PROT_READ);
+	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
+
+	val = r[0];
+	munmap(r, 4096);
+
+	gem_close(fd, obj[0].handle);
+
+	return val;
+}
+
 static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
 {
 	uint32_t val = 0;
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		val = INREG(reg->mmio_offset + reg->addr);
+		if (reg->engine)
+			val = register_srm(config, reg, NULL);
+		else
+			val = INREG(reg->mmio_offset + reg->addr);
 		break;
 	case PORT_PORTIO_VGA:
 		iopl(3);
@@ -299,7 +421,11 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 
 	switch (reg->port_desc.port) {
 	case PORT_MMIO:
-		OUTREG(reg->mmio_offset + reg->addr, val);
+		if (reg->engine) {
+			register_srm(config, reg, &val);
+		} else {
+			OUTREG(reg->mmio_offset + reg->addr, val);
+		}
 		break;
 	case PORT_PORTIO_VGA:
 		if (val > 0xff) {
@@ -351,7 +477,25 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
 	return ret;
 }
 
-/* s has [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR) */
+static int parse_engine(struct reg *reg, const char *s)
+{
+	const struct intel_execution_engine2 *e;
+
+	e = find_engine(s);
+	if (e) {
+		reg->port_desc.port = PORT_MMIO;
+		reg->port_desc.name = strdup(s);
+		reg->port_desc.stride = 4;
+		reg->engine = strdup(s);
+		reg->mmio_offset = 0;
+	} else {
+		reg->engine = NULL;
+	}
+
+	return reg->engine == NULL;
+}
+
+/* s has [(PORTNAME|PORTNUM|ENGINE|MMIO-OFFSET):](REGNAME|REGADDR) */
 static int parse_reg(struct config *config, struct reg *reg, const char *s)
 {
 	unsigned long addr;
@@ -367,7 +511,9 @@ static int parse_reg(struct config *config, struct reg *reg, const char *s)
 	} else if (p) {
 		char *port_name = strndup(s, p - s);
 
-		ret = parse_port_desc(reg, port_name);
+		ret = parse_engine(reg, port_name);
+		if (ret)
+			ret = parse_port_desc(reg, port_name);
 
 		free(port_name);
 		p++;
@@ -616,6 +762,7 @@ static const struct command commands[] = {
 
 static int intel_reg_help(struct config *config, int argc, char *argv[])
 {
+	const struct intel_execution_engine2 *e;
 	int i;
 
 	printf("Intel graphics register multitool\n\n");
@@ -629,14 +776,18 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
 
 	printf("\n");
 	printf("REGISTER is defined as:\n");
-        printf("  [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR)\n");
+        printf("  [(PORTNAME|PORTNUM|ENGINE|MMIO-OFFSET):](REGNAME|REGADDR)\n");
 
 	printf("\n");
 	printf("PORTNAME is one of:\n");
 	intel_reg_spec_print_ports();
-	printf("\n");
+	printf("\n\n");
+
+	printf("ENGINE is one of:\n");
+	for (e = intel_execution_engines2; e->name; e++)
+		printf("%s -%s ", e->name, e->name);
+	printf("\n\n");
 
-	printf("\n");
 	printf("OPTIONS common to most COMMANDS:\n");
 	printf(" --spec=PATH    Read register spec from directory or file\n");
 	printf(" --mmio=FILE    Use an MMIO snapshot\n");
@@ -771,6 +922,7 @@ int main(int argc, char *argv[])
 	const struct command *command = NULL;
 	struct config config = {
 		.count = 1,
+		.fd = -1,
 	};
 	bool help = false;
 
@@ -898,5 +1050,8 @@ int main(int argc, char *argv[])
 
 	free(config.mmiofile);
 
+	if (config.fd >= 0)
+		close(config.fd);
+
 	return ret;
 }
diff --git a/tools/intel_reg_spec.h b/tools/intel_reg_spec.h
index dcb31430..c94c61ba 100644
--- a/tools/intel_reg_spec.h
+++ b/tools/intel_reg_spec.h
@@ -53,6 +53,7 @@ struct port_desc {
 
 struct reg {
 	struct port_desc port_desc;
+	char *engine;
 	uint32_t mmio_offset;
 	uint32_t addr;
 	char *name;
-- 
2.14.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Mika Kuoppala]

Jani Nikula <jani.nikula@intel.com> writes:

> On Mon, 08 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
>> Add option to specify engine for register read/write operation.
>> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
>> to write and read register using a batch targeted at that engine.
>
> Copy-pasting from the man page, we already have the notation:
>
> "Registers are defined as [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR)."
>
> Why don't we add this as ENGINE:REGNAME or something instead of an extra
> --engine parameter? As a "port". Sure, it's more work, but I really like
> the current possibility of reading all types of registers at once. Now
> you prevent dumps that would contain both mmio and batch based reads.
>

Are you ok with the latest version? As discussed in irc, there are
problems with trying to add engines as ports, due to dynamic nature
and also that the existing infra relies on PORTNAME being mmio
for symbolic register dumping to work correctly.

for the wart of if (reg->engine) in read/write paths
we gain the benefits of mmio dumps with engines.

-Mika

> BR,
> Jani.
>
>
>>
>> v2: no MI_NOOP after BBE (Chris)
>> v3: use modern engine names (Chris), use global fd
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>  tools/intel_reg.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 154 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
>> index 00d2a4a1..7f3494ef 100644
>> --- a/tools/intel_reg.c
>> +++ b/tools/intel_reg.c
>> @@ -33,6 +33,7 @@
>>  #include <unistd.h>
>>  
>>  #include "igt.h"
>> +#include "igt_gt.h"
>>  #include "intel_io.h"
>>  #include "intel_chipset.h"
>>  
>> @@ -73,6 +74,11 @@ struct config {
>>  
>>  	/* register spec */
>>  	char *specfile;
>> +
>> +	/* engine to use for lri (write) and srm (read) */
>> +	char *engine;
>> +	int fd;
>> +
>>  	struct reg *regs;
>>  	ssize_t regcount;
>>  
>> @@ -236,13 +242,140 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>>  	}
>>  }
>>  
>> +static const struct intel_execution_engine2 *find_engine(const char *name,
>> +							 bool *secure)
>> +{
>> +	const struct intel_execution_engine2 *e;
>> +
>> +	if (strlen(name) < 2)
>> +		goto out;
>> +
>> +	if (name[0] == '-') {
>> +		*secure = false;
>> +		name++;
>> +	} else {
>> +		*secure = true;
>> +	}
>> +
>> +	for (e = intel_execution_engines2; e->name; e++) {
>> +		if (!strcmp(e->name, name))
>> +			return e;
>> +	}
>> +
>> +out:
>> +	fprintf(stderr, "no such engine as '%s'\n", name);
>> +
>> +	fprintf(stderr, "valid engines:");
>> +	for (e = intel_execution_engines2; e->name; e++)
>> +		fprintf(stderr, " %s", e->name);
>> +
>> +	fprintf(stderr, "\n");
>> +
>> +	exit(EXIT_FAILURE);
>> +}
>> +
>> +static int register_srm(struct config *config, struct reg *reg,
>> +			uint32_t *val_in)
>> +{
>> +	const int gen = intel_gen(config->devid);
>> +	const bool r64b = gen >= 8;
>> +	const uint32_t ctx = 0;
>> +	struct drm_i915_gem_exec_object2 obj[2];
>> +	struct drm_i915_gem_relocation_entry reloc[1];
>> +	struct drm_i915_gem_execbuffer2 execbuf;
>> +	uint32_t *batch, *r;
>> +	const struct intel_execution_engine2 *engine;
>> +	bool secure;
>> +	int fd, i;
>> +	uint32_t val;
>> +
>> +	if (config->fd == -1) {
>> +		config->fd = __drm_open_driver(DRIVER_INTEL);
>> +		if (config->fd == -1) {
>> +			fprintf(stderr, "Error opening driver: %s",
>> +				strerror(errno));
>> +			exit(EXIT_FAILURE);
>> +		}
>> +	}
>> +
>> +	fd = config->fd;
>> +	engine = find_engine(config->engine, &secure);
>> +
>> +	memset(obj, 0, sizeof(obj));
>> +	obj[0].handle = gem_create(fd, 4096);
>> +	obj[1].handle = gem_create(fd, 4096);
>> +	obj[1].relocs_ptr = to_user_pointer(reloc);
>> +	obj[1].relocation_count = 1;
>> +
>> +	batch = gem_mmap__cpu(fd, obj[1].handle, 0, 4096, PROT_WRITE);
>> +	gem_set_domain(fd, obj[1].handle,
>> +		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
>> +
>> +	i = 0;
>> +	if (val_in) {
>> +		batch[i++] = MI_NOOP;
>> +		batch[i++] = MI_NOOP;
>> +
>> +		batch[i++] = MI_LOAD_REGISTER_IMM;
>> +		batch[i++] = reg->addr;
>> +		batch[i++] = *val_in;
>> +		batch[i++] = MI_NOOP;
>> +	}
>> +
>> +	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
>> +	batch[i++] = reg->addr;
>> +	reloc[0].target_handle = obj[0].handle;
>> +	reloc[0].presumed_offset = obj[0].offset;
>> +	reloc[0].offset = i * sizeof(uint32_t);
>> +	reloc[0].delta = 0;
>> +	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
>> +	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
>> +	batch[i++] = reloc[0].delta;
>> +	if (r64b)
>> +		batch[i++] = 0;
>> +
>> +	batch[i++] = MI_BATCH_BUFFER_END;
>> +	munmap(batch, 4096);
>> +
>> +	memset(&execbuf, 0, sizeof(execbuf));
>> +	execbuf.buffers_ptr = to_user_pointer(obj);
>> +	execbuf.buffer_count = 2;
>> +	execbuf.flags = gem_class_instance_to_eb_flags(fd,
>> +						       engine->class,
>> +						       engine->instance);
>> +	if (secure)
>> +		execbuf.flags |= I915_EXEC_SECURE;
>> +
>> +	if (config->verbosity > 0)
>> +		printf("%s: using %sprivileged batch\n",
>> +		       engine->name,
>> +		       secure ? "" : "non-");
>> +
>> +	execbuf.rsvd1 = ctx;
>> +	gem_execbuf(fd, &execbuf);
>> +	gem_close(fd, obj[1].handle);
>> +
>> +	r = gem_mmap__cpu(fd, obj[0].handle, 0, 4096, PROT_READ);
>> +	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
>> +
>> +	val = r[0];
>> +	munmap(r, 4096);
>> +
>> +	gem_close(fd, obj[0].handle);
>> +
>> +	return val;
>> +}
>> +
>>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>>  {
>>  	uint32_t val = 0;
>>  
>>  	switch (reg->port_desc.port) {
>>  	case PORT_MMIO:
>> -		val = INREG(reg->mmio_offset + reg->addr);
>> +		if (config->engine)
>> +			val = register_srm(config, reg, NULL);
>> +		else
>> +			val = INREG(reg->mmio_offset + reg->addr);
>>  		break;
>>  	case PORT_PORTIO_VGA:
>>  		iopl(3);
>> @@ -299,7 +432,11 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>>  
>>  	switch (reg->port_desc.port) {
>>  	case PORT_MMIO:
>> -		OUTREG(reg->mmio_offset + reg->addr, val);
>> +		if (config->engine) {
>> +			register_srm(config, reg, &val);
>> +		} else {
>> +			OUTREG(reg->mmio_offset + reg->addr, val);
>> +		}
>>  		break;
>>  	case PORT_PORTIO_VGA:
>>  		if (val > 0xff) {
>> @@ -641,6 +778,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>>  	printf(" --spec=PATH    Read register spec from directory or file\n");
>>  	printf(" --mmio=FILE    Use an MMIO snapshot\n");
>>  	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
>> +	printf(" --engine=[-]ENGINE Use a specific engine to read/write\n");
>>  	printf(" --all          Decode registers for all known platforms\n");
>>  	printf(" --binary       Binary dump registers\n");
>>  	printf(" --verbose      Increase verbosity\n");
>> @@ -758,6 +896,7 @@ enum opt {
>>  	OPT_ALL,
>>  	OPT_BINARY,
>>  	OPT_SPEC,
>> +	OPT_ENGINE,
>>  	OPT_VERBOSE,
>>  	OPT_QUIET,
>>  	OPT_HELP,
>> @@ -771,11 +910,13 @@ int main(int argc, char *argv[])
>>  	const struct command *command = NULL;
>>  	struct config config = {
>>  		.count = 1,
>> +		.fd = -1,
>>  	};
>>  	bool help = false;
>>  
>>  	static struct option options[] = {
>>  		/* global options */
>> +		{ "engine",	required_argument,	NULL,	OPT_ENGINE },
>>  		{ "spec",	required_argument,	NULL,	OPT_SPEC },
>>  		{ "verbose",	no_argument,		NULL,	OPT_VERBOSE },
>>  		{ "quiet",	no_argument,		NULL,	OPT_QUIET },
>> @@ -830,6 +971,14 @@ int main(int argc, char *argv[])
>>  				return EXIT_FAILURE;
>>  			}
>>  			break;
>> +		case OPT_ENGINE:
>> +			config.engine = strdup(optarg);
>> +			if (!config.engine) {
>> +				fprintf(stderr, "strdup: %s\n",
>> +					strerror(errno));
>> +				return EXIT_FAILURE;
>> +			}
>> +			break;
>>  		case OPT_ALL:
>>  			config.all_platforms = true;
>>  			break;
>> @@ -898,5 +1047,8 @@ int main(int argc, char *argv[])
>>  
>>  	free(config.mmiofile);
>>  
>> +	if (config.fd >= 0)
>> +		close(config.fd);
>> +
>>  	return ret;
>>  }
>
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Jani Nikula]

On Mon, 22 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
> Jani Nikula <jani.nikula@intel.com> writes:
>
>> On Mon, 08 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
>>> Add option to specify engine for register read/write operation.
>>> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
>>> to write and read register using a batch targeted at that engine.
>>
>> Copy-pasting from the man page, we already have the notation:
>>
>> "Registers are defined as [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR)."
>>
>> Why don't we add this as ENGINE:REGNAME or something instead of an extra
>> --engine parameter? As a "port". Sure, it's more work, but I really like
>> the current possibility of reading all types of registers at once. Now
>> you prevent dumps that would contain both mmio and batch based reads.
>>
>
> Are you ok with the latest version? As discussed in irc, there are
> problems with trying to add engines as ports, due to dynamic nature
> and also that the existing infra relies on PORTNAME being mmio
> for symbolic register dumping to work correctly.
>
> for the wart of if (reg->engine) in read/write paths
> we gain the benefits of mmio dumps with engines.

Can't say I would've reviewed all the engine bits and pieces, but

Acked-by: Jani Nikula <jani.nikula@intel.com>


>
> -Mika
>
>> BR,
>> Jani.
>>
>>
>>>
>>> v2: no MI_NOOP after BBE (Chris)
>>> v3: use modern engine names (Chris), use global fd
>>>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>>> ---
>>>  tools/intel_reg.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 154 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/intel_reg.c b/tools/intel_reg.c
>>> index 00d2a4a1..7f3494ef 100644
>>> --- a/tools/intel_reg.c
>>> +++ b/tools/intel_reg.c
>>> @@ -33,6 +33,7 @@
>>>  #include <unistd.h>
>>>  
>>>  #include "igt.h"
>>> +#include "igt_gt.h"
>>>  #include "intel_io.h"
>>>  #include "intel_chipset.h"
>>>  
>>> @@ -73,6 +74,11 @@ struct config {
>>>  
>>>  	/* register spec */
>>>  	char *specfile;
>>> +
>>> +	/* engine to use for lri (write) and srm (read) */
>>> +	char *engine;
>>> +	int fd;
>>> +
>>>  	struct reg *regs;
>>>  	ssize_t regcount;
>>>  
>>> @@ -236,13 +242,140 @@ static void dump_decode(struct config *config, struct reg *reg, uint32_t val)
>>>  	}
>>>  }
>>>  
>>> +static const struct intel_execution_engine2 *find_engine(const char *name,
>>> +							 bool *secure)
>>> +{
>>> +	const struct intel_execution_engine2 *e;
>>> +
>>> +	if (strlen(name) < 2)
>>> +		goto out;
>>> +
>>> +	if (name[0] == '-') {
>>> +		*secure = false;
>>> +		name++;
>>> +	} else {
>>> +		*secure = true;
>>> +	}
>>> +
>>> +	for (e = intel_execution_engines2; e->name; e++) {
>>> +		if (!strcmp(e->name, name))
>>> +			return e;
>>> +	}
>>> +
>>> +out:
>>> +	fprintf(stderr, "no such engine as '%s'\n", name);
>>> +
>>> +	fprintf(stderr, "valid engines:");
>>> +	for (e = intel_execution_engines2; e->name; e++)
>>> +		fprintf(stderr, " %s", e->name);
>>> +
>>> +	fprintf(stderr, "\n");
>>> +
>>> +	exit(EXIT_FAILURE);
>>> +}
>>> +
>>> +static int register_srm(struct config *config, struct reg *reg,
>>> +			uint32_t *val_in)
>>> +{
>>> +	const int gen = intel_gen(config->devid);
>>> +	const bool r64b = gen >= 8;
>>> +	const uint32_t ctx = 0;
>>> +	struct drm_i915_gem_exec_object2 obj[2];
>>> +	struct drm_i915_gem_relocation_entry reloc[1];
>>> +	struct drm_i915_gem_execbuffer2 execbuf;
>>> +	uint32_t *batch, *r;
>>> +	const struct intel_execution_engine2 *engine;
>>> +	bool secure;
>>> +	int fd, i;
>>> +	uint32_t val;
>>> +
>>> +	if (config->fd == -1) {
>>> +		config->fd = __drm_open_driver(DRIVER_INTEL);
>>> +		if (config->fd == -1) {
>>> +			fprintf(stderr, "Error opening driver: %s",
>>> +				strerror(errno));
>>> +			exit(EXIT_FAILURE);
>>> +		}
>>> +	}
>>> +
>>> +	fd = config->fd;
>>> +	engine = find_engine(config->engine, &secure);
>>> +
>>> +	memset(obj, 0, sizeof(obj));
>>> +	obj[0].handle = gem_create(fd, 4096);
>>> +	obj[1].handle = gem_create(fd, 4096);
>>> +	obj[1].relocs_ptr = to_user_pointer(reloc);
>>> +	obj[1].relocation_count = 1;
>>> +
>>> +	batch = gem_mmap__cpu(fd, obj[1].handle, 0, 4096, PROT_WRITE);
>>> +	gem_set_domain(fd, obj[1].handle,
>>> +		       I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
>>> +
>>> +	i = 0;
>>> +	if (val_in) {
>>> +		batch[i++] = MI_NOOP;
>>> +		batch[i++] = MI_NOOP;
>>> +
>>> +		batch[i++] = MI_LOAD_REGISTER_IMM;
>>> +		batch[i++] = reg->addr;
>>> +		batch[i++] = *val_in;
>>> +		batch[i++] = MI_NOOP;
>>> +	}
>>> +
>>> +	batch[i++] = 0x24 << 23 | (1 + r64b); /* SRM */
>>> +	batch[i++] = reg->addr;
>>> +	reloc[0].target_handle = obj[0].handle;
>>> +	reloc[0].presumed_offset = obj[0].offset;
>>> +	reloc[0].offset = i * sizeof(uint32_t);
>>> +	reloc[0].delta = 0;
>>> +	reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
>>> +	reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
>>> +	batch[i++] = reloc[0].delta;
>>> +	if (r64b)
>>> +		batch[i++] = 0;
>>> +
>>> +	batch[i++] = MI_BATCH_BUFFER_END;
>>> +	munmap(batch, 4096);
>>> +
>>> +	memset(&execbuf, 0, sizeof(execbuf));
>>> +	execbuf.buffers_ptr = to_user_pointer(obj);
>>> +	execbuf.buffer_count = 2;
>>> +	execbuf.flags = gem_class_instance_to_eb_flags(fd,
>>> +						       engine->class,
>>> +						       engine->instance);
>>> +	if (secure)
>>> +		execbuf.flags |= I915_EXEC_SECURE;
>>> +
>>> +	if (config->verbosity > 0)
>>> +		printf("%s: using %sprivileged batch\n",
>>> +		       engine->name,
>>> +		       secure ? "" : "non-");
>>> +
>>> +	execbuf.rsvd1 = ctx;
>>> +	gem_execbuf(fd, &execbuf);
>>> +	gem_close(fd, obj[1].handle);
>>> +
>>> +	r = gem_mmap__cpu(fd, obj[0].handle, 0, 4096, PROT_READ);
>>> +	gem_set_domain(fd, obj[0].handle, I915_GEM_DOMAIN_CPU, 0);
>>> +
>>> +	val = r[0];
>>> +	munmap(r, 4096);
>>> +
>>> +	gem_close(fd, obj[0].handle);
>>> +
>>> +	return val;
>>> +}
>>> +
>>>  static int read_register(struct config *config, struct reg *reg, uint32_t *valp)
>>>  {
>>>  	uint32_t val = 0;
>>>  
>>>  	switch (reg->port_desc.port) {
>>>  	case PORT_MMIO:
>>> -		val = INREG(reg->mmio_offset + reg->addr);
>>> +		if (config->engine)
>>> +			val = register_srm(config, reg, NULL);
>>> +		else
>>> +			val = INREG(reg->mmio_offset + reg->addr);
>>>  		break;
>>>  	case PORT_PORTIO_VGA:
>>>  		iopl(3);
>>> @@ -299,7 +432,11 @@ static int write_register(struct config *config, struct reg *reg, uint32_t val)
>>>  
>>>  	switch (reg->port_desc.port) {
>>>  	case PORT_MMIO:
>>> -		OUTREG(reg->mmio_offset + reg->addr, val);
>>> +		if (config->engine) {
>>> +			register_srm(config, reg, &val);
>>> +		} else {
>>> +			OUTREG(reg->mmio_offset + reg->addr, val);
>>> +		}
>>>  		break;
>>>  	case PORT_PORTIO_VGA:
>>>  		if (val > 0xff) {
>>> @@ -641,6 +778,7 @@ static int intel_reg_help(struct config *config, int argc, char *argv[])
>>>  	printf(" --spec=PATH    Read register spec from directory or file\n");
>>>  	printf(" --mmio=FILE    Use an MMIO snapshot\n");
>>>  	printf(" --devid=DEVID  Specify PCI device ID for --mmio=FILE\n");
>>> +	printf(" --engine=[-]ENGINE Use a specific engine to read/write\n");
>>>  	printf(" --all          Decode registers for all known platforms\n");
>>>  	printf(" --binary       Binary dump registers\n");
>>>  	printf(" --verbose      Increase verbosity\n");
>>> @@ -758,6 +896,7 @@ enum opt {
>>>  	OPT_ALL,
>>>  	OPT_BINARY,
>>>  	OPT_SPEC,
>>> +	OPT_ENGINE,
>>>  	OPT_VERBOSE,
>>>  	OPT_QUIET,
>>>  	OPT_HELP,
>>> @@ -771,11 +910,13 @@ int main(int argc, char *argv[])
>>>  	const struct command *command = NULL;
>>>  	struct config config = {
>>>  		.count = 1,
>>> +		.fd = -1,
>>>  	};
>>>  	bool help = false;
>>>  
>>>  	static struct option options[] = {
>>>  		/* global options */
>>> +		{ "engine",	required_argument,	NULL,	OPT_ENGINE },
>>>  		{ "spec",	required_argument,	NULL,	OPT_SPEC },
>>>  		{ "verbose",	no_argument,		NULL,	OPT_VERBOSE },
>>>  		{ "quiet",	no_argument,		NULL,	OPT_QUIET },
>>> @@ -830,6 +971,14 @@ int main(int argc, char *argv[])
>>>  				return EXIT_FAILURE;
>>>  			}
>>>  			break;
>>> +		case OPT_ENGINE:
>>> +			config.engine = strdup(optarg);
>>> +			if (!config.engine) {
>>> +				fprintf(stderr, "strdup: %s\n",
>>> +					strerror(errno));
>>> +				return EXIT_FAILURE;
>>> +			}
>>> +			break;
>>>  		case OPT_ALL:
>>>  			config.all_platforms = true;
>>>  			break;
>>> @@ -898,5 +1047,8 @@ int main(int argc, char *argv[])
>>>  
>>>  	free(config.mmiofile);
>>>  
>>> +	if (config.fd >= 0)
>>> +		close(config.fd);
>>> +
>>>  	return ret;
>>>  }
>>
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine

[Mika Kuoppala]

Jani Nikula <jani.nikula@intel.com> writes:

> On Mon, 22 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
>> Jani Nikula <jani.nikula@intel.com> writes:
>>
>>> On Mon, 08 Jan 2018, Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:
>>>> Add option to specify engine for register read/write operation.
>>>> If engine is specified, use MI_LOAD_REGISTER_IMM and MI_STORE_REGISTER_IMM
>>>> to write and read register using a batch targeted at that engine.
>>>
>>> Copy-pasting from the man page, we already have the notation:
>>>
>>> "Registers are defined as [(PORTNAME|PORTNUM|MMIO-OFFSET):](REGNAME|REGADDR)."
>>>
>>> Why don't we add this as ENGINE:REGNAME or something instead of an extra
>>> --engine parameter? As a "port". Sure, it's more work, but I really like
>>> the current possibility of reading all types of registers at once. Now
>>> you prevent dumps that would contain both mmio and batch based reads.
>>>
>>
>> Are you ok with the latest version? As discussed in irc, there are
>> problems with trying to add engines as ports, due to dynamic nature
>> and also that the existing infra relies on PORTNAME being mmio
>> for symbolic register dumping to work correctly.
>>
>> for the wart of if (reg->engine) in read/write paths
>> we gain the benefits of mmio dumps with engines.
>
> Can't say I would've reviewed all the engine bits and pieces, but
>
> Acked-by: Jani Nikula <jani.nikula@intel.com>

Thanks for ack and review. Pushed.
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev2)

[Patchwork]

== Series Details ==

Series: tools/intel_reg: Add reading and writing registers through engine (rev2)
URL   : https://patchwork.freedesktop.org/series/34755/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
d5e51a60e5cbb807bcacd2655bd4ffe90a686bbb scripts/trace.pl: Optimize event parsing and processing

with latest DRM-Tip kernel build CI_DRM_3607
70afdd6e5dfe drm-tip: 2018y-01m-08d-13h-41m-31s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-elk-e7500) fdo#103989
                incomplete -> PASS       (fi-snb-2520m) fdo#103713 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:422s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:430s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:493s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:484s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:472s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-elk-e7500     total:224  pass:169  dwarn:9   dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:281s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:517s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:393s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:415s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:415s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:468s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:501s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:501s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:577s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:438s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:514s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:434s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:400s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y         total:249  pass:224  dwarn:0   dfail:0   fail:0   skip:24 
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:472s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_753/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for tools/intel_reg: Add reading and writing registers through engine (rev2)

[Patchwork]

== Series Details ==

Series: tools/intel_reg: Add reading and writing registers through engine (rev2)
URL   : https://patchwork.freedesktop.org/series/34755/
State : warning

== Summary ==

Test perf_pmu:
        Subgroup render-node-busy-vcs0:
                pass       -> FAIL       (shard-snb) fdo#104218
Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> SKIP       (shard-hsw) fdo#103540
        Subgroup modeset-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                fail       -> PASS       (shard-snb) fdo#101623 +1
Test kms_atomic_transition:
        Subgroup plane-all-modeset-transition:
                pass       -> DMESG-WARN (shard-hsw)
Test gem_eio:
        Subgroup in-flight-contexts:
                dmesg-warn -> PASS       (shard-snb) fdo#104058

fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058

shard-hsw        total:2695 pass:1522 dwarn:2   dfail:0   fail:10  skip:1160 time:8702s
shard-snb        total:2713 pass:1309 dwarn:1   dfail:0   fail:12  skip:1391 time:7910s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_753/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev3)

[Patchwork]

== Series Details ==

Series: tools/intel_reg: Add reading and writing registers through engine (rev3)
URL   : https://patchwork.freedesktop.org/series/34755/
State : success

== Summary ==

IGT patchset tested on top of latest successful build
19c6c040b11d7a7068c66ae918a88c150243279b lib/igt_kms.c: Unconditionally include poll.h

with latest DRM-Tip kernel build CI_DRM_3618
17fd16225c4d drm-tip: 2018y-01m-10d-09h-37m-51s UTC integration manifest

No testlist changes.

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989
Test kms_frontbuffer_tracking:
        Subgroup basic:
                incomplete -> PASS       (fi-bsw-n3050) fdo#104571
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#104571 https://bugs.freedesktop.org/show_bug.cgi?id=104571
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:424s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:426s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:495s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:490s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:489s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:479s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:461s
fi-elk-e7500     total:224  pass:168  dwarn:9   dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:276s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:515s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:395s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:414s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:462s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:415s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:468s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:502s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:459s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:505s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:580s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:430s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:531s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:505s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:528s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-glk-dsi       total:288  pass:182  dwarn:1   dfail:4   fail:0   skip:101 time:354s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_764/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for tools/intel_reg: Add reading and writing registers through engine (rev3)

[Patchwork]

== Series Details ==

Series: tools/intel_reg: Add reading and writing registers through engine (rev3)
URL   : https://patchwork.freedesktop.org/series/34755/
State : warning

== Summary ==

Warning: bzip CI_DRM_3618/shard-glkb6/results30.json.bz2 wasn't in correct JSON format
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-snb) fdo#104218 +2
Test gem_softpin:
        Subgroup noreloc-s4:
                skip       -> FAIL       (shard-hsw) fdo#103375 +1
Test kms_atomic_transition:
        Subgroup plane-all-modeset-transition:
                pass       -> DMESG-WARN (shard-hsw)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                pass       -> FAIL       (shard-snb) fdo#101623 +1
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-a-planes:
                pass       -> SKIP       (shard-snb) fdo#102365

fdo#104218 https://bugs.freedesktop.org/show_bug.cgi?id=104218
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365

shard-hsw        total:2713 pass:1537 dwarn:2   dfail:0   fail:10  skip:1164 time:9116s
shard-snb        total:2702 pass:1304 dwarn:1   dfail:0   fail:11  skip:1385 time:7616s
Blacklisted hosts:
shard-apl        total:2691 pass:1660 dwarn:1   dfail:0   fail:28  skip:1001 time:13152s
shard-kbl        total:2713 pass:1805 dwarn:1   dfail:0   fail:29  skip:878 time:10581s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_764/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx