Re: [PATCH igt 04/10] igt/gem_exec_schedule: Exercise reordering with many priority levels

Chris Wilson
2017-07-31T15:21:10Z

Quoting Tvrtko Ursulin (2017-07-31 15:35:45)
>
> On 28/07/2017 13:08, Chris Wilson wrote:
> > +static void reorder_wide(int fd, unsigned ring)
> > +{
> > + const int gen = intel_gen(intel_get_drm_devid(fd));
> > + struct drm_i915_gem_relocation_entry reloc;
> > + struct drm_i915_gem_exec_object2 obj[3];
> > + struct drm_i915_gem_execbuffer2 execbuf;
> > + struct cork cork;
> > + uint32_t result, target;
> > + uint32_t *busy;
> > + uint32_t *r, *t;
> > +
> > + result = gem_create(fd, 4096);
> > + target = gem_create(fd, 4096);
> > +
> > + busy = make_busy(fd, result, ring);
>
> What does make_busy do? It submits eight magic batches which I guess
> will not finish until finish_busy? But why eight of them? And..
>
> > + plug(fd, &cork);
>
> ... why do we need that since we also control when the below will be
> runnable via this?

Does it? We only need vgem for the test, make_busy was a precursor that
only works for current and previous execlists.

> I think it is time to put some more comments in IGTs to help other
> people looking at the code. High level description of a subtest at
> least, plus a few notes on the implementation approach.
>
> > +
> > + t = gem_mmap__cpu(fd, target, 0, 4096, PROT_WRITE);
> > + gem_set_domain(fd, target, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU);
> > +
> > + memset(obj, 0, sizeof(obj));
> > + obj[0].handle = cork.handle;
> > + obj[1].handle = result;
> > + obj[2].relocs_ptr = to_user_pointer(&reloc);
> > + obj[2].relocation_count = 1;
> > +
> > + memset(&reloc, 0, sizeof(reloc));
> > + reloc.target_handle = result;
> > + reloc.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
> > + reloc.write_domain = 0; /* lies */
> > +
> > + memset(&execbuf, 0, sizeof(execbuf));
> > + execbuf.buffers_ptr = to_user_pointer(obj);
> > + execbuf.buffer_count = 3;
> > + execbuf.flags = ring;
> > + if (gen < 6)
> > + execbuf.flags |= I915_EXEC_SECURE;
> > +
> > + for (int n = -MAX_PRIO, x = 1; n <= MAX_PRIO; n++, x++) {
> > + uint32_t *batch;
> > +
> > + execbuf.rsvd1 = gem_context_create(fd);
> > + ctx_set_priority(fd, execbuf.rsvd1, n);
> > +
> > + obj[2].handle = gem_create(fd, 128 * 64);
>
> What is the significance od 128 and 64?

128 is an approximation for ring_size and 64 is the aligned size of each
batch (ilk imposes the harshest alignment restriction of batch offsets).

> > +
> > + gem_execbuf(fd, &execbuf);
> > + }
> > +
> > + munmap(batch, 128 * 64);
> > + gem_close(fd, obj[2].handle);
> > + gem_context_destroy(fd, execbuf.rsvd1);
>
> Does the ABI guarantee this field will be preserved?

For two reasons:

- we only use gem_execbuf() so the execbuf arg is read-only for the
kernel

- it's an input parameter that has to maintained so that userspace can
loop over the ioctl to handle errors.

-Chris