All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Engine relative MMIO
@ 2019-04-24  1:50 John.C.Harrison
  2019-04-24  2:49 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Engine relative MMIO (rev4) Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: John.C.Harrison @ 2019-04-24  1:50 UTC (permalink / raw)
  To: Intel-GFX

From: John Harrison <John.C.Harrison@Intel.com>

With virtual engines, it is no longer possible to know which specific
physical engine a given request will be executed on at the time that
request is generated. This means that the request itself must be engine
agnostic - any direct register writes must be relative to the engine
and not absolute addresses.

The LRI command has support for engine relative addressing. However,
the mechanism is not transparent to the driver. The scheme for Gen11
(MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
absolute engine base component. The hardware then adds on the correct
engine offset at execution time.

Due to the non-trivial and differing schemes on different hardware, it
is not possible to simply update the code that creates the LRI
commands to set a remap flag and let the hardware get on with it.
Instead, this patch adds function wrappers for generating the LRI
command itself and then for constructing the correct address to use
with the LRI.

v2: Fix build break in GVT. Remove flags parameter [review feedback
from Chris W].

v3: Fix build break in selftest. Rebase to newer base tree and fix
merge conflict.

v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
code paths [review feedback from Chris W].

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gvt/mmio_context.c       | 16 +++-
 drivers/gpu/drm/i915/i915_cmd_parser.c        |  4 +-
 drivers/gpu/drm/i915/i915_gem_context.c       | 12 +--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c    |  3 +-
 drivers/gpu/drm/i915/i915_perf.c              | 19 +++--
 drivers/gpu/drm/i915/intel_engine_cs.c        | 11 +++
 drivers/gpu/drm/i915/intel_gpu_commands.h     |  6 +-
 drivers/gpu/drm/i915/intel_lrc.c              | 79 ++++++++++---------
 drivers/gpu/drm/i915/intel_lrc_reg.h          |  4 +-
 drivers/gpu/drm/i915/intel_mocs.c             | 17 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c       | 45 +++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h       |  4 +
 drivers/gpu/drm/i915/intel_workarounds.c      |  4 +-
 .../drm/i915/selftests/intel_workarounds.c    |  9 ++-
 14 files changed, 154 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
index e7e14c842be4..1b4d78e55ed6 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -199,14 +199,14 @@ restore_context_mmio_for_inhibit(struct intel_vgpu *vgpu,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(count);
+	*cs++ = i915_get_lri_cmd(req->engine, count);
 	for (mmio = gvt->engine_mmio_list.mmio;
 	     i915_mmio_reg_valid(mmio->reg); mmio++) {
 		if (mmio->ring_id != ring_id ||
 		    !mmio->in_context)
 			continue;
 
-		*cs++ = i915_mmio_reg_offset(mmio->reg);
+		*cs++ = i915_get_lri_reg(req->engine, mmio->reg);
 		*cs++ = vgpu_vreg_t(vgpu, mmio->reg) |
 				(mmio->mask << 16);
 		gvt_dbg_core("add lri reg pair 0x%x:0x%x in inhibit ctx, vgpu:%d, rind_id:%d\n",
@@ -234,7 +234,11 @@ restore_render_mocs_control_for_inhibit(struct intel_vgpu *vgpu,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE);
+	/*
+	 * GEN9_GFX_MOCS is not engine relative, therefore there is no
+	 * need for relative addressing.
+	 */
+	*cs++ = __MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE);
 
 	for (index = 0; index < GEN9_MOCS_SIZE; index++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_GFX_MOCS(index));
@@ -261,7 +265,11 @@ restore_render_mocs_l3cc_for_inhibit(struct intel_vgpu *vgpu,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE / 2);
+	/*
+	 * GEN9_LNCFCMOCS is not engine relative, therefore there is no
+	 * need for relative addressing.
+	 */
+	*cs++ = __MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE / 2);
 
 	for (index = 0; index < GEN9_MOCS_SIZE / 2; index++) {
 		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(index));
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 503d548a55f7..91ebe18aacc6 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -220,7 +220,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = {
 	CMD(  MI_SUSPEND_FLUSH,                 SMI,    F,  1,      S  ),
 	CMD(  MI_SEMAPHORE_MBOX,                SMI,   !F,  0xFF,   R  ),
 	CMD(  MI_STORE_DWORD_INDEX,             SMI,   !F,  0xFF,   R  ),
-	CMD(  MI_LOAD_REGISTER_IMM(1),          SMI,   !F,  0xFF,   W,
+	CMD(  __MI_LOAD_REGISTER_IMM(1),        SMI,   !F,  0xFF,   W,
 	      .reg = { .offset = 1, .mask = 0x007FFFFC, .step = 2 }    ),
 	CMD(  MI_STORE_REGISTER_MEM,            SMI,    F,  3,     W | B,
 	      .reg = { .offset = 1, .mask = 0x007FFFFC },
@@ -1182,7 +1182,7 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 					return false;
 				}
 
-				if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1) &&
+				if (desc->cmd.value == __MI_LOAD_REGISTER_IMM(1) &&
 				    (offset + 2 > length ||
 				     (cmd[offset + 1] & reg->mask) != reg->value)) {
 					DRM_DEBUG_DRIVER("CMD: Rejected LRI to masked register 0x%08X\n",
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index dd728b26b5aa..f25dc613c266 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -1039,11 +1039,11 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data)
 		if (IS_ERR(cs))
 			return PTR_ERR(cs);
 
-		*cs++ = MI_LOAD_REGISTER_IMM(2);
+		*cs++ = i915_get_lri_cmd(engine, 2);
 
-		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 0));
+		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, 0));
 		*cs++ = upper_32_bits(pd_daddr);
-		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 0));
+		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, 0));
 		*cs++ = lower_32_bits(pd_daddr);
 
 		*cs++ = MI_NOOP;
@@ -1053,13 +1053,13 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data)
 		if (IS_ERR(cs))
 			return PTR_ERR(cs);
 
-		*cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES);
+		*cs++ = i915_get_lri_cmd(engine, 2 * GEN8_3LVL_PDPES);
 		for (i = GEN8_3LVL_PDPES; i--; ) {
 			const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
 
-			*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i));
+			*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, i));
 			*cs++ = upper_32_bits(pd_daddr);
-			*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i));
+			*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, i));
 			*cs++ = lower_32_bits(pd_daddr);
 		}
 		*cs++ = MI_NOOP;
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3d672c9edb94..983801f481ba 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1965,7 +1965,8 @@ static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(4);
+	/* Gen7 only so no need to support relative offsets */
+	*cs++ = __MI_LOAD_REGISTER_IMM(4);
 	for (i = 0; i < 4; i++) {
 		*cs++ = i915_mmio_reg_offset(GEN7_SO_WRITE_OFFSET(i));
 		*cs++ = 0;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 39a4804091d7..10d5ab991908 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1628,7 +1628,8 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
  * in the case that the OA unit has been disabled.
  */
 static void
-gen8_update_reg_state_unlocked(struct intel_context *ce,
+gen8_update_reg_state_unlocked(struct intel_engine_cs *engine,
+			       struct intel_context *ce,
 			       u32 *reg_state,
 			       const struct i915_oa_config *oa_config)
 {
@@ -1647,7 +1648,12 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 	};
 	int i;
 
-	CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
+	/*
+	 * NB: The LRI instruction is generated by the hardware.
+	 * Should we read it in and assert that the offset flag is set?
+	 */
+
+	CTX_REG(engine, reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
 		(i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
 		(i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
 		GEN8_OA_COUNTER_RESUME);
@@ -1674,10 +1680,10 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
 			}
 		}
 
-		CTX_REG(reg_state, state_offset, flex_regs[i], value);
+		CTX_REG(engine, reg_state, state_offset, flex_regs[i], value);
 	}
 
-	CTX_REG(reg_state,
+	CTX_REG(engine, reg_state,
 		CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
 		gen8_make_rpcs(i915, &ce->sseu));
 }
@@ -1752,7 +1758,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
 		ce->state->obj->mm.dirty = true;
 		regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
 
-		gen8_update_reg_state_unlocked(ce, regs, oa_config);
+		gen8_update_reg_state_unlocked(engine, ce, regs, oa_config);
 
 		i915_gem_object_unpin_map(ce->state->obj);
 	}
@@ -2146,7 +2152,8 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine,
 
 	stream = engine->i915->perf.oa.exclusive_stream;
 	if (stream)
-		gen8_update_reg_state_unlocked(ce, regs, stream->oa_config);
+		gen8_update_reg_state_unlocked(engine, ce, regs,
+					       stream->oa_config);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index eea9bec04f1b..ee33ce265820 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -246,6 +246,17 @@ static u32 __engine_mmio_base(struct drm_i915_private *i915,
 	return bases[i].base;
 }
 
+bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine)
+{
+	if (INTEL_GEN(engine->i915) < 11)
+		return false;
+
+	if (engine->id == BCS0)
+		return false;
+
+	return true;
+}
+
 static void __sprint_engine_name(char *name, const struct engine_info *info)
 {
 	WARN_ON(snprintf(name, INTEL_ENGINE_CS_MAX_NAME, "%s%u",
diff --git a/drivers/gpu/drm/i915/intel_gpu_commands.h b/drivers/gpu/drm/i915/intel_gpu_commands.h
index a34ece53a771..e7784b3fb759 100644
--- a/drivers/gpu/drm/i915/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/intel_gpu_commands.h
@@ -123,9 +123,13 @@
  *   simply ignores the register load under certain conditions.
  * - One can actually load arbitrary many arbitrary registers: Simply issue x
  *   address/value pairs. Don't overdue it, though, x <= 2^4 must hold!
+ * - Newer hardware supports engine relative addresses but older hardware does
+ *   not. So never call MI_LRI directly, always use the i915_get_lri_cmd()
+ *   and i915_get_lri_reg() helper functions.
  */
-#define MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
+#define __MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
 #define   MI_LRI_FORCE_POSTED		(1<<12)
+#define   MI_LRI_ADD_CS_MMIO_START_GEN11	(1<<19)
 #define MI_STORE_REGISTER_MEM        MI_INSTR(0x24, 1)
 #define MI_STORE_REGISTER_MEM_GEN8   MI_INSTR(0x24, 2)
 #define   MI_SRM_LRM_GLOBAL_GTT		(1<<22)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 4e0a351bfbca..41cbbcd9f0dd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1383,14 +1383,15 @@ static int emit_pdps(struct i915_request *rq)
 		return PTR_ERR(cs);
 
 	/* Ensure the LRI have landed before we invalidate & continue */
-	*cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED;
+	*cs++ = i915_get_lri_cmd(engine, 2 * GEN8_3LVL_PDPES) |
+				 MI_LRI_FORCE_POSTED;
 	for (i = GEN8_3LVL_PDPES; i--; ) {
 		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
 		u32 base = engine->mmio_base;
 
-		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i));
+		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, i));
 		*cs++ = upper_32_bits(pd_daddr);
