Re: [PATCH] drm/i915/display: Don't allow tile4 framebuffer to do hflip on display20 or greater

["Pottumuttu, Sai Teja" <sai.teja.pottumuttu@intel.com>]

On 07-10-2024 23:58, Juha-Pekka Heikkila wrote:
> On display ver 20 onwards tile4 is not supported with horizontal flip
>
> Bspec: 69853

Some nitpicks,

1. Probably, we can add Bspec 68904 as well, as it gives more clarity 
about all the support and remove the extra line below the Bspec line.

2. In the commit subject, I think we would usually prefer display 
version 20 instead of display20 directly, and maybe we can have the 
commit message something like "With display version 20, horizontal flip 
is not supported on tile4 surface formats" just to make it a bit more clear.

But other than that, patch looks good to me

Reviewed-by: Sai Teja Pottumuttu <sai.teja.pottumuttu@intel.com>

>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>   drivers/gpu/drm/i915/display/intel_fb.c            | 13 +++++++++++++
>   drivers/gpu/drm/i915/display/intel_fb.h            |  1 +
>   drivers/gpu/drm/i915/display/skl_universal_plane.c | 11 +++++++++++
>   3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index 6c2679e6c980..735fba73c9ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -439,6 +439,19 @@ bool intel_fb_needs_64k_phys(u64 modifier)
>   				      INTEL_PLANE_CAP_NEED64K_PHYS);
>   }
>   
> +/**
> + * intel_fb_is_tile4_modifier: Check if a modifier is a tile4 modifier type
> + * @modifier: Modifier to check
> + *
> + * Returns:
> + * Returns %true if @modifier is a tile4 modifier.
> + */
> +bool intel_fb_is_tile4_modifier(u64 modifier)
> +{
> +	return plane_caps_contain_any(lookup_modifier(modifier)->plane_caps,
> +				      INTEL_PLANE_CAP_TILING_4);
> +}
> +
>   static bool check_modifier_display_ver_range(const struct intel_modifier_desc *md,
>   					     u8 display_ver_from, u8 display_ver_until)
>   {
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.h b/drivers/gpu/drm/i915/display/intel_fb.h
> index 8240febff84c..d78993e5eb62 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.h
> +++ b/drivers/gpu/drm/i915/display/intel_fb.h
> @@ -36,6 +36,7 @@ bool intel_fb_is_ccs_modifier(u64 modifier);
>   bool intel_fb_is_rc_ccs_cc_modifier(u64 modifier);
>   bool intel_fb_is_mc_ccs_modifier(u64 modifier);
>   bool intel_fb_needs_64k_phys(u64 modifier);
> +bool intel_fb_is_tile4_modifier(u64 modifier);
>   
>   bool intel_fb_is_ccs_aux_plane(const struct drm_framebuffer *fb, int color_plane);
>   int intel_fb_rc_ccs_cc_plane(const struct drm_framebuffer *fb);
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 9207b7e96974..4755a0672739 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -1619,6 +1619,17 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>   		return -EINVAL;
>   	}
>   
> +	/*
> +	 * Display20 onward tile4 hflip is not supported
> +	 */
> +	if (rotation & DRM_MODE_REFLECT_X &&
> +	    intel_fb_is_tile4_modifier(fb->modifier) &&
> +	    DISPLAY_VER(dev_priv) >= 20) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "horizontal flip is not supported with tile4 surface formats\n");
> +		return -EINVAL;
> +	}
> +
>   	if (drm_rotation_90_or_270(rotation)) {
>   		if (!intel_fb_supports_90_270_rotation(to_intel_framebuffer(fb))) {
>   			drm_dbg_kms(&dev_priv->drm,