diff mbox series

[9/9] drm/i915: Nuke the TGL+ chroma plane tile row alignment stuff

Message ID 20240513175942.12910-10-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Polish plane surface alignment handling | expand

Commit Message

Ville Syrjala May 13, 2024, 5:59 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I don't think the display hardware really has such chroma
plane tile row alignment requirements as outlined in
commit d156135e6a54 ("drm/i915/tgl: Make sure a semiplanar
UV plane is tile row size aligned")

Bspec had the same exact thing to say about earlier hardware
as well, but we never cared and things work just fine.

The one thing mentioned in that commit that is definitely
true however is the fence alignment issue. But we don't
deal with that on earlier hardware either. We do have code
to deal with that issue for the first color plane, but not
the chroma planes. So I think if we did want to check this
more extensively we should do it in the same places where
we already check the first color plane (namely
convert_plane_offset_to_xy() and intel_fb_bo_framebuffer_init()).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_fb.c            | 12 +-----------
 drivers/gpu/drm/i915/display/intel_fb.h            |  1 -
 drivers/gpu/drm/i915/display/skl_universal_plane.c | 11 -----------
 3 files changed, 1 insertion(+), 23 deletions(-)

Comments

Imre Deak May 28, 2024, 2 p.m. UTC | #1
On Mon, May 13, 2024 at 08:59:42PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> I don't think the display hardware really has such chroma
> plane tile row alignment requirements as outlined in
> commit d156135e6a54 ("drm/i915/tgl: Make sure a semiplanar
> UV plane is tile row size aligned")
> 
> Bspec had the same exact thing to say about earlier hardware
> as well, but we never cared and things work just fine.
> 
> The one thing mentioned in that commit that is definitely
> true however is the fence alignment issue. But we don't
> deal with that on earlier hardware either. We do have code
> to deal with that issue for the first color plane, but not
> the chroma planes. So I think if we did want to check this
> more extensively we should do it in the same places where
> we already check the first color plane (namely
> convert_plane_offset_to_xy() and intel_fb_bo_framebuffer_init()).

Imo a correct alignment should be required to help users figure out why
a given config doesn't work (even if an incorrect alignment doesn't
cause other HW issues). But agreed that it should be the same then for
all platforms, so ok to remove it in its current form.

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb.c            | 12 +-----------
>  drivers/gpu/drm/i915/display/intel_fb.h            |  1 -
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 11 -----------
>  3 files changed, 1 insertion(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index c80f866f3fb6..fc18da3106fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -584,12 +584,6 @@ static bool is_gen12_ccs_cc_plane(const struct drm_framebuffer *fb, int color_pl
>  	return intel_fb_rc_ccs_cc_plane(fb) == color_plane;
>  }
>  
> -bool is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane)
> -{
> -	return intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
> -		color_plane == 1;
> -}
> -
>  bool is_surface_linear(const struct drm_framebuffer *fb, int color_plane)
>  {
>  	return fb->modifier == DRM_FORMAT_MOD_LINEAR ||
> @@ -1019,11 +1013,7 @@ static int intel_fb_offset_to_xy(int *x, int *y,
>  	struct drm_i915_private *i915 = to_i915(fb->dev);
>  	unsigned int height, alignment, unused;
>  
> -	if (DISPLAY_VER(i915) >= 12 &&
> -	    !intel_fb_needs_pot_stride_remap(to_intel_framebuffer(fb)) &&
> -	    is_semiplanar_uv_plane(fb, color_plane))
> -		alignment = intel_tile_row_size(fb, color_plane);
> -	else if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
> +	if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
>  		alignment = intel_tile_size(i915);
>  	else
>  		alignment = 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.h b/drivers/gpu/drm/i915/display/intel_fb.h
> index 1b1fef2dc39a..6dee0c8b7f22 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.h
> +++ b/drivers/gpu/drm/i915/display/intel_fb.h
> @@ -34,7 +34,6 @@ 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 is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane);
>  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 ca7fc9fae990..476f5b7d9497 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -514,17 +514,6 @@ static u32 tgl_plane_min_alignment(struct intel_plane *plane,
>  	if (intel_fb_is_ccs_aux_plane(fb, color_plane))
>  		return mult * 4 * 1024;
>  
> -	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 (fb->modifier == DRM_FORMAT_MOD_LINEAR)
> -			return 256 * 1024;
> -
> -		return intel_tile_row_size(fb, color_plane);
> -	}

The above will also use the correct 2MB for DPT, which the previous
patch should've kept already. Other than that looks ok:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> -
>  	switch (fb->modifier) {
>  	case DRM_FORMAT_MOD_LINEAR:
>  	case I915_FORMAT_MOD_X_TILED:
> -- 
> 2.43.2
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index c80f866f3fb6..fc18da3106fd 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -584,12 +584,6 @@  static bool is_gen12_ccs_cc_plane(const struct drm_framebuffer *fb, int color_pl
 	return intel_fb_rc_ccs_cc_plane(fb) == color_plane;
 }
 
-bool is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane)
-{
-	return intel_format_info_is_yuv_semiplanar(fb->format, fb->modifier) &&
-		color_plane == 1;
-}
-
 bool is_surface_linear(const struct drm_framebuffer *fb, int color_plane)
 {
 	return fb->modifier == DRM_FORMAT_MOD_LINEAR ||
@@ -1019,11 +1013,7 @@  static int intel_fb_offset_to_xy(int *x, int *y,
 	struct drm_i915_private *i915 = to_i915(fb->dev);
 	unsigned int height, alignment, unused;
 
-	if (DISPLAY_VER(i915) >= 12 &&
-	    !intel_fb_needs_pot_stride_remap(to_intel_framebuffer(fb)) &&
-	    is_semiplanar_uv_plane(fb, color_plane))
-		alignment = intel_tile_row_size(fb, color_plane);
-	else if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
+	if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
 		alignment = intel_tile_size(i915);
 	else
 		alignment = 0;
diff --git a/drivers/gpu/drm/i915/display/intel_fb.h b/drivers/gpu/drm/i915/display/intel_fb.h
index 1b1fef2dc39a..6dee0c8b7f22 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.h
+++ b/drivers/gpu/drm/i915/display/intel_fb.h
@@ -34,7 +34,6 @@  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 is_semiplanar_uv_plane(const struct drm_framebuffer *fb, int color_plane);
 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 ca7fc9fae990..476f5b7d9497 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -514,17 +514,6 @@  static u32 tgl_plane_min_alignment(struct intel_plane *plane,
 	if (intel_fb_is_ccs_aux_plane(fb, color_plane))
 		return mult * 4 * 1024;
 
-	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 (fb->modifier == DRM_FORMAT_MOD_LINEAR)
-			return 256 * 1024;
-
-		return intel_tile_row_size(fb, color_plane);
-	}
-
 	switch (fb->modifier) {
 	case DRM_FORMAT_MOD_LINEAR:
 	case I915_FORMAT_MOD_X_TILED: