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

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]

Date:   Tue, 21 Nov 2023 18:11:33 +0800
From:   Shengjiu Wang <shengjiu.wang@...il.com>
To:     Hans Verkuil <hverkuil@...all.nl>
Cc:     Shengjiu Wang <shengjiu.wang@....com>, sakari.ailus@....fi,
        tfiga@...omium.org, m.szyprowski@...sung.com, mchehab@...nel.org,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        Xiubo.Lee@...il.com, festevam@...il.com, nicoleotsuka@...il.com,
        lgirdwood@...il.com, broonie@...nel.org, perex@...ex.cz,
        tiwai@...e.com, alsa-devel@...a-project.org,
        linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v10 13/14] media: imx-asrc: Add memory to memory driver

On Tue, Nov 21, 2023 at 5:20 PM Hans Verkuil <hverkuil@...all.nl> 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@....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.
>
>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,
> > +};
>