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

[Tvrtko Ursulin]

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

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. Until 
then it is fiddling and guess work.

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?

Regards,

Tvrtko

>   	uint32_t read, write;
>   
>   	__gem_busy(fd, spin->handle, &read, &write);
> @@ -460,6 +460,14 @@ static void basic(int fd, const struct intel_execution_engine2 *e, unsigned flag
>   	igt_spin_free(fd, spin);
>   }
>   
> +static void all(int i915)
> +{
> +	const struct intel_execution_engine2 *e;
> +
> +	__for_each_physical_engine(i915, e)
> +		basic(i915, e, 0);
> +}
> +
>   igt_main
>   {
>   	const struct intel_execution_engine2 *e;
> @@ -477,11 +485,14 @@ igt_main
>   			igt_fork_hang_detector(fd);
>   		}
>   
> +		igt_subtest("busy-all") {
> +			gem_quiescent_gpu(fd);
> +			all(fd);
> +		}
> +
>   		__for_each_physical_engine(fd, e) {
>   			igt_subtest_group {
> -				igt_subtest_f("%sbusy-%s",
> -					      e->class == I915_ENGINE_CLASS_RENDER
> -					      ? "basic-" : "", e->name) {
> +				igt_subtest_f("busy-%s", e->name) {
>   					gem_quiescent_gpu(fd);
>   					basic(fd, e, 0);
>   				}
> diff --git a/tests/i915/gem_exec_basic.c b/tests/i915/gem_exec_basic.c
> index 140cbdbc66a5..1287860be3b1 100644
> --- a/tests/i915/gem_exec_basic.c
> +++ b/tests/i915/gem_exec_basic.c
> @@ -114,6 +114,30 @@ static void gtt(int fd, uint64_t flags)
>   	munmap(execbuf, 4096);
>   }
>   
> +static void all(int i915)
> +{
> +	const struct intel_execution_engine2 *e;
> +
> +	__for_each_physical_engine(i915, e)
> +		noop(i915, e->flags);
> +}
> +
> +static void readonly_all(int i915)
> +{
> +	const struct intel_execution_engine2 *e;
> +
> +	__for_each_physical_engine(i915, e)
> +		readonly(i915, e->flags);
> +}
> +
> +static void gtt_all(int i915)
> +{
> +	const struct intel_execution_engine2 *e;
> +
> +	__for_each_physical_engine(i915, e)
> +		gtt(i915, e->flags);
> +}
> +
>   igt_main
>   {
>   	const struct intel_execution_engine2 *e;
> @@ -126,6 +150,15 @@ igt_main
>   		igt_fork_hang_detector(fd);
>   	}
>   
> +	igt_subtest("basic-all")
> +		all(fd);
> +
> +	igt_subtest("readonly-all")
> +		readonly_all(fd);
> +
> +	igt_subtest("gtt-all")
> +		gtt_all(fd);
> +
>   	__for_each_physical_engine(fd, e) {
>   		igt_subtest_f("basic-%s", e->name)
>   			noop(fd, e->flags);
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index 40475b1ab361..58e6b5c5f882 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -5,7 +5,7 @@ igt at debugfs_test@read_all_entries
>   igt at gem_basic@bad-close
>   igt at gem_basic@create-close
>   igt at gem_basic@create-fd-close
> -igt at gem_busy@basic-busy-default
> +igt at gem_busy@busy-all
>   igt at gem_close_race@basic-process
>   igt at gem_close_race@basic-threads
>   igt at gem_cpu_reloc@basic
> @@ -15,27 +15,7 @@ igt at gem_ctx_exec@basic
>   igt at gem_ctx_param@basic
>   igt at gem_ctx_param@basic-default
>   igt at gem_ctx_switch@basic-default
> -igt at gem_exec_basic@basic-blt
> -igt at gem_exec_basic@basic-bsd
> -igt at gem_exec_basic@basic-bsd1
> -igt at gem_exec_basic@basic-bsd2
> -igt at gem_exec_basic@basic-default
> -igt at gem_exec_basic@basic-render
> -igt at gem_exec_basic@basic-vebox
> -igt at gem_exec_basic@gtt-blt
> -igt at gem_exec_basic@gtt-bsd
> -igt at gem_exec_basic@gtt-bsd1
> -igt at gem_exec_basic@gtt-bsd2
> -igt at gem_exec_basic@gtt-default
> -igt at gem_exec_basic@gtt-render
> -igt at gem_exec_basic@gtt-vebox
> -igt at gem_exec_basic@readonly-blt
> -igt at gem_exec_basic@readonly-bsd
> -igt at gem_exec_basic@readonly-bsd1
> -igt at gem_exec_basic@readonly-bsd2
> -igt at gem_exec_basic@readonly-default
> -igt at gem_exec_basic@readonly-render
> -igt at gem_exec_basic@readonly-vebox
> +igt at gem_exec_basic@basic-all
>   igt at gem_exec_create@basic
>   igt at gem_exec_fence@basic-busy-default
>   igt at gem_exec_fence@basic-wait-default
> @@ -74,13 +54,6 @@ igt at gem_exec_reloc@basic-write-gtt-active
>   igt at gem_exec_reloc@basic-write-read-active
>   igt at gem_exec_reloc@basic-softpin
>   igt at gem_exec_store@basic-all
> -igt at gem_exec_store@basic-blt
> -igt at gem_exec_store@basic-bsd
> -igt at gem_exec_store@basic-bsd1
> -igt at gem_exec_store@basic-bsd2
> -igt at gem_exec_store@basic-default
> -igt at gem_exec_store@basic-render
> -igt at gem_exec_store@basic-vebox
>   igt at gem_exec_suspend@basic
>   igt at gem_exec_suspend@basic-s3
>   igt at gem_exec_suspend@basic-s4-devices
>