Hi Shengjiu,

On 10/11/2023 06:48, Shengjiu Wang wrote:

Audio signal processing has the requirement for memory to memory similar as Video.

This patch is to add this support in v4l2 framework, defined new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format for audio case usage.

The created audio device is named "/dev/v4l-audioX".

Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

.../userspace-api/media/v4l/buffer.rst | 6 ++ .../media/v4l/dev-audio-mem2mem.rst | 71 +++++++++++++++++++ .../userspace-api/media/v4l/devices.rst | 1 + .../media/v4l/vidioc-enum-fmt.rst | 2 + .../userspace-api/media/v4l/vidioc-g-fmt.rst | 4 ++ .../media/videodev2.h.rst.exceptions | 2 + .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++ drivers/media/v4l2-core/v4l2-dev.c | 17 +++++ drivers/media/v4l2-core/v4l2-ioctl.c | 53 ++++++++++++++ include/media/v4l2-dev.h | 2 + include/media/v4l2-ioctl.h | 34 +++++++++ include/uapi/linux/videodev2.h | 17 +++++ 12 files changed, 213 insertions(+) create mode 100644 Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst

While testing this patch series I discovered that you also need to patch drivers/media/v4l2-core/v4l2-compat-ioctl32.c so it knows about the new audio format.

Without that 32 bit apps will fail with a 64 bit kernel.

Regards,

Hans

diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst index 52bbee81c080..a3754ca6f0d6 100644 --- a/Documentation/userspace-api/media/v4l/buffer.rst +++ b/Documentation/userspace-api/media/v4l/buffer.rst @@ -438,6 +438,12 @@ enum v4l2_buf_type * - ``V4L2_BUF_TYPE_META_OUTPUT`` - 14 - Buffer for metadata output, see :ref:`metadata`.

• • • ``V4L2_BUF_TYPE_AUDIO_CAPTURE``

•  - 15
  

•  - Buffer for audio capture, see :ref:`audio`.
  

• • • ``V4L2_BUF_TYPE_AUDIO_OUTPUT``

•  - 16
  

•  - Buffer for audio output, see :ref:`audio`.
  
  
  

.. _buffer-flags: diff --git a/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst new file mode 100644 index 000000000000..68faecfe3a02 --- /dev/null +++ b/Documentation/userspace-api/media/v4l/dev-audio-mem2mem.rst @@ -0,0 +1,71 @@ +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later

+.. _audiomem2mem:

+******************************** +Audio Memory-To-Memory Interface +********************************

+An audio memory-to-memory device can compress, decompress, transform, or +otherwise convert audio data from one format into another format, in memory. +Such memory-to-memory devices set the ``V4L2_CAP_AUDIO_M2M`` capability. +Examples of memory-to-memory devices are audio codecs, audio preprocessing, +audio postprocessing.

+A memory-to-memory audio node supports both output (sending audio frames from +memory to the hardware) and capture (receiving the processed audio frames +from the hardware into memory) stream I/O. An application will have to +setup the stream I/O for both sides and finally call +:ref:`VIDIOC_STREAMON <VIDIOC_STREAMON>` for both capture and output to +start the hardware.

+Memory-to-memory devices function as a shared resource: you can +open the audio node multiple times, each application setting up their +own properties that are local to the file handle, and each can use +it independently from the others. The driver will arbitrate access to +the hardware and reprogram it whenever another file handler gets access.

+Audio memory-to-memory devices are accessed through character device +special files named ``/dev/v4l-audio``

+Querying Capabilities +=====================

+Device nodes supporting the audio memory-to-memory interface set the +``V4L2_CAP_AUDIO_M2M`` flag in the ``device_caps`` field of the +:c:type:`v4l2_capability` structure returned by the :c:func:`VIDIOC_QUERYCAP` +ioctl.

+Data Format Negotiation +=======================

+The audio device uses the :ref:`format` ioctls to select the capture format. +The audio buffer content format is bound to that selected format. In addition +to the basic :ref:`format` ioctls, the :c:func:`VIDIOC_ENUM_FMT` ioctl must be +supported as well.

+To use the :ref:`format` ioctls applications set the ``type`` field of the +:c:type:`v4l2_format` structure to ``V4L2_BUF_TYPE_AUDIO_CAPTURE`` or to +``V4L2_BUF_TYPE_AUDIO_OUTPUT``. Both drivers and applications must set the +remainder of the :c:type:`v4l2_format` structure to 0.

+.. c:type:: v4l2_audio_format

+.. tabularcolumns:: |p{1.4cm}|p{2.4cm}|p{13.5cm}|

+.. flat-table:: struct v4l2_audio_format

• :header-rows: 0
• :stub-columns: 0
• :widths: 1 1 2
• • • __u32

•  - ``pixelformat``
  

•  - The sample format, set by the application. see :ref:`pixfmt-audio`
  

• • • __u32

•  - ``channels``
  

•  - The channel number, set by the application. channel number range is
  

•    [1, 32].
  

• • • __u32

•  - ``buffersize``
  

•  - Maximum buffer size in bytes required for data. The value is set by the
  

•    driver.
  

diff --git a/Documentation/userspace-api/media/v4l/devices.rst b/Documentation/userspace-api/media/v4l/devices.rst index 8bfbad65a9d4..758bd90f1c26 100644 --- a/Documentation/userspace-api/media/v4l/devices.rst +++ b/Documentation/userspace-api/media/v4l/devices.rst @@ -24,3 +24,4 @@ Interfaces dev-event dev-subdev dev-meta

• dev-audio-mem2mem

diff --git a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst index 000c154b0f98..42deb07f4ff4 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-enum-fmt.rst @@ -96,6 +96,8 @@ the ``mbus_code`` field is handled differently: ``V4L2_BUF_TYPE_VIDEO_OVERLAY``, ``V4L2_BUF_TYPE_SDR_CAPTURE``, ``V4L2_BUF_TYPE_SDR_OUTPUT``,

• ``V4L2_BUF_TYPE_AUDIO_CAPTURE``,
• ``V4L2_BUF_TYPE_AUDIO_OUTPUT``, ``V4L2_BUF_TYPE_META_CAPTURE`` and ``V4L2_BUF_TYPE_META_OUTPUT``. See :c:type:`v4l2_buf_type`.

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst b/Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst index 675c385e5aca..528fd9df41aa 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-g-fmt.rst @@ -130,6 +130,10 @@ The format as returned by :ref:`VIDIOC_TRY_FMT <VIDIOC_G_FMT>` must be identical - ``meta`` - Definition of a metadata format, see :ref:`meta-formats`, used by metadata capture devices.

• • • struct :c:type:`v4l2_audio_format`

•  - ``audio``
  

•  - Definition of a audio data format, see :ref:`audiomem2mem`, used by
  

•    audio memory-to-memory devices
  

  • • __u8
    • ``raw_data``\ [200]
    • Place holder for future extensions.

diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions index da6d0b8e4c2c..e61152bb80d1 100644 --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions @@ -29,6 +29,8 @@ replace symbol V4L2_FIELD_SEQ_TB :c:type:`v4l2_field` replace symbol V4L2_FIELD_TOP :c:type:`v4l2_field`

# Documented enum v4l2_buf_type +replace symbol V4L2_BUF_TYPE_AUDIO_CAPTURE :c:type:`v4l2_buf_type` +replace symbol V4L2_BUF_TYPE_AUDIO_OUTPUT :c:type:`v4l2_buf_type` replace symbol V4L2_BUF_TYPE_META_CAPTURE :c:type:`v4l2_buf_type` replace symbol V4L2_BUF_TYPE_META_OUTPUT :c:type:`v4l2_buf_type` replace symbol V4L2_BUF_TYPE_SDR_CAPTURE :c:type:`v4l2_buf_type` diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index c7a54d82a55e..12f2be2773a2 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -785,6 +785,10 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) case V4L2_BUF_TYPE_META_OUTPUT: requested_sizes[0] = f->fmt.meta.buffersize; break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• requested_sizes[0] = f->fmt.audio.buffersize;
  

• break;
  

  default: return -EINVAL; }

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index f81279492682..b92c760b611a 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -553,6 +553,7 @@ static void determine_valid_ioctls(struct video_device *vdev) bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH; bool is_meta = vdev->vfl_type == VFL_TYPE_VIDEO && (vdev->device_caps & meta_caps);

