* [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
@ 2017-12-01 14:16 Mika Kuoppala
2017-12-01 14:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Mika Kuoppala @ 2017-12-01 14:16 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✗ Fi.CI.BAT: failure for tools/intel_reg: Add reading and writing registers through engine
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
@ 2017-12-01 14:21 ` Patchwork
2017-12-01 14:32 ` [PATCH igt] " Chris Wilson
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-12-01 14:21 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
2017-12-01 14:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2017-12-01 14:32 ` Chris Wilson
2018-01-08 14:12 ` Mika Kuoppala
2018-01-08 14:51 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev2) Patchwork
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2017-12-01 14:32 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Jani Nikula
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2017-12-01 14:32 ` [PATCH igt] " Chris Wilson
@ 2018-01-08 14:12 ` Mika Kuoppala
2018-01-08 14:24 ` Chris Wilson
2018-01-08 14:31 ` Jani Nikula
0 siblings, 2 replies; 14+ messages in thread
From: Mika Kuoppala @ 2018-01-08 14:12 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-08 14:12 ` Mika Kuoppala
@ 2018-01-08 14:24 ` Chris Wilson
2018-01-08 14:31 ` Jani Nikula
1 sibling, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2018-01-08 14:24 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx; +Cc: Jani Nikula
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-08 14:12 ` Mika Kuoppala
2018-01-08 14:24 ` Chris Wilson
@ 2018-01-08 14:31 ` Jani Nikula
2018-01-10 13:42 ` Mika Kuoppala
2018-01-22 16:08 ` Mika Kuoppala
1 sibling, 2 replies; 14+ messages in thread
From: Jani Nikula @ 2018-01-08 14:31 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev2)
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
2017-12-01 14:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-12-01 14:32 ` [PATCH igt] " Chris Wilson
@ 2018-01-08 14:51 ` Patchwork
2018-01-08 19:58 ` ✗ Fi.CI.IGT: warning " Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-01-08 14:51 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.IGT: warning for tools/intel_reg: Add reading and writing registers through engine (rev2)
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
` (2 preceding siblings ...)
2018-01-08 14:51 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev2) Patchwork
@ 2018-01-08 19:58 ` Patchwork
2018-01-10 14:10 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev3) Patchwork
2018-01-10 14:56 ` ✗ Fi.CI.IGT: warning " Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-01-08 19:58 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-08 14:31 ` Jani Nikula
@ 2018-01-10 13:42 ` Mika Kuoppala
2018-01-22 16:08 ` Mika Kuoppala
1 sibling, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2018-01-10 13:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev3)
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
` (3 preceding siblings ...)
2018-01-08 19:58 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-01-10 14:10 ` Patchwork
2018-01-10 14:56 ` ✗ Fi.CI.IGT: warning " Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-01-10 14:10 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* ✗ Fi.CI.IGT: warning for tools/intel_reg: Add reading and writing registers through engine (rev3)
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
` (4 preceding siblings ...)
2018-01-10 14:10 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev3) Patchwork
@ 2018-01-10 14:56 ` Patchwork
5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-01-10 14:56 UTC (permalink / raw)
To: Mika Kuoppala; +Cc: intel-gfx
== 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-08 14:31 ` Jani Nikula
2018-01-10 13:42 ` Mika Kuoppala
@ 2018-01-22 16:08 ` Mika Kuoppala
2018-01-31 12:58 ` Jani Nikula
1 sibling, 1 reply; 14+ messages in thread
From: Mika Kuoppala @ 2018-01-22 16:08 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-22 16:08 ` Mika Kuoppala
@ 2018-01-31 12:58 ` Jani Nikula
2018-01-31 13:29 ` Mika Kuoppala
0 siblings, 1 reply; 14+ messages in thread
From: Jani Nikula @ 2018-01-31 12:58 UTC (permalink / raw)
To: Mika Kuoppala, intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH igt] tools/intel_reg: Add reading and writing registers through engine
2018-01-31 12:58 ` Jani Nikula
@ 2018-01-31 13:29 ` Mika Kuoppala
0 siblings, 0 replies; 14+ messages in thread
From: Mika Kuoppala @ 2018-01-31 13:29 UTC (permalink / raw)
To: Jani Nikula, intel-gfx
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-01-31 13:29 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-01 14:16 [PATCH igt] tools/intel_reg: Add reading and writing registers through engine Mika Kuoppala
2017-12-01 14:21 ` ✗ Fi.CI.BAT: failure for " Patchwork
2017-12-01 14:32 ` [PATCH igt] " Chris Wilson
2018-01-08 14:12 ` Mika Kuoppala
2018-01-08 14:24 ` Chris Wilson
2018-01-08 14:31 ` Jani Nikula
2018-01-10 13:42 ` Mika Kuoppala
2018-01-22 16:08 ` Mika Kuoppala
2018-01-31 12:58 ` Jani Nikula
2018-01-31 13:29 ` Mika Kuoppala
2018-01-08 14:51 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev2) Patchwork
2018-01-08 19:58 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-10 14:10 ` ✓ Fi.CI.BAT: success for tools/intel_reg: Add reading and writing registers through engine (rev3) Patchwork
2018-01-10 14:56 ` ✗ Fi.CI.IGT: warning " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).