Re: [PATCH] drm/xe: Merge 16021540221 and 18034896535 WAs

["Chauhan, Shekhar" <shekhar.chauhan@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]


On 4/29/2024 18:42, Lucas De Marchi wrote:
> On Mon, Apr 29, 2024 at 09:14:30AM GMT, Chauhan, Shekhar wrote:
>>
>> On 4/27/2024 19:23, Lucas De Marchi wrote:
>>> In order to detect duplicate implementations for the same workaround,
>>> early in the implementation of RTP it was decided to error out even if
>>> the values set are exactly the same. With the introduction of 
>>> 18034896535
>>> in commit 74671d23ca18 ("drm/xe/xe2: Add workaround 18034896535"), LNL
>>> stepping with graphics stepping A1 now gives the following error on
>>> module load:
>>>
>>>     xe 0000:00:02.0: [drm] *ERROR* GT0: [GT OTHER] \
>>>     discarding save-restore reg e48c (clear: 00000200, set: 00000200,\
>>>     masked: yes, mcr: yes): ret=-22
>>>
>>> RTP may be improved in the future, but for now simply join the entries
>>> like done with e.g. "1607297627, 1607030317, 1607186500".
>>>
>>> Fixes: 74671d23ca18 ("drm/xe/xe2: Add workaround 18034896535")
>>> Cc: Bommu Krishnaiah <krishnaiah.bommu@intel.com>
>>> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>> ---
>>>  drivers/gpu/drm/xe/xe_wa.c | 7 +------
>>>  1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_wa.c b/drivers/gpu/drm/xe/xe_wa.c
>>> index 9d9b7fa7a8f0..db7c7c7875c5 100644
>>> --- a/drivers/gpu/drm/xe/xe_wa.c
>>> +++ b/drivers/gpu/drm/xe/xe_wa.c
>>> @@ -449,12 +449,7 @@ static const struct xe_rtp_entry_sr 
>>> engine_was[] = {
>>>        XE_RTP_RULES(GRAPHICS_VERSION(2004), 
>>> FUNC(xe_rtp_match_first_render_or_compute)),
>>>        XE_RTP_ACTIONS(SET(ROW_CHICKEN3, XE2_EUPEND_CHK_FLUSH_DIS))
>>>      },
>>> -    { XE_RTP_NAME("16021540221"),
>>> -      XE_RTP_RULES(GRAPHICS_VERSION(2004), GRAPHICS_STEP(A0, B0),
>>> -               FUNC(xe_rtp_match_first_render_or_compute)),
>>> -      XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>>> -    },
>>> -    { XE_RTP_NAME("18034896535"),
>>> +    { XE_RTP_NAME("18034896535, 16021540221"), /* 16021540221: 
>>> GRAPHICS_STEP(A0, B0) */
>>>        XE_RTP_RULES(GRAPHICS_VERSION_RANGE(2001, 2004),
>>
>> w/a 16021540221 is applicable to only Steppings A0 to B0 (B0 not 
>> inclusive). With your change, I think the w/a will be applied to all 
>> the steppings. Wouldn't this be incorrect? Please provide some guidance.
>
> both 18034896535 and 16021540221 have the same action:
>
>     SET(ROW_CHICKEN4, DISABLE_TDL_PUSH)
>
> In other words, 18034896535 applies to a superset of platforms
> 16021540221 applies to. So even when we remove support for steppings A*,
> we want to keep the WA (possibly removing the comment about 16021540221
> and renaming it).

Okay, this makes sense. Thanks.

/-shekhar/

>
> Lucas De Marchi
>
>>
>>> FUNC(xe_rtp_match_first_render_or_compute)),
>>>        XE_RTP_ACTIONS(SET(ROW_CHICKEN4, DISABLE_TDL_PUSH))
>>
>> -- 
>> -shekhar
>>
-- 
-shekhar

[-- Attachment #2: Type: text/html, Size: 5831 bytes --]