[02/10] drm/i915/execlists: Clear semaphore immediately upon ELSP promotion
diff mbox series

Message ID 20191014220534.1662-2-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
There is no significance to our delay before clearing the semaphore the
engine is waiting on, so release it as soon as we acknowledge the CS
update following our preemption request.

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

Comments

Tvrtko Ursulin Oct. 15, 2019, 9:16 a.m. UTC | #1
On 14/10/2019 23:05, Chris Wilson wrote:
> There is no significance to our delay before clearing the semaphore the
> engine is waiting on, so release it as soon as we acknowledge the CS
> update following our preemption request.

And significance of moving it earlier? More parallelization between GPU 
and CPU? Could drop a note to say so.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 484efe3b4273..21635db8d76c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1910,6 +1910,9 @@ static void process_csb(struct intel_engine_cs *engine)
>   		else
>   			promote = gen8_csb_parse(execlists, buf + 2 * head);
>   		if (promote) {
> +			if (!inject_preempt_hang(execlists))
> +				ring_set_paused(engine, 0);
> +
>   			/* cancel old inflight, prepare for switch */
>   			trace_ports(execlists, "preempted", execlists->active);
>   			while (*execlists->active)
> @@ -1926,9 +1929,6 @@ static void process_csb(struct intel_engine_cs *engine)
>   			if (enable_timeslice(execlists))
>   				mod_timer(&execlists->timer, jiffies + 1);
>   
> -			if (!inject_preempt_hang(execlists))
> -				ring_set_paused(engine, 0);
> -
>   			WRITE_ONCE(execlists->pending[0], NULL);
>   		} else {
>   			GEM_BUG_ON(!*execlists->active);
> 

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

Regards,

Tvrtko
Chris Wilson Oct. 15, 2019, 9:26 a.m. UTC | #2
Quoting Tvrtko Ursulin (2019-10-15 10:16:01)
> 
> On 14/10/2019 23:05, Chris Wilson wrote:
> > There is no significance to our delay before clearing the semaphore the
> > engine is waiting on, so release it as soon as we acknowledge the CS
> > update following our preemption request.
> 
> And significance of moving it earlier? More parallelization between GPU 
> and CPU? Could drop a note to say so.

Yup. But we talking micro optimisations on a (hopefully :) fast path.
-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 484efe3b4273..21635db8d76c 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1910,6 +1910,9 @@  static void process_csb(struct intel_engine_cs *engine)
 		else
 			promote = gen8_csb_parse(execlists, buf + 2 * head);
 		if (promote) {
+			if (!inject_preempt_hang(execlists))
+				ring_set_paused(engine, 0);
+
 			/* cancel old inflight, prepare for switch */
 			trace_ports(execlists, "preempted", execlists->active);
 			while (*execlists->active)
@@ -1926,9 +1929,6 @@  static void process_csb(struct intel_engine_cs *engine)
 			if (enable_timeslice(execlists))
 				mod_timer(&execlists->timer, jiffies + 1);
 
-			if (!inject_preempt_hang(execlists))
-				ring_set_paused(engine, 0);
-
 			WRITE_ONCE(execlists->pending[0], NULL);
 		} else {
 			GEM_BUG_ON(!*execlists->active);