lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