Re: [PATCH] drm/i915: Fix remapped stride with CCS on ADL+

[Imre Deak <imre.deak@intel.com>]

On Tue, Dec 05, 2023 at 08:03:08PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On ADL+ the hardware automagically calculates the CCS AUX surface
> stride from the main surface stride, so when remapping we can't
> really play a lot of tricks with the main surface stride, or else
> the AUX surface stride would get miscalculated and no longer
> match the actual data layout in memory.
> 
> Supposedly we could remap in 256 main surface tile units
> (AUX page(4096)/cachline(64)*4(4x1 main surface tiles per
> AUX cacheline)=256 main surface tiles), but the extra complexity
> is probably not worth the hassle.
> 
> So let's just make sure our mapping stride is calculated from
> the full framebuffer stride (instead of the framebuffer width).
> This way the stride we program into PLANE_STRIDE will be the
> original framebuffer stride, and thus there will be no change
> to the AUX stride/layout.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index ab634a4c86d1..9f35bdce3eb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -1509,8 +1509,20 @@ static u32 calc_plane_remap_info(const struct intel_framebuffer *fb, int color_p
>  
>  			size += remap_info->size;
>  		} else {
> -			unsigned int dst_stride = plane_view_dst_stride_tiles(fb, color_plane,
> -									      remap_info->width);
> +			unsigned int dst_stride;
> +
> +			/*
> +			 * The hardware automagically calculates the CCS AUX surface
> +			 * stride from the main surface stride so can't really remap a
> +			 * smaller subset (unless we'd remap in whole AUX page units).
> +			 */
> +			if (intel_fb_needs_pot_stride_remap(fb) &&

This fix also applies at least to all !FLAT_CSS platforms. Since
the stride remapping is disabled anyway on all platforms for CCS
modifiers, the same should be done here as well?

> +			    intel_fb_is_ccs_modifier(fb->base.modifier))
> +				dst_stride = remap_info->src_stride;
> +			else
> +				dst_stride = remap_info->width;
> +
> +			dst_stride = plane_view_dst_stride_tiles(fb, color_plane, dst_stride);
>  
>  			assign_chk_ovf(i915, remap_info->dst_stride, dst_stride);
>  			color_plane_info->mapping_stride = dst_stride *
> -- 
> 2.41.0
>