All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/tgl: Register state context definition for Gen12
@ 2019-09-05 11:34 Mika Kuoppala
  2019-09-05 13:40 ` Andi Shyti
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Mika Kuoppala @ 2019-09-05 11:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Michel Thierry, Lucas De Marchi

From: Michel Thierry <michel.thierry@intel.com>

Gen12 has subtle changes in the reg state context offsets (some fields
are gone, some are in a different location), compared to previous Gens.

The simplest approach seems to be keeping Gen12 (and future platform)
changes apart from the previous gens, while keeping the registers that
are contiguous in functions we can reuse.

v2: alias, virtual engine, rpcs, prune unused regs (Mika)

Bspec: 46255
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c     | 194 +++++++++++++++++-------
 drivers/gpu/drm/i915/gt/intel_lrc.h     |   2 +
 drivers/gpu/drm/i915/gt/intel_lrc_reg.h |   7 +-
 3 files changed, 146 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 87b7473a6dfb..f44c78a82915 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -810,6 +810,7 @@ static void virtual_update_register_offsets(u32 *regs,
 
 	/* Must match execlists_init_reg_state()! */
 
+	/* Common part */
 	regs[CTX_CONTEXT_CONTROL] =
 		i915_mmio_reg_offset(RING_CONTEXT_CONTROL(base));
 	regs[CTX_RING_HEAD] = i915_mmio_reg_offset(RING_HEAD(base));
@@ -820,13 +821,16 @@ static void virtual_update_register_offsets(u32 *regs,
 	regs[CTX_BB_HEAD_U] = i915_mmio_reg_offset(RING_BBADDR_UDW(base));
 	regs[CTX_BB_HEAD_L] = i915_mmio_reg_offset(RING_BBADDR(base));
 	regs[CTX_BB_STATE] = i915_mmio_reg_offset(RING_BBSTATE(base));
+
 	regs[CTX_SECOND_BB_HEAD_U] =
 		i915_mmio_reg_offset(RING_SBBADDR_UDW(base));
 	regs[CTX_SECOND_BB_HEAD_L] = i915_mmio_reg_offset(RING_SBBADDR(base));
 	regs[CTX_SECOND_BB_STATE] = i915_mmio_reg_offset(RING_SBBSTATE(base));
 
+	/* PPGTT part */
 	regs[CTX_CTX_TIMESTAMP] =
 		i915_mmio_reg_offset(RING_CTX_TIMESTAMP(base));
+
 	regs[CTX_PDP3_UDW] = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 3));
 	regs[CTX_PDP3_LDW] = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 3));
 	regs[CTX_PDP2_UDW] = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 2));
@@ -844,8 +848,12 @@ static void virtual_update_register_offsets(u32 *regs,
 		regs[CTX_BB_PER_CTX_PTR] =
 			i915_mmio_reg_offset(RING_BB_PER_CTX_PTR(base));
 
-		regs[CTX_R_PWR_CLK_STATE] =
-			i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
+		if (INTEL_GEN(engine->i915) < 11)
+			regs[CTX_R_PWR_CLK_STATE] =
+				i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
+		else
+			regs[GEN12_CTX_R_PWR_CLK_STATE] =
+				i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
 	}
 }
 
@@ -1747,8 +1755,12 @@ __execlists_update_reg_state(struct intel_context *ce,
 
 	/* RPCS */
 	if (engine->class == RENDER_CLASS) {
-		regs[CTX_R_PWR_CLK_STATE + 1] =
-			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+		const u32 rpcs = intel_sseu_make_rpcs(engine->i915, &ce->sseu);
+		const u32 offset = INTEL_GEN(engine->i915) < 11 ?
+			CTX_R_PWR_CLK_STATE + 1 :
+			GEN12_CTX_R_PWR_CLK_STATE + 1;
+
+		regs[offset] = rpcs;
 
 		i915_oa_init_reg_state(engine, ce, regs);
 	}
@@ -3116,35 +3128,13 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
 	return indirect_ctx_offset;
 }
 
-static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
-{
-	if (i915_is_ggtt(vm))
-		return i915_vm_to_ggtt(vm)->alias;
-	else
-		return i915_vm_to_ppgtt(vm);
-}
 
-static void execlists_init_reg_state(u32 *regs,
-				     struct intel_context *ce,
-				     struct intel_engine_cs *engine,
-				     struct intel_ring *ring)
+static void init_common_reg_state(u32 * const regs,
+				  struct i915_ppgtt * const ppgtt,
+				  struct intel_engine_cs *engine,
+				  struct intel_ring *ring)
 {
-	struct i915_ppgtt *ppgtt = vm_alias(ce->vm);
-	bool rcs = engine->class == RENDER_CLASS;
-	u32 base = engine->mmio_base;
-
-	/*
-	 * A context is actually a big batch buffer with several
-	 * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
-	 * values we are setting here are only for the first context restore:
-	 * on a subsequent save, the GPU will recreate this batchbuffer with new
-	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
-	 * we are not initializing here).
-	 *
-	 * Must keep consistent with virtual_update_register_offsets().
-	 */
-	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
-				 MI_LRI_FORCE_POSTED;
+	const u32 base = engine->mmio_base;
 
 	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
 		_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
@@ -3162,38 +3152,44 @@ static void execlists_init_reg_state(u32 *regs,
 	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);
-	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,
-			RING_INDIRECT_CTX_OFFSET(base), 0);
-		if (wa_ctx->indirect_ctx.size) {
-			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
+}
 
-			regs[CTX_RCS_INDIRECT_CTX + 1] =
-				(ggtt_offset + wa_ctx->indirect_ctx.offset) |
-				(wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
+static void init_wa_bb_reg_state(u32 * const regs,
+				 struct intel_engine_cs *engine,
+				 u32 pos_bb_per_ctx)
+{
+	struct i915_ctx_workarounds * const wa_ctx = &engine->wa_ctx;
+	const u32 base = engine->mmio_base;
+	const u32 pos_indirect_ctx = pos_bb_per_ctx + 2;
+	const u32 pos_indirect_ctx_offset = pos_indirect_ctx + 2;
 
-			regs[CTX_RCS_INDIRECT_CTX_OFFSET + 1] =
-				intel_lr_indirect_ctx_offset(engine) << 6;
-		}
+	GEM_BUG_ON(engine->class != RENDER_CLASS);
+	CTX_REG(regs, pos_indirect_ctx, RING_INDIRECT_CTX(base), 0);
+	CTX_REG(regs, pos_indirect_ctx_offset,
+		RING_INDIRECT_CTX_OFFSET(base), 0);
+	if (wa_ctx->indirect_ctx.size) {
+		const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
 
-		CTX_REG(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);
+		regs[pos_indirect_ctx + 1] =
+			(ggtt_offset + wa_ctx->indirect_ctx.offset) |
+			(wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
 
-			regs[CTX_BB_PER_CTX_PTR + 1] =
-				(ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
-		}
+		regs[pos_indirect_ctx_offset + 1] =
+			intel_lr_indirect_ctx_offset(engine) << 6;
 	}
 
-	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
+	CTX_REG(regs, pos_bb_per_ctx, RING_BB_PER_CTX_PTR(base), 0);
+	if (wa_ctx->per_ctx.size) {
+		const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
 
-	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
+		regs[pos_bb_per_ctx + 1] =
+			(ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
+	}
+}
+
+static void init_ppgtt_reg_state(u32 *regs, u32 base,
+				 struct i915_ppgtt *ppgtt)
+{
 	/* 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);
@@ -3216,6 +3212,40 @@ static void execlists_init_reg_state(u32 *regs,
 		ASSIGN_CTX_PDP(ppgtt, regs, 1);
 		ASSIGN_CTX_PDP(ppgtt, regs, 0);
 	}
+}
+
+static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
+{
+	if (i915_is_ggtt(vm))
+		return i915_vm_to_ggtt(vm)->alias;
+	else
+		return i915_vm_to_ppgtt(vm);
+}
+
+static void gen8_init_reg_state(u32 * const regs,
+				struct intel_context *ce,
+				struct intel_engine_cs *engine,
+				struct intel_ring *ring)
+{
+	struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);
+	const bool rcs = engine->class == RENDER_CLASS;
+	const u32 base = engine->mmio_base;
+
+	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
+				 MI_LRI_FORCE_POSTED;
+
+	init_common_reg_state(regs, ppgtt, engine, ring);
+	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);
+	if (rcs)
+		init_wa_bb_reg_state(regs, engine, CTX_BB_PER_CTX_PTR);
+
+	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
+
+	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
+
+	init_ppgtt_reg_state(regs, base, ppgtt);
 
 	if (rcs) {
 		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
@@ -3227,6 +3257,58 @@ static void execlists_init_reg_state(u32 *regs,
 		regs[CTX_END] |= BIT(0);
 }
 
+static void gen12_init_reg_state(u32 * const regs,
+				 struct intel_context *ce,
+				 struct intel_engine_cs *engine,
+				 struct intel_ring *ring)
+{
+	struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);
+	const bool rcs = engine->class == RENDER_CLASS;
+	const u32 base = engine->mmio_base;
+
+	GEM_DEBUG_EXEC(DRM_INFO_ONCE("Using GEN12 Register State Context\n"));
+
+	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(13) |
+		MI_LRI_FORCE_POSTED;
+
+	init_common_reg_state(regs, ppgtt, engine, ring);
+	if (rcs)
+		init_wa_bb_reg_state(regs, engine, GEN12_CTX_BB_PER_CTX_PTR);
+
+	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) |
+		MI_LRI_FORCE_POSTED;
+
+	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
+
+	init_ppgtt_reg_state(regs, base, ppgtt);
+
+	if (rcs) {
+		regs[GEN12_CTX_LRI_HEADER_3] = MI_LOAD_REGISTER_IMM(1);
+		CTX_REG(regs, GEN12_CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
+			0);
+
+		/* TODO: oa_init_reg_state ? */
+	}
+}
+
+static void execlists_init_reg_state(u32 *regs,
+				     struct intel_context *ce,
+				     struct intel_engine_cs *engine,
+				     struct intel_ring *ring)
+{
+	/* A context is actually a big batch buffer with several
+	 * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
+	 * values we are setting here are only for the first context restore:
+	 * on a subsequent save, the GPU will recreate this batchbuffer with new
+	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
+	 * we are not initializing here).
+	 */
+	if (INTEL_GEN(engine->i915) >= 12)
+		gen12_init_reg_state(regs, ce, engine, ring);
+	else
+		gen8_init_reg_state(regs, ce, engine, ring);
+}
+
 static int
 populate_lr_context(struct intel_context *ce,
 		    struct drm_i915_gem_object *ctx_obj,
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/intel_lrc.h
index dc0252e0589e..a4bde38e35d8 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
@@ -49,6 +49,8 @@ struct intel_engine_cs;
 
 #define	  EL_CTRL_LOAD				(1 << 0)
 
+#define GEN12_ENGINE_SEMAPHORE_TOKEN(engine)	_MMIO((engine)->mmio_base + 0x2b4)
+
 /* The docs specify that the write pointer wraps around after 5h, "After status
  * is written out to the last available status QW at offset 5h, this pointer
  * wraps to 0."
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
index b8f20ad71169..8cb6655744cd 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
+++ b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
@@ -9,7 +9,7 @@
 
 #include <linux/types.h>
 
-/* GEN8+ Reg State Context */
+/* GEN8 to GEN11 Reg State Context */
 #define CTX_LRI_HEADER_0		0x01
 #define CTX_CONTEXT_CONTROL		0x02
 #define CTX_RING_HEAD			0x04
@@ -39,6 +39,11 @@
 #define CTX_R_PWR_CLK_STATE		0x42
 #define CTX_END				0x44
 
+/* GEN12+ Reg State Context */
+#define GEN12_CTX_BB_PER_CTX_PTR		0x12
+#define GEN12_CTX_LRI_HEADER_3			0x41
+#define GEN12_CTX_R_PWR_CLK_STATE		0x42
+
 #define CTX_REG(reg_state, pos, reg, val) do { \
 	u32 *reg_state__ = (reg_state); \
 	const u32 pos__ = (pos); \
-- 
2.17.1

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

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

* Re: [PATCH] drm/i915/tgl: Register state context definition for Gen12
  2019-09-05 11:34 [PATCH] drm/i915/tgl: Register state context definition for Gen12 Mika Kuoppala
@ 2019-09-05 13:40 ` Andi Shyti
  2019-09-05 13:47 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Andi Shyti @ 2019-09-05 13:40 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Michel Thierry, intel-gfx, Lucas De Marchi

