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

Chris Wilson
2019-05-23T06:58:05Z

Quoting Tvrtko Ursulin (2019-05-23 07:43:12)
>
> On 23/05/2019 07:37, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@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@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # v1
> > Reviewed-by: Andi Shyti <andi.shyti@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