[igt-dev] [PATCH i-g-t v2 4/5] test/i915: gem_exec_params: use the gem_engine_topology library

[Chris Wilson]

Quoting Ramalingam C (2019-05-30 16:42:21)
> Replace the legacy for_each_engine* defines with the ones implemented
> in the gem_engine_topology library.
> 
> v2:
>   has_ring() check is removed [tvertko]
>   Engine type is identified through CLASS [tvrtko]
> 
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> ---
>  tests/i915/gem_exec_params.c | 41 ++++++------------------------------
>  1 file changed, 7 insertions(+), 34 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_params.c b/tests/i915/gem_exec_params.c
> index 49c56a8d75e9..3bc5b0a9d531 100644
> --- a/tests/i915/gem_exec_params.c
> +++ b/tests/i915/gem_exec_params.c
> @@ -52,30 +52,6 @@
>  #define LOCAL_I915_EXEC_BATCH_FIRST (1 << 18)
>  #define LOCAL_I915_EXEC_FENCE_ARRAY (1 << 19)
>  
> -static bool has_ring(int fd, unsigned ring_exec_flags)
> -{
> -       switch (ring_exec_flags & I915_EXEC_RING_MASK) {
> -       case 0:
> -       case I915_EXEC_RENDER:
> -               return true;
> -
> -       case I915_EXEC_BSD:
> -               if (ring_exec_flags & LOCAL_I915_EXEC_BSD_MASK)
> -                       return gem_has_bsd2(fd);
> -               else
> -                       return gem_has_bsd(fd);
> -
> -       case I915_EXEC_BLT:
> -               return gem_has_blt(fd);
> -
> -       case I915_EXEC_VEBOX:
> -               return gem_has_vebox(fd);
> -       }
> -
> -       igt_assert_f(0, "invalid exec flag 0x%x\n", ring_exec_flags);
> -       return false;
> -}
> -
>  static bool has_exec_batch_first(int fd)
>  {
>         int val = -1;
> @@ -201,7 +177,7 @@ int fd;
>  
>  igt_main
>  {
> -       const struct intel_execution_engine *e;
> +       const struct intel_execution_engine2 *e;
>  
>         igt_fixture {
>                 fd = drm_open_driver(DRIVER_INTEL);
> @@ -235,11 +211,9 @@ igt_main
>         }
>  
>         igt_subtest("control") {
> -               for (e = intel_execution_engines; e->name; e++) {
> -                       if (has_ring(fd, e->exec_id | e->flags)) {
> -                               execbuf.flags = e->exec_id | e->flags;
> -                               gem_execbuf(fd, &execbuf);
> -                       }
> +               __for_each_physical_engine(fd, e) {
> +                       execbuf.flags = e->flags;
> +                       gem_execbuf(fd, &execbuf);

This is silly. Previously it was cross-checking the GETPARAM against the
EXECBUFFER uABI. Now it is asking the same question twice.

This is the example that should not be using physical engines as it
exercising ABI. Please be careful.
-Chris