Re: [PATCH 3/4] drm/xe/xe2: Limit ccs framebuffers to tile4 only

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org
Subject: Re: [PATCH 3/4] drm/xe/xe2: Limit ccs framebuffers to tile4 only
Date: Wed, 31 Jan 2024 13:40:30 +0200	[thread overview]
Message-ID: <ZboxrobbUOShNriY@intel.com> (raw)
In-Reply-To: <20240130193652.374270-4-juhapekka.heikkila@gmail.com>

On Tue, Jan 30, 2024 at 09:36:51PM +0200, Juha-Pekka Heikkila wrote:
> Display engine support ccs only with tile4, prevent other modifiers
> from using compressed memory.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  drivers/gpu/drm/xe/display/xe_fb_pin.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index 722c84a56607..fab0871f0cdf 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -10,9 +10,18 @@
>  #include "intel_fb_pin.h"
>  #include "xe_ggtt.h"
>  #include "xe_gt.h"
> +#include "xe_pat.h"
>  
>  #include <drm/ttm/ttm_bo.h>
>  
> +static bool is_compressed(const struct drm_framebuffer *fb)
> +{
> +	struct xe_bo *bo = intel_fb_obj(fb);
> +	struct xe_device *xe = to_xe_device(to_intel_framebuffer(fb)->base.dev);
> +
> +	return xe_pat_index_has_compression(xe, bo->pat_index);
> +}
> +
>  static void
>  write_dpt_rotated(struct xe_bo *bo, struct iosys_map *map, u32 *dpt_ofs, u32 bo_ofs,
>  		  u32 width, u32 height, u32 src_stride, u32 dst_stride)
> @@ -283,6 +292,17 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
>  	if (ret)
>  		goto err;
>  
> +	if (GRAPHICS_VER(xe) >= 20) {
> +		if (fb->base.modifier != I915_FORMAT_MOD_4_TILED &&
> +		    is_compressed(&fb->base)) {
> +			drm_warn(&xe->drm, "Cannot create ccs framebuffer with other than tile4 mofifier\n");
> +			ttm_bo_unreserve(&bo->ttm);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +		bo->has_sealed_pat_index = true;

That needs to happen when the FB is created, otherwise 
someone can come in afterwards and still frob the PAT
and then you're left with a FB that can not be used.

> +	}
> +
>  	if (IS_DGFX(xe))
>  		ret = xe_bo_migrate(bo, XE_PL_VRAM0);
>  	else
> @@ -308,6 +328,7 @@ static struct i915_vma *__xe_pin_fb_vma(struct intel_framebuffer *fb,
>  	ttm_bo_unpin(&bo->ttm);
>  	ttm_bo_unreserve(&bo->ttm);
>  err:
> +	bo->has_sealed_pat_index = false;
>  	kfree(vma);
>  	return ERR_PTR(ret);
>  }
> @@ -323,6 +344,8 @@ static void __xe_unpin_fb_vma(struct i915_vma *vma)
>  		 vma->bo->ggtt_node.start != vma->node.start)
>  		xe_ggtt_remove_node(ggtt, &vma->node);
>  
> +	vma->bo->has_sealed_pat_index = false;
> +
>  	ttm_bo_reserve(&vma->bo->ttm, false, false, NULL);
>  	ttm_bo_unpin(&vma->bo->ttm);
>  	ttm_bo_unreserve(&vma->bo->ttm);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel