Re: [PATCH v12 11/13] drm/xe: Force flush system memory AuxCCS framebuffers before scan out

[Tvrtko Ursulin <tvrtko.ursulin@igalia.com>]

On 02/10/2025 15:01, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> On 26/09/2025 20:35, Ville Syrjälä wrote:
>> On Fri, Sep 26, 2025 at 10:41:56AM +0300, Ville Syrjälä wrote:
>>> I reverse engineered this a bit and there's definitely a
>>> MOCS issue at play.
>>>
>>> First I noticed that if filled the entire MOCS table with
>>> UC the problem went away. I then filled the entire table
>>> with WB and essentially bisected what I need to make UC
>>> to fix it. And I had to repeat that same process starting
>>> from the other end of table.
>>>
>>> Looks like there is some undocumented magic in the hardware.
>>>
>>> MOCS 61 really is special:
>>> - MOCS 61 UC, others WB, select MOCS 61 -> no corruption
>>>
>>> MOCS 0 and 63 are special in other ways:
>>> - MOCS X UC, others WB, select MOCS X -> corruption
>>> - MOCS X+0 UC, others WB, select MOCS X -> corruption
>>> - MOCS X+63 UC, others WB, select MOCS X -> corruption
>>> - MOCS X+0+63 UC, others WB, select MOCS X -> no corruption
>>>    where X != 61
>>
>> OK, the MOCS 63 issue was caused by me having L3=WB still in
>> MOCS X. If I change MOCS X to L3=UC, MOCS 63 no longer makes
>> a difference. I suppose that means MOCS 63 is still used for
>> L3 evictions, even though bspec no longer mentions that fact
>> explicitly.
>>
>> So MOCS 0 is the thing that really matters for CCS. And for
>> MOCS 0 only the LLC WB vs. UC selection matters. L3 WB vs. UC
>> doesn't seem to make any difference.
>>
>> It's interesting that MOCS 60 is documented as a "CCS special case",
>> but in reality it's MOCS 0 that matters for CCS. I wonder if some
>> wires got crossed in the hw design and the wrong MOCS entry ended
>> up being used for CCS and no one noticed...
> 
> Oh wow, that is an amazing discovery!
> 
> I verified it on my end too. Setting MOCS 0 to uncached and cache dirt 
> is gone. No need to the explicit cache flush patch on first pin.
> 
> Luckily ADL is unsupported so we could change it to UC. I will send a 
> series for CI to see what it will say.

Doh, but it would be bad to change it for all uses which actually do not 
want UC. Sorry did not wake up fully... So assuming the test CI runs 
will be all clear it will still be a difficult situation on what to do.

Regards,

Tvrtko