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

[Shengjiu Wang]

On Tue, Nov 21, 2023 at 5:20 PM Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
>
> 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@xxxxxxx>
> > ---
> >  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.
>
>From the ratio point view, it is 44bits.  maximum 39 fractional bits.

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

what's the helper function look like?  Where can I find an example?

best regards
wang shengjiu


>
> 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,
> > +};
>