On Mon, 22 Jan 2018, Mika Kuoppala <[email protected]> wrote: > Jani Nikula <[email protected]> writes: > >> On Mon, 08 Jan 2018, Mika Kuoppala <[email protected]> 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 <[email protected]> > > -Mika > >> BR, >> Jani. >> >> >>> >>> v2: no MI_NOOP after BBE (Chris) >>> v3: use modern engine names (Chris), use global fd >>> >>> Cc: Jani Nikula <[email protected]> >>> Cc: Chris Wilson <[email protected]> >>> CC: Joonas Lahtinen <[email protected]> >>> Signed-off-by: Mika Kuoppala <[email protected]> >>> --- >>> 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
