[Intel-gfx] [PATCH 02/20] drm/i915/gt: Couple up old virtual breadcrumb on new sibling

[Tvrtko Ursulin]

On 11/05/2020 08:57, Chris Wilson wrote:
> The second try at staging the transfer of the breadcrumb. In part one,
> we realised we could not simply move to the second engine as we were
> only holding the breadcrumb lock on the first. So in commit 6c81e21a4742
> ("drm/i915/gt: Stage the transfer of the virtual breadcrumb"), we
> removed it from the first engine and marked up this request to reattach
> the signaling on the new engine. However, this failed to take into
> account that we only attach the breadcrumb if the new request is added
> at the start of the queue, which if we are transferring, it is because
> we know there to be a request to be signaled (and hence we would not be
> attached). In this second try, we remove from the first list under its
> lock, take ownership of the link, and then take the second lock to
> complete the transfer.

Overall just an optimisation not to call i915_request_enable_breadcrumb, 
I mean not add to the list indirectly?

Regards,

Tvrtko

> Fixes: 6c81e21a4742 ("drm/i915/gt: Stage the transfer of the virtual breadcrumb")
> 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 | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index ed45fc40f884..c5591248dafb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1825,13 +1825,12 @@ static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
>   				     struct i915_request *rq)
>   {
>   	struct intel_engine_cs *old = ve->siblings[0];
> +	bool xfer = false;
>   
>   	/* All unattached (rq->engine == old) must already be completed */
>   
>   	spin_lock(&old->breadcrumbs.irq_lock);
>   	if (!list_empty(&ve->context.signal_link)) {
> -		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),
> @@ -1839,12 +1838,21 @@ static void virtual_xfer_breadcrumbs(struct virtual_engine *ve,
>   		 * 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);
> +		__list_del_entry(&ve->context.signal_link);
> +		xfer = true;
> +	}
> +	spin_unlock(&old->breadcrumbs.irq_lock);
> +
> +	if (xfer) {
> +		struct intel_breadcrumbs *b = &rq->engine->breadcrumbs;
> +
> +		spin_lock(&b->irq_lock);
> +		list_add_tail(&ve->context.signal_link, &b->signalers);
> +		spin_unlock(&b->irq_lock);
>   
>   		/* Also transfer the pending irq_work for the old breadcrumb. */
>   		intel_engine_signal_breadcrumbs(rq->engine);
>   	}
> -	spin_unlock(&old->breadcrumbs.irq_lock);
>   }
>   
>   #define for_each_waiter(p__, rq__) \
>