• bool is_audio = vdev->vfl_type == VFL_TYPE_AUDIO; bool is_rx = vdev->vfl_dir != VFL_DIR_TX; bool is_tx = vdev->vfl_dir != VFL_DIR_RX; bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;

@@ -664,6 +665,19 @@ static void determine_valid_ioctls(struct video_device *vdev) SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out); SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out); }

• if (is_audio && is_rx) {

• /* audio capture specific ioctls */
  

• SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_audio_cap);
  

• SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_cap);
  

• SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_cap);
  

• SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_audio_cap);
  

• } else if (is_audio && is_tx) {

• /* audio output specific ioctls */
  

• SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT, vidioc_enum_fmt_audio_out);
  

• SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_out);
  

• SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_out);
  

• SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_audio_out);
  

• } if (is_vbi) { /* vbi specific ioctls */ if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||

@@ -927,6 +941,9 @@ int __video_register_device(struct video_device *vdev, case VFL_TYPE_TOUCH: name_base = "v4l-touch"; break;

• case VFL_TYPE_AUDIO:

• name_base = "v4l-audio";
  

• break;
  

  default: pr_err("%s called with unknown type: %d\n", __func__, type);

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 9b1de54ce379..1bb03794922a 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -188,6 +188,8 @@ const char *v4l2_type_names[] = { [V4L2_BUF_TYPE_SDR_OUTPUT] = "sdr-out", [V4L2_BUF_TYPE_META_CAPTURE] = "meta-cap", [V4L2_BUF_TYPE_META_OUTPUT] = "meta-out",

• [V4L2_BUF_TYPE_AUDIO_CAPTURE] = "audio-cap",
• [V4L2_BUF_TYPE_AUDIO_OUTPUT] = "audio-out",

}; EXPORT_SYMBOL(v4l2_type_names);

@@ -276,6 +278,7 @@ static void v4l_print_format(const void *arg, bool write_only) const struct v4l2_sliced_vbi_format *sliced; const struct v4l2_window *win; const struct v4l2_meta_format *meta;

• const struct v4l2_audio_format *audio; u32 pixelformat; u32 planes; unsigned i;

@@ -346,6 +349,13 @@ static void v4l_print_format(const void *arg, bool write_only) pr_cont(", dataformat=%p4cc, buffersize=%u\n", &pixelformat, meta->buffersize); break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:
• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• audio = &p->fmt.audio;
  

• pixelformat = audio->audioformat;
  

• pr_cont(", format=%p4cc, channels=%u, buffersize=%u\n",
  

• 	&pixelformat, audio->channels, audio->buffersize);
  

• break;
  

  }

}

@@ -927,6 +937,7 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type) bool is_tch = vfd->vfl_type == VFL_TYPE_TOUCH; bool is_meta = vfd->vfl_type == VFL_TYPE_VIDEO && (vfd->device_caps & meta_caps);

• bool is_audio = vfd->vfl_type == VFL_TYPE_AUDIO; bool is_rx = vfd->vfl_dir != VFL_DIR_TX; bool is_tx = vfd->vfl_dir != VFL_DIR_RX;

@@ -992,6 +1003,14 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type) if (is_meta && is_tx && ops->vidioc_g_fmt_meta_out) return 0; break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:

• if (is_audio && is_rx && ops->vidioc_g_fmt_audio_cap)
  

• 	return 0;
  

• break;
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• if (is_audio && is_tx && ops->vidioc_g_fmt_audio_out)
  

• 	return 0;
  

• break;
  

  default: break; }

@@ -1597,6 +1616,16 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops, break; ret = ops->vidioc_enum_fmt_meta_out(file, fh, arg); break;

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:

• if (unlikely(!ops->vidioc_enum_fmt_audio_cap))
  

• 	break;
  

• ret = ops->vidioc_enum_fmt_audio_cap(file, fh, arg);
  

• break;
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• if (unlikely(!ops->vidioc_enum_fmt_audio_out))
  

• 	break;
  

• ret = ops->vidioc_enum_fmt_audio_out(file, fh, arg);
  

• break;
  

  } if (ret == 0) v4l_fill_fmtdesc(p);

@@ -1673,6 +1702,10 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops, return ops->vidioc_g_fmt_meta_cap(file, fh, arg); case V4L2_BUF_TYPE_META_OUTPUT: return ops->vidioc_g_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:

• return ops->vidioc_g_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• return ops->vidioc_g_fmt_audio_out(file, fh, arg);
  

  } return -EINVAL;

} @@ -1784,6 +1817,16 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops, break; memset_after(p, 0, fmt.meta); return ops->vidioc_s_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:

• if (unlikely(!ops->vidioc_s_fmt_audio_cap))
  

• 	break;
  

• memset_after(p, 0, fmt.audio);
  

• return ops->vidioc_s_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• if (unlikely(!ops->vidioc_s_fmt_audio_out))
  

• 	break;
  

• memset_after(p, 0, fmt.audio);
  

• return ops->vidioc_s_fmt_audio_out(file, fh, arg);
  

  } return -EINVAL;

} @@ -1892,6 +1935,16 @@ static int v4l_try_fmt(const struct v4l2_ioctl_ops *ops, break; memset_after(p, 0, fmt.meta); return ops->vidioc_try_fmt_meta_out(file, fh, arg);

• case V4L2_BUF_TYPE_AUDIO_CAPTURE:

• if (unlikely(!ops->vidioc_try_fmt_audio_cap))
  

• 	break;
  

• memset_after(p, 0, fmt.audio);
  

• return ops->vidioc_try_fmt_audio_cap(file, fh, arg);
  

• case V4L2_BUF_TYPE_AUDIO_OUTPUT:

• if (unlikely(!ops->vidioc_try_fmt_audio_out))
  

• 	break;
  

• memset_after(p, 0, fmt.audio);
  

• return ops->vidioc_try_fmt_audio_out(file, fh, arg);
  

  } return -EINVAL;

} diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h index d82dfdbf6e58..82b63f82d43f 100644 --- a/include/media/v4l2-dev.h +++ b/include/media/v4l2-dev.h @@ -30,6 +30,7 @@

• @VFL_TYPE_SUBDEV: for V4L2 subdevices
• @VFL_TYPE_SDR: for Software Defined Radio tuners
• @VFL_TYPE_TOUCH: for touch sensors

• • @VFL_TYPE_AUDIO: for audio memory-to-memory devices
  • @VFL_TYPE_MAX: number of VFL types, must always be last in the enum
  */

