Re: [PATCH] drm/xe/xe2lpg: Extend Wa_14020338487

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

On 4/18/2024 04:05, Lucas De Marchi wrote:
> On Wed, Apr 17, 2024 at 06:45:47PM GMT, Gustavo Sousa wrote:
>> Quoting Gustavo Sousa (2024-04-17 18:25:01-03:00)
>>> Wa_14020338487 also applies to Xe2_LPG. Replicate the existing entry to
>>> one specific for Xe2_LPG.
>>
>> I would also like to take this as an opportunity to discuss the way we
>> are currently arranging the RTP entries for the workaround. Using this
>> one as example, created a copy of the entry and edited the argument of
>> GRAPHICS_VERSION() to match Xe2_LPG. There are multiple cases already
>> following the same pattern, mainly because we are grouping entries by
>> IP release.
>>
>> Do we want to continue following that pattern and keep the code
>> duplication? Or should we think of a way to avoid code duplication here?
>>
>> A very simple approach that I think of is having a single entry for each
>> lineage. But I guess that's not really feasible today because I guess we
>> do not have a way of expressing logical disjunction in XE_RTP_RULES().
>
> yes, implementing it was always something I considered, but then there
> was also the fact that when we have WAs that are on IPs that are not
> close to each other we may have subtle differences like registers with
> different offset or mcr vs non-mcr.

I see that some registers vary between MCR and non-MCR types. However, 
could you please explain how this difference affects implementation? As 
I understand it, when setting XE_RTP_ACTIONS, we simply choose the 
desired action (SET, CLR etc). So, I'm curious about the specific impact 
of MCR vs non-MCR in this context.

>
> There's also the implementation side: implementing the OR probably means
> increasing the size for every item so some of them can have ORed rules.
>
> The other option that I was considering was to eventually split
> the rules from the actions and follow a similar scheme to what was done
> in drivers/gpu/drm/xe/xe_wa_oob.rules (but generating a table rather
> than the current xe_wa.oob.c). The positive side is that we free
> ourselves from having to implement things in CPP. I'd probably gone
> that direction if xe_wa_oob had been done before the rest.
>
> Lucas De Marchi

-- 
-shekhar