[Intel-gfx] [PATCH 2/7] drm/i915/tgl: Make sure a semiplanar UV plane is tile row size aligned

[Kahola, Mika]

On Wed, 2020-01-01 at 01:37 +0200, Imre Deak wrote:
> Currently the GGTT offset of a UV plane in a semiplanar YUV FB is
> tile
> size (4kB) aligned. I noticed, that enforcing only this alignment
> leads
> oddly to random memory corruptions on TGL while scanning out Y-tiled
> FBs. This issue can be easily reproduced with a UV plane offset that
> is
> not aligned to the plane's tile row size.
> 
> Some experiments showed the correct alignment to be tile row size
> indeed. This also makes sense, since the de-tiling fence created for
> the
> object - with its own stride and so "left" and "right" edge - applies
> to
> all the planes in the FB, so each tile row of all planes should be
> tile
> row aligned.
> 
> In fact BSpec requires this alignment since SKL. On SKL we may
> enforce
> this due to the AUX plane x,y coords check, but on ICL and TGL we
> don't.
> For now enforce this only on TGL; I can follow up with any necessary
> change for ICL after more tests.
> 
> BSpec requires a stricter alignment for linear UV planes too (kind of
> a
> tile row alignment), but it's unclear whether that's really needed
> (couldn't be explained with the de-tiling fence as above) and
> enforcing
> that could break existing user space; so avoid that too for now until
> more tests.
> 
> v2:
> - Clarify the commit log wrt. the address space the alignment applies
> to.
>   (Chris)
> 
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> Acked-by: Chris Wilson <chris at chris-wilson.co.uk>

Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 36
> ++++++++++++++++++--
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 6e4152770c15..bbc9cf288067 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -1995,6 +1995,13 @@ intel_format_info_is_yuv_semiplanar(const
> struct drm_format_info *info,
>  	       info->num_planes == (is_ccs_modifier(modifier) ? 4 : 2);
>  }
>  
> +static 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;
> +}
> +
>  static unsigned int
>  intel_tile_width_bytes(const struct drm_framebuffer *fb, int
> color_plane)
>  {
> @@ -2069,6 +2076,16 @@ static void intel_tile_dims(const struct
> drm_framebuffer *fb, int color_plane,
>  	*tile_height = intel_tile_height(fb, color_plane);
>  }
>  
> +static unsigned int intel_tile_row_size(const struct drm_framebuffer
> *fb,
> +					int color_plane)
> +{
> +	unsigned int tile_width, tile_height;
> +
> +	intel_tile_dims(fb, color_plane, &tile_width, &tile_height);
> +
> +	return fb->pitches[color_plane] * tile_height;
> +}
> +
>  unsigned int
>  intel_fb_align_height(const struct drm_framebuffer *fb,
>  		      int color_plane, unsigned int height)
> @@ -2143,7 +2160,8 @@ static 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 (is_aux_plane(fb, color_plane))
> +	if ((INTEL_GEN(dev_priv) < 12 && is_aux_plane(fb, color_plane))
> ||
> +	    is_ccs_plane(fb, color_plane))
>  		return 4096;
>  
>  	switch (fb->modifier) {
> @@ -2158,6 +2176,10 @@ static unsigned int intel_surf_alignment(const
> struct drm_framebuffer *fb,
>  	case I915_FORMAT_MOD_Y_TILED_CCS:
>  	case I915_FORMAT_MOD_Yf_TILED_CCS:
>  	case I915_FORMAT_MOD_Y_TILED:
> +		if (INTEL_GEN(dev_priv) >= 12 &&
> +		    is_semiplanar_uv_plane(fb, color_plane))
> +			return intel_tile_row_size(fb, color_plane);
> +		/* Fall-through */
>  	case I915_FORMAT_MOD_Yf_TILED:
>  		return 1 * 1024 * 1024;
>  	default:
> @@ -2505,9 +2527,17 @@ static int intel_fb_offset_to_xy(int *x, int
> *y,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(fb->dev);
>  	unsigned int height;
> +	u32 alignment;
> +
> +	if (INTEL_GEN(dev_priv) >= 12 &&
> +	    is_semiplanar_uv_plane(fb, color_plane))
> +		alignment = intel_tile_row_size(fb, color_plane);
> +	else if (fb->modifier != DRM_FORMAT_MOD_LINEAR)
> +		alignment = intel_tile_size(dev_priv);
> +	else
> +		alignment = 0;
>  
> -	if (fb->modifier != DRM_FORMAT_MOD_LINEAR &&
> -	    fb->offsets[color_plane] % intel_tile_size(dev_priv)) {
> +	if (alignment != 0 && fb->offsets[color_plane] % alignment) {
>  		DRM_DEBUG_KMS("Misaligned offset 0x%08x for color plane
> %d\n",
>  			      fb->offsets[color_plane], color_plane);
>  		return -EINVAL;