On Sat, Jun 24, 2023 at 10:17:53AM -0700, Lucas De Marchi wrote: > 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 <[email protected]>
Reviewed-by: Matt Roper <[email protected]> > --- > drivers/gpu/drm/i915/gt/intel_workarounds.c | 51 +++++++-------------- > 1 file changed, 17 insertions(+), 34 deletions(-) > > 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)) { > -- > 2.40.1 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
