On Mon, 06 May 2024, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Mon, May 06, 2024 at 05:03:59PM +0300, Jani Nikula wrote:
>> On Mon, 06 May 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
>> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>> >
>> > 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@xxxxxxxxxxxxxxx>
>> > ---
>> > .../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@xxxxxxxxx>
>
>>
>> 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
