[RFC,1/4] media: videobuf2: Use vb2_get_buffer() as helper everywhere

  The first step before changing how vb2 buffers are stored into queue
is to avoid direct call to bufs arrays.
This patch add 2 helpers functions to set and delete 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.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 69 ++++++++++---------
 .../media/common/videobuf2/videobuf2-v4l2.c   | 17 +++--
 drivers/media/platform/amphion/vpu_dbg.c      |  4 +-
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  2 +-
 .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  2 +-
 drivers/media/test-drivers/visl/visl-dec.c    | 16 +++--
 include/media/videobuf2-core.h                | 20 ++++++
 7 files changed, 81 insertions(+), 49 deletions(-)
  

Hi Benjamin,

W dniu 13.03.2023 o 14:59, Benjamin Gaignard pisze:
> The first step before changing how vb2 buffers are stored into queue
> is to avoid direct call to bufs arrays.
> This patch add 2 helpers functions to set and delete 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.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>   .../media/common/videobuf2/videobuf2-core.c   | 69 ++++++++++---------
>   .../media/common/videobuf2/videobuf2-v4l2.c   | 17 +++--
>   drivers/media/platform/amphion/vpu_dbg.c      |  4 +-
>   .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  2 +-
>   .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  2 +-
>   drivers/media/test-drivers/visl/visl-dec.c    | 16 +++--
>   include/media/videobuf2-core.h                | 20 ++++++
>   7 files changed, 81 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index cf6727d9c81f..b51152ace763 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -359,7 +359,7 @@ static void __setup_offsets(struct vb2_buffer *vb)
>   	unsigned long off = 0;
>   
>   	if (vb->index) {
> -		struct vb2_buffer *prev = q->bufs[vb->index - 1];
> +		struct vb2_buffer *prev = vb2_get_buffer(q, vb->index - 1);

internally vb2_get_buffer() verifies the index is within allowed range, but...

>   		struct vb2_plane *p = &prev->planes[prev->num_planes - 1];
>   
>   		off = PAGE_ALIGN(p->m.offset + p->length);
> @@ -437,7 +437,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
>   		}
>   		call_void_bufop(q, init_buffer, vb);
>   
> -		q->bufs[vb->index] = vb;
> +		vb2_set_buffer(q, vb);
>   
>   		/* Allocate video buffer memory for the MMAP type */
>   		if (memory == VB2_MEMORY_MMAP) {
> @@ -445,7 +445,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_del_buffer(q, vb);
>   				kfree(vb);
>   				break;
>   			}
> @@ -460,7 +460,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_del_buffer(q, vb);
>   				kfree(vb);
>   				break;
>   			}
> @@ -483,7 +483,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;
>   
> @@ -511,7 +511,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);
> @@ -591,8 +591,10 @@ 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 *vb2 = vb2_get_buffer(q, buffer);
> +
> +		vb2_del_buffer(q, vb2);
> +		kfree(vb2);
>   	}
>   
>   	q->num_buffers -= buffers;
> @@ -628,7 +630,7 @@ 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]))
> +		if (vb2_buffer_in_use(q, vb2_get_buffer(q, buffer)))
>   			return true;
>   	}
>   	return false;
> @@ -636,7 +638,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
>   
>   void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb)
>   {
> -	call_void_bufop(q, fill_user_buffer, q->bufs[index], pb);
> +	call_void_bufop(q, fill_user_buffer, vb2_get_buffer(q, index), pb);
>   }
>   EXPORT_SYMBOL_GPL(vb2_core_querybuf);
>   
> @@ -1547,7 +1549,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
>   	struct vb2_buffer *vb;
>   	int ret;
>   
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>   	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
>   		dprintk(q, 1, "invalid buffer state %s\n",
>   			vb2_state_name(vb->state));
> @@ -1618,7 +1620,7 @@ 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->state == VB2_BUF_STATE_ACTIVE)
>   				vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED);
>   		}
> @@ -1646,7 +1648,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>   		return -EIO;
>   	}
>   
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>   
>   	if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
>   	    q->requires_requests) {
> @@ -2022,12 +2024,15 @@ 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) {
> +		for (i = 0; i < q->num_buffers; ++i) {
> +			struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
> +
> +			if (vb2->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);
> +					vb2);
> +				vb2_buffer_done(vb2, VB2_BUF_STATE_ERROR);
>   			}
> +		}
>   		/* Must be zero now */
>   		WARN_ON(atomic_read(&q->owned_by_drv_count));
>   	}
> @@ -2061,7 +2066,7 @@ 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 vb2_buffer *vb = vb2_get_buffer(q, i);
>   		struct media_request *req = vb->req_obj.req;
>   
>   		/*
> @@ -2215,7 +2220,7 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
>   	 * return its buffer and plane numbers.
>   	 */
>   	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> -		vb = q->bufs[buffer];
> +		vb = vb2_get_buffer(q, buffer);
>   
>   		for (plane = 0; plane < vb->num_planes; ++plane) {
>   			if (vb->planes[plane].m.offset == off) {
> @@ -2262,7 +2267,7 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
>   		return -EINVAL;
>   	}
>   
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>   
>   	if (plane >= vb->num_planes) {
>   		dprintk(q, 1, "buffer plane out of range\n");
> @@ -2339,7 +2344,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>   	if (ret)
>   		goto unlock;
>   
> -	vb = q->bufs[buffer];
> +	vb = vb2_get_buffer(q, buffer);
>   
>   	/*
>   	 * MMAP requires page_aligned buffers.
> @@ -2679,7 +2684,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>   	 * Check if plane_count is correct
>   	 * (multiplane buffers are not supported).
>   	 */
> -	if (q->bufs[0]->num_planes != 1) {
> +	if (vb2_get_buffer(q, 0)->num_planes != 1) {
>   		ret = -EBUSY;
>   		goto err_reqbufs;
>   	}
> @@ -2688,12 +2693,14 @@ 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);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
> +
> +		fileio->bufs[i].vaddr = vb2_plane_vaddr(vb2, 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(vb2, 0);
>   	}
>   
>   	/*
> @@ -2821,15 +2828,15 @@ 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 = 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) {
> @@ -2872,7 +2879,7 @@ 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];
> +		struct vb2_buffer *b = vb2_get_buffer(q, index);
>   
>   		/*
>   		 * Check if this is the last buffer to read.
> @@ -2899,7 +2906,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(vb2_get_buffer(q, index), 0);
>   		fileio->q_count += 1;
>   		/*
>   		 * If we are queuing up buffers for the first time, then
> @@ -2970,7 +2977,7 @@ 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++);
>   			prequeue--;
>   		} else {
>   			call_void_qop(q, wait_finish, q);
> @@ -2979,7 +2986,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 1f5d235a8441..01b2bb957239 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -383,7 +383,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>   		return -EINVAL;
>   	}
>   
> -	if (q->bufs[b->index] == NULL) {
> +	if (!vb2_get_buffer(q, b->index)) {
>   		/* Should never happen */
>   		dprintk(q, 1, "%s: buffer is NULL\n", opname);
>   		return -EINVAL;
> @@ -394,7 +394,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>   		return -EINVAL;
>   	}
>   
> -	vb = q->bufs[b->index];
> +	vb = vb2_get_buffer(q, b->index);
>   	vbuf = to_vb2_v4l2_buffer(vb);
>   	ret = __verify_planes_array(vb, b);
>   	if (ret)
> @@ -628,11 +628,14 @@ 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);
> +	for (i = 0; i < q->num_buffers; i++) {
> +		vb2 = vb2_get_buffer(q, i);
> +		if (vb2->copied_timestamp &&
> +		    vb2->timestamp == timestamp)
> +			return vb2;
> +	}
>   	return NULL;
>   }
>   EXPORT_SYMBOL_GPL(vb2_find_buffer);
> @@ -664,7 +667,7 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
>   		dprintk(q, 1, "buffer index out of range\n");
>   		return -EINVAL;
>   	}
> -	vb = q->bufs[b->index];
> +	vb = vb2_get_buffer(q, b->index);
>   	ret = __verify_planes_array(vb, b);
>   	if (!ret)
>   		vb2_core_querybuf(q, b->index, b);
> diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c
> index 44b830ae01d8..8a423c1f6b55 100644
> --- a/drivers/media/platform/amphion/vpu_dbg.c
> +++ b/drivers/media/platform/amphion/vpu_dbg.c
> @@ -133,7 +133,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
>   
>   	vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx);
>   	for (i = 0; i < vq->num_buffers; i++) {
> -		struct vb2_buffer *vb = vq->bufs[i];
> +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>   		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>   
>   		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> @@ -148,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
>   
>   	vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx);
>   	for (i = 0; i < vq->num_buffers; i++) {
> -		struct vb2_buffer *vb = vq->bufs[i];
> +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>   		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>   
>   		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 969516a940ba..0be07f691d9a 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -603,7 +603,7 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
>   		return -EINVAL;
>   	}
>   
> -	vb = vq->bufs[buf->index];
> +	vb = vb2_get_buffer(vq, buf->index);
>   	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
>   	jpeg_src_buf->bs_size = buf->m.planes[0].bytesused;
>   
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> index cbb6728b8a40..f5958b6d834a 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> @@ -1701,7 +1701,7 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
>   
>   	/* update internal buffer's width/height */
>   	for (i = 0; i < vq->num_buffers; i++) {
> -		if (vb == vq->bufs[i]) {
> +		if (vb == vb2_get_buffer(vq, i)) {
>   			instance->dpb[i].width = w;
>   			instance->dpb[i].height = h;
>   			break;
> diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c
> index 318d675e5668..328016b456ba 100644
> --- a/drivers/media/test-drivers/visl/visl-dec.c
> +++ b/drivers/media/test-drivers/visl/visl-dec.c
> @@ -290,13 +290,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
>   	for (i = 0; i < out_q->num_buffers; i++) {
>   		char entry[] = "index: %u, state: %s, request_fd: %d, ";
>   		u32 old_len = len;
> -		char *q_status = visl_get_vb2_state(out_q->bufs[i]->state);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(out_q, i);
> +		char *q_status = visl_get_vb2_state(vb2->state);
>   
>   		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>   				 entry, i, q_status,
> -				 to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd);
> +				 to_vb2_v4l2_buffer(vb2)->request_fd);
>   
> -		len += visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]),
> +		len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2),
>   					   &buf[len],
>   					   TPG_STR_BUF_SZ - len);
>   
> @@ -342,13 +343,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
>   	len = 0;
>   	for (i = 0; i < cap_q->num_buffers; i++) {
>   		u32 old_len = len;
> -		char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(cap_q, i);
> +		char *q_status = visl_get_vb2_state(vb2->state);
>   
>   		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>   				 "index: %u, status: %s, timestamp: %llu, is_held: %d",
> -				 cap_q->bufs[i]->index, q_status,
> -				 cap_q->bufs[i]->timestamp,
> -				 to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held);
> +				 vb2->index, q_status,
> +				 vb2->timestamp,
> +				 to_vb2_v4l2_buffer(vb2)->is_held);
>   
>   		tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, &buf[old_len]);
>   		frame_dprintk(ctx->dev, run->dst->sequence, "%s", &buf[old_len]);
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 4b6a9d2ea372..d18c57e7aef0 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -1244,6 +1244,26 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
>   	return NULL;
>   }
>   
> +/**
> + * vb2_set_buffer() - set 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.
> + */
> +static inline void vb2_set_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> +{
> +	q->bufs[vb->index] = vb;

neither this...

> +}
> +
> +/**
> + * vb2_del_buffer() - remove a buffer from a queue
> + * @q:	pointer to &struct vb2_queue with videobuf2 queue.
> + * @vb:	pointer to &struct vb2_buffer to be removed from the queue.
> + */
> +static inline void vb2_del_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> +{
> +	q->bufs[vb->index] = NULL;

nor this does so. Is it intentional?

> +}
> +
>   /*
>    * The following functions are not part of the vb2 core API, but are useful
>    * functions for videobuf2-*.
  

Le 13/03/2023 à 17:51, Andrzej Pietrasiewicz a écrit :
> Hi Benjamin,
>
> W dniu 13.03.2023 o 14:59, Benjamin Gaignard pisze:
>> The first step before changing how vb2 buffers are stored into queue
>> is to avoid direct call to bufs arrays.
>> This patch add 2 helpers functions to set and delete 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.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> ---
>>   .../media/common/videobuf2/videobuf2-core.c   | 69 ++++++++++---------
>>   .../media/common/videobuf2/videobuf2-v4l2.c   | 17 +++--
>>   drivers/media/platform/amphion/vpu_dbg.c      |  4 +-
>>   .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  2 +-
>>   .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  2 +-
>>   drivers/media/test-drivers/visl/visl-dec.c    | 16 +++--
>>   include/media/videobuf2-core.h                | 20 ++++++
>>   7 files changed, 81 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
>> b/drivers/media/common/videobuf2/videobuf2-core.c
>> index cf6727d9c81f..b51152ace763 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-core.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
>> @@ -359,7 +359,7 @@ static void __setup_offsets(struct vb2_buffer *vb)
>>       unsigned long off = 0;
>>         if (vb->index) {
>> -        struct vb2_buffer *prev = q->bufs[vb->index - 1];
>> +        struct vb2_buffer *prev = vb2_get_buffer(q, vb->index - 1);
>
> internally vb2_get_buffer() verifies the index is within allowed 
> range, but...
>
>>           struct vb2_plane *p = &prev->planes[prev->num_planes - 1];
>>             off = PAGE_ALIGN(p->m.offset + p->length);
>> @@ -437,7 +437,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, 
>> enum vb2_memory memory,
>>           }
>>           call_void_bufop(q, init_buffer, vb);
>>   -        q->bufs[vb->index] = vb;
>> +        vb2_set_buffer(q, vb);
>>             /* Allocate video buffer memory for the MMAP type */
>>           if (memory == VB2_MEMORY_MMAP) {
>> @@ -445,7 +445,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_del_buffer(q, vb);
>>                   kfree(vb);
>>                   break;
>>               }
>> @@ -460,7 +460,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_del_buffer(q, vb);
>>                   kfree(vb);
>>                   break;
>>               }
>> @@ -483,7 +483,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;
>>   @@ -511,7 +511,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);
>> @@ -591,8 +591,10 @@ 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 *vb2 = vb2_get_buffer(q, buffer);
>> +
>> +        vb2_del_buffer(q, vb2);
>> +        kfree(vb2);
>>       }
>>         q->num_buffers -= buffers;
>> @@ -628,7 +630,7 @@ 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]))
>> +        if (vb2_buffer_in_use(q, vb2_get_buffer(q, buffer)))
>>               return true;
>>       }
>>       return false;
>> @@ -636,7 +638,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
>>     void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, 
>> void *pb)
>>   {
>> -    call_void_bufop(q, fill_user_buffer, q->bufs[index], pb);
>> +    call_void_bufop(q, fill_user_buffer, vb2_get_buffer(q, index), pb);
>>   }
>>   EXPORT_SYMBOL_GPL(vb2_core_querybuf);
>>   @@ -1547,7 +1549,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, 
>> unsigned int index, void *pb)
>>       struct vb2_buffer *vb;
>>       int ret;
>>   -    vb = q->bufs[index];
>> +    vb = vb2_get_buffer(q, index);
>>       if (vb->state != VB2_BUF_STATE_DEQUEUED) {
>>           dprintk(q, 1, "invalid buffer state %s\n",
>>               vb2_state_name(vb->state));
>> @@ -1618,7 +1620,7 @@ 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->state == VB2_BUF_STATE_ACTIVE)
>>                   vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED);
>>           }
>> @@ -1646,7 +1648,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned 
>> int index, void *pb,
>>           return -EIO;
>>       }
>>   -    vb = q->bufs[index];
>> +    vb = vb2_get_buffer(q, index);
>>         if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
>>           q->requires_requests) {
>> @@ -2022,12 +2024,15 @@ 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) {
>> +        for (i = 0; i < q->num_buffers; ++i) {
>> +            struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
>> +
>> +            if (vb2->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);
>> +                    vb2);
>> +                vb2_buffer_done(vb2, VB2_BUF_STATE_ERROR);
>>               }
>> +        }
>>           /* Must be zero now */
>>           WARN_ON(atomic_read(&q->owned_by_drv_count));
>>       }
>> @@ -2061,7 +2066,7 @@ 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 vb2_buffer *vb = vb2_get_buffer(q, i);
>>           struct media_request *req = vb->req_obj.req;
>>             /*
>> @@ -2215,7 +2220,7 @@ static int __find_plane_by_offset(struct 
>> vb2_queue *q, unsigned long off,
>>        * return its buffer and plane numbers.
>>        */
>>       for (buffer = 0; buffer < q->num_buffers; ++buffer) {
>> -        vb = q->bufs[buffer];
>> +        vb = vb2_get_buffer(q, buffer);
>>             for (plane = 0; plane < vb->num_planes; ++plane) {
>>               if (vb->planes[plane].m.offset == off) {
>> @@ -2262,7 +2267,7 @@ int vb2_core_expbuf(struct vb2_queue *q, int 
>> *fd, unsigned int type,
>>           return -EINVAL;
>>       }
>>   -    vb = q->bufs[index];
>> +    vb = vb2_get_buffer(q, index);
>>         if (plane >= vb->num_planes) {
>>           dprintk(q, 1, "buffer plane out of range\n");
>> @@ -2339,7 +2344,7 @@ int vb2_mmap(struct vb2_queue *q, struct 
>> vm_area_struct *vma)
>>       if (ret)
>>           goto unlock;
>>   -    vb = q->bufs[buffer];
>> +    vb = vb2_get_buffer(q, buffer);
>>         /*
>>        * MMAP requires page_aligned buffers.
>> @@ -2679,7 +2684,7 @@ static int __vb2_init_fileio(struct vb2_queue 
>> *q, int read)
>>        * Check if plane_count is correct
>>        * (multiplane buffers are not supported).
>>        */
>> -    if (q->bufs[0]->num_planes != 1) {
>> +    if (vb2_get_buffer(q, 0)->num_planes != 1) {
>>           ret = -EBUSY;
>>           goto err_reqbufs;
>>       }
>> @@ -2688,12 +2693,14 @@ 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);
>> +        struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
>> +
>> +        fileio->bufs[i].vaddr = vb2_plane_vaddr(vb2, 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(vb2, 0);
>>       }
>>         /*
>> @@ -2821,15 +2828,15 @@ 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 = 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) {
>> @@ -2872,7 +2879,7 @@ 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];
>> +        struct vb2_buffer *b = vb2_get_buffer(q, index);
>>             /*
>>            * Check if this is the last buffer to read.
>> @@ -2899,7 +2906,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(vb2_get_buffer(q, index), 0);
>>           fileio->q_count += 1;
>>           /*
>>            * If we are queuing up buffers for the first time, then
>> @@ -2970,7 +2977,7 @@ 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++);
>>               prequeue--;
>>           } else {
>>               call_void_qop(q, wait_finish, q);
>> @@ -2979,7 +2986,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 1f5d235a8441..01b2bb957239 100644
>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
>> @@ -383,7 +383,7 @@ static int vb2_queue_or_prepare_buf(struct 
>> vb2_queue *q, struct media_device *md
>>           return -EINVAL;
>>       }
>>   -    if (q->bufs[b->index] == NULL) {
>> +    if (!vb2_get_buffer(q, b->index)) {
>>           /* Should never happen */
>>           dprintk(q, 1, "%s: buffer is NULL\n", opname);
>>           return -EINVAL;
>> @@ -394,7 +394,7 @@ static int vb2_queue_or_prepare_buf(struct 
>> vb2_queue *q, struct media_device *md
>>           return -EINVAL;
>>       }
>>   -    vb = q->bufs[b->index];
>> +    vb = vb2_get_buffer(q, b->index);
>>       vbuf = to_vb2_v4l2_buffer(vb);
>>       ret = __verify_planes_array(vb, b);
>>       if (ret)
>> @@ -628,11 +628,14 @@ 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);
>> +    for (i = 0; i < q->num_buffers; i++) {
>> +        vb2 = vb2_get_buffer(q, i);
>> +        if (vb2->copied_timestamp &&
>> +            vb2->timestamp == timestamp)
>> +            return vb2;
>> +    }
>>       return NULL;
>>   }
>>   EXPORT_SYMBOL_GPL(vb2_find_buffer);
>> @@ -664,7 +667,7 @@ int vb2_querybuf(struct vb2_queue *q, struct 
>> v4l2_buffer *b)
>>           dprintk(q, 1, "buffer index out of range\n");
>>           return -EINVAL;
>>       }
>> -    vb = q->bufs[b->index];
>> +    vb = vb2_get_buffer(q, b->index);
>>       ret = __verify_planes_array(vb, b);
>>       if (!ret)
>>           vb2_core_querybuf(q, b->index, b);
>> diff --git a/drivers/media/platform/amphion/vpu_dbg.c 
>> b/drivers/media/platform/amphion/vpu_dbg.c
>> index 44b830ae01d8..8a423c1f6b55 100644
>> --- a/drivers/media/platform/amphion/vpu_dbg.c
>> +++ b/drivers/media/platform/amphion/vpu_dbg.c
>> @@ -133,7 +133,7 @@ static int vpu_dbg_instance(struct seq_file *s, 
>> void *data)
>>         vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx);
>>       for (i = 0; i < vq->num_buffers; i++) {
>> -        struct vb2_buffer *vb = vq->bufs[i];
>> +        struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>>           struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>             if (vb->state == VB2_BUF_STATE_DEQUEUED)
>> @@ -148,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, 
>> void *data)
>>         vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx);
>>       for (i = 0; i < vq->num_buffers; i++) {
>> -        struct vb2_buffer *vb = vq->bufs[i];
>> +        struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>>           struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>             if (vb->state == VB2_BUF_STATE_DEQUEUED)
>> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c 
>> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> index 969516a940ba..0be07f691d9a 100644
>> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
>> @@ -603,7 +603,7 @@ static int mtk_jpeg_qbuf(struct file *file, void 
>> *priv, struct v4l2_buffer *buf)
>>           return -EINVAL;
>>       }
>>   -    vb = vq->bufs[buf->index];
>> +    vb = vb2_get_buffer(vq, buf->index);
>>       jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
>>       jpeg_src_buf->bs_size = buf->m.planes[0].bytesused;
>>   diff --git 
>> a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c 
>> b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
>> index cbb6728b8a40..f5958b6d834a 100644
>> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
>> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
>> @@ -1701,7 +1701,7 @@ static int 
>> vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
>>         /* update internal buffer's width/height */
>>       for (i = 0; i < vq->num_buffers; i++) {
>> -        if (vb == vq->bufs[i]) {
>> +        if (vb == vb2_get_buffer(vq, i)) {
>>               instance->dpb[i].width = w;
>>               instance->dpb[i].height = h;
>>               break;
>> diff --git a/drivers/media/test-drivers/visl/visl-dec.c 
>> b/drivers/media/test-drivers/visl/visl-dec.c
>> index 318d675e5668..328016b456ba 100644
>> --- a/drivers/media/test-drivers/visl/visl-dec.c
>> +++ b/drivers/media/test-drivers/visl/visl-dec.c
>> @@ -290,13 +290,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, 
>> struct visl_run *run)
>>       for (i = 0; i < out_q->num_buffers; i++) {
>>           char entry[] = "index: %u, state: %s, request_fd: %d, ";
>>           u32 old_len = len;
>> -        char *q_status = visl_get_vb2_state(out_q->bufs[i]->state);
>> +        struct vb2_buffer *vb2 = vb2_get_buffer(out_q, i);
>> +        char *q_status = visl_get_vb2_state(vb2->state);
>>             len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>>                    entry, i, q_status,
>> - to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd);
>> +                 to_vb2_v4l2_buffer(vb2)->request_fd);
>>   -        len += 
>> visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]),
>> +        len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2),
>>                          &buf[len],
>>                          TPG_STR_BUF_SZ - len);
>>   @@ -342,13 +343,14 @@ static void visl_tpg_fill(struct visl_ctx 
>> *ctx, struct visl_run *run)
>>       len = 0;
>>       for (i = 0; i < cap_q->num_buffers; i++) {
>>           u32 old_len = len;
>> -        char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state);
>> +        struct vb2_buffer *vb2 = vb2_get_buffer(cap_q, i);
>> +        char *q_status = visl_get_vb2_state(vb2->state);
>>             len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>>                    "index: %u, status: %s, timestamp: %llu, is_held: 
>> %d",
>> -                 cap_q->bufs[i]->index, q_status,
>> -                 cap_q->bufs[i]->timestamp,
>> - to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held);
>> +                 vb2->index, q_status,
>> +                 vb2->timestamp,
>> +                 to_vb2_v4l2_buffer(vb2)->is_held);
>>             tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, 
>> &buf[old_len]);
>>           frame_dprintk(ctx->dev, run->dst->sequence, "%s", 
>> &buf[old_len]);
>> diff --git a/include/media/videobuf2-core.h 
>> b/include/media/videobuf2-core.h
>> index 4b6a9d2ea372..d18c57e7aef0 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -1244,6 +1244,26 @@ static inline struct vb2_buffer 
>> *vb2_get_buffer(struct vb2_queue *q,
>>       return NULL;
>>   }
>>   +/**
>> + * vb2_set_buffer() - set 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.
>> + */
>> +static inline void vb2_set_buffer(struct vb2_queue *q, struct 
>> vb2_buffer *vb)
>> +{
>> +    q->bufs[vb->index] = vb;
>
> neither this...
>
>> +}
>> +
>> +/**
>> + * vb2_del_buffer() - remove a buffer from a queue
>> + * @q:    pointer to &struct vb2_queue with videobuf2 queue.
>> + * @vb:    pointer to &struct vb2_buffer to be removed from the queue.
>> + */
>> +static inline void vb2_del_buffer(struct vb2_queue *q, struct 
>> vb2_buffer *vb)
>> +{
>> +    q->bufs[vb->index] = NULL;
>
> nor this does so. Is it intentional?

yes inside the helpers that don't make sense to use it.
In the next patch they are replaced by list calls.

Benjamin

>
>> +}
>> +
>>   /*
>>    * The following functions are not part of the vb2 core API, but 
>> are useful
>>    * functions for videobuf2-*.
>
  

Hi Benjamin,

Thank you for the patch.

On Mon, Mar 13, 2023 at 02:59:13PM +0100, Benjamin Gaignard wrote:
> The first step before changing how vb2 buffers are stored into queue
> is to avoid direct call to bufs arrays.

s/call/access/

> This patch add 2 helpers functions to set and delete vb2 buffers

s/add/adds/

> from a queue. With these 2 and vb2_get_buffer(), bufs field of
> struct vb2_queue becomes like a private member of the structure.

As the patch does more than using vb2_get_buffer() everywhere, I would
rewrite the subject line to

media: videobuf2: Access vb2_queue bufs array through helper functions

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>  .../media/common/videobuf2/videobuf2-core.c   | 69 ++++++++++---------
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 17 +++--
>  drivers/media/platform/amphion/vpu_dbg.c      |  4 +-
>  .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  2 +-
>  .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  2 +-
>  drivers/media/test-drivers/visl/visl-dec.c    | 16 +++--
>  include/media/videobuf2-core.h                | 20 ++++++
>  7 files changed, 81 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index cf6727d9c81f..b51152ace763 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -359,7 +359,7 @@ static void __setup_offsets(struct vb2_buffer *vb)
>  	unsigned long off = 0;
>  
>  	if (vb->index) {
> -		struct vb2_buffer *prev = q->bufs[vb->index - 1];
> +		struct vb2_buffer *prev = vb2_get_buffer(q, vb->index - 1);
>  		struct vb2_plane *p = &prev->planes[prev->num_planes - 1];
>  
>  		off = PAGE_ALIGN(p->m.offset + p->length);
> @@ -437,7 +437,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
>  		}
>  		call_void_bufop(q, init_buffer, vb);
>  
> -		q->bufs[vb->index] = vb;
> +		vb2_set_buffer(q, vb);
>  
>  		/* Allocate video buffer memory for the MMAP type */
>  		if (memory == VB2_MEMORY_MMAP) {
> @@ -445,7 +445,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_del_buffer(q, vb);
>  				kfree(vb);

vb2_del_buffer() make it sounds like the buffer gets deleted, yet you
free it right after. That could be confusing. One option would be to
call the function vb2_remove_buffer() (or possibly even better as it's
more explicit, vb2_queue_remove_buffer()). Another one would be to move
the kfree() call to vb2_del_buffer().

Similarly, vb2_add_buffer() or vb2_queue_add_buffer() would be better
names for vb2_set_buffer().

>  				break;
>  			}
> @@ -460,7 +460,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_del_buffer(q, vb);
>  				kfree(vb);
>  				break;
>  			}
> @@ -483,7 +483,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);