-		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i));
+		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, i));
 		*cs++ = lower_32_bits(pd_daddr);
 	}
 	*cs++ = MI_NOOP;
@@ -1464,7 +1465,8 @@ gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine, u32 *batch)
 	*batch++ = i915_scratch_offset(engine->i915) + 256;
 	*batch++ = 0;
 
-	*batch++ = MI_LOAD_REGISTER_IMM(1);
+	/* Gen8/9 only so no need to support relative offsets */
+	*batch++ = __MI_LOAD_REGISTER_IMM(1);
 	*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
 	*batch++ = 0x40400000 | GEN8_LQSC_FLUSH_COHERENT_LINES;
 
@@ -1535,13 +1537,14 @@ struct lri {
 	u32 value;
 };
 
-static u32 *emit_lri(u32 *batch, const struct lri *lri, unsigned int count)
+static u32 *emit_lri(struct intel_engine_cs *engine, u32 *batch,
+		     const struct lri *lri, unsigned int count)
 {
 	GEM_BUG_ON(!count || count > 63);
 
-	*batch++ = MI_LOAD_REGISTER_IMM(count);
+	*batch++ = i915_get_lri_cmd(engine, count);
 	do {
-		*batch++ = i915_mmio_reg_offset(lri->reg);
+		*batch++ = i915_get_lri_reg(engine, lri->reg);
 		*batch++ = lri->value;
 	} while (lri++, --count);
 	*batch++ = MI_NOOP;
@@ -1579,7 +1582,7 @@ static u32 *gen9_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
 	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt,glk */
 	batch = gen8_emit_flush_coherentl3_wa(engine, batch);
 
-	batch = emit_lri(batch, lri, ARRAY_SIZE(lri));
+	batch = emit_lri(engine, batch, lri, ARRAY_SIZE(lri));
 
 	/* WaMediaPoolStateCmdInWABB:bxt,glk */
 	if (HAS_POOLED_EU(engine->i915)) {
@@ -2728,10 +2731,10 @@ static void execlists_init_reg_state(u32 *regs,
 	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
 	 * we are not initializing here).
 	 */
-	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
-				 MI_LRI_FORCE_POSTED;
+	regs[CTX_LRI_HEADER_0] = i915_get_lri_cmd(engine, rcs ? 14 : 11) |
+						  MI_LRI_FORCE_POSTED;
 
-	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
+	CTX_REG(engine, regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
 		_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
 		_MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH));
 	if (INTEL_GEN(engine->i915) < 11) {
@@ -2739,22 +2742,23 @@ static void execlists_init_reg_state(u32 *regs,
 			_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
 					    CTX_CTRL_RS_CTX_ENABLE);
 	}
-	CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
-	CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0);
-	CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0);
-	CTX_REG(regs, CTX_RING_BUFFER_CONTROL, RING_CTL(base),
+	CTX_REG(engine, regs, CTX_RING_HEAD, RING_HEAD(base), 0);
+	CTX_REG(engine, regs, CTX_RING_TAIL, RING_TAIL(base), 0);
+	CTX_REG(engine, regs, CTX_RING_BUFFER_START, RING_START(base), 0);
+	CTX_REG(engine, regs, CTX_RING_BUFFER_CONTROL, RING_CTL(base),
 		RING_CTL_SIZE(ring->size) | RING_VALID);
-	CTX_REG(regs, CTX_BB_HEAD_U, RING_BBADDR_UDW(base), 0);
-	CTX_REG(regs, CTX_BB_HEAD_L, RING_BBADDR(base), 0);
-	CTX_REG(regs, CTX_BB_STATE, RING_BBSTATE(base), RING_BB_PPGTT);
-	CTX_REG(regs, CTX_SECOND_BB_HEAD_U, RING_SBBADDR_UDW(base), 0);
-	CTX_REG(regs, CTX_SECOND_BB_HEAD_L, RING_SBBADDR(base), 0);
-	CTX_REG(regs, CTX_SECOND_BB_STATE, RING_SBBSTATE(base), 0);
+	CTX_REG(engine, regs, CTX_BB_HEAD_U, RING_BBADDR_UDW(base), 0);
+	CTX_REG(engine, regs, CTX_BB_HEAD_L, RING_BBADDR(base), 0);
+	CTX_REG(engine, regs, CTX_BB_STATE, RING_BBSTATE(base), RING_BB_PPGTT);
+	CTX_REG(engine, regs, CTX_SECOND_BB_HEAD_U, RING_SBBADDR_UDW(base), 0);
+	CTX_REG(engine, regs, CTX_SECOND_BB_HEAD_L, RING_SBBADDR(base), 0);
+	CTX_REG(engine, regs, CTX_SECOND_BB_STATE, RING_SBBSTATE(base), 0);
 	if (rcs) {
 		struct i915_ctx_workarounds *wa_ctx = &engine->wa_ctx;
 
-		CTX_REG(regs, CTX_RCS_INDIRECT_CTX, RING_INDIRECT_CTX(base), 0);
-		CTX_REG(regs, CTX_RCS_INDIRECT_CTX_OFFSET,
+		CTX_REG(engine, regs, CTX_RCS_INDIRECT_CTX,
+			RING_INDIRECT_CTX(base), 0);
+		CTX_REG(engine, regs, CTX_RCS_INDIRECT_CTX_OFFSET,
 			RING_INDIRECT_CTX_OFFSET(base), 0);
 		if (wa_ctx->indirect_ctx.size) {
 			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
@@ -2767,7 +2771,8 @@ static void execlists_init_reg_state(u32 *regs,
 				intel_lr_indirect_ctx_offset(engine) << 6;
 		}
 
-		CTX_REG(regs, CTX_BB_PER_CTX_PTR, RING_BB_PER_CTX_PTR(base), 0);
+		CTX_REG(engine, regs, CTX_BB_PER_CTX_PTR,
+			RING_BB_PER_CTX_PTR(base), 0);
 		if (wa_ctx->per_ctx.size) {
 			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
 
@@ -2776,18 +2781,19 @@ static void execlists_init_reg_state(u32 *regs,
 		}
 	}
 
-	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
+	regs[CTX_LRI_HEADER_1] = i915_get_lri_cmd(engine, 9) |
+						  MI_LRI_FORCE_POSTED;
 
-	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
+	CTX_REG(engine, regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
 	/* PDP values well be assigned later if needed */
-	CTX_REG(regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);
-	CTX_REG(regs, CTX_PDP3_LDW, GEN8_RING_PDP_LDW(base, 3), 0);
-	CTX_REG(regs, CTX_PDP2_UDW, GEN8_RING_PDP_UDW(base, 2), 0);
-	CTX_REG(regs, CTX_PDP2_LDW, GEN8_RING_PDP_LDW(base, 2), 0);
-	CTX_REG(regs, CTX_PDP1_UDW, GEN8_RING_PDP_UDW(base, 1), 0);
-	CTX_REG(regs, CTX_PDP1_LDW, GEN8_RING_PDP_LDW(base, 1), 0);
-	CTX_REG(regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(base, 0), 0);
-	CTX_REG(regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(base, 0), 0);
+ 	CTX_REG(engine, regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);
+	CTX_REG(engine, regs, CTX_PDP3_LDW, GEN8_RING_PDP_LDW(base, 3), 0);
+	CTX_REG(engine, regs, CTX_PDP2_UDW, GEN8_RING_PDP_UDW(base, 2), 0);
+	CTX_REG(engine, regs, CTX_PDP2_LDW, GEN8_RING_PDP_LDW(base, 2), 0);
+	CTX_REG(engine, regs, CTX_PDP1_UDW, GEN8_RING_PDP_UDW(base, 1), 0);
+	CTX_REG(engine, regs, CTX_PDP1_LDW, GEN8_RING_PDP_LDW(base, 1), 0);
+	CTX_REG(engine, regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(base, 0), 0);
+	CTX_REG(engine, regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(base, 0), 0);
 
 	if (i915_vm_is_4lvl(&ppgtt->vm)) {
 		/* 64b PPGTT (48bit canonical)
@@ -2803,8 +2809,9 @@ static void execlists_init_reg_state(u32 *regs,
 	}
 
 	if (rcs) {
-		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
-		CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
+		regs[CTX_LRI_HEADER_2] = i915_get_lri_cmd(engine, 1);
+		CTX_REG(engine, regs, CTX_R_PWR_CLK_STATE,
+			GEN8_R_PWR_CLK_STATE, 0);
 
 		i915_oa_init_reg_state(engine, ce, regs);
 	}
diff --git a/drivers/gpu/drm/i915/intel_lrc_reg.h b/drivers/gpu/drm/i915/intel_lrc_reg.h
index 5ef932d810a7..40b1142d0d74 100644
--- a/drivers/gpu/drm/i915/intel_lrc_reg.h
+++ b/drivers/gpu/drm/i915/intel_lrc_reg.h
@@ -39,10 +39,10 @@
 #define CTX_R_PWR_CLK_STATE		0x42
 #define CTX_END				0x44
 
-#define CTX_REG(reg_state, pos, reg, val) do { \
+#define CTX_REG(engine, reg_state, pos, reg, val) do { \
 	u32 *reg_state__ = (reg_state); \
 	const u32 pos__ = (pos); \
-	(reg_state__)[(pos__) + 0] = i915_mmio_reg_offset(reg); \
+	(reg_state__)[(pos__) + 0] = i915_get_lri_reg((engine), (reg)); \
 	(reg_state__)[(pos__) + 1] = (val); \
 } while (0)
 
diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
index 274ba78500c0..bb11d0f68bba 100644
--- a/drivers/gpu/drm/i915/intel_mocs.c
+++ b/drivers/gpu/drm/i915/intel_mocs.c
@@ -322,9 +322,6 @@ static u32 get_entry_control(const struct drm_i915_mocs_table *table,
 /**
  * intel_mocs_init_engine() - emit the mocs control table
  * @engine:	The engine for whom to emit the registers.
- *
- * This function simply emits a MI_LOAD_REGISTER_IMM command for the
- * given table starting at the given address.
  */
 void intel_mocs_init_engine(struct intel_engine_cs *engine)
 {
@@ -378,18 +375,20 @@ static int emit_mocs_control_table(struct i915_request *rq,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries);
+	*cs++ = i915_get_lri_cmd(rq->engine, table->n_entries);
 
 	for (index = 0; index < table->size; index++) {
 		u32 value = get_entry_control(table, index);
 
-		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
+		*cs++ = i915_get_lri_reg(rq->engine,
+					 mocs_register(engine, index));
 		*cs++ = value;
 	}
 
 	/* All remaining entries are also unused */
 	for (; index < table->n_entries; index++) {
-		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
+		*cs++ = i915_get_lri_reg(rq->engine,
+					 mocs_register(engine, index));
 		*cs++ = unused_value;
 	}
 
@@ -447,7 +446,11 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries / 2);
+	/*
+	 * GEN9_LNCFCMOCS is not engine relative, therefore there is no
+	 * need for relative addressing?
+	 */
+	*cs++ = __MI_LOAD_REGISTER_IMM(table->n_entries / 2);
 
 	for (i = 0; i < table->size / 2; i++) {
 		u16 low = get_entry_l3cc(table, 2 * i);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3844581f622c..107ed7c0d1fa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1592,12 +1592,13 @@ static int load_pd_dir(struct i915_request *rq,
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(1);
-	*cs++ = i915_mmio_reg_offset(RING_PP_DIR_DCLV(engine->mmio_base));
+	/* Can these not be merged into a single LRI??? */
+	*cs++ = i915_get_lri_cmd(engine, 1);
+	*cs++ = i915_get_lri_reg(engine, RING_PP_DIR_DCLV(engine->mmio_base));
 	*cs++ = PP_DIR_DCLV_2G;
 
-	*cs++ = MI_LOAD_REGISTER_IMM(1);
-	*cs++ = i915_mmio_reg_offset(RING_PP_DIR_BASE(engine->mmio_base));
+	*cs++ = i915_get_lri_cmd(engine, 1);
+	*cs++ = i915_get_lri_reg(engine, RING_PP_DIR_BASE(engine->mmio_base));
 	*cs++ = ppgtt->pd.base.ggtt_offset << 10;
 
 	intel_ring_advance(rq, cs);
@@ -1662,7 +1663,11 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
 		if (num_engines) {
 			struct intel_engine_cs *signaller;
 
-			*cs++ = MI_LOAD_REGISTER_IMM(num_engines);
+			/*
+			 * Must use absolute engine address as the register
+			 * write is targeting a different engine.
+			 */
+			*cs++ = __MI_LOAD_REGISTER_IMM(num_engines);
 			for_each_engine(signaller, i915, id) {
 				if (signaller == engine)
 					continue;
@@ -1708,7 +1713,11 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
 			struct intel_engine_cs *signaller;
 			i915_reg_t last_reg = {}; /* keep gcc quiet */
 
-			*cs++ = MI_LOAD_REGISTER_IMM(num_engines);
+			/*
+			 * Must use absolute engine address as the register
+			 * write is targeting a different engine.
+			 */
+			*cs++ = __MI_LOAD_REGISTER_IMM(num_engines);
 			for_each_engine(signaller, i915, id) {
 				if (signaller == engine)
 					continue;
@@ -1750,9 +1759,9 @@ static int remap_l3(struct i915_request *rq, int slice)
 	 * here because no other code should access these registers other than
 	 * at initialization time.
 	 */
-	*cs++ = MI_LOAD_REGISTER_IMM(GEN7_L3LOG_SIZE/4);
+	*cs++ = i915_get_lri_cmd(rq->engine, GEN7_L3LOG_SIZE/4);
 	for (i = 0; i < GEN7_L3LOG_SIZE/4; i++) {
-		*cs++ = i915_mmio_reg_offset(GEN7_L3LOG(slice, i));
+		*cs++ = i915_get_lri_reg(rq->engine, GEN7_L3LOG(slice, i));
 		*cs++ = remap_info[i];
 	}
 	*cs++ = MI_NOOP;
@@ -2337,3 +2346,23 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
 
 	return intel_init_ring_buffer(engine);
 }
+
+u32 i915_get_lri_cmd(const struct intel_engine_cs *engine, u32 word_count)
+{
+	u32 word;
+
+	word = __MI_LOAD_REGISTER_IMM(word_count);
+
+	if (i915_engine_has_relative_lri(engine))
+		word |= MI_LRI_ADD_CS_MMIO_START_GEN11;
+
+	return word;
+}
+
+u32 i915_get_lri_reg(const struct intel_engine_cs *engine, i915_reg_t reg)
+{
+	if (!i915_engine_has_relative_lri(engine))
+		return i915_mmio_reg_offset(reg);
+
+	return i915_mmio_reg_offset(reg) - engine->mmio_base;
+}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 72c7c337ace9..261b3c433069 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -580,4 +580,8 @@ intel_engine_get_hangcheck_seqno(struct intel_engine_cs *engine)
 	return intel_read_status_page(engine, I915_GEM_HWS_HANGCHECK);
 }
 
+bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine);
+u32 i915_get_lri_cmd(const struct intel_engine_cs *engine, u32 word_count);
+u32 i915_get_lri_reg(const struct intel_engine_cs *engine, i915_reg_t reg);
+
 #endif /* _INTEL_RINGBUFFER_H_ */
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index b3cbed1ee1c9..a50c47993c88 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -629,9 +629,9 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
 	if (IS_ERR(cs))
 		return PTR_ERR(cs);
 
-	*cs++ = MI_LOAD_REGISTER_IMM(wal->count);
+	*cs++ = i915_get_lri_cmd(rq->engine, wal->count);
 	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
-		*cs++ = i915_mmio_reg_offset(wa->reg);
+		*cs++ = i915_get_lri_reg(rq->engine, wa->reg);
 		*cs++ = wa->val;
 	}
 	*cs++ = MI_NOOP;
diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index a363748a7a4f..dbe3cd4d4981 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -444,6 +444,7 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 
 	for (i = 0; i < engine->whitelist.count; i++) {
 		u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
+		u32 regLRI = i915_get_lri_reg(engine, engine->whitelist.list[i].reg);
 		u64 addr = scratch->node.start;
 		struct i915_request *rq;
 		u32 srm, lrm, rsvd;
@@ -476,8 +477,8 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 		idx = 1;
 		for (v = 0; v < ARRAY_SIZE(values); v++) {
 			/* LRI garbage */
-			*cs++ = MI_LOAD_REGISTER_IMM(1);
-			*cs++ = reg;
+			*cs++ = i915_get_lri_cmd(engine, 1);
+			*cs++ = regLRI;
 			*cs++ = values[v];
 
 			/* SRM result */
@@ -489,8 +490,8 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
 		}
 		for (v = 0; v < ARRAY_SIZE(values); v++) {
 			/* LRI garbage */
-			*cs++ = MI_LOAD_REGISTER_IMM(1);
-			*cs++ = reg;
+			*cs++ = i915_get_lri_cmd(engine, 1);
+			*cs++ = regLRI;
 			*cs++ = ~values[v];
 
 			/* SRM result */
-- 
2.21.0.5.gaeb582a983

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Engine relative MMIO (rev4)
  2019-04-24  1:50 [PATCH] drm/i915: Engine relative MMIO John.C.Harrison
@ 2019-04-24  2:49 ` Patchwork
  2019-04-24  3:56 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-04-24  2:49 UTC (permalink / raw)
  To: John.C.Harrison; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Engine relative MMIO (rev4)
URL   : https://patchwork.freedesktop.org/series/57117/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f8a1f0afe196 drm/i915: Engine relative MMIO
-:92: ERROR:SPACING: space prohibited after that open parenthesis '('
#92: FILE: drivers/gpu/drm/i915/i915_cmd_parser.c:223:
+	CMD(  __MI_LOAD_REGISTER_IMM(1),        SMI,   !F,  0xFF,   W,

-:93: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#93: FILE: drivers/gpu/drm/i915/i915_cmd_parser.c:224:
+	CMD(  __MI_LOAD_REGISTER_IMM(1),        SMI,   !F,  0xFF,   W,
 	      .reg = { .offset = 1, .mask = 0x007FFFFC, .step = 2 }    ),

-:250: CHECK:SPACING: spaces preferred around that '*' (ctx:VxV)
#250: FILE: drivers/gpu/drm/i915/intel_gpu_commands.h:130:
+#define __MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
                                  	                ^

-:250: CHECK:SPACING: spaces preferred around that '-' (ctx:VxV)
#250: FILE: drivers/gpu/drm/i915/intel_gpu_commands.h:130:
+#define __MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
                                  	                    ^

-:252: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#252: FILE: drivers/gpu/drm/i915/intel_gpu_commands.h:132:
+#define   MI_LRI_ADD_CS_MMIO_START_GEN11	(1<<19)
                                         	  ^

-:395: ERROR:CODE_INDENT: code indent should use tabs where possible
#395: FILE: drivers/gpu/drm/i915/intel_lrc.c:2789:
+ ^ICTX_REG(engine, regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);$

-:395: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#395: FILE: drivers/gpu/drm/i915/intel_lrc.c:2789:
+ ^ICTX_REG(engine, regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);$

-:395: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#395: FILE: drivers/gpu/drm/i915/intel_lrc.c:2789:
+ ^ICTX_REG(engine, regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);$

-:539: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#539: FILE: drivers/gpu/drm/i915/intel_ringbuffer.c:1762:
+	*cs++ = i915_get_lri_cmd(rq->engine, GEN7_L3LOG_SIZE/4);
 	                                                    ^

-:607: CHECK:CAMELCASE: Avoid CamelCase: <regLRI>
#607: FILE: drivers/gpu/drm/i915/selftests/intel_workarounds.c:447:
+		u32 regLRI = i915_get_lri_reg(engine, engine->whitelist.list[i].reg);

total: 2 errors, 2 warnings, 6 checks, 503 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Engine relative MMIO (rev4)
  2019-04-24  1:50 [PATCH] drm/i915: Engine relative MMIO John.C.Harrison
  2019-04-24  2:49 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Engine relative MMIO (rev4) Patchwork
@ 2019-04-24  3:56 ` Patchwork
  2019-04-24  6:33 ` ✓ Fi.CI.IGT: " Patchwork
  2019-05-06 21:36 ` [PATCH] drm/i915: Engine relative MMIO Rodrigo Vivi
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-04-24  3:56 UTC (permalink / raw)
  To: John.C.Harrison; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Engine relative MMIO (rev4)
URL   : https://patchwork.freedesktop.org/series/57117/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5979 -> Patchwork_12857
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57117/revisions/4/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12857 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-icl-y:           NOTRUN -> [SKIP][1] ([fdo#109315]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@amdgpu/amd_cs_nop@fork-compute0.html

  * igt@gem_ctx_create@basic-files:
    - fi-gdg-551:         NOTRUN -> [SKIP][2] ([fdo#109271]) +101 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-gdg-551/igt@gem_ctx_create@basic-files.html

  * igt@gem_exec_basic@basic-bsd2:
    - fi-icl-y:           NOTRUN -> [SKIP][3] ([fdo#109276]) +7 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@gem_exec_basic@basic-bsd2.html

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-pnv-d510:        NOTRUN -> [SKIP][4] ([fdo#109271]) +71 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-pnv-d510/igt@gem_exec_basic@readonly-bsd2.html

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-y:           NOTRUN -> [SKIP][5] ([fdo#109289]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@gem_exec_parse@basic-rejected.html

  * igt@kms_busy@basic-flip-c:
    - fi-gdg-551:         NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#109278])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-gdg-551/igt@kms_busy@basic-flip-c.html
    - fi-pnv-d510:        NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#109278])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-pnv-d510/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-icl-y:           NOTRUN -> [SKIP][8] ([fdo#109284]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> [SKIP][9] ([fdo#109285]) +3 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-byt-clapper:     [PASS][10] -> [FAIL][11] ([fdo#103191])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/fi-byt-clapper/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-byt-clapper/igt@kms_pipe_crc_basic@hang-read-crc-pipe-a.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> [SKIP][12] ([fdo#110189]) +3 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-y:           NOTRUN -> [SKIP][13] ([fdo#109294])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-icl-y/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [DMESG-FAIL][14] ([fdo#110235 ]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (47 -> 43)
------------------------------

  Additional (3): fi-icl-y fi-gdg-551 fi-pnv-d510 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_5979 -> Patchwork_12857

  CI_DRM_5979: b9faf41db57725605f502c2074b35eb4266643b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12857: f8a1f0afe196d187b7ddc7894151c8ce423b9fb7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f8a1f0afe196 drm/i915: Engine relative MMIO

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Engine relative MMIO (rev4)
  2019-04-24  1:50 [PATCH] drm/i915: Engine relative MMIO John.C.Harrison
  2019-04-24  2:49 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Engine relative MMIO (rev4) Patchwork
  2019-04-24  3:56 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-04-24  6:33 ` Patchwork
  2019-05-06 21:36 ` [PATCH] drm/i915: Engine relative MMIO Rodrigo Vivi
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-04-24  6:33 UTC (permalink / raw)
  To: John.C.Harrison; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Engine relative MMIO (rev4)
URL   : https://patchwork.freedesktop.org/series/57117/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5979_full -> Patchwork_12857_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12857_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@bsd2:
    - shard-iclb:         NOTRUN -> [SKIP][1] ([fdo#109276]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb4/igt@gem_exec_parallel@bsd2.html

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-skl:          [PASS][2] -> [INCOMPLETE][3] ([fdo#107803] / [fdo#107807])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl9/igt@i915_pm_rpm@gem-execbuf-stress.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl9/igt@i915_pm_rpm@gem-execbuf-stress.html

  * igt@i915_pm_rpm@gem-mmap-cpu:
    - shard-skl:          NOTRUN -> [INCOMPLETE][4] ([fdo#107807])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl3/igt@i915_pm_rpm@gem-mmap-cpu.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-apl7/igt@i915_suspend@sysfs-reader.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl1/igt@i915_suspend@sysfs-reader.html

  * igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
    - shard-apl:          NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#109278]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl4/igt@kms_atomic_transition@3x-modeset-transitions-nonblocking.html

  * igt@kms_busy@extended-modeset-hang-oldfb-render-f:
    - shard-skl:          NOTRUN -> [SKIP][8] ([fdo#109271] / [fdo#109278]) +7 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl5/igt@kms_busy@extended-modeset-hang-oldfb-render-f.html

  * igt@kms_cursor_crc@cursor-512x512-random:
    - shard-iclb:         NOTRUN -> [SKIP][9] ([fdo#109279])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb4/igt@kms_cursor_crc@cursor-512x512-random.html

  * igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge:
    - shard-snb:          NOTRUN -> [SKIP][10] ([fdo#109271] / [fdo#109278]) +6 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-snb2/igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge.html

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         [PASS][11] -> [SKIP][12] ([fdo#109349])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb5/igt@kms_dp_dsc@basic-dsc-enable-edp.html

  * igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108040])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl9/igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl3/igt@kms_frontbuffer_tracking@fbc-1p-indfb-fliptrack.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> [SKIP][15] ([fdo#109271]) +71 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl3/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][16] -> [FAIL][17] ([fdo#103167]) +3 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-snb:          NOTRUN -> [SKIP][18] ([fdo#109271]) +30 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-snb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-move:
    - shard-apl:          NOTRUN -> [SKIP][19] ([fdo#109271]) +24 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl4/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-spr-indfb-draw-render:
    - shard-iclb:         NOTRUN -> [SKIP][20] ([fdo#109280])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-spr-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#103167])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-iclb:         [PASS][23] -> [INCOMPLETE][24] ([fdo#106978] / [fdo#107713])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> [FAIL][25] ([fdo#105456])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl5/igt@kms_panel_fitting@legacy.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-iclb:         [PASS][26] -> [INCOMPLETE][27] ([fdo#107713] / [fdo#110042])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [PASS][28] -> [DMESG-WARN][29] ([fdo#108566]) +4 similar issues
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][30] -> [FAIL][31] ([fdo#108145])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][32] -> [FAIL][33] ([fdo#103166])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          [PASS][34] -> [SKIP][35] ([fdo#109271] / [fdo#109278])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-glk9/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-glk1/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][36] -> [SKIP][37] ([fdo#109441]) +1 similar issue
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb8/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          [PASS][38] -> [DMESG-FAIL][39] ([fdo#105763])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-kbl1/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-kbl5/igt@kms_rotation_crc@multiplane-rotation-cropping-bottom.html

  * igt@perf@short-reads:
    - shard-kbl:          [PASS][40] -> [FAIL][41] ([fdo#103183])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-kbl1/igt@perf@short-reads.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-kbl5/igt@perf@short-reads.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-kbl:          [DMESG-WARN][42] ([fdo#108566]) -> [PASS][43] +4 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-kbl2/igt@gem_ctx_isolation@bcs0-s3.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-kbl3/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-apl:          [DMESG-WARN][44] ([fdo#108566]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-apl2/igt@gem_workarounds@suspend-resume-fd.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl1/igt@gem_workarounds@suspend-resume-fd.html

  * igt@i915_pm_rpm@debugfs-read:
    - shard-skl:          [INCOMPLETE][46] ([fdo#107807]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl1/igt@i915_pm_rpm@debugfs-read.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl5/igt@i915_pm_rpm@debugfs-read.html

  * igt@kms_ccs@pipe-b-crc-primary-basic:
    - shard-apl:          [INCOMPLETE][48] ([fdo#103927]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-apl5/igt@kms_ccs@pipe-b-crc-primary-basic.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl4/igt@kms_ccs@pipe-b-crc-primary-basic.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][50] ([fdo#104108] / [fdo#107773]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl4/igt@kms_fbcon_fbt@psr-suspend.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl2/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [FAIL][52] ([fdo#105363]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl4/igt@kms_flip@flip-vs-expired-vblank.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-apl:          [FAIL][54] ([fdo#103167] / [fdo#110378]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-rte.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl2/igt@kms_frontbuffer_tracking@fbc-1p-rte.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-iclb:         [FAIL][56] ([fdo#103167]) -> [PASS][57] +4 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [FAIL][58] ([fdo#108145] / [fdo#110403]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][60] ([fdo#103166]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [SKIP][62] ([fdo#109441]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][64] ([fdo#99912]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-apl2/igt@kms_setmode@basic.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-apl1/igt@kms_setmode@basic.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [FAIL][66] ([fdo#100047]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_5979/shard-iclb2/igt@kms_sysfs_edid_timing.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/shard-iclb8/igt@kms_sysfs_edid_timing.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103183]: https://bugs.freedesktop.org/show_bug.cgi?id=103183
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106978]: https://bugs.freedesktop.org/show_bug.cgi?id=106978
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110042]: https://bugs.freedesktop.org/show_bug.cgi?id=110042
  [fdo#110378]: https://bugs.freedesktop.org/show_bug.cgi?id=110378
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110499]: https://bugs.freedesktop.org/show_bug.cgi?id=110499
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

  * Linux: CI_DRM_5979 -> Patchwork_12857

  CI_DRM_5979: b9faf41db57725605f502c2074b35eb4266643b9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4959: 504367d33b787de2ba8e007a5b620cfd6f0b3074 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12857: f8a1f0afe196d187b7ddc7894151c8ce423b9fb7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12857/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Engine relative MMIO
  2019-04-24  1:50 [PATCH] drm/i915: Engine relative MMIO John.C.Harrison
                   ` (2 preceding siblings ...)
  2019-04-24  6:33 ` ✓ Fi.CI.IGT: " Patchwork
@ 2019-05-06 21:36 ` Rodrigo Vivi
  2019-05-07 18:55   ` John Harrison
  3 siblings, 1 reply; 7+ messages in thread
From: Rodrigo Vivi @ 2019-05-06 21:36 UTC (permalink / raw)
  To: John.C.Harrison; +Cc: Intel-GFX

On Tue, Apr 23, 2019 at 06:50:13PM -0700, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> With virtual engines, it is no longer possible to know which specific
> physical engine a given request will be executed on at the time that
> request is generated. This means that the request itself must be engine
> agnostic - any direct register writes must be relative to the engine
> and not absolute addresses.
> 
> The LRI command has support for engine relative addressing. However,
> the mechanism is not transparent to the driver. The scheme for Gen11
> (MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
> absolute engine base component. The hardware then adds on the correct
> engine offset at execution time.
> 
> Due to the non-trivial and differing schemes on different hardware, it
> is not possible to simply update the code that creates the LRI
> commands to set a remap flag and let the hardware get on with it.
> Instead, this patch adds function wrappers for generating the LRI
> command itself and then for constructing the correct address to use
> with the LRI.
> 
> v2: Fix build break in GVT. Remove flags parameter [review feedback
> from Chris W].
> 
> v3: Fix build break in selftest. Rebase to newer base tree and fix
> merge conflict.
> 
> v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
> code paths [review feedback from Chris W].

First of all, would you have a rebased version after gt/ ?

So, from my point of view v3 was better than this because this spread
the __MI_LOAD_REGISTER_IMM everywhere.

Maybe I just disagree with Chris and I'd prefer a single place
like v3, but anyway we could probably arrive in an intermediate
solution like: Couldn't we do in a way that we keep the MI_LRI without
'__' and use this new function only on the paths needed?

and maybe name this function gen11_get_lri_cmd? to make it clear
that gen11+ needs to use this path.

> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/mmio_context.c       | 16 +++-
>  drivers/gpu/drm/i915/i915_cmd_parser.c        |  4 +-
>  drivers/gpu/drm/i915/i915_gem_context.c       | 12 +--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c    |  3 +-
>  drivers/gpu/drm/i915/i915_perf.c              | 19 +++--
>  drivers/gpu/drm/i915/intel_engine_cs.c        | 11 +++
>  drivers/gpu/drm/i915/intel_gpu_commands.h     |  6 +-
>  drivers/gpu/drm/i915/intel_lrc.c              | 79 ++++++++++---------
>  drivers/gpu/drm/i915/intel_lrc_reg.h          |  4 +-
>  drivers/gpu/drm/i915/intel_mocs.c             | 17 ++--
>  drivers/gpu/drm/i915/intel_ringbuffer.c       | 45 +++++++++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h       |  4 +
>  drivers/gpu/drm/i915/intel_workarounds.c      |  4 +-
>  .../drm/i915/selftests/intel_workarounds.c    |  9 ++-
>  14 files changed, 154 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
> index e7e14c842be4..1b4d78e55ed6 100644
> --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> @@ -199,14 +199,14 @@ restore_context_mmio_for_inhibit(struct intel_vgpu *vgpu,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(count);
> +	*cs++ = i915_get_lri_cmd(req->engine, count);
>  	for (mmio = gvt->engine_mmio_list.mmio;
>  	     i915_mmio_reg_valid(mmio->reg); mmio++) {
>  		if (mmio->ring_id != ring_id ||
>  		    !mmio->in_context)
>  			continue;
>  
> -		*cs++ = i915_mmio_reg_offset(mmio->reg);
> +		*cs++ = i915_get_lri_reg(req->engine, mmio->reg);
>  		*cs++ = vgpu_vreg_t(vgpu, mmio->reg) |
>  				(mmio->mask << 16);
>  		gvt_dbg_core("add lri reg pair 0x%x:0x%x in inhibit ctx, vgpu:%d, rind_id:%d\n",
> @@ -234,7 +234,11 @@ restore_render_mocs_control_for_inhibit(struct intel_vgpu *vgpu,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE);
> +	/*
> +	 * GEN9_GFX_MOCS is not engine relative, therefore there is no
> +	 * need for relative addressing.
> +	 */
> +	*cs++ = __MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE);
>  
>  	for (index = 0; index < GEN9_MOCS_SIZE; index++) {
>  		*cs++ = i915_mmio_reg_offset(GEN9_GFX_MOCS(index));
> @@ -261,7 +265,11 @@ restore_render_mocs_l3cc_for_inhibit(struct intel_vgpu *vgpu,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE / 2);
> +	/*
> +	 * GEN9_LNCFCMOCS is not engine relative, therefore there is no
> +	 * need for relative addressing.
> +	 */
> +	*cs++ = __MI_LOAD_REGISTER_IMM(GEN9_MOCS_SIZE / 2);
>  
>  	for (index = 0; index < GEN9_MOCS_SIZE / 2; index++) {
>  		*cs++ = i915_mmio_reg_offset(GEN9_LNCFCMOCS(index));
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 503d548a55f7..91ebe18aacc6 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -220,7 +220,7 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = {
>  	CMD(  MI_SUSPEND_FLUSH,                 SMI,    F,  1,      S  ),
>  	CMD(  MI_SEMAPHORE_MBOX,                SMI,   !F,  0xFF,   R  ),
>  	CMD(  MI_STORE_DWORD_INDEX,             SMI,   !F,  0xFF,   R  ),
> -	CMD(  MI_LOAD_REGISTER_IMM(1),          SMI,   !F,  0xFF,   W,
> +	CMD(  __MI_LOAD_REGISTER_IMM(1),        SMI,   !F,  0xFF,   W,
>  	      .reg = { .offset = 1, .mask = 0x007FFFFC, .step = 2 }    ),
>  	CMD(  MI_STORE_REGISTER_MEM,            SMI,    F,  3,     W | B,
>  	      .reg = { .offset = 1, .mask = 0x007FFFFC },
> @@ -1182,7 +1182,7 @@ static bool check_cmd(const struct intel_engine_cs *engine,
>  					return false;
>  				}
>  
> -				if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1) &&
> +				if (desc->cmd.value == __MI_LOAD_REGISTER_IMM(1) &&
>  				    (offset + 2 > length ||
>  				     (cmd[offset + 1] & reg->mask) != reg->value)) {
>  					DRM_DEBUG_DRIVER("CMD: Rejected LRI to masked register 0x%08X\n",
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index dd728b26b5aa..f25dc613c266 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -1039,11 +1039,11 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data)
>  		if (IS_ERR(cs))
>  			return PTR_ERR(cs);
>  
> -		*cs++ = MI_LOAD_REGISTER_IMM(2);
> +		*cs++ = i915_get_lri_cmd(engine, 2);
>  
> -		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 0));
> +		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, 0));
>  		*cs++ = upper_32_bits(pd_daddr);
> -		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 0));
> +		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, 0));
>  		*cs++ = lower_32_bits(pd_daddr);
>  
>  		*cs++ = MI_NOOP;
> @@ -1053,13 +1053,13 @@ static int emit_ppgtt_update(struct i915_request *rq, void *data)
>  		if (IS_ERR(cs))
>  			return PTR_ERR(cs);
>  
> -		*cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES);
> +		*cs++ = i915_get_lri_cmd(engine, 2 * GEN8_3LVL_PDPES);
>  		for (i = GEN8_3LVL_PDPES; i--; ) {
>  			const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
>  
> -			*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i));
> +			*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, i));
>  			*cs++ = upper_32_bits(pd_daddr);
> -			*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i));
> +			*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, i));
>  			*cs++ = lower_32_bits(pd_daddr);
>  		}
>  		*cs++ = MI_NOOP;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 3d672c9edb94..983801f481ba 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1965,7 +1965,8 @@ static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(4);
> +	/* Gen7 only so no need to support relative offsets */
> +	*cs++ = __MI_LOAD_REGISTER_IMM(4);
>  	for (i = 0; i < 4; i++) {
>  		*cs++ = i915_mmio_reg_offset(GEN7_SO_WRITE_OFFSET(i));
>  		*cs++ = 0;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 39a4804091d7..10d5ab991908 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1628,7 +1628,8 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv)
>   * in the case that the OA unit has been disabled.
>   */
>  static void
> -gen8_update_reg_state_unlocked(struct intel_context *ce,
> +gen8_update_reg_state_unlocked(struct intel_engine_cs *engine,
> +			       struct intel_context *ce,
>  			       u32 *reg_state,
>  			       const struct i915_oa_config *oa_config)
>  {
> @@ -1647,7 +1648,12 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
>  	};
>  	int i;
>  
> -	CTX_REG(reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
> +	/*
> +	 * NB: The LRI instruction is generated by the hardware.
> +	 * Should we read it in and assert that the offset flag is set?
> +	 */
> +
> +	CTX_REG(engine, reg_state, ctx_oactxctrl, GEN8_OACTXCONTROL,
>  		(i915->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) |
>  		(i915->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) |
>  		GEN8_OA_COUNTER_RESUME);
> @@ -1674,10 +1680,10 @@ gen8_update_reg_state_unlocked(struct intel_context *ce,
>  			}
>  		}
>  
> -		CTX_REG(reg_state, state_offset, flex_regs[i], value);
> +		CTX_REG(engine, reg_state, state_offset, flex_regs[i], value);
>  	}
>  
> -	CTX_REG(reg_state,
> +	CTX_REG(engine, reg_state,
>  		CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
>  		gen8_make_rpcs(i915, &ce->sseu));
>  }
> @@ -1752,7 +1758,7 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
>  		ce->state->obj->mm.dirty = true;
>  		regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
>  
> -		gen8_update_reg_state_unlocked(ce, regs, oa_config);
> +		gen8_update_reg_state_unlocked(engine, ce, regs, oa_config);
>  
>  		i915_gem_object_unpin_map(ce->state->obj);
>  	}
> @@ -2146,7 +2152,8 @@ void i915_oa_init_reg_state(struct intel_engine_cs *engine,
>  
>  	stream = engine->i915->perf.oa.exclusive_stream;
>  	if (stream)
> -		gen8_update_reg_state_unlocked(ce, regs, stream->oa_config);
> +		gen8_update_reg_state_unlocked(engine, ce, regs,
> +					       stream->oa_config);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index eea9bec04f1b..ee33ce265820 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -246,6 +246,17 @@ static u32 __engine_mmio_base(struct drm_i915_private *i915,
>  	return bases[i].base;
>  }
>  
> +bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine)
> +{
> +	if (INTEL_GEN(engine->i915) < 11)
> +		return false;
> +
> +	if (engine->id == BCS0)
> +		return false;
> +
> +	return true;
> +}
> +
>  static void __sprint_engine_name(char *name, const struct engine_info *info)
>  {
>  	WARN_ON(snprintf(name, INTEL_ENGINE_CS_MAX_NAME, "%s%u",
> diff --git a/drivers/gpu/drm/i915/intel_gpu_commands.h b/drivers/gpu/drm/i915/intel_gpu_commands.h
> index a34ece53a771..e7784b3fb759 100644
> --- a/drivers/gpu/drm/i915/intel_gpu_commands.h
> +++ b/drivers/gpu/drm/i915/intel_gpu_commands.h
> @@ -123,9 +123,13 @@
>   *   simply ignores the register load under certain conditions.
>   * - One can actually load arbitrary many arbitrary registers: Simply issue x
>   *   address/value pairs. Don't overdue it, though, x <= 2^4 must hold!
> + * - Newer hardware supports engine relative addresses but older hardware does
> + *   not. So never call MI_LRI directly, always use the i915_get_lri_cmd()
> + *   and i915_get_lri_reg() helper functions.
>   */
> -#define MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
> +#define __MI_LOAD_REGISTER_IMM(x)	MI_INSTR(0x22, 2*(x)-1)
>  #define   MI_LRI_FORCE_POSTED		(1<<12)
> +#define   MI_LRI_ADD_CS_MMIO_START_GEN11	(1<<19)
>  #define MI_STORE_REGISTER_MEM        MI_INSTR(0x24, 1)
>  #define MI_STORE_REGISTER_MEM_GEN8   MI_INSTR(0x24, 2)
>  #define   MI_SRM_LRM_GLOBAL_GTT		(1<<22)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 4e0a351bfbca..41cbbcd9f0dd 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1383,14 +1383,15 @@ static int emit_pdps(struct i915_request *rq)
>  		return PTR_ERR(cs);
>  
>  	/* Ensure the LRI have landed before we invalidate & continue */
> -	*cs++ = MI_LOAD_REGISTER_IMM(2 * GEN8_3LVL_PDPES) | MI_LRI_FORCE_POSTED;
> +	*cs++ = i915_get_lri_cmd(engine, 2 * GEN8_3LVL_PDPES) |
> +				 MI_LRI_FORCE_POSTED;
>  	for (i = GEN8_3LVL_PDPES; i--; ) {
>  		const dma_addr_t pd_daddr = i915_page_dir_dma_addr(ppgtt, i);
>  		u32 base = engine->mmio_base;
>  
> -		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, i));
> +		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_UDW(base, i));
>  		*cs++ = upper_32_bits(pd_daddr);
> -		*cs++ = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, i));
> +		*cs++ = i915_get_lri_reg(engine, GEN8_RING_PDP_LDW(base, i));
>  		*cs++ = lower_32_bits(pd_daddr);
>  	}
>  	*cs++ = MI_NOOP;
> @@ -1464,7 +1465,8 @@ gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine, u32 *batch)
>  	*batch++ = i915_scratch_offset(engine->i915) + 256;
>  	*batch++ = 0;
>  
> -	*batch++ = MI_LOAD_REGISTER_IMM(1);
> +	/* Gen8/9 only so no need to support relative offsets */
> +	*batch++ = __MI_LOAD_REGISTER_IMM(1);
>  	*batch++ = i915_mmio_reg_offset(GEN8_L3SQCREG4);
>  	*batch++ = 0x40400000 | GEN8_LQSC_FLUSH_COHERENT_LINES;
>  
> @@ -1535,13 +1537,14 @@ struct lri {
>  	u32 value;
>  };
>  
> -static u32 *emit_lri(u32 *batch, const struct lri *lri, unsigned int count)
> +static u32 *emit_lri(struct intel_engine_cs *engine, u32 *batch,
> +		     const struct lri *lri, unsigned int count)
>  {
>  	GEM_BUG_ON(!count || count > 63);
>  
> -	*batch++ = MI_LOAD_REGISTER_IMM(count);
> +	*batch++ = i915_get_lri_cmd(engine, count);
>  	do {
> -		*batch++ = i915_mmio_reg_offset(lri->reg);
> +		*batch++ = i915_get_lri_reg(engine, lri->reg);
>  		*batch++ = lri->value;
>  	} while (lri++, --count);
>  	*batch++ = MI_NOOP;
> @@ -1579,7 +1582,7 @@ static u32 *gen9_init_indirectctx_bb(struct intel_engine_cs *engine, u32 *batch)
>  	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt,glk */
>  	batch = gen8_emit_flush_coherentl3_wa(engine, batch);
>  
> -	batch = emit_lri(batch, lri, ARRAY_SIZE(lri));
> +	batch = emit_lri(engine, batch, lri, ARRAY_SIZE(lri));
>  
>  	/* WaMediaPoolStateCmdInWABB:bxt,glk */
>  	if (HAS_POOLED_EU(engine->i915)) {
> @@ -2728,10 +2731,10 @@ static void execlists_init_reg_state(u32 *regs,
>  	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
>  	 * we are not initializing here).
>  	 */
> -	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
> -				 MI_LRI_FORCE_POSTED;
> +	regs[CTX_LRI_HEADER_0] = i915_get_lri_cmd(engine, rcs ? 14 : 11) |
> +						  MI_LRI_FORCE_POSTED;
>  
> -	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
> +	CTX_REG(engine, regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
>  		_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
>  		_MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH));
>  	if (INTEL_GEN(engine->i915) < 11) {
> @@ -2739,22 +2742,23 @@ static void execlists_init_reg_state(u32 *regs,
>  			_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT |
>  					    CTX_CTRL_RS_CTX_ENABLE);
>  	}
> -	CTX_REG(regs, CTX_RING_HEAD, RING_HEAD(base), 0);
> -	CTX_REG(regs, CTX_RING_TAIL, RING_TAIL(base), 0);
> -	CTX_REG(regs, CTX_RING_BUFFER_START, RING_START(base), 0);
> -	CTX_REG(regs, CTX_RING_BUFFER_CONTROL, RING_CTL(base),
> +	CTX_REG(engine, regs, CTX_RING_HEAD, RING_HEAD(base), 0);
> +	CTX_REG(engine, regs, CTX_RING_TAIL, RING_TAIL(base), 0);
> +	CTX_REG(engine, regs, CTX_RING_BUFFER_START, RING_START(base), 0);
> +	CTX_REG(engine, regs, CTX_RING_BUFFER_CONTROL, RING_CTL(base),
>  		RING_CTL_SIZE(ring->size) | RING_VALID);
> -	CTX_REG(regs, CTX_BB_HEAD_U, RING_BBADDR_UDW(base), 0);
> -	CTX_REG(regs, CTX_BB_HEAD_L, RING_BBADDR(base), 0);
> -	CTX_REG(regs, CTX_BB_STATE, RING_BBSTATE(base), RING_BB_PPGTT);
> -	CTX_REG(regs, CTX_SECOND_BB_HEAD_U, RING_SBBADDR_UDW(base), 0);
> -	CTX_REG(regs, CTX_SECOND_BB_HEAD_L, RING_SBBADDR(base), 0);
> -	CTX_REG(regs, CTX_SECOND_BB_STATE, RING_SBBSTATE(base), 0);
> +	CTX_REG(engine, regs, CTX_BB_HEAD_U, RING_BBADDR_UDW(base), 0);
> +	CTX_REG(engine, regs, CTX_BB_HEAD_L, RING_BBADDR(base), 0);
> +	CTX_REG(engine, regs, CTX_BB_STATE, RING_BBSTATE(base), RING_BB_PPGTT);
> +	CTX_REG(engine, regs, CTX_SECOND_BB_HEAD_U, RING_SBBADDR_UDW(base), 0);
> +	CTX_REG(engine, regs, CTX_SECOND_BB_HEAD_L, RING_SBBADDR(base), 0);
> +	CTX_REG(engine, regs, CTX_SECOND_BB_STATE, RING_SBBSTATE(base), 0);
>  	if (rcs) {
>  		struct i915_ctx_workarounds *wa_ctx = &engine->wa_ctx;
>  
> -		CTX_REG(regs, CTX_RCS_INDIRECT_CTX, RING_INDIRECT_CTX(base), 0);
> -		CTX_REG(regs, CTX_RCS_INDIRECT_CTX_OFFSET,
> +		CTX_REG(engine, regs, CTX_RCS_INDIRECT_CTX,
> +			RING_INDIRECT_CTX(base), 0);
> +		CTX_REG(engine, regs, CTX_RCS_INDIRECT_CTX_OFFSET,
>  			RING_INDIRECT_CTX_OFFSET(base), 0);
>  		if (wa_ctx->indirect_ctx.size) {
>  			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
> @@ -2767,7 +2771,8 @@ static void execlists_init_reg_state(u32 *regs,
>  				intel_lr_indirect_ctx_offset(engine) << 6;
>  		}
>  
> -		CTX_REG(regs, CTX_BB_PER_CTX_PTR, RING_BB_PER_CTX_PTR(base), 0);
> +		CTX_REG(engine, regs, CTX_BB_PER_CTX_PTR,
> +			RING_BB_PER_CTX_PTR(base), 0);
>  		if (wa_ctx->per_ctx.size) {
>  			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>  
> @@ -2776,18 +2781,19 @@ static void execlists_init_reg_state(u32 *regs,
>  		}
>  	}
>  
> -	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
> +	regs[CTX_LRI_HEADER_1] = i915_get_lri_cmd(engine, 9) |
> +						  MI_LRI_FORCE_POSTED;
>  
> -	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
> +	CTX_REG(engine, regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
>  	/* PDP values well be assigned later if needed */
> -	CTX_REG(regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);
> -	CTX_REG(regs, CTX_PDP3_LDW, GEN8_RING_PDP_LDW(base, 3), 0);
> -	CTX_REG(regs, CTX_PDP2_UDW, GEN8_RING_PDP_UDW(base, 2), 0);
> -	CTX_REG(regs, CTX_PDP2_LDW, GEN8_RING_PDP_LDW(base, 2), 0);
> -	CTX_REG(regs, CTX_PDP1_UDW, GEN8_RING_PDP_UDW(base, 1), 0);
> -	CTX_REG(regs, CTX_PDP1_LDW, GEN8_RING_PDP_LDW(base, 1), 0);
> -	CTX_REG(regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(base, 0), 0);
> -	CTX_REG(regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(base, 0), 0);
> + 	CTX_REG(engine, regs, CTX_PDP3_UDW, GEN8_RING_PDP_UDW(base, 3), 0);
> +	CTX_REG(engine, regs, CTX_PDP3_LDW, GEN8_RING_PDP_LDW(base, 3), 0);
> +	CTX_REG(engine, regs, CTX_PDP2_UDW, GEN8_RING_PDP_UDW(base, 2), 0);
> +	CTX_REG(engine, regs, CTX_PDP2_LDW, GEN8_RING_PDP_LDW(base, 2), 0);
> +	CTX_REG(engine, regs, CTX_PDP1_UDW, GEN8_RING_PDP_UDW(base, 1), 0);
> +	CTX_REG(engine, regs, CTX_PDP1_LDW, GEN8_RING_PDP_LDW(base, 1), 0);
> +	CTX_REG(engine, regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(base, 0), 0);
> +	CTX_REG(engine, regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(base, 0), 0);
>  
>  	if (i915_vm_is_4lvl(&ppgtt->vm)) {
>  		/* 64b PPGTT (48bit canonical)
> @@ -2803,8 +2809,9 @@ static void execlists_init_reg_state(u32 *regs,
>  	}
>  
>  	if (rcs) {
> -		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
> -		CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, 0);
> +		regs[CTX_LRI_HEADER_2] = i915_get_lri_cmd(engine, 1);
> +		CTX_REG(engine, regs, CTX_R_PWR_CLK_STATE,
> +			GEN8_R_PWR_CLK_STATE, 0);
>  
>  		i915_oa_init_reg_state(engine, ce, regs);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_lrc_reg.h b/drivers/gpu/drm/i915/intel_lrc_reg.h
> index 5ef932d810a7..40b1142d0d74 100644
> --- a/drivers/gpu/drm/i915/intel_lrc_reg.h
> +++ b/drivers/gpu/drm/i915/intel_lrc_reg.h
> @@ -39,10 +39,10 @@
>  #define CTX_R_PWR_CLK_STATE		0x42
>  #define CTX_END				0x44
>  
> -#define CTX_REG(reg_state, pos, reg, val) do { \
> +#define CTX_REG(engine, reg_state, pos, reg, val) do { \
>  	u32 *reg_state__ = (reg_state); \
>  	const u32 pos__ = (pos); \
> -	(reg_state__)[(pos__) + 0] = i915_mmio_reg_offset(reg); \
> +	(reg_state__)[(pos__) + 0] = i915_get_lri_reg((engine), (reg)); \
>  	(reg_state__)[(pos__) + 1] = (val); \
>  } while (0)
>  
> diff --git a/drivers/gpu/drm/i915/intel_mocs.c b/drivers/gpu/drm/i915/intel_mocs.c
> index 274ba78500c0..bb11d0f68bba 100644
> --- a/drivers/gpu/drm/i915/intel_mocs.c
> +++ b/drivers/gpu/drm/i915/intel_mocs.c
> @@ -322,9 +322,6 @@ static u32 get_entry_control(const struct drm_i915_mocs_table *table,
>  /**
>   * intel_mocs_init_engine() - emit the mocs control table
>   * @engine:	The engine for whom to emit the registers.
> - *
> - * This function simply emits a MI_LOAD_REGISTER_IMM command for the
> - * given table starting at the given address.
>   */
>  void intel_mocs_init_engine(struct intel_engine_cs *engine)
>  {
> @@ -378,18 +375,20 @@ static int emit_mocs_control_table(struct i915_request *rq,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries);
> +	*cs++ = i915_get_lri_cmd(rq->engine, table->n_entries);
>  
>  	for (index = 0; index < table->size; index++) {
>  		u32 value = get_entry_control(table, index);
>  
> -		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
> +		*cs++ = i915_get_lri_reg(rq->engine,
> +					 mocs_register(engine, index));
>  		*cs++ = value;
>  	}
>  
>  	/* All remaining entries are also unused */
>  	for (; index < table->n_entries; index++) {
> -		*cs++ = i915_mmio_reg_offset(mocs_register(engine, index));
> +		*cs++ = i915_get_lri_reg(rq->engine,
> +					 mocs_register(engine, index));
>  		*cs++ = unused_value;
>  	}
>  
> @@ -447,7 +446,11 @@ static int emit_mocs_l3cc_table(struct i915_request *rq,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(table->n_entries / 2);
> +	/*
> +	 * GEN9_LNCFCMOCS is not engine relative, therefore there is no
> +	 * need for relative addressing?
> +	 */
> +	*cs++ = __MI_LOAD_REGISTER_IMM(table->n_entries / 2);
>  
>  	for (i = 0; i < table->size / 2; i++) {
>  		u16 low = get_entry_l3cc(table, 2 * i);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 3844581f622c..107ed7c0d1fa 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1592,12 +1592,13 @@ static int load_pd_dir(struct i915_request *rq,
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(1);
> -	*cs++ = i915_mmio_reg_offset(RING_PP_DIR_DCLV(engine->mmio_base));
> +	/* Can these not be merged into a single LRI??? */
> +	*cs++ = i915_get_lri_cmd(engine, 1);
> +	*cs++ = i915_get_lri_reg(engine, RING_PP_DIR_DCLV(engine->mmio_base));
>  	*cs++ = PP_DIR_DCLV_2G;
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(1);
> -	*cs++ = i915_mmio_reg_offset(RING_PP_DIR_BASE(engine->mmio_base));
> +	*cs++ = i915_get_lri_cmd(engine, 1);
> +	*cs++ = i915_get_lri_reg(engine, RING_PP_DIR_BASE(engine->mmio_base));
>  	*cs++ = ppgtt->pd.base.ggtt_offset << 10;
>  
>  	intel_ring_advance(rq, cs);
> @@ -1662,7 +1663,11 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
>  		if (num_engines) {
>  			struct intel_engine_cs *signaller;
>  
> -			*cs++ = MI_LOAD_REGISTER_IMM(num_engines);
> +			/*
> +			 * Must use absolute engine address as the register
> +			 * write is targeting a different engine.
> +			 */
> +			*cs++ = __MI_LOAD_REGISTER_IMM(num_engines);
>  			for_each_engine(signaller, i915, id) {
>  				if (signaller == engine)
>  					continue;
> @@ -1708,7 +1713,11 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
>  			struct intel_engine_cs *signaller;
>  			i915_reg_t last_reg = {}; /* keep gcc quiet */
>  
> -			*cs++ = MI_LOAD_REGISTER_IMM(num_engines);
> +			/*
> +			 * Must use absolute engine address as the register
> +			 * write is targeting a different engine.
> +			 */
> +			*cs++ = __MI_LOAD_REGISTER_IMM(num_engines);
>  			for_each_engine(signaller, i915, id) {
>  				if (signaller == engine)
>  					continue;
> @@ -1750,9 +1759,9 @@ static int remap_l3(struct i915_request *rq, int slice)
>  	 * here because no other code should access these registers other than
>  	 * at initialization time.
>  	 */
> -	*cs++ = MI_LOAD_REGISTER_IMM(GEN7_L3LOG_SIZE/4);
> +	*cs++ = i915_get_lri_cmd(rq->engine, GEN7_L3LOG_SIZE/4);
>  	for (i = 0; i < GEN7_L3LOG_SIZE/4; i++) {
> -		*cs++ = i915_mmio_reg_offset(GEN7_L3LOG(slice, i));
> +		*cs++ = i915_get_lri_reg(rq->engine, GEN7_L3LOG(slice, i));
>  		*cs++ = remap_info[i];
>  	}
>  	*cs++ = MI_NOOP;
> @@ -2337,3 +2346,23 @@ int intel_init_vebox_ring_buffer(struct intel_engine_cs *engine)
>  
>  	return intel_init_ring_buffer(engine);
>  }
> +
> +u32 i915_get_lri_cmd(const struct intel_engine_cs *engine, u32 word_count)
> +{
> +	u32 word;
> +
> +	word = __MI_LOAD_REGISTER_IMM(word_count);
> +
> +	if (i915_engine_has_relative_lri(engine))
> +		word |= MI_LRI_ADD_CS_MMIO_START_GEN11;
> +
> +	return word;
> +}
> +
> +u32 i915_get_lri_reg(const struct intel_engine_cs *engine, i915_reg_t reg)
> +{
> +	if (!i915_engine_has_relative_lri(engine))
> +		return i915_mmio_reg_offset(reg);
> +
> +	return i915_mmio_reg_offset(reg) - engine->mmio_base;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 72c7c337ace9..261b3c433069 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -580,4 +580,8 @@ intel_engine_get_hangcheck_seqno(struct intel_engine_cs *engine)
>  	return intel_read_status_page(engine, I915_GEM_HWS_HANGCHECK);
>  }
>  
> +bool i915_engine_has_relative_lri(const struct intel_engine_cs *engine);
> +u32 i915_get_lri_cmd(const struct intel_engine_cs *engine, u32 word_count);
> +u32 i915_get_lri_reg(const struct intel_engine_cs *engine, i915_reg_t reg);
> +
>  #endif /* _INTEL_RINGBUFFER_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
> index b3cbed1ee1c9..a50c47993c88 100644
> --- a/drivers/gpu/drm/i915/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/intel_workarounds.c
> @@ -629,9 +629,9 @@ int intel_engine_emit_ctx_wa(struct i915_request *rq)
>  	if (IS_ERR(cs))
>  		return PTR_ERR(cs);
>  
> -	*cs++ = MI_LOAD_REGISTER_IMM(wal->count);
> +	*cs++ = i915_get_lri_cmd(rq->engine, wal->count);
>  	for (i = 0, wa = wal->list; i < wal->count; i++, wa++) {
> -		*cs++ = i915_mmio_reg_offset(wa->reg);
> +		*cs++ = i915_get_lri_reg(rq->engine, wa->reg);
>  		*cs++ = wa->val;
>  	}
>  	*cs++ = MI_NOOP;
> diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> index a363748a7a4f..dbe3cd4d4981 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
> @@ -444,6 +444,7 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>  
>  	for (i = 0; i < engine->whitelist.count; i++) {
>  		u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
> +		u32 regLRI = i915_get_lri_reg(engine, engine->whitelist.list[i].reg);
>  		u64 addr = scratch->node.start;
>  		struct i915_request *rq;
>  		u32 srm, lrm, rsvd;
> @@ -476,8 +477,8 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>  		idx = 1;
>  		for (v = 0; v < ARRAY_SIZE(values); v++) {
>  			/* LRI garbage */
> -			*cs++ = MI_LOAD_REGISTER_IMM(1);
> -			*cs++ = reg;
> +			*cs++ = i915_get_lri_cmd(engine, 1);
> +			*cs++ = regLRI;
>  			*cs++ = values[v];
>  
>  			/* SRM result */
> @@ -489,8 +490,8 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>  		}
>  		for (v = 0; v < ARRAY_SIZE(values); v++) {
>  			/* LRI garbage */
> -			*cs++ = MI_LOAD_REGISTER_IMM(1);
> -			*cs++ = reg;
> +			*cs++ = i915_get_lri_cmd(engine, 1);
> +			*cs++ = regLRI;
>  			*cs++ = ~values[v];
>  
>  			/* SRM result */
> -- 
> 2.21.0.5.gaeb582a983
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Engine relative MMIO
  2019-05-06 21:36 ` [PATCH] drm/i915: Engine relative MMIO Rodrigo Vivi
@ 2019-05-07 18:55   ` John Harrison
  2019-05-08  6:06     ` Rodrigo Vivi
  0 siblings, 1 reply; 7+ messages in thread
From: John Harrison @ 2019-05-07 18:55 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Intel-GFX

On 5/6/2019 14:36, Rodrigo Vivi wrote:
> On Tue, Apr 23, 2019 at 06:50:13PM -0700, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> With virtual engines, it is no longer possible to know which specific
>> physical engine a given request will be executed on at the time that
>> request is generated. This means that the request itself must be engine
>> agnostic - any direct register writes must be relative to the engine
>> and not absolute addresses.
>>
>> The LRI command has support for engine relative addressing. However,
>> the mechanism is not transparent to the driver. The scheme for Gen11
>> (MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
>> absolute engine base component. The hardware then adds on the correct
>> engine offset at execution time.
>>
>> Due to the non-trivial and differing schemes on different hardware, it
>> is not possible to simply update the code that creates the LRI
>> commands to set a remap flag and let the hardware get on with it.
>> Instead, this patch adds function wrappers for generating the LRI
>> command itself and then for constructing the correct address to use
>> with the LRI.
>>
>> v2: Fix build break in GVT. Remove flags parameter [review feedback
>> from Chris W].
>>
>> v3: Fix build break in selftest. Rebase to newer base tree and fix
>> merge conflict.
>>
>> v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
>> code paths [review feedback from Chris W].
> First of all, would you have a rebased version after gt/ ?
I have just done the rebase. Was planning to resend shortly. Although if 
there is more discussion about the best direction to take then I would 
rather hold off posting until a consensus is reached.


> So, from my point of view v3 was better than this because this spread
> the __MI_LOAD_REGISTER_IMM everywhere.
>
> Maybe I just disagree with Chris and I'd prefer a single place
> like v3, but anyway we could probably arrive in an intermediate
> solution like: Couldn't we do in a way that we keep the MI_LRI without
> '__' and use this new function only on the paths needed?
>
> and maybe name this function gen11_get_lri_cmd? to make it clear
> that gen11+ needs to use this path.

The intention was to make it clear that no new code should be directly 
writing MI_LRI. Everything should go through the helper function. Hence 
renaming to add the '__' to make it obvious. Otherwise, someone might 
use the old one by accident and we won't notice until some random and 
hard to track down failure related to virtual engines.

Not sure I would say that the __MI_LRI  is spreading 'everywhere'. There 
are only 8 instances versus double that of the get_lri_cmd version. Note 
also that it is not only Gen11+ specific paths. There are multiple 
places that are gen agnostic. So, unless you want to split those into 
pre/post Gen11 versions as well, you would end up with Gen7 calling a 
Gen11 labelled function.

John.

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Engine relative MMIO
  2019-05-07 18:55   ` John Harrison
@ 2019-05-08  6:06     ` Rodrigo Vivi
  0 siblings, 0 replies; 7+ messages in thread
From: Rodrigo Vivi @ 2019-05-08  6:06 UTC (permalink / raw)
  To: John Harrison; +Cc: Intel-GFX

On Tue, May 07, 2019 at 11:55:11AM -0700, John Harrison wrote:
> On 5/6/2019 14:36, Rodrigo Vivi wrote:
> > On Tue, Apr 23, 2019 at 06:50:13PM -0700, John.C.Harrison@Intel.com wrote:
> > > From: John Harrison <John.C.Harrison@Intel.com>
> > > 
> > > With virtual engines, it is no longer possible to know which specific
> > > physical engine a given request will be executed on at the time that
> > > request is generated. This means that the request itself must be engine
> > > agnostic - any direct register writes must be relative to the engine
> > > and not absolute addresses.
> > > 
> > > The LRI command has support for engine relative addressing. However,
> > > the mechanism is not transparent to the driver. The scheme for Gen11
> > > (MI_LRI_ADD_CS_MMIO_START) requires the LRI address to have no
> > > absolute engine base component. The hardware then adds on the correct
> > > engine offset at execution time.
> > > 
> > > Due to the non-trivial and differing schemes on different hardware, it
> > > is not possible to simply update the code that creates the LRI
> > > commands to set a remap flag and let the hardware get on with it.
> > > Instead, this patch adds function wrappers for generating the LRI
> > > command itself and then for constructing the correct address to use
> > > with the LRI.
> > > 
> > > v2: Fix build break in GVT. Remove flags parameter [review feedback
> > > from Chris W].
> > > 
> > > v3: Fix build break in selftest. Rebase to newer base tree and fix
> > > merge conflict.
> > > 
> > > v4: More rebasing. Rmoved relative addressing support from Gen7-9 only
> > > code paths [review feedback from Chris W].
> > First of all, would you have a rebased version after gt/ ?
> I have just done the rebase. Was planning to resend shortly. Although if
> there is more discussion about the best direction to take then I would
> rather hold off posting until a consensus is reached.
> 
> 
> > So, from my point of view v3 was better than this because this spread
> > the __MI_LOAD_REGISTER_IMM everywhere.
> > 
> > Maybe I just disagree with Chris and I'd prefer a single place
> > like v3, but anyway we could probably arrive in an intermediate
> > solution like: Couldn't we do in a way that we keep the MI_LRI without
> > '__' and use this new function only on the paths needed?
> > 
> > and maybe name this function gen11_get_lri_cmd? to make it clear
> > that gen11+ needs to use this path.
> 
> The intention was to make it clear that no new code should be directly
> writing MI_LRI. Everything should go through the helper function. Hence
> renaming to add the '__' to make it obvious. Otherwise, someone might use
> the old one by accident and we won't notice until some random and hard to
> track down failure related to virtual engines.
> 
> Not sure I would say that the __MI_LRI  is spreading 'everywhere'. There are
> only 8 instances versus double that of the get_lri_cmd version. Note also
> that it is not only Gen11+ specific paths. There are multiple places that
> are gen agnostic. So, unless you want to split those into pre/post Gen11
> versions as well, you would end up with Gen7 calling a Gen11 labelled
> function.

makes sense. Although I prefer the use of v3 with __MI_LRI only used inside
the function, it seems I'm the only one... let's move with v4 then...


> 
> John.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-05-08  6:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-24  1:50 [PATCH] drm/i915: Engine relative MMIO John.C.Harrison
2019-04-24  2:49 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Engine relative MMIO (rev4) Patchwork
2019-04-24  3:56 ` ✓ Fi.CI.BAT: success " Patchwork
2019-04-24  6:33 ` ✓ Fi.CI.IGT: " Patchwork
2019-05-06 21:36 ` [PATCH] drm/i915: Engine relative MMIO Rodrigo Vivi
2019-05-07 18:55   ` John Harrison
2019-05-08  6:06     ` Rodrigo Vivi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.