[igt-dev] [PATCH i-g-t 4/6] igt/gem_exec_nop: Drip feed nops

[Katarzyna Dec]

On Tue, Jun 19, 2018 at 11:49:18AM +0100, Chris Wilson wrote:
Few questions below.

> Wait until the previous nop batch is running before submitting the next.
> This prevents the kernel from batching up sequential requests into a
> a ringfull, more strenuous exercising the "lite-restore" execution path.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  tests/gem_exec_nop.c | 146 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 142 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
> index 50f0a3aad..0523b1c02 100644
> --- a/tests/gem_exec_nop.c
> +++ b/tests/gem_exec_nop.c
> @@ -104,6 +104,129 @@ static double nop_on_ring(int fd, uint32_t handle, unsigned ring_id,
>  	return elapsed(&start, &now);
>  }
>  
> +static void poll_ring(int fd, unsigned ring, const char *name, int timeout)
> +{
> +	const int gen = intel_gen(intel_get_drm_devid(fd));
> +	const uint32_t MI_ARB_CHK = 0x5 << 23;
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	struct drm_i915_gem_exec_object2 obj;
> +	struct drm_i915_gem_relocation_entry reloc[4], *r;
> +	uint32_t *bbe[2], *state, *batch;
> +	unsigned engines[16], nengine, flags;
> +	struct timespec tv = {};
> +	unsigned long cycles;
> +	uint64_t elapsed;
> +
> +	flags = I915_EXEC_NO_RELOC;
This flag means we will prepare relocations table for kernel?

> +	if (gen == 4 || gen == 5)
> +		flags |= I915_EXEC_SECURE;
> +
> +	nengine = 0;
> +	if (ring == ALL_ENGINES) {
> +		for_each_physical_engine(fd, ring) {
> +			if (!gem_can_store_dword(fd, ring))
> +				continue;
> +
> +			engines[nengine++] = ring;
> +		}
> +	} else {
> +		gem_require_ring(fd, ring);
> +		igt_require(gem_can_store_dword(fd, ring));
> +		engines[nengine++] = ring;
> +	}
> +	igt_require(nengine);
> +
> +	memset(&obj, 0, sizeof(obj));
> +	obj.handle = gem_create(fd, 4096);
> +	obj.relocs_ptr = to_user_pointer(reloc);
> +	obj.relocation_count = ARRAY_SIZE(reloc);
> +
> +	r = memset(reloc, 0, sizeof(reloc));
> +	batch = gem_mmap__wc(fd, obj.handle, 0, 4096, PROT_WRITE);
> +
> +	for (unsigned int start_offset = 0;
> +	     start_offset <= 128;
> +	     start_offset += 128) {
It looks like this loop will run only once. Why to use such 'strange'
values and why we need loop here?
> +		uint32_t *b = batch + start_offset / sizeof(*batch);
I am curious why in b we add batch and below in r->offset we subtract it?
> +
> +		r->target_handle = obj.handle;
> +		r->offset = (b - batch + 1) * sizeof(uint32_t);

> +		r->delta = 4092;
> +		r->read_domains = I915_GEM_DOMAIN_RENDER;
> +
> +		*b = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
> +		if (gen >= 8) {
> +			*++b = r->delta;
> +			*++b = 0;
> +		} else if (gen >= 4) {
> +			r->offset += sizeof(uint32_t);
> +			*++b = 0;
> +			*++b = r->delta;
> +		} else {
> +			*b -= 1;
> +			*++b = r->delta;
> +		}
> +		*++b = start_offset != 0;
> +		r++;
> +
Could you explain why we need such 'hacky' batch settings?^^^
> +		b = batch + (start_offset + 64) / sizeof(*batch);
> +		bbe[start_offset != 0] = b;
> +		*b++ = MI_ARB_CHK;
> +
> +		r->target_handle = obj.handle;
> +		r->offset = (b - batch + 1) * sizeof(uint32_t);
> +		r->read_domains = I915_GEM_DOMAIN_COMMAND;
Why do we need to change domain from render to command?
> +		r->delta = start_offset + 64;
> +		if (gen >= 8) {
> +			*b++ = MI_BATCH_BUFFER_START | 1 << 8 | 1;
> +			*b++ = r->delta;
> +			*b++ = 0;
> +		} else if (gen >= 6) {
> +			*b++ = MI_BATCH_BUFFER_START | 1 << 8;
> +			*b++ = r->delta;
> +		} else {
> +			*b++ = MI_BATCH_BUFFER_START | 2 << 6;
> +			if (gen < 4)
> +				r->delta |= 1;
> +			*b++ = r->delta;
> +		}
> +		r++;
> +	}
> +	igt_assert(r == reloc + ARRAY_SIZE(reloc));
> +	state = batch + 1023;
> +
> +	memset(&execbuf, 0, sizeof(execbuf));
> +	execbuf.buffers_ptr = to_user_pointer(&obj);
If I understand correctly obj is 'containing' previously prepared batch, right?

I hope my questions are not that silly.
Kasia :)