[Intel-gfx] [PATCH 2/2] drm/i915: Remove requirement for holding i915_request.lock for breadcrumbs

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-07-16 10:01:15)
> 
> On 14/07/2020 10:47, Chris Wilson wrote:
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index e0280a672f1d..aa7be7f05f8c 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -1148,20 +1148,6 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
> >               } else {
> >                       struct intel_engine_cs *owner = rq->context->engine;
> >   
> > -                     /*
> > -                      * Decouple the virtual breadcrumb before moving it
> > -                      * back to the virtual engine -- we don't want the
> > -                      * request to complete in the background and try
> > -                      * and cancel the breadcrumb on the virtual engine
> > -                      * (instead of the old engine where it is linked)!
> > -                      */
> > -                     if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
> > -                                  &rq->fence.flags)) {
> > -                             spin_lock_nested(&rq->lock,
> > -                                              SINGLE_DEPTH_NESTING);
> > -                             i915_request_cancel_breadcrumb(rq);
> > -                             spin_unlock(&rq->lock);
> > -                     }
> 
> Why is this not needed any more?

Hindsight says that this was just paper. We have just done the same
cancellation step in i915_request_unsubmit, so all this was doing is
shrinking the window for a breadcrumb being attached [and with the
serialisation around testing the I915_FENCE_FLAG_ACTIVE_BIT there should
be no window at all] while the request is being moved back to the virtual
engine.

Now the question is how do we end up attaching to the virtual engine
while unsubmitted, and that appears to be entirely due to the ACTIVE
flag being set in retire after completion. The other way is that we use
the local virtual engine breadcrumb list to defer the signaling on
submission of an old request.

So... If I can fix the issues around retirement that means we should be
able to kill off virtual_xfer_breadcrumbs... I hope so.
-Chris