[PATCH] drm/i915/execlists: Avoid false matching of preemption when not enabled

Chris Wilson chris at chris-wilson.co.uk
Sat Jan 27 19:19:08 UTC 2018


If we haven't enabled preemption for execlists, the PREEMPT_ID may be
assigned to an ordinary user context, for which we should not falsely
declare preemption complete!

If we record the id assigned to the preempt context (or an invalid one
if preemption is disabled), then we can forgo hardcoding a fixed
signature for detecting the end of preemption.

Reported-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
Fixes: beecec901790 ("drm/i915/execlists: Preemption!")
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Michal Winiarski <michal.winiarski at intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
Cc: Mika Kuoppala <mika.kuoppala at intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 13 +++++++++----
 drivers/gpu/drm/i915/intel_ringbuffer.h |  5 +++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2fa328d512fc..e7d933790c5d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -161,7 +161,6 @@
 #define EXECLISTS_REQUEST_SIZE 64 /* bytes */
 #define WA_TAIL_DWORDS 2
 #define WA_TAIL_BYTES (sizeof(u32) * WA_TAIL_DWORDS)
-#define PREEMPT_ID 0x1
 
 static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 					    struct intel_engine_cs *engine);
@@ -448,7 +447,8 @@ static void inject_preempt_context(struct intel_engine_cs *engine)
 		&engine->i915->preempt_context->engine[engine->id];
 	unsigned int n;
 
-	GEM_BUG_ON(engine->i915->preempt_context->hw_id != PREEMPT_ID);
+	GEM_BUG_ON(engine->execlists.preempt_complete_status !=
+		   upper_32_bits(ce->lrc_desc));
 	GEM_BUG_ON(!IS_ALIGNED(ce->ring->size, WA_TAIL_BYTES));
 
 	memset(ce->ring->vaddr + ce->ring->tail, 0, WA_TAIL_BYTES);
@@ -844,7 +844,7 @@ static void execlists_submission_tasklet(unsigned long data)
 			GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE);
 
 			if (status & GEN8_CTX_STATUS_COMPLETE &&
-			    buf[2*head + 1] == PREEMPT_ID) {
+			    buf[2*head + 1] == execlists->preempt_complete_status) {
 				GEM_TRACE("%s preempt-idle\n", engine->name);
 
 				execlists_cancel_port_requests(execlists);
@@ -1982,6 +1982,11 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 	engine->execlists.elsp =
 		engine->i915->regs + i915_mmio_reg_offset(RING_ELSP(engine));
 
+	engine->execlists.preempt_complete_status = ~0u;
+	if (engine->i915->preempt_context)
+		engine->execlists.preempt_complete_status =
+			upper_32_bits(engine->i915->preempt_context->engine[engine->id].lrc_desc);
+
 	return 0;
 
 error:
@@ -2244,7 +2249,7 @@ populate_lr_context(struct i915_gem_context *ctx,
 	if (!engine->default_state)
 		regs[CTX_CONTEXT_CONTROL + 1] |=
 			_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT);
-	if (ctx->hw_id == PREEMPT_ID)
+	if (ctx == ctx->i915->preempt_context)
 		regs[CTX_CONTEXT_CONTROL + 1] |=
 			_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT |
 					   CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c5ff203e42d6..03be8995f415 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -279,6 +279,11 @@ struct intel_engine_execlists {
 	 * @csb_use_mmio: access csb through mmio, instead of hwsp
 	 */
 	bool csb_use_mmio;
+
+	/**
+	 * @preempt_complete_status: expected CSB upon completing preemption
+	 */
+	u32 preempt_complete_status;
 };
 
 #define INTEL_ENGINE_CS_MAX_NAME 8
-- 
2.15.1



More information about the Intel-gfx-trybot mailing list