[09/10] drm/i915/execlist: Trim immediate timeslice expiry
diff mbox series

Message ID 20191014220534.1662-9-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [01/10] drm/i915/gem: Distinguish each object type
Related show

Commit Message

Chris Wilson Oct. 14, 2019, 10:05 p.m. UTC
We perform timeslicing immediately upon receipt of a request that may be
put into the second ELSP slot. The idea behind this was that since we
didn't install the timer if the second ELSP slot was empty, we would not
have any idea of how long ELSP[0] had been running and so giving the
newcomer a chance on the GPU was fair. However, this causes us extra
busy work that we may be able to avoid if we wait a jiffie for the first
timeslice as normal.

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

Comments

Tvrtko Ursulin Oct. 15, 2019, 12:43 p.m. UTC | #1
On 14/10/2019 23:05, Chris Wilson wrote:
> We perform timeslicing immediately upon receipt of a request that may be
> put into the second ELSP slot. The idea behind this was that since we
> didn't install the timer if the second ELSP slot was empty, we would not
> have any idea of how long ELSP[0] had been running and so giving the
> newcomer a chance on the GPU was fair. However, this causes us extra
> busy work that we may be able to avoid if we wait a jiffie for the first
> timeslice as normal.
 >
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 60e897c000b2..f9bf78032ad7 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1613,7 +1613,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>   			last->hw_context->lrc_desc |= CTX_DESC_FORCE_RESTORE;
>   			last = NULL;
>   		} else if (need_timeslice(engine, last) &&
> -			   !timer_pending(&engine->execlists.timer)) {
> +			   timer_expired(&engine->execlists.timer)) {

timer_expired is !timer_pending(..) && t->expires. Presumably that means 
it was scheduled at least once in the past but currently isn't. Then 
immediately after the first timer the t->expires check becomes useless? 
I must be missing something.. Oh, you have piggy-backed some custom code 
to timers.. hm.. Looks like it would work.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

>   			GEM_TRACE("%s: expired last=%llx:%lld, prio=%d, hint=%d\n",
>   				  engine->name,
>   				  last->fence.context,
> @@ -2074,6 +2074,8 @@ static void process_csb(struct intel_engine_cs *engine)
>   
>   			if (enable_timeslice(execlists))
>   				mod_timer(&execlists->timer, jiffies + 1);
> +			else
> +				cancel_timer(&execlists->timer);
>   
>   			WRITE_ONCE(execlists->pending[0], NULL);
>   		} else {
> @@ -3591,7 +3593,7 @@ gen12_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>   
>   static void execlists_park(struct intel_engine_cs *engine)
>   {
> -	del_timer(&engine->execlists.timer);
> +	cancel_timer(&engine->execlists.timer);
>   	cancel_timer(&engine->execlists.preempt);
>   }
>   
>

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 60e897c000b2..f9bf78032ad7 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1613,7 +1613,7 @@  static void execlists_dequeue(struct intel_engine_cs *engine)
 			last->hw_context->lrc_desc |= CTX_DESC_FORCE_RESTORE;
 			last = NULL;
 		} else if (need_timeslice(engine, last) &&
-			   !timer_pending(&engine->execlists.timer)) {
+			   timer_expired(&engine->execlists.timer)) {
 			GEM_TRACE("%s: expired last=%llx:%lld, prio=%d, hint=%d\n",
 				  engine->name,
 				  last->fence.context,
@@ -2074,6 +2074,8 @@  static void process_csb(struct intel_engine_cs *engine)
 
 			if (enable_timeslice(execlists))
 				mod_timer(&execlists->timer, jiffies + 1);
+			else
+				cancel_timer(&execlists->timer);
 
 			WRITE_ONCE(execlists->pending[0], NULL);
 		} else {
@@ -3591,7 +3593,7 @@  gen12_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 
 static void execlists_park(struct intel_engine_cs *engine)
 {
-	del_timer(&engine->execlists.timer);
+	cancel_timer(&engine->execlists.timer);
 	cancel_timer(&engine->execlists.preempt);
 }