* [PATCH i-g-t] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
@ 2025-10-03 8:37 Tvrtko Ursulin
2025-10-03 11:20 ` Ville Syrjälä
0 siblings, 1 reply; 2+ messages in thread
From: Tvrtko Ursulin @ 2025-10-03 8:37 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>
---
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..eefe83cf2116 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 external_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->external_index = 1;
mocs->defer_to_pat_index = 0;
} else if (IS_METEORLAKE(devid)) {
mocs->uc_index = 5;
mocs->wb_index = 1;
+ mocs->external_index = 14;
} else if (IS_DG2(devid)) {
mocs->uc_index = 1;
mocs->wb_index = 3;
+ mocs->external_index = 3;
} else if (IS_DG1(devid)) {
mocs->uc_index = 1;
mocs->wb_index = 5;
- } else if (IS_GEN12(devid)) {
+ mocs->external_index = 5;
+ } else if (ip_ver >= IP_VER(12, 0)) {
mocs->uc_index = 3;
mocs->wb_index = 2;
+ mocs->external_index = 61;
} else {
mocs->uc_index = I915_MOCS_PTE;
mocs->wb_index = I915_MOCS_CACHED;
+ mocs->external_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.external_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] 2+ messages in thread
* Re: [PATCH i-g-t] lib/igt_fb/intel: Use correct MOCS for displayable surfaces
2025-10-03 8:37 [PATCH i-g-t] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
@ 2025-10-03 11:20 ` Ville Syrjälä
0 siblings, 0 replies; 2+ messages in thread
From: Ville Syrjälä @ 2025-10-03 11:20 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: intel-xe, intel-gfx, igt-dev
On Fri, Oct 03, 2025 at 09:37:46AM +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>
> ---
> 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..eefe83cf2116 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 external_index;
"external" doesn't mean anything for igt, so I would
just call it "displayable" or something.
> 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->external_index = 1;
> mocs->defer_to_pat_index = 0;
> } else if (IS_METEORLAKE(devid)) {
> mocs->uc_index = 5;
> mocs->wb_index = 1;
> + mocs->external_index = 14;
> } else if (IS_DG2(devid)) {
> mocs->uc_index = 1;
> mocs->wb_index = 3;
> + mocs->external_index = 3;
> } else if (IS_DG1(devid)) {
> mocs->uc_index = 1;
> mocs->wb_index = 5;
> - } else if (IS_GEN12(devid)) {
> + mocs->external_index = 5;
> + } else if (ip_ver >= IP_VER(12, 0)) {
> mocs->uc_index = 3;
> mocs->wb_index = 2;
> + mocs->external_index = 61;
> } else {
> mocs->uc_index = I915_MOCS_PTE;
> mocs->wb_index = I915_MOCS_CACHED;
> + mocs->external_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.external_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)
Why do we even have these weird values? Ie. why can't we just
always provide the correct MOCS from the start?
And I really think this should handle *all* the platforms the
same way, but currently the MOCS for older platforms is handled
in some completely different way :(
But I suppose neither of those issues should be addressed in
this patch.
>
> 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] 2+ messages in thread
end of thread, other threads:[~2025-10-03 11:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 8:37 [PATCH i-g-t] lib/igt_fb/intel: Use correct MOCS for displayable surfaces Tvrtko Ursulin
2025-10-03 11:20 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox