Re: [RFC PATCH v6 00/11] media: v4l2: Add extended fmt and buffer ioctls

From: Helen Koike
Date: Fri Feb 05 2021 - 13:43:04 EST

Next message: Jarkko Sakkinen: "[PATCH 2/2] x86/sgx: Maintain encl->refcount for each encl->mm_list entry"
Previous message: Johannes Weiner: "[PATCH 5/8] cgroup: rstat: punt root-level optimization to individual controllers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hello,

On 1/14/21 3:07 PM, Helen Koike wrote:
> Hello,
>
> This is v6 of the Extended API for formats and buffers (see below the new API).
>
> The new API comes for free for old drivers through the conversion layer, which
> is independent of vb2.
>
> I completly refactored several patches. I would like to request comments not
> only in the uAPI, but also the kAPI for drivers, and I would appreciate any
> ideas on improving the quality of the code (in short: please review everything).
>
> NOTE: The Ext API wans't tested yet. My next step is to patch v4l2-compliance.

I implemented on libcamera to test it, please check:

https://lists.libcamera.org/pipermail/libcamera-devel/2021-February/017169.html

Thanks,
Helen

>
> Regression tests - v4l2-compliance with test-media script:
> vivid: http://ix.io/2M0G - Final Summary: 1856, Succeeded: 1856, Failed: 0, Warnings: 0)
> vimc: http://ix.io/2M0I - Final Summary: 488, Succeeded: 488, Failed: 0, Warnings: 0
>
> Git: https://gitlab.collabora.com/koike/linux/-/tree/v4l2/ext-api/v6
>
> v5: https://patchwork.linuxtv.org/project/linux-media/cover/20200804192939.2251988-1-helen.koike@xxxxxxxxxxxxx/
> v4: https://patchwork.linuxtv.org/project/linux-media/cover/20200717115435.2632623-1-helen.koike@xxxxxxxxxxxxx/
> v3: https://patchwork.linuxtv.org/cover/59345/
> v2: https://patchwork.kernel.org/project/linux-media/list/?series=101153
> v1: https://patchwork.kernel.org/project/linux-media/list/?series=93707
>
> Conversion layer:
> =================
>
> * Old drivers implementing only ops->vidioc_*_fmt_vid_cap supports
> VIDIOC_*_EXT_PIX_FMT automatically with limitations[1].
>
> * New drivers implementing only ops->vidioc_*_ext_pix_fmt_vid_cap supports
> VIDIOC_*_FMT automatically.
>
> * Old drivers implementing only ops->vidioc_*buf support
> VIDIOC_EXT_*BUF automatically with limitations[2].
>
> * New drivers should implement both ops->vidioc_*buf and ops->vidioc_*buf
> to overcome limitations[2] and support both APIs.
> Which is easy with vb2:
> static const struct v4l2_ioctl_ops ioctl_ops = {
> ...
> + .vidioc_ext_qbuf = vb2_ioctl_ext_qbuf,
> + .vidioc_ext_dqbuf = vb2_ioctl_ext_dqbuf,
> ...
> }
> ...
> + /* Inform vb2 how to split the memory buffer in case a single one is used */
> + vb2_set_pixelformat(dev->pixelformat)
>
> [1] There are some limitations in the conversion such as modifiers that are
> ignored when converting v4l2_ext_pix_format to v4l_format
>
> [2] Ext API allows a single buffer with planes placed in random locations,
> which is not possible with v4l2_buffer.
>
>
> Major changes in v6:
> ====================
>
> Fixed color planes vs memory planes handling.
>
> Removed VIDIOC_EXT_PREPARE_BUF, since this is an optimization, it doesn't blocks
> the API, we can add it later (my goal was to simplify this patchset).
>
> Removed VIDIOC_EXT_CREATE_BUFS, since this is useful only to MMAP (thus low priority)
> with the new format.
> Classic VIDIOC_CREATE_BUFS and VIDIOC_REQBUFS can still be used.
>
> Reformulated conversion layer as per above.
>
> Removed conversions in vb2, it is easier to add hooks to drivers.
>
> Fixed vb2 to allow Ext API only to Video types.
>
> API updates:
> * remove buffer and plane lengths
> * move `memory` field to v4l2_ext_buffer instead of v4l2_ext_plane
> * remove struct v4l2_plane_ext_pix_format
> * reordering
>
> Make Ext API valid only for Video types, and not for touch, vbi, meta, etc.
>
> Sereval code refactoring, simplification, fixes and applied suggestions from v5.
>
> New API (for convenience):
> ==========================
>
> int ioctl(int fd, VIDIOC_G_EXT_PIX_FMT, struct v4l2_ext_pix_format *argp)
> int ioctl(int fd, VIDIOC_S_EXT_PIX_FMT, struct v4l2_ext_pix_format *argp)
> int ioctl(int fd, VIDIOC_TRY_EXT_PIX_FMT, struct v4l2_ext_pix_format *argp)
> int ioctl(int fd, VIDIOC_EXT_QBUF, struct v4l2_ext_buffer *argp)
> int ioctl(int fd, VIDIOC_EXT_DQBUF, struct v4l2_ext_buffer *argp)
>
> struct v4l2_ext_pix_format {
> __u32 type;
> __u32 width;
> __u32 height;
> __u32 field;
> struct v4l2_plane_pix_format plane_fmt[VIDEO_MAX_PLANES];
> __u32 pixelformat;
> __u64 modifier;
> __u32 colorspace;
> __u32 xfer_func;
> union {
> __u32 ycbcr_enc;
> __u32 hsv_enc;
> };
> __u32 quantization;
> __u32 reserved[9];
> };
>
> struct v4l2_ext_buffer {
> __u32 index;
> __u32 type;
> __u32 field;
> __u32 sequence;
> __u64 flags;
> __u64 timestamp;
> __u32 memory;
> __s32 request_fd;
> struct v4l2_ext_plane planes[VIDEO_MAX_PLANES];
> __u32 reserved[10];
> };
>
> struct v4l2_ext_plane {
> __u32 offset;
> __u32 bytesused;
> union {
> __u32 mmap_offset;
> __u64 userptr;
> __s32 dmabuf_fd;
> } m;
> __u32 reserved[6];
> };
>
> Helen Koike (11):
> media: v4l2-common: add normalized pixelformat field to struct
> v4l2_format_info
> media: v4l2: Extend pixel formats to unify single/multi-planar
> handling (and more)
> media: v4l2: Add extended buffer (de)queue operations for video types
> media: videobuf2-v4l2: reorganize flags handling
> media: videobuf2: Expose helpers for Ext qbuf/dqbuf
> media: vivid: use vb2_ioctls_ext_{d}qbuf hooks
> media: vimc: use vb2_ioctls_ext_{d}qbuf hooks
> media: mediabus: Add helpers to convert a ext_pix format to/from a
> mbus_fmt
> media: vivid: Convert to v4l2_ext_pix_format
> media: vimc: Convert to v4l2_ext_pix_format
> media: docs: add documentation for the Extended API
>
> .../userspace-api/media/v4l/buffer.rst | 5 +
> .../userspace-api/media/v4l/common.rst | 1 +
> .../userspace-api/media/v4l/dev-capture.rst | 6 +
> .../userspace-api/media/v4l/dev-output.rst | 6 +
> .../userspace-api/media/v4l/ext-api.rst | 89 +++
> .../userspace-api/media/v4l/format.rst | 18 +-
> .../userspace-api/media/v4l/user-func.rst | 5 +
> .../media/v4l/vidioc-ext-qbuf.rst | 188 +++++
> .../media/v4l/vidioc-g-ext-pix-fmt.rst | 116 +++
> .../userspace-api/media/v4l/vidioc-qbuf.rst | 2 +-
> .../media/common/videobuf2/videobuf2-core.c | 46 +-
> .../media/common/videobuf2/videobuf2-v4l2.c | 500 +++++++++---
> .../media/test-drivers/vimc/vimc-capture.c | 57 +-
> drivers/media/test-drivers/vimc/vimc-common.c | 6 +-
> drivers/media/test-drivers/vimc/vimc-common.h | 2 +-
> drivers/media/test-drivers/vivid/vivid-core.c | 209 ++---
> .../media/test-drivers/vivid/vivid-vid-cap.c | 203 ++---
> .../media/test-drivers/vivid/vivid-vid-cap.h | 15 +-
> .../media/test-drivers/vivid/vivid-vid-out.c | 198 ++---
> .../media/test-drivers/vivid/vivid-vid-out.h | 15 +-
> drivers/media/v4l2-core/v4l2-common.c | 16 +-
> drivers/media/v4l2-core/v4l2-dev.c | 31 +-
> drivers/media/v4l2-core/v4l2-ioctl.c | 722 +++++++++++++++++-
> include/media/v4l2-common.h | 3 +
> include/media/v4l2-ioctl.h | 36 +
> include/media/v4l2-mediabus.h | 42 +
> include/media/videobuf2-core.h | 33 +-
> include/media/videobuf2-v4l2.h | 8 +-
> include/uapi/linux/videodev2.h | 96 +++
> 29 files changed, 2131 insertions(+), 543 deletions(-)
> create mode 100644 Documentation/userspace-api/media/v4l/ext-api.rst
> create mode 100644 Documentation/userspace-api/media/v4l/vidioc-ext-qbuf.rst
> create mode 100644 Documentation/userspace-api/media/v4l/vidioc-g-ext-pix-fmt.rst
>

Next message: Jarkko Sakkinen: "[PATCH 2/2] x86/sgx: Maintain encl->refcount for each encl->mm_list entry"
Previous message: Johannes Weiner: "[PATCH 5/8] cgroup: rstat: punt root-level optimization to individual controllers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]