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

Jani Nikula jani.nikula at intel.com
Wed Jan 31 12:58:25 UTC 2018


On Mon, 22 Jan 2018, Mika Kuoppala <mika.kuoppala at linux.intel.com> wrote:
> 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.

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

Acked-by: Jani Nikula <jani.nikula at 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 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

-- 
Jani Nikula, Intel Open Source Technology Center


More information about the Intel-gfx mailing list