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

Mika Kuoppala mika.kuoppala at linux.intel.com
Mon Jan 22 16:08:57 UTC 2018


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

> On Mon, 08 Jan 2018, Mika Kuoppala <mika.kuoppala at 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 at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> CC: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala at 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


More information about the Intel-gfx mailing list