diff mbox

[v2] drm/i915/execlists: Track begin/end of execlists submission sequences

Message ID 20180331130626.10712-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson March 31, 2018, 1:06 p.m. UTC
We would like to start doing some bookkeeping at the beginning, between
contexts and at the end of execlists submission. We already mark the
beginning and end using EXECLISTS_ACTIVE_USER, to provide an indication
when the HW is idle. This give us a pair of sequence points we can then
expand on for further bookkeeping.

v2: Refactor guc submission to share the same begin/end.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Francisco Jerez <currojerez@riseup.net>
Reviewed-by: Francisco Jerez <currojerez@riseup.net> #v1
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 17 ++++++----
 drivers/gpu/drm/i915/intel_lrc.c            | 50 ++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_ringbuffer.h     | 15 ++++++++-
 3 files changed, 63 insertions(+), 19 deletions(-)

Comments

Francisco Jerez April 2, 2018, 4:32 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> We would like to start doing some bookkeeping at the beginning, between
> contexts and at the end of execlists submission. We already mark the
> beginning and end using EXECLISTS_ACTIVE_USER, to provide an indication
> when the HW is idle. This give us a pair of sequence points we can then
> expand on for further bookkeeping.
>
> v2: Refactor guc submission to share the same begin/end.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Francisco Jerez <currojerez@riseup.net>
> Reviewed-by: Francisco Jerez <currojerez@riseup.net> #v1

Thanks, v2 is also:

Reviewed-by: Francisco Jerez <currojerez@riseup.net>

