[Intel-gfx] [PATCH] drm/i915/tgl: Add Wa_1608008084

[Lucas De Marchi]

On Fri, Feb 21, 2020 at 04:36:53PM -0800, Jose Souza wrote:
>+ CCing people involved in the patch fixed.
>
>On Fri, 2020-02-21 at 16:28 -0800, Lucas De Marchi wrote:
>> Wa_1608008084 is an additional WA that applies to writes on FF_MODE2
>> register. We can't read it back either from CPU or GPU. Since the
>> other
>> bits should be 0, recommendation to handle Wa_1604555607 is to
>> actually
>> just write the timer value.
>>
>> Do a write only and don't try to read it, neither before or after
>> the WA is applied.
>>
>> Fixes: ff690b2111ba ("drm/i915/tgl: Implement Wa_1604555607")
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 22 ++++++++++---------
>> --
>>  1 file changed, 10 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> index 887e0dc701f7..0d76e1d6ec87 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> @@ -580,24 +580,22 @@ static void icl_ctx_workarounds_init(struct
>> intel_engine_cs *engine,
>>  static void tgl_ctx_workarounds_init(struct intel_engine_cs *engine,
>>  				     struct i915_wa_list *wal)
>>  {
>> -	u32 val;
>> -
>>  	/* Wa_1409142259:tgl */
>>  	WA_SET_BIT_MASKED(GEN11_COMMON_SLICE_CHICKEN3,
>>  			  GEN12_DISABLE_CPS_AWARE_COLOR_PIPE);
>>
>> -	/* Wa_1604555607:tgl */
>> -	val = intel_uncore_read(engine->uncore, FF_MODE2);
>> -	val &= ~FF_MODE2_TDS_TIMER_MASK;
>> -	val |= FF_MODE2_TDS_TIMER_128;
>>  	/*
>> -	 * FIXME: FF_MODE2 register is not readable till TGL B0. We can
>> -	 * enable verification of WA from the later steppings, which
>> enables
>> -	 * the read of FF_MODE2.
>> +	 * Wa_1604555607:gen12
>> +	 * FF_MODE2 register is not readable till TGL B0, either by CPU
>> or GPU.
>
>The line above could be removed as the comments above explain it
>better, also BSpec don't say that it will be fixed in B0.

1604555607 was documented to be fixed in B0. Just saw it's not anymore.

>
>With that:
>Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

thanks
Lucas De Marchi

>
>> +	 *
>> +	 * Wa_1608008084:gen12
>> +	 * 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 the
>> TDS timer
>> +	 * value for Wa_1604555607.
>>  	 */
>> -	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK, val,
>> -	       IS_TGL_REVID(engine->i915, TGL_REVID_A0, TGL_REVID_A0) ?
>> 0 :
>> -			    FF_MODE2_TDS_TIMER_MASK);
>> +	wa_add(wal, FF_MODE2, FF_MODE2_TDS_TIMER_MASK,
>> +	       FF_MODE2_TDS_TIMER_128, 0);
>>  }
>>
>>  static void