enum vfl_devnode_type { @@ -39,6 +40,7 @@ enum vfl_devnode_type { VFL_TYPE_SUBDEV, VFL_TYPE_SDR, VFL_TYPE_TOUCH,

• VFL_TYPE_AUDIO, VFL_TYPE_MAX /* Shall be the last one */

};

diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h index edb733f21604..f840cf740ce1 100644 --- a/include/media/v4l2-ioctl.h +++ b/include/media/v4l2-ioctl.h @@ -45,6 +45,12 @@ struct v4l2_fh;

• @vidioc_enum_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• for metadata output

• • @vidioc_enum_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• • for audio capture
• • @vidioc_enum_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_ENUM_FMT <vidioc_enum_fmt>` ioctl logic
• • for audio output
  • @vidioc_g_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -79,6 +85,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_g_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_g_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_g_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_G_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_s_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -113,6 +123,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_s_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_s_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_s_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_S_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_try_fmt_vid_cap: pointer to the function that implements
  • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for video capture
  • in single plane mode

@@ -149,6 +163,10 @@ struct v4l2_fh;

• :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for metadata capture
• @vidioc_try_fmt_meta_out: pointer to the function that implements
• :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for metadata output

• • @vidioc_try_fmt_audio_cap: pointer to the function that implements
• • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for audio capture
• • @vidioc_try_fmt_audio_out: pointer to the function that implements
• • :ref:`VIDIOC_TRY_FMT <vidioc_g_fmt>` ioctl logic for audio output
  • @vidioc_reqbufs: pointer to the function that implements
  • :ref:`VIDIOC_REQBUFS <vidioc_reqbufs>` ioctl
  • @vidioc_querybuf: pointer to the function that implements

@@ -315,6 +333,10 @@ struct v4l2_ioctl_ops { struct v4l2_fmtdesc *f); int (*vidioc_enum_fmt_meta_out)(struct file *file, void *fh, struct v4l2_fmtdesc *f);

• int (*vidioc_enum_fmt_audio_cap)(struct file *file, void *fh,

• 			 struct v4l2_fmtdesc *f);
  

• int (*vidioc_enum_fmt_audio_out)(struct file *file, void *fh,

• 			 struct v4l2_fmtdesc *f);
  
  

  /* VIDIOC_G_FMT handlers */ int (*vidioc_g_fmt_vid_cap)(struct file *file, void *fh,

@@ -345,6 +367,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_g_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_g_fmt_audio_cap)(struct file *file, void *fh,

• 		      struct v4l2_format *f);
  

• int (*vidioc_g_fmt_audio_out)(struct file *file, void *fh,

• 		      struct v4l2_format *f);
  
  

  /* VIDIOC_S_FMT handlers */ int (*vidioc_s_fmt_vid_cap)(struct file *file, void *fh,

@@ -375,6 +401,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_s_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_s_fmt_audio_cap)(struct file *file, void *fh,

• 		      struct v4l2_format *f);
  

• int (*vidioc_s_fmt_audio_out)(struct file *file, void *fh,

• 		      struct v4l2_format *f);
  
  

  /* VIDIOC_TRY_FMT handlers */ int (*vidioc_try_fmt_vid_cap)(struct file *file, void *fh,

@@ -405,6 +435,10 @@ struct v4l2_ioctl_ops { struct v4l2_format *f); int (*vidioc_try_fmt_meta_out)(struct file *file, void *fh, struct v4l2_format *f);

• int (*vidioc_try_fmt_audio_cap)(struct file *file, void *fh,

• 			struct v4l2_format *f);
  

• int (*vidioc_try_fmt_audio_out)(struct file *file, void *fh,

• 			struct v4l2_format *f);
  
  

  /* Buffer handlers */ int (*vidioc_reqbufs)(struct file *file, void *fh,

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 5053f66c501a..1e06ac73b6fa 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -153,6 +153,8 @@ enum v4l2_buf_type { V4L2_BUF_TYPE_SDR_OUTPUT = 12, V4L2_BUF_TYPE_META_CAPTURE = 13, V4L2_BUF_TYPE_META_OUTPUT = 14,

• V4L2_BUF_TYPE_AUDIO_CAPTURE = 15,
• V4L2_BUF_TYPE_AUDIO_OUTPUT = 16, /* Deprecated, do not use */ V4L2_BUF_TYPE_PRIVATE = 0x80,

}; @@ -169,6 +171,7 @@ enum v4l2_buf_type { || (type) == V4L2_BUF_TYPE_VBI_OUTPUT \ || (type) == V4L2_BUF_TYPE_SLICED_VBI_OUTPUT \ || (type) == V4L2_BUF_TYPE_SDR_OUTPUT \

• || (type) == V4L2_BUF_TYPE_AUDIO_OUTPUT		\
  

  || (type) == V4L2_BUF_TYPE_META_OUTPUT)

#define V4L2_TYPE_IS_CAPTURE(type) (!V4L2_TYPE_IS_OUTPUT(type)) @@ -2419,6 +2422,18 @@ struct v4l2_meta_format { __u32 buffersize; } __attribute__ ((packed));

+/**

• • struct v4l2_audio_format - audio data format definition
• • @audioformat: little endian four character code (fourcc)
• • @channels: channel numbers
• • @buffersize: maximum size in bytes required for data
• */

+struct v4l2_audio_format {

• __u32 audioformat;
• __u32 channels;
• __u32 buffersize;

+} __attribute__ ((packed));

/**

• struct v4l2_format - stream data format
• @type: enum v4l2_buf_type; type of the data stream

@@ -2427,6 +2442,7 @@ struct v4l2_meta_format {

• @win: definition of an overlaid image
• @vbi: raw VBI capture or output parameters
• @sliced: sliced VBI capture or output parameters

• • @audio: definition of an audio format
  • @raw_data: placeholder for future extensions and custom formats
  • @fmt: union of @pix, @pix_mp, @win, @vbi, @sliced, @sdr, @meta

  • and @raw_data
    

@@ -2441,6 +2457,7 @@ struct v4l2_format { struct v4l2_sliced_vbi_format sliced; /* V4L2_BUF_TYPE_SLICED_VBI_CAPTURE */ struct v4l2_sdr_format sdr; /* V4L2_BUF_TYPE_SDR_CAPTURE */ struct v4l2_meta_format meta; /* V4L2_BUF_TYPE_META_CAPTURE */

• struct v4l2_audio_format	audio;   /* V4L2_BUF_TYPE_AUDIO_CAPTURE */
  

  __u8 raw_data[200]; /* user-defined */ } fmt;

};

Hi Shengjiu,

On 10/11/2023 06:48, Shengjiu Wang wrote:

Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format.

Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

This patch adds a new control type. This is something that also needs to be tested by v4l2-compliance, and for that we need to add support for this to one of the media test-drivers. The best place for that is the vivid driver, since that has already a bunch of test controls for other control types.

See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c.

Can you add a patch adding a fixed point test control to vivid?

Thanks!

Hans

---

.../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++++++------ .../userspace-api/media/v4l/vidioc-queryctrl.rst | 9 ++++++++- .../userspace-api/media/videodev2.h.rst.exceptions | 1 + drivers/media/v4l2-core/v4l2-ctrls-api.c | 5 ++++- drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ include/uapi/linux/videodev2.h | 1 + 6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst index e8475f9fd2cf..e7e5d78dc11e 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst @@ -162,13 +162,13 @@ still cause this situation. * - __s32 - ``value`` - New value or current value. Valid if this control is not of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • __s64
    • ``value64``
    • New value or current value. Valid if this control is of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • char *
    • ``string``
    • A pointer to a string. Valid if this control is of type

@@ -193,8 +193,9 @@ still cause this situation. * - __s64 * - ``p_s64`` - A pointer to a matrix control of signed 64-bit values. Valid if

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and
  

•    ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set.
  

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64``,
  

•    ``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
  

•    is set.
  

  • • struct :c:type:`v4l2_area` *
    • ``p_area``
    • A pointer to a struct :c:type:`v4l2_area`. Valid if this control is

diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst index 4d38acafe8e1..f3995ec57044 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. - ``default_value`` - The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8``

• or ``_U16`` control. Not valid for other types of controls.

• ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of

• controls.

  .. note::

@@ -549,6 +550,12 @@ See also the examples in :ref:`control`. - n/a - A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film Grain parameters for stateless video decoders.

• • • ``V4L2_CTRL_TYPE_FIXED_POINT``

•  - any
  

•  - any
  

•  - any
  

•  - A 64-bit integer valued control, containing parameter which is
  

•    Q31.32 format.
  
  

.. raw:: latex

diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions index e61152bb80d1..2faa5a2015eb 100644 --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type`

# V4L2 capability defines replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..e6a0fb8d6791 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c, return copy_to_user(c->string, ptr.p_char, len + 1) ? -EFAULT : 0; case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: c->value64 = *ptr.p_s64; break; default:

@@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)

