[04/33] drm/i915/gt: Check for a completed last request once
diff mbox series

Message ID 20200701084053.6086-4-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [01/33] drm/i915/gt: Harden the heartbeat against a stuck driver
Related show

Commit Message

Chris Wilson July 1, 2020, 8:40 a.m. UTC
Pull the repeated check for the last active request being completed to a
single spot, when deciding whether or not execlist preemption is
required.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

Comments

Mika Kuoppala July 2, 2020, 3:46 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> Pull the repeated check for the last active request being completed to a
> single spot, when deciding whether or not execlist preemption is
> required.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 4eb397b0e14d..7bdbfac26d7b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2137,12 +2137,11 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  	 */
>  
>  	if ((last = *active)) {
> -		if (need_preempt(engine, last, rb)) {
> -			if (i915_request_completed(last)) {
> -				tasklet_hi_schedule(&execlists->tasklet);
> -				return;
> -			}
> +		if (i915_request_completed(last) &&
> +		    !list_is_last(&last->sched.link, &engine->active.requests))

You return if it is not last? Also the hi schedule is gone.
-Mika


> +			return;
>  
> +		if (need_preempt(engine, last, rb)) {
>  			ENGINE_TRACE(engine,
>  				     "preempting last=%llx:%lld, prio=%d, hint=%d\n",
>  				     last->fence.context,
> @@ -2170,11 +2169,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  			last = NULL;
>  		} else if (need_timeslice(engine, last, rb) &&
>  			   timeslice_expired(execlists, last)) {
> -			if (i915_request_completed(last)) {
> -				tasklet_hi_schedule(&execlists->tasklet);
> -				return;
> -			}
> -
>  			ENGINE_TRACE(engine,
>  				     "expired last=%llx:%lld, prio=%d, hint=%d, yield?=%s\n",
>  				     last->fence.context,
> -- 
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Chris Wilson July 2, 2020, 3:53 p.m. UTC | #2
Quoting Mika Kuoppala (2020-07-02 16:46:22)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Pull the repeated check for the last active request being completed to a
> > single spot, when deciding whether or not execlist preemption is
> > required.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 14 ++++----------
> >  1 file changed, 4 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index 4eb397b0e14d..7bdbfac26d7b 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2137,12 +2137,11 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> >        */
> >  
> >       if ((last = *active)) {
> > -             if (need_preempt(engine, last, rb)) {
> > -                     if (i915_request_completed(last)) {
> > -                             tasklet_hi_schedule(&execlists->tasklet);
> > -                             return;
> > -                     }
> > +             if (i915_request_completed(last) &&
> > +                 !list_is_last(&last->sched.link, &engine->active.requests))
> 
> You return if it is not last? Also the hi schedule is gone.

The kick was just causing us to busyspin ahead of the HW CS event. On
tracing, it did not seem worth it.

If this is the last request, the GPU is now idling and we know that we
will not try and lite restore into that request/context. So instead of
waiting for the CS event, we go ahead and prepare the next pair of
contexts.

If it was not the last request, we know there is a context the GPU will
switch into, so the urgency is not an issue. However, we have to be
careful that we don't issue an ELSP into that second context in case we
catch it as it idles (thus hanging the HW).
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 4eb397b0e14d..7bdbfac26d7b 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2137,12 +2137,11 @@  static void execlists_dequeue(struct intel_engine_cs *engine)
 	 */
 
 	if ((last = *active)) {
-		if (need_preempt(engine, last, rb)) {
-			if (i915_request_completed(last)) {
-				tasklet_hi_schedule(&execlists->tasklet);
-				return;
-			}
+		if (i915_request_completed(last) &&
+		    !list_is_last(&last->sched.link, &engine->active.requests))
+			return;
 
+		if (need_preempt(engine, last, rb)) {
 			ENGINE_TRACE(engine,
 				     "preempting last=%llx:%lld, prio=%d, hint=%d\n",
 				     last->fence.context,
@@ -2170,11 +2169,6 @@  static void execlists_dequeue(struct intel_engine_cs *engine)
 			last = NULL;
 		} else if (need_timeslice(engine, last, rb) &&
 			   timeslice_expired(execlists, last)) {
-			if (i915_request_completed(last)) {
-				tasklet_hi_schedule(&execlists->tasklet);
-				return;
-			}
-
 			ENGINE_TRACE(engine,
 				     "expired last=%llx:%lld, prio=%d, hint=%d, yield?=%s\n",
 				     last->fence.context,