[Intel-gfx] [PATCH v2] drm/i915/execlists: Ensure arbitration is disabled for the breadcrumb

[Chris Wilson]

If we interrupt building of the request, we may emit a request with no
payload at all, with the consequence that we never disable arbitration
prior to the breadcrumb. If we get preempted during the breadcrumb, it
appears possible to lose the association of the interrupt with
breadcrumb, and under the right conditions miss the breadcrumb interrupt
entirely, leaving the request's waiters dangling.

Now that we always disable the arbitration in the breadcrumb, we can
remove the redundant command to disable it after emitting the batch.

Testcase: igt/gem_concurrent_blit
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 851e62ddcb87..c8e87011044e 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2117,7 +2117,7 @@ static int gen9_emit_bb_start(struct i915_request *rq,
 {
 	u32 *cs;
 
-	cs = intel_ring_begin(rq, 6);
+	cs = intel_ring_begin(rq, 4);
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
@@ -2128,9 +2128,6 @@ static int gen9_emit_bb_start(struct i915_request *rq,
 	*cs++ = lower_32_bits(offset);
 	*cs++ = upper_32_bits(offset);
 
-	*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
-	*cs++ = MI_NOOP;
-
 	intel_ring_advance(rq, cs);
 
 	return 0;
@@ -2267,19 +2264,21 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
 
 static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
 {
+	*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
+
 	cs = gen8_emit_ggtt_write(cs,
 				  request->fence.seqno,
 				  request->timeline->hwsp_offset,
 				  0);
+	*cs++ = MI_USER_INTERRUPT;
 
 	cs = gen8_emit_ggtt_write(cs,
 				  intel_engine_next_hangcheck_seqno(request->engine),
 				  I915_GEM_HWS_HANGCHECK_ADDR,
 				  MI_FLUSH_DW_STORE_INDEX);
 
-
-	*cs++ = MI_USER_INTERRUPT;
 	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
+	*cs++ = MI_NOOP;
 
 	request->tail = intel_ring_offset(request, cs);
 	assert_ring_tail_valid(request->ring, request->tail);
@@ -2289,6 +2288,8 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
 
 static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 {
+	*cs++ = MI_ARB_ON_OFF | MI_ARB_DISABLE;
+
 	cs = gen8_emit_ggtt_write_rcs(cs,
 				      request->fence.seqno,
 				      request->timeline->hwsp_offset,
@@ -2297,14 +2298,15 @@ static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 				      PIPE_CONTROL_DC_FLUSH_ENABLE |
 				      PIPE_CONTROL_FLUSH_ENABLE |
 				      PIPE_CONTROL_CS_STALL);
+	*cs++ = MI_USER_INTERRUPT;
 
 	cs = gen8_emit_ggtt_write_rcs(cs,
 				      intel_engine_next_hangcheck_seqno(request->engine),
 				      I915_GEM_HWS_HANGCHECK_ADDR,
 				      PIPE_CONTROL_STORE_DATA_INDEX);
 
-	*cs++ = MI_USER_INTERRUPT;
 	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
+	*cs++ = MI_NOOP;
 
 	request->tail = intel_ring_offset(request, cs);
 	assert_ring_tail_valid(request->ring, request->tail);
-- 
2.20.1