If queue->lock is different from the video_device lock, then
you need to serialize queue_setup with VIDIOC_S_FMT, and this
should be done by the driver.
Signed-off-by: Hans Verkuil <hverkuil@xxxxxxxxx>
---
.../media/common/videobuf2/videobuf2-core.c | 51 +++++++++++++------
include/media/videobuf2-core.h | 19 +++++++
2 files changed, 55 insertions(+), 15 deletions(-)
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index f7e7e633bcd7..269485920beb 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -465,7 +465,8 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
*/
if (q->num_buffers) {
bool unbalanced = q->cnt_start_streaming != q->cnt_stop_streaming ||
- q->cnt_wait_prepare != q->cnt_wait_finish;
+ q->cnt_wait_prepare != q->cnt_wait_finish ||
+ q->cnt_queue_setup_lock != q->cnt_queue_setup_unlock;
if (unbalanced || debug) {
pr_info("counters for queue %p:%s\n", q,
@@ -473,10 +474,14 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
pr_info(" setup: %u start_streaming: %u stop_streaming: %u\n",
q->cnt_queue_setup, q->cnt_start_streaming,
q->cnt_stop_streaming);
+ pr_info(" queue_setup_lock: %u queue_setup_unlock: %u\n",
+ q->cnt_queue_setup_lock, q->cnt_queue_setup_unlock);
pr_info(" wait_prepare: %u wait_finish: %u\n",
q->cnt_wait_prepare, q->cnt_wait_finish);
}
q->cnt_queue_setup = 0;
+ q->cnt_queue_setup_lock = 0;
+ q->cnt_queue_setup_unlock = 0;
q->cnt_wait_prepare = 0;
q->cnt_wait_finish = 0;
q->cnt_start_streaming = 0;
@@ -717,6 +722,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME);
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
+ call_void_qop(q, queue_setup_lock, q);
/*
* Ask the driver how many buffers and planes per buffer it requires.
@@ -725,22 +731,27 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
plane_sizes, q->alloc_devs);
if (ret)
- return ret;
+ goto unlock;
/* Check that driver has set sane values */
- if (WARN_ON(!num_planes))
- return -EINVAL;
+ if (WARN_ON(!num_planes)) {
+ ret = -EINVAL;
+ goto unlock;
+ }
for (i = 0; i < num_planes; i++)
- if (WARN_ON(!plane_sizes[i]))
- return -EINVAL;
+ if (WARN_ON(!plane_sizes[i])) {
+ ret = -EINVAL;
+ goto unlock;
+ }
/* Finally, allocate buffers and video memory */
allocated_buffers =
__vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);
if (allocated_buffers == 0) {
dprintk(1, "memory allocation failed\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto unlock;
}
/*
@@ -775,19 +786,19 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
*/
}
- mutex_lock(&q->mmap_lock);
q->num_buffers = allocated_buffers;
+ call_void_qop(q, queue_setup_unlock, q);
if (ret < 0) {
/*
* Note: __vb2_queue_free() will subtract 'allocated_buffers'
* from q->num_buffers.
*/
+ mutex_lock(&q->mmap_lock);
__vb2_queue_free(q, allocated_buffers);
mutex_unlock(&q->mmap_lock);
return ret;
}
- mutex_unlock(&q->mmap_lock);
/*
* Return the number of successfully allocated buffers
@@ -795,8 +806,11 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
*/
*count = allocated_buffers;
q->waiting_for_buffers = !q->is_output;
-
return 0;
+
+unlock:
+ call_void_qop(q, queue_setup_unlock, q);
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_core_reqbufs);
@@ -813,10 +827,12 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
return -ENOBUFS;
}
+ call_void_qop(q, queue_setup_lock, q);
if (!q->num_buffers) {
if (q->waiting_in_dqbuf && *count) {
dprintk(1, "another dup()ped fd is waiting for a buffer\n");
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock;
}
memset(q->alloc_devs, 0, sizeof(q->alloc_devs));
q->memory = memory;
@@ -837,14 +853,15 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
ret = call_qop(q, queue_setup, q, &num_buffers,
&num_planes, plane_sizes, q->alloc_devs);
if (ret)
- return ret;
+ goto unlock;
/* Finally, allocate buffers and video memory */
allocated_buffers = __vb2_queue_alloc(q, memory, num_buffers,
num_planes, plane_sizes);
if (allocated_buffers == 0) {
dprintk(1, "memory allocation failed\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto unlock;
}
/*
@@ -869,19 +886,19 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
*/
}
- mutex_lock(&q->mmap_lock);
q->num_buffers += allocated_buffers;
+ call_void_qop(q, queue_setup_unlock, q);
if (ret < 0) {
/*
* Note: __vb2_queue_free() will subtract 'allocated_buffers'
* from q->num_buffers.
*/
+ mutex_lock(&q->mmap_lock);
__vb2_queue_free(q, allocated_buffers);
mutex_unlock(&q->mmap_lock);
return -ENOMEM;
}
- mutex_unlock(&q->mmap_lock);
/*
* Return the number of successfully allocated buffers
@@ -890,6 +907,10 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory,
*count = allocated_buffers;
return 0;
+
+unlock:
+ call_void_qop(q, queue_setup_unlock, q);
+ return ret;
}
EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 613f22910174..92861b6fe7f8 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -333,6 +333,20 @@ struct vb2_buffer {
* \*num_buffers are being allocated additionally to
* q->num_buffers. If either \*num_planes or the requested
* sizes are invalid callback must return %-EINVAL.
+ * @queue_setup_lock: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
+ * to serialize @queue_setup with ioctls like
+ * VIDIOC_S_FMT() that change the buffer size. Only
+ * required if queue->lock differs from the mutex that is
+ * used to serialize the ioctls that change the buffer
+ * size. This callback should lock the ioctl serialization
+ * mutex.
+ * @queue_setup_unlock: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
+ * to serialize @queue_setup with ioctls like
+ * VIDIOC_S_FMT() that change the buffer size. Only
+ * required if queue->lock differs from the mutex that is
+ * used to serialize the ioctls that change the buffer
+ * size. This callback should unlock the ioctl
+ * serialization mutex.
* @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
@@ -403,10 +417,13 @@ struct vb2_ops {
int (*queue_setup)(struct vb2_queue *q,
unsigned int *num_buffers, unsigned int *num_planes,
unsigned int sizes[], struct device *alloc_devs[]);
+ void (*queue_setup_lock)(struct vb2_queue *q);
+ void (*queue_setup_unlock)(struct vb2_queue *q);
void (*wait_prepare)(struct vb2_queue *q);
void (*wait_finish)(struct vb2_queue *q);
+
int (*buf_init)(struct vb2_buffer *vb);
int (*buf_prepare)(struct vb2_buffer *vb);
void (*buf_finish)(struct vb2_buffer *vb);
@@ -599,6 +616,8 @@ struct vb2_queue {
* called. Used to check for unbalanced ops.
*/
u32 cnt_queue_setup;
+ u32 cnt_queue_setup_lock;
+ u32 cnt_queue_setup_unlock;
u32 cnt_wait_prepare;
u32 cnt_wait_finish;
u32 cnt_start_streaming;
--
2.19.1
