| Message ID | 20180423134345.28153-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23/04/18 06:43, Chris Wilson wrote: > In the existing ABI, each engine operates its own timeline > (fence.context) and so should execute independently of any other. If we > install a blocker on all other engines, that should not affect execution > on the local engine. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > +static void independent(int fd, unsigned int engine) > +{ > + IGT_CORK_HANDLE(cork); > + uint32_t scratch, plug, batch; > + igt_spin_t *spin = NULL; > + unsigned int other; > + uint32_t *ptr; > + > + igt_require(engine != 0); > + > + scratch = gem_create(fd, 4096); > + plug = igt_cork_plug(&cork, fd); > + > + /* Check that we can submit to engine while all others are blocked */ > + for_each_physical_engine(fd, other) { > + if (other == engine) > + continue; > + > + if (spin == NULL) { > + spin = __igt_spin_batch_new(fd, 0, other, 0); > + } else { > + struct drm_i915_gem_exec_object2 obj = { > + .handle = spin->handle, > + }; > + struct drm_i915_gem_execbuffer2 eb = { > + .buffer_count = 1, > + .buffers_ptr = to_user_pointer(&obj), > + .flags = other, > + }; > + gem_execbuf(fd, &eb); > + } > + > + store_dword(fd, 0, other, scratch, 0, other, plug, 0); > + } > + igt_require(spin); > + > + /* Same priority, but different timeline (as different engine) */ > + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); It would be interesting to check that priority scheduling/preemption is still happening on the free engine. > + > + unplug_show_queue(fd, &cork, engine); > + gem_close(fd, plug); > + > + gem_sync(fd, batch); > + gem_close(fd, batch); > + igt_assert(gem_bo_busy(fd, spin->handle)); > + > + ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ); > + gem_close(fd, scratch); > + igt_assert_eq(ptr[0], engine); > + > + igt_spin_batch_free(fd, spin); > + gem_quiescent_gpu(fd); > + > + /* And we expect the others to have overwritten us, order unspecified */ > + igt_assert_neq(ptr[0], engine); > + munmap(ptr, 4096); > +} > + > static void smoketest(int fd, unsigned ring, unsigned timeout) > { > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); > @@ -1070,10 +1138,16 @@ igt_main > if (e->exec_id == 0) > continue; > > - igt_subtest_f("fifo-%s", e->name) { > - igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); > - igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); > - fifo(fd, e->exec_id | e->flags); > + igt_subtest_group { > + igt_fixture { > + igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); I thought that we added this to the subtests to have a constant list of tests for all platforms. Thanks, Antonio > + igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); > + } > + > + igt_subtest_f("fifo-%s", e->name) > + fifo(fd, e->exec_id | e->flags); > + igt_subtest_f("independent-%s", e->name) > + independent(fd, e->exec_id | e->flags); > } > } > } >
Quoting Antonio Argenziano (2018-04-23 16:37:17) > > > On 23/04/18 06:43, Chris Wilson wrote: > > In the existing ABI, each engine operates its own timeline > > (fence.context) and so should execute independently of any other. If we > > install a blocker on all other engines, that should not affect execution > > on the local engine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > +static void independent(int fd, unsigned int engine) > > +{ > > + IGT_CORK_HANDLE(cork); > > + uint32_t scratch, plug, batch; > > + igt_spin_t *spin = NULL; > > + unsigned int other; > > + uint32_t *ptr; > > + > > + igt_require(engine != 0); > > + > > + scratch = gem_create(fd, 4096); > > + plug = igt_cork_plug(&cork, fd); > > + > > + /* Check that we can submit to engine while all others are blocked */ > > + for_each_physical_engine(fd, other) { > > + if (other == engine) > > + continue; > > + > > + if (spin == NULL) { > > + spin = __igt_spin_batch_new(fd, 0, other, 0); > > + } else { > > + struct drm_i915_gem_exec_object2 obj = { > > + .handle = spin->handle, > > + }; > > + struct drm_i915_gem_execbuffer2 eb = { > > + .buffer_count = 1, > > + .buffers_ptr = to_user_pointer(&obj), > > + .flags = other, > > + }; > > + gem_execbuf(fd, &eb); > > + } > > + > > + store_dword(fd, 0, other, scratch, 0, other, plug, 0); > > + } > > + igt_require(spin); > > + > > + /* Same priority, but different timeline (as different engine) */ > > + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); > > It would be interesting to check that priority scheduling/preemption is > still happening on the free engine. It's being run on machines without scheduling as well. Reordering tests are later; not sure if I care about reordering while blocking, that's an entirely different set of tests being worked on for queues. -Chris
On 23/04/18 08:51, Chris Wilson wrote: > Quoting Antonio Argenziano (2018-04-23 16:37:17) >> >> >> On 23/04/18 06:43, Chris Wilson wrote: >>> In the existing ABI, each engine operates its own timeline >>> (fence.context) and so should execute independently of any other. If we >>> install a blocker on all other engines, that should not affect execution >>> on the local engine. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >>> +static void independent(int fd, unsigned int engine) >>> +{ >>> + IGT_CORK_HANDLE(cork); >>> + uint32_t scratch, plug, batch; >>> + igt_spin_t *spin = NULL; >>> + unsigned int other; >>> + uint32_t *ptr; >>> + >>> + igt_require(engine != 0); >>> + >>> + scratch = gem_create(fd, 4096); >>> + plug = igt_cork_plug(&cork, fd); >>> + >>> + /* Check that we can submit to engine while all others are blocked */ >>> + for_each_physical_engine(fd, other) { >>> + if (other == engine) >>> + continue; >>> + >>> + if (spin == NULL) { >>> + spin = __igt_spin_batch_new(fd, 0, other, 0); >>> + } else { >>> + struct drm_i915_gem_exec_object2 obj = { >>> + .handle = spin->handle, >>> + }; >>> + struct drm_i915_gem_execbuffer2 eb = { >>> + .buffer_count = 1, >>> + .buffers_ptr = to_user_pointer(&obj), >>> + .flags = other, >>> + }; >>> + gem_execbuf(fd, &eb); >>> + } >>> + >>> + store_dword(fd, 0, other, scratch, 0, other, plug, 0); >>> + } >>> + igt_require(spin); >>> + >>> + /* Same priority, but different timeline (as different engine) */ >>> + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); >> >> It would be interesting to check that priority scheduling/preemption is >> still happening on the free engine. > > It's being run on machines without scheduling as well. Reordering tests > are later; not sure if I care about reordering while blocking, that's an > entirely different set of tests being worked on for queues. Cool, a different set of tests is what I had in mind as well :). Oh BTW, with the igt_require in the subtests this is: Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com> Thanks, Antonio > -Chris >
On 23/04/2018 14:43, Chris Wilson wrote: > In the existing ABI, each engine operates its own timeline > (fence.context) and so should execute independently of any other. If we > install a blocker on all other engines, that should not affect execution > on the local engine. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > tests/gem_exec_schedule.c | 90 +++++++++++++++++++++++++++++++++++---- > 1 file changed, 82 insertions(+), 8 deletions(-) > > diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c > index 5d0f215b2..471275169 100644 > --- a/tests/gem_exec_schedule.c > +++ b/tests/gem_exec_schedule.c > @@ -49,9 +49,9 @@ > > IGT_TEST_DESCRIPTION("Check that we can control the order of execution"); > > -static void store_dword(int fd, uint32_t ctx, unsigned ring, > - uint32_t target, uint32_t offset, uint32_t value, > - uint32_t cork, unsigned write_domain) > +static uint32_t __store_dword(int fd, uint32_t ctx, unsigned ring, > + uint32_t target, uint32_t offset, uint32_t value, > + uint32_t cork, unsigned write_domain) > { > const int gen = intel_gen(intel_get_drm_devid(fd)); > struct drm_i915_gem_exec_object2 obj[3]; > @@ -100,7 +100,17 @@ static void store_dword(int fd, uint32_t ctx, unsigned ring, > batch[++i] = MI_BATCH_BUFFER_END; > gem_write(fd, obj[2].handle, 0, batch, sizeof(batch)); > gem_execbuf(fd, &execbuf); > - gem_close(fd, obj[2].handle); > + > + return obj[2].handle; > +} > + > +static void store_dword(int fd, uint32_t ctx, unsigned ring, > + uint32_t target, uint32_t offset, uint32_t value, > + uint32_t cork, unsigned write_domain) > +{ > + gem_close(fd, __store_dword(fd, ctx, ring, > + target, offset, value, > + cork, write_domain)); > } > > static uint32_t create_highest_priority(int fd) > @@ -161,6 +171,64 @@ static void fifo(int fd, unsigned ring) > munmap(ptr, 4096); > } > > +static void independent(int fd, unsigned int engine) > +{ > + IGT_CORK_HANDLE(cork); > + uint32_t scratch, plug, batch; > + igt_spin_t *spin = NULL; > + unsigned int other; > + uint32_t *ptr; > + > + igt_require(engine != 0); > + > + scratch = gem_create(fd, 4096); > + plug = igt_cork_plug(&cork, fd); > + > + /* Check that we can submit to engine while all others are blocked */ > + for_each_physical_engine(fd, other) { > + if (other == engine) > + continue; > + > + if (spin == NULL) { > + spin = __igt_spin_batch_new(fd, 0, other, 0); > + } else { > + struct drm_i915_gem_exec_object2 obj = { > + .handle = spin->handle, > + }; > + struct drm_i915_gem_execbuffer2 eb = { > + .buffer_count = 1, > + .buffers_ptr = to_user_pointer(&obj), > + .flags = other, > + }; > + gem_execbuf(fd, &eb); > + } > + > + store_dword(fd, 0, other, scratch, 0, other, plug, 0); > + } > + igt_require(spin); > + > + /* Same priority, but different timeline (as different engine) */ > + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); > + > + unplug_show_queue(fd, &cork, engine); > + gem_close(fd, plug); > + > + gem_sync(fd, batch); > + gem_close(fd, batch); Strictly speaking I think you need to use the poll-able spinner and wait on it here, before the busy assert. It's unlikely, but spinners on 'other' engines are getting submitted async to the store dword batch on 'engine'. > + igt_assert(gem_bo_busy(fd, spin->handle)); > + > + ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ); > + gem_close(fd, scratch); > + igt_assert_eq(ptr[0], engine); > + > + igt_spin_batch_free(fd, spin); > + gem_quiescent_gpu(fd); > + > + /* And we expect the others to have overwritten us, order unspecified */ > + igt_assert_neq(ptr[0], engine); > + munmap(ptr, 4096); > +} > + > static void smoketest(int fd, unsigned ring, unsigned timeout) > { > const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); > @@ -1070,10 +1138,16 @@ igt_main > if (e->exec_id == 0) > continue; > > - igt_subtest_f("fifo-%s", e->name) { > - igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); > - igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); > - fifo(fd, e->exec_id | e->flags); > + igt_subtest_group { > + igt_fixture { > + igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); > + igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); > + } > + > + igt_subtest_f("fifo-%s", e->name) > + fifo(fd, e->exec_id | e->flags); > + igt_subtest_f("independent-%s", e->name) > + independent(fd, e->exec_id | e->flags); > } > } > } > Apart from what Antonio already commented, and waiting on spinner, looks OK to me. Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-04-23 17:52:54) > > On 23/04/2018 14:43, Chris Wilson wrote: > > In the existing ABI, each engine operates its own timeline > > (fence.context) and so should execute independently of any other. If we > > install a blocker on all other engines, that should not affect execution > > on the local engine. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > tests/gem_exec_schedule.c | 90 +++++++++++++++++++++++++++++++++++---- > > 1 file changed, 82 insertions(+), 8 deletions(-) > > > > diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c > > index 5d0f215b2..471275169 100644 > > --- a/tests/gem_exec_schedule.c > > +++ b/tests/gem_exec_schedule.c > > @@ -49,9 +49,9 @@ > > > > IGT_TEST_DESCRIPTION("Check that we can control the order of execution"); > > > > -static void store_dword(int fd, uint32_t ctx, unsigned ring, > > - uint32_t target, uint32_t offset, uint32_t value, > > - uint32_t cork, unsigned write_domain) > > +static uint32_t __store_dword(int fd, uint32_t ctx, unsigned ring, > > + uint32_t target, uint32_t offset, uint32_t value, > > + uint32_t cork, unsigned write_domain) > > { > > const int gen = intel_gen(intel_get_drm_devid(fd)); > > struct drm_i915_gem_exec_object2 obj[3]; > > @@ -100,7 +100,17 @@ static void store_dword(int fd, uint32_t ctx, unsigned ring, > > batch[++i] = MI_BATCH_BUFFER_END; > > gem_write(fd, obj[2].handle, 0, batch, sizeof(batch)); > > gem_execbuf(fd, &execbuf); > > - gem_close(fd, obj[2].handle); > > + > > + return obj[2].handle; > > +} > > + > > +static void store_dword(int fd, uint32_t ctx, unsigned ring, > > + uint32_t target, uint32_t offset, uint32_t value, > > + uint32_t cork, unsigned write_domain) > > +{ > > + gem_close(fd, __store_dword(fd, ctx, ring, > > + target, offset, value, > > + cork, write_domain)); > > } > > > > static uint32_t create_highest_priority(int fd) > > @@ -161,6 +171,64 @@ static void fifo(int fd, unsigned ring) > > munmap(ptr, 4096); > > } > > > > +static void independent(int fd, unsigned int engine) > > +{ > > + IGT_CORK_HANDLE(cork); > > + uint32_t scratch, plug, batch; > > + igt_spin_t *spin = NULL; > > + unsigned int other; > > + uint32_t *ptr; > > + > > + igt_require(engine != 0); > > + > > + scratch = gem_create(fd, 4096); > > + plug = igt_cork_plug(&cork, fd); > > + > > + /* Check that we can submit to engine while all others are blocked */ > > + for_each_physical_engine(fd, other) { > > + if (other == engine) > > + continue; > > + > > + if (spin == NULL) { > > + spin = __igt_spin_batch_new(fd, 0, other, 0); > > + } else { > > + struct drm_i915_gem_exec_object2 obj = { > > + .handle = spin->handle, > > + }; > > + struct drm_i915_gem_execbuffer2 eb = { > > + .buffer_count = 1, > > + .buffers_ptr = to_user_pointer(&obj), > > + .flags = other, > > + }; > > + gem_execbuf(fd, &eb); > > + } > > + > > + store_dword(fd, 0, other, scratch, 0, other, plug, 0); > > + } > > + igt_require(spin); > > + > > + /* Same priority, but different timeline (as different engine) */ > > + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); > > + > > + unplug_show_queue(fd, &cork, engine); > > + gem_close(fd, plug); > > + > > + gem_sync(fd, batch); > > + gem_close(fd, batch); > > Strictly speaking I think you need to use the poll-able spinner and wait > on it here, before the busy assert. It's unlikely, but spinners on > 'other' engines are getting submitted async to the store dword batch on > 'engine'. We've waited for its completion, so we know batch is idle and the others are still busy. We then check its seqno is written to the scratch; so using pollable here is redundant. And then we check that the others are run after. -Chris
On 23/04/2018 18:08, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2018-04-23 17:52:54) >> >> On 23/04/2018 14:43, Chris Wilson wrote: >>> In the existing ABI, each engine operates its own timeline >>> (fence.context) and so should execute independently of any other. If we >>> install a blocker on all other engines, that should not affect execution >>> on the local engine. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >>> --- >>> tests/gem_exec_schedule.c | 90 +++++++++++++++++++++++++++++++++++---- >>> 1 file changed, 82 insertions(+), 8 deletions(-) >>> >>> diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c >>> index 5d0f215b2..471275169 100644 >>> --- a/tests/gem_exec_schedule.c >>> +++ b/tests/gem_exec_schedule.c >>> @@ -49,9 +49,9 @@ >>> >>> IGT_TEST_DESCRIPTION("Check that we can control the order of execution"); >>> >>> -static void store_dword(int fd, uint32_t ctx, unsigned ring, >>> - uint32_t target, uint32_t offset, uint32_t value, >>> - uint32_t cork, unsigned write_domain) >>> +static uint32_t __store_dword(int fd, uint32_t ctx, unsigned ring, >>> + uint32_t target, uint32_t offset, uint32_t value, >>> + uint32_t cork, unsigned write_domain) >>> { >>> const int gen = intel_gen(intel_get_drm_devid(fd)); >>> struct drm_i915_gem_exec_object2 obj[3]; >>> @@ -100,7 +100,17 @@ static void store_dword(int fd, uint32_t ctx, unsigned ring, >>> batch[++i] = MI_BATCH_BUFFER_END; >>> gem_write(fd, obj[2].handle, 0, batch, sizeof(batch)); >>> gem_execbuf(fd, &execbuf); >>> - gem_close(fd, obj[2].handle); >>> + >>> + return obj[2].handle; >>> +} >>> + >>> +static void store_dword(int fd, uint32_t ctx, unsigned ring, >>> + uint32_t target, uint32_t offset, uint32_t value, >>> + uint32_t cork, unsigned write_domain) >>> +{ >>> + gem_close(fd, __store_dword(fd, ctx, ring, >>> + target, offset, value, >>> + cork, write_domain)); >>> } >>> >>> static uint32_t create_highest_priority(int fd) >>> @@ -161,6 +171,64 @@ static void fifo(int fd, unsigned ring) >>> munmap(ptr, 4096); >>> } >>> >>> +static void independent(int fd, unsigned int engine) >>> +{ >>> + IGT_CORK_HANDLE(cork); >>> + uint32_t scratch, plug, batch; >>> + igt_spin_t *spin = NULL; >>> + unsigned int other; >>> + uint32_t *ptr; >>> + >>> + igt_require(engine != 0); >>> + >>> + scratch = gem_create(fd, 4096); >>> + plug = igt_cork_plug(&cork, fd); >>> + >>> + /* Check that we can submit to engine while all others are blocked */ >>> + for_each_physical_engine(fd, other) { >>> + if (other == engine) >>> + continue; >>> + >>> + if (spin == NULL) { >>> + spin = __igt_spin_batch_new(fd, 0, other, 0); >>> + } else { >>> + struct drm_i915_gem_exec_object2 obj = { >>> + .handle = spin->handle, >>> + }; >>> + struct drm_i915_gem_execbuffer2 eb = { >>> + .buffer_count = 1, >>> + .buffers_ptr = to_user_pointer(&obj), >>> + .flags = other, >>> + }; >>> + gem_execbuf(fd, &eb); >>> + } >>> + >>> + store_dword(fd, 0, other, scratch, 0, other, plug, 0); >>> + } >>> + igt_require(spin); >>> + >>> + /* Same priority, but different timeline (as different engine) */ >>> + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); >>> + >>> + unplug_show_queue(fd, &cork, engine); >>> + gem_close(fd, plug); >>> + >>> + gem_sync(fd, batch); >>> + gem_close(fd, batch); >> >> Strictly speaking I think you need to use the poll-able spinner and wait >> on it here, before the busy assert. It's unlikely, but spinners on >> 'other' engines are getting submitted async to the store dword batch on >> 'engine'. > > We've waited for its completion, so we know batch is idle and the others > are still busy. We then check its seqno is written to the scratch; so > using pollable here is redundant. And then we check that the others are > run after. Yeah I was confused, thinking busy check on spinner could return false if the respective tasklet on those engines hadn't ran yet - but of course busy is true immediately after execbuf so as I said, total confusion. Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-04-24 09:55:20) > > On 23/04/2018 18:08, Chris Wilson wrote: > > Quoting Tvrtko Ursulin (2018-04-23 17:52:54) > >> > >> On 23/04/2018 14:43, Chris Wilson wrote: > >>> In the existing ABI, each engine operates its own timeline > >>> (fence.context) and so should execute independently of any other. If we > >>> install a blocker on all other engines, that should not affect execution > >>> on the local engine. > >>> > >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >>> --- > >>> tests/gem_exec_schedule.c | 90 +++++++++++++++++++++++++++++++++++---- > >>> 1 file changed, 82 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c > >>> index 5d0f215b2..471275169 100644 > >>> --- a/tests/gem_exec_schedule.c > >>> +++ b/tests/gem_exec_schedule.c > >>> @@ -49,9 +49,9 @@ > >>> > >>> IGT_TEST_DESCRIPTION("Check that we can control the order of execution"); > >>> > >>> -static void store_dword(int fd, uint32_t ctx, unsigned ring, > >>> - uint32_t target, uint32_t offset, uint32_t value, > >>> - uint32_t cork, unsigned write_domain) > >>> +static uint32_t __store_dword(int fd, uint32_t ctx, unsigned ring, > >>> + uint32_t target, uint32_t offset, uint32_t value, > >>> + uint32_t cork, unsigned write_domain) > >>> { > >>> const int gen = intel_gen(intel_get_drm_devid(fd)); > >>> struct drm_i915_gem_exec_object2 obj[3]; > >>> @@ -100,7 +100,17 @@ static void store_dword(int fd, uint32_t ctx, unsigned ring, > >>> batch[++i] = MI_BATCH_BUFFER_END; > >>> gem_write(fd, obj[2].handle, 0, batch, sizeof(batch)); > >>> gem_execbuf(fd, &execbuf); > >>> - gem_close(fd, obj[2].handle); > >>> + > >>> + return obj[2].handle; > >>> +} > >>> + > >>> +static void store_dword(int fd, uint32_t ctx, unsigned ring, > >>> + uint32_t target, uint32_t offset, uint32_t value, > >>> + uint32_t cork, unsigned write_domain) > >>> +{ > >>> + gem_close(fd, __store_dword(fd, ctx, ring, > >>> + target, offset, value, > >>> + cork, write_domain)); > >>> } > >>> > >>> static uint32_t create_highest_priority(int fd) > >>> @@ -161,6 +171,64 @@ static void fifo(int fd, unsigned ring) > >>> munmap(ptr, 4096); > >>> } > >>> > >>> +static void independent(int fd, unsigned int engine) > >>> +{ > >>> + IGT_CORK_HANDLE(cork); > >>> + uint32_t scratch, plug, batch; > >>> + igt_spin_t *spin = NULL; > >>> + unsigned int other; > >>> + uint32_t *ptr; > >>> + > >>> + igt_require(engine != 0); > >>> + > >>> + scratch = gem_create(fd, 4096); > >>> + plug = igt_cork_plug(&cork, fd); > >>> + > >>> + /* Check that we can submit to engine while all others are blocked */ > >>> + for_each_physical_engine(fd, other) { > >>> + if (other == engine) > >>> + continue; > >>> + > >>> + if (spin == NULL) { > >>> + spin = __igt_spin_batch_new(fd, 0, other, 0); > >>> + } else { > >>> + struct drm_i915_gem_exec_object2 obj = { > >>> + .handle = spin->handle, > >>> + }; > >>> + struct drm_i915_gem_execbuffer2 eb = { > >>> + .buffer_count = 1, > >>> + .buffers_ptr = to_user_pointer(&obj), > >>> + .flags = other, > >>> + }; > >>> + gem_execbuf(fd, &eb); > >>> + } > >>> + > >>> + store_dword(fd, 0, other, scratch, 0, other, plug, 0); > >>> + } > >>> + igt_require(spin); > >>> + > >>> + /* Same priority, but different timeline (as different engine) */ > >>> + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); > >>> + > >>> + unplug_show_queue(fd, &cork, engine); > >>> + gem_close(fd, plug); > >>> + > >>> + gem_sync(fd, batch); > >>> + gem_close(fd, batch); > >> > >> Strictly speaking I think you need to use the poll-able spinner and wait > >> on it here, before the busy assert. It's unlikely, but spinners on > >> 'other' engines are getting submitted async to the store dword batch on > >> 'engine'. > > > > We've waited for its completion, so we know batch is idle and the others > > are still busy. We then check its seqno is written to the scratch; so > > using pollable here is redundant. And then we check that the others are > > run after. > > Yeah I was confused, thinking busy check on spinner could return false > if the respective tasklet on those engines hadn't ran yet - but of > course busy is true immediately after execbuf so as I said, total confusion. So having checked the logic should produce N-1 blocked engines and ourselves free to complete; care for the r-b so we can add it to the mix of tests? -Chris
diff --git a/tests/gem_exec_schedule.c b/tests/gem_exec_schedule.c index 5d0f215b2..471275169 100644 --- a/tests/gem_exec_schedule.c +++ b/tests/gem_exec_schedule.c @@ -49,9 +49,9 @@ IGT_TEST_DESCRIPTION("Check that we can control the order of execution"); -static void store_dword(int fd, uint32_t ctx, unsigned ring, - uint32_t target, uint32_t offset, uint32_t value, - uint32_t cork, unsigned write_domain) +static uint32_t __store_dword(int fd, uint32_t ctx, unsigned ring, + uint32_t target, uint32_t offset, uint32_t value, + uint32_t cork, unsigned write_domain) { const int gen = intel_gen(intel_get_drm_devid(fd)); struct drm_i915_gem_exec_object2 obj[3]; @@ -100,7 +100,17 @@ static void store_dword(int fd, uint32_t ctx, unsigned ring, batch[++i] = MI_BATCH_BUFFER_END; gem_write(fd, obj[2].handle, 0, batch, sizeof(batch)); gem_execbuf(fd, &execbuf); - gem_close(fd, obj[2].handle); + + return obj[2].handle; +} + +static void store_dword(int fd, uint32_t ctx, unsigned ring, + uint32_t target, uint32_t offset, uint32_t value, + uint32_t cork, unsigned write_domain) +{ + gem_close(fd, __store_dword(fd, ctx, ring, + target, offset, value, + cork, write_domain)); } static uint32_t create_highest_priority(int fd) @@ -161,6 +171,64 @@ static void fifo(int fd, unsigned ring) munmap(ptr, 4096); } +static void independent(int fd, unsigned int engine) +{ + IGT_CORK_HANDLE(cork); + uint32_t scratch, plug, batch; + igt_spin_t *spin = NULL; + unsigned int other; + uint32_t *ptr; + + igt_require(engine != 0); + + scratch = gem_create(fd, 4096); + plug = igt_cork_plug(&cork, fd); + + /* Check that we can submit to engine while all others are blocked */ + for_each_physical_engine(fd, other) { + if (other == engine) + continue; + + if (spin == NULL) { + spin = __igt_spin_batch_new(fd, 0, other, 0); + } else { + struct drm_i915_gem_exec_object2 obj = { + .handle = spin->handle, + }; + struct drm_i915_gem_execbuffer2 eb = { + .buffer_count = 1, + .buffers_ptr = to_user_pointer(&obj), + .flags = other, + }; + gem_execbuf(fd, &eb); + } + + store_dword(fd, 0, other, scratch, 0, other, plug, 0); + } + igt_require(spin); + + /* Same priority, but different timeline (as different engine) */ + batch = __store_dword(fd, 0, engine, scratch, 0, engine, plug, 0); + + unplug_show_queue(fd, &cork, engine); + gem_close(fd, plug); + + gem_sync(fd, batch); + gem_close(fd, batch); + igt_assert(gem_bo_busy(fd, spin->handle)); + + ptr = gem_mmap__gtt(fd, scratch, 4096, PROT_READ); + gem_close(fd, scratch); + igt_assert_eq(ptr[0], engine); + + igt_spin_batch_free(fd, spin); + gem_quiescent_gpu(fd); + + /* And we expect the others to have overwritten us, order unspecified */ + igt_assert_neq(ptr[0], engine); + munmap(ptr, 4096); +} + static void smoketest(int fd, unsigned ring, unsigned timeout) { const int ncpus = sysconf(_SC_NPROCESSORS_ONLN); @@ -1070,10 +1138,16 @@ igt_main if (e->exec_id == 0) continue; - igt_subtest_f("fifo-%s", e->name) { - igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); - igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); - fifo(fd, e->exec_id | e->flags); + igt_subtest_group { + igt_fixture { + igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags)); + igt_require(gem_can_store_dword(fd, e->exec_id) | e->flags); + } + + igt_subtest_f("fifo-%s", e->name) + fifo(fd, e->exec_id | e->flags); + igt_subtest_f("independent-%s", e->name) + independent(fd, e->exec_id | e->flags); } } }
In the existing ABI, each engine operates its own timeline (fence.context) and so should execute independently of any other. If we install a blocker on all other engines, that should not affect execution on the local engine. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- tests/gem_exec_schedule.c | 90 +++++++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 8 deletions(-)