Re: [PATCH 2/9] drm/i915: Clean up skl+ plane stride limits

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, May 06, 2024 at 05:03:59PM +0300, Jani Nikula wrote:
> On Mon, 06 May 2024, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > skl_plane_max_stride() is pretty messy. Streamline it and
> > split it into clear skl+ vs. adl+ variants.
> >
> > TODO: Deal with icl and tgl strude limits properly
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  .../drm/i915/display/skl_universal_plane.c    | 65 +++++++++++--------
> >  1 file changed, 37 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > index 0a8e781a3648..b8103d6ebc1f 100644
> > --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> > @@ -461,41 +461,46 @@ static int icl_plane_max_height(const struct drm_framebuffer *fb,
> >  }
> >  
> >  static unsigned int
> > -skl_plane_max_stride(struct intel_plane *plane,
> > -		     u32 pixel_format, u64 modifier,
> > -		     unsigned int rotation)
> > +plane_max_stride(struct intel_plane *plane,
> > +		 u32 pixel_format, u64 modifier,
> > +		 unsigned int rotation,
> > +		 unsigned int max_pixels,
> > +		 unsigned int max_bytes)
> >  {
> > -	struct drm_i915_private *i915 = to_i915(plane->base.dev);
> >  	const struct drm_format_info *info = drm_format_info(pixel_format);
> >  	int cpp = info->cpp[0];
> > -	int max_horizontal_pixels = 8192;
> > -	int max_stride_bytes;
> > -
> > -	if (DISPLAY_VER(i915) >= 13) {
> > -		/*
> > -		 * The stride in bytes must not exceed of the size
> > -		 * of 128K bytes. For pixel formats of 64bpp will allow
> > -		 * for a 16K pixel surface.
> > -		 */
> > -		max_stride_bytes = 131072;
> > -		if (cpp == 8)
> > -			max_horizontal_pixels = 16384;
> 
> The commit message doesn't mention anything about this being dropped.

16k pixels * 8 cpp == 128k bytes, so it's completely redundant.

> 
> BR,
> Jani.
> 
> > -		else
> > -			max_horizontal_pixels = 65536;
> > -	} else {
> > -		/*
> > -		 * "The stride in bytes must not exceed the
> > -		 * of the size of 8K pixels and 32K bytes."
> > -		 */
> > -		max_stride_bytes = 32768;
> > -	}
> >  
> >  	if (drm_rotation_90_or_270(rotation))
> > -		return min(max_horizontal_pixels, max_stride_bytes / cpp);
> > +		return min(max_pixels, max_bytes / cpp);
> >  	else
> > -		return min(max_horizontal_pixels * cpp, max_stride_bytes);
> > +		return min(max_pixels * cpp, max_bytes);
> >  }
> >  
> > +static unsigned int
> > +adl_plane_max_stride(struct intel_plane *plane,
> > +		     u32 pixel_format, u64 modifier,
> > +		     unsigned int rotation)
> > +{
> > +	unsigned int max_pixels = 65536; /* PLANE_OFFSET limit */
> > +	unsigned int max_bytes = 128 * 1024;
> > +
> > +	return plane_max_stride(plane, pixel_format,
> > +				modifier, rotation,
> > +				max_pixels, max_bytes);
> > +}
> > +
> > +static unsigned int
> > +skl_plane_max_stride(struct intel_plane *plane,
> > +		     u32 pixel_format, u64 modifier,
> > +		     unsigned int rotation)
> > +{
> > +	unsigned int max_pixels = 8192; /* PLANE_OFFSET limit */
> > +	unsigned int max_bytes = 32 * 1024;
> > +
> > +	return plane_max_stride(plane, pixel_format,
> > +				modifier, rotation,
> > +				max_pixels, max_bytes);
> > +}
> >  
> >  /* Preoffset values for YUV to RGB Conversion */
> >  #define PREOFF_YUV_TO_RGB_HI		0x1800
> > @@ -2357,7 +2362,11 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
> >  		plane->min_cdclk = skl_plane_min_cdclk;
> >  	}
> >  
> > -	plane->max_stride = skl_plane_max_stride;
> > +	if (DISPLAY_VER(dev_priv) >= 13)
> > +		plane->max_stride = adl_plane_max_stride;
> > +	else
> > +		plane->max_stride = skl_plane_max_stride;
> > +
> >  	if (DISPLAY_VER(dev_priv) >= 11) {
> >  		plane->update_noarm = icl_plane_update_noarm;
> >  		plane->update_arm = icl_plane_update_arm;
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel