[igt-dev] [PATCH i-g-t v2] tests/i915: Restore some BAT coverage

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-05-23 07:43:12)
> 
> On 23/05/2019 07:37, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > 
> > Engine enumerated test names have changed so fast-feedback.testlist needs
> > to be updated. However listing all engines there won't scale. So instead
> > add new tests cases which iterate all engines internally.
> > 
> > v2:
> >   * Fix basic-all test name.
> >   * Fix params to basic (bool false to zero).
> >   * Use I915_EXEC_DEFAULT in has_extended_busy_ioctl to workaround stateful
> >     contexts for now.
> >   * Have only basic-all in BAT. (Chris)
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> # v1
> > Reviewed-by: Andi Shyti <andi.shyti at intel.com> # v1
> > ---
> >   tests/i915/gem_busy.c                 | 19 +++++++++++----
> >   tests/i915/gem_exec_basic.c           | 33 +++++++++++++++++++++++++++
> >   tests/intel-ci/fast-feedback.testlist | 31 ++-----------------------
> >   3 files changed, 50 insertions(+), 33 deletions(-)
> > 
> > diff --git a/tests/i915/gem_busy.c b/tests/i915/gem_busy.c
> > index 781a3bfab1d1..f3ebb37a33b4 100644
> > --- a/tests/i915/gem_busy.c
> > +++ b/tests/i915/gem_busy.c
> > @@ -417,7 +417,7 @@ static bool has_semaphores(int fd)
> >   
> >   static bool has_extended_busy_ioctl(int fd)
> >   {
> > -     igt_spin_t *spin = igt_spin_new(fd, .engine = I915_EXEC_RENDER);
> > +     igt_spin_t *spin = igt_spin_new(fd, .engine = I915_EXEC_DEFAULT);
> 
> (This fails on platforms with only rcs0 (no other engines) due context 
> has a map now, and I915_EXEC_RENDER == 1 == -EINVAL.)

In which case, it probably should be a plain 0 as it no longer has the
old EXEC_RING semantics but is just an index, i.e. igt_spin_new(fd);

> We need to come up with a robust and easy to remember solution for 
> dealing with the fact contexts are stateful now and 
> __for_each_physical_engine iterator configures the default one.
> 
> Could end game for test conversion be to stop passing in eb.flags to 
> igt_spin_new and do class:instance instead? That would enable dummyload 
> to unambiguously know what to use, depending on get_engines query.

Speak to Mika, we abuse igt_spin_t much more by resubmitting the same
dummyload to multiple engines and contexts.

> Until 
> then it is fiddling and guess work.

Hmm, but it shouldn't be. We pass both context and engine to
igt_spin_new(). It is fully under the caller's control. What has
happened is that we've changed the caller's context...

> But are there more tests which could be affected? Must be..
> 
> Or we drop the aspiration to support 
> non-engine-discovery-and-no-context-map kernels?

igt must be able to run on linus and drm-tip, at the very least.
-Chris