Messages in this thread | | | Date | Tue, 27 Jun 2023 10:43:31 +0200 | Subject | Re: [PATCH v3 10/11] media: v4l2: Add DELETE_BUF ioctl | From | Benjamin Gaignard <> |
| |
Le 27/06/2023 à 09:30, Hsia-Jun Li a écrit : > > On 6/22/23 21:13, Benjamin Gaignard wrote: >> CAUTION: Email originated externally, do not click links or open >> attachments unless you recognize the sender and know the content is >> safe. >> >> >> VIDIOC_DELETE_BUF ioctl allows to delete a buffer from a queue. >> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> >> --- >> .../userspace-api/media/v4l/user-func.rst | 1 + >> .../media/v4l/vidioc-delete-buf.rst | 51 +++++++++++++++++++ >> .../media/common/videobuf2/videobuf2-core.c | 33 ++++++++++++ >> .../media/common/videobuf2/videobuf2-v4l2.c | 6 +++ >> drivers/media/v4l2-core/v4l2-dev.c | 1 + >> drivers/media/v4l2-core/v4l2-ioctl.c | 10 ++++ >> include/media/v4l2-ioctl.h | 4 ++ >> include/media/videobuf2-core.h | 9 ++++ >> include/media/videobuf2-v4l2.h | 11 ++++ >> include/uapi/linux/videodev2.h | 2 + >> 10 files changed, 128 insertions(+) >> create mode 100644 >> Documentation/userspace-api/media/v4l/vidioc-delete-buf.rst >> >> diff --git a/Documentation/userspace-api/media/v4l/user-func.rst >> b/Documentation/userspace-api/media/v4l/user-func.rst >> index 15ff0bf7bbe6..8c74016e12fd 100644 >> --- a/Documentation/userspace-api/media/v4l/user-func.rst >> +++ b/Documentation/userspace-api/media/v4l/user-func.rst >> @@ -17,6 +17,7 @@ Function Reference >> vidioc-dbg-g-chip-info >> vidioc-dbg-g-register >> vidioc-decoder-cmd >> + vidioc-delete-buf >> vidioc-dqevent >> vidioc-dv-timings-cap >> vidioc-encoder-cmd >> diff --git >> a/Documentation/userspace-api/media/v4l/vidioc-delete-buf.rst >> b/Documentation/userspace-api/media/v4l/vidioc-delete-buf.rst >> new file mode 100644 >> index 000000000000..0e7ce58f91bc >> --- /dev/null >> +++ b/Documentation/userspace-api/media/v4l/vidioc-delete-buf.rst >> @@ -0,0 +1,51 @@ >> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later >> +.. c:namespace:: V4L >> + >> +.. _VIDIOC_DELETE_BUF: >> + >> +************************ >> +ioctl VIDIOC_DELETE_BUF >> +************************ >> + >> +Name >> +==== >> + >> +VIDIOC_DELETE_BUF - Delete a buffer from a queue >> + >> +Synopsis >> +======== >> + >> +.. c:macro:: VIDIOC_DELETE_BUF >> + >> +``int ioctl(int fd, VIDIOC_DELETE_BUF, struct v4l2_buffer *argp)`` >> + >> +Arguments >> +========= >> + >> +``fd`` >> + File descriptor returned by :c:func:`open()`. >> + >> +``argp`` >> + Pointer to struct :c:type:`v4l2_buffer`. >> + >> +Description >> +=========== >> + >> +Applications can optionally call the :ref:`VIDIOC_DELETE_BUF` ioctl to >> +delete a buffer from a queue. >> + >> +The struct :c:type:`v4l2_buffer` structure is specified in >> +:ref:`buffer`. >> + >> +Return Value >> +============ >> + >> +On success 0 is returned, on error -1 and the ``errno`` variable is set >> +appropriately. The generic error codes are described at the >> +:ref:`Generic Error Codes <gen-errors>` chapter. >> + >> +EBUSY >> + File I/O is in progress. >> + >> +EINVAL >> + The buffer ``index`` doesn't exist in the queue. >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c >> b/drivers/media/common/videobuf2/videobuf2-core.c >> index 899783f67580..aa546c972c3d 100644 >> --- a/drivers/media/common/videobuf2/videobuf2-core.c >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c >> @@ -1637,6 +1637,39 @@ int vb2_core_prepare_buf(struct vb2_queue *q, >> unsigned int index, void *pb) >> } >> EXPORT_SYMBOL_GPL(vb2_core_prepare_buf); >> >> +int vb2_core_delete_buf(struct vb2_queue *q, unsigned int index) >> +{ >> + struct vb2_buffer *vb; >> + >> + vb = vb2_get_buffer(q, index); >> + if (!vb) { >> + dprintk(q, 1, "invalid buffer index %d\n", index); >> + return -EINVAL; >> + } >> + >> + if (vb->state != VB2_BUF_STATE_DEQUEUED) { >> + dprintk(q, 1, "can't delete non dequeued buffer index >> %d\n", index); >> + return -EINVAL; >> + } >> + > I know the driver could implement its own > v4l2_ioctl_ops->vidioc_delete_buf() that check whether a buffer is > used by the hardware as a future reference frame. > But I think we need a flag to let the user know which buffer is still > used by the hardware. > Alternative ref case is safe, we only know it's existing when it is > dequeued in current V4L2 buffer mechanism. > While the Golden reference frame, such long term reference frame could > last much longer.
It is up to userland stack to know frames life time, it got the information for that.
>> + if (vb->planes[0].mem_priv) >> + call_void_vb_qop(vb, buf_cleanup, vb); >> + >> + /* Free MMAP buffers or release USERPTR buffers */ >> + if (q->memory == VB2_MEMORY_MMAP) >> + __vb2_buf_mem_free(vb); >> + else if (q->memory == VB2_MEMORY_DMABUF) >> + __vb2_buf_dmabuf_put(vb); >> + else >> + __vb2_buf_userptr_put(vb); >> + >> + vb2_queue_remove_buffer(q, vb); >> + kfree(vb); >> + >> + dprintk(q, 2, "buffer %d deleted\n", index); >> + return 0; >> +} >> + >> /* >> * vb2_start_streaming() - Attempt to start streaming. >> * @q: videobuf2 queue >> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c >> b/drivers/media/common/videobuf2/videobuf2-v4l2.c >> index 724135d41f7f..cea666c17b41 100644 >> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c >> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c >> @@ -751,6 +751,12 @@ int vb2_prepare_buf(struct vb2_queue *q, struct >> media_device *mdev, >> } >> EXPORT_SYMBOL_GPL(vb2_prepare_buf); >> >> +int vb2_delete_buf(struct vb2_queue *q, struct v4l2_buffer *b) >> +{ >> + return vb2_core_delete_buf(q, b->index); >> +} >> +EXPORT_SYMBOL_GPL(vb2_delete_buf); >> + >> int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers >> *create) >> { >> unsigned requested_planes = 1; >> diff --git a/drivers/media/v4l2-core/v4l2-dev.c >> b/drivers/media/v4l2-core/v4l2-dev.c >> index f81279492682..80ace2e1e932 100644 >> --- a/drivers/media/v4l2-core/v4l2-dev.c >> +++ b/drivers/media/v4l2-core/v4l2-dev.c >> @@ -720,6 +720,7 @@ static void determine_valid_ioctls(struct >> video_device *vdev) >> SET_VALID_IOCTL(ops, VIDIOC_PREPARE_BUF, >> vidioc_prepare_buf); >> SET_VALID_IOCTL(ops, VIDIOC_STREAMON, vidioc_streamon); >> SET_VALID_IOCTL(ops, VIDIOC_STREAMOFF, >> vidioc_streamoff); >> + SET_VALID_IOCTL(ops, VIDIOC_DELETE_BUF, >> vidioc_delete_buf); >> } >> >> if (is_vid || is_vbi || is_meta) { >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c >> b/drivers/media/v4l2-core/v4l2-ioctl.c >> index a858acea6547..1c737279d3ef 100644 >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c >> @@ -2156,6 +2156,15 @@ static int v4l_prepare_buf(const struct >> v4l2_ioctl_ops *ops, >> return ret ? ret : ops->vidioc_prepare_buf(file, fh, b); >> } >> >> +static int v4l_delete_buf(const struct v4l2_ioctl_ops *ops, >> + struct file *file, void *fh, void *arg) >> +{ >> + struct v4l2_buffer *b = arg; >> + int ret = check_fmt(file, b->type); >> + >> + return ret ? ret : ops->vidioc_delete_buf(file, fh, b); >> +} >> + >> static int v4l_g_parm(const struct v4l2_ioctl_ops *ops, >> struct file *file, void *fh, void *arg) >> { >> @@ -2905,6 +2914,7 @@ static const struct v4l2_ioctl_info >> v4l2_ioctls[] = { >> IOCTL_INFO(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, >> v4l_print_freq_band, 0), >> IOCTL_INFO(VIDIOC_DBG_G_CHIP_INFO, v4l_dbg_g_chip_info, >> v4l_print_dbg_chip_info, INFO_FL_CLEAR(v4l2_dbg_chip_info, match)), >> IOCTL_INFO(VIDIOC_QUERY_EXT_CTRL, v4l_query_ext_ctrl, >> v4l_print_query_ext_ctrl, INFO_FL_CTRL | >> INFO_FL_CLEAR(v4l2_query_ext_ctrl, id)), >> + IOCTL_INFO(VIDIOC_DELETE_BUF, v4l_delete_buf, >> v4l_print_buffer, INFO_FL_QUEUE), >> }; >> #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) >> >> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h >> index edb733f21604..2f232ed884c7 100644 >> --- a/include/media/v4l2-ioctl.h >> +++ b/include/media/v4l2-ioctl.h >> @@ -163,6 +163,8 @@ struct v4l2_fh; >> * :ref:`VIDIOC_CREATE_BUFS <vidioc_create_bufs>` ioctl >> * @vidioc_prepare_buf: pointer to the function that implements >> * :ref:`VIDIOC_PREPARE_BUF <vidioc_prepare_buf>` ioctl >> + * @vidioc_delete_buf: pointer to the function that implements >> + * :ref:`VIDIOC_DELETE_BUF <vidioc_delete_buf>` ioctl >> * @vidioc_overlay: pointer to the function that implements >> * :ref:`VIDIOC_OVERLAY <vidioc_overlay>` ioctl >> * @vidioc_g_fbuf: pointer to the function that implements >> @@ -422,6 +424,8 @@ struct v4l2_ioctl_ops { >> struct v4l2_create_buffers *b); >> int (*vidioc_prepare_buf)(struct file *file, void *fh, >> struct v4l2_buffer *b); >> + int (*vidioc_delete_buf)(struct file *file, void *fh, >> + struct v4l2_buffer *b); >> >> int (*vidioc_overlay)(struct file *file, void *fh, unsigned >> int i); >> int (*vidioc_g_fbuf)(struct file *file, void *fh, >> diff --git a/include/media/videobuf2-core.h >> b/include/media/videobuf2-core.h >> index 080b783d608d..0f9e68f76b77 100644 >> --- a/include/media/videobuf2-core.h >> +++ b/include/media/videobuf2-core.h >> @@ -840,6 +840,15 @@ int vb2_core_create_bufs(struct vb2_queue *q, >> enum vb2_memory memory, >> */ >> int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, >> void *pb); >> >> +/** >> + * vb2_core_delete_buf() - >> + * @q: pointer to &struct vb2_queue with videobuf2 queue. >> + * @index: id number of the buffer. >> + * >> + * Return: returns zero on success; an error code otherwise. >> + */ >> +int vb2_core_delete_buf(struct vb2_queue *q, unsigned int index); >> + >> /** >> * vb2_core_qbuf() - Queue a buffer from userspace >> * >> diff --git a/include/media/videobuf2-v4l2.h >> b/include/media/videobuf2-v4l2.h >> index 88a7a565170e..3beeb4c735f0 100644 >> --- a/include/media/videobuf2-v4l2.h >> +++ b/include/media/videobuf2-v4l2.h >> @@ -114,6 +114,17 @@ int vb2_create_bufs(struct vb2_queue *q, struct >> v4l2_create_buffers *create); >> */ >> int vb2_prepare_buf(struct vb2_queue *q, struct media_device *mdev, >> struct v4l2_buffer *b); >> +/** >> + * vb2_delete_buf() - Delete the buffer from the queue >> + * >> + * @q: pointer to &struct vb2_queue with videobuf2 queue. >> + * @b: buffer structure passed from userspace to >> + * &v4l2_ioctl_ops->vidioc_delete_buf handler in driver >> + * >> + * The return values from this function are intended to be directly >> returned >> + * from &v4l2_ioctl_ops->vidioc_delete_buf handler in driver. >> + */ >> +int vb2_delete_buf(struct vb2_queue *q, struct v4l2_buffer *b); >> >> /** >> * vb2_qbuf() - Queue a buffer from userspace >> diff --git a/include/uapi/linux/videodev2.h >> b/include/uapi/linux/videodev2.h >> index aee75eb9e686..31bba1915642 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -2702,6 +2702,8 @@ struct v4l2_create_buffers { >> #define VIDIOC_DBG_G_CHIP_INFO _IOWR('V', 102, struct >> v4l2_dbg_chip_info) >> >> #define VIDIOC_QUERY_EXT_CTRL _IOWR('V', 103, struct >> v4l2_query_ext_ctrl) >> +#define VIDIOC_DELETE_BUF _IOWR('V', 104, struct v4l2_buffer) >> + >> >> /* Reminder: when adding new ioctls please add support for them to >> drivers/media/v4l2-core/v4l2-compat-ioctl32.c as well! */ >> -- >> 2.39.2 >>
| |