[Intel-gfx] [PATCH v2 2/6] drm/i915/gt: Clear all bits from GEN12_FF_MODE2

[Matt Roper]

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 at intel.com>

Reviewed-by: Matt Roper <matthew.d.roper at 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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation