[Intel-gfx] [PATCH] drm/i915/gt: Prevent timeslicing into unpreemptible requests

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-05-27 17:13:50)
> 
> On 27/05/2020 15:07, Chris Wilson wrote:
> > We have a I915_REQUEST_NOPREEMPT flag that we set when we must prevent
> > the HW from preempting during the course of this request. We need to
> > honour this flag and protect the HW even if we have a heartbeat request,
> > or other maximum priority barrier, pending. As such, restrict the
> > timeslicing check to avoid preempting into the topmost priority band,
> > leaving the unpreemptable requests in blissful peace running
> > uninterrupted on the HW.
> > 
> > Fixes: 2a98f4e65bba ("drm/i915: add infrastructure to hold off preemption on a request")
> > 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    |  11 +++
> >   drivers/gpu/drm/i915/gt/selftest_lrc.c | 116 +++++++++++++++++++++++++
> >   2 files changed, 127 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 3214a4ecc31a..012afb9e0324 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -1896,6 +1896,15 @@ static void defer_active(struct intel_engine_cs *engine)
> >       defer_request(rq, i915_sched_lookup_priolist(engine, rq_prio(rq)));
> >   }
> >   
> > +static inline int never_timeslice(int prio)
> > +{
> > +     /* Don't allow timeslicing of the 'unpreemptible' requests */
> > +     if (prio == I915_PRIORITY_UNPREEMPTABLE)
> > +             prio--;
> > +
> > +     return prio;
> > +}
> > +
> >   static bool
> >   need_timeslice(const struct intel_engine_cs *engine,
> >              const struct i915_request *rq,
> > @@ -1927,6 +1936,7 @@ need_timeslice(const struct intel_engine_cs *engine,
> >   
> >       if (!list_is_last(&rq->sched.link, &engine->active.requests))
> >               hint = max(hint, rq_prio(list_next_entry(rq, sched.link)));
> > +     hint = never_timeslice(hint);
> >   
> >       return hint >= effective_prio(rq);
> 
> Can INT_MAX ever end up in the queue? I am thinking if we limit it to 
> effective_prio it may be more obvious what's happening. Or is it 
> heartbeats? Should they be INT_MAX - 1 then?

Yes, we insert barriers into the queue with INT_MAX.

I liked #define I915_PRIORITY_BARRIER INT_MAX! But yes, that would be a
cleaner way to solve it.
-Chris