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 <[email protected]> --- .../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
