* [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish
@ 2025-06-05 6:57 Hans Verkuil
2025-06-05 6:57 ` [PATCH 1/5] media: dvb-core: dmxdevfilter must always flush bufs Hans Verkuil
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab
The last user of the vb2 wait_prepare/finish callbacks was dvb-core.
It turned out that the DVB streaming I/O code suffered somewhat from bit
rot, especially since the patches adding streaming I/O support to v4l-utils
were never applied, so there was no way to actually use it in
dvbv5-scan/zap.
I will post a separate series adding support for this, based on the
original patches from 2017 (!).
The first two patches for dvb-core fix two issues found while
regression testing. The last three patches in this series remove the
support for the wait_prepare/finish callbacks.
Regards,
Hans
Hans Verkuil (5):
media: dvb-core: dmxdevfilter must always flush bufs
media: dvb-core/dmxdev: drop locks around mmap()
media: dvb-core: dvb_vb2: drop wait_prepare/finish callbacks
media: vb2: remove vb2_ops_wait_prepare/finish helpers
media: vb2: drop wait_prepare/finish callbacks
Documentation/driver-api/media/v4l2-dev.rst | 8 +--
.../userspace-api/media/conf_nitpick.py | 2 -
.../media/common/videobuf2/videobuf2-core.c | 49 ++++---------------
.../media/common/videobuf2/videobuf2-v4l2.c | 14 ------
drivers/media/dvb-core/dmxdev.c | 46 ++++++++---------
drivers/media/dvb-core/dvb_vb2.c | 45 ++++++-----------
include/media/dvb_vb2.h | 17 ++++---
include/media/videobuf2-core.h | 23 ++-------
include/media/videobuf2-v4l2.h | 18 -------
9 files changed, 62 insertions(+), 160 deletions(-)
--
2.47.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] media: dvb-core: dmxdevfilter must always flush bufs
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
@ 2025-06-05 6:57 ` Hans Verkuil
2025-06-05 6:57 ` [PATCH 2/5] media: dvb-core/dmxdev: drop locks around mmap() Hans Verkuil
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil
Currently the buffers are being filled until full, which works fine
for the transport stream, but not when reading sections, those have
to be returned to userspace immediately, otherwise dvbv5-scan will
just wait forever.
Add a 'flush' argument to dvb_vb2_fill_buffer to indicate whether
the buffer must be flushed or wait until it is full.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/dvb-core/dmxdev.c | 8 ++++----
drivers/media/dvb-core/dvb_vb2.c | 5 +++--
include/media/dvb_vb2.h | 6 ++++--
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index 1e985f943944..e4cf5e6bb542 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -396,11 +396,11 @@ static int dvb_dmxdev_section_callback(const u8 *buffer1, size_t buffer1_len,
if (dvb_vb2_is_streaming(&dmxdevfilter->vb2_ctx)) {
ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
buffer1, buffer1_len,
- buffer_flags);
+ buffer_flags, true);
if (ret == buffer1_len)
ret = dvb_vb2_fill_buffer(&dmxdevfilter->vb2_ctx,
buffer2, buffer2_len,
- buffer_flags);
+ buffer_flags, true);
} else {
ret = dvb_dmxdev_buffer_write(&dmxdevfilter->buffer,
buffer1, buffer1_len);
@@ -451,10 +451,10 @@ static int dvb_dmxdev_ts_callback(const u8 *buffer1, size_t buffer1_len,
if (dvb_vb2_is_streaming(ctx)) {
ret = dvb_vb2_fill_buffer(ctx, buffer1, buffer1_len,
- buffer_flags);
+ buffer_flags, false);
if (ret == buffer1_len)
ret = dvb_vb2_fill_buffer(ctx, buffer2, buffer2_len,
- buffer_flags);
+ buffer_flags, false);
} else {
if (buffer->error) {
spin_unlock(&dmxdevfilter->dev->lock);
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 29edaaff7a5c..7444bbc2f24d 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -249,7 +249,8 @@ int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx)
int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
const unsigned char *src, int len,
- enum dmx_buffer_flags *buffer_flags)
+ enum dmx_buffer_flags *buffer_flags,
+ bool flush)
{
unsigned long flags = 0;
void *vbuf = NULL;
@@ -306,7 +307,7 @@ int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
}
}
- if (ctx->nonblocking && ctx->buf) {
+ if (flush && ctx->buf) {
vb2_set_plane_payload(&ctx->buf->vb, 0, ll);
vb2_buffer_done(&ctx->buf->vb, VB2_BUF_STATE_DONE);
list_del(&ctx->buf->list);
diff --git a/include/media/dvb_vb2.h b/include/media/dvb_vb2.h
index 8cb88452cd6c..0fbbfc65157e 100644
--- a/include/media/dvb_vb2.h
+++ b/include/media/dvb_vb2.h
@@ -124,7 +124,7 @@ static inline int dvb_vb2_release(struct dvb_vb2_ctx *ctx)
return 0;
};
#define dvb_vb2_is_streaming(ctx) (0)
-#define dvb_vb2_fill_buffer(ctx, file, wait, flags) (0)
+#define dvb_vb2_fill_buffer(ctx, file, wait, flags, flush) (0)
static inline __poll_t dvb_vb2_poll(struct dvb_vb2_ctx *ctx,
struct file *file,
@@ -166,10 +166,12 @@ int dvb_vb2_is_streaming(struct dvb_vb2_ctx *ctx);
* @buffer_flags:
* pointer to buffer flags as defined by &enum dmx_buffer_flags.
* can be NULL.
+ * @flush: flush the buffer, even if it isn't full.
*/
int dvb_vb2_fill_buffer(struct dvb_vb2_ctx *ctx,
const unsigned char *src, int len,
- enum dmx_buffer_flags *buffer_flags);
+ enum dmx_buffer_flags *buffer_flags,
+ bool flush);
/**
* dvb_vb2_poll - Wrapper to vb2_core_streamon() for Digital TV
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/5] media: dvb-core/dmxdev: drop locks around mmap()
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
2025-06-05 6:57 ` [PATCH 1/5] media: dvb-core: dmxdevfilter must always flush bufs Hans Verkuil
@ 2025-06-05 6:57 ` Hans Verkuil
2025-06-05 6:57 ` [PATCH 3/5] media: dvb-core: dvb_vb2: drop wait_prepare/finish callbacks Hans Verkuil
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil
vb2 no longer requires locking around mmap since commit
f035eb4e976e ("[media] videobuf2: fix lockdep warning").
Since the streaming I/O mode for DVB support is by default off, and
the dvb utilities were never updated with streaming support, and
we never had regression tests for this streaming mode, this was
never noticed before.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/dvb-core/dmxdev.c | 23 ++---------------------
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index e4cf5e6bb542..ffeedb0c0950 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -1216,24 +1216,11 @@ static int dvb_demux_mmap(struct file *file, struct vm_area_struct *vma)
{
struct dmxdev_filter *dmxdevfilter = file->private_data;
struct dmxdev *dmxdev = dmxdevfilter->dev;
- int ret;
if (!dmxdev->may_do_mmap)
return -ENOTTY;
- if (mutex_lock_interruptible(&dmxdev->mutex))
- return -ERESTARTSYS;
-
- if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
- mutex_unlock(&dmxdev->mutex);
- return -ERESTARTSYS;
- }
- ret = dvb_vb2_mmap(&dmxdevfilter->vb2_ctx, vma);
-
- mutex_unlock(&dmxdevfilter->mutex);
- mutex_unlock(&dmxdev->mutex);
-
- return ret;
+ return dvb_vb2_mmap(&dmxdevfilter->vb2_ctx, vma);
}
#endif
@@ -1366,7 +1353,6 @@ static int dvb_dvr_mmap(struct file *file, struct vm_area_struct *vma)
{
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
- int ret;
if (!dmxdev->may_do_mmap)
return -ENOTTY;
@@ -1374,12 +1360,7 @@ static int dvb_dvr_mmap(struct file *file, struct vm_area_struct *vma)
if (dmxdev->exit)
return -ENODEV;
- if (mutex_lock_interruptible(&dmxdev->mutex))
- return -ERESTARTSYS;
-
- ret = dvb_vb2_mmap(&dmxdev->dvr_vb2_ctx, vma);
- mutex_unlock(&dmxdev->mutex);
- return ret;
+ return dvb_vb2_mmap(&dmxdev->dvr_vb2_ctx, vma);
}
#endif
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/5] media: dvb-core: dvb_vb2: drop wait_prepare/finish callbacks
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
2025-06-05 6:57 ` [PATCH 1/5] media: dvb-core: dmxdevfilter must always flush bufs Hans Verkuil
2025-06-05 6:57 ` [PATCH 2/5] media: dvb-core/dmxdev: drop locks around mmap() Hans Verkuil
@ 2025-06-05 6:57 ` Hans Verkuil
2025-06-05 6:57 ` [PATCH 4/5] media: vb2: remove vb2_ops_wait_prepare/finish helpers Hans Verkuil
2025-06-05 6:57 ` [PATCH 5/5] media: vb2: drop wait_prepare/finish callbacks Hans Verkuil
4 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil
Since commit 88785982a19d ("media: vb2: use lock if wait_prepare/finish
are NULL") it is no longer needed to set the wait_prepare/finish
vb2_ops callbacks as long as the lock field in vb2_queue is set.
Set the queue lock to &ctx->mutex, which makes it possible to drop
the wait_prepare/finish callbacks.
This simplifies the code and this is a step towards the goal of deleting
these callbacks.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
drivers/media/dvb-core/dmxdev.c | 15 +++++++++++-
drivers/media/dvb-core/dvb_vb2.c | 40 +++++++++-----------------------
include/media/dvb_vb2.h | 11 ++++-----
3 files changed, 30 insertions(+), 36 deletions(-)
diff --git a/drivers/media/dvb-core/dmxdev.c b/drivers/media/dvb-core/dmxdev.c
index ffeedb0c0950..19d168bd6c54 100644
--- a/drivers/media/dvb-core/dmxdev.c
+++ b/drivers/media/dvb-core/dmxdev.c
@@ -171,6 +171,7 @@ static int dvb_dvr_open(struct inode *inode, struct file *file)
dvb_ringbuffer_init(&dmxdev->dvr_buffer, mem, DVR_BUFFER_SIZE);
if (dmxdev->may_do_mmap)
dvb_vb2_init(&dmxdev->dvr_vb2_ctx, "dvr",
+ &dmxdev->mutex,
file->f_flags & O_NONBLOCK);
dvbdev->readers--;
}
@@ -814,9 +815,21 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
dmxdev->may_do_mmap = 0;
#endif
+ /*
+ * The mutex passed to dvb_vb2_init is unlocked when a buffer
+ * is in a blocking wait. However, dmxdevfilter has two mutexes:
+ * dmxdevfilter->mutex and dmxdev->mutex. So this will not work.
+ * The solution would be to support unlocking two mutexes in vb2,
+ * but since this problem has been here since the beginning and
+ * nobody ever complained, we leave it as-is rather than adding
+ * that second mutex pointer to vb2.
+ *
+ * In the unlikely event that someone complains about this, then
+ * this comment will hopefully help.
+ */
dvb_ringbuffer_init(&dmxdevfilter->buffer, NULL, 8192);
dvb_vb2_init(&dmxdevfilter->vb2_ctx, "demux_filter",
- file->f_flags & O_NONBLOCK);
+ &dmxdevfilter->mutex, file->f_flags & O_NONBLOCK);
dmxdevfilter->type = DMXDEV_TYPE_NONE;
dvb_dmxdev_filter_state_set(dmxdevfilter, DMXDEV_STATE_ALLOCATED);
timer_setup(&dmxdevfilter->timer, dvb_dmxdev_filter_timeout, 0);
diff --git a/drivers/media/dvb-core/dvb_vb2.c b/drivers/media/dvb-core/dvb_vb2.c
index 7444bbc2f24d..672b0efdca21 100644
--- a/drivers/media/dvb-core/dvb_vb2.c
+++ b/drivers/media/dvb-core/dvb_vb2.c
@@ -103,31 +103,12 @@ static void _stop_streaming(struct vb2_queue *vq)
spin_unlock_irqrestore(&ctx->slock, flags);
}
-static void _dmxdev_lock(struct vb2_queue *vq)
-{
- struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
-
- mutex_lock(&ctx->mutex);
- dprintk(3, "[%s]\n", ctx->name);
-}
-
-static void _dmxdev_unlock(struct vb2_queue *vq)
-{
- struct dvb_vb2_ctx *ctx = vb2_get_drv_priv(vq);
-
- if (mutex_is_locked(&ctx->mutex))
- mutex_unlock(&ctx->mutex);
- dprintk(3, "[%s]\n", ctx->name);
-}
-
static const struct vb2_ops dvb_vb2_qops = {
.queue_setup = _queue_setup,
.buf_prepare = _buffer_prepare,
.buf_queue = _buffer_queue,
.start_streaming = _start_streaming,
.stop_streaming = _stop_streaming,
- .wait_prepare = _dmxdev_unlock,
- .wait_finish = _dmxdev_lock,
};
static void _fill_dmx_buffer(struct vb2_buffer *vb, void *pb)
@@ -158,9 +139,10 @@ static const struct vb2_buf_ops dvb_vb2_buf_ops = {
};
/*
- * Videobuf operations
+ * vb2 operations
*/
-int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
+int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name,
+ struct mutex *mutex, int nonblocking)
{
struct vb2_queue *q = &ctx->vb_q;
int ret;
@@ -175,14 +157,7 @@ int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
q->ops = &dvb_vb2_qops;
q->mem_ops = &vb2_vmalloc_memops;
q->buf_ops = &dvb_vb2_buf_ops;
- ret = vb2_core_queue_init(q);
- if (ret) {
- ctx->state = DVB_VB2_STATE_NONE;
- dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
- return ret;
- }
-
- mutex_init(&ctx->mutex);
+ q->lock = mutex;
spin_lock_init(&ctx->slock);
INIT_LIST_HEAD(&ctx->dvb_q);
@@ -190,6 +165,13 @@ int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int nonblocking)
ctx->nonblocking = nonblocking;
ctx->state = DVB_VB2_STATE_INIT;
+ ret = vb2_core_queue_init(q);
+ if (ret) {
+ ctx->state = DVB_VB2_STATE_NONE;
+ dprintk(1, "[%s] errno=%d\n", ctx->name, ret);
+ return ret;
+ }
+
dprintk(3, "[%s]\n", ctx->name);
return 0;
diff --git a/include/media/dvb_vb2.h b/include/media/dvb_vb2.h
index 0fbbfc65157e..8932396d2c99 100644
--- a/include/media/dvb_vb2.h
+++ b/include/media/dvb_vb2.h
@@ -72,8 +72,6 @@ struct dvb_buffer {
/**
* struct dvb_vb2_ctx - control struct for VB2 handler
* @vb_q: pointer to &struct vb2_queue with videobuf2 queue.
- * @mutex: mutex to serialize vb2 operations. Used by
- * vb2 core %wait_prepare and %wait_finish operations.
* @slock: spin lock used to protect buffer filling at dvb_vb2.c.
* @dvb_q: List of buffers that are not filled yet.
* @buf: Pointer to the buffer that are currently being filled.
@@ -96,7 +94,6 @@ struct dvb_buffer {
*/
struct dvb_vb2_ctx {
struct vb2_queue vb_q;
- struct mutex mutex;
spinlock_t slock;
struct list_head dvb_q;
struct dvb_buffer *buf;
@@ -114,8 +111,8 @@ struct dvb_vb2_ctx {
};
#ifndef CONFIG_DVB_MMAP
-static inline int dvb_vb2_init(struct dvb_vb2_ctx *ctx,
- const char *name, int non_blocking)
+static inline int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name,
+ struct mutex *mutex, int non_blocking)
{
return 0;
};
@@ -138,10 +135,12 @@ static inline __poll_t dvb_vb2_poll(struct dvb_vb2_ctx *ctx,
*
* @ctx: control struct for VB2 handler
* @name: name for the VB2 handler
+ * @mutex: pointer to the mutex that serializes vb2 ioctls
* @non_blocking:
* if not zero, it means that the device is at non-blocking mode
*/
-int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name, int non_blocking);
+int dvb_vb2_init(struct dvb_vb2_ctx *ctx, const char *name,
+ struct mutex *mutex, int non_blocking);
/**
* dvb_vb2_release - Releases the VB2 handler allocated resources and
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/5] media: vb2: remove vb2_ops_wait_prepare/finish helpers
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
` (2 preceding siblings ...)
2025-06-05 6:57 ` [PATCH 3/5] media: dvb-core: dvb_vb2: drop wait_prepare/finish callbacks Hans Verkuil
@ 2025-06-05 6:57 ` Hans Verkuil
2025-06-05 6:57 ` [PATCH 5/5] media: vb2: drop wait_prepare/finish callbacks Hans Verkuil
4 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil
Since vb2 now relies on the presence of the vb2_queue lock
field and there are no more drivers that use these helpers, it is safe
to drop them.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
Documentation/driver-api/media/v4l2-dev.rst | 8 ++++----
.../media/common/videobuf2/videobuf2-v4l2.c | 14 --------------
include/media/videobuf2-v4l2.h | 18 ------------------
3 files changed, 4 insertions(+), 36 deletions(-)
diff --git a/Documentation/driver-api/media/v4l2-dev.rst b/Documentation/driver-api/media/v4l2-dev.rst
index d5cb19b21a9f..dd239ad42051 100644
--- a/Documentation/driver-api/media/v4l2-dev.rst
+++ b/Documentation/driver-api/media/v4l2-dev.rst
@@ -157,10 +157,10 @@ changing the e.g. exposure of the webcam.
Of course, you can always do all the locking yourself by leaving both lock
pointers at ``NULL``.
-In the case of :ref:`videobuf2 <vb2_framework>` you will need to implement the
-``wait_prepare()`` and ``wait_finish()`` callbacks to unlock/lock if applicable.
-If you use the ``queue->lock`` pointer, then you can use the helper functions
-:c:func:`vb2_ops_wait_prepare` and :c:func:`vb2_ops_wait_finish`.
+In the case of :ref:`videobuf2 <vb2_framework>` you must set the ``queue->lock``
+pointer to the lock you use to serialize the queuing ioctls. This ensures that
+that lock is released while waiting in ``VIDIOC_DQBUF`` for a buffer to arrive,
+and it is retaken afterwards.
The implementation of a hotplug disconnect should also take the lock from
:c:type:`video_device` before calling v4l2_device_disconnect. If you are also
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 1cd26faee503..c16e920b752b 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -1301,20 +1301,6 @@ void vb2_video_unregister_device(struct video_device *vdev)
}
EXPORT_SYMBOL_GPL(vb2_video_unregister_device);
-/* vb2_ops helpers. Only use if vq->lock is non-NULL. */
-
-void vb2_ops_wait_prepare(struct vb2_queue *vq)
-{
- mutex_unlock(vq->lock);
-}
-EXPORT_SYMBOL_GPL(vb2_ops_wait_prepare);
-
-void vb2_ops_wait_finish(struct vb2_queue *vq)
-{
- mutex_lock(vq->lock);
-}
-EXPORT_SYMBOL_GPL(vb2_ops_wait_finish);
-
/*
* Note that this function is called during validation time and
* thus the req_queue_mutex is held to ensure no request objects
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 77ce8238ab30..71d2864fb235 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -367,24 +367,6 @@ unsigned long vb2_fop_get_unmapped_area(struct file *file, unsigned long addr,
*/
void vb2_video_unregister_device(struct video_device *vdev);
-/**
- * vb2_ops_wait_prepare - helper function to lock a struct &vb2_queue
- *
- * @vq: pointer to &struct vb2_queue
- *
- * ..note:: only use if vq->lock is non-NULL.
- */
-void vb2_ops_wait_prepare(struct vb2_queue *vq);
-
-/**
- * vb2_ops_wait_finish - helper function to unlock a struct &vb2_queue
- *
- * @vq: pointer to &struct vb2_queue
- *
- * ..note:: only use if vq->lock is non-NULL.
- */
-void vb2_ops_wait_finish(struct vb2_queue *vq);
-
struct media_request;
int vb2_request_validate(struct media_request *req);
void vb2_request_queue(struct media_request *req);
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] media: vb2: drop wait_prepare/finish callbacks
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
` (3 preceding siblings ...)
2025-06-05 6:57 ` [PATCH 4/5] media: vb2: remove vb2_ops_wait_prepare/finish helpers Hans Verkuil
@ 2025-06-05 6:57 ` Hans Verkuil
4 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2025-06-05 6:57 UTC (permalink / raw)
To: linux-media; +Cc: Mauro Carvalho Chehab, Hans Verkuil
Drop the wait_prepare/finish callbacks. Instead require that the vb2_queue
lock field is always set and use that lock when waiting for buffers to
arrive.
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>
---
.../userspace-api/media/conf_nitpick.py | 2 -
.../media/common/videobuf2/videobuf2-core.c | 49 ++++---------------
include/media/videobuf2-core.h | 23 ++-------
3 files changed, 15 insertions(+), 59 deletions(-)
diff --git a/Documentation/userspace-api/media/conf_nitpick.py b/Documentation/userspace-api/media/conf_nitpick.py
index 0a8e236d07ab..445a29c01d1b 100644
--- a/Documentation/userspace-api/media/conf_nitpick.py
+++ b/Documentation/userspace-api/media/conf_nitpick.py
@@ -42,8 +42,6 @@ nitpick_ignore = [
("c:func", "struct fd_set"),
("c:func", "struct pollfd"),
("c:func", "usb_make_path"),
- ("c:func", "wait_finish"),
- ("c:func", "wait_prepare"),
("c:func", "write"),
("c:type", "atomic_t"),
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 2df566f409b6..2d1f253b4929 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -605,8 +605,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned i
*/
if (vb2_get_num_buffers(q)) {
bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming ||
- q->cnt_prepare_streaming != q->cnt_unprepare_streaming ||
- q->cnt_wait_prepare != q->cnt_wait_finish;
+ q->cnt_prepare_streaming != q->cnt_unprepare_streaming;
if (unbalanced) {
pr_info("unbalanced counters for queue %p:\n", q);
@@ -617,13 +616,8 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int start, unsigned i
if (q->cnt_prepare_streaming != q->cnt_unprepare_streaming)
pr_info(" prepare_streaming: %u unprepare_streaming: %u\n",
q->cnt_prepare_streaming, q->cnt_unprepare_streaming);
- if (q->cnt_wait_prepare != q->cnt_wait_finish)
- pr_info(" wait_prepare: %u wait_finish: %u\n",
- q->cnt_wait_prepare, q->cnt_wait_finish);
}
q->cnt_queue_setup = 0;
- q->cnt_wait_prepare = 0;
- q->cnt_wait_finish = 0;
q->cnt_prepare_streaming = 0;
q->cnt_start_streaming = 0;
q->cnt_stop_streaming = 0;
@@ -2037,10 +2031,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
* become ready or for streamoff. Driver's lock is released to
* allow streamoff or qbuf to be called while waiting.
*/
- if (q->ops->wait_prepare)
- call_void_qop(q, wait_prepare, q);
- else if (q->lock)
- mutex_unlock(q->lock);
+ mutex_unlock(q->lock);
/*
* All locks have been released, it is safe to sleep now.
@@ -2050,10 +2041,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
!list_empty(&q->done_list) || !q->streaming ||
q->error);
- if (q->ops->wait_finish)
- call_void_qop(q, wait_finish, q);
- else if (q->lock)
- mutex_lock(q->lock);
+ mutex_lock(q->lock);
q->waiting_in_dqbuf = 0;
/*
@@ -2653,12 +2641,8 @@ int vb2_core_queue_init(struct vb2_queue *q)
if (WARN_ON(q->min_reqbufs_allocation > q->max_num_buffers))
return -EINVAL;
- /* Either both or none are set */
- if (WARN_ON(!q->ops->wait_prepare ^ !q->ops->wait_finish))
- return -EINVAL;
-
- /* Warn if q->lock is NULL and no custom wait_prepare is provided */
- if (WARN_ON(!q->lock && !q->ops->wait_prepare))
+ /* Warn if q->lock is NULL */
+ if (WARN_ON(!q->lock))
return -EINVAL;
INIT_LIST_HEAD(&q->queued_list);
@@ -3220,17 +3204,10 @@ static int vb2_thread(void *data)
continue;
prequeue--;
} else {
- if (!threadio->stop) {
- if (q->ops->wait_finish)
- call_void_qop(q, wait_finish, q);
- else if (q->lock)
- mutex_lock(q->lock);
+ mutex_lock(q->lock);
+ if (!threadio->stop)
ret = vb2_core_dqbuf(q, &index, NULL, 0);
- if (q->ops->wait_prepare)
- call_void_qop(q, wait_prepare, q);
- else if (q->lock)
- mutex_unlock(q->lock);
- }
+ mutex_unlock(q->lock);
dprintk(q, 5, "file io: vb2_dqbuf result: %d\n", ret);
if (!ret)
vb = vb2_get_buffer(q, index);
@@ -3245,15 +3222,9 @@ static int vb2_thread(void *data)
if (copy_timestamp)
vb->timestamp = ktime_get_ns();
if (!threadio->stop) {
- if (q->ops->wait_finish)
- call_void_qop(q, wait_finish, q);
- else if (q->lock)
- mutex_lock(q->lock);
+ mutex_lock(q->lock);
ret = vb2_core_qbuf(q, vb, NULL, NULL);
- if (q->ops->wait_prepare)
- call_void_qop(q, wait_prepare, q);
- else if (q->lock)
- mutex_unlock(q->lock);
+ mutex_unlock(q->lock);
}
if (ret || threadio->stop)
break;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 9b02aeba4108..4424d481d7f7 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -351,13 +351,6 @@ struct vb2_buffer {
* \*num_buffers are being allocated additionally to
* the buffers already allocated. If either \*num_planes
* or the requested sizes are invalid callback must return %-EINVAL.
- * @wait_prepare: release any locks taken while calling vb2 functions;
- * it is called before an ioctl needs to wait for a new
- * buffer to arrive; required to avoid a deadlock in
- * blocking access type.
- * @wait_finish: reacquire all locks released in the previous callback;
- * required to continue operation after sleeping while
- * waiting for a new buffer to arrive.
* @buf_out_validate: called when the output buffer is prepared or queued
* to a request; drivers can use this to validate
* userspace-provided information; this is required only
@@ -436,9 +429,6 @@ struct vb2_ops {
unsigned int *num_buffers, unsigned int *num_planes,
unsigned int sizes[], struct device *alloc_devs[]);
- void (*wait_prepare)(struct vb2_queue *q);
- void (*wait_finish)(struct vb2_queue *q);
-
int (*buf_out_validate)(struct vb2_buffer *vb);
int (*buf_init)(struct vb2_buffer *vb);
int (*buf_prepare)(struct vb2_buffer *vb);
@@ -521,10 +511,10 @@ struct vb2_buf_ops {
* @non_coherent_mem: when set queue will attempt to allocate buffers using
* non-coherent memory.
* @lock: pointer to a mutex that protects the &struct vb2_queue. The
- * driver can set this to a mutex to let the v4l2 core serialize
- * the queuing ioctls. If the driver wants to handle locking
- * itself, then this should be set to NULL. This lock is not used
- * by the videobuf2 core API.
+ * driver must set this to a mutex to let the v4l2 core serialize
+ * the queuing ioctls. This lock is used when waiting for a new
+ * buffer to arrive: the lock is released, we wait for the new
+ * buffer, and then retaken.
* @owner: The filehandle that 'owns' the buffers, i.e. the filehandle
* that called reqbufs, create_buffers or started fileio.
* This field is not used by the videobuf2 core API, but it allows
@@ -680,8 +670,6 @@ struct vb2_queue {
* called. Used to check for unbalanced ops.
*/
u32 cnt_queue_setup;
- u32 cnt_wait_prepare;
- u32 cnt_wait_finish;
u32 cnt_prepare_streaming;
u32 cnt_start_streaming;
u32 cnt_stop_streaming;
@@ -766,8 +754,7 @@ void vb2_discard_done(struct vb2_queue *q);
* @q: pointer to &struct vb2_queue with videobuf2 queue.
*
* This function will wait until all buffers that have been given to the driver
- * by &vb2_ops->buf_queue are given back to vb2 with vb2_buffer_done(). It
- * doesn't call &vb2_ops->wait_prepare/&vb2_ops->wait_finish pair.
+ * by &vb2_ops->buf_queue are given back to vb2 with vb2_buffer_done().
* It is intended to be called with all locks taken, for example from
* &vb2_ops->stop_streaming callback.
*/
--
2.47.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-05 7:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-05 6:57 [PATCH 0/5] media: dvb/vb2: fix DVB streaming, drop wait_prepare/finish Hans Verkuil
2025-06-05 6:57 ` [PATCH 1/5] media: dvb-core: dmxdevfilter must always flush bufs Hans Verkuil
2025-06-05 6:57 ` [PATCH 2/5] media: dvb-core/dmxdev: drop locks around mmap() Hans Verkuil
2025-06-05 6:57 ` [PATCH 3/5] media: dvb-core: dvb_vb2: drop wait_prepare/finish callbacks Hans Verkuil
2025-06-05 6:57 ` [PATCH 4/5] media: vb2: remove vb2_ops_wait_prepare/finish helpers Hans Verkuil
2025-06-05 6:57 ` [PATCH 5/5] media: vb2: drop wait_prepare/finish callbacks Hans Verkuil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).