diff mbox series

[v14,08/56] media: videobuf2: Use vb2_get_num_buffers() helper

Message ID 20231031163104.112469-9-benjamin.gaignard@collabora.com
State New
Headers show
Series Add DELETE_BUF ioctl | expand

Commit Message

Benjamin Gaignard Oct. 31, 2023, 4:30 p.m. UTC 
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 <benjamin.gaignard@collabora.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 92 +++++++++++--------
 .../media/common/videobuf2/videobuf2-v4l2.c   |  4 +-
 2 files changed, 54 insertions(+), 42 deletions(-)

Comments

Tomasz Figa Nov. 9, 2023, 4:36 a.m. UTC | #1 
On Wed, Nov 8, 2023 at 10:22 PM Benjamin Gaignard
<benjamin.gaignard@collabora.com> wrote:
>
>
> Le 08/11/2023 à 10:42, Tomasz Figa a écrit :
> > On Tue, Oct 31, 2023 at 05:30:16PM +0100, Benjamin Gaignard wrote:
> >> 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 <benjamin.gaignard@collabora.com>
> >> ---
> >>   .../media/common/videobuf2/videobuf2-core.c   | 92 +++++++++++--------
> >>   .../media/common/videobuf2/videobuf2-v4l2.c   |  4 +-
> >>   2 files changed, 54 insertions(+), 42 deletions(-)
> >>
> >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> >> index b406a30a9b35..c5c5ae4d213d 100644
> >> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> >> @@ -444,13 +444,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);
> > I guess it's safe in this specific situation, but was there any reason
> > behind not just calling vb2_get_num_buffers() directly here?
> >
> >>
> >>      for (buffer = 0; buffer < num_buffers; ++buffer) {
> >>              /* Allocate vb2 buffer structures */
> >> @@ -470,7 +471,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);
> > In this case it should also be fine, but actually now this is a loop and if
> > somone doesn't know what the other code in the loop does, one could be
> > concerned that the num buffers actually could have changed, but we still
> > use the cached one that we got at the beginning of the function.
> >
> > (Ideally I'd imagine vb2_queue_add_buffer() to append the buffer
> > at the end of the queue and increment the num_buffers internally, but it
> > doesn't have to happen now, as this series is already quite complex...)
>
> That will be the case later in the series when I replace num_buffers field
> by a bitmap. Until that I prefer to limit the changes in this loop.
>
> >
> >>              call_void_bufop(q, init_buffer, vb);
> >>
> >>              /* Allocate video buffer memory for the MMAP type */
> > [snip]
> >> @@ -2513,7 +2519,8 @@ 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));
> >> +    q->num_buffers = 0;
> > Unrelated change?
>
> No because I found a case where q->num_buffers wasn't correctly reset while testing.
>

Could you provide more details about that case? Shouldn't it be fixed instead?

It's a bit weird to me, because __vb2_queue_free() is supposed to
decrement q->num_buffers by the number of buffers freed and it's
called with vb2_get_num_buffers() just one line above.

> >
> >>      mutex_unlock(&q->mmap_lock);
> >>   }
> >>   EXPORT_SYMBOL_GPL(vb2_core_queue_release);
> >> @@ -2542,7 +2549,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))
> >> @@ -2580,7 +2587,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)) {
> >> @@ -2629,8 +2636,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 already queued buffers
> >> + *          then a new buffer must be dequeued.
> > Hmm, that's a significant meaning change compared to the original text. Is
> > it indended?
>
> Does "If equal to number of buffers in the vb2_queue then a new buffer must be dequeued."
> sound better for you ?

Yes, I think now it matches the original meaning. Thanks.

>
> >
> >>    * @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
> >> @@ -2640,7 +2647,7 @@ 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
> >> + *          in this field. Once it is equal to num_buffers all
> > It's not clear what num_buffers means here. Would it make sense to instead
> > say "number of buffers in the vb2_queue"?
>
> Yes I will change that
>
> >
> >>    *         available buffers have been queued and __vb2_perform_fileio()
> >>    *         should start the normal dequeue/queue cycle.
> >>    *
> >> @@ -2690,7 +2697,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;
> >>
> >>      /*
> >> @@ -2740,7 +2747,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);
> >>
> >> @@ -2759,18 +2766,23 @@ 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);
> >> +            for (i = 0; i < vb2_get_num_buffers(q); i++) {
> >> +                    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;
> >>              }
> > Doesn't this part belong to the previous patch that changes q->bufs[x] to
> > vb2_get_buffer()?
>
> Yes I will change that too.
>
> >
> >>              /*
> >>               * All buffers have been queued, so mark that by setting
> >> -             * initial_index to q->num_buffers
> >> +             * initial_index to num_buffers
> > What num_buffers?
>
> I will use your wording: "the number of buffers in the vb2_queue"
>

Thanks!

> >
> >>               */
> >> -            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;
> >>      }
> >>
> >>      /*
> >> @@ -2964,12 +2976,12 @@ 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
> >> +             * queued for the first time (initial_index < num_buffers)
> >> +             * or it is equal to num_buffers, meaning that the next
> > What num_buffers?
>
> Same here

Thanks!

Best regards,
Tomasz
diff mbox series

Patch

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b406a30a9b35..c5c5ae4d213d 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -444,13 +444,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 */
@@ -470,7 +471,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 */
@@ -514,8 +515,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)
@@ -539,11 +541,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);
 
@@ -559,7 +562,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;
@@ -585,7 +588,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;
 
@@ -637,7 +640,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);
 
@@ -649,7 +652,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);
 	}
@@ -680,7 +683,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)
@@ -806,6 +809,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;
@@ -821,7 +825,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)) {
 		/*
@@ -839,7 +843,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);
 
 		/*
@@ -934,7 +938,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);
@@ -968,10 +972,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;
 	}
@@ -1000,7 +1005,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;
@@ -1032,7 +1037,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,
@@ -1053,7 +1058,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);
@@ -1670,7 +1675,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)
@@ -2076,7 +2081,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)
@@ -2120,7 +2125,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;
 
@@ -2168,6 +2173,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) {
@@ -2180,12 +2186,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;
@@ -2513,7 +2519,8 @@  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));
+	q->num_buffers = 0;
 	mutex_unlock(&q->mmap_lock);
 }
 EXPORT_SYMBOL_GPL(vb2_core_queue_release);
@@ -2542,7 +2549,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))
@@ -2580,7 +2587,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)) {
@@ -2629,8 +2636,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 already queued buffers
+ *		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
@@ -2640,7 +2647,7 @@  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
+ *		in this field. Once it is equal to num_buffers all
  *		available buffers have been queued and __vb2_perform_fileio()
  *		should start the normal dequeue/queue cycle.
  *
@@ -2690,7 +2697,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;
 
 	/*
@@ -2740,7 +2747,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);
 
@@ -2759,18 +2766,23 @@  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);
+		for (i = 0; i < vb2_get_num_buffers(q); i++) {
+			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;
 		}
 		/*
 		 * All buffers have been queued, so mark that by setting
-		 * initial_index to q->num_buffers
+		 * initial_index to num_buffers
 		 */
-		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;
 	}
 
 	/*
@@ -2964,12 +2976,12 @@  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
+		 * queued for the first time (initial_index < num_buffers)
+		 * or it is equal to num_buffers, meaning that the next
 		 * time we need to dequeue a buffer since we've now queued up
 		 * all the 'first time' buffers.
 		 */
@@ -3016,7 +3028,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 c6ebc8d2c537..7d798fb15c0b 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -627,7 +627,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)
@@ -761,7 +761,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;