VP9 dynamic resolution change use case requires to change resolution
without doing stream off/on to keep references frames.
In consequence the numbers of buffers increase until all 'old'
reference frames are deprecated.
To make it possible this series remove the 32 buffers limit per queue
and introduce DELETE_BUF ioctl to delete buffers from a queue without
doing stream off/on sequence.
change in version 2:
- Use a dynamic array and not a list to keep trace of allocated buffers.
Not use IDR interface because it is marked as deprecated in kernel
documentation.
- Add a module parameter to limit the number of buffer per queue.
- Add DELETE_BUF ioctl and m2m helpers.
Benjamin Gaignard (8):
media: videobuf2: Access vb2_queue bufs array through helper functions
media: videobuf2: Make bufs array dynamic allocated
media: videobuf2: Add a module param to limit vb2 queue buffer storage
media: videobuf2: Stop define VB2_MAX_FRAME as global
media: v4l2: Add DELETE_BUF ioctl
media: v4l2: Add mem2mem helpers for DELETE_BUF ioctl
media: vim2m: Use v4l2-mem2mem helpers for VIDIOC_DELETE_BUF ioctl
media: verisilicon: Use v4l2-mem2mem helpers for VIDIOC_DELETE_BUF
ioctl
.../userspace-api/media/v4l/user-func.rst | 1 +
.../media/v4l/vidioc-delete-buf.rst | 51 ++++++
.../media/common/videobuf2/videobuf2-core.c | 168 +++++++++++++-----
.../media/common/videobuf2/videobuf2-v4l2.c | 23 ++-
drivers/media/platform/amphion/vdec.c | 1 +
drivers/media/platform/amphion/vpu_dbg.c | 4 +-
.../platform/mediatek/jpeg/mtk_jpeg_core.c | 2 +-
.../vcodec/vdec/vdec_vp9_req_lat_if.c | 4 +-
drivers/media/platform/qcom/venus/hfi.h | 2 +
.../media/platform/verisilicon/hantro_hw.h | 2 +
.../media/platform/verisilicon/hantro_v4l2.c | 1 +
drivers/media/test-drivers/vim2m.c | 1 +
drivers/media/test-drivers/visl/visl-dec.c | 16 +-
drivers/media/v4l2-core/v4l2-dev.c | 1 +
drivers/media/v4l2-core/v4l2-ioctl.c | 10 ++
drivers/media/v4l2-core/v4l2-mem2mem.c | 20 +++
.../staging/media/atomisp/pci/atomisp_ioctl.c | 2 +-
drivers/staging/media/ipu3/ipu3-v4l2.c | 2 +
include/media/v4l2-ioctl.h | 4 +
include/media/v4l2-mem2mem.h | 12 ++
include/media/videobuf2-core.h | 84 ++++++++-
include/media/videobuf2-v4l2.h | 15 +-
include/uapi/linux/videodev2.h | 1 +
23 files changed, 353 insertions(+), 74 deletions(-)
create mode 100644 Documentation/userspace-api/media/v4l/vidioc-delete-buf.rst
--
2.34.1
After changing bufs arrays to a dynamic allocated array
VB2_MAX_FRAME doesn't mean anything for videobuf2 core.
Remove it from the core definitions but keep it for drivers internal
needs.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/media/common/videobuf2/videobuf2-core.c | 2 ++
drivers/media/platform/amphion/vdec.c | 1 +
.../media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c | 2 ++
drivers/media/platform/qcom/venus/hfi.h | 2 ++
drivers/media/platform/verisilicon/hantro_hw.h | 2 ++
drivers/staging/media/ipu3/ipu3-v4l2.c | 2 ++
include/media/videobuf2-core.h | 1 -
include/media/videobuf2-v4l2.h | 4 ----
8 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f4da917ccf3f..3c6ced360770 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -31,6 +31,8 @@
#include <trace/events/vb2.h>
+#define VB2_MAX_FRAME 32
+
static int debug;
module_param(debug, int, 0644);
diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
index 87f9f8e90ab1..bef0c1b869be 100644
--- a/drivers/media/platform/amphion/vdec.c
+++ b/drivers/media/platform/amphion/vdec.c
@@ -28,6 +28,7 @@
#define VDEC_MIN_BUFFER_CAP 8
#define VDEC_MIN_BUFFER_OUT 8
+#define VB2_MAX_FRAME 32
struct vdec_fs_info {
char name[8];
diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
index f5958b6d834a..ba208caf3043 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
@@ -16,6 +16,8 @@
#include "../vdec_drv_if.h"
#include "../vdec_vpu_if.h"
+#define VB2_MAX_FRAME 32
+
/* reset_frame_context defined in VP9 spec */
#define VP9_RESET_FRAME_CONTEXT_NONE0 0
#define VP9_RESET_FRAME_CONTEXT_NONE1 1
diff --git a/drivers/media/platform/qcom/venus/hfi.h b/drivers/media/platform/qcom/venus/hfi.h
index f25d412d6553..bd5ca5a8b945 100644
--- a/drivers/media/platform/qcom/venus/hfi.h
+++ b/drivers/media/platform/qcom/venus/hfi.h
@@ -10,6 +10,8 @@
#include "hfi_helper.h"
+#define VB2_MAX_FRAME 32
+
#define VIDC_SESSION_TYPE_VPE 0
#define VIDC_SESSION_TYPE_ENC 1
#define VIDC_SESSION_TYPE_DEC 2
diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
index e83f0c523a30..9e8faf7ba6fb 100644
--- a/drivers/media/platform/verisilicon/hantro_hw.h
+++ b/drivers/media/platform/verisilicon/hantro_hw.h
@@ -15,6 +15,8 @@
#include <media/v4l2-vp9.h>
#include <media/videobuf2-core.h>
+#define VB2_MAX_FRAME 32
+
#define DEC_8190_ALIGN_MASK 0x07U
#define MB_DIM 16
diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
index e530767e80a5..6627b5c2d4d6 100644
--- a/drivers/staging/media/ipu3/ipu3-v4l2.c
+++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
@@ -10,6 +10,8 @@
#include "ipu3.h"
#include "ipu3-dmamap.h"
+#define VB2_MAX_FRAME 32
+
/******************** v4l2_subdev_ops ********************/
#define IPU3_RUNNING_MODE_VIDEO 0
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index b8b34a993e04..4aa43c5c7c58 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -21,7 +21,6 @@
#include <media/media-request.h>
#include <media/frame_vector.h>
-#define VB2_MAX_FRAME (32)
#define VB2_MAX_PLANES (8)
/**
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 5a845887850b..88a7a565170e 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -15,10 +15,6 @@
#include <linux/videodev2.h>
#include <media/videobuf2-core.h>
-#if VB2_MAX_FRAME != VIDEO_MAX_FRAME
-#error VB2_MAX_FRAME != VIDEO_MAX_FRAME
-#endif
-
#if VB2_MAX_PLANES != VIDEO_MAX_PLANES
#error VB2_MAX_PLANES != VIDEO_MAX_PLANES
#endif
--
2.34.1
Create v4l2-mem2mem helpers for VIDIOC_DELETE_BUF ioctl.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/media/v4l2-core/v4l2-mem2mem.c | 20 ++++++++++++++++++++
include/media/v4l2-mem2mem.h | 12 ++++++++++++
2 files changed, 32 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 0cc30397fbad..42f51ca25379 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -831,6 +831,17 @@ int v4l2_m2m_prepare_buf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
}
EXPORT_SYMBOL_GPL(v4l2_m2m_prepare_buf);
+int v4l2_m2m_delete_buf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
+ struct v4l2_buffer *buf)
+{
+ struct vb2_queue *vq;
+
+ vq = v4l2_m2m_get_vq(m2m_ctx, buf->type);
+
+ return vb2_delete_buf(vq, buf);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_delete_buf);
+
int v4l2_m2m_create_bufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
struct v4l2_create_buffers *create)
{
@@ -1377,6 +1388,15 @@ int v4l2_m2m_ioctl_create_bufs(struct file *file, void *priv,
}
EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_create_bufs);
+int v4l2_m2m_ioctl_delete_buf(struct file *file, void *priv,
+ struct v4l2_buffer *buf)
+{
+ struct v4l2_fh *fh = file->private_data;
+
+ return v4l2_m2m_delete_buf(file, fh->m2m_ctx, buf);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_delete_buf);
+
int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
struct v4l2_buffer *buf)
{
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index bb9de6a899e0..96f1b1f3b840 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -381,6 +381,16 @@ int v4l2_m2m_dqbuf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
int v4l2_m2m_prepare_buf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
struct v4l2_buffer *buf);
+/**
+ * v4l2_m2m_delete_buf() - delete buffer from the queue
+ *
+ * @file: pointer to struct &file
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @buf: pointer to struct &v4l2_buffer
+ */
+int v4l2_m2m_delete_buf(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
+ struct v4l2_buffer *buf);
+
/**
* v4l2_m2m_create_bufs() - create a source or destination buffer, depending
* on the type
@@ -846,6 +856,8 @@ int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
struct v4l2_requestbuffers *rb);
int v4l2_m2m_ioctl_create_bufs(struct file *file, void *fh,
struct v4l2_create_buffers *create);
+int v4l2_m2m_ioctl_delete_buf(struct file *file, void *priv,
+ struct v4l2_buffer *buf);
int v4l2_m2m_ioctl_querybuf(struct file *file, void *fh,
struct v4l2_buffer *buf);
int v4l2_m2m_ioctl_expbuf(struct file *file, void *fh,
--
2.34.1
VIDIOC_DELETE_BUF ioctl allows to delete a buffer from a queue.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
.../userspace-api/media/v4l/user-func.rst | 1 +
.../media/v4l/vidioc-delete-buf.rst | 51 ++++++++++++++++
.../media/common/videobuf2/videobuf2-core.c | 59 ++++++++++++++++++-
.../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 | 1 +
10 files changed, 152 insertions(+), 1 deletion(-)
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 228c1521f190..93e0a6a117fc 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 3c6ced360770..ec241d726fe6 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -401,6 +401,24 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb)
vb->skip_cache_sync_on_finish = 1;
}
+/*
+ * __vb2_queue_get_free_index() - find a free index in the queue for vb2 buffer.
+ *
+ * Returns an index for vb2 buffer.
+ */
+static int __vb2_queue_get_free_index(struct vb2_queue *q)
+{
+ int i;
+
+ spin_lock(&q->bufs_lock);
+ for (i = 0; i < q->max_num_bufs; i++)
+ if (!q->bufs[i])
+ break;
+ spin_unlock(&q->bufs_lock);
+
+ return i;
+}
+
/*
* __vb2_queue_alloc() - allocate vb2 buffer structures and (for MMAP type)
* video buffer memory for all buffers/planes on the queue and initializes the
@@ -427,7 +445,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
vb->state = VB2_BUF_STATE_DEQUEUED;
vb->vb2_queue = q;
vb->num_planes = num_planes;
- vb->index = q->num_buffers + buffer;
+ vb->index = __vb2_queue_get_free_index(q);
vb->type = q->type;
vb->memory = memory;
init_buffer_cache_hints(q, vb);
@@ -1570,6 +1588,45 @@ 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_QUEUED) {
+ dprintk(q, 1, "can't delete queued buffer index %d\n", index);
+ return -EINVAL;
+ }
+
+ if (vb && 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);
+
+ q->num_buffers--;
+ if (!q->num_buffers) {
+ q->memory = VB2_MEMORY_UNKNOWN;
+ INIT_LIST_HEAD(&q->queued_list);
+ }
+
+ 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 01b2bb957239..7d0b4ed48a06 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -742,6 +742,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 397d553177fa..4f49d2115838 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -719,6 +719,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 87f163a89c80..2e0af50cac45 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2152,6 +2152,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)
{
@@ -2902,6 +2911,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 4aa43c5c7c58..16e95278f6e8 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -847,6 +847,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 17a9b975177a..9cee9ccf1da8 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2681,6 +2681,7 @@ struct v4l2_create_buffers {
#define VIDIOC_QUERY_DV_TIMINGS _IOR('V', 99, struct v4l2_dv_timings)
#define VIDIOC_DV_TIMINGS_CAP _IOWR('V', 100, struct v4l2_dv_timings_cap)
#define VIDIOC_ENUM_FREQ_BANDS _IOWR('V', 101, struct v4l2_frequency_band)
+#define VIDIOC_DELETE_BUF _IOWR('V', 102, struct v4l2_buffer)
/*
* Experimental, meant for debugging, testing and internal use.
--
2.34.1
Make Hantro decoder support VIDIOC_DELETE_BUF ioctl.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/media/platform/verisilicon/hantro_v4l2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index d238d407f986..8f1414085f47 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -740,6 +740,7 @@ const struct v4l2_ioctl_ops hantro_ioctl_ops = {
.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
+ .vidioc_delete_buf = v4l2_m2m_ioctl_delete_buf,
.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
--
2.34.1
Make vim2m support VIDIOC_DELETE_BUF ioctl.
Signed-off-by: Benjamin Gaignard <[email protected]>
---
drivers/media/test-drivers/vim2m.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/test-drivers/vim2m.c b/drivers/media/test-drivers/vim2m.c
index 7964426bf2f7..3500a3df66c8 100644
--- a/drivers/media/test-drivers/vim2m.c
+++ b/drivers/media/test-drivers/vim2m.c
@@ -960,6 +960,7 @@ static const struct v4l2_ioctl_ops vim2m_ioctl_ops = {
.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
+ .vidioc_delete_buf = v4l2_m2m_ioctl_delete_buf,
.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
.vidioc_streamon = v4l2_m2m_ioctl_streamon,
--
2.34.1
Hi Benjamin,
I love your patch! Yet something to improve:
[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v6.3-rc3 next-20230321]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
base: git://linuxtv.org/media_tree.git master
patch link: https://lore.kernel.org/r/20230321102855.346732-6-benjamin.gaignard%40collabora.com
patch subject: [PATCH v2 5/8] media: v4l2: Add DELETE_BUF ioctl
config: x86_64-randconfig-a002-20230320 (https://download.01.org/0day-ci/archive/20230322/[email protected]/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/c33ab7329647eb04482423ac9945dee579038e84
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Benjamin-Gaignard/media-videobuf2-Access-vb2_queue-bufs-array-through-helper-functions/20230321-183154
git checkout c33ab7329647eb04482423ac9945dee579038e84
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "vb2_core_delete_buf" [drivers/media/common/videobuf2/videobuf2-v4l2.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On Tue, Mar 21, 2023 at 11:28:51AM +0100, Benjamin Gaignard wrote:
> After changing bufs arrays to a dynamic allocated array
> VB2_MAX_FRAME doesn't mean anything for videobuf2 core.
> Remove it from the core definitions but keep it for drivers internal
> needs.
This made me think that some drivers could behave really badly if they
get more than VB2_MAX_FRAME (or VIDEO_MAX_FRAME) buffers. I certainly
see some having fixed-size arrays of exactly that size. Should we have a
queue flag that enables more buffers, so it is only available for
drivers which can handle them?
Best regards,
Tomasz
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 2 ++
> drivers/media/platform/amphion/vdec.c | 1 +
> .../media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c | 2 ++
> drivers/media/platform/qcom/venus/hfi.h | 2 ++
> drivers/media/platform/verisilicon/hantro_hw.h | 2 ++
> drivers/staging/media/ipu3/ipu3-v4l2.c | 2 ++
> include/media/videobuf2-core.h | 1 -
> include/media/videobuf2-v4l2.h | 4 ----
> 8 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index f4da917ccf3f..3c6ced360770 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -31,6 +31,8 @@
>
> #include <trace/events/vb2.h>
>
> +#define VB2_MAX_FRAME 32
> +
> static int debug;
> module_param(debug, int, 0644);
>
> diff --git a/drivers/media/platform/amphion/vdec.c b/drivers/media/platform/amphion/vdec.c
> index 87f9f8e90ab1..bef0c1b869be 100644
> --- a/drivers/media/platform/amphion/vdec.c
> +++ b/drivers/media/platform/amphion/vdec.c
> @@ -28,6 +28,7 @@
>
> #define VDEC_MIN_BUFFER_CAP 8
> #define VDEC_MIN_BUFFER_OUT 8
> +#define VB2_MAX_FRAME 32
>
> struct vdec_fs_info {
> char name[8];
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> index f5958b6d834a..ba208caf3043 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> @@ -16,6 +16,8 @@
> #include "../vdec_drv_if.h"
> #include "../vdec_vpu_if.h"
>
> +#define VB2_MAX_FRAME 32
> +
> /* reset_frame_context defined in VP9 spec */
> #define VP9_RESET_FRAME_CONTEXT_NONE0 0
> #define VP9_RESET_FRAME_CONTEXT_NONE1 1
> diff --git a/drivers/media/platform/qcom/venus/hfi.h b/drivers/media/platform/qcom/venus/hfi.h
> index f25d412d6553..bd5ca5a8b945 100644
> --- a/drivers/media/platform/qcom/venus/hfi.h
> +++ b/drivers/media/platform/qcom/venus/hfi.h
> @@ -10,6 +10,8 @@
>
> #include "hfi_helper.h"
>
> +#define VB2_MAX_FRAME 32
> +
> #define VIDC_SESSION_TYPE_VPE 0
> #define VIDC_SESSION_TYPE_ENC 1
> #define VIDC_SESSION_TYPE_DEC 2
> diff --git a/drivers/media/platform/verisilicon/hantro_hw.h b/drivers/media/platform/verisilicon/hantro_hw.h
> index e83f0c523a30..9e8faf7ba6fb 100644
> --- a/drivers/media/platform/verisilicon/hantro_hw.h
> +++ b/drivers/media/platform/verisilicon/hantro_hw.h
> @@ -15,6 +15,8 @@
> #include <media/v4l2-vp9.h>
> #include <media/videobuf2-core.h>
>
> +#define VB2_MAX_FRAME 32
> +
> #define DEC_8190_ALIGN_MASK 0x07U
>
> #define MB_DIM 16
> diff --git a/drivers/staging/media/ipu3/ipu3-v4l2.c b/drivers/staging/media/ipu3/ipu3-v4l2.c
> index e530767e80a5..6627b5c2d4d6 100644
> --- a/drivers/staging/media/ipu3/ipu3-v4l2.c
> +++ b/drivers/staging/media/ipu3/ipu3-v4l2.c
> @@ -10,6 +10,8 @@
> #include "ipu3.h"
> #include "ipu3-dmamap.h"
>
> +#define VB2_MAX_FRAME 32
> +
> /******************** v4l2_subdev_ops ********************/
>
> #define IPU3_RUNNING_MODE_VIDEO 0
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index b8b34a993e04..4aa43c5c7c58 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -21,7 +21,6 @@
> #include <media/media-request.h>
> #include <media/frame_vector.h>
>
> -#define VB2_MAX_FRAME (32)
> #define VB2_MAX_PLANES (8)
>
> /**
> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> index 5a845887850b..88a7a565170e 100644
> --- a/include/media/videobuf2-v4l2.h
> +++ b/include/media/videobuf2-v4l2.h
> @@ -15,10 +15,6 @@
> #include <linux/videodev2.h>
> #include <media/videobuf2-core.h>
>
> -#if VB2_MAX_FRAME != VIDEO_MAX_FRAME
> -#error VB2_MAX_FRAME != VIDEO_MAX_FRAME
> -#endif
> -
> #if VB2_MAX_PLANES != VIDEO_MAX_PLANES
> #error VB2_MAX_PLANES != VIDEO_MAX_PLANES
> #endif
> --
> 2.34.1
>
On Tue, Mar 21, 2023 at 11:28:52AM +0100, Benjamin Gaignard wrote:
> VIDIOC_DELETE_BUF ioctl allows to delete a buffer from a queue.
>
> Signed-off-by: Benjamin Gaignard <[email protected]>
> ---
> .../userspace-api/media/v4l/user-func.rst | 1 +
> .../media/v4l/vidioc-delete-buf.rst | 51 ++++++++++++++++
> .../media/common/videobuf2/videobuf2-core.c | 59 ++++++++++++++++++-
> .../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 | 1 +
> 10 files changed, 152 insertions(+), 1 deletion(-)
> 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 228c1521f190..93e0a6a117fc 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`.
Would struct v4l2_create_buffers make more sense here? With it, we could
actually make this ioctl VIDIOC_DELETE_BUF*S*, consistently with
VIDIOC_CREATE_BUF*S* and allow the user space to remove a block of buffers
the same way it created a block of buffers in the first place.
> +
> +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 3c6ced360770..ec241d726fe6 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -401,6 +401,24 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb)
> vb->skip_cache_sync_on_finish = 1;
> }
>
> +/*
> + * __vb2_queue_get_free_index() - find a free index in the queue for vb2 buffer.
> + *
> + * Returns an index for vb2 buffer.
> + */
> +static int __vb2_queue_get_free_index(struct vb2_queue *q)
> +{
> + int i;
> +
> + spin_lock(&q->bufs_lock);
> + for (i = 0; i < q->max_num_bufs; i++)
> + if (!q->bufs[i])
> + break;
> + spin_unlock(&q->bufs_lock);
> +
> + return i;
> +}
Another reason to go with XArray, so that we don't have to open code
index reclaim.
> +
> /*
> * __vb2_queue_alloc() - allocate vb2 buffer structures and (for MMAP type)
> * video buffer memory for all buffers/planes on the queue and initializes the
> @@ -427,7 +445,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
> vb->state = VB2_BUF_STATE_DEQUEUED;
> vb->vb2_queue = q;
> vb->num_planes = num_planes;
> - vb->index = q->num_buffers + buffer;
> + vb->index = __vb2_queue_get_free_index(q);
> vb->type = q->type;
> vb->memory = memory;
> init_buffer_cache_hints(q, vb);
> @@ -1570,6 +1588,45 @@ 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_QUEUED) {
How about other states? I'd probably only allow this when the state is
DEQUEUED for simplicity. Is there a need to allow deleting buffers in
other states?
Also, do we need to synchronize this with other contexts which could change
the buffer state?
> + dprintk(q, 1, "can't delete queued buffer index %d\n", index);
> + return -EINVAL;
> + }
> +
Don't we also need to hold q->mmap_lock to prevent racing with an attempt
to mmap the buffer?
> + if (vb && vb->planes[0].mem_priv)
nit: At this point it's not possible for vb to be NULL, since we already
ruled that out a few lines above.
> + 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);
> +
> + q->num_buffers--;
> + if (!q->num_buffers) {
> + q->memory = VB2_MEMORY_UNKNOWN;
> + INIT_LIST_HEAD(&q->queued_list);
> + }
Would it make sense to refactor __vb2_queue_free() instead to take a
range of buffer indexes rather than duplicating the code here?
Best regards,
Tomasz