Quoting Mika Kuoppala (2020-02-28 15:09:28)
> Chris Wilson <ch...@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 <ch...@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
