Re: [PATCH i-g-t] tests/intel/kms_ccs: stop using l3 enabled PAT index

[Matthew Auld <matthew.auld@intel.com>]

On 09/09/2025 08:23, Zbigniew Kempczyński wrote:
> On Mon, Sep 08, 2025 at 07:20:46PM +0100, Matthew Auld wrote:
>> When populating the fb using an l3 compression enabled PAT index some or
>> all of the data may end up cached. However the HW looks to only perform
>> the compression step in this case upon evicting those entries, when also
>> trying to write them back to VRAM. This seems to be the cause of this
>> test randomly failing on BMG with the CCS state appearing to sometimes
>> be zeroed even after doing a compression enabled copy. From
>> experimentation adding a sleep before the surface copy cures the
>> failure, which fits since this will give plenty of time to enter rc6
>> which will indirectly also nuke the l3.  Grabbing a forcewake around the
>> sleep brings back the failure which also makes sense since this will
>> inhibit the flush and also rules out missing synchronisation hidden by
>> the sleep. Using a large fb also cures the issue, which also fits since
>> the fb will now be larger than the entire l3, so some data will have to
>> be compressed when evicted.
>>
>> To fix this don't use an l3 enabled PAT index prior to taking a snapshot
>> of the raw CCS state. Probably this also means the test is maybe too
>> much looking at implementation details, by assuming that zeroed CCS
>> state must also imply that there is no compression, even if compression
>> is merely delayed until the data is evicted.
> 
> Display is reading directly from vram, so IIUC at moment of scanout
> to properly decompress fb it should have an access to flushed surface

Right, it might be that some modes allow scanout with compression so it 
might also be able to read CCS, so no need to decompress, but not 
completely sure.

> and its CCS data, am I right? Blit to WT surface with delayed flush of
> CCS data creates such race - WT surface is flushed, but CCS is not.

I don't think it's a delayed flush of CCS, but rather the compressor 
stage is delayed until actually writing data to VRAM. At the point of 
writing to VRAM I guess the compressor makes the decision on whether to 
write to CSS or VRAM, depending on whether stuff is compressible? So if 
you peek at the CCS state too early you might not see it in some cases. 
I think that makes sense?

Also I'm not sure if the WT thing actually works as expected, since the 
WT seems to only apply to l4, and not l3, according to BSpec, but AFAIK 
there is no l4 on BMG. So I think index 15 and 11 are perhaps the same 
on BMG?

> What causes access_flat_ccs_surface() called before cache flush is a
> problem and there's no problem with scanout? For me access (regardless
> surf-copy or scanout) looks similar, only timing may vary.

Right, display is not coherent (goes directly to VRAM) so you can't use 
standard l3 caching, but you can use l3:xd, which is the transient 
display version, and those special cache entries will be flushed by KMD 
when doing the display flip, so before the scanout happens. And 
flushing/evicting looks to trigger compression as needed, if cached.

So for scanout there is an existing transient flush to handle l3:xd, but 
for access_flat_ccs_surface(), there is no guaranteed flush anywhere, 
unless you get "lucky" with rc6, or the fb is somehow larger than the 
l2, in which you should be guaranteed to see at least some compression, 
since something will have to get evicted.

> 
> I don't understand why WT in this case causes delayed CCS flush,
> where UC_COMP is not. Does UC_COMP applies to CCS data as well?
> 
> --
> Zbigniew
> 
>>
>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5941
>> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/5376
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
>> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   tests/intel/kms_ccs.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/intel/kms_ccs.c b/tests/intel/kms_ccs.c
>> index cb0c80f03..ab081aa75 100644
>> --- a/tests/intel/kms_ccs.c
>> +++ b/tests/intel/kms_ccs.c
>> @@ -812,7 +812,7 @@ static struct igt_fb *get_fb(data_t *data, u64 modifier, double r, double g,
>>   		fb = get_fb(data, modifier, r, g, b, width, height,
>>   			    data->format);
>>   
>> -		igt_xe2_blit_with_dst_pat(fb, temp_fb, intel_get_pat_idx_wt(fb->fd));
>> +		igt_xe2_blit_with_dst_pat(fb, temp_fb, intel_get_pat_idx_uc_comp(fb->fd));
>>   		access_flat_ccs_surface(fb, true);
>>   		return fb;
>>   
>> -- 
>> 2.51.0
>>