[Intel-gfx] [PATCH igt 04/10] igt/gem_exec_schedule: Exercise reordering with many priority levels

[Chris Wilson]

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