Re: [PATCH] drm/xe/display: mark dpt as uncached

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

On 14/03/2024 16:20, Juha-Pekka Heikkila wrote:
> make dpt as uncached to avoid pipe faults on some devices

s/make/Mark/

Also missing full stop.

> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

Did this help on ADL btw? I don't think there are any PAT bits in the 
GGTT on that hw so this just noops, right? Or am I misremembering...

In addition to this I think we might also want something like:
https://patchwork.freedesktop.org/series/131148/

> ---
>   drivers/gpu/drm/xe/display/xe_fb_pin.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index 722c84a56607..98592994c8d4 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -30,7 +30,7 @@ write_dpt_rotated(struct xe_bo *bo, struct iosys_map *map, u32 *dpt_ofs, u32 bo_
>   
>   		for (row = 0; row < height; row++) {
>   			u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx * XE_PAGE_SIZE,
> -							      xe->pat.idx[XE_CACHE_WB]);
> +							      xe->pat.idx[XE_CACHE_NONE]);
>   
>   			iosys_map_wr(map, *dpt_ofs, u64, pte);
>   			*dpt_ofs += 8;
> @@ -62,7 +62,7 @@ write_dpt_remapped(struct xe_bo *bo, struct iosys_map *map, u32 *dpt_ofs,
>   		for (column = 0; column < width; column++) {
>   			iosys_map_wr(map, *dpt_ofs, u64,
>   				     pte_encode_bo(bo, src_idx * XE_PAGE_SIZE,
> -				     xe->pat.idx[XE_CACHE_WB]));
> +				     xe->pat.idx[XE_CACHE_NONE]));
>   
>   			*dpt_ofs += 8;
>   			src_idx++;
> @@ -119,7 +119,7 @@ static int __xe_pin_fb_vma_dpt(struct intel_framebuffer *fb,
>   
>   		for (x = 0; x < size / XE_PAGE_SIZE; x++) {
>   			u64 pte = ggtt->pt_ops->pte_encode_bo(bo, x * XE_PAGE_SIZE,
> -							      xe->pat.idx[XE_CACHE_WB]);
> +							      xe->pat.idx[XE_CACHE_NONE]);
>   
>   			iosys_map_wr(&dpt->vmap, x * 8, u64, pte);
>   		}
> @@ -165,7 +165,7 @@ write_ggtt_rotated(struct xe_bo *bo, struct xe_ggtt *ggtt, u32 *ggtt_ofs, u32 bo
>   
>   		for (row = 0; row < height; row++) {
>   			u64 pte = ggtt->pt_ops->pte_encode_bo(bo, src_idx * XE_PAGE_SIZE,
> -							      xe->pat.idx[XE_CACHE_WB]);
> +							      xe->pat.idx[XE_CACHE_NONE]);
>   
>   			xe_ggtt_set_pte(ggtt, *ggtt_ofs, pte);
>   			*ggtt_ofs += XE_PAGE_SIZE;
> @@ -211,7 +211,7 @@ static int __xe_pin_fb_vma_ggtt(struct intel_framebuffer *fb,
>   
>   		for (x = 0; x < size; x += XE_PAGE_SIZE) {
>   			u64 pte = ggtt->pt_ops->pte_encode_bo(bo, x,
> -							      xe->pat.idx[XE_CACHE_WB]);
> +							      xe->pat.idx[XE_CACHE_NONE]);

This looks unrelated to DPT? Maybe tweak the commit title/message.

Anyway, I think change looks reasonable since you usually don't want to 
mess around with caching for display stuff.

With commit title/message tweaked,
Reviewed-by: Matthew Auld <matthew.auld@intel.com>

>   
>   			xe_ggtt_set_pte(ggtt, vma->node.start + x, pte);
>   		}