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

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> 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.

I have wandered there yes. The reason for this abuse is still unclear.
We could do a clearer interface for creating spinners but is the
reusage due to latency concerns?

-Mika