Re: [Intel-gfx] [PATCH v2] drm/i915: Simplify CCS and UV plane alignment handling

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

look ok to me.

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

On 21.4.2021 20.32, Imre Deak wrote:
> We can handle the surface alignment of CCS and UV color planes for all
> modifiers at one place, so do this. An AUX color plane can be a CCS or a
> UV plane, use only the more specific query functions and remove
> is_aux_plane() becoming redundant.
> 
> While at it add a TODO for linear UV color plane alignments. The spec
> requires this to be stride-in-bytes * 64 on all platforms, whereas the
> driver uses an alignment of 4k for gen<12 and 256k for gen>=12 for
> linear UV planes.
> 
> v2:
> - Restore previous alignment for linear UV surfaces.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display.c | 27 +++++++++++++-------
>   drivers/gpu/drm/i915/display/intel_fb.c      |  8 ------
>   drivers/gpu/drm/i915/display/intel_fb.h      |  1 -
>   3 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index a10e26380ef3d..e246e5cf75866 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -973,10 +973,26 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>   	struct drm_i915_private *dev_priv = to_i915(fb->dev);
>   
>   	/* AUX_DIST needs only 4K alignment */
> -	if ((DISPLAY_VER(dev_priv) < 12 && is_aux_plane(fb, color_plane)) ||
> -	    is_ccs_plane(fb, color_plane))
> +	if (is_ccs_plane(fb, color_plane))
>   		return 4096;
>   
> +	if (is_semiplanar_uv_plane(fb, color_plane)) {
> +		/*
> +		 * TODO: cross-check wrt. the bspec stride in bytes * 64 bytes
> +		 * alignment for linear UV planes on all platforms.
> +		 */
> +		if (DISPLAY_VER(dev_priv) >= 12) {
> +			if (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> +				return intel_linear_alignment(dev_priv);
> +
> +			return intel_tile_row_size(fb, color_plane);
> +		}
> +
> +		return 4096;
> +	}
> +
> +	drm_WARN_ON(&dev_priv->drm, color_plane != 0);
> +
>   	switch (fb->modifier) {
>   	case DRM_FORMAT_MOD_LINEAR:
>   		return intel_linear_alignment(dev_priv);
> @@ -985,19 +1001,12 @@ unsigned int intel_surf_alignment(const struct drm_framebuffer *fb,
>   			return 256 * 1024;
>   		return 0;
>   	case I915_FORMAT_MOD_Y_TILED_GEN12_MC_CCS:
> -		if (is_semiplanar_uv_plane(fb, color_plane))
> -			return intel_tile_row_size(fb, color_plane);
> -		fallthrough;
>   	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS:
>   	case I915_FORMAT_MOD_Y_TILED_GEN12_RC_CCS_CC:
>   		return 16 * 1024;
>   	case I915_FORMAT_MOD_Y_TILED_CCS:
>   	case I915_FORMAT_MOD_Yf_TILED_CCS:
>   	case I915_FORMAT_MOD_Y_TILED:
> -		if (DISPLAY_VER(dev_priv) >= 12 &&
> -		    is_semiplanar_uv_plane(fb, color_plane))
> -			return intel_tile_row_size(fb, color_plane);
> -		fallthrough;
>   	case I915_FORMAT_MOD_Yf_TILED:
>   		return 1 * 1024 * 1024;
>   	default:
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index 0ec9ad7220a14..c8aaca3e79e97 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -30,14 +30,6 @@ bool is_gen12_ccs_cc_plane(const struct drm_framebuffer *fb, int plane)
>   	       plane == 2;
>   }
>   
> -bool is_aux_plane(const struct drm_framebuffer *fb, int plane)
> -{
> -	if (is_ccs_modifier(fb->modifier))
> -		return is_ccs_plane(fb, plane);
> -
> -	return plane == 1;
> -}
> -
>   bool is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane)
>   {
>   	return intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.h b/drivers/gpu/drm/i915/display/intel_fb.h
> index 6acf792a8c44a..13244ec1ad214 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.h
> +++ b/drivers/gpu/drm/i915/display/intel_fb.h
> @@ -19,7 +19,6 @@ struct intel_plane_state;
>   bool is_ccs_plane(const struct drm_framebuffer *fb, int plane);
>   bool is_gen12_ccs_plane(const struct drm_framebuffer *fb, int plane);
>   bool is_gen12_ccs_cc_plane(const struct drm_framebuffer *fb, int plane);
> -bool is_aux_plane(const struct drm_framebuffer *fb, int plane);
>   bool is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane);
>   
>   bool is_surface_linear(const struct drm_framebuffer *fb, int color_plane);
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx