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

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@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@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.

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 <mika.kuoppala@linux.intel.com>

>  
> -- 
> 2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx