[Intel-gfx] [PATCH 18/24] drm/i915/selftests: Wait for the kernel context switch

[Chris Wilson]

Quoting Mika Kuoppala (2020-02-28 15:09:28)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > As we require a context switch to ensure that the current context is
> > switched out and saved to memory, perform an explicit switch to the
> > kernel context and wait for it.
> 
> The patch subject is not incorrect. Just feels that the kernel
> context is a patsy in here.
> 
> So I would s/kernel// on subject but keep in commit msg
> 
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/selftest_lrc.c | 37 +++++++++++++++++++-------
> >  1 file changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > index d7f98aada626..95da6b880e3f 100644
> > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> > @@ -4015,6 +4015,31 @@ static int emit_semaphore_signal(struct intel_context *ce, void *slot)
> >       return 0;
> >  }
> >  
> > +static int context_sync(struct intel_context *ce)
> > +{
> > +     struct i915_request *rq;
> > +     struct dma_fence *fence;
> > +     int err = 0;
> > +
> > +     rq = intel_engine_create_kernel_request(ce->engine);
> > +     if (IS_ERR(rq))
> > +             return PTR_ERR(rq);
> > +
> > +     fence = i915_active_fence_get(&ce->timeline->last_request);
> > +     if (fence) {
> > +             i915_request_await_dma_fence(rq, fence);
> > +             dma_fence_put(fence);
> > +     }
> > +
> > +     rq = i915_request_get(rq);
> > +     i915_request_add(rq);
> > +     if (i915_request_wait(rq, 0, HZ / 2) < 0)
> > +             err = -ETIME;
> > +     i915_request_put(rq);
> > +
> > +     return err;
> > +}
> > +
> >  static int live_lrc_layout(void *arg)
> >  {
> >       struct intel_gt *gt = arg;
> > @@ -4638,16 +4663,10 @@ static int __lrc_timestamp(const struct lrc_timestamp *arg, bool preempt)
> >               wmb();
> >       }
> >  
> > -     if (i915_request_wait(rq, 0, HZ / 2) < 0) {
> > -             err = -ETIME;
> > -             goto err;
> > -     }
> > -
> > -     /* and wait for switch to kernel */
> > -     if (igt_flush_test(arg->engine->i915)) {
> > -             err = -EIO;
> > +     /* and wait for switch to kernel (to save our context to memory) */
> > +     err = context_sync(arg->ce[0]);
> > +     if (err)
> >               goto err;
> > -     }
> >  
> >       rmb();
> 
> For me the context_sync could be context_flush and it would
> allow the rmb() to be snuck inside.

I hear you. The rmb() is really associated with the action of confirming
the switch and can justifiably be inside the context_sync/flush.

The rmb is just there to placate inner daemons, so I didn't think about
it when forcing the emission of the kernel request.
-Chris