[PATCH 2/5] drm/xe/gen12: Add Wa_1604555607

[Tvrtko Ursulin]

On 29/01/2025 18:12, Lucas De Marchi wrote:
> On Wed, Jan 29, 2025 at 03:56:24PM +0000, Tvrtko Ursulin wrote:
>> According to the i915 code base, apart from setting the GS timer, all
>> Gen12 platforms should also set the TDS timer. Make it so.
> 
> I have a faint memory of that actually being wrong and got removed
> in cedbc0b75790 ("drm/xe: Fix duplicated setting for register 0x6604")
> while fixing another bug.

Hm cedbc0b75790 does not really explain why it removed Wa_1604555607. It 
wasn't simply a duplicate setting as that commit explains, but the same 
register with different bits set (which is also not visible from the log 
pasted in the commit.)

I think a definitive answer is needed to whether the TDS part is 
applicable to Gen12 or not. If not we need to remove it from i915 as well.

But again, the comment added in 2b5298b0aa09 suggests someone got the 
information from somewhere that TDS tuning is needed even on platforms 
which don't explicitly list that WA.

Regards,

Tvrtko

> The TDS remains for DG2:
> 
>          { XE_RTP_NAME("Tuning: TDS gang timer"),
>            XE_RTP_RULES(PLATFORM(DG2), ENGINE_CLASS(RENDER)),
>            /* read verification is ignored as in i915 - need to check 
> enabling */
>            XE_RTP_ACTIONS(FIELD_SET_NO_READ_MASK(XEHP_FF_MODE2,
>                                                  FF_MODE2_TDS_TIMER_MASK,
>                                                  FF_MODE2_TDS_TIMER_128))
>          },
> 
> Lucas De Marchi
> 
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at igalia.com>
>> References: 2b5298b0aa09 ("drm/i915/gen12: Add recommended hardware 
>> tuning value")
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> Cc: Matt Roper <matthew.d.roper at intel.com>
>> ---
>> This needs checking by someone with access to the proper docs and proper
>> workaround database.
>> ---
>> drivers/gpu/drm/xe/xe_tuning.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_tuning.c 
>> b/drivers/gpu/drm/xe/xe_tuning.c
>> index d449de0fb6ec..9e7ec36dbd47 100644
>> --- a/drivers/gpu/drm/xe/xe_tuning.c
>> +++ b/drivers/gpu/drm/xe/xe_tuning.c
>> @@ -97,12 +97,14 @@ static const struct xe_rtp_entry_sr 
>> engine_tunings[] = {
>> };
>>
>> static const struct xe_rtp_entry_sr lrc_tunings[] = {
>> -    { XE_RTP_NAME("Tuning: ganged timer, also known as 16011163337"),
>> +    { XE_RTP_NAME("Tuning: ganged timers, also known as 16011163337 
>> and 1604555607"),
>>       XE_RTP_RULES(GRAPHICS_VERSION_RANGE(1200, 1210), 
>> ENGINE_CLASS(RENDER)),
>>       /* read verification is ignored due to 1608008084. */
>>       XE_RTP_ACTIONS(FIELD_SET_NO_READ_MASK(FF_MODE2,
>> -                        FF_MODE2_GS_TIMER_MASK,
>> -                        FF_MODE2_GS_TIMER_224))
>> +                        FF_MODE2_GS_TIMER_MASK |
>> +                        FF_MODE2_TDS_TIMER_MASK,
>> +                        FF_MODE2_GS_TIMER_224 |
>> +                        FF_MODE2_TDS_TIMER_128))
> 
> this got removed in commit cedbc0b75790 ("drm/xe: Fix duplicated setting
> for register 0x6604")
> 
> 
>>     },
>>
>>     /* DG2 */
>> -- 
>> 2.48.0
>>