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@xxxxxxxxxxxxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> --- 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__) \
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
