Re: [Intel-gfx] [PATCH 18/24] drm/i915/selftests: Wait for the kernel context switch
Quoting Mika Kuoppala (2020-02-28 15:09:28)
> Chris Wilson 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
> > ---
> > 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(>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
Re: [Intel-gfx] [PATCH 18/24] drm/i915/selftests: Wait for the kernel context switch
Chris Wilson 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
> ---
> 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(>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.
But I seem to gravitate towards lower resolution and
apparently you prefer to be more fine grained and
explicit on callsites so,
Reviewed-by: Mika Kuoppala
>
> --
> 2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 18/24] drm/i915/selftests: Wait for the kernel context switch
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.
Signed-off-by: Chris Wilson
---
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(>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();
--
2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
