Re: [PATCH v3 05/27] media: v4l2-common: add v4l2_fill_pixfmt_mp_aligned helper

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 6084 bytes --]

Le mardi 27 janvier 2026 à 15:39 +0100, Sven Püschel a écrit :
> Add a v4l2_fill_pixfmt_mp_aligned helper which allows the user to
> specify a custom stride alignment in bytes. This is necessary for
> hardware like the Rockchip RGA3, which requires the stride value to be
> aligned to a 16 bytes boundary.
> 
> Signed-off-by: Sven Püschel <s.pueschel@pengutronix.de>
> ---
>  drivers/media/v4l2-core/v4l2-common.c | 45 +++++++++++++++++++++++++----------
>  include/media/v4l2-common.h           |  4 ++++
>  2 files changed, 37 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 2b5ccedeb6841..e61506ead9150 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -431,14 +431,15 @@ static inline unsigned int v4l2_format_block_height(const struct v4l2_format_inf
>  }
>  
>  static inline unsigned int v4l2_format_plane_stride(const struct v4l2_format_info *info, int plane,
> -						    unsigned int width)
> +						    unsigned int width, u8 byte_alignment)
>  {
>  	unsigned int hdiv = plane ? info->hdiv : 1;
>  	unsigned int aligned_width =
>  		ALIGN(width, v4l2_format_block_width(info, plane));
>  
> -	return DIV_ROUND_UP(aligned_width, hdiv) *
> -	       info->bpp[plane] / info->bpp_div[plane];
> +	return ALIGN(DIV_ROUND_UP(aligned_width, hdiv) * info->bpp[plane] /
> +			     info->bpp_div[plane],
> +		     byte_alignment);
>  }
>  
>  static inline unsigned int v4l2_format_plane_height(const struct v4l2_format_info *info, int plane,
> @@ -452,9 +453,10 @@ static inline unsigned int v4l2_format_plane_height(const struct v4l2_format_inf
>  }
>  
>  static inline unsigned int v4l2_format_plane_size(const struct v4l2_format_info *info, int plane,
> -						  unsigned int width, unsigned int height)
> +						  unsigned int width, unsigned int height,
> +						  u8 stride_alignment)
>  {
> -	return v4l2_format_plane_stride(info, plane, width) *
> +	return v4l2_format_plane_stride(info, plane, width, stride_alignment) *
>  	       v4l2_format_plane_height(info, plane, height);
>  }
>  
> @@ -475,8 +477,9 @@ void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
>  }
>  EXPORT_SYMBOL_GPL(v4l2_apply_frmsize_constraints);
>  
> -int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
> -			u32 pixelformat, u32 width, u32 height)
> +int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> +				u32 pixelformat, u32 width, u32 height,
> +				u8 stride_alignment)
>  {
>  	const struct v4l2_format_info *info;
>  	struct v4l2_plane_pix_format *plane;
> @@ -493,23 +496,41 @@ int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
>  
>  	if (info->mem_planes == 1) {
>  		plane = &pixfmt->plane_fmt[0];
> -		plane->bytesperline = v4l2_format_plane_stride(info, 0, width);
> +		/*
> +		 * In case of multiple planes the stride of the other planes
> +		 * is derived from the y stride. To avoid breaking the stride
> +		 * alignment for the non y plane, multiply it by 2.
> +		 */
> +		if (info->comp_planes > 1)
> +			stride_alignment *= 2;

This generalization does not seem right for NV12, or YUV444 formats. Perhaps
move the correction down into v4l2_format_plane_stride() implementation. It
already have code for scaling the stride, maybe it can be leverage to pre-scale
the alignment.

Nicolas

> +		plane->bytesperline = v4l2_format_plane_stride(info, 0, width,
> +							       stride_alignment);
>  		plane->sizeimage = 0;
>  
>  		for (i = 0; i < info->comp_planes; i++)
>  			plane->sizeimage +=
> -				v4l2_format_plane_size(info, i, width, height);
> +				v4l2_format_plane_size(info, i, width, height,
> +						       stride_alignment);
>  	} else {
>  		for (i = 0; i < info->comp_planes; i++) {
>  			plane = &pixfmt->plane_fmt[i];
>  			plane->bytesperline =
> -				v4l2_format_plane_stride(info, i, width);
> +				v4l2_format_plane_stride(info, i, width,
> +							 stride_alignment);
>  			plane->sizeimage = plane->bytesperline *
>  				v4l2_format_plane_height(info, i, height);
>  		}
>  	}
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp_aligned);
> +
> +int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt,
> +			u32 pixelformat, u32 width, u32 height)
> +{
> +	return v4l2_fill_pixfmt_mp_aligned(pixfmt, pixelformat,
> +					   width, height, 1);
> +}
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt_mp);
>  
>  int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> @@ -529,12 +550,12 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  	pixfmt->width = width;
>  	pixfmt->height = height;
>  	pixfmt->pixelformat = pixelformat;
> -	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width);
> +	pixfmt->bytesperline = v4l2_format_plane_stride(info, 0, width, 1);
>  	pixfmt->sizeimage = 0;
>  
>  	for (i = 0; i < info->comp_planes; i++)
>  		pixfmt->sizeimage +=
> -			v4l2_format_plane_size(info, i, width, height);
> +			v4l2_format_plane_size(info, i, width, height, 1);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fill_pixfmt);
> diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> index 401d8506c24b5..edd416178c333 100644
> --- a/include/media/v4l2-common.h
> +++ b/include/media/v4l2-common.h
> @@ -558,6 +558,10 @@ int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
>  		     u32 width, u32 height);
>  int v4l2_fill_pixfmt_mp(struct v4l2_pix_format_mplane *pixfmt, u32 pixelformat,
>  			u32 width, u32 height);
> +/* @stride_alignment is a power of 2 value in bytes */
> +int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> +				u32 pixelformat, u32 width, u32 height,
> +				u8 stride_alignment);
>  
>  /**
>   * v4l2_get_link_freq - Get link rate from transmitter

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]