Re: [PATCH v5 3/5] drm/i915: Create the locked version of the request add

[Andrzej Hajda <andrzej.hajda@xxxxxxxxx>]

On 12.04.2023 13:33, Andi Shyti wrote:
> i915_request_add() assumes that the timeline is locked whtn the
*when
> function is called. Before exiting it releases the lock. But in
> the next commit we have one case where releasing the timeline
> mutex is not necessary and we don't want that.
>
> Make a new i915_request_add_locked() version of the function
> where the lock is not released.
>
> Signed-off-by: Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx

Have you looked for other potential users of these new helpers?

Reviewed-by: Andrzej Hajda <andrzej.hajda@xxxxxxxxx>

Regards
Andrzej

> ---
>   drivers/gpu/drm/i915/i915_request.c | 14 +++++++++++---
>   drivers/gpu/drm/i915/i915_request.h |  1 +
>   2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 58662360ac34e..21032b3b9d330 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1852,13 +1852,13 @@ void __i915_request_queue(struct i915_request *rq,
>   	local_bh_enable(); /* kick tasklets */
>   }
>   
> -void i915_request_add(struct i915_request *rq)
> +void i915_request_add_locked(struct i915_request *rq)
>   {
>   	struct intel_timeline * const tl = i915_request_timeline(rq);
>   	struct i915_sched_attr attr = {};
>   	struct i915_gem_context *ctx;
>   
> -	lockdep_assert_held(&tl->mutex);
> +	intel_context_assert_timeline_is_locked(tl);
>   	lockdep_unpin_lock(&tl->mutex, rq->cookie);
>   
>   	trace_i915_request_add(rq);
> @@ -1873,7 +1873,15 @@ void i915_request_add(struct i915_request *rq)
>   
>   	__i915_request_queue(rq, &attr);
>   
> -	mutex_unlock(&tl->mutex);
> +}
> +
> +void i915_request_add(struct i915_request *rq)
> +{
> +	struct intel_timeline * const tl = i915_request_timeline(rq);
> +
> +	i915_request_add_locked(rq);
> +
> +	intel_context_timeline_unlock(tl);
>   }
>   
>   static unsigned long local_clock_ns(unsigned int *cpu)
> diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
> index bb48bd4605c03..29e3a37c300a7 100644
> --- a/drivers/gpu/drm/i915/i915_request.h
> +++ b/drivers/gpu/drm/i915/i915_request.h
> @@ -425,6 +425,7 @@ int i915_request_await_deps(struct i915_request *rq, const struct i915_deps *dep
>   int i915_request_await_execution(struct i915_request *rq,
>   				 struct dma_fence *fence);
>   
> +void i915_request_add_locked(struct i915_request *rq);
>   void i915_request_add(struct i915_request *rq);
>   
>   bool __i915_request_submit(struct i915_request *request);