Re: [PATCH v7 12/28] media: rockchip: rga: avoid odd frame sizes for YUV formats

[Michael Tretter <m.tretter@pengutronix.de>]

On Thu, 21 May 2026 00:44:17 +0200, Sven Püschel wrote:
> Avoid odd frame sizes for YUV formats, as they may cause undefined
> behavior. This is done in preparation for the RGA3, which hangs when the
> output format is set to 129x129 pixel YUV420 SP (NV12).
> 
> This requirement is documented explicitly for the RGA3 in  section 5.6.3
> of the RK3588 TRM Part 2. For the RGA2 the RK3588 TRM Part 2
> (section 6.1.2) and RK3568 TRM Part 2 (section 14.2) only mentions the
> x/y offsets and stride aligning requirements. But the vendor driver for
> the RGA2 also contains checks for the width and height to be aligned to
> 2 bytes.
> 
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index f599c992829dd..77b8c7ab74274 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -337,6 +337,19 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  	struct rga_ctx *ctx = file_to_rga_ctx(file);
>  	const struct rga_hw *hw = ctx->rga->hw;
>  	struct rga_fmt *fmt;
> +	struct v4l2_frmsize_stepwise frmsize = {
> +		.min_width = hw->min_width,
> +		.max_width = hw->max_width,
> +		.min_height = hw->min_height,
> +		.max_height = hw->max_height,
> +		.step_width = 1,
> +		.step_height = 1,
> +	};
> +
> +	if (v4l2_is_format_yuv(v4l2_format_info(pix_fmt->pixelformat))) {
> +		frmsize.step_width = 2;
> +		frmsize.step_height = 2;

If I understand correctly, this limitation may be a result of 4:2:0
chroma subsampling. Thus, formats with 4:2:2 subsampling would also work
with step_height = 1.

As this may be some hardware limitation, a comment that points to the
TRM (in addition to the commit message) may be beneficial, too.

Michael

> +	}
>  
>  	if (V4L2_TYPE_IS_CAPTURE(f->type)) {
>  		const struct rga_frame *frm;
> @@ -358,11 +371,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  	if (!fmt)
>  		fmt = &hw->formats[0];
>  
> -	pix_fmt->width = clamp(pix_fmt->width,
> -			       hw->min_width, hw->max_width);
> -	pix_fmt->height = clamp(pix_fmt->height,
> -				hw->min_height, hw->max_height);
> -
> +	v4l2_apply_frmsize_constraints(&pix_fmt->width, &pix_fmt->height, &frmsize);
>  	v4l2_fill_pixfmt_mp(pix_fmt, fmt->fourcc, pix_fmt->width, pix_fmt->height);
>  	pix_fmt->field = V4L2_FIELD_NONE;
>  
> 
> -- 
> 2.54.0
> 
>