[igt-dev] [PATCH i-g-t] i915/gem_exec_schedule: Check deps along implicit inter-engine semaphores

[Chris Wilson]

Quoting Daniele Ceraolo Spurio (2019-04-24 18:53:08)
> 
> 
> On 4/24/19 8:47 AM, Chris Wilson wrote:
> > Given an implicit semaphore from one engine to the next, check that if
> > we skip the wait on that semaphore the following batch although
> > submitted early (as it depends along the single engine timeline) is not
> > executed ahead of its dependency.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> > Cc: Matthew Brost <matthew.brost at intel.com>
> 
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> 
> some nitpicks below.
> 
> > ---
> > Make sure there is actually a semaphore (read-read doesn't generate an
> > interengine dependency!)
> > ---
> >   tests/i915/gem_exec_schedule.c | 75 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 75 insertions(+)
> > 
> > diff --git a/tests/i915/gem_exec_schedule.c b/tests/i915/gem_exec_schedule.c
> > index 3590b739a..a6b74b3b6 100644
> > --- a/tests/i915/gem_exec_schedule.c
> > +++ b/tests/i915/gem_exec_schedule.c
> > @@ -603,6 +603,79 @@ static void semaphore_resolve(int i915)
> >       gem_context_destroy(i915, outer);
> >   }
> >   
> > +static void semaphore_noskip(int i915)
> > +{
> > +     unsigned int engine, other;
> > +     uint32_t ctx;
> > +
> > +     igt_assert(intel_get_drm_devid(i915) >= 8);
> 
> I would move this to igt_require since the subtest_group only requires 
> scheduler & priority and even if we currently don't enable those 
> pre-gen8 I don't think we can assume we never will.

Sure. It's copy-and-paste from the previous test that was using
MI_SEMAPHORE_WAIT. This test could be written to be gen agnostic,
so might as well.

> > +
> > +     ctx = gem_context_create(i915);
> > +
> > +     for_each_physical_engine(i915, engine) {
> > +     for_each_physical_engine(i915, other) {
> 
> Is it worth hiding the double loop in its own macro? e.g.:
> 
> for_each_physical_engine_pair(i915, engine1, engine2)

I'm still waiting for Andi to rewrite the iterators :)
-Chris