Re: [PATCH v2] drm/i915/gt: Transfer old virtual breadcrumbs to irq_worker

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/05/2020 17:20, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-05-12 17:07:23)

On 12/05/2020 16:52, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2020-05-12 16:17:30)

On 12/05/2020 14:22, Chris Wilson wrote:
-     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),
-              * 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);
-
-             /* Also transfer the pending irq_work for the old breadcrumb. */
-             intel_engine_signal_breadcrumbs(rq->engine);
-     }
-     spin_unlock(&old->breadcrumbs.irq_lock);
+     intel_engine_transfer_breadcrumbs(ve->siblings[0], &ve->context);

But isn't ve->siblings[0] the old engine at this point so new target
engine would have to be explicitly passed in?

ve->siblings[0] is the old engine, which is holding the completed
requests and their signals. Since their rq->engine == ve->siblings[0]
and we can't update rq->engine as we can't take the required locks, we
need to keep the breadcrumbs relative to ve->siblings[0] and not the new
engine (the i915_request_cancel_breadcrumb conundrum).

Who then enables breadcrumbs on the new engine?

If we enable signaling on the request we are about to submit, that
will enable the breadcrumbs on the new engine. We've cleared out
ce->signals and removed it from the old engine, so as soon as we want to
enable a new breadcrumb, it will be attached to the [new] rq->engine.

So current code was wrong to think it needed to set DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT? Should it instead assert it is set? Because if it is not set signalling on the new engine won't be enabled.

Regards,

Tvrtko

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



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux