[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <43c953bd75ad65f249cf71115bb87179b6ba3f47.camel@ndufresne.ca>
Date: Wed, 24 Dec 2025 09:47:39 -0500
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sven Püschel <s.pueschel@...gutronix.de>, Jacob Chen
<jacob-chen@...wrt.com>, Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
Mauro Carvalho Chehab
<mchehab@...nel.org>, Heiko Stuebner <heiko@...ech.de>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>
Cc: linux-media@...r.kernel.org, linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, kernel@...gutronix.de
Subject: Re: [PATCH v2 03/22] media: v4l2-common: add
v4l2_fill_pixfmt_mp_aligned helper
Hi,
Le mercredi 03 décembre 2025 à 16:52 +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 byte boundary.
byte -> bytes
You way aligned to bytes here ...
>
> Signed-off-by: Sven Püschel <s.pueschel@...gutronix.de>
> ---
> drivers/media/v4l2-core/v4l2-common.c | 34 ++++++++++++++++++++++++----------
> include/media/v4l2-common.h | 4 ++++
> 2 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
> index 2c7ac70c0f486..f86e7d7d29b8e 100644
> --- a/drivers/media/v4l2-core/v4l2-common.c
> +++ b/drivers/media/v4l2-core/v4l2-common.c
> @@ -404,11 +404,12 @@ 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 alignment)
> {
> unsigned int hdiv = plane ? info->hdiv : 1;
> unsigned int aligned_width =
> ALIGN(width, v4l2_format_block_width(info, plane));
> + aligned_width = ALIGN(aligned_width, alignment);
But you actually align the width, which can already be achieved using frmsize-
>step_width and v4l2_apply_frmsize_constraints(). It would make little sense for
the step to not be a multiple of the block for tiled format, but it shows you
forgot to take into considering the fact that block_width has precedence here,
since you can't split tiles.
If you go back to your original idea, stride alignment, remember that you cannot
break the block alignment. For non-mplane format, you also need to special case
subsampled planer format (3 planes) so that userspace can guess the subsequence
plane strides (which is done by dividing the luma plane stride by 2, and if you
need 16 bytes alignment across all strides, then you need the luma plane to be
32 bytes alignment). The mplane format don't have this requirement, since you
pass explicitly the stride of each planes, though, you may want to double it
anyway for compatibility reason (small cost, big gain).
Nicolas
>
> return DIV_ROUND_UP(aligned_width, hdiv) *
> info->bpp[plane] / info->bpp_div[plane];
> @@ -425,9 +426,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);
> }
>
> @@ -448,8 +450,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;
> @@ -466,23 +469,34 @@ 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);
> + 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,
> @@ -502,12 +516,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
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists