From 3c147c29310b838fb79a84f01448e34fbcbf0334 Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:29:16 +0100 Subject: media: videobuf2: Use vb2_buffer instead of index Directly use vb2_buffer pointer instead of index inside queue array. Signed-off-by: Benjamin Gaignard Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 51 +++++++++++++++---------- 1 file changed, 30 insertions(+), 21 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index c7a54d82a55e..d19d82a75ac6 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -364,13 +364,12 @@ static void set_buffer_cache_hints(struct vb2_queue *q, } static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, - struct v4l2_buffer *b, bool is_prepare, - struct media_request **p_req) + struct vb2_buffer *vb, struct v4l2_buffer *b, + bool is_prepare, struct media_request **p_req) { const char *opname = is_prepare ? "prepare_buf" : "qbuf"; struct media_request *req; struct vb2_v4l2_buffer *vbuf; - struct vb2_buffer *vb; int ret; if (b->type != q->type) { @@ -378,23 +377,11 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md return -EINVAL; } - if (b->index >= q->num_buffers) { - dprintk(q, 1, "%s: buffer index out of range\n", opname); - return -EINVAL; - } - - if (q->bufs[b->index] == NULL) { - /* Should never happen */ - dprintk(q, 1, "%s: buffer is NULL\n", opname); - return -EINVAL; - } - if (b->memory != q->memory) { dprintk(q, 1, "%s: invalid memory type\n", opname); return -EINVAL; } - vb = q->bufs[b->index]; vbuf = to_vb2_v4l2_buffer(vb); ret = __verify_planes_array(vb, b); if (ret) @@ -667,7 +654,7 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) vb = q->bufs[b->index]; ret = __verify_planes_array(vb, b); if (!ret) - vb2_core_querybuf(q, b->index, b); + vb2_core_querybuf(q, vb, b); return ret; } EXPORT_SYMBOL(vb2_querybuf); @@ -723,6 +710,7 @@ EXPORT_SYMBOL_GPL(vb2_reqbufs); int vb2_prepare_buf(struct vb2_queue *q, struct media_device *mdev, struct v4l2_buffer *b) { + struct vb2_buffer *vb; int ret; if (vb2_fileio_is_active(q)) { @@ -733,9 +721,15 @@ int vb2_prepare_buf(struct vb2_queue *q, struct media_device *mdev, if (b->flags & V4L2_BUF_FLAG_REQUEST_FD) return -EINVAL; - ret = vb2_queue_or_prepare_buf(q, mdev, b, true, NULL); + if (b->index >= q->num_buffers) { + dprintk(q, 1, "buffer index out of range\n"); + return -EINVAL; + } + vb = q->bufs[b->index]; - return ret ? ret : vb2_core_prepare_buf(q, b->index, b); + ret = vb2_queue_or_prepare_buf(q, mdev, vb, b, true, NULL); + + return ret ? ret : vb2_core_prepare_buf(q, vb, b); } EXPORT_SYMBOL_GPL(vb2_prepare_buf); @@ -803,6 +797,7 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev, struct v4l2_buffer *b) { struct media_request *req = NULL; + struct vb2_buffer *vb; int ret; if (vb2_fileio_is_active(q)) { @@ -810,10 +805,16 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev, return -EBUSY; } - ret = vb2_queue_or_prepare_buf(q, mdev, b, false, &req); + if (b->index >= q->num_buffers) { + dprintk(q, 1, "buffer index out of range\n"); + return -EINVAL; + } + vb = q->bufs[b->index]; + + ret = vb2_queue_or_prepare_buf(q, mdev, vb, b, false, &req); if (ret) return ret; - ret = vb2_core_qbuf(q, b->index, b, req); + ret = vb2_core_qbuf(q, vb, b, req); if (req) media_request_put(req); return ret; @@ -873,7 +874,15 @@ EXPORT_SYMBOL_GPL(vb2_streamoff); int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb) { - return vb2_core_expbuf(q, &eb->fd, eb->type, eb->index, + struct vb2_buffer *vb; + + if (eb->index >= q->num_buffers) { + dprintk(q, 1, "buffer index out of range\n"); + return -EINVAL; + } + vb = q->bufs[eb->index]; + + return vb2_core_expbuf(q, &eb->fd, eb->type, vb, eb->plane, eb->flags); } EXPORT_SYMBOL_GPL(vb2_expbuf); -- cgit v1.2.3 From 6ea001f94ab571e5e7f8a187dad261710e58563b Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:29:17 +0100 Subject: media: videobuf2: Access vb2_queue bufs array through helper functions This patch adds 2 helpers functions to add and remove vb2 buffers from a queue. With these 2 and vb2_get_buffer(), bufs field of struct vb2_queue becomes like a private member of the structure. After each call to vb2_get_buffer() we need to be sure that we get a valid pointer in preparation for when buffers can be deleted. Signed-off-by: Benjamin Gaignard Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-core.c | 158 ++++++++++++++++++------ drivers/media/common/videobuf2/videobuf2-v4l2.c | 44 +++++-- 2 files changed, 149 insertions(+), 53 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index df515134bb0c..0024b82663d4 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -413,6 +413,31 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb) vb->skip_cache_sync_on_finish = 1; } +/** + * vb2_queue_add_buffer() - add a buffer to a queue + * @q: pointer to &struct vb2_queue with videobuf2 queue. + * @vb: pointer to &struct vb2_buffer to be added to the queue. + * @index: index where add vb2_buffer in the queue + */ +static void vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, unsigned int index) +{ + WARN_ON(index >= VB2_MAX_FRAME || q->bufs[index] || vb->vb2_queue); + + q->bufs[index] = vb; + vb->index = index; + vb->vb2_queue = q; +} + +/** + * vb2_queue_remove_buffer() - remove a buffer from a queue + * @vb: pointer to &struct vb2_buffer to be removed from the queue. + */ +static void vb2_queue_remove_buffer(struct vb2_buffer *vb) +{ + vb->vb2_queue->bufs[vb->index] = NULL; + vb->vb2_queue = NULL; +} + /* * __vb2_queue_alloc() - allocate vb2 buffer structures and (for MMAP type) * video buffer memory for all buffers/planes on the queue and initializes the @@ -441,9 +466,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->type = q->type; vb->memory = memory; init_buffer_cache_hints(q, vb); @@ -451,9 +474,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, vb->planes[plane].length = plane_sizes[plane]; vb->planes[plane].min_length = plane_sizes[plane]; } - call_void_bufop(q, init_buffer, vb); - q->bufs[vb->index] = vb; + vb2_queue_add_buffer(q, vb, q->num_buffers + buffer); + call_void_bufop(q, init_buffer, vb); /* Allocate video buffer memory for the MMAP type */ if (memory == VB2_MEMORY_MMAP) { @@ -461,7 +484,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, if (ret) { dprintk(q, 1, "failed allocating memory for buffer %d\n", buffer); - q->bufs[vb->index] = NULL; + vb2_queue_remove_buffer(vb); kfree(vb); break; } @@ -476,7 +499,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, dprintk(q, 1, "buffer %d %p initialization failed\n", buffer, vb); __vb2_buf_mem_free(vb); - q->bufs[vb->index] = NULL; + vb2_queue_remove_buffer(vb); kfree(vb); break; } @@ -499,7 +522,7 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - vb = q->bufs[buffer]; + vb = vb2_get_buffer(q, buffer); if (!vb) continue; @@ -527,7 +550,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) /* Call driver-provided cleanup function for each buffer, if provided */ for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - struct vb2_buffer *vb = q->bufs[buffer]; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); if (vb && vb->planes[0].mem_priv) call_void_vb_qop(vb, buf_cleanup, vb); @@ -568,15 +591,20 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) q->cnt_unprepare_streaming = 0; } for (buffer = 0; buffer < q->num_buffers; ++buffer) { - struct vb2_buffer *vb = q->bufs[buffer]; - bool unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || - vb->cnt_mem_prepare != vb->cnt_mem_finish || - vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || - vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || - vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || - vb->cnt_buf_queue != vb->cnt_buf_done || - vb->cnt_buf_prepare != vb->cnt_buf_finish || - vb->cnt_buf_init != vb->cnt_buf_cleanup; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + bool unbalanced; + + if (!vb) + continue; + + unbalanced = vb->cnt_mem_alloc != vb->cnt_mem_put || + vb->cnt_mem_prepare != vb->cnt_mem_finish || + vb->cnt_mem_get_userptr != vb->cnt_mem_put_userptr || + vb->cnt_mem_attach_dmabuf != vb->cnt_mem_detach_dmabuf || + vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf || + vb->cnt_buf_queue != vb->cnt_buf_done || + vb->cnt_buf_prepare != vb->cnt_buf_finish || + vb->cnt_buf_init != vb->cnt_buf_cleanup; if (unbalanced) { pr_info("unbalanced counters for queue %p, buffer %d:\n", @@ -616,8 +644,13 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) /* Free vb2 buffers */ for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; ++buffer) { - kfree(q->bufs[buffer]); - q->bufs[buffer] = NULL; + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + + if (!vb) + continue; + + vb2_queue_remove_buffer(vb); + kfree(vb); } q->num_buffers -= buffers; @@ -653,7 +686,12 @@ static bool __buffers_in_use(struct vb2_queue *q) { unsigned int buffer; for (buffer = 0; buffer < q->num_buffers; ++buffer) { - if (vb2_buffer_in_use(q, q->bufs[buffer])) + struct vb2_buffer *vb = vb2_get_buffer(q, buffer); + + if (!vb) + continue; + + if (vb2_buffer_in_use(q, vb)) return true; } return false; @@ -1638,7 +1676,11 @@ static int vb2_start_streaming(struct vb2_queue *q) * correctly return them to vb2. */ for (i = 0; i < q->num_buffers; ++i) { - vb = q->bufs[i]; + vb = vb2_get_buffer(q, i); + + if (!vb) + continue; + if (vb->state == VB2_BUF_STATE_ACTIVE) vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED); } @@ -2039,12 +2081,18 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * to vb2 in stop_streaming(). */ if (WARN_ON(atomic_read(&q->owned_by_drv_count))) { - for (i = 0; i < q->num_buffers; ++i) - if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) { - pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n", - q->bufs[i]); - vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR); + for (i = 0; i < q->num_buffers; ++i) { + struct vb2_buffer *vb = vb2_get_buffer(q, i); + + if (!vb) + continue; + + if (vb->state == VB2_BUF_STATE_ACTIVE) { + pr_warn("driver bug: stop_streaming operation is leaving buffer %u in active state\n", + vb->index); + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR); } + } /* Must be zero now */ WARN_ON(atomic_read(&q->owned_by_drv_count)); } @@ -2078,9 +2126,14 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * be changed, so we can't move the buf_finish() to __vb2_dqbuf(). */ for (i = 0; i < q->num_buffers; ++i) { - struct vb2_buffer *vb = q->bufs[i]; - struct media_request *req = vb->req_obj.req; + struct vb2_buffer *vb; + struct media_request *req; + vb = vb2_get_buffer(q, i); + if (!vb) + continue; + + req = vb->req_obj.req; /* * If a request is associated with this buffer, then * call buf_request_cancel() to give the driver to complete() @@ -2229,10 +2282,12 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long offset, buffer = (offset >> PLANE_INDEX_SHIFT) & BUFFER_INDEX_MASK; *plane = (offset >> PAGE_SHIFT) & PLANE_INDEX_MASK; - if (buffer >= q->num_buffers || *plane >= q->bufs[buffer]->num_planes) + *vb = vb2_get_buffer(q, buffer); + if (!*vb) + return -EINVAL; + if (*plane >= (*vb)->num_planes) return -EINVAL; - *vb = q->bufs[buffer]; return 0; } @@ -2620,6 +2675,7 @@ struct vb2_fileio_data { static int __vb2_init_fileio(struct vb2_queue *q, int read) { struct vb2_fileio_data *fileio; + struct vb2_buffer *vb; int i, ret; unsigned int count = 0; @@ -2670,11 +2726,18 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) if (ret) goto err_kfree; + /* + * Userspace can never add or delete buffers later, so there + * will never be holes. It is safe to assume that vb2_get_buffer(q, 0) + * will always return a valid vb pointer + */ + vb = vb2_get_buffer(q, 0); + /* * Check if plane_count is correct * (multiplane buffers are not supported). */ - if (q->bufs[0]->num_planes != 1) { + if (vb->num_planes != 1) { ret = -EBUSY; goto err_reqbufs; } @@ -2683,12 +2746,15 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Get kernel address of each buffer. */ for (i = 0; i < q->num_buffers; i++) { - fileio->bufs[i].vaddr = vb2_plane_vaddr(q->bufs[i], 0); + /* vb can never be NULL when using fileio. */ + vb = vb2_get_buffer(q, i); + + fileio->bufs[i].vaddr = vb2_plane_vaddr(vb, 0); if (fileio->bufs[i].vaddr == NULL) { ret = -EINVAL; goto err_reqbufs; } - fileio->bufs[i].size = vb2_plane_size(q->bufs[i], 0); + fileio->bufs[i].size = vb2_plane_size(vb, 0); } /* @@ -2699,7 +2765,12 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Queue all buffers. */ for (i = 0; i < q->num_buffers; i++) { - ret = vb2_core_qbuf(q, q->bufs[i], NULL, NULL); + struct vb2_buffer *vb2 = vb2_get_buffer(q, i); + + if (!vb2) + continue; + + ret = vb2_core_qbuf(q, vb2, NULL, NULL); if (ret) goto err_reqbufs; fileio->bufs[i].queued = 1; @@ -2816,15 +2887,17 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ fileio->cur_index = index; buf = &fileio->bufs[index]; - b = q->bufs[index]; + + /* b can never be NULL when using fileio. */ + b = vb2_get_buffer(q, index); /* * Get number of bytes filled by the driver */ buf->pos = 0; buf->queued = 0; - buf->size = read ? vb2_get_plane_payload(q->bufs[index], 0) - : vb2_plane_size(q->bufs[index], 0); + buf->size = read ? vb2_get_plane_payload(b, 0) + : vb2_plane_size(b, 0); /* Compensate for data_offset on read in the multiplanar case. */ if (is_multiplanar && read && b->planes[0].data_offset < buf->size) { @@ -2867,7 +2940,8 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ * Queue next buffer if required. */ if (buf->pos == buf->size || (!read && fileio->write_immediately)) { - struct vb2_buffer *b = q->bufs[index]; + /* b can never be NULL when using fileio. */ + struct vb2_buffer *b = vb2_get_buffer(q, index); /* * Check if this is the last buffer to read. @@ -2894,7 +2968,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ */ buf->pos = 0; buf->queued = 1; - buf->size = vb2_plane_size(q->bufs[index], 0); + buf->size = vb2_plane_size(b, 0); fileio->q_count += 1; /* * If we are queuing up buffers for the first time, then @@ -2965,7 +3039,9 @@ static int vb2_thread(void *data) * Call vb2_dqbuf to get buffer back. */ if (prequeue) { - vb = q->bufs[index++]; + vb = vb2_get_buffer(q, index++); + if (!vb) + continue; prequeue--; } else { call_void_qop(q, wait_finish, q); @@ -2974,7 +3050,7 @@ static int vb2_thread(void *data) call_void_qop(q, wait_prepare, q); dprintk(q, 5, "file io: vb2_dqbuf result: %d\n", ret); if (!ret) - vb = q->bufs[index]; + vb = vb2_get_buffer(q, index); } if (ret || threadio->stop) break; diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index d19d82a75ac6..37f814a89ace 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -615,11 +615,22 @@ static const struct vb2_buf_ops v4l2_buf_ops = { struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp) { unsigned int i; + struct vb2_buffer *vb2; - for (i = 0; i < q->num_buffers; i++) - if (q->bufs[i]->copied_timestamp && - q->bufs[i]->timestamp == timestamp) - return vb2_get_buffer(q, i); + /* + * This loop doesn't scale if there is a really large number of buffers. + * Maybe something more efficient will be needed in this case. + */ + for (i = 0; i < q->num_buffers; i++) { + vb2 = vb2_get_buffer(q, i); + + if (!vb2) + continue; + + if (vb2->copied_timestamp && + vb2->timestamp == timestamp) + return vb2; + } return NULL; } EXPORT_SYMBOL_GPL(vb2_find_buffer); @@ -647,11 +658,12 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) return -EINVAL; } - if (b->index >= q->num_buffers) { - dprintk(q, 1, "buffer index out of range\n"); + vb = vb2_get_buffer(q, b->index); + if (!vb) { + dprintk(q, 1, "can't find the requested buffer %u\n", b->index); return -EINVAL; } - vb = q->bufs[b->index]; + ret = __verify_planes_array(vb, b); if (!ret) vb2_core_querybuf(q, vb, b); @@ -721,11 +733,11 @@ int vb2_prepare_buf(struct vb2_queue *q, struct media_device *mdev, if (b->flags & V4L2_BUF_FLAG_REQUEST_FD) return -EINVAL; - if (b->index >= q->num_buffers) { - dprintk(q, 1, "buffer index out of range\n"); + vb = vb2_get_buffer(q, b->index); + if (!vb) { + dprintk(q, 1, "can't find the requested buffer %u\n", b->index); return -EINVAL; } - vb = q->bufs[b->index]; ret = vb2_queue_or_prepare_buf(q, mdev, vb, b, true, NULL); @@ -809,7 +821,11 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev, dprintk(q, 1, "buffer index out of range\n"); return -EINVAL; } - vb = q->bufs[b->index]; + vb = vb2_get_buffer(q, b->index); + if (!vb) { + dprintk(q, 1, "can't find the requested buffer %u\n", b->index); + return -EINVAL; + } ret = vb2_queue_or_prepare_buf(q, mdev, vb, b, false, &req); if (ret) @@ -880,7 +896,11 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb) dprintk(q, 1, "buffer index out of range\n"); return -EINVAL; } - vb = q->bufs[eb->index]; + vb = vb2_get_buffer(q, eb->index); + if (!vb) { + dprintk(q, 1, "can't find the requested buffer %u\n", eb->index); + return -EINVAL; + } return vb2_core_expbuf(q, &eb->fd, eb->type, vb, eb->plane, eb->flags); -- cgit v1.2.3 From 5287f48952e0e03be919e04d25cb832207d9661a Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:29:18 +0100 Subject: media: videobuf2: Remove duplicated index vs q->num_buffers check vb2_get_buffer() already checks if the requested index is valid. Stop duplicating this kind of check everywhere. Signed-off-by: Benjamin Gaignard Reviewed-by: Andrzej Pietrasiewicz Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 8 -------- 1 file changed, 8 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 37f814a89ace..6ba13a95323c 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -817,10 +817,6 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev, return -EBUSY; } - if (b->index >= q->num_buffers) { - dprintk(q, 1, "buffer index out of range\n"); - return -EINVAL; - } vb = vb2_get_buffer(q, b->index); if (!vb) { dprintk(q, 1, "can't find the requested buffer %u\n", b->index); @@ -892,10 +888,6 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb) { struct vb2_buffer *vb; - if (eb->index >= q->num_buffers) { - dprintk(q, 1, "buffer index out of range\n"); - return -EINVAL; - } vb = vb2_get_buffer(q, eb->index); if (!vb) { dprintk(q, 1, "can't find the requested buffer %u\n", eb->index); -- cgit v1.2.3 From da8fc26b0b7d6c0cf46695ef717458fa248a71fb Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:29:20 +0100 Subject: media: videobuf2: Use vb2_get_num_buffers() helper Stop using queue num_buffers field directly, instead use vb2_get_num_buffers(). This prepares for the future 'delete buffers' feature where there are holes in the buffer indices. Signed-off-by: Benjamin Gaignard Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-core.c | 94 +++++++++++++------------ drivers/media/common/videobuf2/videobuf2-v4l2.c | 4 +- 2 files changed, 52 insertions(+), 46 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 0024b82663d4..ce5b84b9bcd4 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -449,13 +449,14 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, unsigned int num_buffers, unsigned int num_planes, const unsigned plane_sizes[VB2_MAX_PLANES]) { + unsigned int q_num_buffers = vb2_get_num_buffers(q); unsigned int buffer, plane; struct vb2_buffer *vb; int ret; /* Ensure that q->num_buffers+num_buffers is below VB2_MAX_FRAME */ num_buffers = min_t(unsigned int, num_buffers, - VB2_MAX_FRAME - q->num_buffers); + VB2_MAX_FRAME - q_num_buffers); for (buffer = 0; buffer < num_buffers; ++buffer) { /* Allocate vb2 buffer structures */ @@ -475,7 +476,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, vb->planes[plane].min_length = plane_sizes[plane]; } - vb2_queue_add_buffer(q, vb, q->num_buffers + buffer); + vb2_queue_add_buffer(q, vb, q_num_buffers + buffer); call_void_bufop(q, init_buffer, vb); /* Allocate video buffer memory for the MMAP type */ @@ -519,8 +520,9 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) { unsigned int buffer; struct vb2_buffer *vb; + unsigned int q_num_buffers = vb2_get_num_buffers(q); - for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; + for (buffer = q_num_buffers - buffers; buffer < q_num_buffers; ++buffer) { vb = vb2_get_buffer(q, buffer); if (!vb) @@ -544,11 +546,12 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) { unsigned int buffer; + unsigned int q_num_buffers = vb2_get_num_buffers(q); lockdep_assert_held(&q->mmap_lock); /* Call driver-provided cleanup function for each buffer, if provided */ - for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; + for (buffer = q_num_buffers - buffers; buffer < q_num_buffers; ++buffer) { struct vb2_buffer *vb = vb2_get_buffer(q, buffer); @@ -564,7 +567,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) * Check that all the calls were balanced during the life-time of this * queue. If not then dump the counters to the kernel log. */ - if (q->num_buffers) { + if (q_num_buffers) { 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; @@ -590,7 +593,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) q->cnt_stop_streaming = 0; q->cnt_unprepare_streaming = 0; } - for (buffer = 0; buffer < q->num_buffers; ++buffer) { + for (buffer = 0; buffer < vb2_get_num_buffers(q); buffer++) { struct vb2_buffer *vb = vb2_get_buffer(q, buffer); bool unbalanced; @@ -642,7 +645,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) #endif /* Free vb2 buffers */ - for (buffer = q->num_buffers - buffers; buffer < q->num_buffers; + for (buffer = q_num_buffers - buffers; buffer < q_num_buffers; ++buffer) { struct vb2_buffer *vb = vb2_get_buffer(q, buffer); @@ -654,7 +657,7 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) } q->num_buffers -= buffers; - if (!q->num_buffers) { + if (!vb2_get_num_buffers(q)) { q->memory = VB2_MEMORY_UNKNOWN; INIT_LIST_HEAD(&q->queued_list); } @@ -685,7 +688,7 @@ EXPORT_SYMBOL(vb2_buffer_in_use); static bool __buffers_in_use(struct vb2_queue *q) { unsigned int buffer; - for (buffer = 0; buffer < q->num_buffers; ++buffer) { + for (buffer = 0; buffer < vb2_get_num_buffers(q); ++buffer) { struct vb2_buffer *vb = vb2_get_buffer(q, buffer); if (!vb) @@ -811,6 +814,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, unsigned int flags, unsigned int *count) { unsigned int num_buffers, allocated_buffers, num_planes = 0; + unsigned int q_num_bufs = vb2_get_num_buffers(q); unsigned plane_sizes[VB2_MAX_PLANES] = { }; bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; unsigned int i; @@ -826,7 +830,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, return -EBUSY; } - if (*count == 0 || q->num_buffers != 0 || + if (*count == 0 || q_num_bufs != 0 || (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory) || !verify_coherency_flags(q, non_coherent_mem)) { /* @@ -844,7 +848,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, * queued without ever calling STREAMON. */ __vb2_queue_cancel(q); - __vb2_queue_free(q, q->num_buffers); + __vb2_queue_free(q, q_num_bufs); mutex_unlock(&q->mmap_lock); /* @@ -939,7 +943,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, if (ret < 0) { /* * Note: __vb2_queue_free() will subtract 'allocated_buffers' - * from q->num_buffers and it will reset q->memory to + * from already queued buffers and it will reset q->memory to * VB2_MEMORY_UNKNOWN. */ __vb2_queue_free(q, allocated_buffers); @@ -973,10 +977,11 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, unsigned int num_planes = 0, num_buffers, allocated_buffers; unsigned plane_sizes[VB2_MAX_PLANES] = { }; bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; - bool no_previous_buffers = !q->num_buffers; - int ret; + unsigned int q_num_bufs = vb2_get_num_buffers(q); + bool no_previous_buffers = !q_num_bufs; + int ret = 0; - if (q->num_buffers == VB2_MAX_FRAME) { + if (q_num_bufs == VB2_MAX_FRAME) { dprintk(q, 1, "maximum number of buffers already allocated\n"); return -ENOBUFS; } @@ -1005,7 +1010,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, return -EINVAL; } - num_buffers = min(*count, VB2_MAX_FRAME - q->num_buffers); + num_buffers = min(*count, VB2_MAX_FRAME - q_num_bufs); if (requested_planes && requested_sizes) { num_planes = requested_planes; @@ -1037,7 +1042,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, num_buffers = allocated_buffers; /* - * q->num_buffers contains the total number of buffers, that the + * num_buffers contains the total number of buffers, that the * queue driver has set up */ ret = call_qop(q, queue_setup, q, &num_buffers, @@ -1058,7 +1063,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, if (ret < 0) { /* * Note: __vb2_queue_free() will subtract 'allocated_buffers' - * from q->num_buffers and it will reset q->memory to + * from already queued buffers and it will reset q->memory to * VB2_MEMORY_UNKNOWN. */ __vb2_queue_free(q, allocated_buffers); @@ -1675,7 +1680,7 @@ static int vb2_start_streaming(struct vb2_queue *q) * Forcefully reclaim buffers if the driver did not * correctly return them to vb2. */ - for (i = 0; i < q->num_buffers; ++i) { + for (i = 0; i < vb2_get_num_buffers(q); ++i) { vb = vb2_get_buffer(q, i); if (!vb) @@ -2081,7 +2086,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * to vb2 in stop_streaming(). */ if (WARN_ON(atomic_read(&q->owned_by_drv_count))) { - for (i = 0; i < q->num_buffers; ++i) { + for (i = 0; i < vb2_get_num_buffers(q); i++) { struct vb2_buffer *vb = vb2_get_buffer(q, i); if (!vb) @@ -2125,7 +2130,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * call to __fill_user_buffer() after buf_finish(). That order can't * be changed, so we can't move the buf_finish() to __vb2_dqbuf(). */ - for (i = 0; i < q->num_buffers; ++i) { + for (i = 0; i < vb2_get_num_buffers(q); i++) { struct vb2_buffer *vb; struct media_request *req; @@ -2173,6 +2178,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) int vb2_core_streamon(struct vb2_queue *q, unsigned int type) { + unsigned int q_num_bufs = vb2_get_num_buffers(q); int ret; if (type != q->type) { @@ -2185,12 +2191,12 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type) return 0; } - if (!q->num_buffers) { + if (!q_num_bufs) { dprintk(q, 1, "no buffers have been allocated\n"); return -EINVAL; } - if (q->num_buffers < q->min_buffers_needed) { + if (q_num_bufs < q->min_buffers_needed) { dprintk(q, 1, "need at least %u allocated buffers\n", q->min_buffers_needed); return -EINVAL; @@ -2518,7 +2524,7 @@ void vb2_core_queue_release(struct vb2_queue *q) __vb2_cleanup_fileio(q); __vb2_queue_cancel(q); mutex_lock(&q->mmap_lock); - __vb2_queue_free(q, q->num_buffers); + __vb2_queue_free(q, vb2_get_num_buffers(q)); mutex_unlock(&q->mmap_lock); } EXPORT_SYMBOL_GPL(vb2_core_queue_release); @@ -2547,7 +2553,7 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file, /* * Start file I/O emulator only if streaming API has not been used yet. */ - if (q->num_buffers == 0 && !vb2_fileio_is_active(q)) { + if (vb2_get_num_buffers(q) == 0 && !vb2_fileio_is_active(q)) { if (!q->is_output && (q->io_modes & VB2_READ) && (req_events & (EPOLLIN | EPOLLRDNORM))) { if (__vb2_init_fileio(q, 1)) @@ -2585,7 +2591,7 @@ __poll_t vb2_core_poll(struct vb2_queue *q, struct file *file, * For output streams you can call write() as long as there are fewer * buffers queued than there are buffers available. */ - if (q->is_output && q->fileio && q->queued_count < q->num_buffers) + if (q->is_output && q->fileio && q->queued_count < vb2_get_num_buffers(q)) return EPOLLOUT | EPOLLWRNORM; if (list_empty(&q->done_list)) { @@ -2634,8 +2640,8 @@ struct vb2_fileio_buf { * struct vb2_fileio_data - queue context used by file io emulator * * @cur_index: the index of the buffer currently being read from or - * written to. If equal to q->num_buffers then a new buffer - * must be dequeued. + * written to. If equal to number of buffers in the vb2_queue + * then a new buffer must be dequeued. * @initial_index: in the read() case all buffers are queued up immediately * in __vb2_init_fileio() and __vb2_perform_fileio() just cycles * buffers. However, in the write() case no buffers are initially @@ -2645,9 +2651,9 @@ struct vb2_fileio_buf { * buffers. This means that initially __vb2_perform_fileio() * needs to know what buffer index to use when it is queuing up * the buffers for the first time. That initial index is stored - * in this field. Once it is equal to q->num_buffers all - * available buffers have been queued and __vb2_perform_fileio() - * should start the normal dequeue/queue cycle. + * in this field. Once it is equal to number of buffers in the + * vb2_queue all available buffers have been queued and + * __vb2_perform_fileio() should start the normal dequeue/queue cycle. * * vb2 provides a compatibility layer and emulator of file io (read and * write) calls on top of streaming API. For proper operation it required @@ -2695,7 +2701,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) /* * Check if streaming api has not been already activated. */ - if (q->streaming || q->num_buffers > 0) + if (q->streaming || vb2_get_num_buffers(q) > 0) return -EBUSY; /* @@ -2745,7 +2751,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) /* * Get kernel address of each buffer. */ - for (i = 0; i < q->num_buffers; i++) { + for (i = 0; i < vb2_get_num_buffers(q); i++) { /* vb can never be NULL when using fileio. */ vb = vb2_get_buffer(q, i); @@ -2764,7 +2770,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) /* * Queue all buffers. */ - for (i = 0; i < q->num_buffers; i++) { + for (i = 0; i < vb2_get_num_buffers(q); i++) { struct vb2_buffer *vb2 = vb2_get_buffer(q, i); if (!vb2) @@ -2777,10 +2783,10 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) } /* * All buffers have been queued, so mark that by setting - * initial_index to q->num_buffers + * initial_index to the number of buffers in the vb2_queue */ - fileio->initial_index = q->num_buffers; - fileio->cur_index = q->num_buffers; + fileio->initial_index = vb2_get_num_buffers(q); + fileio->cur_index = fileio->initial_index; } /* @@ -2873,7 +2879,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ * Check if we need to dequeue the buffer. */ index = fileio->cur_index; - if (index >= q->num_buffers) { + if (index >= vb2_get_num_buffers(q)) { struct vb2_buffer *b; /* @@ -2974,14 +2980,14 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ * If we are queuing up buffers for the first time, then * increase initial_index by one. */ - if (fileio->initial_index < q->num_buffers) + if (fileio->initial_index < vb2_get_num_buffers(q)) fileio->initial_index++; /* * The next buffer to use is either a buffer that's going to be - * queued for the first time (initial_index < q->num_buffers) - * or it is equal to q->num_buffers, meaning that the next - * time we need to dequeue a buffer since we've now queued up - * all the 'first time' buffers. + * queued for the first time (initial_index < number of buffers in the vb2_queue) + * or it is equal to the number of buffers in the vb2_queue, + * meaning that the next time we need to dequeue a buffer since + * we've now queued up all the 'first time' buffers. */ fileio->cur_index = fileio->initial_index; } @@ -3026,7 +3032,7 @@ static int vb2_thread(void *data) int ret = 0; if (q->is_output) { - prequeue = q->num_buffers; + prequeue = vb2_get_num_buffers(q); copy_timestamp = q->copy_timestamp; } diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 6ba13a95323c..9d3b828cc804 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -621,7 +621,7 @@ struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp) * This loop doesn't scale if there is a really large number of buffers. * Maybe something more efficient will be needed in this case. */ - for (i = 0; i < q->num_buffers; i++) { + for (i = 0; i < vb2_get_num_buffers(q); i++) { vb2 = vb2_get_buffer(q, i); if (!vb2) @@ -755,7 +755,7 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) fill_buf_caps(q, &create->capabilities); validate_memory_flags(q, create->memory, &create->flags); - create->index = q->num_buffers; + create->index = vb2_get_num_buffers(q); if (create->count == 0) return ret != -EBUSY ? ret : 0; -- cgit v1.2.3 From c838530d230bc638d79b78737fc4488ffc28c1ee Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:34:59 +0100 Subject: media: media videobuf2: Be more flexible on the number of queue stored buffers Add 'max_num_buffers' field in vb2_queue struct to let drivers decide how many buffers could be stored in a queue. This require 'bufs' array to be allocated at queue init time and freed when releasing the queue. By default VB2_MAX_FRAME remains the limit. Signed-off-by: Benjamin Gaignard Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-core.c | 46 ++++++++++++++++++++----- drivers/media/common/videobuf2/videobuf2-v4l2.c | 6 ++-- include/media/videobuf2-core.h | 12 ++++++- 3 files changed, 52 insertions(+), 12 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index ce5b84b9bcd4..8c1df829745b 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -421,7 +421,7 @@ static void init_buffer_cache_hints(struct vb2_queue *q, struct vb2_buffer *vb) */ static void vb2_queue_add_buffer(struct vb2_queue *q, struct vb2_buffer *vb, unsigned int index) { - WARN_ON(index >= VB2_MAX_FRAME || q->bufs[index] || vb->vb2_queue); + WARN_ON(index >= q->max_num_buffers || q->bufs[index] || vb->vb2_queue); q->bufs[index] = vb; vb->index = index; @@ -454,9 +454,12 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory, struct vb2_buffer *vb; int ret; - /* Ensure that q->num_buffers+num_buffers is below VB2_MAX_FRAME */ + /* + * Ensure that the number of already queue + the number of buffers already + * in the queue is below q->max_num_buffers + */ num_buffers = min_t(unsigned int, num_buffers, - VB2_MAX_FRAME - q_num_buffers); + q->max_num_buffers - q_num_buffers); for (buffer = 0; buffer < num_buffers; ++buffer) { /* Allocate vb2 buffer structures */ @@ -818,7 +821,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, unsigned plane_sizes[VB2_MAX_PLANES] = { }; bool non_coherent_mem = flags & V4L2_MEMORY_FLAG_NON_COHERENT; unsigned int i; - int ret; + int ret = 0; if (q->streaming) { dprintk(q, 1, "streaming active\n"); @@ -862,17 +865,22 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, /* * Make sure the requested values and current defaults are sane. */ - WARN_ON(q->min_buffers_needed > VB2_MAX_FRAME); num_buffers = max_t(unsigned int, *count, q->min_buffers_needed); - num_buffers = min_t(unsigned int, num_buffers, VB2_MAX_FRAME); + num_buffers = min_t(unsigned int, num_buffers, q->max_num_buffers); memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); /* * Set this now to ensure that drivers see the correct q->memory value * in the queue_setup op. */ mutex_lock(&q->mmap_lock); + if (!q->bufs) + q->bufs = kcalloc(q->max_num_buffers, sizeof(*q->bufs), GFP_KERNEL); + if (!q->bufs) + ret = -ENOMEM; q->memory = memory; mutex_unlock(&q->mmap_lock); + if (ret) + return ret; set_queue_coherency(q, non_coherent_mem); /* @@ -981,7 +989,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, bool no_previous_buffers = !q_num_bufs; int ret = 0; - if (q_num_bufs == VB2_MAX_FRAME) { + if (q->num_buffers == q->max_num_buffers) { dprintk(q, 1, "maximum number of buffers already allocated\n"); return -ENOBUFS; } @@ -998,7 +1006,13 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, */ mutex_lock(&q->mmap_lock); q->memory = memory; + if (!q->bufs) + q->bufs = kcalloc(q->max_num_buffers, sizeof(*q->bufs), GFP_KERNEL); + if (!q->bufs) + ret = -ENOMEM; mutex_unlock(&q->mmap_lock); + if (ret) + return ret; q->waiting_for_buffers = !q->is_output; set_queue_coherency(q, non_coherent_mem); } else { @@ -1010,7 +1024,7 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum vb2_memory memory, return -EINVAL; } - num_buffers = min(*count, VB2_MAX_FRAME - q_num_bufs); + num_buffers = min(*count, q->max_num_buffers - q_num_bufs); if (requested_planes && requested_sizes) { num_planes = requested_planes; @@ -2470,6 +2484,16 @@ int vb2_core_queue_init(struct vb2_queue *q) /* * Sanity check */ + /* + * For drivers who don't support max_num_buffers ensure + * a backward compatibility. + */ + if (!q->max_num_buffers) + q->max_num_buffers = VB2_MAX_FRAME; + + /* The maximum is limited by offset cookie encoding pattern */ + q->max_num_buffers = min_t(unsigned int, q->max_num_buffers, MAX_BUFFER_INDEX); + if (WARN_ON(!q) || WARN_ON(!q->ops) || WARN_ON(!q->mem_ops) || @@ -2479,6 +2503,10 @@ int vb2_core_queue_init(struct vb2_queue *q) WARN_ON(!q->ops->buf_queue)) return -EINVAL; + if (WARN_ON(q->max_num_buffers > MAX_BUFFER_INDEX) || + WARN_ON(q->min_buffers_needed > q->max_num_buffers)) + return -EINVAL; + if (WARN_ON(q->requires_requests && !q->supports_requests)) return -EINVAL; @@ -2525,6 +2553,8 @@ void vb2_core_queue_release(struct vb2_queue *q) __vb2_queue_cancel(q); mutex_lock(&q->mmap_lock); __vb2_queue_free(q, vb2_get_num_buffers(q)); + kfree(q->bufs); + q->bufs = NULL; mutex_unlock(&q->mmap_lock); } EXPORT_SYMBOL_GPL(vb2_core_queue_release); diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 9d3b828cc804..3d71c205406d 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -621,7 +621,7 @@ struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp) * This loop doesn't scale if there is a really large number of buffers. * Maybe something more efficient will be needed in this case. */ - for (i = 0; i < vb2_get_num_buffers(q); i++) { + for (i = 0; i < q->max_num_buffers; i++) { vb2 = vb2_get_buffer(q, i); if (!vb2) @@ -1136,7 +1136,7 @@ int _vb2_fop_release(struct file *file, struct mutex *lock) if (lock) mutex_lock(lock); - if (file->private_data == vdev->queue->owner) { + if (!vdev->queue->owner || file->private_data == vdev->queue->owner) { vb2_queue_release(vdev->queue); vdev->queue->owner = NULL; } @@ -1264,7 +1264,7 @@ void vb2_video_unregister_device(struct video_device *vdev) */ get_device(&vdev->dev); video_unregister_device(vdev); - if (vdev->queue && vdev->queue->owner) { + if (vdev->queue) { struct mutex *lock = vdev->queue->lock ? vdev->queue->lock : vdev->lock; diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 8f9d9e4af5b1..5557d78b6f20 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -558,6 +558,9 @@ struct vb2_buf_ops { * @dma_dir: DMA mapping direction. * @bufs: videobuf2 buffer structures * @num_buffers: number of allocated/used buffers + * @max_num_buffers: upper limit of number of allocated/used buffers. + * If set to 0 v4l2 core will change it VB2_MAX_FRAME + * for backward compatibility. * @queued_list: list of buffers currently queued from userspace * @queued_count: number of buffers queued and ready for streaming. * @owned_by_drv_count: number of buffers owned by the driver @@ -619,8 +622,9 @@ struct vb2_queue { struct mutex mmap_lock; unsigned int memory; enum dma_data_direction dma_dir; - struct vb2_buffer *bufs[VB2_MAX_FRAME]; + struct vb2_buffer **bufs; unsigned int num_buffers; + unsigned int max_num_buffers; struct list_head queued_list; unsigned int queued_count; @@ -1248,6 +1252,12 @@ static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q) static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q, unsigned int index) { + if (!q->bufs) + return NULL; + + if (index >= q->max_num_buffers) + return NULL; + if (index < q->num_buffers) return q->bufs[index]; return NULL; -- cgit v1.2.3 From d055a76c006540defd4eb80dcdea217cee0a141a Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Thu, 9 Nov 2023 17:35:00 +0100 Subject: media: core: Report the maximum possible number of buffers for the queue Use one of the struct v4l2_create_buffers reserved bytes to report the maximum possible number of buffers for the queue. V4l2 framework set V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS flags in queue capabilities so userland can know when the field is valid. Does the same change in v4l2_create_buffers32 structure. Signed-off-by: Benjamin Gaignard Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst | 8 ++++++-- Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst | 1 + drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 ++ drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 10 +++++++++- drivers/media/v4l2-core/v4l2-ioctl.c | 4 ++-- include/uapi/linux/videodev2.h | 7 ++++++- 6 files changed, 26 insertions(+), 6 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst index a048a9f6b7b6..49232c9006c2 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-create-bufs.rst @@ -116,9 +116,13 @@ than the number requested. - ``flags`` - Specifies additional buffer management attributes. See :ref:`memory-flags`. - * - __u32 - - ``reserved``\ [6] + - ``max_num_buffers`` + - If the V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS capability flag is set + this field indicates the maximum possible number of buffers + for this queue. + * - __u32 + - ``reserved``\ [5] - A place holder for future extensions. Drivers and applications must set the array to zero. diff --git a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst index 099fa6695167..0b3a41a45d05 100644 --- a/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst +++ b/Documentation/userspace-api/media/v4l/vidioc-reqbufs.rst @@ -120,6 +120,7 @@ aborting or finishing any DMA in progress, an implicit .. _V4L2-BUF-CAP-SUPPORTS-ORPHANED-BUFS: .. _V4L2-BUF-CAP-SUPPORTS-M2M-HOLD-CAPTURE-BUF: .. _V4L2-BUF-CAP-SUPPORTS-MMAP-CACHE-HINTS: +.. _V4L2-BUF-CAP-SUPPORTS-MAX-NUM-BUFFERS: .. raw:: latex diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 3d71c205406d..440c3b1c18ec 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -756,6 +756,8 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) fill_buf_caps(q, &create->capabilities); validate_memory_flags(q, create->memory, &create->flags); create->index = vb2_get_num_buffers(q); + create->max_num_buffers = q->max_num_buffers; + create->capabilities |= V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS; if (create->count == 0) return ret != -EBUSY ? ret : 0; diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index f3bed37859a2..8c07400bd280 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -116,6 +116,9 @@ struct v4l2_format32 { * @flags: additional buffer management attributes (ignored unless the * queue has V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS capability and * configured for MMAP streaming I/O). + * @max_num_buffers: if V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS capability flag is set + * this field indicate the maximum possible number of buffers + * for this queue. * @reserved: future extensions */ struct v4l2_create_buffers32 { @@ -125,7 +128,8 @@ struct v4l2_create_buffers32 { struct v4l2_format32 format; __u32 capabilities; __u32 flags; - __u32 reserved[6]; + __u32 max_num_buffers; + __u32 reserved[5]; }; static int get_v4l2_format32(struct v4l2_format *p64, @@ -175,6 +179,9 @@ static int get_v4l2_create32(struct v4l2_create_buffers *p64, return -EFAULT; if (copy_from_user(&p64->flags, &p32->flags, sizeof(p32->flags))) return -EFAULT; + if (copy_from_user(&p64->max_num_buffers, &p32->max_num_buffers, + sizeof(p32->max_num_buffers))) + return -EFAULT; return get_v4l2_format32(&p64->format, &p32->format); } @@ -221,6 +228,7 @@ static int put_v4l2_create32(struct v4l2_create_buffers *p64, offsetof(struct v4l2_create_buffers32, format)) || put_user(p64->capabilities, &p32->capabilities) || put_user(p64->flags, &p32->flags) || + put_user(p64->max_num_buffers, &p32->max_num_buffers) || copy_to_user(p32->reserved, p64->reserved, sizeof(p64->reserved))) return -EFAULT; return put_v4l2_format32(&p64->format, &p32->format); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 9b1de54ce379..4d90424cbfc4 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -483,9 +483,9 @@ static void v4l_print_create_buffers(const void *arg, bool write_only) { const struct v4l2_create_buffers *p = arg; - pr_cont("index=%d, count=%d, memory=%s, capabilities=0x%08x, ", + pr_cont("index=%d, count=%d, memory=%s, capabilities=0x%08x, max num buffers=%u", p->index, p->count, prt_names(p->memory, v4l2_memory_names), - p->capabilities); + p->capabilities, p->max_num_buffers); v4l_print_format(&p->format, write_only); } diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c3d4e490ce7c..13ddb5abf584 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1035,6 +1035,7 @@ struct v4l2_requestbuffers { #define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4) #define V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF (1 << 5) #define V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS (1 << 6) +#define V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS (1 << 7) /** * struct v4l2_plane - plane info for multi-planar buffers @@ -2605,6 +2606,9 @@ struct v4l2_dbg_chip_info { * @flags: additional buffer management attributes (ignored unless the * queue has V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS capability * and configured for MMAP streaming I/O). + * @max_num_buffers: if V4L2_BUF_CAP_SUPPORTS_MAX_NUM_BUFFERS capability flag is set + * this field indicate the maximum possible number of buffers + * for this queue. * @reserved: future extensions */ struct v4l2_create_buffers { @@ -2614,7 +2618,8 @@ struct v4l2_create_buffers { struct v4l2_format format; __u32 capabilities; __u32 flags; - __u32 reserved[6]; + __u32 max_num_buffers; + __u32 reserved[5]; }; /* -- cgit v1.2.3 From 26cb92f7f7c409c2674c019e5f4e44119209dd24 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Fri, 10 Nov 2023 09:53:00 +0100 Subject: media: drop CONFIG_MEDIA_CONTROLLER_REQUEST_API This config option was added during the development of the Request API to make it easy to disable it. The Request API is now stable so it is time to drop this option altogether. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 -- drivers/media/mc/Kconfig | 7 ------- drivers/media/mc/mc-device.c | 4 ---- drivers/media/platform/mediatek/vcodec/Kconfig | 1 - drivers/media/platform/nvidia/tegra-vde/Kconfig | 1 - drivers/media/platform/verisilicon/Kconfig | 1 - drivers/media/test-drivers/Kconfig | 1 - drivers/media/test-drivers/vicodec/Kconfig | 1 - drivers/media/test-drivers/visl/Kconfig | 1 - drivers/media/test-drivers/vivid/Kconfig | 1 - drivers/staging/media/rkvdec/Kconfig | 1 - drivers/staging/media/sunxi/cedrus/Kconfig | 1 - 12 files changed, 22 deletions(-) (limited to 'drivers/media/common/videobuf2/videobuf2-v4l2.c') diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 440c3b1c18ec..54d572c3b515 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -684,10 +684,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) *caps |= V4L2_BUF_CAP_SUPPORTS_M2M_HOLD_CAPTURE_BUF; if (q->allow_cache_hints && q->io_modes & VB2_MMAP) *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP_CACHE_HINTS; -#ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API if (q->supports_requests) *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS; -#endif } static void validate_memory_flags(struct vb2_queue *q, diff --git a/drivers/media/mc/Kconfig b/drivers/media/mc/Kconfig index 375b09612981..c82b07d2ef36 100644 --- a/drivers/media/mc/Kconfig +++ b/drivers/media/mc/Kconfig @@ -11,10 +11,3 @@ config MEDIA_CONTROLLER_DVB Enable the media controller API support for DVB. This is currently experimental. - -config MEDIA_CONTROLLER_REQUEST_API - bool - depends on MEDIA_CONTROLLER - help - This option enables the Request API for the Media controller and V4L2 - interfaces. It is currently needed by a few stateless codec drivers. diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c index 8cee956e38d4..c0dd4ae57227 100644 --- a/drivers/media/mc/mc-device.c +++ b/drivers/media/mc/mc-device.c @@ -372,16 +372,12 @@ static long media_device_get_topology(struct media_device *mdev, void *arg) static long media_device_request_alloc(struct media_device *mdev, void *arg) { -#ifdef CONFIG_MEDIA_CONTROLLER_REQUEST_API int *alloc_fd = arg; if (!mdev->ops || !mdev->ops->req_validate || !mdev->ops->req_queue) return -ENOTTY; return media_request_alloc(mdev, alloc_fd); -#else - return -ENOTTY; -#endif } static long copy_arg_from_user(void *karg, void __user *uarg, unsigned int cmd) diff --git a/drivers/media/platform/mediatek/vcodec/Kconfig b/drivers/media/platform/mediatek/vcodec/Kconfig index 74b00eb1bc97..bc8292232530 100644 --- a/drivers/media/platform/mediatek/vcodec/Kconfig +++ b/drivers/media/platform/mediatek/vcodec/Kconfig @@ -24,7 +24,6 @@ config VIDEO_MEDIATEK_VCODEC select V4L2_H264 select V4L2_VP9 select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API help Mediatek video codec driver provides HW capability to encode and decode in a range of video formats on MT8173 diff --git a/drivers/media/platform/nvidia/tegra-vde/Kconfig b/drivers/media/platform/nvidia/tegra-vde/Kconfig index f7454823bbbb..2fe13f39c95b 100644 --- a/drivers/media/platform/nvidia/tegra-vde/Kconfig +++ b/drivers/media/platform/nvidia/tegra-vde/Kconfig @@ -6,7 +6,6 @@ config VIDEO_TEGRA_VDE select DMA_SHARED_BUFFER select IOMMU_IOVA select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API select SRAM select VIDEOBUF2_DMA_CONTIG select VIDEOBUF2_DMA_SG diff --git a/drivers/media/platform/verisilicon/Kconfig b/drivers/media/platform/verisilicon/Kconfig index e65b836b9d78..24b927d8f182 100644 --- a/drivers/media/platform/verisilicon/Kconfig +++ b/drivers/media/platform/verisilicon/Kconfig @@ -8,7 +8,6 @@ config VIDEO_HANTRO depends on V4L_MEM2MEM_DRIVERS depends on VIDEO_DEV select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API select VIDEOBUF2_DMA_CONTIG select VIDEOBUF2_VMALLOC select V4L2_MEM2MEM_DEV diff --git a/drivers/media/test-drivers/Kconfig b/drivers/media/test-drivers/Kconfig index 459b433e9fae..5a5379524bde 100644 --- a/drivers/media/test-drivers/Kconfig +++ b/drivers/media/test-drivers/Kconfig @@ -12,7 +12,6 @@ config VIDEO_VIM2M select VIDEOBUF2_VMALLOC select V4L2_MEM2MEM_DEV select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API help This is a virtual test device for the memory-to-memory driver framework. diff --git a/drivers/media/test-drivers/vicodec/Kconfig b/drivers/media/test-drivers/vicodec/Kconfig index a7a828eec2a4..4ea0689c3abe 100644 --- a/drivers/media/test-drivers/vicodec/Kconfig +++ b/drivers/media/test-drivers/vicodec/Kconfig @@ -5,7 +5,6 @@ config VIDEO_VICODEC select VIDEOBUF2_VMALLOC select V4L2_MEM2MEM_DEV select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API help Driver for a Virtual Codec diff --git a/drivers/media/test-drivers/visl/Kconfig b/drivers/media/test-drivers/visl/Kconfig index 7508b904f196..37be9f267224 100644 --- a/drivers/media/test-drivers/visl/Kconfig +++ b/drivers/media/test-drivers/visl/Kconfig @@ -7,7 +7,6 @@ config VIDEO_VISL select VIDEOBUF2_VMALLOC select V4L2_MEM2MEM_DEV select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API select VIDEO_V4L2_TPG help diff --git a/drivers/media/test-drivers/vivid/Kconfig b/drivers/media/test-drivers/vivid/Kconfig index 5b08a5ad291e..ec2e71d76965 100644 --- a/drivers/media/test-drivers/vivid/Kconfig +++ b/drivers/media/test-drivers/vivid/Kconfig @@ -10,7 +10,6 @@ config VIDEO_VIVID select VIDEOBUF2_DMA_CONTIG select VIDEO_V4L2_TPG select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API help Enables a virtual video driver. This driver emulates a webcam, TV, S-Video and HDMI capture hardware, including VBI support for diff --git a/drivers/staging/media/rkvdec/Kconfig b/drivers/staging/media/rkvdec/Kconfig index e963d60cc6ad..5f3bdd848a2c 100644 --- a/drivers/staging/media/rkvdec/Kconfig +++ b/drivers/staging/media/rkvdec/Kconfig @@ -4,7 +4,6 @@ config VIDEO_ROCKCHIP_VDEC depends on ARCH_ROCKCHIP || COMPILE_TEST depends on VIDEO_DEV select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API select VIDEOBUF2_DMA_CONTIG select VIDEOBUF2_VMALLOC select V4L2_MEM2MEM_DEV diff --git a/drivers/staging/media/sunxi/cedrus/Kconfig b/drivers/staging/media/sunxi/cedrus/Kconfig index 621944f9907a..cb07a343c9c2 100644 --- a/drivers/staging/media/sunxi/cedrus/Kconfig +++ b/drivers/staging/media/sunxi/cedrus/Kconfig @@ -6,7 +6,6 @@ config VIDEO_SUNXI_CEDRUS depends on HAS_DMA depends on OF select MEDIA_CONTROLLER - select MEDIA_CONTROLLER_REQUEST_API select SUNXI_SRAM select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV -- cgit v1.2.3