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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Mon, 06 May 2024, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> 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.

Right. The old one was just so hard to read. :/

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
>> 
>> 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

-- 
Jani Nikula, Intel