I wonder if this could be optimized in subsequent patches by using a
list walk instead of a for loop on the buffer index. Same in multiple
locations below. I'd add the list walks right after this patch. A
vb2_buffer list walk macro (for_each_vb2_buffer for instance) would be
useful.

>  		if (!vb)
>  			continue;
>  
> @@ -511,7 +511,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);
> @@ -591,8 +591,10 @@ 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 *vb2 = vb2_get_buffer(q, buffer);
> +
> +		vb2_del_buffer(q, vb2);
> +		kfree(vb2);
>  	}
>  
>  	q->num_buffers -= buffers;
> @@ -628,7 +630,7 @@ 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]))
> +		if (vb2_buffer_in_use(q, vb2_get_buffer(q, buffer)))
>  			return true;
>  	}
>  	return false;
> @@ -636,7 +638,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
>  
>  void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb)
>  {
> -	call_void_bufop(q, fill_user_buffer, q->bufs[index], pb);
> +	call_void_bufop(q, fill_user_buffer, vb2_get_buffer(q, index), pb);
>  }
>  EXPORT_SYMBOL_GPL(vb2_core_querybuf);
>  
> @@ -1547,7 +1549,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
>  	struct vb2_buffer *vb;
>  	int ret;
>  
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>  	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
>  		dprintk(q, 1, "invalid buffer state %s\n",
>  			vb2_state_name(vb->state));
> @@ -1618,7 +1620,7 @@ 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->state == VB2_BUF_STATE_ACTIVE)
>  				vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED);
>  		}
> @@ -1646,7 +1648,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>  		return -EIO;
>  	}
>  
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>  
>  	if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
>  	    q->requires_requests) {
> @@ -2022,12 +2024,15 @@ 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) {
> +		for (i = 0; i < q->num_buffers; ++i) {
> +			struct vb2_buffer *vb2 = vb2_get_buffer(q, i);

videobuf2 usually names vb2_buffer variables just 'vb' (there's no
occurrence of 'vb2' in the existing code base). Could you rename this
and other variables in this patch ?

> +
> +			if (vb2->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);
> +					vb2);
> +				vb2_buffer_done(vb2, VB2_BUF_STATE_ERROR);
>  			}
> +		}
>  		/* Must be zero now */
>  		WARN_ON(atomic_read(&q->owned_by_drv_count));
>  	}
> @@ -2061,7 +2066,7 @@ 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 vb2_buffer *vb = vb2_get_buffer(q, i);
>  		struct media_request *req = vb->req_obj.req;
>  
>  		/*
> @@ -2215,7 +2220,7 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
>  	 * return its buffer and plane numbers.
>  	 */
>  	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> -		vb = q->bufs[buffer];
> +		vb = vb2_get_buffer(q, buffer);
>  
>  		for (plane = 0; plane < vb->num_planes; ++plane) {
>  			if (vb->planes[plane].m.offset == off) {
> @@ -2262,7 +2267,7 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
>  		return -EINVAL;
>  	}
>  
> -	vb = q->bufs[index];
> +	vb = vb2_get_buffer(q, index);
>  
>  	if (plane >= vb->num_planes) {
>  		dprintk(q, 1, "buffer plane out of range\n");
> @@ -2339,7 +2344,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>  	if (ret)
>  		goto unlock;
>  
> -	vb = q->bufs[buffer];
> +	vb = vb2_get_buffer(q, buffer);
>  
>  	/*
>  	 * MMAP requires page_aligned buffers.
> @@ -2679,7 +2684,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>  	 * Check if plane_count is correct
>  	 * (multiplane buffers are not supported).
>  	 */
> -	if (q->bufs[0]->num_planes != 1) {
> +	if (vb2_get_buffer(q, 0)->num_planes != 1) {

This may become problematic as there will be no guarantee going forward
that buffer 0 exists. Maybe it's fine for the fileio helpers though.

>  		ret = -EBUSY;
>  		goto err_reqbufs;
>  	}
> @@ -2688,12 +2693,14 @@ 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);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
> +
> +		fileio->bufs[i].vaddr = vb2_plane_vaddr(vb2, 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(vb2, 0);
>  	}
>  
>  	/*
> @@ -2821,15 +2828,15 @@ 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 = 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) {
> @@ -2872,7 +2879,7 @@ 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];
> +		struct vb2_buffer *b = vb2_get_buffer(q, index);
>  
>  		/*
>  		 * Check if this is the last buffer to read.
> @@ -2899,7 +2906,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(vb2_get_buffer(q, index), 0);
>  		fileio->q_count += 1;
>  		/*
>  		 * If we are queuing up buffers for the first time, then
> @@ -2970,7 +2977,7 @@ 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++);
>  			prequeue--;
>  		} else {
>  			call_void_qop(q, wait_finish, q);
> @@ -2979,7 +2986,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 1f5d235a8441..01b2bb957239 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -383,7 +383,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>  		return -EINVAL;
>  	}
>  
> -	if (q->bufs[b->index] == NULL) {
> +	if (!vb2_get_buffer(q, b->index)) {
>  		/* Should never happen */
>  		dprintk(q, 1, "%s: buffer is NULL\n", opname);
>  		return -EINVAL;
> @@ -394,7 +394,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
>  		return -EINVAL;
>  	}
>  
> -	vb = q->bufs[b->index];
> +	vb = vb2_get_buffer(q, b->index);
>  	vbuf = to_vb2_v4l2_buffer(vb);
>  	ret = __verify_planes_array(vb, b);
>  	if (ret)
> @@ -628,11 +628,14 @@ 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);
> +	for (i = 0; i < q->num_buffers; i++) {
> +		vb2 = vb2_get_buffer(q, i);
> +		if (vb2->copied_timestamp &&
> +		    vb2->timestamp == timestamp)
> +			return vb2;
> +	}
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(vb2_find_buffer);
> @@ -664,7 +667,7 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
>  		dprintk(q, 1, "buffer index out of range\n");
>  		return -EINVAL;
>  	}
> -	vb = q->bufs[b->index];
> +	vb = vb2_get_buffer(q, b->index);
>  	ret = __verify_planes_array(vb, b);
>  	if (!ret)
>  		vb2_core_querybuf(q, b->index, b);
> diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c
> index 44b830ae01d8..8a423c1f6b55 100644
> --- a/drivers/media/platform/amphion/vpu_dbg.c
> +++ b/drivers/media/platform/amphion/vpu_dbg.c
> @@ -133,7 +133,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
>  
>  	vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx);
>  	for (i = 0; i < vq->num_buffers; i++) {
> -		struct vb2_buffer *vb = vq->bufs[i];
> +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>  		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>  
>  		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> @@ -148,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
>  
>  	vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx);
>  	for (i = 0; i < vq->num_buffers; i++) {
> -		struct vb2_buffer *vb = vq->bufs[i];
> +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
>  		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>  
>  		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 969516a940ba..0be07f691d9a 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -603,7 +603,7 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
>  		return -EINVAL;
>  	}
>  
> -	vb = vq->bufs[buf->index];
> +	vb = vb2_get_buffer(vq, buf->index);
>  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
>  	jpeg_src_buf->bs_size = buf->m.planes[0].bytesused;
>  
> diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> index cbb6728b8a40..f5958b6d834a 100644
> --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> @@ -1701,7 +1701,7 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
>  
>  	/* update internal buffer's width/height */
>  	for (i = 0; i < vq->num_buffers; i++) {
> -		if (vb == vq->bufs[i]) {
> +		if (vb == vb2_get_buffer(vq, i)) {
>  			instance->dpb[i].width = w;
>  			instance->dpb[i].height = h;
>  			break;
> diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c
> index 318d675e5668..328016b456ba 100644
> --- a/drivers/media/test-drivers/visl/visl-dec.c
> +++ b/drivers/media/test-drivers/visl/visl-dec.c
> @@ -290,13 +290,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
>  	for (i = 0; i < out_q->num_buffers; i++) {
>  		char entry[] = "index: %u, state: %s, request_fd: %d, ";
>  		u32 old_len = len;
> -		char *q_status = visl_get_vb2_state(out_q->bufs[i]->state);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(out_q, i);
> +		char *q_status = visl_get_vb2_state(vb2->state);
>  
>  		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>  				 entry, i, q_status,
> -				 to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd);
> +				 to_vb2_v4l2_buffer(vb2)->request_fd);
>  
> -		len += visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]),
> +		len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2),
>  					   &buf[len],
>  					   TPG_STR_BUF_SZ - len);
>  
> @@ -342,13 +343,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
>  	len = 0;
>  	for (i = 0; i < cap_q->num_buffers; i++) {
>  		u32 old_len = len;
> -		char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state);
> +		struct vb2_buffer *vb2 = vb2_get_buffer(cap_q, i);
> +		char *q_status = visl_get_vb2_state(vb2->state);
>  
>  		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
>  				 "index: %u, status: %s, timestamp: %llu, is_held: %d",
> -				 cap_q->bufs[i]->index, q_status,
> -				 cap_q->bufs[i]->timestamp,
> -				 to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held);
> +				 vb2->index, q_status,
> +				 vb2->timestamp,
> +				 to_vb2_v4l2_buffer(vb2)->is_held);
>  
>  		tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, &buf[old_len]);
>  		frame_dprintk(ctx->dev, run->dst->sequence, "%s", &buf[old_len]);
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 4b6a9d2ea372..d18c57e7aef0 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -1244,6 +1244,26 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
>  	return NULL;
>  }
>  
> +/**
> + * vb2_set_buffer() - set 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.
> + */
> +static inline void vb2_set_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> +{
> +	q->bufs[vb->index] = vb;
> +}
> +
> +/**
> + * vb2_del_buffer() - remove a buffer from a queue
> + * @q:	pointer to &struct vb2_queue with videobuf2 queue.
> + * @vb:	pointer to &struct vb2_buffer to be removed from the queue.
> + */
> +static inline void vb2_del_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> +{
> +	q->bufs[vb->index] = NULL;
> +}
> +
>  /*
>   * The following functions are not part of the vb2 core API, but are useful
>   * functions for videobuf2-*.
  

On Mon, Mar 13, 2023 at 08:01:09PM +0200, Laurent Pinchart wrote:
> Hi Benjamin,
> 
> Thank you for the patch.
> 
> On Mon, Mar 13, 2023 at 02:59:13PM +0100, Benjamin Gaignard wrote:
> > The first step before changing how vb2 buffers are stored into queue
> > is to avoid direct call to bufs arrays.
> 
> s/call/access/
> 
> > This patch add 2 helpers functions to set and delete vb2 buffers
> 
> s/add/adds/
> 
> > from a queue. With these 2 and vb2_get_buffer(), bufs field of
> > struct vb2_queue becomes like a private member of the structure.
> 
> As the patch does more than using vb2_get_buffer() everywhere, I would
> rewrite the subject line to
> 
> media: videobuf2: Access vb2_queue bufs array through helper functions
> 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> >  .../media/common/videobuf2/videobuf2-core.c   | 69 ++++++++++---------
> >  .../media/common/videobuf2/videobuf2-v4l2.c   | 17 +++--
> >  drivers/media/platform/amphion/vpu_dbg.c      |  4 +-
> >  .../platform/mediatek/jpeg/mtk_jpeg_core.c    |  2 +-
> >  .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  2 +-
> >  drivers/media/test-drivers/visl/visl-dec.c    | 16 +++--
> >  include/media/videobuf2-core.h                | 20 ++++++
> >  7 files changed, 81 insertions(+), 49 deletions(-)
> > 
> > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> > index cf6727d9c81f..b51152ace763 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-core.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> > @@ -359,7 +359,7 @@ static void __setup_offsets(struct vb2_buffer *vb)
> >  	unsigned long off = 0;
> >  
> >  	if (vb->index) {
> > -		struct vb2_buffer *prev = q->bufs[vb->index - 1];
> > +		struct vb2_buffer *prev = vb2_get_buffer(q, vb->index - 1);
> >  		struct vb2_plane *p = &prev->planes[prev->num_planes - 1];
> >  
> >  		off = PAGE_ALIGN(p->m.offset + p->length);
> > @@ -437,7 +437,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
> >  		}
> >  		call_void_bufop(q, init_buffer, vb);
> >  
> > -		q->bufs[vb->index] = vb;
> > +		vb2_set_buffer(q, vb);
> >  
> >  		/* Allocate video buffer memory for the MMAP type */
> >  		if (memory == VB2_MEMORY_MMAP) {
> > @@ -445,7 +445,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_del_buffer(q, vb);
> >  				kfree(vb);
> 
> vb2_del_buffer() make it sounds like the buffer gets deleted, yet you
> free it right after. That could be confusing. One option would be to
> call the function vb2_remove_buffer() (or possibly even better as it's
> more explicit, vb2_queue_remove_buffer()). Another one would be to move
> the kfree() call to vb2_del_buffer().
> 
> Similarly, vb2_add_buffer() or vb2_queue_add_buffer() would be better
> names for vb2_set_buffer().
> 
> >  				break;
> >  			}
> > @@ -460,7 +460,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_del_buffer(q, vb);
> >  				kfree(vb);
> >  				break;
> >  			}
> > @@ -483,7 +483,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);
> 
> I wonder if this could be optimized in subsequent patches by using a
> list walk instead of a for loop on the buffer index. Same in multiple
> locations below. I'd add the list walks right after this patch. A
> vb2_buffer list walk macro (for_each_vb2_buffer for instance) would be
> useful.

