[v3,2/7] drm/i915/gt: Clear all bits from GEN12_FF_MODE2

Commit Message

Lucas De Marchi June 30, 2023, 8:35 p.m. UTC

Right now context workarounds don't do a rmw and instead only write to
the register. Since 2 separate programmings to the same register are
coalesced into a single write, this is not problematic for
GEN12_FF_MODE2 since both TDS and GS timer are going to be written
together and the other remaining bits be zeroed.

However in order to fix other workarounds that may want to preserve the
unrelated bits in the same register, context workarounds need to
be changed to a rmw. To prepare for that, move the programming of
GEN12_FF_MODE2 to a single place so the value passed for "clear" can
be all the bits. Otherwise the second workaround would be dropped as
it'd be detected as overwriting a previously programmed workaround.

Signed-off-by: Lucas De Marchi <[e-mail]>
Reviewed-by: Matt Roper <[e-mail]>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 51 +++++++--------------
 1 file changed, 17 insertions(+), 34 deletions(-)

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 8f8346df3c18..7d48bd57b6ef 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -693,40 +693,11 @@  static void dg2_ctx_gt_tuning_init(struct intel_engine_cs *engine,
 		   0, false);
 }
 
-/*
- * These settings aren't actually workarounds, but general tuning settings that
- * need to be programmed on several platforms.
- */
-static void gen12_ctx_gt_tuning_init(struct intel_engine_cs *engine,
-				     struct i915_wa_list *wal)
-{
-	/*
-	 * Although some platforms refer to it as Wa_1604555607, we need to
-	 * program it even on those that don't explicitly list that
-	 * workaround.
-	 *
-	 * Note that the programming of this register is further modified
-	 * according to the FF_MODE2 guidance given by Wa_1608008084:gen12.
-	 * Wa_1608008084 tells us the FF_MODE2 register will return the wrong
-	 * value when read. The default value for this register is zero for all
-	 * fields and there are no bit masks. So instead of doing a RMW we
-	 * should just write TDS timer value. For the same reason read
-	 * verification is ignored.
-	 */
-	wa_add(wal,
-	       GEN12_FF_MODE2,
-	       FF_MODE2_TDS_TIMER_MASK,
-	       FF_MODE2_TDS_TIMER_128,
-	       0, false);
-}
-
 static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
 				       struct i915_wa_list *wal)
 {
 	struct drm_i915_private *i915 = engine->i915;
 
-	gen12_ctx_gt_tuning_init(engine, wal);
-
 	/*
 	 * Wa_1409142259:tgl,dg1,adl-p
 	 * Wa_1409347922:tgl,dg1,adl-p
@@ -748,15 +719,27 @@  static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
 			    GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
 
 	/*
-	 * Wa_16011163337
+	 * Wa_16011163337 - GS_TIMER
+	 *
+	 * TDS_TIMER: Although some platforms refer to it as Wa_1604555607, we
+	 * need to program it even on those that don't explicitly list that
+	 * workaround.
+	 *
+	 * Note that the programming of GEN12_FF_MODE2 is further modified
+	 * according to the FF_MODE2 guidance given by Wa_1608008084.
+	 * Wa_1608008084 tells us the FF_MODE2 register will return the wrong
+	 * value when read from the CPU.
 	 *
-	 * Like in gen12_ctx_gt_tuning_init(), read verification is ignored due
-	 * to Wa_1608008084.
+	 * The default value for this register is zero for all fields.
+	 * So instead of doing a RMW we should just write the desired values
+	 * for TDS and GS timers. Note that since the readback can't be trusted,
+	 * the clear mask is just set to ~0 to make sure other bits are not
+	 * inadvertently set. For the same reason read verification is ignored.
 	 */
 	wa_add(wal,
 	       GEN12_FF_MODE2,
-	       FF_MODE2_GS_TIMER_MASK,
-	       FF_MODE2_GS_TIMER_224,
+	       ~0,
+	       FF_MODE2_TDS_TIMER_128 | FF_MODE2_GS_TIMER_224,
 	       0, false);
 
 	if (!IS_DG1(i915)) {