> ---
>  drivers/gpu/drm/i915/intel_guc_submission.c | 17 ++++++----
>  drivers/gpu/drm/i915/intel_lrc.c            | 50 ++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.h     | 15 ++++++++-
>  3 files changed, 63 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 207cda062626..749f27916a02 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -728,7 +728,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>  	execlists->first = rb;
>  	if (submit) {
>  		port_assign(port, last);
> -		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
> +		execlists_user_begin(execlists, execlists->port);
>  		guc_submit(engine);
>  	}
>  
> @@ -748,17 +748,20 @@ static void guc_submission_tasklet(unsigned long data)
>  	struct execlist_port *port = execlists->port;
>  	struct i915_request *rq;
>  
> -	rq = port_request(&port[0]);
> +	rq = port_request(port);
>  	while (rq && i915_request_completed(rq)) {
>  		trace_i915_request_out(rq);
>  		i915_request_put(rq);
>  
> -		execlists_port_complete(execlists, port);
> -
> -		rq = port_request(&port[0]);
> +		port = execlists_port_complete(execlists, port);
> +		if (port_isset(port)) {
> +			execlists_user_begin(execlists, port);
> +			rq = port_request(port);
> +		} else {
> +			execlists_user_end(execlists);
> +			rq = NULL;
> +		}
>  	}
> -	if (!rq)
> -		execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
>  
>  	if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) &&
>  	    intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) ==
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f60b61bf8b3b..4d08875422b6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -374,6 +374,19 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
>  				   status, rq);
>  }
>  
> +inline void
> +execlists_user_begin(struct intel_engine_execlists *execlists,
> +		     const struct execlist_port *port)
> +{
> +	execlists_set_active_once(execlists, EXECLISTS_ACTIVE_USER);
> +}
> +
> +inline void
> +execlists_user_end(struct intel_engine_execlists *execlists)
> +{
> +	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +}
> +
>  static inline void
>  execlists_context_schedule_in(struct i915_request *rq)
>  {
> @@ -711,7 +724,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  	spin_unlock_irq(&engine->timeline->lock);
>  
>  	if (submit) {
> -		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
> +		execlists_user_begin(execlists, execlists->port);
>  		execlists_submit_ports(engine);
>  	}
>  
> @@ -742,7 +755,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>  		port++;
>  	}
>  
> -	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +	execlists_user_end(execlists);
>  }
>  
>  static void clear_gtiir(struct intel_engine_cs *engine)
> @@ -873,7 +886,7 @@ static void execlists_submission_tasklet(unsigned long data)
>  {
>  	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
>  	struct intel_engine_execlists * const execlists = &engine->execlists;
> -	struct execlist_port * const port = execlists->port;
> +	struct execlist_port *port = execlists->port;
>  	struct drm_i915_private *dev_priv = engine->i915;
>  	bool fw = false;
>  
> @@ -1012,10 +1025,28 @@ static void execlists_submission_tasklet(unsigned long data)
>  
>  			GEM_BUG_ON(count == 0);
>  			if (--count == 0) {
> +				/*
> +				 * On the final event corresponding to the
> +				 * submission of this context, we expect either
> +				 * an element-switch event or a completion
> +				 * event (and on completion, the active-idle
> +				 * marker). No more preemptions, lite-restore
> +				 * or otherwise.
> +				 */
>  				GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
>  				GEM_BUG_ON(port_isset(&port[1]) &&
>  					   !(status & GEN8_CTX_STATUS_ELEMENT_SWITCH));
> +				GEM_BUG_ON(!port_isset(&port[1]) &&
> +					   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
> +
> +				/*
> +				 * We rely on the hardware being strongly
> +				 * ordered, that the breadcrumb write is
> +				 * coherent (visible from the CPU) before the
> +				 * user interrupt and CSB is processed.
> +				 */
>  				GEM_BUG_ON(!i915_request_completed(rq));
> +
>  				execlists_context_schedule_out(rq);
>  				trace_i915_request_out(rq);
>  				i915_request_put(rq);
> @@ -1023,17 +1054,14 @@ static void execlists_submission_tasklet(unsigned long data)
>  				GEM_TRACE("%s completed ctx=%d\n",
>  					  engine->name, port->context_id);
>  
> -				execlists_port_complete(execlists, port);
> +				port = execlists_port_complete(execlists, port);
> +				if (port_isset(port))
> +					execlists_user_begin(execlists, port);
> +				else
> +					execlists_user_end(execlists);
>  			} else {
>  				port_set(port, port_pack(rq, count));
>  			}
> -
> -			/* After the final element, the hw should be idle */
> -			GEM_BUG_ON(port_count(port) == 0 &&
> -				   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
> -			if (port_count(port) == 0)
> -				execlists_clear_active(execlists,
> -						       EXECLISTS_ACTIVE_USER);
>  		}
>  
>  		if (head != execlists->csb_head) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index a02c7b3b9d55..40461e29cdab 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -638,6 +638,13 @@ execlists_set_active(struct intel_engine_execlists *execlists,
>  	__set_bit(bit, (unsigned long *)&execlists->active);
>  }
>  
> +static inline bool
> +execlists_set_active_once(struct intel_engine_execlists *execlists,
> +			  unsigned int bit)
> +{
> +	return !__test_and_set_bit(bit, (unsigned long *)&execlists->active);
> +}
> +
>  static inline void
>  execlists_clear_active(struct intel_engine_execlists *execlists,
>  		       unsigned int bit)
> @@ -652,6 +659,10 @@ execlists_is_active(const struct intel_engine_execlists *execlists,
>  	return test_bit(bit, (unsigned long *)&execlists->active);
>  }
>  
> +void execlists_user_begin(struct intel_engine_execlists *execlists,
> +			  const struct execlist_port *port);
> +void execlists_user_end(struct intel_engine_execlists *execlists);
> +
>  void
>  execlists_cancel_port_requests(struct intel_engine_execlists * const execlists);
>  
> @@ -664,7 +675,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>  	return execlists->port_mask + 1;
>  }
>  
> -static inline void
> +static inline struct execlist_port *
>  execlists_port_complete(struct intel_engine_execlists * const execlists,
>  			struct execlist_port * const port)
>  {
> @@ -675,6 +686,8 @@ execlists_port_complete(struct intel_engine_execlists * const execlists,
>  
>  	memmove(port, port + 1, m * sizeof(struct execlist_port));
>  	memset(port + m, 0, sizeof(struct execlist_port));
> +
> +	return port;
>  }
>  
>  static inline unsigned int
> -- 
> 2.16.3
Mika Kuoppala April 3, 2018, 11:17 a.m. UTC | #2
Chris Wilson <chris@chris-wilson.co.uk> writes:

> We would like to start doing some bookkeeping at the beginning, between
> contexts and at the end of execlists submission. We already mark the
> beginning and end using EXECLISTS_ACTIVE_USER, to provide an indication
> when the HW is idle. This give us a pair of sequence points we can then
> expand on for further bookkeeping.
>
> v2: Refactor guc submission to share the same begin/end.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Francisco Jerez <currojerez@riseup.net>
> Reviewed-by: Francisco Jerez <currojerez@riseup.net> #v1
> ---
>  drivers/gpu/drm/i915/intel_guc_submission.c | 17 ++++++----
>  drivers/gpu/drm/i915/intel_lrc.c            | 50 ++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_ringbuffer.h     | 15 ++++++++-
>  3 files changed, 63 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index 207cda062626..749f27916a02 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -728,7 +728,7 @@ static void guc_dequeue(struct intel_engine_cs *engine)
>  	execlists->first = rb;
>  	if (submit) {
>  		port_assign(port, last);
> -		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
> +		execlists_user_begin(execlists, execlists->port);
>  		guc_submit(engine);
>  	}
>  
> @@ -748,17 +748,20 @@ static void guc_submission_tasklet(unsigned long data)
>  	struct execlist_port *port = execlists->port;
>  	struct i915_request *rq;
>  
> -	rq = port_request(&port[0]);
> +	rq = port_request(port);
>  	while (rq && i915_request_completed(rq)) {
>  		trace_i915_request_out(rq);
>  		i915_request_put(rq);
>  
> -		execlists_port_complete(execlists, port);
> -
> -		rq = port_request(&port[0]);
> +		port = execlists_port_complete(execlists, port);
> +		if (port_isset(port)) {
> +			execlists_user_begin(execlists, port);
> +			rq = port_request(port);
> +		} else {
> +			execlists_user_end(execlists);
> +			rq = NULL;

I did ponder doing the user_begin/end pair in complete
but better not to hide it in there as we might end up
doing completes without notifying users.

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

> +		}
>  	}
> -	if (!rq)
> -		execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
>  
>  	if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) &&
>  	    intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) ==
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f60b61bf8b3b..4d08875422b6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -374,6 +374,19 @@ execlists_context_status_change(struct i915_request *rq, unsigned long status)
>  				   status, rq);
>  }
>  
> +inline void
> +execlists_user_begin(struct intel_engine_execlists *execlists,
> +		     const struct execlist_port *port)
> +{
> +	execlists_set_active_once(execlists, EXECLISTS_ACTIVE_USER);
> +}
> +
> +inline void
> +execlists_user_end(struct intel_engine_execlists *execlists)
> +{
> +	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +}
> +
>  static inline void
>  execlists_context_schedule_in(struct i915_request *rq)
>  {
> @@ -711,7 +724,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>  	spin_unlock_irq(&engine->timeline->lock);
>  
>  	if (submit) {
> -		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
> +		execlists_user_begin(execlists, execlists->port);
>  		execlists_submit_ports(engine);
>  	}
>  
> @@ -742,7 +755,7 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
>  		port++;
>  	}
>  
> -	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
> +	execlists_user_end(execlists);
>  }
>  
>  static void clear_gtiir(struct intel_engine_cs *engine)
> @@ -873,7 +886,7 @@ static void execlists_submission_tasklet(unsigned long data)
>  {
>  	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
>  	struct intel_engine_execlists * const execlists = &engine->execlists;
> -	struct execlist_port * const port = execlists->port;
> +	struct execlist_port *port = execlists->port;
>  	struct drm_i915_private *dev_priv = engine->i915;
>  	bool fw = false;
>  
> @@ -1012,10 +1025,28 @@ static void execlists_submission_tasklet(unsigned long data)
>  
>  			GEM_BUG_ON(count == 0);
>  			if (--count == 0) {
> +				/*
> +				 * On the final event corresponding to the
> +				 * submission of this context, we expect either
> +				 * an element-switch event or a completion
> +				 * event (and on completion, the active-idle
> +				 * marker). No more preemptions, lite-restore
> +				 * or otherwise.
> +				 */
>  				GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
>  				GEM_BUG_ON(port_isset(&port[1]) &&
>  					   !(status & GEN8_CTX_STATUS_ELEMENT_SWITCH));
> +				GEM_BUG_ON(!port_isset(&port[1]) &&
> +					   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
> +
> +				/*
> +				 * We rely on the hardware being strongly
> +				 * ordered, that the breadcrumb write is
> +				 * coherent (visible from the CPU) before the
> +				 * user interrupt and CSB is processed.
> +				 */
>  				GEM_BUG_ON(!i915_request_completed(rq));
> +
>  				execlists_context_schedule_out(rq);
>  				trace_i915_request_out(rq);
>  				i915_request_put(rq);
> @@ -1023,17 +1054,14 @@ static void execlists_submission_tasklet(unsigned long data)
>  				GEM_TRACE("%s completed ctx=%d\n",
>  					  engine->name, port->context_id);
>  
> -				execlists_port_complete(execlists, port);
> +				port = execlists_port_complete(execlists, port);
> +				if (port_isset(port))
> +					execlists_user_begin(execlists, port);
> +				else
> +					execlists_user_end(execlists);
>  			} else {
>  				port_set(port, port_pack(rq, count));
>  			}
> -
> -			/* After the final element, the hw should be idle */
> -			GEM_BUG_ON(port_count(port) == 0 &&
> -				   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
> -			if (port_count(port) == 0)
> -				execlists_clear_active(execlists,
> -						       EXECLISTS_ACTIVE_USER);
>  		}
>  
>  		if (head != execlists->csb_head) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index a02c7b3b9d55..40461e29cdab 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -638,6 +638,13 @@ execlists_set_active(struct intel_engine_execlists *execlists,
>  	__set_bit(bit, (unsigned long *)&execlists->active);
>  }
>  
> +static inline bool
> +execlists_set_active_once(struct intel_engine_execlists *execlists,
> +			  unsigned int bit)
> +{
> +	return !__test_and_set_bit(bit, (unsigned long *)&execlists->active);
> +}
> +
>  static inline void
>  execlists_clear_active(struct intel_engine_execlists *execlists,
>  		       unsigned int bit)
> @@ -652,6 +659,10 @@ execlists_is_active(const struct intel_engine_execlists *execlists,
>  	return test_bit(bit, (unsigned long *)&execlists->active);
>  }
>  
> +void execlists_user_begin(struct intel_engine_execlists *execlists,
> +			  const struct execlist_port *port);
> +void execlists_user_end(struct intel_engine_execlists *execlists);
> +
>  void
>  execlists_cancel_port_requests(struct intel_engine_execlists * const execlists);
>  
> @@ -664,7 +675,7 @@ execlists_num_ports(const struct intel_engine_execlists * const execlists)
>  	return execlists->port_mask + 1;
>  }
>  
> -static inline void
> +static inline struct execlist_port *
>  execlists_port_complete(struct intel_engine_execlists * const execlists,
>  			struct execlist_port * const port)
>  {
> @@ -675,6 +686,8 @@ execlists_port_complete(struct intel_engine_execlists * const execlists,
>  
>  	memmove(port, port + 1, m * sizeof(struct execlist_port));
>  	memset(port + m, 0, sizeof(struct execlist_port));
> +
> +	return port;
>  }
>  
>  static inline unsigned int
> -- 
> 2.16.3
Chris Wilson April 3, 2018, 11:33 a.m. UTC | #3
Quoting Francisco Jerez (2018-04-02 17:32:20)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > We would like to start doing some bookkeeping at the beginning, between
> > contexts and at the end of execlists submission. We already mark the
> > beginning and end using EXECLISTS_ACTIVE_USER, to provide an indication
> > when the HW is idle. This give us a pair of sequence points we can then
> > expand on for further bookkeeping.
> >
> > v2: Refactor guc submission to share the same begin/end.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Francisco Jerez <currojerez@riseup.net>
> > Reviewed-by: Francisco Jerez <currojerez@riseup.net> #v1
> 
> Thanks, v2 is also:
> 
> Reviewed-by: Francisco Jerez <currojerez@riseup.net>

Pushed this as it should give us a common hook for the two series.

I belatedly remembered, that we do have a similar begin/end sequence
markers for *user* activity, mark_busy, mark_idle. These are signaled on
receipt of the first request from the user (may be an arbitrary time
before execution) and after completion of the final request, plus some
hysteresis of 100ms to 2s. It's not as exact as tracking activity on
execlists, but it would be nice if you could balance the pros/cons when
hooking up. That knowledge will be helpful for directing future api
changes, e.g. no one has done a thorough investigation of how far we can
push S0ix afaik.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 207cda062626..749f27916a02 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -728,7 +728,7 @@  static void guc_dequeue(struct intel_engine_cs *engine)
 	execlists->first = rb;
 	if (submit) {
 		port_assign(port, last);
-		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
+		execlists_user_begin(execlists, execlists->port);
 		guc_submit(engine);
 	}
 
@@ -748,17 +748,20 @@  static void guc_submission_tasklet(unsigned long data)
 	struct execlist_port *port = execlists->port;
 	struct i915_request *rq;
 
-	rq = port_request(&port[0]);
+	rq = port_request(port);
 	while (rq && i915_request_completed(rq)) {
 		trace_i915_request_out(rq);
 		i915_request_put(rq);
 
-		execlists_port_complete(execlists, port);
-
-		rq = port_request(&port[0]);
+		port = execlists_port_complete(execlists, port);
+		if (port_isset(port)) {
+			execlists_user_begin(execlists, port);
+			rq = port_request(port);
+		} else {
+			execlists_user_end(execlists);
+			rq = NULL;
+		}
 	}
-	if (!rq)
-		execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
 
 	if (execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT) &&
 	    intel_read_status_page(engine, I915_GEM_HWS_PREEMPT_INDEX) ==
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f60b61bf8b3b..4d08875422b6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -374,6 +374,19 @@  execlists_context_status_change(struct i915_request *rq, unsigned long status)
 				   status, rq);
 }
 