I meant right after patch 2/4, as we need a list first :-)

> >  		if (!vb)
> >  			continue;
> >  
> > @@ -511,7 +511,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);
> > @@ -591,8 +591,10 @@ 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 *vb2 = vb2_get_buffer(q, buffer);
> > +
> > +		vb2_del_buffer(q, vb2);
> > +		kfree(vb2);
> >  	}
> >  
> >  	q->num_buffers -= buffers;
> > @@ -628,7 +630,7 @@ 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]))
> > +		if (vb2_buffer_in_use(q, vb2_get_buffer(q, buffer)))
> >  			return true;
> >  	}
> >  	return false;
> > @@ -636,7 +638,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
> >  
> >  void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb)
> >  {
> > -	call_void_bufop(q, fill_user_buffer, q->bufs[index], pb);
> > +	call_void_bufop(q, fill_user_buffer, vb2_get_buffer(q, index), pb);
> >  }
> >  EXPORT_SYMBOL_GPL(vb2_core_querybuf);
> >  
> > @@ -1547,7 +1549,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
> >  	struct vb2_buffer *vb;
> >  	int ret;
> >  
> > -	vb = q->bufs[index];
> > +	vb = vb2_get_buffer(q, index);
> >  	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> >  		dprintk(q, 1, "invalid buffer state %s\n",
> >  			vb2_state_name(vb->state));
> > @@ -1618,7 +1620,7 @@ 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->state == VB2_BUF_STATE_ACTIVE)
> >  				vb2_buffer_done(vb, VB2_BUF_STATE_QUEUED);
> >  		}
> > @@ -1646,7 +1648,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
> >  		return -EIO;
> >  	}
> >  
> > -	vb = q->bufs[index];
> > +	vb = vb2_get_buffer(q, index);
> >  
> >  	if (!req && vb->state != VB2_BUF_STATE_IN_REQUEST &&
> >  	    q->requires_requests) {
> > @@ -2022,12 +2024,15 @@ 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) {
> > +		for (i = 0; i < q->num_buffers; ++i) {
> > +			struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
> 
> videobuf2 usually names vb2_buffer variables just 'vb' (there's no
> occurrence of 'vb2' in the existing code base). Could you rename this
> and other variables in this patch ?
> 
> > +
> > +			if (vb2->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);
> > +					vb2);
> > +				vb2_buffer_done(vb2, VB2_BUF_STATE_ERROR);
> >  			}
> > +		}
> >  		/* Must be zero now */
> >  		WARN_ON(atomic_read(&q->owned_by_drv_count));
> >  	}
> > @@ -2061,7 +2066,7 @@ 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 vb2_buffer *vb = vb2_get_buffer(q, i);
> >  		struct media_request *req = vb->req_obj.req;
> >  
> >  		/*
> > @@ -2215,7 +2220,7 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
> >  	 * return its buffer and plane numbers.
> >  	 */
> >  	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> > -		vb = q->bufs[buffer];
> > +		vb = vb2_get_buffer(q, buffer);
> >  
> >  		for (plane = 0; plane < vb->num_planes; ++plane) {
> >  			if (vb->planes[plane].m.offset == off) {
> > @@ -2262,7 +2267,7 @@ int vb2_core_expbuf(struct vb2_queue *q, int *fd, unsigned int type,
> >  		return -EINVAL;
> >  	}
> >  
> > -	vb = q->bufs[index];
> > +	vb = vb2_get_buffer(q, index);
> >  
> >  	if (plane >= vb->num_planes) {
> >  		dprintk(q, 1, "buffer plane out of range\n");
> > @@ -2339,7 +2344,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
> >  	if (ret)
> >  		goto unlock;
> >  
> > -	vb = q->bufs[buffer];
> > +	vb = vb2_get_buffer(q, buffer);
> >  
> >  	/*
> >  	 * MMAP requires page_aligned buffers.
> > @@ -2679,7 +2684,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
> >  	 * Check if plane_count is correct
> >  	 * (multiplane buffers are not supported).
> >  	 */
> > -	if (q->bufs[0]->num_planes != 1) {
> > +	if (vb2_get_buffer(q, 0)->num_planes != 1) {
> 
> This may become problematic as there will be no guarantee going forward
> that buffer 0 exists. Maybe it's fine for the fileio helpers though.
> 
> >  		ret = -EBUSY;
> >  		goto err_reqbufs;
> >  	}
> > @@ -2688,12 +2693,14 @@ 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);
> > +		struct vb2_buffer *vb2 = vb2_get_buffer(q, i);
> > +
> > +		fileio->bufs[i].vaddr = vb2_plane_vaddr(vb2, 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(vb2, 0);
> >  	}
> >  
> >  	/*
> > @@ -2821,15 +2828,15 @@ 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 = 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) {
> > @@ -2872,7 +2879,7 @@ 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];
> > +		struct vb2_buffer *b = vb2_get_buffer(q, index);
> >  
> >  		/*
> >  		 * Check if this is the last buffer to read.
> > @@ -2899,7 +2906,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(vb2_get_buffer(q, index), 0);
> >  		fileio->q_count += 1;
> >  		/*
> >  		 * If we are queuing up buffers for the first time, then
> > @@ -2970,7 +2977,7 @@ 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++);
> >  			prequeue--;
> >  		} else {
> >  			call_void_qop(q, wait_finish, q);
> > @@ -2979,7 +2986,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 1f5d235a8441..01b2bb957239 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -383,7 +383,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (q->bufs[b->index] == NULL) {
> > +	if (!vb2_get_buffer(q, b->index)) {
> >  		/* Should never happen */
> >  		dprintk(q, 1, "%s: buffer is NULL\n", opname);
> >  		return -EINVAL;
> > @@ -394,7 +394,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
> >  		return -EINVAL;
> >  	}
> >  
> > -	vb = q->bufs[b->index];
> > +	vb = vb2_get_buffer(q, b->index);
> >  	vbuf = to_vb2_v4l2_buffer(vb);
> >  	ret = __verify_planes_array(vb, b);
> >  	if (ret)
> > @@ -628,11 +628,14 @@ 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);
> > +	for (i = 0; i < q->num_buffers; i++) {
> > +		vb2 = vb2_get_buffer(q, i);
> > +		if (vb2->copied_timestamp &&
> > +		    vb2->timestamp == timestamp)
> > +			return vb2;
> > +	}
> >  	return NULL;
> >  }
> >  EXPORT_SYMBOL_GPL(vb2_find_buffer);
> > @@ -664,7 +667,7 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b)
> >  		dprintk(q, 1, "buffer index out of range\n");
> >  		return -EINVAL;
> >  	}
> > -	vb = q->bufs[b->index];
> > +	vb = vb2_get_buffer(q, b->index);
> >  	ret = __verify_planes_array(vb, b);
> >  	if (!ret)
> >  		vb2_core_querybuf(q, b->index, b);
> > diff --git a/drivers/media/platform/amphion/vpu_dbg.c b/drivers/media/platform/amphion/vpu_dbg.c
> > index 44b830ae01d8..8a423c1f6b55 100644
> > --- a/drivers/media/platform/amphion/vpu_dbg.c
> > +++ b/drivers/media/platform/amphion/vpu_dbg.c
> > @@ -133,7 +133,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
> >  
> >  	vq = v4l2_m2m_get_src_vq(inst->fh.m2m_ctx);
> >  	for (i = 0; i < vq->num_buffers; i++) {
> > -		struct vb2_buffer *vb = vq->bufs[i];
> > +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
> >  		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >  
> >  		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> > @@ -148,7 +148,7 @@ static int vpu_dbg_instance(struct seq_file *s, void *data)
> >  
> >  	vq = v4l2_m2m_get_dst_vq(inst->fh.m2m_ctx);
> >  	for (i = 0; i < vq->num_buffers; i++) {
> > -		struct vb2_buffer *vb = vq->bufs[i];
> > +		struct vb2_buffer *vb = vb2_get_buffer(vq, i);
> >  		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >  
> >  		if (vb->state == VB2_BUF_STATE_DEQUEUED)
> > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > index 969516a940ba..0be07f691d9a 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -603,7 +603,7 @@ static int mtk_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> >  		return -EINVAL;
> >  	}
> >  
> > -	vb = vq->bufs[buf->index];
> > +	vb = vb2_get_buffer(vq, buf->index);
> >  	jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(vb);
> >  	jpeg_src_buf->bs_size = buf->m.planes[0].bytesused;
> >  
> > diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> > index cbb6728b8a40..f5958b6d834a 100644
> > --- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> > +++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
> > @@ -1701,7 +1701,7 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
> >  
> >  	/* update internal buffer's width/height */
> >  	for (i = 0; i < vq->num_buffers; i++) {
> > -		if (vb == vq->bufs[i]) {
> > +		if (vb == vb2_get_buffer(vq, i)) {
> >  			instance->dpb[i].width = w;
> >  			instance->dpb[i].height = h;
> >  			break;
> > diff --git a/drivers/media/test-drivers/visl/visl-dec.c b/drivers/media/test-drivers/visl/visl-dec.c
> > index 318d675e5668..328016b456ba 100644
> > --- a/drivers/media/test-drivers/visl/visl-dec.c
> > +++ b/drivers/media/test-drivers/visl/visl-dec.c
> > @@ -290,13 +290,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
> >  	for (i = 0; i < out_q->num_buffers; i++) {
> >  		char entry[] = "index: %u, state: %s, request_fd: %d, ";
> >  		u32 old_len = len;
> > -		char *q_status = visl_get_vb2_state(out_q->bufs[i]->state);
> > +		struct vb2_buffer *vb2 = vb2_get_buffer(out_q, i);
> > +		char *q_status = visl_get_vb2_state(vb2->state);
> >  
> >  		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
> >  				 entry, i, q_status,
> > -				 to_vb2_v4l2_buffer(out_q->bufs[i])->request_fd);
> > +				 to_vb2_v4l2_buffer(vb2)->request_fd);
> >  
> > -		len += visl_fill_bytesused(to_vb2_v4l2_buffer(out_q->bufs[i]),
> > +		len += visl_fill_bytesused(to_vb2_v4l2_buffer(vb2),
> >  					   &buf[len],
> >  					   TPG_STR_BUF_SZ - len);
> >  
> > @@ -342,13 +343,14 @@ static void visl_tpg_fill(struct visl_ctx *ctx, struct visl_run *run)
> >  	len = 0;
> >  	for (i = 0; i < cap_q->num_buffers; i++) {
> >  		u32 old_len = len;
> > -		char *q_status = visl_get_vb2_state(cap_q->bufs[i]->state);
> > +		struct vb2_buffer *vb2 = vb2_get_buffer(cap_q, i);
> > +		char *q_status = visl_get_vb2_state(vb2->state);
> >  
> >  		len += scnprintf(&buf[len], TPG_STR_BUF_SZ - len,
> >  				 "index: %u, status: %s, timestamp: %llu, is_held: %d",
> > -				 cap_q->bufs[i]->index, q_status,
> > -				 cap_q->bufs[i]->timestamp,
> > -				 to_vb2_v4l2_buffer(cap_q->bufs[i])->is_held);
> > +				 vb2->index, q_status,
> > +				 vb2->timestamp,
> > +				 to_vb2_v4l2_buffer(vb2)->is_held);
> >  
> >  		tpg_gen_text(&ctx->tpg, basep, line++ * line_height, 16, &buf[old_len]);
> >  		frame_dprintk(ctx->dev, run->dst->sequence, "%s", &buf[old_len]);
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index 4b6a9d2ea372..d18c57e7aef0 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -1244,6 +1244,26 @@ static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> >  	return NULL;
> >  }
> >  
> > +/**
> > + * vb2_set_buffer() - set 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.
> > + */
> > +static inline void vb2_set_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> > +{
> > +	q->bufs[vb->index] = vb;
> > +}
> > +
> > +/**
> > + * vb2_del_buffer() - remove a buffer from a queue
> > + * @q:	pointer to &struct vb2_queue with videobuf2 queue.
> > + * @vb:	pointer to &struct vb2_buffer to be removed from the queue.
> > + */
> > +static inline void vb2_del_buffer(struct vb2_queue *q, struct vb2_buffer *vb)
> > +{
> > +	q->bufs[vb->index] = NULL;
> > +}
> > +
> >  /*
> >   * The following functions are not part of the vb2 core API, but are useful
> >   * functions for videobuf2-*.