diff mbox series

i915/gt: Move context wa to appropriate *_ctx_workarounds_init

Message ID a3oztkscewrfopetalmu65jaouxhxdmiu2fqi4pdbysxyg22kb@iqgeqy7zohvr (mailing list archive)
State New
Headers show
Series i915/gt: Move context wa to appropriate *_ctx_workarounds_init | expand

Commit Message

Sebastian Brzezinka March 19, 2025, 11:53 a.m. UTC 
According to bspecs, CACHE_MODE_0 and  CACHE_MODE_1 are saved and
restored as part of Context. Let's move it to `ctx_workaround_init` instead
of `rcs_engine_wa_init`, this way workarounds are applied and saved into
the default context.

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 68 ++++++++++-----------
 1 file changed, 34 insertions(+), 34 deletions(-)

Comments

Krzysztof Karas March 21, 2025, 11:53 a.m. UTC | #1 
Hi Sebastian,

> According to bspecs, CACHE_MODE_0 and  CACHE_MODE_1 are saved and
> restored as part of Context. Let's move it to `ctx_workaround_init` instead
> of `rcs_engine_wa_init`, this way workarounds are applied and saved into
> the default context.

These flags are already used a bit inconsistently in this file.
Before this patch CACHE_MODE_0_GEN7 was used in
gen8_ctx_workarounds_init() to intialize the default context,
while all remaining usages were in rcs_engine_wa_init().

After your patch the inconsistency still remains for Haswell and
for Ivybridge, but to remedy that we'd probably need to
introduce new functions "*_ctx_workarounds_init()" for those
gens to have their workarounds included during default context
initialization.

So the thing to consider here is whether we prefer these WAs to
be associated with context or RCS engine functions and commit to
one of the approaches.

Best Regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 116683ebe074..e7969a4a7af6 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -337,12 +337,37 @@  static void gen6_ctx_workarounds_init(struct intel_engine_cs *engine,
 				      struct i915_wa_list *wal)
 {
 	wa_masked_en(wal, INSTPM, INSTPM_FORCE_ORDERING);
+
+	/* WaDisable_RenderCache_OperationalFlush:snb */
+	wa_masked_dis(wal, CACHE_MODE_0, RC_OP_FLUSH_ENABLE);
+
+	/*
+	 * From the Sandybridge PRM, volume 1 part 3, page 24:
+	 * "If this bit is set, STCunit will have LRA as replacement
+	 *  policy. [...] This bit must be reset. LRA replacement
+	 *  policy is not supported."
+	 */
+	wa_masked_dis(wal,
+		      CACHE_MODE_0,
+		      CM0_STC_EVICT_DISABLE_LRA_SNB);
 }
 
 static void gen7_ctx_workarounds_init(struct intel_engine_cs *engine,
 				      struct i915_wa_list *wal)
 {
 	wa_masked_en(wal, INSTPM, INSTPM_FORCE_ORDERING);
+
+	/* WaDisable_RenderCache_OperationalFlush:ivb,vlv,hsw */
+	wa_masked_dis(wal, CACHE_MODE_0_GEN7, RC_OP_FLUSH_ENABLE);
+
+	/*
+	 * BSpec says this must be set, even though
+	 * WaDisable4x2SubspanOptimization:ivb,hsw
+	 * WaDisable4x2SubspanOptimization isn't listed for VLV.
+	 */
+	wa_masked_en(wal,
+		     CACHE_MODE_1,
+		     PIXEL_SUBSPAN_COLLECT_OPT_DISABLE);
 }
 
 static void gen8_ctx_workarounds_init(struct intel_engine_cs *engine,
@@ -669,6 +694,15 @@  static void icl_ctx_workarounds_init(struct intel_engine_cs *engine,
 
 	/* Wa_1406306137:icl,ehl */
 	wa_mcr_masked_en(wal, GEN9_ROW_CHICKEN4, GEN11_DIS_PICK_2ND_EU);
+
+	if (IS_JASPERLAKE(engine->i915) || IS_ELKHARTLAKE(engine->i915)) {
+		/*
+		 * "Disable Repacking for Compression (masked R/W access)
+		 *  before rendering compressed surfaces for display."
+		 */
+		wa_masked_en(wal, CACHE_MODE_0_GEN7,
+			     DISABLE_REPACKING_FOR_COMPRESSION);
+	}
 }
 
 /*
@@ -2306,15 +2340,6 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			     GEN8_RC_SEMA_IDLE_MSG_DISABLE);
 	}
 
-	if (IS_JASPERLAKE(i915) || IS_ELKHARTLAKE(i915)) {
-		/*
-		 * "Disable Repacking for Compression (masked R/W access)
-		 *  before rendering compressed surfaces for display."
-		 */
-		wa_masked_en(wal, CACHE_MODE_0_GEN7,
-			     DISABLE_REPACKING_FOR_COMPRESSION);
-	}
-
 	if (GRAPHICS_VER(i915) == 11) {
 		/* This is not an Wa. Enable for better image quality */
 		wa_masked_en(wal,
@@ -2565,18 +2590,6 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 			     RING_MODE_GEN7(RENDER_RING_BASE),
 			     GFX_TLB_INVALIDATE_EXPLICIT | GFX_REPLAY_MODE);
 
-		/* WaDisable_RenderCache_OperationalFlush:ivb,vlv,hsw */
-		wa_masked_dis(wal, CACHE_MODE_0_GEN7, RC_OP_FLUSH_ENABLE);
-
-		/*
-		 * BSpec says this must be set, even though
-		 * WaDisable4x2SubspanOptimization:ivb,hsw
-		 * WaDisable4x2SubspanOptimization isn't listed for VLV.
-		 */
-		wa_masked_en(wal,
-			     CACHE_MODE_1,
-			     PIXEL_SUBSPAN_COLLECT_OPT_DISABLE);
-
 		/*
 		 * BSpec recommends 8x4 when MSAA is used,
 		 * however in practice 16x4 seems fastest.
@@ -2642,19 +2655,6 @@  rcs_engine_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal)
 				    GEN6_GT_MODE,
 				    GEN6_WIZ_HASHING_MASK,
 				    GEN6_WIZ_HASHING_16x4);
-
-		/* WaDisable_RenderCache_OperationalFlush:snb */
-		wa_masked_dis(wal, CACHE_MODE_0, RC_OP_FLUSH_ENABLE);
-
-		/*
-		 * From the Sandybridge PRM, volume 1 part 3, page 24:
-		 * "If this bit is set, STCunit will have LRA as replacement
-		 *  policy. [...] This bit must be reset. LRA replacement
-		 *  policy is not supported."
-		 */
-		wa_masked_dis(wal,
-			      CACHE_MODE_0,
-			      CM0_STC_EVICT_DISABLE_LRA_SNB);
 	}
 
 	if (IS_GRAPHICS_VER(i915, 4, 6))