switch (ctrl->type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: *ctrl->p_new.p_s64 = c->value64; break; case V4L2_CTRL_TYPE_STRING:

@@ -540,7 +542,8 @@ static int validate_ctrls(struct v4l2_ext_controls *cs, */ if (ctrl->is_ptr) continue;

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
  

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64 ||
  

•     ctrl->type == V4L2_CTRL_TYPE_FIXED_POINT)
  p_new.p_s64 = &cs->controls[i].value64;
  

  else p_new.p_s32 = &cs->controls[i].value;

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..9d50df0d9874 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -1187,6 +1187,7 @@ static int std_validate_elem(const struct v4l2_ctrl *ctrl, u32 idx, case V4L2_CTRL_TYPE_INTEGER: return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl); case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: /*
  • We can't use the ROUND_TO_RANGE define here due to
  • the u64 divide that needs special care.

@@ -1779,6 +1780,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, /* Prefill elem_size for all types handled by std_type_ops */ switch ((u32)type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: elem_size = sizeof(s64); break; case V4L2_CTRL_TYPE_STRING:

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index cf8c44595a1d..9482ac66a675 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1903,6 +1903,7 @@ enum v4l2_ctrl_type { V4L2_CTRL_TYPE_STRING = 7, V4L2_CTRL_TYPE_BITMASK = 8, V4L2_CTRL_TYPE_INTEGER_MENU = 9,

• V4L2_CTRL_TYPE_FIXED_POINT = 10,

  /* Compound types are >= 0x0100 */ V4L2_CTRL_COMPOUND_TYPES = 0x0100,

On 13/11/2023 11:42, Laurent Pinchart wrote:

On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote:

...

Hi Shengjiu,

On 10/11/2023 06:48, Shengjiu Wang wrote:

...

Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format.

Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

This patch adds a new control type. This is something that also needs to be tested by v4l2-compliance, and for that we need to add support for this to one of the media test-drivers. The best place for that is the vivid driver, since that has already a bunch of test controls for other control types.

See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c.

Can you add a patch adding a fixed point test control to vivid?

I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to relate more to units than control types. We have lots of fixed-point values in controls already, using the 32-bit and 64-bit integer control types. They use various locations for the decimal point, depending on the control. If we want to make this more explicit to users, we should work on adding unit support to the V4L2 controls.

"Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units.

A quick "git grep -i "fixed point" Documentation/userspace-api/media/' only shows a single driver specific control (dw100.rst).

I'm not aware of other controls in mainline that use fixed point.

Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting min/max/step you can easily map that to just about any QN.M format where N <= 31 and M <= 32.

In the case of dw100 it is a bit different in that it is quite specialized and it had to fit in 16 bits.

Regards,

Hans

...

...

---

.../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++++++------ .../userspace-api/media/v4l/vidioc-queryctrl.rst | 9 ++++++++- .../userspace-api/media/videodev2.h.rst.exceptions | 1 + drivers/media/v4l2-core/v4l2-ctrls-api.c | 5 ++++- drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ include/uapi/linux/videodev2.h | 1 + 6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst index e8475f9fd2cf..e7e5d78dc11e 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst @@ -162,13 +162,13 @@ still cause this situation. * - __s32 - ``value`` - New value or current value. Valid if this control is not of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • __s64
    • ``value64``
    • New value or current value. Valid if this control is of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • char *
    • ``string``
    • A pointer to a string. Valid if this control is of type

@@ -193,8 +193,9 @@ still cause this situation. * - __s64 * - ``p_s64`` - A pointer to a matrix control of signed 64-bit values. Valid if

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and
  

•    ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set.
  

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64``,
  

•    ``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
  

•    is set.
  

  • • struct :c:type:`v4l2_area` *
    • ``p_area``
    • A pointer to a struct :c:type:`v4l2_area`. Valid if this control is

diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst index 4d38acafe8e1..f3995ec57044 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. - ``default_value`` - The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8``

• or ``_U16`` control. Not valid for other types of controls.

• ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of

• controls.

  .. note::

@@ -549,6 +550,12 @@ See also the examples in :ref:`control`. - n/a - A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film Grain parameters for stateless video decoders.

• • • ``V4L2_CTRL_TYPE_FIXED_POINT``

•  - any
  

•  - any
  

•  - any
  

•  - A 64-bit integer valued control, containing parameter which is
  

•    Q31.32 format.
  
  

.. raw:: latex

diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions index e61152bb80d1..2faa5a2015eb 100644 --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type`

# V4L2 capability defines replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..e6a0fb8d6791 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c, return copy_to_user(c->string, ptr.p_char, len + 1) ? -EFAULT : 0; case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: c->value64 = *ptr.p_s64; break; default:

@@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)

switch (ctrl->type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: *ctrl->p_new.p_s64 = c->value64; break; case V4L2_CTRL_TYPE_STRING:

@@ -540,7 +542,8 @@ static int validate_ctrls(struct v4l2_ext_controls *cs, */ if (ctrl->is_ptr) continue;

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
  

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64 ||
  

•     ctrl->type == V4L2_CTRL_TYPE_FIXED_POINT)
  p_new.p_s64 = &cs->controls[i].value64;
  

  else p_new.p_s32 = &cs->controls[i].value;

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..9d50df0d9874 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -1187,6 +1187,7 @@ static int std_validate_elem(const struct v4l2_ctrl *ctrl, u32 idx, case V4L2_CTRL_TYPE_INTEGER: return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl); case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: /*
  • We can't use the ROUND_TO_RANGE define here due to
  • the u64 divide that needs special care.

@@ -1779,6 +1780,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, /* Prefill elem_size for all types handled by std_type_ops */ switch ((u32)type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: elem_size = sizeof(s64); break; case V4L2_CTRL_TYPE_STRING:

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index cf8c44595a1d..9482ac66a675 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1903,6 +1903,7 @@ enum v4l2_ctrl_type { V4L2_CTRL_TYPE_STRING = 7, V4L2_CTRL_TYPE_BITMASK = 8, V4L2_CTRL_TYPE_INTEGER_MENU = 9,

• V4L2_CTRL_TYPE_FIXED_POINT = 10,

  /* Compound types are >= 0x0100 */ V4L2_CTRL_COMPOUND_TYPES = 0x0100,

Here is an RFC patch adding support for 'fraction_bits'. It's lacking documentation, but it can be used for testing.

It was rather a pain logging fixed point number in a reasonable format, but I think it is OK.

In userspace (where you can use floating point) it is a lot easier:

printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));

I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl. I could add it to struct v4l2_queryctrl, but I did not think that was necessary. Other opinions are welcome.

In the meantime, let me know if this works for your patch series. If it does, then I can clean this up.

Regards,

Hans

Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl --- drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 + drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++++++++++++++++++---- include/media/v4l2-ctrls.h | 7 ++- include/uapi/linux/videodev2.h | 20 ++++++- 4 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..3132df315b17 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr qc->elems = ctrl->elems; qc->nr_of_dims = ctrl->nr_of_dims; memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0])); + qc->fraction_bits = ctrl->fraction_bits; qc->minimum = ctrl->minimum; qc->maximum = ctrl->maximum; qc->default_value = ctrl->default_value; diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..0e08a371af5c 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl *ctrl, u32 from_idx, } EXPORT_SYMBOL(v4l2_ctrl_type_op_init);

+static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits) +{ + s64 i = v4l2_fp_integer(v, fraction_bits); + s64 f = v4l2_fp_fraction(v, fraction_bits); + + if (!f) { + pr_cont("%lld", i); + } else if (fraction_bits < 20) { + u64 div = 1ULL << fraction_bits; + + if (!i && f < 0) + pr_cont("-%lld/%llu", -f, div); + else if (!i) + pr_cont("%lld/%llu", f, div); + else if (i < 0 || f < 0) + pr_cont("-%lld-%llu/%llu", -i, -f, div); + else + pr_cont("%lld+%llu/%llu", i, f, div); + } else { + if (!i && f < 0) + pr_cont("-%lld/(2^%u)", -f, fraction_bits); + else if (!i) + pr_cont("%lld/(2^%u)", f, fraction_bits); + else if (i < 0 || f < 0) + pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits); + else + pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits); + } +} + void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) { union v4l2_ctrl_ptr ptr = ctrl->p_cur;

if (ctrl->is_array) { - unsigned i; + unsigned int i;

for (i = 0; i < ctrl->nr_of_dims; i++) pr_cont("[%u]", ctrl->dims[i]); @@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)

switch (ctrl->type) { case V4L2_CTRL_TYPE_INTEGER: - pr_cont("%d", *ptr.p_s32); + if (!ctrl->fraction_bits) + pr_cont("%d", *ptr.p_s32); + else + v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits); break; case V4L2_CTRL_TYPE_BOOLEAN: pr_cont("%s", *ptr.p_s32 ? "true" : "false"); @@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) pr_cont("0x%08x", *ptr.p_s32); break; case V4L2_CTRL_TYPE_INTEGER64: - pr_cont("%lld", *ptr.p_s64); + if (!ctrl->fraction_bits) + pr_cont("%lld", *ptr.p_s64); + else + v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits); break; case V4L2_CTRL_TYPE_STRING: pr_cont("%s", ptr.p_char); break; case V4L2_CTRL_TYPE_U8: - pr_cont("%u", (unsigned)*ptr.p_u8); + if (!ctrl->fraction_bits) + pr_cont("%u", (unsigned int)*ptr.p_u8); + else + v4l2_ctrl_log_fp((unsigned int)*ptr.p_u8, ctrl->fraction_bits); break; case V4L2_CTRL_TYPE_U16: - pr_cont("%u", (unsigned)*ptr.p_u16); + if (!ctrl->fraction_bits) + pr_cont("%u", (unsigned int)*ptr.p_u16); + else + v4l2_ctrl_log_fp((unsigned int)*ptr.p_u16, ctrl->fraction_bits); break; case V4L2_CTRL_TYPE_U32: - pr_cont("%u", (unsigned)*ptr.p_u32); + if (!ctrl->fraction_bits) + pr_cont("%u", (unsigned int)*ptr.p_u32); + else + v4l2_ctrl_log_fp((unsigned int)*ptr.p_u32, ctrl->fraction_bits); break; case V4L2_CTRL_TYPE_H264_SPS: pr_cont("H264_SPS"); @@ -1752,7 +1797,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, u32 id, const char *name, enum v4l2_ctrl_type type, s64 min, s64 max, u64 step, s64 def, const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size, - u32 flags, const char * const *qmenu, + u32 fraction_bits, u32 flags, const char * const *qmenu, const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def, void *priv) { @@ -1939,6 +1984,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, ctrl->name = name; ctrl->type = type; ctrl->flags = flags; + ctrl->fraction_bits = fraction_bits; ctrl->minimum = min; ctrl->maximum = max; ctrl->step = step; @@ -2037,7 +2083,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl, ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->type_ops, cfg->id, name, type, min, max, is_menu ? cfg->menu_skip_mask : step, def, - cfg->dims, cfg->elem_size, + cfg->dims, cfg->elem_size, cfg->fraction_bits, flags, qmenu, qmenu_int, cfg->p_def, priv); if (ctrl) ctrl->is_private = cfg->is_private; @@ -2062,7 +2108,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, - min, max, step, def, NULL, 0, + min, max, step, def, NULL, 0, 0, flags, NULL, NULL, ptr_null, NULL); } EXPORT_SYMBOL(v4l2_ctrl_new_std); @@ -2095,7 +2141,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, - 0, max, mask, def, NULL, 0, + 0, max, mask, def, NULL, 0, 0, flags, qmenu, qmenu_int, ptr_null, NULL); } EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); @@ -2127,7 +2173,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, - 0, max, mask, def, NULL, 0, + 0, max, mask, def, NULL, 0, 0, flags, qmenu, NULL, ptr_null, NULL);

} @@ -2149,7 +2195,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, - min, max, step, def, NULL, 0, + min, max, step, def, NULL, 0, 0, flags, NULL, NULL, p_def, NULL); } EXPORT_SYMBOL(v4l2_ctrl_new_std_compound); @@ -2173,7 +2219,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, - 0, max, 0, def, NULL, 0, + 0, max, 0, def, NULL, 0, 0, flags, NULL, qmenu_int, ptr_null, NULL); } EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 59679a42b3e7..c35514c5bf88 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -211,7 +211,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv); * except for dynamic arrays. In that case it is in the range of * 1 to @p_array_alloc_elems. * @dims: The size of each dimension. - * @nr_of_dims:The number of dimensions in @dims. + * @nr_of_dims: The number of dimensions in @dims. + * @fraction_bits: The number of fraction bits for fixed point values. * @menu_skip_mask: The control's skip mask for menu controls. This makes it * easy to skip menu items that are not valid. If bit X is set, * then menu item X is skipped. Of course, this only works for @@ -228,6 +229,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv); * :math:`ceil(\frac{maximum - minimum}{step}) + 1`. * Used only if the @type is %V4L2_CTRL_TYPE_INTEGER_MENU. * @flags: The control's flags. + * @fraction_bits: The number of fraction bits for fixed point values. * @priv: The control's private pointer. For use by the driver. It is * untouched by the control framework. Note that this pointer is * not freed when the control is deleted. Should this be needed @@ -286,6 +288,7 @@ struct v4l2_ctrl { u32 new_elems; u32 dims[V4L2_CTRL_MAX_DIMS]; u32 nr_of_dims; + u32 fraction_bits; union { u64 step; u64 menu_skip_mask; @@ -426,6 +429,7 @@ struct v4l2_ctrl_handler { * @dims: The size of each dimension. * @elem_size: The size in bytes of the control. * @flags: The control's flags. + * @fraction_bits: The number of fraction bits for fixed point values. * @menu_skip_mask: The control's skip mask for menu controls. This makes it * easy to skip menu items that are not valid. If bit X is set, * then menu item X is skipped. Of course, this only works for @@ -455,6 +459,7 @@ struct v4l2_ctrl_config { u32 dims[V4L2_CTRL_MAX_DIMS]; u32 elem_size; u32 flags; + u32 fraction_bits; u64 menu_skip_mask; const char * const *qmenu; const s64 *qmenu_int; diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c3d4e490ce7c..26ecac19722a 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1944,9 +1944,27 @@ struct v4l2_query_ext_ctrl { __u32 elems; __u32 nr_of_dims; __u32 dims[V4L2_CTRL_MAX_DIMS]; - __u32 reserved[32]; + __u32 fraction_bits; + __u32 reserved[31]; };

+static inline __s64 v4l2_fp_compose(__s64 i, __s64 f, unsigned int fraction_bits) +{ + return (i << fraction_bits) + f; +} + +static inline __s64 v4l2_fp_integer(__s64 v, unsigned int fraction_bits) +{ + return v / (1LL << fraction_bits); +} + +static inline __s64 v4l2_fp_fraction(__s64 v, unsigned int fraction_bits) +{ + __u64 mask = (1ULL << fraction_bits) - 1; + + return v < 0 ? -((-v) & mask) : (v & mask); +} + /* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */ struct v4l2_querymenu { __u32 id;

Hi Hans,

Thank you for the patch.

On Fri, Nov 17, 2023 at 01:07:44PM +0100, Hans Verkuil wrote:

Here is an RFC patch adding support for 'fraction_bits'. It's lacking documentation, but it can be used for testing.

It was rather a pain logging fixed point number in a reasonable format, but I think it is OK.

In userspace (where you can use floating point) it is a lot easier:

printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));

I wonder if we could add a printk() format specifier for this. Doesn't need to be done right now though, just an idea.

I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl. I could add it to struct v4l2_queryctrl, but I did not think that was necessary. Other opinions are welcome.

In the meantime, let me know if this works for your patch series. If it does, then I can clean this up.

Regards,

Hans

Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl

drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 + drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++++++++++++++++++---- include/media/v4l2-ctrls.h | 7 ++- include/uapi/linux/videodev2.h | 20 ++++++- 4 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..3132df315b17 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr qc->elems = ctrl->elems; qc->nr_of_dims = ctrl->nr_of_dims; memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));

• qc->fraction_bits = ctrl->fraction_bits; qc->minimum = ctrl->minimum; qc->maximum = ctrl->maximum; qc->default_value = ctrl->default_value;

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..0e08a371af5c 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl *ctrl, u32 from_idx, } EXPORT_SYMBOL(v4l2_ctrl_type_op_init);

+static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits) +{

• s64 i = v4l2_fp_integer(v, fraction_bits);
• s64 f = v4l2_fp_fraction(v, fraction_bits);
• if (!f) {

• pr_cont("%lld", i);
  

• } else if (fraction_bits < 20) {

• u64 div = 1ULL << fraction_bits;
  

• if (!i && f < 0)
  

• 	pr_cont("-%lld/%llu", -f, div);
  

• else if (!i)
  

• 	pr_cont("%lld/%llu", f, div);
  

• else if (i < 0 || f < 0)
  

• 	pr_cont("-%lld-%llu/%llu", -i, -f, div);
  

• else
  

• 	pr_cont("%lld+%llu/%llu", i, f, div);
  

• } else {

• if (!i && f < 0)
  

• 	pr_cont("-%lld/(2^%u)", -f, fraction_bits);
  

• else if (!i)
  

• 	pr_cont("%lld/(2^%u)", f, fraction_bits);
  

• else if (i < 0 || f < 0)
  

• 	pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
  

• else
  

• 	pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
  

• }

+}

void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) { union v4l2_ctrl_ptr ptr = ctrl->p_cur;

if (ctrl->is_array) {

• unsigned i;
  

• unsigned int i;
  
  

  for (i = 0; i < ctrl->nr_of_dims; i++) pr_cont("[%u]", ctrl->dims[i]);

@@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)

switch (ctrl->type) { case V4L2_CTRL_TYPE_INTEGER:

• pr_cont("%d", *ptr.p_s32);
  

• if (!ctrl->fraction_bits)
  

• 	pr_cont("%d", *ptr.p_s32);
  

• else
  

• 	v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
  

  break; case V4L2_CTRL_TYPE_BOOLEAN: pr_cont("%s", *ptr.p_s32 ? "true" : "false");

@@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) pr_cont("0x%08x", *ptr.p_s32); break; case V4L2_CTRL_TYPE_INTEGER64:

• pr_cont("%lld", *ptr.p_s64);
  

• if (!ctrl->fraction_bits)
  

• 	pr_cont("%lld", *ptr.p_s64);
  

• else
  

• 	v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
  

  break; case V4L2_CTRL_TYPE_STRING: pr_cont("%s", ptr.p_char); break; case V4L2_CTRL_TYPE_U8:

• pr_cont("%u", (unsigned)*ptr.p_u8);
  

• if (!ctrl->fraction_bits)
  

• 	pr_cont("%u", (unsigned int)*ptr.p_u8);
  

• else
  

• 	v4l2_ctrl_log_fp((unsigned int)*ptr.p_u8, ctrl->fraction_bits);
  

  break; case V4L2_CTRL_TYPE_U16:

• pr_cont("%u", (unsigned)*ptr.p_u16);
  

• if (!ctrl->fraction_bits)
  

• 	pr_cont("%u", (unsigned int)*ptr.p_u16);
  

• else
  

• 	v4l2_ctrl_log_fp((unsigned int)*ptr.p_u16, ctrl->fraction_bits);
  

  break; case V4L2_CTRL_TYPE_U32:

• pr_cont("%u", (unsigned)*ptr.p_u32);
  

• if (!ctrl->fraction_bits)
  

• 	pr_cont("%u", (unsigned int)*ptr.p_u32);
  

• else
  

• 	v4l2_ctrl_log_fp((unsigned int)*ptr.p_u32, ctrl->fraction_bits);
  

  break; case V4L2_CTRL_TYPE_H264_SPS: pr_cont("H264_SPS");

@@ -1752,7 +1797,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, u32 id, const char *name, enum v4l2_ctrl_type type, s64 min, s64 max, u64 step, s64 def, const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size,

• 	u32 flags, const char * const *qmenu,
  

• 	u32 fraction_bits, u32 flags, const char * const *qmenu,
  const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def,
  void *priv)
  

{ @@ -1939,6 +1984,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, ctrl->name = name; ctrl->type = type; ctrl->flags = flags;

• ctrl->fraction_bits = fraction_bits; ctrl->minimum = min; ctrl->maximum = max; ctrl->step = step;

@@ -2037,7 +2083,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl, ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->type_ops, cfg->id, name, type, min, max, is_menu ? cfg->menu_skip_mask : step, def,

• 	cfg->dims, cfg->elem_size,
  

• 	cfg->dims, cfg->elem_size, cfg->fraction_bits,
  flags, qmenu, qmenu_int, cfg->p_def, priv);
  

  if (ctrl) ctrl->is_private = cfg->is_private;

@@ -2062,7 +2108,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

• 	     min, max, step, def, NULL, 0,
  

• 	     min, max, step, def, NULL, 0, 0,
       flags, NULL, NULL, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std); @@ -2095,7 +2141,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

• 	     0, max, mask, def, NULL, 0,
  

• 	     0, max, mask, def, NULL, 0, 0,
       flags, qmenu, qmenu_int, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); @@ -2127,7 +2173,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

• 	     0, max, mask, def, NULL, 0,
  

• 	     0, max, mask, def, NULL, 0, 0,
       flags, qmenu, NULL, ptr_null, NULL);
  
  

} @@ -2149,7 +2195,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

• 	     min, max, step, def, NULL, 0,
  

• 	     min, max, step, def, NULL, 0, 0,
       flags, NULL, NULL, p_def, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std_compound); @@ -2173,7 +2219,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

• 	     0, max, 0, def, NULL, 0,
  

• 	     0, max, 0, def, NULL, 0, 0,
       flags, NULL, qmenu_int, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 59679a42b3e7..c35514c5bf88 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -211,7 +211,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);

• except for dynamic arrays. In that case it is in the range of
  

• 1 to @p_array_alloc_elems.
  

• @dims: The size of each dimension.

• • @nr_of_dims:The number of dimensions in @dims.

• • @nr_of_dims: The number of dimensions in @dims.
• • @fraction_bits: The number of fraction bits for fixed point values.
  • @menu_skip_mask: The control's skip mask for menu controls. This makes it

  • easy to skip menu items that are not valid. If bit X is set,
    

  • then menu item X is skipped. Of course, this only works for
    

@@ -228,6 +229,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);

• :math:`ceil(\frac{maximum - minimum}{step}) + 1`.
  

• Used only if the @type is %V4L2_CTRL_TYPE_INTEGER_MENU.
  

• @flags: The control's flags.

• • @fraction_bits: The number of fraction bits for fixed point values.
  • @priv: The control's private pointer. For use by the driver. It is

  • untouched by the control framework. Note that this pointer is
    

  • not freed when the control is deleted. Should this be needed
    

@@ -286,6 +288,7 @@ struct v4l2_ctrl { u32 new_elems; u32 dims[V4L2_CTRL_MAX_DIMS]; u32 nr_of_dims;

• u32 fraction_bits; union { u64 step; u64 menu_skip_mask;

@@ -426,6 +429,7 @@ struct v4l2_ctrl_handler {

• @dims: The size of each dimension.
• @elem_size: The size in bytes of the control.
• @flags: The control's flags.

• • @fraction_bits: The number of fraction bits for fixed point values.
  • @menu_skip_mask: The control's skip mask for menu controls. This makes it

  • easy to skip menu items that are not valid. If bit X is set,
    

  • then menu item X is skipped. Of course, this only works for
    

@@ -455,6 +459,7 @@ struct v4l2_ctrl_config { u32 dims[V4L2_CTRL_MAX_DIMS]; u32 elem_size; u32 flags;

• u32 fraction_bits; u64 menu_skip_mask; const char * const *qmenu; const s64 *qmenu_int;

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c3d4e490ce7c..26ecac19722a 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1944,9 +1944,27 @@ struct v4l2_query_ext_ctrl { __u32 elems; __u32 nr_of_dims; __u32 dims[V4L2_CTRL_MAX_DIMS];

• __u32 reserved[32];

• __u32 fraction_bits;

u8 would suffice. Not that we'd be short of space but still...

• __u32 reserved[31];

};

+static inline __s64 v4l2_fp_compose(__s64 i, __s64 f, unsigned int fraction_bits) +{

• return (i << fraction_bits) + f;

+}

+static inline __s64 v4l2_fp_integer(__s64 v, unsigned int fraction_bits) +{

• return v / (1LL << fraction_bits);

Why not just:

return v >> fraction_bits;

I'd use macros so you could use whatever control types with this without casting. E.g.

#define V4L2_FP_INTEGER(v, fraction_bits) ((v) >> fraction_bits)

A more generic way to expose this could be to have base and exponent, the base being 2 in this case. Just an idea. This would of course be a little bit more difficult to use.

+}

+static inline __s64 v4l2_fp_fraction(__s64 v, unsigned int fraction_bits) +{

• __u64 mask = (1ULL << fraction_bits) - 1;
• return v < 0 ? -((-v) & mask) : (v & mask);

+}

/* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */ struct v4l2_querymenu { __u32 id;

On Fri, Nov 17, 2023 at 8:07 PM Hans Verkuil hverkuil@xs4all.nl wrote:

Here is an RFC patch adding support for 'fraction_bits'. It's lacking documentation, but it can be used for testing.

It was rather a pain logging fixed point number in a reasonable format, but I think it is OK.

In userspace (where you can use floating point) it is a lot easier:

printf("%.*g\n", fraction_bits, (double)v * (1.0 / (1ULL << fraction_bits)));

I decided to only expose fraction_bits in struct v4l2_query_ext_ctrl. I could add it to struct v4l2_queryctrl, but I did not think that was necessary. Other opinions are welcome.

In the meantime, let me know if this works for your patch series. If it does, then I can clean this up.

Thanks. It works for me. What I have done are: 1. drop FIXED_POINT 2. use v4l2_ctrl_new_custom

Best regards Wang shengjiu

Regards,

    Hans

Signed-off-by: Hans Verkuil hverkuil-cisco@xs4all.nl

drivers/media/v4l2-core/v4l2-ctrls-api.c | 1 + drivers/media/v4l2-core/v4l2-ctrls-core.c | 72 +++++++++++++++++++---- include/media/v4l2-ctrls.h | 7 ++- include/uapi/linux/videodev2.h | 20 ++++++- 4 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..3132df315b17 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -1101,6 +1101,7 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr qc->elems = ctrl->elems; qc->nr_of_dims = ctrl->nr_of_dims; memcpy(qc->dims, ctrl->dims, qc->nr_of_dims * sizeof(qc->dims[0]));

•   qc->fraction_bits = ctrl->fraction_bits;
    qc->minimum = ctrl->minimum;
    qc->maximum = ctrl->maximum;
    qc->default_value = ctrl->default_value;
  

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..0e08a371af5c 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c @@ -252,12 +252,42 @@ void v4l2_ctrl_type_op_init(const struct v4l2_ctrl *ctrl, u32 from_idx, } EXPORT_SYMBOL(v4l2_ctrl_type_op_init);

+static void v4l2_ctrl_log_fp(s64 v, unsigned int fraction_bits) +{

•   s64 i = v4l2_fp_integer(v, fraction_bits);
  

•   s64 f = v4l2_fp_fraction(v, fraction_bits);
  

•   if (!f) {
  

•           pr_cont("%lld", i);
  

•   } else if (fraction_bits < 20) {
  

•           u64 div = 1ULL << fraction_bits;
  

•           if (!i && f < 0)
  

•                   pr_cont("-%lld/%llu", -f, div);
  

•           else if (!i)
  

•                   pr_cont("%lld/%llu", f, div);
  

•           else if (i < 0 || f < 0)
  

•                   pr_cont("-%lld-%llu/%llu", -i, -f, div);
  

•           else
  

•                   pr_cont("%lld+%llu/%llu", i, f, div);
  

•   } else {
  

•           if (!i && f < 0)
  

•                   pr_cont("-%lld/(2^%u)", -f, fraction_bits);
  

•           else if (!i)
  

•                   pr_cont("%lld/(2^%u)", f, fraction_bits);
  

•           else if (i < 0 || f < 0)
  

•                   pr_cont("-%lld-%llu/(2^%u)", -i, -f, fraction_bits);
  

•           else
  

•                   pr_cont("%lld+%llu/(2^%u)", i, f, fraction_bits);
  

•   }
  

+}

void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) { union v4l2_ctrl_ptr ptr = ctrl->p_cur;

    if (ctrl->is_array) {

•           unsigned i;
  

•           unsigned int i;
  
            for (i = 0; i < ctrl->nr_of_dims; i++)
                    pr_cont("[%u]", ctrl->dims[i]);
  

@@ -266,7 +296,10 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl)

    switch (ctrl->type) {
    case V4L2_CTRL_TYPE_INTEGER:

•           pr_cont("%d", *ptr.p_s32);
  

•           if (!ctrl->fraction_bits)
  

•                   pr_cont("%d", *ptr.p_s32);
  

•           else
  

•                   v4l2_ctrl_log_fp(*ptr.p_s32, ctrl->fraction_bits);
            break;
    case V4L2_CTRL_TYPE_BOOLEAN:
            pr_cont("%s", *ptr.p_s32 ? "true" : "false");
  

@@ -281,19 +314,31 @@ void v4l2_ctrl_type_op_log(const struct v4l2_ctrl *ctrl) pr_cont("0x%08x", *ptr.p_s32); break; case V4L2_CTRL_TYPE_INTEGER64:

•           pr_cont("%lld", *ptr.p_s64);
  

•           if (!ctrl->fraction_bits)
  

•                   pr_cont("%lld", *ptr.p_s64);
  

•           else
  

•                   v4l2_ctrl_log_fp(*ptr.p_s64, ctrl->fraction_bits);
            break;
    case V4L2_CTRL_TYPE_STRING:
            pr_cont("%s", ptr.p_char);
            break;
    case V4L2_CTRL_TYPE_U8:
  

•           pr_cont("%u", (unsigned)*ptr.p_u8);
  

•           if (!ctrl->fraction_bits)
  

•                   pr_cont("%u", (unsigned int)*ptr.p_u8);
  

•           else
  

•                   v4l2_ctrl_log_fp((unsigned int)*ptr.p_u8, ctrl->fraction_bits);
            break;
    case V4L2_CTRL_TYPE_U16:
  

•           pr_cont("%u", (unsigned)*ptr.p_u16);
  

•           if (!ctrl->fraction_bits)
  

•                   pr_cont("%u", (unsigned int)*ptr.p_u16);
  

•           else
  

•                   v4l2_ctrl_log_fp((unsigned int)*ptr.p_u16, ctrl->fraction_bits);
            break;
    case V4L2_CTRL_TYPE_U32:
  

•           pr_cont("%u", (unsigned)*ptr.p_u32);
  

•           if (!ctrl->fraction_bits)
  

•                   pr_cont("%u", (unsigned int)*ptr.p_u32);
  

•           else
  

•                   v4l2_ctrl_log_fp((unsigned int)*ptr.p_u32, ctrl->fraction_bits);
            break;
    case V4L2_CTRL_TYPE_H264_SPS:
            pr_cont("H264_SPS");
  

@@ -1752,7 +1797,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, u32 id, const char *name, enum v4l2_ctrl_type type, s64 min, s64 max, u64 step, s64 def, const u32 dims[V4L2_CTRL_MAX_DIMS], u32 elem_size,

•                   u32 flags, const char * const *qmenu,
  

•                   u32 fraction_bits, u32 flags, const char * const *qmenu,
                    const s64 *qmenu_int, const union v4l2_ctrl_ptr p_def,
                    void *priv)
  

{ @@ -1939,6 +1984,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, ctrl->name = name; ctrl->type = type; ctrl->flags = flags;

•   ctrl->fraction_bits = fraction_bits;
    ctrl->minimum = min;
    ctrl->maximum = max;
    ctrl->step = step;
  

@@ -2037,7 +2083,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct v4l2_ctrl_handler *hdl, ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->type_ops, cfg->id, name, type, min, max, is_menu ? cfg->menu_skip_mask : step, def,

•                   cfg->dims, cfg->elem_size,
  

•                   cfg->dims, cfg->elem_size, cfg->fraction_bits,
                    flags, qmenu, qmenu_int, cfg->p_def, priv);
    if (ctrl)
            ctrl->is_private = cfg->is_private;
  

@@ -2062,7 +2108,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

•                        min, max, step, def, NULL, 0,
  

•                        min, max, step, def, NULL, 0, 0,
                         flags, NULL, NULL, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std); @@ -2095,7 +2141,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

•                        0, max, mask, def, NULL, 0,
  

•                        0, max, mask, def, NULL, 0, 0,
                         flags, qmenu, qmenu_int, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std_menu); @@ -2127,7 +2173,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

•                        0, max, mask, def, NULL, 0,
  

•                        0, max, mask, def, NULL, 0, 0,
                         flags, qmenu, NULL, ptr_null, NULL);
  
  

} @@ -2149,7 +2195,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_compound(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

•                        min, max, step, def, NULL, 0,
  

•                        min, max, step, def, NULL, 0, 0,
                         flags, NULL, NULL, p_def, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_std_compound); @@ -2173,7 +2219,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl, return NULL; } return v4l2_ctrl_new(hdl, ops, NULL, id, name, type,

•                        0, max, 0, def, NULL, 0,
  

•                        0, max, 0, def, NULL, 0, 0,
                         flags, NULL, qmenu_int, ptr_null, NULL);
  

} EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h index 59679a42b3e7..c35514c5bf88 100644 --- a/include/media/v4l2-ctrls.h +++ b/include/media/v4l2-ctrls.h @@ -211,7 +211,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);

•         except for dynamic arrays. In that case it is in the range of
  

•         1 to @p_array_alloc_elems.
  

• @dims: The size of each dimension.

• • @nr_of_dims:The number of dimensions in @dims.

• • @nr_of_dims: The number of dimensions in @dims.
• • @fraction_bits: The number of fraction bits for fixed point values.
  • @menu_skip_mask: The control's skip mask for menu controls. This makes it

  •         easy to skip menu items that are not valid. If bit X is set,
    

  •         then menu item X is skipped. Of course, this only works for
    

@@ -228,6 +229,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);

•         :math:`ceil(\frac{maximum - minimum}{step}) + 1`.
  

•         Used only if the @type is %V4L2_CTRL_TYPE_INTEGER_MENU.
  

• @flags: The control's flags.

• • @fraction_bits: The number of fraction bits for fixed point values.
  • @priv: The control's private pointer. For use by the driver. It is

  •         untouched by the control framework. Note that this pointer is
    

  •         not freed when the control is deleted. Should this be needed
    

@@ -286,6 +288,7 @@ struct v4l2_ctrl { u32 new_elems; u32 dims[V4L2_CTRL_MAX_DIMS]; u32 nr_of_dims;

•   u32 fraction_bits;
    union {
            u64 step;
            u64 menu_skip_mask;
  

@@ -426,6 +429,7 @@ struct v4l2_ctrl_handler {

• @dims: The size of each dimension.
• @elem_size: The size in bytes of the control.
• @flags: The control's flags.

• • @fraction_bits: The number of fraction bits for fixed point values.
  • @menu_skip_mask: The control's skip mask for menu controls. This makes it

  •         easy to skip menu items that are not valid. If bit X is set,
    

  •         then menu item X is skipped. Of course, this only works for
    

@@ -455,6 +459,7 @@ struct v4l2_ctrl_config { u32 dims[V4L2_CTRL_MAX_DIMS]; u32 elem_size; u32 flags;

•   u32 fraction_bits;
    u64 menu_skip_mask;
    const char * const *qmenu;
    const s64 *qmenu_int;
  

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c3d4e490ce7c..26ecac19722a 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1944,9 +1944,27 @@ struct v4l2_query_ext_ctrl { __u32 elems; __u32 nr_of_dims; __u32 dims[V4L2_CTRL_MAX_DIMS];

•   __u32                reserved[32];
  

•   __u32                fraction_bits;
  

•   __u32                reserved[31];
  

};

+static inline __s64 v4l2_fp_compose(__s64 i, __s64 f, unsigned int fraction_bits) +{

•   return (i << fraction_bits) + f;
  

+}

+static inline __s64 v4l2_fp_integer(__s64 v, unsigned int fraction_bits) +{

•   return v / (1LL << fraction_bits);
  

+}

+static inline __s64 v4l2_fp_fraction(__s64 v, unsigned int fraction_bits) +{

•   __u64 mask = (1ULL << fraction_bits) - 1;
  

•   return v < 0 ? -((-v) & mask) : (v & mask);
  

+}

/* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */ struct v4l2_querymenu { __u32 id; -- 2.42.0

On 10/11/2023 06:48, Shengjiu Wang wrote:

Fixed point controls are used by the user to configure a fixed point value in 64bits, which Q31.32 format.

Signed-off-by: Shengjiu Wang shengjiu.wang@nxp.com

.../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++++++------ .../userspace-api/media/v4l/vidioc-queryctrl.rst | 9 ++++++++- .../userspace-api/media/videodev2.h.rst.exceptions | 1 + drivers/media/v4l2-core/v4l2-ctrls-api.c | 5 ++++- drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ include/uapi/linux/videodev2.h | 1 + 6 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst index e8475f9fd2cf..e7e5d78dc11e 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst @@ -162,13 +162,13 @@ still cause this situation. * - __s32 - ``value`` - New value or current value. Valid if this control is not of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • __s64
    • ``value64``
    • New value or current value. Valid if this control is of type

• ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is
• not set.

• ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and
• ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set.
  • • char *
    • ``string``
    • A pointer to a string. Valid if this control is of type

@@ -193,8 +193,9 @@ still cause this situation. * - __s64 * - ``p_s64`` - A pointer to a matrix control of signed 64-bit values. Valid if

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and
  

•    ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set.
  

•    this control is of type ``V4L2_CTRL_TYPE_INTEGER64``,
  

•    ``V4L2_CTRL_TYPE_FIXED_POINT`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD``
  

•    is set.
  

  • • struct :c:type:`v4l2_area` *
    • ``p_area``
    • A pointer to a struct :c:type:`v4l2_area`. Valid if this control is

diff --git a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst index 4d38acafe8e1..f3995ec57044 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-queryctrl.rst @@ -235,7 +235,8 @@ See also the examples in :ref:`control`. - ``default_value`` - The default value of a ``V4L2_CTRL_TYPE_INTEGER``, ``_INTEGER64``, ``_BOOLEAN``, ``_BITMASK``, ``_MENU``, ``_INTEGER_MENU``, ``_U8``

• or ``_U16`` control. Not valid for other types of controls.

• ``_FIXED_POINT`` or ``_U16`` control. Not valid for other types of

• controls.

  .. note::

@@ -549,6 +550,12 @@ See also the examples in :ref:`control`. - n/a - A struct :c:type:`v4l2_ctrl_av1_film_grain`, containing AV1 Film Grain parameters for stateless video decoders.

• • • ``V4L2_CTRL_TYPE_FIXED_POINT``

•  - any
  

•  - any
  

•  - any
  

•  - A 64-bit integer valued control, containing parameter which is
  

•    Q31.32 format.
  
  

.. raw:: latex

diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions index e61152bb80d1..2faa5a2015eb 100644 --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type`

# V4L2 capability defines replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c index 002ea6588edf..e6a0fb8d6791 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c, return copy_to_user(c->string, ptr.p_char, len + 1) ? -EFAULT : 0; case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT:n c->value64 = *ptr.p_s64; break; default:

@@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c, struct v4l2_ctrl *ctrl)

switch (ctrl->type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: *ctrl->p_new.p_s64 = c->value64; break; case V4L2_CTRL_TYPE_STRING:

@@ -540,7 +542,8 @@ static int validate_ctrls(struct v4l2_ext_controls *cs, */ if (ctrl->is_ptr) continue;

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
  

• if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64 ||
  

•     ctrl->type == V4L2_CTRL_TYPE_FIXED_POINT)
  p_new.p_s64 = &cs->controls[i].value64;
  

  else p_new.p_s32 = &cs->controls[i].value;

You missed a few more: everywhere in this source where V4L2_CTRL_TYPE_INTEGER64 is used you need to add a check for FIXED_POINT as well.

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c index a662fb60f73f..9d50df0d9874 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c

Same here for this source.

Note that v4l2_ctrl_type_op_log() needs a separate case for FIXED_POINT so that it can be logged properly.

@@ -1187,6 +1187,7 @@ static int std_validate_elem(const struct v4l2_ctrl *ctrl, u32 idx, case V4L2_CTRL_TYPE_INTEGER: return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl); case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: /*
  • We can't use the ROUND_TO_RANGE define here due to
  • the u64 divide that needs special care.

@@ -1779,6 +1780,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl, /* Prefill elem_size for all types handled by std_type_ops */ switch ((u32)type) { case V4L2_CTRL_TYPE_INTEGER64:

• case V4L2_CTRL_TYPE_FIXED_POINT: elem_size = sizeof(s64); break; case V4L2_CTRL_TYPE_STRING:

There is one more place where V4L2_CTRL_TYPE_INTEGER64 is used: drivers/media/v4l2-core/v4l2-ioctl.c. That should report a fixed point value in the same way as v4l2_ctrl_type_op_log().

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index cf8c44595a1d..9482ac66a675 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1903,6 +1903,7 @@ enum v4l2_ctrl_type { V4L2_CTRL_TYPE_STRING = 7, V4L2_CTRL_TYPE_BITMASK = 8, V4L2_CTRL_TYPE_INTEGER_MENU = 9,

• V4L2_CTRL_TYPE_FIXED_POINT = 10,

  /* Compound types are >= 0x0100 */ V4L2_CTRL_COMPOUND_TYPES = 0x0100,

We will probably need a helper to create a fixed point value from the integer and fractional part, and one to extract them.

Regards,

Hans