[igt-dev] [PATCH i-g-t] tests/i915/gem_exec_async: Update with engine discovery

[Chris Wilson]

Quoting Ramalingam C (2019-07-24 03:21:18)
> Legacy execbuf abi tests are prefixed with legacy. New test are added to
> run on physical engines accessed through engine discovery.
> 
> So legacy tests run on the unconfigured (with engine map) context and
> use a new helper gem_eb_flags_to_engine to look up the engine from the
> intel_execution_engines2 static list. This is only to enable the core
> test code to be shared.
> 
> Places where new contexts are created had to be updated to either
> equally configure the contexts or not.
> 
> v2:
>   retained the test as it is for legacy uapi testing and duplciated for
>         new engine discovery [Tvrtko]
> v3:
>   Few nits addressed [Tvrtko]
> 
> Signed-off-by: Ramalingam C <ramalingam.c at intel.com>
> ---
>  tests/i915/gem_exec_async.c | 55 +++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> index 9a06af7e29cb..de3652da452a 100644
> --- a/tests/i915/gem_exec_async.c
> +++ b/tests/i915/gem_exec_async.c
> @@ -80,9 +80,10 @@ static void store_dword(int fd, unsigned ring,
>         gem_close(fd, obj[1].handle);
>  }
>  
> -static void one(int fd, unsigned ring, uint32_t flags)
> +static void one(int fd, const struct intel_execution_engine2 *e2, bool legacy)
>  {
>         const int gen = intel_gen(intel_get_drm_devid(fd));
> +       const struct intel_execution_engine2 *other2;
>         struct drm_i915_gem_exec_object2 obj[2];
>  #define SCRATCH 0
>  #define BATCH 1
> @@ -138,20 +139,36 @@ static void one(int fd, unsigned ring, uint32_t flags)
>         memset(&execbuf, 0, sizeof(execbuf));
>         execbuf.buffers_ptr = to_user_pointer(obj);
>         execbuf.buffer_count = 2;
> -       execbuf.flags = ring | flags;
> +       execbuf.flags = e2->flags;
>         igt_require(__gem_execbuf(fd, &execbuf) == 0);
>         gem_close(fd, obj[BATCH].handle);
>  
>         i = 0;
> -       for_each_physical_engine(fd, other) {
> -               if (other == ring)
> -                       continue;
> +       if (legacy) {
> +               for_each_engine(fd, other) {
> +                       if (!gem_ring_has_physical_engine(fd, other))
> +                               continue;
>  
> -               if (!gem_can_store_dword(fd, other))
> -                       continue;
> +                       if (e2->flags == other)
> +                               continue;
> +
> +                       if (!gem_can_store_dword(fd, other))
> +                               continue;
> +
> +                       store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> +                       i++;
> +               }
> +       } else {
> +               __for_each_physical_engine(fd, other2) {
> +                       if (gem_engine_is_equal(e2, other2))
> +                               continue;
>  
> -               store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> -               i++;
> +                       if (!gem_class_can_store_dword(fd, other2->class))
> +                               continue;
> +
> +                       store_dword(fd, other2->flags, obj[SCRATCH].handle, 4*i, i);
> +                       i++;
> +               }
>         }

Does this look neater if we pass in the array of engines and just skip
if others[] == this_engine. The array can be constructed once during
each subgroup fixture, where legacy/non-legacy mode is know. That array
generation is also fairly common so perhaps having a
gem_engine_topology.c function return the set of physical engines with
ALL_ENGINES_STORE_DWORD | ALL_ENGINES_LEGACY? Bonus points for a reverse
lookup to string.
-Chris