[PATCH v5 6/7] drm/xe/xelp: Implement Wa_16010904313

[Tvrtko Ursulin]

On 26/06/2025 16:01, Lucas De Marchi wrote:
> On Thu, Jun 26, 2025 at 09:57:05AM +0100, Tvrtko Ursulin wrote:
>>
>> On 25/06/2025 22:47, Lucas De Marchi wrote:
>>> On Wed, Jun 25, 2025 at 04:36:31PM +0100, Tvrtko Ursulin wrote:
>>>> Add XeLP workaround 16010904313.
>>>>
>>>> The description calls for it to be emitted as the indirect context 
>>>> buffer
>>>> workaround for render and compute, and from the workaround batch buffer
>>>> for the other engines. Therefore we plug into the previously added
>>>> respective top level emission functions.
>>>>
>>>> The actual command streamer programming sequence differs from what is
>>>> described in the PRM, in that it assumes the listed LRCA offset was
>>>> supposed to actually refer to the location of the CTX_TIMESTAMP 
>>>> register
>>>> instead of LRCA + 0x180c (which is in GPR space). Latter appears to 
>>>> make
>>>> more sense under the assumption that multiple writes are helping with
>>>> restoring the CTX_TIMESTAMP register content from the saved context 
>>>> state.
>>>
>>> humn... but i915, which is supposedly doing the right thing,
>>> diverges from the PRM by using LRM + LRR + LRR. Here you are doing
>>
>> Matt reported i915 was doing the wrong thing actually, compared to 
>> what was documented in the WA database at least. Not that the latter 
>> was completely clear, but we had a brief guessing game and I 
>> understood the conclusion was 3x LRM to forcefully restore 
>> CTX_TIMESTAMP is what made most sense.
>>
>> See https://lore.kernel.org/intel-xe/20250523174135.GS5080@mdroper- 
>> desk1.amr.corp.intel.com/.
>>
>>> something different from both. Does gputop show saner values after this?
>>
>> Gputop looks okay to me. Are you saying this WA is fixing something is 
>> regularly goes wrong so could be easily observed? I thought it was 
>> about a rare failure to restore CTX_TIMESTAMP but that was just a 
>> guess to be fair.
> 
> yes, it doesn't really work in TGL.... it just keeps reporting ~0% or
> 100%, rather than something sane like it does in DG2 and later.

Hmm.. TGL, with i915, so execlists? I don't think gputop depends on the 
CTX_TIMESTMAP register there.

Are you sure 16010904313 was created for this?

> We could go the other way around for testing: do the way you have it
> here in i915 and then check gputop if we start seeing the same thing.
> Then at least we will know if this is really the case instead of
> debugging the addition of indirect ctx.

I also need the indirect context for the AuxCCS invalidations. That's in 
a separate series which is waiting for this one to get sorted first.

So something trivial must be wrong in this one, but annoyingly it only 
effects LNL and BMG in the shards and haven't spotted the problem yet.

Regards,

Tvrtko