* [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
@ 2025-10-03 13:05 Tvrtko Ursulin
2025-10-09 14:12 ` Kamil Konieczny
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2025-10-03 13:05 UTC (permalink / raw)
To: intel-xe, intel-gfx, igt-dev; +Cc: Tvrtko Ursulin, Ville Syrjälä
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-03 13:05 [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
@ 2025-10-09 14:12 ` Kamil Konieczny
2025-10-10 8:29 ` Tvrtko Ursulin
2025-10-17 11:29 ` Ville Syrjälä
2025-10-20 9:39 ` Kamil Konieczny
2 siblings, 1 reply; 7+ messages in thread
From: Kamil Konieczny @ 2025-10-09 14:12 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-xe, intel-gfx, igt-dev, Ville Syrjälä
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.
Regards,
Kamil
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-09 14:12 ` Kamil Konieczny
@ 2025-10-10 8:29 ` Tvrtko Ursulin
2025-10-17 7:51 ` Tvrtko Ursulin
0 siblings, 1 reply; 7+ messages in thread
From: Tvrtko Ursulin @ 2025-10-10 8:29 UTC (permalink / raw)
To: Kamil Konieczny, intel-xe, intel-gfx, igt-dev,
Ville Syrjälä
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
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-10 8:29 ` Tvrtko Ursulin
@ 2025-10-17 7:51 ` Tvrtko Ursulin
0 siblings, 0 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2025-10-17 7:51 UTC (permalink / raw)
To: Kamil Konieczny, intel-xe, intel-gfx, igt-dev,
Ville Syrjälä
Gentle ping on this one - primarily to Ville - do you think we can land
this so that we can have some kernel series CI runs against it?
Regards,
Tvrtko
On 10/10/2025 09:29, Tvrtko Ursulin wrote:
>
> 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
>>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-03 13:05 [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
2025-10-09 14:12 ` Kamil Konieczny
@ 2025-10-17 11:29 ` Ville Syrjälä
2025-10-17 13:44 ` Tvrtko Ursulin
2025-10-20 9:39 ` Kamil Konieczny
2 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2025-10-17 11:29 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-xe, intel-gfx, igt-dev
On Fri, Oct 03, 2025 at 02:05:17PM +0100, Tvrtko Ursulin wrote:
> 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>
The numbers looked all right to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
There's a bunch of other MOCS stuff under tests/ and I'm not sure
the chosen numbers there make complete sense. Someone might want
to have a good look at those at some point. A few examples:
- kms_ccs uses UC MOCS for some flat CCS stuff even though it
needs to deal with display coherency, but I'm not sure if there's
any significant difference between the UC vs. displayable MOCS
entry on flat CCS platforms
- gem_ccs uses UC even though the data is only accessed by the CPU,
but then again I'm not sure what the deal is these days with the
whole LLC/L4 vs. L3 vs. snooping stuff on new platforms
> ---
> 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
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-17 11:29 ` Ville Syrjälä
@ 2025-10-17 13:44 ` Tvrtko Ursulin
0 siblings, 0 replies; 7+ messages in thread
From: Tvrtko Ursulin @ 2025-10-17 13:44 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: intel-xe, intel-gfx, igt-dev
On 17/10/2025 12:29, Ville Syrjälä wrote:
> On Fri, Oct 03, 2025 at 02:05:17PM +0100, Tvrtko Ursulin wrote:
>> 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>
>
> The numbers looked all right to me.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Thanks! I've pushed it and will keep an eye on the results.
> There's a bunch of other MOCS stuff under tests/ and I'm not sure
> the chosen numbers there make complete sense. Someone might want
> to have a good look at those at some point. A few examples:
>
> - kms_ccs uses UC MOCS for some flat CCS stuff even though it
> needs to deal with display coherency, but I'm not sure if there's
> any significant difference between the UC vs. displayable MOCS
> entry on flat CCS platforms
>
> - gem_ccs uses UC even though the data is only accessed by the CPU,
> but then again I'm not sure what the deal is these days with the
> whole LLC/L4 vs. L3 vs. snooping stuff on new platforms
I'll have a brief look in case I spot something obvious. But as flat ccs
is something I haven't worked with so far, likewise the new platforms,
it may not be a very effective look.
Regards,
Tvrtko
>> ---
>> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-03 13:05 [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
2025-10-09 14:12 ` Kamil Konieczny
2025-10-17 11:29 ` Ville Syrjälä
@ 2025-10-20 9:39 ` Kamil Konieczny
2 siblings, 0 replies; 7+ messages in thread
From: Kamil Konieczny @ 2025-10-20 9:39 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-xe, intel-gfx, igt-dev, Ville Syrjälä
Hi Tvrtko,
On 2025-10-03 at 14:05:17 +0100, Tvrtko Ursulin wrote:
> 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);
>
[cut]
> @@ -63,6 +71,15 @@ uint8_t intel_get_uc_mocs_index(int fd)
> return mocs.uc_index;
> }
>
All new lib functions needs to be documented. As this was
already merged maybe someone could document a bunch of
these similar functions here.
Regards,
Kamil
> +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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-20 9:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 13:05 [PATCH i-g-t v2] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
2025-10-09 14:12 ` Kamil Konieczny
2025-10-10 8:29 ` Tvrtko Ursulin
2025-10-17 7:51 ` Tvrtko Ursulin
2025-10-17 11:29 ` Ville Syrjälä
2025-10-17 13:44 ` Tvrtko Ursulin
2025-10-20 9:39 ` Kamil Konieczny
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox