[igt-dev] [PATCH i-g-t] tests/i915/gem_ctx_engines: Added out fence for execute-one subtest

[Chris Wilson]

Quoting Arjun Melkaveri (2021-12-06 16:42:51)
> To make spinner to be on same context for following execbuf,
> spin out_fence is passed to execbuf.rsvd2.
> 
> Used IGT_SPIN_FENCE_OUT flag in spinner.
> 
> Cc: Chris Wilson <chris.p.wilson at intel.com>
> Signed-off-by: Arjun Melkaveri <arjun.melkaveri at intel.com>
> ---
>  tests/i915/gem_ctx_engines.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
> index 003dd171..c746f268 100644
> --- a/tests/i915/gem_ctx_engines.c
> +++ b/tests/i915/gem_ctx_engines.c
> @@ -298,7 +298,7 @@ static void execute_one(int i915)
>                         spin = igt_spin_new(i915,
>                                             .ahnd = ahnd,
>                                             .ctx = ctx,
> -                                           .flags = (IGT_SPIN_NO_PREEMPTION |
> +                                           .flags = (IGT_SPIN_FENCE_OUT | IGT_SPIN_NO_PREEMPTION |

Introducing FENCE_OUT means we can stop using the no-preemption, as no
subsequent request is allowed to execute before the cork.

But we do have to add I915_EXEC_FENCE_IN to the execbuf.flags so that it
is executed after the spinner.

>                                                       IGT_SPIN_POLL_RUN));
>  
>                         do_ioctl(i915, DRM_IOCTL_I915_GEM_BUSY, &busy);
> @@ -320,6 +320,7 @@ static void execute_one(int i915)
>                                 int expected = j == i ? 0 : -EINVAL;
>  
>                                 execbuf.rsvd1 = ctx->id;
> +                               execbuf.rsvd2 = spin->out_fence;

It's also complicated by that we re-execute the spinner for each engine,
each time it will have a new out fence not recorded in spin->out_fence,
but in spin->execbuf.rsvd2.

6a4199ef96 HEAD@{1222}: am: i915/gem_ctx_engines: Replace NO_PREEMPT spinners with explicit fencing
diff --git a/tests/i915/gem_ctx_engines.c b/tests/i915/gem_ctx_engines.c
index 2804cae0fe..13555bc525 100644
--- a/tests/i915/gem_ctx_engines.c
+++ b/tests/i915/gem_ctx_engines.c
@@ -331,6 +331,7 @@ static void execute_one(int i915)
                .buffers_ptr = to_user_pointer(&obj),
                .buffer_count = 1,
                .rsvd1 = param.ctx_id,
+               .flags = I915_EXEC_FENCE_IN,
        };
        const uint32_t bbe = MI_BATCH_BUFFER_END;
        const struct intel_execution_engine2 *e;
@@ -338,7 +339,7 @@ static void execute_one(int i915)

        /* Prewarm the spinner */
        spin = igt_spin_new(i915, .ctx_id = param.ctx_id,
-                           .flags = (IGT_SPIN_NO_PREEMPTION |
+                           .flags = (IGT_SPIN_FENCE_OUT |
                                      IGT_SPIN_POLL_RUN));

        gem_write(i915, obj.handle, 0, &bbe, sizeof(bbe));
@@ -369,6 +370,7 @@ static void execute_one(int i915)
                        gem_sync(i915, spin->handle);
                        igt_spin_reset(spin);
                        gem_execbuf(i915, &spin->execbuf);
+                       execbuf.rsvd2 = spin->execbuf.rsvd2 >> 32;

                        do_ioctl(i915, DRM_IOCTL_I915_GEM_BUSY, &busy);
                        igt_assert_eq(busy.busy, 0);
@@ -382,7 +384,6 @@ static void execute_one(int i915)
                        param.size = sizeof(uint64_t) + (i + 1) * sizeof(uint32_t);
                        gem_context_set_param(i915, &param);

-                       igt_spin_busywait_until_started(spin);
                        for (int j = 0; j <= I915_EXEC_RING_MASK; j++) {
                                int expected = j == i ? 0 : -EINVAL;

@@ -397,6 +398,7 @@ static void execute_one(int i915)
                                      i != -1 ? 1 << e->class : 0);

                        igt_spin_end(spin);
+                       close(spin->execbuf.rsvd2 >> 32);

                        gem_sync(i915, obj.handle);
                        do_ioctl(i915, DRM_IOCTL_I915_GEM_BUSY, &busy);

The bigger challenge is removing the NO_PREEMPTION from independent-all.
-Chris