+inline void
+execlists_user_begin(struct intel_engine_execlists *execlists,
+		     const struct execlist_port *port)
+{
+	execlists_set_active_once(execlists, EXECLISTS_ACTIVE_USER);
+}
+
+inline void
+execlists_user_end(struct intel_engine_execlists *execlists)
+{
+	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
+}
+
 static inline void
 execlists_context_schedule_in(struct i915_request *rq)
 {
@@ -711,7 +724,7 @@  static void execlists_dequeue(struct intel_engine_cs *engine)
 	spin_unlock_irq(&engine->timeline->lock);
 
 	if (submit) {
-		execlists_set_active(execlists, EXECLISTS_ACTIVE_USER);
+		execlists_user_begin(execlists, execlists->port);
 		execlists_submit_ports(engine);
 	}
 
@@ -742,7 +755,7 @@  execlists_cancel_port_requests(struct intel_engine_execlists * const execlists)
 		port++;
 	}
 
-	execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER);
+	execlists_user_end(execlists);
 }
 
 static void clear_gtiir(struct intel_engine_cs *engine)
@@ -873,7 +886,7 @@  static void execlists_submission_tasklet(unsigned long data)
 {
 	struct intel_engine_cs * const engine = (struct intel_engine_cs *)data;
 	struct intel_engine_execlists * const execlists = &engine->execlists;
-	struct execlist_port * const port = execlists->port;
+	struct execlist_port *port = execlists->port;
 	struct drm_i915_private *dev_priv = engine->i915;
 	bool fw = false;
 
@@ -1012,10 +1025,28 @@  static void execlists_submission_tasklet(unsigned long data)
 
 			GEM_BUG_ON(count == 0);
 			if (--count == 0) {
+				/*
+				 * On the final event corresponding to the
+				 * submission of this context, we expect either
+				 * an element-switch event or a completion
+				 * event (and on completion, the active-idle
+				 * marker). No more preemptions, lite-restore
+				 * or otherwise.
+				 */
 				GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
 				GEM_BUG_ON(port_isset(&port[1]) &&
 					   !(status & GEN8_CTX_STATUS_ELEMENT_SWITCH));
+				GEM_BUG_ON(!port_isset(&port[1]) &&
+					   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
+
+				/*
+				 * We rely on the hardware being strongly
+				 * ordered, that the breadcrumb write is
+				 * coherent (visible from the CPU) before the
+				 * user interrupt and CSB is processed.
+				 */
 				GEM_BUG_ON(!i915_request_completed(rq));
+
 				execlists_context_schedule_out(rq);
 				trace_i915_request_out(rq);
 				i915_request_put(rq);
@@ -1023,17 +1054,14 @@  static void execlists_submission_tasklet(unsigned long data)
 				GEM_TRACE("%s completed ctx=%d\n",
 					  engine->name, port->context_id);
 
-				execlists_port_complete(execlists, port);
+				port = execlists_port_complete(execlists, port);
+				if (port_isset(port))
+					execlists_user_begin(execlists, port);
+				else
+					execlists_user_end(execlists);
 			} else {
 				port_set(port, port_pack(rq, count));
 			}
-
-			/* After the final element, the hw should be idle */
-			GEM_BUG_ON(port_count(port) == 0 &&
-				   !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
-			if (port_count(port) == 0)
-				execlists_clear_active(execlists,
-						       EXECLISTS_ACTIVE_USER);
 		}
 
 		if (head != execlists->csb_head) {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a02c7b3b9d55..40461e29cdab 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -638,6 +638,13 @@  execlists_set_active(struct intel_engine_execlists *execlists,
 	__set_bit(bit, (unsigned long *)&execlists->active);
 }
 
+static inline bool
+execlists_set_active_once(struct intel_engine_execlists *execlists,
+			  unsigned int bit)
+{
+	return !__test_and_set_bit(bit, (unsigned long *)&execlists->active);
+}
+
 static inline void
 execlists_clear_active(struct intel_engine_execlists *execlists,
 		       unsigned int bit)
@@ -652,6 +659,10 @@  execlists_is_active(const struct intel_engine_execlists *execlists,
 	return test_bit(bit, (unsigned long *)&execlists->active);
 }
 
+void execlists_user_begin(struct intel_engine_execlists *execlists,
+			  const struct execlist_port *port);
+void execlists_user_end(struct intel_engine_execlists *execlists);
+
 void
 execlists_cancel_port_requests(struct intel_engine_execlists * const execlists);
 
@@ -664,7 +675,7 @@  execlists_num_ports(const struct intel_engine_execlists * const execlists)
 	return execlists->port_mask + 1;
 }
 
-static inline void
+static inline struct execlist_port *
 execlists_port_complete(struct intel_engine_execlists * const execlists,
 			struct execlist_port * const port)
 {
@@ -675,6 +686,8 @@  execlists_port_complete(struct intel_engine_execlists * const execlists,
 
 	memmove(port, port + 1, m * sizeof(struct execlist_port));
 	memset(port + m, 0, sizeof(struct execlist_port));
+
+	return port;
 }
 
 static inline unsigned int