From b70886ff5833cf499e77af77d2324ce8f68b60ce Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Thu, 5 Jun 2025 08:57:39 +0200 Subject: [PATCH] media: vb2: drop wait_prepare/finish callbacks 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 Signed-off-by: Mauro Carvalho Chehab --- .../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 0a8e236d07ab9..445a29c01d1bd 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 2df566f409b65..2d1f253b4929a 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 9b02aeba41089..4424d481d7f74 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.3