[Intel-gfx] [PATCH 2/3] drm/i915/execlists: Move request unwinding to a separate function

[Chris Wilson]

Quoting Mika Kuoppala (2017-09-25 13:55:26)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > In the future, we will want to unwind requests following a preemption
> > point. This requires the same steps as for unwinding upon a reset, so
> > extract the existing code to a separate function for later use.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_lrc.c | 53 +++++++++++++++++++++++++---------------
> >  1 file changed, 33 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 2c07f3c08bd3..c84831c7ea4a 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -348,6 +348,37 @@ lookup_priolist(struct intel_engine_cs *engine,
> >       return ptr_pack_bits(p, first, 1);
> >  }
> >  
> > +static void unwind_wa_tail(struct drm_i915_gem_request *rq)
> > +{
> > +     rq->tail = intel_ring_wrap(rq->ring,
> > +                                rq->wa_tail - WA_TAIL_DWORDS*sizeof(u32));
> > +     assert_ring_tail_valid(rq->ring, rq->tail);
> > +}
> > +
> > +static void unwind_incomplete_requests(struct intel_engine_cs *engine)
> > +{
> > +     struct drm_i915_gem_request *rq, *rn;
> > +
> > +     lockdep_assert_held(&engine->timeline->lock);
> > +     list_for_each_entry_safe_reverse(rq, rn,
> > +                                      &engine->timeline->requests,
> > +                                      link) {
> > +             struct i915_priolist *p;
> > +
> > +             if (i915_gem_request_completed(rq))
> > +                     return;
> > +
> > +             __i915_gem_request_unsubmit(rq);
> > +             unwind_wa_tail(rq);
> 
> This here raised my attention. Why do you want
> to do this for all requests now?

It is a safety catch for preemption tasks. The worry being that if we
unwind a rq that was previously lite-restored, then on resubmission we
may not coalesce the requests again until perhaps a later interrupt.
It should never happen in practice. It requires a couple of reorderings
to happen at just the right moment, without triggering preemption...
-Chris