[Intel-gfx] [PATCH] drm/i915/gt: Stage the transfer of the virtual breadcrumb

[Tvrtko Ursulin]

On 25/03/2020 13:00, Chris Wilson wrote:
> We move the virtual breadcrumb from one physical engine to the next, if
> the next virtual request is scheduled on a new physical engine. Since
> the virtual context can only be in one signal queue, we need it to track
> the current physical engine for the new breadcrumbs. However, to move
> the list we need both breadcrumb locks -- and since we cannot take both
> at the same time (unless we are careful and always ensure consistent
> ordering) stage the movement of the signaler via the current virtual
> request.
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/1510
> Fixes: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index f88d3b95c4e1..2b0923cb0483 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1663,7 +1663,7 @@ static bool virtual_matches(const struct virtual_engine *ve,
>   }
>   
>   static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
> -				     struct intel_engine_cs *engine)
> +				     struct i915_request *rq)
>   {
>   	struct intel_engine_cs *old = ve->siblings[0];
>   
> @@ -1671,9 +1671,17 @@ static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
>   
>   	spin_lock(&old->breadcrumbs.irq_lock);
>   	if (!list_empty(&ve->context.signal_link)) {
> -		list_move_tail(&ve->context.signal_link,
> -			       &engine->breadcrumbs.signalers);
> -		intel_engine_signal_breadcrumbs(engine);
> +		list_del_init(&ve->context.signal_link);
> +
> +		/*
> +		 * We cannot acquire the new engine->breadcrumbs.irq_lock
> +		 * (as we are holding a breadcrumbs.irq_lock already),
> +		 * so attach this request to the signaler on submission.
> +		 * The queued irq_work will occur when we finally drop
> +		 * the engine->active.lock after dequeue.
> +		 */
> +		set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &rq->fence.flags);
> +		intel_engine_signal_breadcrumbs(rq->engine);
>   	}
>   	spin_unlock(&old->breadcrumbs.irq_lock);
>   }
> @@ -2045,7 +2053,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   									engine);
>   
>   				if (!list_empty(&ve->context.signals))
> -					virtual_xfer_breadcrumbs(ve, engine);
> +					virtual_xfer_breadcrumbs(ve, rq);
>   
>   				/*
>   				 * Move the bound engine to the top of the list
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko