Re: [PATCH v10 13/14] media: imx-asrc: Add memory to memory driver

[Hans Verkuil <hverkuil@xs4all.nl>]

On 21/11/2023 08:37, Shengjiu Wang wrote:
> Implement the ASRC memory to memory function using
> the v4l2 framework, user can use this function with
> v4l2 ioctl interface.
> 
> User send the output and capture buffer to driver and
> driver store the converted data to the capture buffer.
> 
> This feature can be shared by ASRC and EASRC drivers
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  drivers/media/platform/nxp/Kconfig    |   13 +
>  drivers/media/platform/nxp/Makefile   |    1 +
>  drivers/media/platform/nxp/imx-asrc.c | 1264 +++++++++++++++++++++++++
>  3 files changed, 1278 insertions(+)
>  create mode 100644 drivers/media/platform/nxp/imx-asrc.c
> 

<snip>

> +static const struct v4l2_ctrl_config asrc_src_rate_off_control = {
> +	.ops = &asrc_m2m_ctrl_ops,
> +	.id = V4L2_CID_M2M_AUDIO_SOURCE_RATE_OFFSET,
> +	.name = "Audio Source Sample Rate Offset",
> +	.type = V4L2_CTRL_TYPE_INTEGER64,

Do you need this to be INTEGER64? If the actual number of bits
of the fixed point value that the hardware uses fits in 32 bits,
then you can just use the INTEGER type.

> +	.min = 0xFFFFFF0000000000,
> +	.max = 0x7fffffffff,
> +	.def = 0,
> +	.step = 1,
> +	.flags = V4L2_CTRL_FLAG_UPDATE,
> +	.fraction_bits = 32,

Can't you use the actual number of fraction bits that the hardware
uses? I can't imagine that it is actually 32 bits, it is almost
certainly less.

I do think that we need a helper function to fill in the min/max values.

Regards,

	Hans

> +};
> +
> +static const struct v4l2_ctrl_config asrc_dst_rate_off_control = {
> +	.ops = &asrc_m2m_ctrl_ops,
> +	.id = V4L2_CID_M2M_AUDIO_DEST_RATE_OFFSET,
> +	.name = "Audio Dest Sample Rate Offset",
> +	.type = V4L2_CTRL_TYPE_INTEGER64,
> +	.min = 0xFFFFFF0000000000,
> +	.max = 0x7fffffffff,
> +	.def = 0,
> +	.step = 1,
> +	.flags = V4L2_CTRL_FLAG_UPDATE,
> +	.fraction_bits = 32,
> +};