[PATCH 1/3] drm/i915/execlists: Enable timeslice on partial virtual engine dequeue

[PATCH 1/3] drm/i915/execlists: Enable timeslice on partial virtual engine dequeue

[Chris Wilson]

If we stop filling the ELSP due to an incompatible virtual engine
request, check if we should enable the timeslice on behalf of the queue.

This fixes the case where we are inspecting the last->next element when
we know that the last element is the last request in the execution queue,
and so decided we did not need to enable timeslicing despite the intent
to do so!

Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 13941d1c0a4a..a1d268880cfe 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1757,11 +1757,9 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
 	if (!intel_engine_has_timeslices(engine))
 		return false;
 
-	if (list_is_last(&rq->sched.link, &engine->active.requests))
-		return false;
-
-	hint = max(rq_prio(list_next_entry(rq, sched.link)),
-		   engine->execlists.queue_priority_hint);
+	hint = engine->execlists.queue_priority_hint;
+	if (!list_is_last(&rq->sched.link, &engine->active.requests))
+		hint = max(hint, rq_prio(list_next_entry(rq, sched.link)));
 
 	return hint >= effective_prio(rq);
 }
@@ -1803,6 +1801,18 @@ static void set_timeslice(struct intel_engine_cs *engine)
 	set_timer_ms(&engine->execlists.timer, active_timeslice(engine));
 }
 
+static void start_timeslice(struct intel_engine_cs *engine)
+{
+	struct intel_engine_execlists *execlists = &engine->execlists;
+
+	execlists->switch_priority_hint = execlists->queue_priority_hint;
+
+	if (timer_pending(&execlists->timer))
+		return;
+
+	set_timer_ms(&execlists->timer, timeslice(engine));
+}
+
 static void record_preemption(struct intel_engine_execlists *execlists)
 {
 	(void)I915_SELFTEST_ONLY(execlists->preempt_hang.count++);
@@ -1966,11 +1976,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 				 * Even if ELSP[1] is occupied and not worthy
 				 * of timeslices, our queue might be.
 				 */
-				if (!execlists->timer.expires &&
-				    need_timeslice(engine, last))
-					set_timer_ms(&execlists->timer,
-						     timeslice(engine));
-
+				start_timeslice(engine);
 				return;
 			}
 		}
@@ -2005,7 +2011,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 			if (last && !can_merge_rq(last, rq)) {
 				spin_unlock(&ve->base.active.lock);
-				return; /* leave this for another */
+				start_timeslice(engine);
+				return; /* leave this for another sibling */
 			}
 
 			ENGINE_TRACE(engine,
-- 
2.25.1

Re: [PATCH 1/3] drm/i915/execlists: Enable timeslice on partial virtual engine dequeue

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> If we stop filling the ELSP due to an incompatible virtual engine
> request, check if we should enable the timeslice on behalf of the queue.
>
> This fixes the case where we are inspecting the last->next element when
> we know that the last element is the last request in the execution queue,
> and so decided we did not need to enable timeslicing despite the intent
> to do so!
>
> Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 13941d1c0a4a..a1d268880cfe 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1757,11 +1757,9 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
>  	if (!intel_engine_has_timeslices(engine))
>  		return false;
>  
> -	if (list_is_last(&rq->sched.link, &engine->active.requests))
> -		return false;
> -
> -	hint = max(rq_prio(list_next_entry(rq, sched.link)),
> -		   engine->execlists.queue_priority_hint);
> +	hint = engine->execlists.queue_priority_hint;
> +	if (!list_is_last(&rq->sched.link, &engine->active.requests))
> +		hint = max(hint, rq_prio(list_next_entry(rq, sched.link)));
>  
>  	return hint >= effective_prio(rq);
>  }
> @@ -1803,6 +1801,18 @@ static void set_timeslice(struct intel_engine_cs *engine)
>  	set_timer_ms(&engine->execlists.timer, active_timeslice(engine));
>  }
>  
> +static void start_timeslice(struct intel_engine_cs *engine)
> +{
> +	struct intel_engine_execlists *execlists = &engine->execlists;
> +
> +	execlists->switch_priority_hint = execlists->queue_priority_hint;
> +
> +	if (timer_pending(&execlists->timer))
> +		return;
> +
> +	set_timer_ms(&execlists->timer, timeslice(engine));
> +}
> +
>  static void record_preemption(struct intel_engine_execlists *execlists)
>  {
>  	(void)I915_SELFTEST_ONLY(execlists->preempt_hang.count++);
> @@ -1966,11 +1976,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  				 * Even if ELSP[1] is occupied and not worthy
>  				 * of timeslices, our queue might be.
>  				 */
> -				if (!execlists->timer.expires &&
> -				    need_timeslice(engine, last))
> -					set_timer_ms(&execlists->timer,
> -						     timeslice(engine));
> -
> +				start_timeslice(engine);
>  				return;
>  			}
>  		}
> @@ -2005,7 +2011,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  
>  			if (last && !can_merge_rq(last, rq)) {
>  				spin_unlock(&ve->base.active.lock);
> -				return; /* leave this for another */
> +				start_timeslice(engine);
> +				return; /* leave this for another sibling */
>  			}
>  
>  			ENGINE_TRACE(engine,
> -- 
> 2.25.1