| Message ID | 20240119094944.26763-2-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DELETE_BUF ioctl | expand |
The language needs tightening up a bit: On 19/01/2024 10:49, Benjamin Gaignard wrote: > Add 'min_reqbufs_allocation' field in vb2_queue structure so drivers in -> in the > can specificy the minimum number of buffers to allocate when calling specificy -> specify > VIDIOC_REQBUFS. > When initializing the queue, v4l2 core makes sure that the following > constraint are respected: constraint -> constraints > - minimum number of buffers to allocate should be at least 2 because minimum -> the minimum should -> must > one buffer is used by the hardware and other processed by userspace. and other -> while the other is being > - if a minimum number of buffers before start streaming is specify > the minimum number of buffers to allocate should be min_queued_buffers > + 1 to keep one buffer available for userspace. Replace this with: - if the driver needs 'min_queued_buffers' in the queue before calling start_streaming(), then the minimum requirement is 'min_queued_buffers + 1' to keep at least one buffer available for userspace. > > Simplify __vb2_init_fileio() by using 'min_reqbufs_allocation' directly > to avoid duplicating the minimum number of buffers to allocate computation. > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > --- > version 17: > - rewrite commit message. > - simplify __vb2_init_fileio() code. > - rewrite documentation > > .../media/common/videobuf2/videobuf2-core.c | 37 +++++++++++-------- > include/media/videobuf2-core.h | 15 +++++++- > 2 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index b6bf8f232f48..d74e93d00f58 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -865,7 +865,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > /* > * Make sure the requested values and current defaults are sane. > */ > - num_buffers = max_t(unsigned int, *count, q->min_queued_buffers); > + num_buffers = max_t(unsigned int, *count, q->min_reqbufs_allocation); > num_buffers = min_t(unsigned int, num_buffers, q->max_num_buffers); > memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); > /* > @@ -917,7 +917,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, > * There is no point in continuing if we can't allocate the minimum > * number of buffers needed by this vb2_queue. > */ > - if (allocated_buffers < q->min_queued_buffers) > + if (allocated_buffers < q->min_reqbufs_allocation) > ret = -ENOMEM; > > /* > @@ -2521,6 +2521,24 @@ int vb2_core_queue_init(struct vb2_queue *q) > if (WARN_ON(q->supports_requests && q->min_queued_buffers)) > return -EINVAL; > > + /* > + * The minimum requirement should really be 2: one buffer is used should really be -> is > + * by the hardware and one other processed by userspace. and one other -> while the other is being > + */ > + if (q->min_reqbufs_allocation < 2) > + q->min_reqbufs_allocation = 2; > + > + /* > + * If the driver needs 'min_queued_buffers' before start streaming before -> in the queue before calling start streaming -> calling start_streaming(), > + * then the minimum requirement is min_queued_buffers + 1 to keep Add quotes around 'min_queued_buffers + 1' > + * at least one buffer available for userspace > + */ > + if (q->min_reqbufs_allocation < q->min_queued_buffers + 1) > + q->min_reqbufs_allocation = q->min_queued_buffers + 1; > + > + if (WARN_ON(q->min_reqbufs_allocation > q->max_num_buffers)) > + return -EINVAL; > + > INIT_LIST_HEAD(&q->queued_list); > INIT_LIST_HEAD(&q->done_list); > spin_lock_init(&q->done_lock); > @@ -2713,7 +2731,6 @@ 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; > > /* > * Sanity check > @@ -2734,18 +2751,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) > if (q->streaming || vb2_get_num_buffers(q) > 0) > return -EBUSY; > > - /* > - * Start with q->min_queued_buffers + 1, driver can increase it in > - * queue_setup() > - * > - * 'min_queued_buffers' buffers need to be queued up before you > - * can start streaming, plus 1 for userspace (or in this case, > - * kernelspace) processing. > - */ > - count = max(2, q->min_queued_buffers + 1); > - > dprintk(q, 3, "setting up file io: mode %s, count %d, read_once %d, write_immediately %d\n", > - (read) ? "read" : "write", count, q->fileio_read_once, > + (read) ? "read" : "write", q->min_reqbufs_allocation, q->fileio_read_once, > q->fileio_write_immediately); > > fileio = kzalloc(sizeof(*fileio), GFP_KERNEL); > @@ -2759,7 +2766,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) > * Request buffers and use MMAP type to force driver > * to allocate buffers by itself. > */ > - fileio->count = count; > + fileio->count = q->min_reqbufs_allocation; > fileio->memory = VB2_MEMORY_MMAP; > fileio->type = q->type; > q->fileio = fileio; > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index 56719a26a46c..fe3423ff3807 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -550,9 +550,21 @@ struct vb2_buf_ops { > * @start_streaming can be called. Used when a DMA engine > * cannot be started unless at least this number of buffers > * have been queued into the driver. > - * VIDIOC_REQBUFS will ensure at least @min_queued_buffers > + * VIDIOC_REQBUFS will ensure at least @min_queued_buffers + 1 > * buffers will be allocated. Note that VIDIOC_CREATE_BUFS will not > * modify the requested buffer count. > + * @min_reqbufs_allocation: the minimum number of buffers to be allocated when > + * calling VIDIOC_REQBUFS. Note that VIDIOC_CREATE_BUFS will *not* > + * modify the requested buffer count and does not use this field. > + * Drivers can set this if there has to be a certain number of > + * buffers available for the hardware to work effectively. > + * This allows calling VIDIOC_REQBUFS with a buffer count of 1 and > + * it will be automatically adjusted to a workable buffer count. > + * If set, then @min_reqbufs_allocation must be larger than > + * @min_queued_buffers + 1. > + * If this field is > 3, then it is highly recommended that the > + * driver implements the V4L2_CID_MIN_BUFFERS_FOR_CAPTURE/OUTPUT > + * control. > */ > /* > * Private elements (won't appear at the uAPI book): > @@ -618,6 +630,7 @@ struct vb2_queue { > u32 timestamp_flags; > gfp_t gfp_flags; > u32 min_queued_buffers; > + u32 min_reqbufs_allocation; > > struct device *alloc_devs[VB2_MAX_PLANES]; >
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index b6bf8f232f48..d74e93d00f58 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -865,7 +865,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, /* * Make sure the requested values and current defaults are sane. */ - num_buffers = max_t(unsigned int, *count, q->min_queued_buffers); + num_buffers = max_t(unsigned int, *count, q->min_reqbufs_allocation); num_buffers = min_t(unsigned int, num_buffers, q->max_num_buffers); memset(q->alloc_devs, 0, sizeof(q->alloc_devs)); /* @@ -917,7 +917,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, * There is no point in continuing if we can't allocate the minimum * number of buffers needed by this vb2_queue. */ - if (allocated_buffers < q->min_queued_buffers) + if (allocated_buffers < q->min_reqbufs_allocation) ret = -ENOMEM; /* @@ -2521,6 +2521,24 @@ int vb2_core_queue_init(struct vb2_queue *q) if (WARN_ON(q->supports_requests && q->min_queued_buffers)) return -EINVAL; + /* + * The minimum requirement should really be 2: one buffer is used + * by the hardware and one other processed by userspace. + */ + if (q->min_reqbufs_allocation < 2) + q->min_reqbufs_allocation = 2; + + /* + * If the driver needs 'min_queued_buffers' before start streaming + * then the minimum requirement is min_queued_buffers + 1 to keep + * at least one buffer available for userspace + */ + if (q->min_reqbufs_allocation < q->min_queued_buffers + 1) + q->min_reqbufs_allocation = q->min_queued_buffers + 1; + + if (WARN_ON(q->min_reqbufs_allocation > q->max_num_buffers)) + return -EINVAL; + INIT_LIST_HEAD(&q->queued_list); INIT_LIST_HEAD(&q->done_list); spin_lock_init(&q->done_lock); @@ -2713,7 +2731,6 @@ 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; /* * Sanity check @@ -2734,18 +2751,8 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) if (q->streaming || vb2_get_num_buffers(q) > 0) return -EBUSY; - /* - * Start with q->min_queued_buffers + 1, driver can increase it in - * queue_setup() - * - * 'min_queued_buffers' buffers need to be queued up before you - * can start streaming, plus 1 for userspace (or in this case, - * kernelspace) processing. - */ - count = max(2, q->min_queued_buffers + 1); - dprintk(q, 3, "setting up file io: mode %s, count %d, read_once %d, write_immediately %d\n", - (read) ? "read" : "write", count, q->fileio_read_once, + (read) ? "read" : "write", q->min_reqbufs_allocation, q->fileio_read_once, q->fileio_write_immediately); fileio = kzalloc(sizeof(*fileio), GFP_KERNEL); @@ -2759,7 +2766,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read) * Request buffers and use MMAP type to force driver * to allocate buffers by itself. */ - fileio->count = count; + fileio->count = q->min_reqbufs_allocation; fileio->memory = VB2_MEMORY_MMAP; fileio->type = q->type; q->fileio = fileio; diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 56719a26a46c..fe3423ff3807 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -550,9 +550,21 @@ struct vb2_buf_ops { * @start_streaming can be called. Used when a DMA engine * cannot be started unless at least this number of buffers * have been queued into the driver. - * VIDIOC_REQBUFS will ensure at least @min_queued_buffers + * VIDIOC_REQBUFS will ensure at least @min_queued_buffers + 1 * buffers will be allocated. Note that VIDIOC_CREATE_BUFS will not * modify the requested buffer count. + * @min_reqbufs_allocation: the minimum number of buffers to be allocated when + * calling VIDIOC_REQBUFS. Note that VIDIOC_CREATE_BUFS will *not* + * modify the requested buffer count and does not use this field. + * Drivers can set this if there has to be a certain number of + * buffers available for the hardware to work effectively. + * This allows calling VIDIOC_REQBUFS with a buffer count of 1 and + * it will be automatically adjusted to a workable buffer count. + * If set, then @min_reqbufs_allocation must be larger than + * @min_queued_buffers + 1. + * If this field is > 3, then it is highly recommended that the + * driver implements the V4L2_CID_MIN_BUFFERS_FOR_CAPTURE/OUTPUT + * control. */ /* * Private elements (won't appear at the uAPI book): @@ -618,6 +630,7 @@ struct vb2_queue { u32 timestamp_flags; gfp_t gfp_flags; u32 min_queued_buffers; + u32 min_reqbufs_allocation; struct device *alloc_devs[VB2_MAX_PLANES];
Add 'min_reqbufs_allocation' field in vb2_queue structure so drivers can specificy the minimum number of buffers to allocate when calling VIDIOC_REQBUFS. When initializing the queue, v4l2 core makes sure that the following constraint are respected: - minimum number of buffers to allocate should be at least 2 because one buffer is used by the hardware and other processed by userspace. - if a minimum number of buffers before start streaming is specify the minimum number of buffers to allocate should be min_queued_buffers + 1 to keep one buffer available for userspace. Simplify __vb2_init_fileio() by using 'min_reqbufs_allocation' directly to avoid duplicating the minimum number of buffers to allocate computation. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- version 17: - rewrite commit message. - simplify __vb2_init_fileio() code. - rewrite documentation .../media/common/videobuf2/videobuf2-core.c | 37 +++++++++++-------- include/media/videobuf2-core.h | 15 +++++++- 2 files changed, 36 insertions(+), 16 deletions(-)