Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces

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

On 09/10/2025 15:12, Kamil Konieczny wrote:
> Hi Tvrtko,
> On 2025-10-03 at 14:05:17 +0100, Tvrtko Ursulin wrote:
> 
> small nit, as this is touching more than one lib, you could
> just write it short:
> 
> [PATCH i-g-t v2] lib: Use correct MOCS for displayable surfaces
> 
> No need for resend unless you will send v3 or want to retest it
> again.

I think I started with a smaller patch and forgot to change the tags 
when I refactored it.

Anyway, CI looks good AFAICT so we could merge it if everyone agrees?

I would then need to send the rebased xe series to see if it is 
completely stable without the clflush patch.

Regards,

Tvrtko

>> Using the uncached MOCS for displayable surfaces is not always correct,
>> especially when CCS compression is used with which some platforms require
>> a special uncached entry, otherwise writes get unexpectedly cached.
>>
>> Lets copy the knowledge of what is the correct MOCS for displayable
>> surfaces from Mesa and add some new helpers to get it.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>> v2:
>>   * Renamed external_index to displayable_index. (Ville)
>> ---
>>   lib/igt_fb.c       |  2 +-
>>   lib/intel_bufops.c |  2 ++
>>   lib/intel_mocs.c   | 21 +++++++++++++++++++--
>>   lib/intel_mocs.h   |  2 ++
>>   4 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
>> index 03ede3a6fa20..b5a16f9cbe90 100644
>> --- a/lib/igt_fb.c
>> +++ b/lib/igt_fb.c
>> @@ -2712,7 +2712,7 @@ igt_fb_create_intel_buf(int fd, struct buf_ops *bops,
>>   				    fb->strides[0],
>>   				    region,
>>   				    intel_get_pat_idx_uc(fd),
>> -				    DEFAULT_MOCS_INDEX);
>> +				    DISPLAYABLE_MOCS_INDEX);
>>   	intel_buf_set_name(buf, name);
>>   
>>   	/* only really needed for proper CCS handling */
>> diff --git a/lib/intel_bufops.c b/lib/intel_bufops.c
>> index 475b0d1f7b10..1196069a500f 100644
>> --- a/lib/intel_bufops.c
>> +++ b/lib/intel_bufops.c
>> @@ -1008,6 +1008,8 @@ static void __intel_buf_init(struct buf_ops *bops,
>>   	buf->pat_index = pat_index;
>>   	if (mocs_index == DEFAULT_MOCS_INDEX)
>>   		mocs_index = intel_get_uc_mocs_index(bops->fd);
>> +	else if (mocs_index == DISPLAYABLE_MOCS_INDEX)
>> +		mocs_index = intel_get_displayable_mocs_index(bops->fd);
>>   	buf->mocs_index = mocs_index;
>>   	IGT_INIT_LIST_HEAD(&buf->link);
>>   
>> diff --git a/lib/intel_mocs.c b/lib/intel_mocs.c
>> index e0c33c31c088..9809e32113eb 100644
>> --- a/lib/intel_mocs.c
>> +++ b/lib/intel_mocs.c
>> @@ -9,12 +9,14 @@
>>   struct drm_intel_mocs_index {
>>   	uint8_t uc_index;
>>   	uint8_t wb_index;
>> +	uint8_t displayable_index;
>>   	uint8_t defer_to_pat_index;
>>   };
>>   
>>   static void get_mocs_index(int fd, struct drm_intel_mocs_index *mocs)
>>   {
>>   	uint16_t devid = intel_get_drm_devid(fd);
>> +	unsigned int ip_ver = intel_graphics_ver(devid);
>>   
>>   	/*
>>   	 * Gen >= 12 onwards don't have a setting for PTE,
>> @@ -23,25 +25,31 @@ static void get_mocs_index(int fd, struct drm_intel_mocs_index *mocs)
>>   	 * This helper function is providing current UC as well
>>   	 * as WB MOCS index based on platform.
>>   	 */
>> -	if (intel_graphics_ver(devid) >= IP_VER(20, 0)) {
>> +	if (ip_ver >= IP_VER(20, 0)) {
>>   		mocs->uc_index = 3;
>>   		mocs->wb_index = 4;
>> +		mocs->displayable_index = 1;
>>   		mocs->defer_to_pat_index = 0;
>>   	} else if (IS_METEORLAKE(devid)) {
>>   		mocs->uc_index = 5;
>>   		mocs->wb_index = 1;
>> +		mocs->displayable_index = 14;
>>   	} else if (IS_DG2(devid)) {
>>   		mocs->uc_index = 1;
>>   		mocs->wb_index = 3;
>> +		mocs->displayable_index = 3;
>>   	} else if (IS_DG1(devid)) {
>>   		mocs->uc_index = 1;
>>   		mocs->wb_index = 5;
>> -	} else if (IS_GEN12(devid)) {
>> +		mocs->displayable_index = 5;
>> +	} else if (ip_ver >= IP_VER(12, 0)) {
>>   		mocs->uc_index = 3;
>>   		mocs->wb_index = 2;
>> +		mocs->displayable_index = 61;
>>   	} else {
>>   		mocs->uc_index = I915_MOCS_PTE;
>>   		mocs->wb_index = I915_MOCS_CACHED;
>> +		mocs->displayable_index = I915_MOCS_PTE;
>>   	}
>>   }
>>   
>> @@ -63,6 +71,15 @@ uint8_t intel_get_uc_mocs_index(int fd)
>>   	return mocs.uc_index;
>>   }
>>   
>> +uint8_t intel_get_displayable_mocs_index(int fd)
>> +{
>> +	struct drm_intel_mocs_index mocs;
>> +
>> +	get_mocs_index(fd, &mocs);
>> +
>> +	return mocs.displayable_index;
>> +}
>> +
>>   uint8_t intel_get_defer_to_pat_mocs_index(int fd)
>>   {
>>   	struct drm_intel_mocs_index mocs;
>> diff --git a/lib/intel_mocs.h b/lib/intel_mocs.h
>> index 8597286d259d..394bb41be042 100644
>> --- a/lib/intel_mocs.h
>> +++ b/lib/intel_mocs.h
>> @@ -9,9 +9,11 @@
>>   #include <stdint.h>
>>   
>>   #define DEFAULT_MOCS_INDEX ((uint8_t)-1)
>> +#define DISPLAYABLE_MOCS_INDEX ((uint8_t)-2)
>>   
>>   uint8_t intel_get_wb_mocs_index(int fd);
>>   uint8_t intel_get_uc_mocs_index(int fd);
>> +uint8_t intel_get_displayable_mocs_index(int fd);
>>   uint8_t intel_get_defer_to_pat_mocs_index(int fd);
>>   
>>   #endif /* _INTEL_MOCS_H */
>> -- 
>> 2.48.0
>>