[Intel-gfx] [PATCH 6/6] drm/i915/gt: Remove timeslice suppression

[Chris Wilson]

Quoting Andi Shyti (2021-01-06 00:07:40)
> Hi Chris,
> 
> On Mon, Jan 04, 2021 at 11:51:45AM +0000, Chris Wilson wrote:
> > In the next patch, we remove the strict priority system and continuously
> 
> next patch?

If you take a birds eye view, and can look back in time, that sentence
made more sense.

> 
> [...]
> 
> > -static bool
> > -timeslice_expired(const struct intel_engine_execlists *el,
> > -               const struct i915_request *rq)
> > +static bool needs_timeslice(const struct intel_engine_cs *engine,
> > +                         const struct i915_request *rq)
> >  {
> > -     return timer_expired(&el->timer) || timeslice_yield(el, rq);
> > -}
> > +     if (!intel_engine_has_timeslices(engine))
> > +             return false;
> >  
> > -static int
> > -switch_prio(struct intel_engine_cs *engine, const struct i915_request *rq)
> > -{
> > -     if (list_is_last(&rq->sched.link, &engine->active.requests))
> > -             return engine->execlists.queue_priority_hint;
> > +     /* If not currently active, or about to switch, wait for next event */
> > +     if (!rq || __i915_request_is_complete(rq))
> > +             return false;
> >  
> > -     return rq_prio(list_next_entry(rq, sched.link));
> > -}
> > +     /* We do not need to start the timeslice until after the ACK */
> > +     if (READ_ONCE(engine->execlists.pending[0]))
> > +             return false;
> >  
> > -static inline unsigned long
> > -timeslice(const struct intel_engine_cs *engine)
> > -{
> > -     return READ_ONCE(engine->props.timeslice_duration_ms);
> > +     /* If ELSP[1] is occupied, always check to see if worth slicing */
> > +     if (!list_is_last_rcu(&rq->sched.link, &engine->active.requests))
> > +             return true;
> > +
> > +     /* Otherwise, ELSP[0] is by itself, but may be waiting in the queue */
> > +     if (!RB_EMPTY_ROOT(&engine->execlists.queue.rb_root))
> > +             return true;
> > +
> > +     return !RB_EMPTY_ROOT(&engine->execlists.virtual.rb_root);
> >  }
> 
> the above diff is quite unreadable and besides the patch does not
> apply on the latest drm-tip.

The diff is quite meaningless since it removes one flow and replaces it
with a different system. I shall play with

--diff-algorithm={patience|minimal|histogram|myers}

and see what happens.


> In order to have a better review, I would suggest either one of
> the following:
> 
>  - use a different diff algorithm for generating the patch
>  - you rebase everything on top of the latest drm-tip so that I
>    can apply and check
>  - give me a branch so that I can checkout that branch and review
>    it from there.

fdo is quite full... But
https://cgit.freedesktop.org/~ickle/linux-2.6/log/?h=ringscheduler
made it intact.
-Chris