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

Chris Wilson
2019-04-24T17:57:07Z

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@chris-wilson.co.uk>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@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