| Message ID | 20230624171757.3906095-3-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix ctx workarounds for non-masked regs | expand |
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 <lucas.demarchi@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > 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 >
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)) {
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 <lucas.demarchi@intel.com> --- drivers/gpu/drm/i915/gt/intel_workarounds.c | 51 +++++++-------------- 1 file changed, 17 insertions(+), 34 deletions(-)