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 <[email protected]>
> Suggested-by: Ville Syrjälä <[email protected]>
> ---
> 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
> 

Reply via email to