On Thu, Sep 05, 2019 at 02:34:55PM +0300, Mika Kuoppala wrote:
> From: Michel Thierry <michel.thierry@intel.com>
> 
> Gen12 has subtle changes in the reg state context offsets (some fields
> are gone, some are in a different location), compared to previous Gens.
> 
> The simplest approach seems to be keeping Gen12 (and future platform)
> changes apart from the previous gens, while keeping the registers that
> are contiguous in functions we can reuse.
> 
> v2: alias, virtual engine, rpcs, prune unused regs (Mika)
> 
> Bspec: 46255
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

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

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

* ✓ Fi.CI.BAT: success for drm/i915/tgl: Register state context definition for Gen12
  2019-09-05 11:34 [PATCH] drm/i915/tgl: Register state context definition for Gen12 Mika Kuoppala
  2019-09-05 13:40 ` Andi Shyti
@ 2019-09-05 13:47 ` Patchwork
  2019-09-05 16:13 ` [PATCH] " Daniele Ceraolo Spurio
  2019-09-05 17:05 ` ✓ Fi.CI.IGT: success for " Patchwork
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-09-05 13:47 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Register state context definition for Gen12
URL   : https://patchwork.freedesktop.org/series/66276/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6838 -> Patchwork_14286
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_14286:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_gttfill@basic:
    - {fi-tgl-u}:         [FAIL][1] ([fdo#111562]) -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-tgl-u/igt@gem_exec_gttfill@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-tgl-u/igt@gem_exec_gttfill@basic.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@create-close:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-icl-u3/igt@gem_basic@create-close.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-icl-u3/igt@gem_basic@create-close.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [PASS][5] -> [INCOMPLETE][6] ([fdo#111514])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  
#### Possible fixes ####

  * igt@gem_busy@busy-all:
    - {fi-tgl-u}:         [FAIL][7] ([fdo#111560]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-tgl-u/igt@gem_busy@busy-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-tgl-u/igt@gem_busy@busy-all.html

  * igt@gem_close_race@basic-process:
    - {fi-tgl-u}:         [DMESG-FAIL][9] ([fdo#111562]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-tgl-u/igt@gem_close_race@basic-process.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-tgl-u/igt@gem_close_race@basic-process.html

  * igt@gem_exec_create@basic:
    - {fi-tgl-u}:         [FAIL][11] ([fdo#111562]) -> [PASS][12] +5 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-tgl-u/igt@gem_exec_create@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-tgl-u/igt@gem_exec_create@basic.html

  * igt@gem_mmap_gtt@basic:
    - fi-glk-dsi:         [INCOMPLETE][13] ([fdo#103359] / [k.org#198133]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-glk-dsi/igt@gem_mmap_gtt@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-glk-dsi/igt@gem_mmap_gtt@basic.html

  * igt@kms_busy@basic-flip-c:
    - fi-skl-6770hq:      [SKIP][15] ([fdo#109271] / [fdo#109278]) -> [PASS][16] +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-kbl-7500u:       [WARN][17] ([fdo#109483]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [SKIP][19] ([fdo#109271]) -> [PASS][20] +23 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][21] ([fdo#102614]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  * igt@vgem_basic@debugfs:
    - fi-icl-u3:          [DMESG-WARN][23] ([fdo#107724]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/fi-icl-u3/igt@vgem_basic@debugfs.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/fi-icl-u3/igt@vgem_basic@debugfs.html

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
  [fdo#111514]: https://bugs.freedesktop.org/show_bug.cgi?id=111514
  [fdo#111560]: https://bugs.freedesktop.org/show_bug.cgi?id=111560
  [fdo#111562]: https://bugs.freedesktop.org/show_bug.cgi?id=111562
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (53 -> 47)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6838 -> Patchwork_14286

  CI-20190529: 20190529
  CI_DRM_6838: 8e907b7591b620dba402c7ada493a31ca0320c99 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5171: 1911564805fe454919e8a5846534a0c1ef376a33 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14286: d2f84d7ccf298368e7f518f51e9ab1c169e868ea @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d2f84d7ccf29 drm/i915/tgl: Register state context definition for Gen12

== Logs ==

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

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

* Re: [PATCH] drm/i915/tgl: Register state context definition for Gen12
  2019-09-05 11:34 [PATCH] drm/i915/tgl: Register state context definition for Gen12 Mika Kuoppala
  2019-09-05 13:40 ` Andi Shyti
  2019-09-05 13:47 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-09-05 16:13 ` Daniele Ceraolo Spurio
  2019-09-05 22:48   ` Daniele Ceraolo Spurio
  2019-09-05 17:05 ` ✓ Fi.CI.IGT: success for " Patchwork
  3 siblings, 1 reply; 6+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-09-05 16:13 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx; +Cc: Michel Thierry, Lucas De Marchi



On 9/5/2019 4:34 AM, Mika Kuoppala wrote:
> From: Michel Thierry <michel.thierry@intel.com>
>
> Gen12 has subtle changes in the reg state context offsets (some fields
> are gone, some are in a different location), compared to previous Gens.
>
> The simplest approach seems to be keeping Gen12 (and future platform)
> changes apart from the previous gens, while keeping the registers that
> are contiguous in functions we can reuse.
>
> v2: alias, virtual engine, rpcs, prune unused regs (Mika)
>
> Bspec: 46255
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c     | 194 +++++++++++++++++-------
>   drivers/gpu/drm/i915/gt/intel_lrc.h     |   2 +
>   drivers/gpu/drm/i915/gt/intel_lrc_reg.h |   7 +-
>   3 files changed, 146 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 87b7473a6dfb..f44c78a82915 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -810,6 +810,7 @@ static void virtual_update_register_offsets(u32 *regs,
>   
>   	/* Must match execlists_init_reg_state()! */
>   
> +	/* Common part */
>   	regs[CTX_CONTEXT_CONTROL] =
>   		i915_mmio_reg_offset(RING_CONTEXT_CONTROL(base));
>   	regs[CTX_RING_HEAD] = i915_mmio_reg_offset(RING_HEAD(base));
> @@ -820,13 +821,16 @@ static void virtual_update_register_offsets(u32 *regs,
>   	regs[CTX_BB_HEAD_U] = i915_mmio_reg_offset(RING_BBADDR_UDW(base));
>   	regs[CTX_BB_HEAD_L] = i915_mmio_reg_offset(RING_BBADDR(base));
>   	regs[CTX_BB_STATE] = i915_mmio_reg_offset(RING_BBSTATE(base));
> +
>   	regs[CTX_SECOND_BB_HEAD_U] =
>   		i915_mmio_reg_offset(RING_SBBADDR_UDW(base));
>   	regs[CTX_SECOND_BB_HEAD_L] = i915_mmio_reg_offset(RING_SBBADDR(base));
>   	regs[CTX_SECOND_BB_STATE] = i915_mmio_reg_offset(RING_SBBSTATE(base));
>   

We should do those 3 only on pre-gen12, or at least add a comment saying 
that those are not applicable to gen12+ and we rely to follow-up writes 
to overwrite them.

> +	/* PPGTT part */
>   	regs[CTX_CTX_TIMESTAMP] =
>   		i915_mmio_reg_offset(RING_CTX_TIMESTAMP(base));
> +
>   	regs[CTX_PDP3_UDW] = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 3));
>   	regs[CTX_PDP3_LDW] = i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 3));
>   	regs[CTX_PDP2_UDW] = i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 2));
> @@ -844,8 +848,12 @@ static void virtual_update_register_offsets(u32 *regs,
>   		regs[CTX_BB_PER_CTX_PTR] =
>   			i915_mmio_reg_offset(RING_BB_PER_CTX_PTR(base));

CTX_BB_PER_CTX_PTR and the 2 indirect ctx entries are different on gen12 
as well

>   
> -		regs[CTX_R_PWR_CLK_STATE] =
> -			i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
> +		if (INTEL_GEN(engine->i915) < 11)

This should be < 12

> +			regs[CTX_R_PWR_CLK_STATE] =
> +				i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
> +		else
> +			regs[GEN12_CTX_R_PWR_CLK_STATE] =
> +				i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
>   	}
>   }
>   
> @@ -1747,8 +1755,12 @@ __execlists_update_reg_state(struct intel_context *ce,
>   
>   	/* RPCS */
>   	if (engine->class == RENDER_CLASS) {
> -		regs[CTX_R_PWR_CLK_STATE + 1] =
> -			intel_sseu_make_rpcs(engine->i915, &ce->sseu);
> +		const u32 rpcs = intel_sseu_make_rpcs(engine->i915, &ce->sseu);
> +		const u32 offset = INTEL_GEN(engine->i915) < 11 ?

< 12

> +			CTX_R_PWR_CLK_STATE + 1 :
> +			GEN12_CTX_R_PWR_CLK_STATE + 1;
> +
> +		regs[offset] = rpcs;
>   
>   		i915_oa_init_reg_state(engine, ce, regs);
>   	}
> @@ -3116,35 +3128,13 @@ static u32 intel_lr_indirect_ctx_offset(struct intel_engine_cs *engine)
>   	return indirect_ctx_offset;
>   }
>   
> -static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
> -{
> -	if (i915_is_ggtt(vm))
> -		return i915_vm_to_ggtt(vm)->alias;
> -	else
> -		return i915_vm_to_ppgtt(vm);
> -}
>   
> -static void execlists_init_reg_state(u32 *regs,
> -				     struct intel_context *ce,
> -				     struct intel_engine_cs *engine,
> -				     struct intel_ring *ring)
> +static void init_common_reg_state(u32 * const regs,
> +				  struct i915_ppgtt * const ppgtt,
> +				  struct intel_engine_cs *engine,
> +				  struct intel_ring *ring)
>   {
> -	struct i915_ppgtt *ppgtt = vm_alias(ce->vm);
> -	bool rcs = engine->class == RENDER_CLASS;
> -	u32 base = engine->mmio_base;
> -
> -	/*
> -	 * A context is actually a big batch buffer with several
> -	 * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
> -	 * values we are setting here are only for the first context restore:
> -	 * on a subsequent save, the GPU will recreate this batchbuffer with new
> -	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
> -	 * we are not initializing here).
> -	 *
> -	 * Must keep consistent with virtual_update_register_offsets().
> -	 */
> -	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
> -				 MI_LRI_FORCE_POSTED;
> +	const u32 base = engine->mmio_base;
>   
>   	CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
>   		_MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
> @@ -3162,38 +3152,44 @@ static void execlists_init_reg_state(u32 *regs,
>   	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);
> -	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,
> -			RING_INDIRECT_CTX_OFFSET(base), 0);
> -		if (wa_ctx->indirect_ctx.size) {
> -			u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
> +}
>   
> -			regs[CTX_RCS_INDIRECT_CTX + 1] =
> -				(ggtt_offset + wa_ctx->indirect_ctx.offset) |
> -				(wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
> +static void init_wa_bb_reg_state(u32 * const regs,
> +				 struct intel_engine_cs *engine,
> +				 u32 pos_bb_per_ctx)
> +{
> +	struct i915_ctx_workarounds * const wa_ctx = &engine->wa_ctx;
> +	const u32 base = engine->mmio_base;
> +	const u32 pos_indirect_ctx = pos_bb_per_ctx + 2;
> +	const u32 pos_indirect_ctx_offset = pos_indirect_ctx + 2;
>   
> -			regs[CTX_RCS_INDIRECT_CTX_OFFSET + 1] =
> -				intel_lr_indirect_ctx_offset(engine) << 6;
> -		}
> +	GEM_BUG_ON(engine->class != RENDER_CLASS);
> +	CTX_REG(regs, pos_indirect_ctx, RING_INDIRECT_CTX(base), 0);
> +	CTX_REG(regs, pos_indirect_ctx_offset,
> +		RING_INDIRECT_CTX_OFFSET(base), 0);
> +	if (wa_ctx->indirect_ctx.size) {
> +		const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>   
> -		CTX_REG(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);
> +		regs[pos_indirect_ctx + 1] =
> +			(ggtt_offset + wa_ctx->indirect_ctx.offset) |
> +			(wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
>   
> -			regs[CTX_BB_PER_CTX_PTR + 1] =
> -				(ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
> -		}
> +		regs[pos_indirect_ctx_offset + 1] =
> +			intel_lr_indirect_ctx_offset(engine) << 6;
>   	}
>   
> -	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
> +	CTX_REG(regs, pos_bb_per_ctx, RING_BB_PER_CTX_PTR(base), 0);
> +	if (wa_ctx->per_ctx.size) {
> +		const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>   
> -	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
> +		regs[pos_bb_per_ctx + 1] =
> +			(ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
> +	}
> +}
> +
> +static void init_ppgtt_reg_state(u32 *regs, u32 base,
> +				 struct i915_ppgtt *ppgtt)
> +{
>   	/* 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);
> @@ -3216,6 +3212,40 @@ static void execlists_init_reg_state(u32 *regs,
>   		ASSIGN_CTX_PDP(ppgtt, regs, 1);
>   		ASSIGN_CTX_PDP(ppgtt, regs, 0);
>   	}
> +}
> +
> +static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
> +{
> +	if (i915_is_ggtt(vm))
> +		return i915_vm_to_ggtt(vm)->alias;
> +	else
> +		return i915_vm_to_ppgtt(vm);
> +}
> +
> +static void gen8_init_reg_state(u32 * const regs,
> +				struct intel_context *ce,
> +				struct intel_engine_cs *engine,
> +				struct intel_ring *ring)
> +{
> +	struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);
> +	const bool rcs = engine->class == RENDER_CLASS;
> +	const u32 base = engine->mmio_base;
> +
> +	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
> +				 MI_LRI_FORCE_POSTED;
> +
> +	init_common_reg_state(regs, ppgtt, engine, ring);
> +	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);
> +	if (rcs)
> +		init_wa_bb_reg_state(regs, engine, CTX_BB_PER_CTX_PTR);
> +
> +	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | MI_LRI_FORCE_POSTED;
> +
> +	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
> +
> +	init_ppgtt_reg_state(regs, base, ppgtt);
>   
>   	if (rcs) {
>   		regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
> @@ -3227,6 +3257,58 @@ static void execlists_init_reg_state(u32 *regs,
>   		regs[CTX_END] |= BIT(0);
>   }
>   
> +static void gen12_init_reg_state(u32 * const regs,
> +				 struct intel_context *ce,
> +				 struct intel_engine_cs *engine,
> +				 struct intel_ring *ring)
> +{
> +	struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);

Do we expect appgtt gen12+? If not we could use i915_vm_to_ppgtt() directly

> +	const bool rcs = engine->class == RENDER_CLASS;
> +	const u32 base = engine->mmio_base;
> +
> +	GEM_DEBUG_EXEC(DRM_INFO_ONCE("Using GEN12 Register State Context\n"));
> +
> +	regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(13) |
> +		MI_LRI_FORCE_POSTED;
> +
> +	init_common_reg_state(regs, ppgtt, engine, ring);
> +	if (rcs)
> +		init_wa_bb_reg_state(regs, engine, GEN12_CTX_BB_PER_CTX_PTR);
> +
> +	regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) |
> +		MI_LRI_FORCE_POSTED;
> +
> +	CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
> +
> +	init_ppgtt_reg_state(regs, base, ppgtt);
> +
> +	if (rcs) {
> +		regs[GEN12_CTX_LRI_HEADER_3] = MI_LOAD_REGISTER_IMM(1);
> +		CTX_REG(regs, GEN12_CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
> +			0);
> +
> +		/* TODO: oa_init_reg_state ? */
> +	}
> +}
> +
> +static void execlists_init_reg_state(u32 *regs,
> +				     struct intel_context *ce,
> +				     struct intel_engine_cs *engine,
> +				     struct intel_ring *ring)
> +{
> +	/* A context is actually a big batch buffer with several
> +	 * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
> +	 * values we are setting here are only for the first context restore:
> +	 * on a subsequent save, the GPU will recreate this batchbuffer with new
> +	 * values (including all the missing MI_LOAD_REGISTER_IMM commands that
> +	 * we are not initializing here).
> +	 */
> +	if (INTEL_GEN(engine->i915) >= 12)
> +		gen12_init_reg_state(regs, ce, engine, ring);
> +	else
> +		gen8_init_reg_state(regs, ce, engine, ring);
> +}
> +
>   static int
>   populate_lr_context(struct intel_context *ce,
>   		    struct drm_i915_gem_object *ctx_obj,
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h b/drivers/gpu/drm/i915/gt/intel_lrc.h
> index dc0252e0589e..a4bde38e35d8 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
> @@ -49,6 +49,8 @@ struct intel_engine_cs;
>   
>   #define	  EL_CTRL_LOAD				(1 << 0)
>   
> +#define GEN12_ENGINE_SEMAPHORE_TOKEN(engine)	_MMIO((engine)->mmio_base + 0x2b4)
> +

As I commented on the previous revision, defining this in this patch 
without using it or setting it in the context doesn't make much sense.

>   /* The docs specify that the write pointer wraps around after 5h, "After status
>    * is written out to the last available status QW at offset 5h, this pointer
>    * wraps to 0."
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
> index b8f20ad71169..8cb6655744cd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
> @@ -9,7 +9,7 @@
>   
>   #include <linux/types.h>
>   
> -/* GEN8+ Reg State Context */
> +/* GEN8 to GEN11 Reg State Context */
>   #define CTX_LRI_HEADER_0		0x01
>   #define CTX_CONTEXT_CONTROL		0x02
>   #define CTX_RING_HEAD			0x04
> @@ -39,6 +39,11 @@
>   #define CTX_R_PWR_CLK_STATE		0x42
>   #define CTX_END				0x44
>   
> +/* GEN12+ Reg State Context */
> +#define GEN12_CTX_BB_PER_CTX_PTR		0x12
> +#define GEN12_CTX_LRI_HEADER_3			0x41

Gen11 has the same number and positioning of LRIs as gen12, so marking 
this as a gen12 change doesn't feel too reflective of teh HW changes 
IMO, but not a blocker.


> +#define GEN12_CTX_R_PWR_CLK_STATE		0x42

This hasn't moved compared to gen11:
      #define CTX_R_PWR_CLK_STATE             0x42

so no need for a different defs and all those ifs higher up in the patch.

Daniele

> +
>   #define CTX_REG(reg_state, pos, reg, val) do { \
>   	u32 *reg_state__ = (reg_state); \
>   	const u32 pos__ = (pos); \

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

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

* ✓ Fi.CI.IGT: success for drm/i915/tgl: Register state context definition for Gen12
  2019-09-05 11:34 [PATCH] drm/i915/tgl: Register state context definition for Gen12 Mika Kuoppala
                   ` (2 preceding siblings ...)
  2019-09-05 16:13 ` [PATCH] " Daniele Ceraolo Spurio
@ 2019-09-05 17:05 ` Patchwork
  3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2019-09-05 17:05 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/tgl: Register state context definition for Gen12
URL   : https://patchwork.freedesktop.org/series/66276/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6838_full -> Patchwork_14286_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +5 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl2/igt@gem_ctx_isolation@rcs0-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl5/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][3] -> [SKIP][4] ([fdo#111325]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb5/igt@gem_exec_schedule@in-order-bsd.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_exec_schedule@promotion-bsd1:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#109276]) +6 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb4/igt@gem_exec_schedule@promotion-bsd1.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb7/igt@gem_exec_schedule@promotion-bsd1.html

  * igt@gem_exec_suspend@basic-s3-devices:
    - shard-hsw:          [PASS][7] -> [FAIL][8] ([fdo#111550])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw8/igt@gem_exec_suspend@basic-s3-devices.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw7/igt@gem_exec_suspend@basic-s3-devices.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - shard-apl:          [PASS][9] -> [FAIL][10] ([fdo#111550])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl8/igt@gem_exec_suspend@basic-s4-devices.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl8/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
    - shard-apl:          [PASS][11] -> [SKIP][12] ([fdo#109271]) +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl8/igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl8/igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render.html

  * igt@gem_softpin@noreloc-s3:
    - shard-hsw:          [PASS][13] -> [FAIL][14] ([fdo#103375]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw6/igt@gem_softpin@noreloc-s3.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw7/igt@gem_softpin@noreloc-s3.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([fdo#108686])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl5/igt@gem_tiled_swapping@non-threaded.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl3/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_rpm@gem-pread:
    - shard-hsw:          [PASS][17] -> [FAIL][18] ([fdo#111548]) +6 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw7/igt@i915_pm_rpm@gem-pread.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw7/igt@i915_pm_rpm@gem-pread.html

  * igt@i915_suspend@forcewake:
    - shard-snb:          [PASS][19] -> [FAIL][20] ([fdo#103375]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-snb2/igt@i915_suspend@forcewake.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-snb5/igt@i915_suspend@forcewake.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#103927])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl8/igt@i915_suspend@sysfs-reader.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl5/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([fdo#105363])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl10/igt@kms_flip@flip-vs-expired-vblank.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl9/igt@kms_flip@flip-vs-expired-vblank.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#103167])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [PASS][27] -> [FAIL][28] ([fdo#103167]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-apl:          [PASS][29] -> [FAIL][30] ([fdo#103375])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][31] -> [SKIP][32] ([fdo#109441]) +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb4/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][33] -> [INCOMPLETE][34] ([fdo#103665]) +1 similar issue
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@other-read-0:
    - shard-apl:          [PASS][35] -> [FAIL][36] ([fdo#111545]) +6 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl8/igt@perf_pmu@other-read-0.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl8/igt@perf_pmu@other-read-0.html

  
#### Possible fixes ####

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][37] ([fdo#110841]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb1/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb6/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_async@concurrent-writes-bsd:
    - shard-iclb:         [SKIP][39] ([fdo#111325]) -> [PASS][40] +1 similar issue
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb4/igt@gem_exec_async@concurrent-writes-bsd.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb8/igt@gem_exec_async@concurrent-writes-bsd.html

  * igt@gem_exec_reloc@basic-gtt-active:
    - shard-skl:          [DMESG-WARN][41] ([fdo#106107]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl5/igt@gem_exec_reloc@basic-gtt-active.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl10/igt@gem_exec_reloc@basic-gtt-active.html

  * igt@gem_exec_schedule@independent-bsd2:
    - shard-iclb:         [SKIP][43] ([fdo#109276]) -> [PASS][44] +7 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb7/igt@gem_exec_schedule@independent-bsd2.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb2/igt@gem_exec_schedule@independent-bsd2.html

  * igt@gem_request_retire@retire-vma-not-inactive:
    - shard-apl:          [INCOMPLETE][45] ([fdo#103927]) -> [PASS][46] +3 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl6/igt@gem_request_retire@retire-vma-not-inactive.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl3/igt@gem_request_retire@retire-vma-not-inactive.html

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          [INCOMPLETE][47] ([fdo#104108] / [fdo#107773]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl1/igt@gem_softpin@noreloc-s3.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl4/igt@gem_softpin@noreloc-s3.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          [SKIP][49] ([fdo#109271]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-snb4/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-snb5/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-hsw:          [FAIL][51] ([fdo#111548]) -> [PASS][52] +4 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw2/igt@i915_pm_rpm@system-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw2/igt@i915_pm_rpm@system-suspend.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][53] ([fdo#105363]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-apl:          [DMESG-WARN][55] ([fdo#108566]) -> [PASS][56] +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-apl5/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-apl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [FAIL][57] ([fdo#103167]) -> [PASS][58] +5 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-snb:          [FAIL][59] ([fdo#103375]) -> [PASS][60] +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-snb7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-snb4/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-kbl:          [INCOMPLETE][61] ([fdo#103665]) -> [PASS][62]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-hsw:          [FAIL][63] ([fdo#103375]) -> [PASS][64] +4 similar issues
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][65] ([fdo#108145]) -> [PASS][66] +1 similar issue
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-iclb:         [INCOMPLETE][67] ([fdo#107713]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][69] ([fdo#109441]) -> [PASS][70] +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb7/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-reset-bsd2:
    - shard-iclb:         [SKIP][71] ([fdo#109276]) -> [FAIL][72] ([fdo#111330]) +1 similar issue
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-iclb5/igt@gem_mocs_settings@mocs-reset-bsd2.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-iclb1/igt@gem_mocs_settings@mocs-reset-bsd2.html

  * igt@i915_pm_rpm@pc8-residency:
    - shard-hsw:          [SKIP][73] ([fdo#109271]) -> [FAIL][74] ([fdo#111548])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6838/shard-hsw7/igt@i915_pm_rpm@pc8-residency.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14286/shard-hsw7/igt@i915_pm_rpm@pc8-residency.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [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#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
  [fdo#111545]: https://bugs.freedesktop.org/show_bug.cgi?id=111545
  [fdo#111548]: https://bugs.freedesktop.org/show_bug.cgi?id=111548
  [fdo#111550]: https://bugs.freedesktop.org/show_bug.cgi?id=111550


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

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6838 -> Patchwork_14286

  CI-20190529: 20190529
  CI_DRM_6838: 8e907b7591b620dba402c7ada493a31ca0320c99 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5171: 1911564805fe454919e8a5846534a0c1ef376a33 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14286: d2f84d7ccf298368e7f518f51e9ab1c169e868ea @ 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_14286/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/tgl: Register state context definition for Gen12
  2019-09-05 16:13 ` [PATCH] " Daniele Ceraolo Spurio
@ 2019-09-05 22:48   ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 6+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-09-05 22:48 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx



On 9/5/19 9:13 AM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 9/5/2019 4:34 AM, Mika Kuoppala wrote:
>> From: Michel Thierry <michel.thierry@intel.com>
>>
>> Gen12 has subtle changes in the reg state context offsets (some fields
>> are gone, some are in a different location), compared to previous Gens.
>>
>> The simplest approach seems to be keeping Gen12 (and future platform)
>> changes apart from the previous gens, while keeping the registers that
>> are contiguous in functions we can reuse.
>>
>> v2: alias, virtual engine, rpcs, prune unused regs (Mika)
>>
>> Bspec: 46255
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_lrc.c     | 194 +++++++++++++++++-------
>>   drivers/gpu/drm/i915/gt/intel_lrc.h     |   2 +
>>   drivers/gpu/drm/i915/gt/intel_lrc_reg.h |   7 +-
>>   3 files changed, 146 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
>> b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> index 87b7473a6dfb..f44c78a82915 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>> @@ -810,6 +810,7 @@ static void virtual_update_register_offsets(u32 

I've been looking a bit more into the relative MMIO stuff and I've 
realized that we've been using it all along without realizing. When the 
context is saved by the HW, the Add CS MMIO Start Offset flag is set in 
the context LRIs (according to the specs this is true Gen11+, but I've 
only explicitly verified it on TGL), so updating the offsets like we do 
below doesn't make much sense. It all works only because during the ctx 
restore (and only then, see Bspec 20206 & 45728) the HW is smart enough 
to implicitly convert absolute offsets to relative ones and then add 
back the correct MMIO base.

I've tried setting the flag in the context LRIs we write (to cover the 
first submission) and disable this function on a TGL and I was able to 
run gem_exec_balancer@nop, with the same context being submitted to 
different engines without issues. Maybe that's a quicker solution 
compared to keeping this function in sync with the context changes for 
gen12, considering that a couple more updates to the context image are 
going to be required for new functionality?

Daniele

>> *regs,
>>       /* Must match execlists_init_reg_state()! */
>> +    /* Common part */
>>       regs[CTX_CONTEXT_CONTROL] =
>>           i915_mmio_reg_offset(RING_CONTEXT_CONTROL(base));
>>       regs[CTX_RING_HEAD] = i915_mmio_reg_offset(RING_HEAD(base));
>> @@ -820,13 +821,16 @@ static void virtual_update_register_offsets(u32 
>> *regs,
>>       regs[CTX_BB_HEAD_U] = i915_mmio_reg_offset(RING_BBADDR_UDW(base));
>>       regs[CTX_BB_HEAD_L] = i915_mmio_reg_offset(RING_BBADDR(base));
>>       regs[CTX_BB_STATE] = i915_mmio_reg_offset(RING_BBSTATE(base));
>> +
>>       regs[CTX_SECOND_BB_HEAD_U] =
>>           i915_mmio_reg_offset(RING_SBBADDR_UDW(base));
>>       regs[CTX_SECOND_BB_HEAD_L] = 
>> i915_mmio_reg_offset(RING_SBBADDR(base));
>>       regs[CTX_SECOND_BB_STATE] = 
>> i915_mmio_reg_offset(RING_SBBSTATE(base));
> 
> We should do those 3 only on pre-gen12, or at least add a comment saying 
> that those are not applicable to gen12+ and we rely to follow-up writes 
> to overwrite them.
> 
>> +    /* PPGTT part */
>>       regs[CTX_CTX_TIMESTAMP] =
>>           i915_mmio_reg_offset(RING_CTX_TIMESTAMP(base));
>> +
>>       regs[CTX_PDP3_UDW] = 
>> i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 3));
>>       regs[CTX_PDP3_LDW] = 
>> i915_mmio_reg_offset(GEN8_RING_PDP_LDW(base, 3));
>>       regs[CTX_PDP2_UDW] = 
>> i915_mmio_reg_offset(GEN8_RING_PDP_UDW(base, 2));
>> @@ -844,8 +848,12 @@ static void virtual_update_register_offsets(u32 
>> *regs,
>>           regs[CTX_BB_PER_CTX_PTR] =
>>               i915_mmio_reg_offset(RING_BB_PER_CTX_PTR(base));
> 
> CTX_BB_PER_CTX_PTR and the 2 indirect ctx entries are different on gen12 
> as well
> 
>> -        regs[CTX_R_PWR_CLK_STATE] =
>> -            i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
>> +        if (INTEL_GEN(engine->i915) < 11)
> 
> This should be < 12
> 
>> +            regs[CTX_R_PWR_CLK_STATE] =
>> +                i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
>> +        else
>> +            regs[GEN12_CTX_R_PWR_CLK_STATE] =
>> +                i915_mmio_reg_offset(GEN8_R_PWR_CLK_STATE);
>>       }
>>   }
>> @@ -1747,8 +1755,12 @@ __execlists_update_reg_state(struct 
>> intel_context *ce,
>>       /* RPCS */
>>       if (engine->class == RENDER_CLASS) {
>> -        regs[CTX_R_PWR_CLK_STATE + 1] =
>> -            intel_sseu_make_rpcs(engine->i915, &ce->sseu);
>> +        const u32 rpcs = intel_sseu_make_rpcs(engine->i915, &ce->sseu);
>> +        const u32 offset = INTEL_GEN(engine->i915) < 11 ?
> 
> < 12
> 
>> +            CTX_R_PWR_CLK_STATE + 1 :
>> +            GEN12_CTX_R_PWR_CLK_STATE + 1;
>> +
>> +        regs[offset] = rpcs;
>>           i915_oa_init_reg_state(engine, ce, regs);
>>       }
>> @@ -3116,35 +3128,13 @@ static u32 intel_lr_indirect_ctx_offset(struct 
>> intel_engine_cs *engine)
>>       return indirect_ctx_offset;
>>   }
>> -static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
>> -{
>> -    if (i915_is_ggtt(vm))
>> -        return i915_vm_to_ggtt(vm)->alias;
>> -    else
>> -        return i915_vm_to_ppgtt(vm);
>> -}
>> -static void execlists_init_reg_state(u32 *regs,
>> -                     struct intel_context *ce,
>> -                     struct intel_engine_cs *engine,
>> -                     struct intel_ring *ring)
>> +static void init_common_reg_state(u32 * const regs,
>> +                  struct i915_ppgtt * const ppgtt,
>> +                  struct intel_engine_cs *engine,
>> +                  struct intel_ring *ring)
>>   {
>> -    struct i915_ppgtt *ppgtt = vm_alias(ce->vm);
>> -    bool rcs = engine->class == RENDER_CLASS;
>> -    u32 base = engine->mmio_base;
>> -
>> -    /*
>> -     * A context is actually a big batch buffer with several
>> -     * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
>> -     * values we are setting here are only for the first context 
>> restore:
>> -     * on a subsequent save, the GPU will recreate this batchbuffer 
>> with new
>> -     * values (including all the missing MI_LOAD_REGISTER_IMM 
>> commands that
>> -     * we are not initializing here).
>> -     *
>> -     * Must keep consistent with virtual_update_register_offsets().
>> -     */
>> -    regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
>> -                 MI_LRI_FORCE_POSTED;
>> +    const u32 base = engine->mmio_base;
>>       CTX_REG(regs, CTX_CONTEXT_CONTROL, RING_CONTEXT_CONTROL(base),
>>           _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) |
>> @@ -3162,38 +3152,44 @@ static void execlists_init_reg_state(u32 *regs,
>>       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);
>> -    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,
>> -            RING_INDIRECT_CTX_OFFSET(base), 0);
>> -        if (wa_ctx->indirect_ctx.size) {
>> -            u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>> +}
>> -            regs[CTX_RCS_INDIRECT_CTX + 1] =
>> -                (ggtt_offset + wa_ctx->indirect_ctx.offset) |
>> -                (wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
>> +static void init_wa_bb_reg_state(u32 * const regs,
>> +                 struct intel_engine_cs *engine,
>> +                 u32 pos_bb_per_ctx)
>> +{
>> +    struct i915_ctx_workarounds * const wa_ctx = &engine->wa_ctx;
>> +    const u32 base = engine->mmio_base;
>> +    const u32 pos_indirect_ctx = pos_bb_per_ctx + 2;
>> +    const u32 pos_indirect_ctx_offset = pos_indirect_ctx + 2;
>> -            regs[CTX_RCS_INDIRECT_CTX_OFFSET + 1] =
>> -                intel_lr_indirect_ctx_offset(engine) << 6;
>> -        }
>> +    GEM_BUG_ON(engine->class != RENDER_CLASS);
>> +    CTX_REG(regs, pos_indirect_ctx, RING_INDIRECT_CTX(base), 0);
>> +    CTX_REG(regs, pos_indirect_ctx_offset,
>> +        RING_INDIRECT_CTX_OFFSET(base), 0);
>> +    if (wa_ctx->indirect_ctx.size) {
>> +        const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>> -        CTX_REG(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);
>> +        regs[pos_indirect_ctx + 1] =
>> +            (ggtt_offset + wa_ctx->indirect_ctx.offset) |
>> +            (wa_ctx->indirect_ctx.size / CACHELINE_BYTES);
>> -            regs[CTX_BB_PER_CTX_PTR + 1] =
>> -                (ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
>> -        }
>> +        regs[pos_indirect_ctx_offset + 1] =
>> +            intel_lr_indirect_ctx_offset(engine) << 6;
>>       }
>> -    regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | 
>> MI_LRI_FORCE_POSTED;
>> +    CTX_REG(regs, pos_bb_per_ctx, RING_BB_PER_CTX_PTR(base), 0);
>> +    if (wa_ctx->per_ctx.size) {
>> +        const u32 ggtt_offset = i915_ggtt_offset(wa_ctx->vma);
>> -    CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
>> +        regs[pos_bb_per_ctx + 1] =
>> +            (ggtt_offset + wa_ctx->per_ctx.offset) | 0x01;
>> +    }
>> +}
>> +
>> +static void init_ppgtt_reg_state(u32 *regs, u32 base,
>> +                 struct i915_ppgtt *ppgtt)
>> +{
>>       /* 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);
>> @@ -3216,6 +3212,40 @@ static void execlists_init_reg_state(u32 *regs,
>>           ASSIGN_CTX_PDP(ppgtt, regs, 1);
>>           ASSIGN_CTX_PDP(ppgtt, regs, 0);
>>       }
>> +}
>> +
>> +static struct i915_ppgtt *vm_alias(struct i915_address_space *vm)
>> +{
>> +    if (i915_is_ggtt(vm))
>> +        return i915_vm_to_ggtt(vm)->alias;
>> +    else
>> +        return i915_vm_to_ppgtt(vm);
>> +}
>> +
>> +static void gen8_init_reg_state(u32 * const regs,
>> +                struct intel_context *ce,
>> +                struct intel_engine_cs *engine,
>> +                struct intel_ring *ring)
>> +{
>> +    struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);
>> +    const bool rcs = engine->class == RENDER_CLASS;
>> +    const u32 base = engine->mmio_base;
>> +
>> +    regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(rcs ? 14 : 11) |
>> +                 MI_LRI_FORCE_POSTED;
>> +
>> +    init_common_reg_state(regs, ppgtt, engine, ring);
>> +    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);
>> +    if (rcs)
>> +        init_wa_bb_reg_state(regs, engine, CTX_BB_PER_CTX_PTR);
>> +
>> +    regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) | 
>> MI_LRI_FORCE_POSTED;
>> +
>> +    CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
>> +
>> +    init_ppgtt_reg_state(regs, base, ppgtt);
>>       if (rcs) {
>>           regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
>> @@ -3227,6 +3257,58 @@ static void execlists_init_reg_state(u32 *regs,
>>           regs[CTX_END] |= BIT(0);
>>   }
>> +static void gen12_init_reg_state(u32 * const regs,
>> +                 struct intel_context *ce,
>> +                 struct intel_engine_cs *engine,
>> +                 struct intel_ring *ring)
>> +{
>> +    struct i915_ppgtt * const ppgtt = vm_alias(ce->vm);
> 
> Do we expect appgtt gen12+? If not we could use i915_vm_to_ppgtt() directly
> 
>> +    const bool rcs = engine->class == RENDER_CLASS;
>> +    const u32 base = engine->mmio_base;
>> +
>> +    GEM_DEBUG_EXEC(DRM_INFO_ONCE("Using GEN12 Register State 
>> Context\n"));
>> +
>> +    regs[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(13) |
>> +        MI_LRI_FORCE_POSTED;
>> +
>> +    init_common_reg_state(regs, ppgtt, engine, ring);
>> +    if (rcs)
>> +        init_wa_bb_reg_state(regs, engine, GEN12_CTX_BB_PER_CTX_PTR);
>> +
>> +    regs[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9) |
>> +        MI_LRI_FORCE_POSTED;
>> +
>> +    CTX_REG(regs, CTX_CTX_TIMESTAMP, RING_CTX_TIMESTAMP(base), 0);
>> +
>> +    init_ppgtt_reg_state(regs, base, ppgtt);
>> +
>> +    if (rcs) {
>> +        regs[GEN12_CTX_LRI_HEADER_3] = MI_LOAD_REGISTER_IMM(1);
>> +        CTX_REG(regs, GEN12_CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE,
>> +            0);
>> +
>> +        /* TODO: oa_init_reg_state ? */
>> +    }
>> +}
>> +
>> +static void execlists_init_reg_state(u32 *regs,
>> +                     struct intel_context *ce,
>> +                     struct intel_engine_cs *engine,
>> +                     struct intel_ring *ring)
>> +{
>> +    /* A context is actually a big batch buffer with several
>> +     * MI_LOAD_REGISTER_IMM commands followed by (reg, value) pairs. The
>> +     * values we are setting here are only for the first context 
>> restore:
>> +     * on a subsequent save, the GPU will recreate this batchbuffer 
>> with new
>> +     * values (including all the missing MI_LOAD_REGISTER_IMM 
>> commands that
>> +     * we are not initializing here).
>> +     */
>> +    if (INTEL_GEN(engine->i915) >= 12)
>> +        gen12_init_reg_state(regs, ce, engine, ring);
>> +    else
>> +        gen8_init_reg_state(regs, ce, engine, ring);
>> +}
>> +
>>   static int
>>   populate_lr_context(struct intel_context *ce,
>>               struct drm_i915_gem_object *ctx_obj,
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.h 
>> b/drivers/gpu/drm/i915/gt/intel_lrc.h
>> index dc0252e0589e..a4bde38e35d8 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.h
>> @@ -49,6 +49,8 @@ struct intel_engine_cs;
>>   #define      EL_CTRL_LOAD                (1 << 0)
>> +#define GEN12_ENGINE_SEMAPHORE_TOKEN(engine)    
>> _MMIO((engine)->mmio_base + 0x2b4)
>> +
> 
> As I commented on the previous revision, defining this in this patch 
> without using it or setting it in the context doesn't make much sense.
> 
>>   /* The docs specify that the write pointer wraps around after 5h, 
>> "After status
>>    * is written out to the last available status QW at offset 5h, this 
>> pointer
>>    * wraps to 0."
>> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h 
>> b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>> index b8f20ad71169..8cb6655744cd 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_lrc_reg.h
>> @@ -9,7 +9,7 @@
>>   #include <linux/types.h>
>> -/* GEN8+ Reg State Context */
>> +/* GEN8 to GEN11 Reg State Context */
>>   #define CTX_LRI_HEADER_0        0x01
>>   #define CTX_CONTEXT_CONTROL        0x02
>>   #define CTX_RING_HEAD            0x04
>> @@ -39,6 +39,11 @@
>>   #define CTX_R_PWR_CLK_STATE        0x42
>>   #define CTX_END                0x44
>> +/* GEN12+ Reg State Context */
>> +#define GEN12_CTX_BB_PER_CTX_PTR        0x12
>> +#define GEN12_CTX_LRI_HEADER_3            0x41
> 
> Gen11 has the same number and positioning of LRIs as gen12, so marking 
> this as a gen12 change doesn't feel too reflective of teh HW changes 
> IMO, but not a blocker.
> 
> 
>> +#define GEN12_CTX_R_PWR_CLK_STATE        0x42
> 
> This hasn't moved compared to gen11:
>       #define CTX_R_PWR_CLK_STATE             0x42
> 
> so no need for a different defs and all those ifs higher up in the patch.
> 
> Daniele
> 
>> +
>>   #define CTX_REG(reg_state, pos, reg, val) do { \
>>       u32 *reg_state__ = (reg_state); \
>>       const u32 pos__ = (pos); \
> 
> _______________________________________________
> 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] 6+ messages in thread

end of thread, other threads:[~2019-09-05 22:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 11:34 [PATCH] drm/i915/tgl: Register state context definition for Gen12 Mika Kuoppala
2019-09-05 13:40 ` Andi Shyti
2019-09-05 13:47 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-09-05 16:13 ` [PATCH] " Daniele Ceraolo Spurio
2019-09-05 22:48   ` Daniele Ceraolo Spurio
2019-09-05 17:05 ` ✓ Fi.CI.IGT: success for " Patchwork

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.