Re: [PATCH v2 2/4] media: v4l2-common: Add kernel-doc for v4l2_fill_pixfmt_mp_aligned()

[Tommaso Merciai]

Hi Sven,
Thanks for your review.

On Fri, Jul 03, 2026 at 02:14:55PM +0200, Sven Püschel wrote:
> Hi Tommaso,
> 
> thanks for adding documentation.
> 
> On 7/1/26 6:49 PM, Tommaso Merciai wrote:
> > Replace the bare placeholder comment with a full kernel-doc block
> > documenting all parameters, the function behaviour for both single
> > memory plane (mem_planes == 1) and multiple memory plane (mem_planes > 1)
> > formats, and the return value.
> > 
> > Signed-off-by: Tommaso Merciai<tommaso.merciai.xr@xxxxxxxxxxxxxx>
> > ---
> > v1->v2:
> >   - New patch
> > 
> >   include/media/v4l2-common.h | 27 ++++++++++++++++++++++++++-
> >   1 file changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/media/v4l2-common.h b/include/media/v4l2-common.h
> > index 749fe38c134e..db59dda578f0 100644
> > --- a/include/media/v4l2-common.h
> > +++ b/include/media/v4l2-common.h
> > @@ -557,7 +557,32 @@ void v4l2_apply_frmsize_constraints(u32 *width, u32 *height,
> >   int v4l2_fill_pixfmt(struct v4l2_pix_format *pixfmt, u32 pixelformat,
> >   		     u32 width, u32 height);
> > -/* @stride_alignment is a power of 2 value in bytes */
> > +/**
> > + * v4l2_fill_pixfmt_mp_aligned - Fill in a &struct v4l2_pix_format_mplane with
> > + *	stride alignment requirements.
> > + *
> > + * @pixfmt: pointer to the &struct v4l2_pix_format_mplane to be filled
> > + * @pixelformat: the V4L2 pixel format (V4L2_PIX_FMT_*)
> > + * @width: image width in pixels
> > + * @height: image height in pixels
> > + * @stride_alignment: stride alignment in bytes; must be a power of 2
> > + *
> > + * Fills all fields of @pixfmt for the given pixel format, dimensions, and
> > + * stride alignment.
> > + *
> > + * For formats stored in a single memory plane (mem_planes == 1), the
> > + * behaviour matches v4l2_fill_pixfmt_aligned(): plane_fmt[0].bytesperline
> Given that the v4l2_fill_pixfmt_aligned helper is added later in patch 3,
> don't you want to move this patch after adding the helper?

You are correct, thanks.
I will move this after adding the helper in v3.

> > + * is set to the primary plane stride rounded up to @stride_alignment, and
> > + * per-component alignments are scaled to keep chroma strides consistently
> > + * derivable from the luma stride. plane_fmt[0].sizeimage covers all
> 
> No, it's the other way: The strides of all components are aligned to the
> @stride_alignment. To keep the chroma strides consistently derivable from
> the luma stride, strides may be aligned to a multiple of the
> @stride_alignment.
> 
> (feel free to use the above if it sounds good)

Thanks for the clarification.
I will use your suggestion in v3.

Kind Regards,
Tommaso

> 
> At least I read your text, as if only the y stride would be rounded up to
> the nearest alignment and for other component strides may be scaled down to
> 1/2 or 1/4 of the alignment. But most of the time the y stride will be
> scaled up (e.g. YUV420 or YUV410) and in rare cases (e.g. NV24/42) the
> component stride is scaled up.
> 
> Sincerely
>     Sven
> 
> > + * component planes.
> > + *
> > + * For formats with multiple memory planes (mem_planes > 1), each plane's
> > + * bytesperline is independently rounded up to @stride_alignment, and
> > + * sizeimage is set to bytesperline multiplied by the plane height.
> > + *
> > + * Return: 0 on success, -EINVAL if @pixelformat is unknown.
> > + */
> >   int v4l2_fill_pixfmt_mp_aligned(struct v4l2_pix_format_mplane *pixfmt,
> >   				u32 pixelformat, u32 width, u32 height,
> >   				u8 stride_alignment);