V4L2 Explicit Synchronization

The in-fence implementation involves having a per-buffer fence callback, that triggers on the fence signal. The fence callback is called asynchronously
and needs a valid reference to the associated ideobuf2 buffer.

Allow this by making the vb2_buffer refcounted, so it can be passed to other contexts.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/common/videobuf2/videobuf2-core.c | 27
++++++++++++++++++++++---
 include/media/videobuf2-core.h                  |  7 +++++--
 2 files changed, 29 insertions(+), 5 deletions(-)

drivers/media/common/videobuf2/videobuf2-core.c +21 -0

index d3f7bb33a54d..f1feb45c1e37 100644
@@ -190,6 +190,26 @@ module_param(debug, int, 0644);
 static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
 
+static void __vb2_buffer_free(struct kref *kref)
+{
+	struct vb2_buffer *vb =
+		container_of(kref, struct vb2_buffer, refcount);
+	kfree(vb);
+}
+
+static void __vb2_buffer_put(struct vb2_buffer *vb)
+{
+	if (vb)
+		kref_put(&vb->refcount, __vb2_buffer_free);
+}
+
+static struct vb2_buffer *__vb2_buffer_get(struct vb2_buffer *vb)
+{
+	if (vb)
+		kref_get(&vb->refcount);
+	return vb;
+}
+
 /*
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
  */

@@ -346,6 +366,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
 vb2_memory memory,
 			break;
 		}
 
+		kref_init(&vb->refcount);
 		vb->state = VB2_BUF_STATE_DEQUEUED;
 		vb->vb2_queue = q;

drivers/media/common/videobuf2/videobuf2-core.c +1 -1

 		vb->num_planes = num_planes;

@@ -365,7 +386,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
 vb2_memory memory,
 				dprintk(1, "failed allocating memory for buffer %d\n",
 					buffer);
 				q->bufs[vb->index] = NULL;
-				kfree(vb);
+				__vb2_buffer_put(vb);
 				break;
 			}

drivers/media/common/videobuf2/videobuf2-core.c +1 -1

 			__setup_offsets(vb);

@@ -380,7 +401,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
 vb2_memory memory,
 					buffer, vb);
 				__vb2_buf_mem_free(vb);
 				q->bufs[vb->index] = NULL;
-				kfree(vb);
+				__vb2_buffer_put(vb);
 				break;
 			}

drivers/media/common/videobuf2/videobuf2-core.c +1 -1

 		}

@@ -520,7 +541,7 @@ static int __vb2_queue_free(struct vb2_queue *q,
 unsigned int buffers)
 	/* Free videobuf buffers */
 	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
 	     ++buffer) {
-		kfree(q->bufs[buffer]);
+		__vb2_buffer_put(q->bufs[buffer]);
 		q->bufs[buffer] = NULL;
 	}

include/media/videobuf2-core.h +4 -2

index f6818f732f34..baa4632c7e59 100644
@@ -12,11 +12,12 @@
 #ifndef _MEDIA_VIDEOBUF2_CORE_H
 #define _MEDIA_VIDEOBUF2_CORE_H
 
+#include <linux/bitops.h>
+#include <linux/dma-buf.h>
+#include <linux/kref.h>
 #include <linux/mm_types.h>
 #include <linux/mutex.h>
 #include <linux/poll.h>
-#include <linux/dma-buf.h>
-#include <linux/bitops.h>
 
 #define VB2_MAX_FRAME	(32)
 #define VB2_MAX_PLANES	(8)

@@ -249,6 +250,7 @@ struct vb2_buffer {
 
 	/* private: internal use only
 	 *
+	 * refcount:		refcount for this buffer
 	 * state:		current buffer state; do not change
 	 * queued_entry:	entry on the queued buffers list, which holds
 	 *			all buffers queued from userspace

Hello,

Quoted 8 lines · Ezequiel Garcia

The in-fence implementation involves having a per-buffer fence callback, that triggers on the fence signal. The fence callback is called asynchronously
and needs a valid reference to the associated ideobuf2 buffer.

Allow this by making the vb2_buffer refcounted, so it can be passed to other contexts.

-Is it really required, because when a queued buffer with an in_fence is deallocated, firstly queue is cancelled.

(quoted diff) +0 -2

-And __vb2_dqbuf is called which calls dma_fence_remove_callback.
-So if fence callback has been called -__vb2_dqbuf will wait to

acquire fence lock.

(quoted diff) +0 -2

-So during execution of fence callback, buffers and queue are still valid.
-And if __vb2_dqbuf remove callback first ,then dma_fence_signal will

wait for lock
- so there won't be any fence callback to call for that buffer when dma_fence_signal resumes.

Quoted 17 lines · sathyam panda

Hello,

Quoted 9 lines · Ezequiel Garcia

The in-fence implementation involves having a per-buffer fence callback,
that triggers on the fence signal. The fence callback is called asynchronously
and needs a valid reference to the associated ideobuf2 buffer.

Allow this by making the vb2_buffer refcounted, so it can be passed to other contexts.

-Is it really required, because when a queued buffer with an in_fence is deallocated, firstly queue is cancelled.

(quoted diff) +0 -2

-And __vb2_dqbuf is called which calls dma_fence_remove_callback.
-So if fence callback has been called -__vb2_dqbuf will wait to

Quoted 8 lines

acquire fence lock.
-So during execution of fence callback, buffers and queue are still valid.
-And if __vb2_dqbuf remove callback first ,then dma_fence_signal will wait for lock
- so there won't be any fence callback to call for that buffer when dma_fence_signal resumes.

Hi Sathyam,

Thanks for your review! The refcount is definitely required, as the fence callback only schedules a workqueue, which is completely asynchronous with respect to the rest of the ioctls.

In particular, the workqueue is not synchronized with vb2_core_queue_release.

Also, another subtle detail, dma_fence_remove_callback can fail to remove the callback.

Thanks,
Eze

From: Gustavo Padovan <[email protected]>

Now that we've introduced the V4L2_FMT_FLAG_UNORDERED flag, mark the appropriate formats.

v2: Set unordered flag before calling the driver callback.

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ioctl.c | 74 +++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 17 deletions(-)

drivers/media/v4l2-core/v4l2-ioctl.c +8 -0

index f48c505550e0..2135ac235a96 100644
@@ -1102,6 +1102,27 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops,
 	return ops->vidioc_enum_output(file, fh, p);
 }
 
+static void v4l_fill_unordered_fmtdesc(struct v4l2_fmtdesc *fmt)
+{
+	switch (fmt->pixelformat) {
+	case V4L2_PIX_FMT_MPEG:
+	case V4L2_PIX_FMT_H264:
+	case V4L2_PIX_FMT_H264_NO_SC:
+	case V4L2_PIX_FMT_H264_MVC:
+	case V4L2_PIX_FMT_H263:

drivers/media/v4l2-core/v4l2-ioctl.c +19 -17

+	}
+}
+
 static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 {
 	const unsigned sz = sizeof(fmt->description);

@@ -1310,61 +1331,80 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strlcpy(fmt->description, descr, sz) >= sz);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
-static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-				struct file *file, void *fh, void *arg)
-{
-	struct v4l2_fmtdesc *p = arg;
-	int ret = check_fmt(file, p->type);
 
-	if (ret)
-		return ret;
-	ret = -EINVAL;
+static int __vidioc_enum_fmt(const struct v4l2_ioctl_ops *ops,
+			     struct v4l2_fmtdesc *p,
+			     struct file *file, void *fh)
+{
+	int ret = 0;
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_overlay))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_out))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_meta_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, p);
 		break;
 	}
+	return ret;
+}

drivers/media/v4l2-core/v4l2-ioctl.c +6 -0

+	/*
+	 * Set the unordered flag and call the driver
+	 * again so it has the chance to clear the flag.
+	 */
+	v4l_fill_unordered_fmtdesc(p);
+	ret = __vidioc_enum_fmt(ops, p, file, fh);
 	if (ret == 0)
 		v4l_fill_fmtdesc(p);
 	return ret;

Quoted 34 lines · Ezequiel Garcia

From: Gustavo Padovan <[email protected]>

Now that we've introduced the V4L2_FMT_FLAG_UNORDERED flag, mark the appropriate formats.

v2: Set unordered flag before calling the driver callback.

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ioctl.c | 74 +++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 17 deletions(-)

drivers/media/v4l2-core/v4l2-ioctl.c +13 -0

index f48c505550e0..2135ac235a96 100644
@@ -1102,6 +1102,27 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops,
 	return ops->vidioc_enum_output(file, fh, p);
 }
 
+static void v4l_fill_unordered_fmtdesc(struct v4l2_fmtdesc *fmt)
+{
+	switch (fmt->pixelformat) {
+	case V4L2_PIX_FMT_MPEG:
+	case V4L2_PIX_FMT_H264:
+	case V4L2_PIX_FMT_H264_NO_SC:
+	case V4L2_PIX_FMT_H264_MVC:
+	case V4L2_PIX_FMT_H263:
+	case V4L2_PIX_FMT_MPEG1:
+	case V4L2_PIX_FMT_MPEG2:
+	case V4L2_PIX_FMT_MPEG4:
+	case V4L2_PIX_FMT_XVID:
+	case V4L2_PIX_FMT_VC1_ANNEX_G:

drivers/media/v4l2-core/v4l2-ioctl.c static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, · line 1122

Quoted 5 lines

drivers/media/v4l2-core/v4l2-ioctl.c +5 -0

+	case V4L2_PIX_FMT_VC1_ANNEX_L:
+	case V4L2_PIX_FMT_VP8:
+	case V4L2_PIX_FMT_VP9:
+	case V4L2_PIX_FMT_HEVC:
+		fmt->flags |= V4L2_FMT_FLAG_UNORDERED;

Please add a break here. This prevents potential future errors if new cases are added below this line.

Quoted 85 lines

drivers/media/v4l2-core/v4l2-ioctl.c +23 -17

+	}
+}
+
 static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 {
 	const unsigned sz = sizeof(fmt->description);

@@ -1310,61 +1331,80 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strlcpy(fmt->description, descr, sz) >= sz);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
-static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-				struct file *file, void *fh, void *arg)
-{
-	struct v4l2_fmtdesc *p = arg;
-	int ret = check_fmt(file, p->type);
 
-	if (ret)
-		return ret;
-	ret = -EINVAL;
+static int __vidioc_enum_fmt(const struct v4l2_ioctl_ops *ops,
+			     struct v4l2_fmtdesc *p,
+			     struct file *file, void *fh)
+{
+	int ret = 0;
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_overlay))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_out))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_meta_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, p);
 		break;
 	}
+	return ret;
+}
+
+static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1397

Quoted 6 lines

drivers/media/v4l2-core/v4l2-ioctl.c +6 -0

+	struct v4l2_fmtdesc *p = arg;
+	int ret = check_fmt(file, p->type);
+
+	if (ret)
+		return ret;
+	ret = -EINVAL;

Why set ret when it is overwritten below?

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1401

Quoted 4 lines

drivers/media/v4l2-core/v4l2-ioctl.c +4 -0

+
+	ret = __vidioc_enum_fmt(ops, p, file, fh);
+	if (ret)
+		return ret;

Huh? Why call the driver twice? As far as I can see you can just drop these three lines above.

Quoted 35 lines · Hans Verkuil

Quoted 34 lines · Ezequiel Garcia

From: Gustavo Padovan <[email protected]>

Now that we've introduced the V4L2_FMT_FLAG_UNORDERED flag, mark the appropriate formats.

v2: Set unordered flag before calling the driver callback.

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/v4l2-core/v4l2-ioctl.c | 74 +++++++++++++++++++++++++++---------
 1 file changed, 57 insertions(+), 17 deletions(-)

drivers/media/v4l2-core/v4l2-ioctl.c +13 -0

index f48c505550e0..2135ac235a96 100644
@@ -1102,6 +1102,27 @@ static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops,
 	return ops->vidioc_enum_output(file, fh, p);
 }
 
+static void v4l_fill_unordered_fmtdesc(struct v4l2_fmtdesc *fmt)
+{
+	switch (fmt->pixelformat) {
+	case V4L2_PIX_FMT_MPEG:
+	case V4L2_PIX_FMT_H264:
+	case V4L2_PIX_FMT_H264_NO_SC:
+	case V4L2_PIX_FMT_H264_MVC:
+	case V4L2_PIX_FMT_H263:
+	case V4L2_PIX_FMT_MPEG1:
+	case V4L2_PIX_FMT_MPEG2:
+	case V4L2_PIX_FMT_MPEG4:
+	case V4L2_PIX_FMT_XVID:
+	case V4L2_PIX_FMT_VC1_ANNEX_G:

Quoted 9 lines

drivers/media/v4l2-core/v4l2-ioctl.c static int v4l_enumoutput(const struct v4l2_ioctl_ops *ops, · line 1122

Quoted 5 lines

drivers/media/v4l2-core/v4l2-ioctl.c +5 -0

+	case V4L2_PIX_FMT_VC1_ANNEX_L:
+	case V4L2_PIX_FMT_VP8:
+	case V4L2_PIX_FMT_VP9:
+	case V4L2_PIX_FMT_HEVC:
+		fmt->flags |= V4L2_FMT_FLAG_UNORDERED;

Please add a break here. This prevents potential future errors if new cases are added below this line.

Sure.

Quoted 85 lines

Quoted 85 lines

drivers/media/v4l2-core/v4l2-ioctl.c +23 -17

+	}
+}
+
 static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 {
 	const unsigned sz = sizeof(fmt->description);

@@ -1310,61 +1331,80 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 
 	if (descr)
 		WARN_ON(strlcpy(fmt->description, descr, sz) >= sz);
-	fmt->flags = flags;
+	fmt->flags |= flags;
 }
 
-static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
-				struct file *file, void *fh, void *arg)
-{
-	struct v4l2_fmtdesc *p = arg;
-	int ret = check_fmt(file, p->type);
 
-	if (ret)
-		return ret;
-	ret = -EINVAL;
+static int __vidioc_enum_fmt(const struct v4l2_ioctl_ops *ops,
+			     struct v4l2_fmtdesc *p,
+			     struct file *file, void *fh)
+{
+	int ret = 0;
 
 	switch (p->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_cap_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_cap_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_overlay))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_overlay(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (unlikely(!ops->vidioc_enum_fmt_vid_out_mplane))
 			break;
-		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_vid_out_mplane(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_cap(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_SDR_OUTPUT:
 		if (unlikely(!ops->vidioc_enum_fmt_sdr_out))
 			break;
-		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_sdr_out(file, fh, p);
 		break;
 	case V4L2_BUF_TYPE_META_CAPTURE:
 		if (unlikely(!ops->vidioc_enum_fmt_meta_cap))
 			break;
-		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, arg);
+		ret = ops->vidioc_enum_fmt_meta_cap(file, fh, p);
 		break;
 	}
+	return ret;
+}
+
+static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh, void *arg)
+{

Quoted 9 lines

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1397

Quoted 6 lines

drivers/media/v4l2-core/v4l2-ioctl.c +6 -0

+	struct v4l2_fmtdesc *p = arg;
+	int ret = check_fmt(file, p->type);
+
+	if (ret)
+		return ret;
+	ret = -EINVAL;

Why set ret when it is overwritten below?

Oops, seems to be some leftover code.

Quoted 9 lines

drivers/media/v4l2-core/v4l2-ioctl.c static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt) · line 1401

Quoted 4 lines

drivers/media/v4l2-core/v4l2-ioctl.c +4 -0

+
+	ret = __vidioc_enum_fmt(ops, p, file, fh);
+	if (ret)
+		return ret;

Huh? Why call the driver twice? As far as I can see you can just drop these three lines above.

Well, because I thought this was the outcome of v9 [1]. Let me quote you:

""
I realized that this is a problem since this function is called *after* the driver. So the driver has no chance to clear this flag if it knows that the queue is always ordered.
""

So, we first call the driver to get struct v4l2_fmtdesc pixelformat field filled by the driver, then we call v4l_fill_unordered_fmtdesc() to set FLAG_UNORDERED if needed, and finally we call the driver again so it can clear the FLAG_UNORDERED.

Does that make sense?

[1] https://lkml.org/lkml/2018/5/7/349

From: Gustavo Padovan <[email protected]>

The cobalt driver may reorder the capture buffers so we need to report it as such.

v3: set formats as unordered
v2: use vb2_ops_set_unordered() helper

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/pci/cobalt/cobalt-v4l2.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/media/pci/cobalt/cobalt-v4l2.c +2 -0

index e2a4c705d353..ccca1a96df90 100644
@@ -430,6 +430,7 @@ static const struct vb2_ops cobalt_qops = {
 	.stop_streaming = cobalt_stop_streaming,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
+	.is_unordered = vb2_ops_is_unordered,
 };
 
 /* V4L2 ioctls */

@@ -695,14 +696,17 @@ static int cobalt_enum_fmt_vid_cap(struct file *file, void *priv_fh,
 	case 0:
 		strlcpy(f->description, "YUV 4:2:2", sizeof(f->description));
 		f->pixelformat = V4L2_PIX_FMT_YUYV;
+		f->flags |= V4L2_FMT_FLAG_UNORDERED;

drivers/media/pci/cobalt/cobalt-v4l2.c +0 -0

 		break;
 	default:
 		return -EINVAL;

From: Gustavo Padovan <[email protected]>

Turn the reserved2 field into fence_fd that we will use to send an in-fence to the kernel or return an out-fence from the kernel to userspace.

Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used when sending an in-fence to the kernel to be waited on, and V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence.

v8: return -1 if new flags are set.
v7: minor fixes on the Documentation (Hans Verkuil)

v6: big improvement on doc (Hans Verkuil)

v5: - keep using reserved2 field for cpia2
- set fence_fd to 0 for now, for compat with userspace(Mauro)

v4: make it a union with reserved2 and fence_fd (Hans Verkuil)

v3: make the out_fence refer to the current buffer (Hans Verkuil)

v2: add documentation

Signed-off-by Gustavo Padovan <[email protected]>

---
 Documentation/media/uapi/v4l/buffer.rst         | 48 ++++++++++++++++++++++---
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  6 +++-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c   |  4 +--
 include/uapi/linux/videodev2.h                  |  8 ++++-
 4 files changed, 58 insertions(+), 8 deletions(-)

Documentation/media/uapi/v4l/buffer.rst +44 -4

index e2c85ddc990b..971b7453040c 100644
@@ -300,11 +300,23 @@ struct v4l2_buffer
 	multi-planar API the application sets this to the number of
 	elements in the ``planes`` array. The driver will fill in the
 	actual number of valid elements in that array.
-    * - __u32
-      - ``reserved2``
+    * - __s32
+      - ``fence_fd``
       -
-      - A place holder for future extensions. Drivers and applications
-	must set this to 0.
+      - Used to communicate a fence file descriptor from userspace to kernel
+	and vice-versa. On :ref:`VIDIOC_QBUF <VIDIOC_QBUF>` when sending
+	an in-fence for V4L2 to wait on, the ``V4L2_BUF_FLAG_IN_FENCE`` flag must
+	be used and this field set to the fence file descriptor of the in-fence.
+	If the in-fence is not valid ` VIDIOC_QBUF`` returns an error.
+
+        To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE``
+	must be set, the kernel will return the out-fence file descriptor in
+	this field. If it fails to create the out-fence ``VIDIOC_QBUF` returns
+        an error.
+
+	For all other ioctls V4L2 sets this field to -1 if
+	``V4L2_BUF_FLAG_IN_FENCE`` and/or ``V4L2_BUF_FLAG_OUT_FENCE`` are set,
+	otherwise this field is set to 0 for backward compatibility.
     * - __u32
       - ``reserved``
       -

@@ -648,6 +660,34 @@ Buffer Flags
       - Start Of Exposure. The buffer timestamp has been taken when the
 	exposure of the frame has begun. This is only valid for the
 	``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type.
+    * .. _`V4L2-BUF-FLAG-IN-FENCE`:
+
+      - ``V4L2_BUF_FLAG_IN_FENCE``
+      - 0x00200000
+      - Ask V4L2 to wait on the fence passed in the ``fence_fd`` field. The
+	buffer won't be queued to the driver until the fence signals. The order
+	in which buffers are queued is guaranteed to be preserved, so any
+	buffers queued after this buffer will also be blocked until this fence
+	signals. This flag must be set before calling ``VIDIOC_QBUF``. For
+	other ioctls the driver just reports the value of the flag.
+
+        If the fence signals the flag is cleared and not reported anymore.
+	If the fence is not valid ``VIDIOC_QBUF`` returns an error.
+    * .. _`V4L2-BUF-FLAG-OUT-FENCE`:
+
+      - ``V4L2_BUF_FLAG_OUT_FENCE``
+      - 0x00400000
+      - Request for a fence to be attached to the buffer. The driver will fill
+	in the out-fence fd in the ``fence_fd`` field when :ref:`VIDIOC_QBUF
+	<VIDIOC_QBUF>` returns. This flag must be set before calling
+	``VIDIOC_QBUF``. This flag is only an input, and is not set by the kernel.
+
+        If the creation of the out-fence fails ``VIDIOC_QBUF`` returns an
+	error.
+
+        Note that it is valid to set both ``V4L2_BUF_FLAG_IN_FENCE`` and
+        `V4L2_BUF_FLAG_OUT_FENCE`` flags. In such case, the ``fence_fd``
+        field is used to both set the in-fence and return the out-fence.
 
 
 

drivers/media/common/videobuf2/videobuf2-v4l2.c +0 -0

index 64503615d00b..8312f61adfa6 100644

drivers/media/common/videobuf2/videobuf2-v4l2.c +1 -0

+
 	if (q->is_multiplanar) {
 		/*
 		 * Fill in plane-related data if userspace provided an array

drivers/media/v4l2-core/v4l2-compat-ioctl32.c +2 -2

index 4312935f1dfc..93c752459aec 100644
@@ -388,7 +388,7 @@ struct v4l2_buffer32 {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	__s32			fence_fd;
 	__u32			reserved;
 };
 

@@ -606,7 +606,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp,
 	    assign_in_user(&up->timestamp.tv_usec, &kp->timestamp.tv_usec) ||
 	    copy_in_user(&up->timecode, &kp->timecode, sizeof(kp->timecode)) ||
 	    assign_in_user(&up->sequence, &kp->sequence) ||
-	    assign_in_user(&up->reserved2, &kp->reserved2) ||
+	    assign_in_user(&up->fence_fd, &kp->fence_fd) ||
 	    assign_in_user(&up->reserved, &kp->reserved) ||
 	    get_user(length, &kp->length) ||
 	    put_user(length, &up->length))

include/uapi/linux/videodev2.h +7 -1

index a8842a5ca636..1f18dc68ecab 100644
@@ -934,7 +934,10 @@ struct v4l2_buffer {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	union {
+		__s32		fence_fd;
+		__u32		reserved2;
+	};
 	__u32			reserved;
 };
 

@@ -971,6 +974,9 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE		0x00010000
 /* mem2mem encoder/decoder */
 #define V4L2_BUF_FLAG_LAST			0x00100000
+/* Explicit synchronization */
+#define V4L2_BUF_FLAG_IN_FENCE			0x00200000
+#define V4L2_BUF_FLAG_OUT_FENCE			0x00400000
 
 /**
  * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor

Quoted 110 lines · Ezequiel Garcia

From: Gustavo Padovan <[email protected]>

Turn the reserved2 field into fence_fd that we will use to send an in-fence to the kernel or return an out-fence from the kernel to userspace.

Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used when sending an in-fence to the kernel to be waited on, and V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence.

v8: return -1 if new flags are set.
v7: minor fixes on the Documentation (Hans Verkuil)

v6: big improvement on doc (Hans Verkuil)

v5: - keep using reserved2 field for cpia2
- set fence_fd to 0 for now, for compat with userspace(Mauro)

v4: make it a union with reserved2 and fence_fd (Hans Verkuil)

v3: make the out_fence refer to the current buffer (Hans Verkuil)

v2: add documentation

Signed-off-by Gustavo Padovan <[email protected]>

---
 Documentation/media/uapi/v4l/buffer.rst         | 48 ++++++++++++++++++++++---
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  6 +++-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c   |  4 +--
 include/uapi/linux/videodev2.h                  |  8 ++++-
 4 files changed, 58 insertions(+), 8 deletions(-)

Documentation/media/uapi/v4l/buffer.rst +44 -4

index e2c85ddc990b..971b7453040c 100644
@@ -300,11 +300,23 @@ struct v4l2_buffer
 	multi-planar API the application sets this to the number of
 	elements in the ``planes`` array. The driver will fill in the
 	actual number of valid elements in that array.
-    * - __u32
-      - ``reserved2``
+    * - __s32
+      - ``fence_fd``
       -
-      - A place holder for future extensions. Drivers and applications
-	must set this to 0.
+      - Used to communicate a fence file descriptor from userspace to kernel
+	and vice-versa. On :ref:`VIDIOC_QBUF <VIDIOC_QBUF>` when sending
+	an in-fence for V4L2 to wait on, the ``V4L2_BUF_FLAG_IN_FENCE`` flag must
+	be used and this field set to the fence file descriptor of the in-fence.
+	If the in-fence is not valid ` VIDIOC_QBUF`` returns an error.
+
+        To get an out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE``
+	must be set, the kernel will return the out-fence file descriptor in
+	this field. If it fails to create the out-fence ``VIDIOC_QBUF` returns
+        an error.
+
+	For all other ioctls V4L2 sets this field to -1 if
+	``V4L2_BUF_FLAG_IN_FENCE`` and/or ``V4L2_BUF_FLAG_OUT_FENCE`` are set,
+	otherwise this field is set to 0 for backward compatibility.
     * - __u32
       - ``reserved``
       -

@@ -648,6 +660,34 @@ Buffer Flags
       - Start Of Exposure. The buffer timestamp has been taken when the
 	exposure of the frame has begun. This is only valid for the
 	``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type.
+    * .. _`V4L2-BUF-FLAG-IN-FENCE`:
+
+      - ``V4L2_BUF_FLAG_IN_FENCE``
+      - 0x00200000
+      - Ask V4L2 to wait on the fence passed in the ``fence_fd`` field. The
+	buffer won't be queued to the driver until the fence signals. The order
+	in which buffers are queued is guaranteed to be preserved, so any
+	buffers queued after this buffer will also be blocked until this fence
+	signals. This flag must be set before calling ``VIDIOC_QBUF``. For
+	other ioctls the driver just reports the value of the flag.
+
+        If the fence signals the flag is cleared and not reported anymore.
+	If the fence is not valid ``VIDIOC_QBUF`` returns an error.
+    * .. _`V4L2-BUF-FLAG-OUT-FENCE`:
+
+      - ``V4L2_BUF_FLAG_OUT_FENCE``
+      - 0x00400000
+      - Request for a fence to be attached to the buffer. The driver will fill
+	in the out-fence fd in the ``fence_fd`` field when :ref:`VIDIOC_QBUF
+	<VIDIOC_QBUF>` returns. This flag must be set before calling
+	``VIDIOC_QBUF``. This flag is only an input, and is not set by the kernel.
+
+        If the creation of the out-fence fails ``VIDIOC_QBUF`` returns an
+	error.
+
+        Note that it is valid to set both ``V4L2_BUF_FLAG_IN_FENCE`` and
+        `V4L2_BUF_FLAG_OUT_FENCE`` flags. In such case, the ``fence_fd``
+        field is used to both set the in-fence and return the out-fence.
 
 
 

drivers/media/common/videobuf2/videobuf2-v4l2.c +0 -1

index 64503615d00b..8312f61adfa6 100644
@@ -203,9 +203,13 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
 	b->timestamp = ns_to_timeval(vb->timestamp);
 	b->timecode = vbuf->timecode;
 	b->sequence = vbuf->sequence;
-	b->reserved2 = 0;
 	b->reserved = 0;
 

drivers/media/common/videobuf2/videobuf2-v4l2.c static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) · line 211

Quoted 4 lines

drivers/media/common/videobuf2/videobuf2-v4l2.c +4 -0

+	if (b->flags & (V4L2_BUF_FLAG_IN_FENCE | V4L2_BUF_FLAG_OUT_FENCE))
+		b->fence_fd = -1;
+	else
+		b->fence_fd = 0;

I don't see why we can't just always set fence_fd to -1.

There is no need for backwards compatibility that I can see.

From: Gustavo Padovan <[email protected]>

Receive in-fence from userspace and add support for waiting on them before queueing the buffer to the driver. Buffers can't be queued to the driver before its fences signal. And a buffer can't be queued to the driver out of the order they were queued from userspace. That means that even if its fence signals it must wait for all other buffers, ahead of it in the queue, to signal first.

If the fence for some buffer fails we do not queue it to the driver, instead we mark it as error and wait until the previous buffer is done to notify userspace of the error. We wait here to deliver the buffers back to userspace in order.

v13: - cleanup implementation.
- remove wrong Kconfig changes.
- print noisy warning on unexpected enqueue conditioin
- schedule a vb2_start_streaming work from the fence callback

v12: fixed dvb_vb2.c usage of vb2_core_qbuf.

v11: - minor doc/comments fixes (Hans Verkuil)
- reviewed the in-fence path at __fill_v4l2_buffer()

v10: - rename fence to in_fence in many places
- handle fences signalling with error better (Hans Verkuil)

v9: - improve comments and docs (Hans Verkuil)
- fix unlocking of vb->fence_cb_lock on vb2_core_qbuf (Hans Verkuil)
- move in-fences code that was in the out-fences patch here (Alex)

v8: - improve comments about fences with errors

v7: - get rid of the fence array stuff for ordering and just use
get_num_buffers_ready() (Hans)
- fix issue of queuing the buffer twice (Hans)
- avoid the dma_fence_wait() in core_qbuf() (Alex)
- merge preparation commit in

v6: - With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4: - Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
- Remove if (vb->state != VB2_BUF_STATE_QUEUED) from vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2: - fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/common/videobuf2/Kconfig          |   1 +
 drivers/media/common/videobuf2/videobuf2-core.c | 224 ++++++++++++++++++++----
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  37 +++-
 drivers/media/dvb-core/dvb_vb2.c                |   2 +-
 include/media/videobuf2-core.h                  |  19 +-
 5 files changed, 242 insertions(+), 41 deletions(-)

drivers/media/common/videobuf2/Kconfig +1 -0

index 17c32ea58395..27ad9e8a268b 100644
@@ -1,6 +1,7 @@
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
 	select DMA_SHARED_BUFFER
+	select SYNC_FILE
 	tristate
 
 config VIDEOBUF2_V4L2

drivers/media/common/videobuf2/videobuf2-core.c +71 -12

index a9a0a9d1decb..86b5ebe25263 100644
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 
 #include <media/videobuf2-core.h>
+#include <media/v4l2-ioctl.h>
 #include <media/v4l2-mc.h>
 
 #include <trace/events/vb2.h>

@@ -189,6 +190,7 @@ module_param(debug, int, 0644);
 
 static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
+static void __qbuf_work(struct work_struct *work);
 
 static void __vb2_buffer_free(struct kref *kref)
 {

@@ -373,6 +375,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
 		vb->index = q->num_buffers + buffer;
 		vb->type = q->type;
 		vb->memory = memory;
+		INIT_WORK(&vb->qbuf_work, __qbuf_work);
 		for (plane = 0; plane < num_planes; ++plane) {
 			vb->planes[plane].length = plane_sizes[plane];
 			vb->planes[plane].min_length = plane_sizes[plane];

@@ -932,21 +935,12 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int plane_no)
 }
 EXPORT_SYMBOL_GPL(vb2_plane_cookie);
 
-void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+static void vb2_process_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
 	unsigned int plane;
 
-	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
-		return;
-
-	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
-		    state != VB2_BUF_STATE_ERROR &&
-		    state != VB2_BUF_STATE_QUEUED &&
-		    state != VB2_BUF_STATE_REQUEUEING))
-		state = VB2_BUF_STATE_ERROR;
-
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
 	 * Although this is not a callback, it still does have to balance

@@ -962,6 +956,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 
 	spin_lock_irqsave(&q->done_lock, flags);
+	if (vb->state == VB2_BUF_STATE_ACTIVE)
+		atomic_dec(&q->owned_by_drv_count);
+
 	if (state == VB2_BUF_STATE_QUEUED ||
 	    state == VB2_BUF_STATE_REQUEUEING) {
 		vb->state = VB2_BUF_STATE_QUEUED;

@@ -970,7 +967,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		list_add_tail(&vb->done_entry, &q->done_list);
 		vb->state = state;
 	}
-	atomic_dec(&q->owned_by_drv_count);
+
 	spin_unlock_irqrestore(&q->done_lock, flags);
 
 	trace_vb2_buf_done(q, vb);

@@ -987,6 +984,47 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		wake_up(&q->done_wq);
 		break;
 	}
+
+}
+
+void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+{
+	struct vb2_buffer *next;
+	struct vb2_queue *q;
+
+	dprintk(4, "called done on buffer %d state: %d -> %d\n",
+			vb->index, vb->state, state);
+
+	if (vb->state == VB2_BUF_STATE_ERROR)
+		return;
+	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
+		return;
+
+	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+		    state != VB2_BUF_STATE_ERROR &&
+		    state != VB2_BUF_STATE_QUEUED &&
+		    state != VB2_BUF_STATE_REQUEUEING))
+		state = VB2_BUF_STATE_ERROR;
+
+	vb2_process_buffer_done(vb, state);
+
+	/*
+	 * Check if there is any buffer with an in-fence error in the next
+	 * position of the queue. Buffers whose in-fence signaled with error
+	 * are not queued to the driver and kept on the queue until the buffer
+	 * before them is done.
+	 * So here we process any existing buffers with in-fence errors and
+	 * wake up userspace.
+	 */
+	q = vb->vb2_queue;
+	next = list_next_entry(vb, queued_entry);
+	list_for_each_entry_from(next, &q->queued_list, queued_entry) {
+		if (!next->in_fence || !next->in_fence->error)
+			break;
+		if (next->in_fence && next->in_fence->error &&
+		    vb->state != VB2_BUF_STATE_ERROR)
+			vb2_process_buffer_done(next, VB2_BUF_STATE_ERROR);
+	}
 }
 EXPORT_SYMBOL_GPL(vb2_buffer_done);
 

@@ -1271,6 +1309,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 
+	if (WARN_ON(vb->state == VB2_BUF_STATE_ACTIVE))
+		return;
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->owned_by_drv_count);
 

@@ -1322,6 +1362,25 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
 	return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)
+{
+	struct vb2_buffer *vb;
+	int ready_count = 0;
+
+	/*
+	 * Count num of buffers ready in front of the queued_list.
+	 * We want to stop counting when we find a buffer with an
+	 * unsignaled fence.
+	 */
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
+		ready_count++;
+	}
+
+	return ready_count;
+}
+
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 {
 	struct vb2_buffer *vb;

@@ -1338,7 +1397,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 	if (ret)
 		return ret;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	call_void_bufop(q, fill_user_buffer, vb, pb);
 
 	dprintk(2, "prepare of buffer %d succeeded\n", vb->index);

@@ -1364,11 +1423,16 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	int ret;

drivers/media/common/videobuf2/videobuf2-core.c +14 -1

+			continue;
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
 		__enqueue_in_driver(vb);
+	}
 
 	/* Tell the driver to start streaming */
 	q->start_streaming_called = 1;

@@ -1410,7 +1474,41 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	return ret;
 }
 
-int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
+static void __qbuf_work(struct work_struct *work)
+{
+	struct vb2_buffer *vb;
+	struct vb2_queue *q;
+
+	vb = container_of(work, struct vb2_buffer, qbuf_work);
+	q = vb->vb2_queue;
+
+	if (q->lock)
+		mutex_lock(q->lock);

drivers/media/common/videobuf2/videobuf2-core.c +41 -9

+
+	if (q->lock)
+		mutex_unlock(q->lock);
+	__vb2_buffer_put(vb);
+}
+
+static void vb2_qbuf_fence_cb(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+	struct vb2_buffer *vb = container_of(cb, struct vb2_buffer, fence_cb);
+
+	schedule_work(&vb->qbuf_work);
+}
+
+int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
+		  struct dma_fence *in_fence)
 {
 	struct vb2_buffer *vb;
 	int ret;

@@ -1421,16 +1519,18 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	case VB2_BUF_STATE_DEQUEUED:
 		ret = __buf_prepare(vb, pb);
 		if (ret)
-			return ret;
+			goto err_release;
 		break;
 	case VB2_BUF_STATE_PREPARED:
 		break;
 	case VB2_BUF_STATE_PREPARING:
 		dprintk(1, "buffer still being prepared\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_release;
 	default:
 		dprintk(1, "invalid buffer state %d\n", vb->state);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_release;
 	}
 
 	/*

@@ -1448,15 +1548,44 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	trace_vb2_qbuf(q, vb);
 
 	/*
-	 * If already streaming, give the buffer to driver for processing.
-	 * If not, the buffer will be given to driver on next streamon.
+	 * If the fence didn't signal yet, we setup a callback to queue
+	 * the buffer once the fence signals. If the fence already signaled,
+	 * then we lose the reference and queue the buffer.
 	 */
-	if (q->start_streaming_called)
-		__enqueue_in_driver(vb);
+	if (in_fence) {
+		WARN_ON(vb->in_fence);
+		vb->in_fence = in_fence;
+		__vb2_buffer_get(vb);
+		ret = dma_fence_add_callback(in_fence, &vb->fence_cb,
+					     vb2_qbuf_fence_cb);
+		/* is the fence signaled? */
+		if (ret == -ENOENT) {
+			if (in_fence->error)
+				/* error-signaled fence, reject this buffer */
+				goto err_release;
+			dma_fence_put(in_fence);
+			vb->in_fence = NULL;
+			__vb2_buffer_put(vb);
+		} else if (ret) {
+			goto err_release;
+		}
+	}
 
-	/* Fill buffer information for the userspace */
-	if (pb)

drivers/media/common/videobuf2/videobuf2-core.c +33 -4

+				continue;
+			if (b->in_fence && dma_fence_get_status(b->in_fence) == 0)
+				break;
+			__enqueue_in_driver(b);
+		}
+	}
 
 	/*
 	 * If streamon has been called, and we haven't yet called

@@ -1465,14 +1594,32 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	 * then we can finally call start_streaming().
 	 */
 	if (q->streaming && !q->start_streaming_called &&
-	    q->queued_count >= q->min_buffers_needed) {
+	    __get_num_ready_buffers(q) >= q->min_buffers_needed) {
 		ret = vb2_start_streaming(q);
 		if (ret)
-			return ret;
+			goto err_release;
 	}
 
+	/* Fill buffer information for userspace */
+	if (pb)
+		call_void_bufop(q, fill_user_buffer, vb, pb);
+
 	dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
 	return 0;
+
+err_release:
+	/* Fill buffer information for userspace */
+	if (pb)
+		call_void_bufop(q, fill_user_buffer, vb, pb);
+
+	if (in_fence) {
+		dma_fence_put(in_fence);
+		vb->in_fence = NULL;
+		__vb2_buffer_put(vb);
+	}
+
+	return ret;
+
 }
 EXPORT_SYMBOL_GPL(vb2_core_qbuf);
 

@@ -1615,7 +1762,12 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 		return;
 
 	vb->state = VB2_BUF_STATE_DEQUEUED;
-
+	if (vb->in_fence) {
+		if (dma_fence_remove_callback(vb->in_fence, &vb->fence_cb))
+			__vb2_buffer_put(vb);
+		dma_fence_put(vb->in_fence);
+		vb->in_fence = NULL;
+	}
 	/* unmap DMABUF buffer */
 	if (q->memory == VB2_MEMORY_DMABUF)
 		for (i = 0; i < vb->num_planes; ++i) {

@@ -1653,7 +1805,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	if (pindex)
 		*pindex = vb->index;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	if (pb)
 		call_void_bufop(q, fill_user_buffer, vb, pb);
 

drivers/media/common/videobuf2/videobuf2-core.c +4 -4

 		/* Must be zero now */

@@ -1783,7 +1935,7 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
 	 * Tell driver to start streaming provided sufficient buffers
 	 * are available.
 	 */
-	if (q->queued_count >= q->min_buffers_needed) {
+	if (__get_num_ready_buffers(q) >= q->min_buffers_needed) {
 		ret = v4l_vb2q_enable_media_source(q);
 		if (ret)
 			return ret;

@@ -2305,7 +2457,7 @@ 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, i, NULL);
+			ret = vb2_core_qbuf(q, i, NULL, NULL);
 			if (ret)
 				goto err_reqbufs;
 			fileio->bufs[i].queued = 1;

@@ -2484,7 +2636,7 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
 
 		if (copy_timestamp)
 			b->timestamp = ktime_get_ns();
-		ret = vb2_core_qbuf(q, index, NULL);
+		ret = vb2_core_qbuf(q, index, NULL, NULL);
 		dprintk(5, "vb2_dbuf result: %d\n", ret);
 		if (ret)
 			return ret;

@@ -2587,7 +2739,7 @@ static int vb2_thread(void *data)
 		if (copy_timestamp)
 			vb->timestamp = ktime_get_ns();
 		if (!threadio->stop)
-			ret = vb2_core_qbuf(q, vb->index, NULL);
+			ret = vb2_core_qbuf(q, vb->index, NULL, NULL);
 		call_void_qop(q, wait_prepare, q);
 		if (ret || threadio->stop)
 			break;

drivers/media/common/videobuf2/videobuf2-v4l2.c +1 -0

index 8312f61adfa6..ac14cc8ab1c5 100644
@@ -23,6 +23,7 @@
 #include <linux/sched.h>
 #include <linux/freezer.h>
 #include <linux/kthread.h>
+#include <linux/sync_file.h>
 
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>

drivers/media/common/videobuf2/videobuf2-v4l2.c +31 -1

+	}
+
+	if (b->fence_fd < 0 && (b->flags & V4L2_BUF_FLAG_IN_FENCE)) {
+		dprintk(1, "%s: IN_FENCE flag set but no fence_fd\n", opname);
+		return -EINVAL;
+	}
+
+	if ((b->flags & V4L2_BUF_FLAG_IN_FENCE) && !q->lock) {
+		pr_warn("%s: IN_FENCE flag set, but no queue lock\n", opname);
+		b->flags &= ~V4L2_BUF_FLAG_IN_FENCE;
+	}
+
 	return __verify_planes_array(q->bufs[b->index], b);
 }
 

@@ -210,6 +227,11 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
 	else
 		b->fence_fd = 0;
 
+	if (vb->in_fence)
+		b->flags |= V4L2_BUF_FLAG_IN_FENCE;
+	else
+		b->flags &= ~V4L2_BUF_FLAG_IN_FENCE;
+
 	if (q->is_multiplanar) {
 		/*
 		 * Fill in plane-related data if userspace provided an array

@@ -566,6 +588,7 @@ EXPORT_SYMBOL_GPL(vb2_create_bufs);
 
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
+	struct dma_fence *in_fence = NULL;
 	int ret;
 
 	if (vb2_fileio_is_active(q)) {

@@ -574,7 +597,19 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 	}
 
 	ret = vb2_queue_or_prepare_buf(q, b, "qbuf");
-	return ret ? ret : vb2_core_qbuf(q, b->index, b);
+	if (ret)
+		return ret;
+
+	if (b->flags & V4L2_BUF_FLAG_IN_FENCE) {
+		in_fence = sync_file_get_fence(b->fence_fd);
+		if (!in_fence) {
+			dprintk(1, "failed to get in-fence from fd %d\n",
+				b->fence_fd);
+			return -EINVAL;
+		}
+	}
+
+	return vb2_core_qbuf(q, b->index, b, in_fence);
 }
 EXPORT_SYMBOL_GPL(vb2_qbuf);
 

drivers/media/dvb-core/dvb_vb2.c +1 -1

index b811adf88afa..7da53f10db1a 100644
@@ -385,7 +385,7 @@ int dvb_vb2_qbuf(struct dvb_vb2_ctx *ctx, struct dmx_buffer *b)
 {
 	int ret;
 
-	ret = vb2_core_qbuf(&ctx->vb_q, b->index, b);
+	ret = vb2_core_qbuf(&ctx->vb_q, b->index, b, NULL);
 	if (ret) {
 		dprintk(1, "[%s] index=%d errno=%d\n", ctx->name,
 			b->index, ret);

include/media/videobuf2-core.h +14 -2

index 71538ae2c255..ddd555f59dbf 100644
@@ -14,6 +14,7 @@
 
 #include <linux/bitops.h>
 #include <linux/dma-buf.h>
+#include <linux/dma-fence.h>
 #include <linux/kref.h>
 #include <linux/mm_types.h>
 #include <linux/mutex.h>

@@ -257,6 +258,10 @@ struct vb2_buffer {
 	 * done_entry:		entry on the list that stores all buffers ready
 	 *			to be dequeued to userspace
 	 * vb2_plane:		per-plane information; do not change
+	 * in_fence:		fence received from vb2 client to wait on before
+	 *			using the buffer (queueing to the driver)
+	 * fence_cb:		fence callback information
+	 * qbuf_work:		work for deferred qbuf operation
 	 */
 	struct kref		refcount;
 	enum vb2_buffer_state	state;

@@ -264,6 +269,11 @@ struct vb2_buffer {
 	struct vb2_plane	planes[VB2_MAX_PLANES];
 	struct list_head	queued_entry;
 	struct list_head	done_entry;
+
+	struct dma_fence	*in_fence;
+	struct dma_fence_cb	fence_cb;
+	struct work_struct	qbuf_work;
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
 	 * Counters for how often these buffer-related ops are

@@ -488,8 +498,9 @@ struct vb2_buf_ops {
  * @lock:	pointer to a mutex that protects the &struct vb2_queue. The
  *		driver can set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. If the driver wants to handle locking
- *		itself, then this should be set to NULL. This lock is not used
- *		by the videobuf2 core API.
+ *		itself, then this should be set to NULL. This lock is required
+ *		by the videobuf2 core only on the fence-deferred start streaming
+ *		path.
  * @owner:	The filehandle that 'owns' the buffers, i.e. the filehandle
  *		that called reqbufs, create_buffers or started fileio.
  *		This field is not used by the videobuf2 core API, but it allows

@@ -789,6 +800,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb);
  * @index:	id number of the buffer
  * @pb:		buffer structure passed from userspace to
  *		v4l2_ioctl_ops->vidioc_qbuf handler in driver
+ * @in_fence:	in-fence to wait on before queueing the buffer
  *
  * Videobuf2 core helper to implement VIDIOC_QBUF() operation. It is called
  * internally by VB2 by an API-specific handler, like ``videobuf2-v4l2.h``.

Quoted 261 lines · Ezequiel Garcia

From: Gustavo Padovan <[email protected]>

Receive in-fence from userspace and add support for waiting on them before queueing the buffer to the driver. Buffers can't be queued to the driver before its fences signal. And a buffer can't be queued to the driver out of the order they were queued from userspace. That means that even if its fence signals it must wait for all other buffers, ahead of it in the queue, to signal first.

If the fence for some buffer fails we do not queue it to the driver, instead we mark it as error and wait until the previous buffer is done to notify userspace of the error. We wait here to deliver the buffers back to userspace in order.

v13: - cleanup implementation.
- remove wrong Kconfig changes.
- print noisy warning on unexpected enqueue conditioin
- schedule a vb2_start_streaming work from the fence callback

v12: fixed dvb_vb2.c usage of vb2_core_qbuf.

v11: - minor doc/comments fixes (Hans Verkuil)
- reviewed the in-fence path at __fill_v4l2_buffer()

v10: - rename fence to in_fence in many places
- handle fences signalling with error better (Hans Verkuil)

v9: - improve comments and docs (Hans Verkuil)
- fix unlocking of vb->fence_cb_lock on vb2_core_qbuf (Hans Verkuil)
- move in-fences code that was in the out-fences patch here (Alex)

v8: - improve comments about fences with errors

v7: - get rid of the fence array stuff for ordering and just use
get_num_buffers_ready() (Hans)
- fix issue of queuing the buffer twice (Hans)
- avoid the dma_fence_wait() in core_qbuf() (Alex)
- merge preparation commit in

v6: - With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4: - Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
- Remove if (vb->state != VB2_BUF_STATE_QUEUED) from vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2: - fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/common/videobuf2/Kconfig          |   1 +
 drivers/media/common/videobuf2/videobuf2-core.c | 224 ++++++++++++++++++++----
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  37 +++-
 drivers/media/dvb-core/dvb_vb2.c                |   2 +-
 include/media/videobuf2-core.h                  |  19 +-
 5 files changed, 242 insertions(+), 41 deletions(-)

drivers/media/common/videobuf2/Kconfig +1 -0

index 17c32ea58395..27ad9e8a268b 100644
@@ -1,6 +1,7 @@
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
 	select DMA_SHARED_BUFFER
+	select SYNC_FILE
 	tristate
 
 config VIDEOBUF2_V4L2

drivers/media/common/videobuf2/videobuf2-core.c +71 -12

index a9a0a9d1decb..86b5ebe25263 100644
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 
 #include <media/videobuf2-core.h>
+#include <media/v4l2-ioctl.h>
 #include <media/v4l2-mc.h>
 
 #include <trace/events/vb2.h>

@@ -189,6 +190,7 @@ module_param(debug, int, 0644);
 
 static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
+static void __qbuf_work(struct work_struct *work);
 
 static void __vb2_buffer_free(struct kref *kref)
 {

@@ -373,6 +375,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
 		vb->index = q->num_buffers + buffer;
 		vb->type = q->type;
 		vb->memory = memory;
+		INIT_WORK(&vb->qbuf_work, __qbuf_work);
 		for (plane = 0; plane < num_planes; ++plane) {
 			vb->planes[plane].length = plane_sizes[plane];
 			vb->planes[plane].min_length = plane_sizes[plane];

@@ -932,21 +935,12 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned int plane_no)
 }
 EXPORT_SYMBOL_GPL(vb2_plane_cookie);
 
-void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+static void vb2_process_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
 	unsigned int plane;
 
-	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
-		return;
-
-	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
-		    state != VB2_BUF_STATE_ERROR &&
-		    state != VB2_BUF_STATE_QUEUED &&
-		    state != VB2_BUF_STATE_REQUEUEING))
-		state = VB2_BUF_STATE_ERROR;
-
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
 	 * Although this is not a callback, it still does have to balance

@@ -962,6 +956,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 
 	spin_lock_irqsave(&q->done_lock, flags);
+	if (vb->state == VB2_BUF_STATE_ACTIVE)
+		atomic_dec(&q->owned_by_drv_count);
+
 	if (state == VB2_BUF_STATE_QUEUED ||
 	    state == VB2_BUF_STATE_REQUEUEING) {
 		vb->state = VB2_BUF_STATE_QUEUED;

@@ -970,7 +967,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		list_add_tail(&vb->done_entry, &q->done_list);
 		vb->state = state;
 	}
-	atomic_dec(&q->owned_by_drv_count);
+
 	spin_unlock_irqrestore(&q->done_lock, flags);
 
 	trace_vb2_buf_done(q, vb);

@@ -987,6 +984,47 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 		wake_up(&q->done_wq);
 		break;
 	}
+
+}
+
+void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+{
+	struct vb2_buffer *next;
+	struct vb2_queue *q;
+
+	dprintk(4, "called done on buffer %d state: %d -> %d\n",
+			vb->index, vb->state, state);
+
+	if (vb->state == VB2_BUF_STATE_ERROR)
+		return;
+	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
+		return;
+
+	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+		    state != VB2_BUF_STATE_ERROR &&
+		    state != VB2_BUF_STATE_QUEUED &&
+		    state != VB2_BUF_STATE_REQUEUEING))
+		state = VB2_BUF_STATE_ERROR;
+
+	vb2_process_buffer_done(vb, state);
+
+	/*
+	 * Check if there is any buffer with an in-fence error in the next
+	 * position of the queue. Buffers whose in-fence signaled with error
+	 * are not queued to the driver and kept on the queue until the buffer
+	 * before them is done.
+	 * So here we process any existing buffers with in-fence errors and
+	 * wake up userspace.
+	 */
+	q = vb->vb2_queue;
+	next = list_next_entry(vb, queued_entry);
+	list_for_each_entry_from(next, &q->queued_list, queued_entry) {
+		if (!next->in_fence || !next->in_fence->error)
+			break;
+		if (next->in_fence && next->in_fence->error &&
+		    vb->state != VB2_BUF_STATE_ERROR)
+			vb2_process_buffer_done(next, VB2_BUF_STATE_ERROR);
+	}
 }
 EXPORT_SYMBOL_GPL(vb2_buffer_done);
 

@@ -1271,6 +1309,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 
+	if (WARN_ON(vb->state == VB2_BUF_STATE_ACTIVE))
+		return;
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->owned_by_drv_count);
 

@@ -1322,6 +1362,25 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
 	return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)
+{
+	struct vb2_buffer *vb;
+	int ready_count = 0;
+
+	/*
+	 * Count num of buffers ready in front of the queued_list.
+	 * We want to stop counting when we find a buffer with an
+	 * unsignaled fence.
+	 */
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
+		ready_count++;
+	}
+
+	return ready_count;
+}
+
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 {
 	struct vb2_buffer *vb;

@@ -1338,7 +1397,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 	if (ret)
 		return ret;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	call_void_bufop(q, fill_user_buffer, vb, pb);
 
 	dprintk(2, "prepare of buffer %d succeeded\n", vb->index);

@@ -1364,11 +1423,16 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	int ret;

drivers/media/common/videobuf2/videobuf2-core.c static int vb2_start_streaming(struct vb2_queue *q) · line 1430

Quoted 10 lines

drivers/media/common/videobuf2/videobuf2-core.c +4 -3

 
 	/*
-	 * If any buffers were queued before streamon,
-	 * we can now pass them to driver for processing.
+	 * Activate all buffers currently queued,
+	 * until we get an unsignaled fence.
 	 */
-	list_for_each_entry(vb, &q->queued_list, queued_entry)
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (vb->state != VB2_BUF_STATE_QUEUED)

I prefer: if (vb->state == VB2_BUF_STATE_ERROR)

It also needs a comment.

Quoted 23 lines

drivers/media/common/videobuf2/videobuf2-core.c +14 -1

+			continue;
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
 		__enqueue_in_driver(vb);
+	}
 
 	/* Tell the driver to start streaming */
 	q->start_streaming_called = 1;

@@ -1410,7 +1474,41 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	return ret;
 }
 
-int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
+static void __qbuf_work(struct work_struct *work)
+{
+	struct vb2_buffer *vb;
+	struct vb2_queue *q;
+
+	vb = container_of(work, struct vb2_buffer, qbuf_work);
+	q = vb->vb2_queue;
+
+	if (q->lock)
+		mutex_lock(q->lock);

drivers/media/common/videobuf2/videobuf2-core.c static int vb2_start_streaming(struct vb2_queue *q) · line 1496

Quoted 10 lines

drivers/media/common/videobuf2/videobuf2-core.c +10 -0

+	/* By the time we run, the buffer state could have changed,
+	 * so we need to check it's in the queued state.
+	 * This is the only valid state.
+	 */
+	if (q->start_streaming_called && vb->state == VB2_BUF_STATE_QUEUED)
+		__enqueue_in_driver(vb);
+
+	if (q->streaming && !q->start_streaming_called &&
+	    __get_num_ready_buffers(q) >= q->min_buffers_needed)
+		vb2_start_streaming(q);

What happens if vb2_start_streaming fails?

Quoted 73 lines

drivers/media/common/videobuf2/videobuf2-core.c +41 -9

+
+	if (q->lock)
+		mutex_unlock(q->lock);
+	__vb2_buffer_put(vb);
+}
+
+static void vb2_qbuf_fence_cb(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+	struct vb2_buffer *vb = container_of(cb, struct vb2_buffer, fence_cb);
+
+	schedule_work(&vb->qbuf_work);
+}
+
+int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
+		  struct dma_fence *in_fence)
 {
 	struct vb2_buffer *vb;
 	int ret;

@@ -1421,16 +1519,18 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	case VB2_BUF_STATE_DEQUEUED:
 		ret = __buf_prepare(vb, pb);
 		if (ret)
-			return ret;
+			goto err_release;
 		break;
 	case VB2_BUF_STATE_PREPARED:
 		break;
 	case VB2_BUF_STATE_PREPARING:
 		dprintk(1, "buffer still being prepared\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_release;
 	default:
 		dprintk(1, "invalid buffer state %d\n", vb->state);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_release;
 	}
 
 	/*

@@ -1448,15 +1548,44 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	trace_vb2_qbuf(q, vb);
 
 	/*
-	 * If already streaming, give the buffer to driver for processing.
-	 * If not, the buffer will be given to driver on next streamon.
+	 * If the fence didn't signal yet, we setup a callback to queue
+	 * the buffer once the fence signals. If the fence already signaled,
+	 * then we lose the reference and queue the buffer.
 	 */
-	if (q->start_streaming_called)
-		__enqueue_in_driver(vb);
+	if (in_fence) {
+		WARN_ON(vb->in_fence);
+		vb->in_fence = in_fence;
+		__vb2_buffer_get(vb);
+		ret = dma_fence_add_callback(in_fence, &vb->fence_cb,
+					     vb2_qbuf_fence_cb);
+		/* is the fence signaled? */
+		if (ret == -ENOENT) {
+			if (in_fence->error)
+				/* error-signaled fence, reject this buffer */
+				goto err_release;
+			dma_fence_put(in_fence);
+			vb->in_fence = NULL;
+			__vb2_buffer_put(vb);
+		} else if (ret) {
+			goto err_release;
+		}
+	}
 
-	/* Fill buffer information for the userspace */
-	if (pb)

drivers/media/common/videobuf2/videobuf2-core.c int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb) · line 1582

Quoted 10 lines

drivers/media/common/videobuf2/videobuf2-core.c +9 -1

-		call_void_bufop(q, fill_user_buffer, vb, pb);
+	/*
+	 * If already streaming and there is no fence to wait on
+	 * give the buffer to driver for processing.
+	 */
+	if (q->start_streaming_called) {
+		struct vb2_buffer *b;
+
+		list_for_each_entry(b, &q->queued_list, queued_entry) {
+			if (b->state != VB2_BUF_STATE_QUEUED)

b->state == VB2_BUF_STATE_ERROR

Quoted 67 lines

drivers/media/common/videobuf2/videobuf2-core.c +33 -4

+				continue;
+			if (b->in_fence && dma_fence_get_status(b->in_fence) == 0)
+				break;
+			__enqueue_in_driver(b);
+		}
+	}
 
 	/*
 	 * If streamon has been called, and we haven't yet called

@@ -1465,14 +1594,32 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	 * then we can finally call start_streaming().
 	 */
 	if (q->streaming && !q->start_streaming_called &&
-	    q->queued_count >= q->min_buffers_needed) {
+	    __get_num_ready_buffers(q) >= q->min_buffers_needed) {
 		ret = vb2_start_streaming(q);
 		if (ret)
-			return ret;
+			goto err_release;
 	}
 
+	/* Fill buffer information for userspace */
+	if (pb)
+		call_void_bufop(q, fill_user_buffer, vb, pb);
+
 	dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
 	return 0;
+
+err_release:
+	/* Fill buffer information for userspace */
+	if (pb)
+		call_void_bufop(q, fill_user_buffer, vb, pb);
+
+	if (in_fence) {
+		dma_fence_put(in_fence);
+		vb->in_fence = NULL;
+		__vb2_buffer_put(vb);
+	}
+
+	return ret;
+
 }
 EXPORT_SYMBOL_GPL(vb2_core_qbuf);
 

@@ -1615,7 +1762,12 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 		return;
 
 	vb->state = VB2_BUF_STATE_DEQUEUED;
-
+	if (vb->in_fence) {
+		if (dma_fence_remove_callback(vb->in_fence, &vb->fence_cb))
+			__vb2_buffer_put(vb);
+		dma_fence_put(vb->in_fence);
+		vb->in_fence = NULL;
+	}
 	/* unmap DMABUF buffer */
 	if (q->memory == VB2_MEMORY_DMABUF)
 		for (i = 0; i < vb->num_planes; ++i) {

@@ -1653,7 +1805,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	if (pindex)
 		*pindex = vb->index;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	if (pb)
 		call_void_bufop(q, fill_user_buffer, vb, pb);
 

drivers/media/common/videobuf2/videobuf2-core.c static void __vb2_queue_cancel(struct vb2_queue *q) · line 1859

Quoted 11 lines

drivers/media/common/videobuf2/videobuf2-core.c +2 -2

@@ -1700,8 +1852,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
 		for (i = 0; i < q->num_buffers; ++i)
 			if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
-				pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n",
-					q->bufs[i]);
+				pr_warn("driver bug: stop_streaming operation is leaving buf[%d] 0x%p in active
 state\n",
+					q->bufs[i]->index, q->bufs[i]);
 				vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR);
 			}

Shouldn't any pending fences be canceled here?

I feel uncomfortable with the refcounting of buffers, I'd rather that when we cancel the queue all fences for buffers are removed/canceled/whatever.

Is there any reason for refcounting if we cancel all pending fences here?

Note that besides canceling fences you also need to cancel/flush __qbuf_work.

Quoted 23 lines

Quoted 23 lines

drivers/media/common/videobuf2/videobuf2-core.c +7 -2

@@ -1615,7 +1762,12 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 		return;
 
 	vb->state = VB2_BUF_STATE_DEQUEUED;
-
+	if (vb->in_fence) {
+		if (dma_fence_remove_callback(vb->in_fence, &vb->fence_cb))
+			__vb2_buffer_put(vb);
+		dma_fence_put(vb->in_fence);
+		vb->in_fence = NULL;
+	}
 	/* unmap DMABUF buffer */
 	if (q->memory == VB2_MEMORY_DMABUF)
 		for (i = 0; i < vb->num_planes; ++i) {

@@ -1653,7 +1805,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	if (pindex)
 		*pindex = vb->index;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	if (pb)
 		call_void_bufop(q, fill_user_buffer, vb, pb);
 

Quoted 14 lines

drivers/media/common/videobuf2/videobuf2-core.c static void __vb2_queue_cancel(struct vb2_queue *q) · line 1859

Quoted 11 lines

drivers/media/common/videobuf2/videobuf2-core.c +2 -2

@@ -1700,8 +1852,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
 		for (i = 0; i < q->num_buffers; ++i)
 			if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
-				pr_warn("driver bug: stop_streaming operation is leaving buf %p in active state\n",
-					q->bufs[i]);
+				pr_warn("driver bug: stop_streaming operation is leaving buf[%d] 0x%p in active
 state\n",
+					q->bufs[i]->index, q->bufs[i]);
 				vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR);
 			}

Shouldn't any pending fences be canceled here?

No, we don't have to flush -- that's the reason of the refcount :) The qbuf_work won't do anything if all the buffers are returned by the driver (with error or done state), and if !streaming.

Also, note that's why qbuf_work checks for the queued state, and not for the error state.

Quoted 8 lines

I feel uncomfortable with the refcounting of buffers, I'd rather that when we cancel the queue all fences for buffers are removed/canceled/whatever.

Is there any reason for refcounting if we cancel all pending fences here?

Note that besides canceling fences you also need to cancel/flush __qbuf_work.

Like I said above, I'm trying to avoid cancel/flushing the workqueue. Currently, I believe it works fine without any flushing, provided we refcount the buffers.

The problem with cancelling the workqueue, is that you need to unlock the queue lock, to avoid a deadlock. It seemed to me that having a refcount is more natural.

Thoughts?

Quoted 59 lines · Ezequiel Garcia

Quoted 23 lines

Quoted 23 lines

drivers/media/common/videobuf2/videobuf2-core.c +7 -2

@@ -1615,7 +1762,12 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
 		return;
 
 	vb->state = VB2_BUF_STATE_DEQUEUED;
-
+	if (vb->in_fence) {
+		if (dma_fence_remove_callback(vb->in_fence, &vb->fence_cb))
+			__vb2_buffer_put(vb);
+		dma_fence_put(vb->in_fence);
+		vb->in_fence = NULL;
+	}
 	/* unmap DMABUF buffer */
 	if (q->memory == VB2_MEMORY_DMABUF)
 		for (i = 0; i < vb->num_planes; ++i) {

@@ -1653,7 +1805,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	if (pindex)
 		*pindex = vb->index;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	if (pb)
 		call_void_bufop(q, fill_user_buffer, vb, pb);
 

Quoted 15 lines

Quoted 11 lines

drivers/media/common/videobuf2/videobuf2-core.c +2 -2

@@ -1700,8 +1852,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
 		for (i = 0; i < q->num_buffers; ++i)
 			if (q->bufs[i]->state == VB2_BUF_STATE_ACTIVE) {
-				pr_warn("driver bug: stop_streaming operation is leaving buf %p in active
 state\n",
-					q->bufs[i]);
+				pr_warn("driver bug: stop_streaming operation is leaving buf[%d] 0x%p in active
 state\n",
+					q->bufs[i]->index, q->bufs[i]);
 				vb2_buffer_done(q->bufs[i], VB2_BUF_STATE_ERROR);

Quoted 1 line

}

Shouldn't any pending fences be canceled here?

No, we don't have to flush -- that's the reason of the refcount :) The qbuf_work won't do anything if all the buffers are returned by the driver (with error or done state), and if !streaming.

Also, note that's why qbuf_work checks for the queued state, and not for the error state.

Quoted 8 lines

I feel uncomfortable with the refcounting of buffers, I'd rather that when we cancel the queue all fences for buffers are removed/canceled/whatever.

Is there any reason for refcounting if we cancel all pending fences here?

Note that besides canceling fences you also need to cancel/flush __qbuf_work.

Like I said above, I'm trying to avoid cancel/flushing the workqueue. Currently, I believe it works fine without any flushing, provided we refcount the buffers.

Quoted 5 lines

The problem with cancelling the workqueue, is that you need to unlock the queue lock, to avoid a deadlock. It seemed to me that having a refcount is more natural.

Thoughts?

I'll take another look tomorrow morning. Do you have a public git tree containing this series that I can browse?

From: Gustavo Padovan <[email protected]>

Receive in-fence from userspace and add support for waiting on them before queueing the buffer to the driver. Buffers can't be queued to the driver before its fences signal. And a buffer can't be queued to the driver out of the order they were queued from userspace. That means that even if its fence signals it must wait for all other buffers, ahead of it in the queue,
to signal first.

If the fence for some buffer fails we do not queue it to the driver, instead we mark it as error and wait until the previous buffer is done to notify userspace of the error. We wait here to deliver the buffers back to userspace in order.

v13: - cleanup implementation.
- remove wrong Kconfig changes.
- print noisy warning on unexpected enqueue conditioin
- schedule a vb2_start_streaming work from the fence callback

v12: fixed dvb_vb2.c usage of vb2_core_qbuf.

v11: - minor doc/comments fixes (Hans Verkuil)
- reviewed the in-fence path at __fill_v4l2_buffer()

v10: - rename fence to in_fence in many places
- handle fences signalling with error better (Hans Verkuil)

v9: - improve comments and docs (Hans Verkuil)
- fix unlocking of vb->fence_cb_lock on vb2_core_qbuf (Hans Verkuil)
- move in-fences code that was in the out-fences patch here (Alex)

v8: - improve comments about fences with errors

v7: - get rid of the fence array stuff for ordering and just use
get_num_buffers_ready() (Hans)
- fix issue of queuing the buffer twice (Hans)
- avoid the dma_fence_wait() in core_qbuf() (Alex)
- merge preparation commit in

v6: - With fences always keep the order userspace queues the buffers.
- Protect in_fence manipulation with a lock (Brian Starkey)
- check if fences have the same context before adding a fence array
- Fix last_fence ref unbalance in __set_in_fence() (Brian Starkey)
- Clean up fence if __set_in_fence() fails (Brian Starkey)
- treat -EINVAL from dma_fence_add_callback() (Brian Starkey)

v5: - use fence_array to keep buffers ordered in vb2 core when
needed (Brian Starkey)
- keep backward compat on the reserved2 field (Brian Starkey)
- protect fence callback removal with lock (Brian Starkey)

v4: - Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
- Remove if (vb->state != VB2_BUF_STATE_QUEUED) from vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2: - fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by Gustavo Padovan <[email protected]>

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/common/videobuf2/Kconfig          |   1 +
 drivers/media/common/videobuf2/videobuf2-core.c | 224
++++++++++++++++++++----
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  37 +++-
 drivers/media/dvb-core/dvb_vb2.c                |   2 +-
 include/media/videobuf2-core.h                  |  19 +-
 5 files changed, 242 insertions(+), 41 deletions(-)

drivers/media/common/videobuf2/Kconfig +1 -0

index 17c32ea58395..27ad9e8a268b 100644
@@ -1,6 +1,7 @@
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
 	select DMA_SHARED_BUFFER
+	select SYNC_FILE
 	tristate
 
 config VIDEOBUF2_V4L2

drivers/media/common/videobuf2/videobuf2-core.c +3 -0

index a9a0a9d1decb..86b5ebe25263 100644
@@ -27,6 +27,7 @@
 #include <linux/kthread.h>
 
 #include <media/videobuf2-core.h>
+#include <media/v4l2-ioctl.h>
 #include <media/v4l2-mc.h>
 
 #include <trace/events/vb2.h>

@@ -189,6 +190,7 @@ module_param(debug, int, 0644);
 
 static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
+static void __qbuf_work(struct work_struct *work);
 
 static void __vb2_buffer_free(struct kref *kref)
 {

@@ -373,6 +375,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum
 vb2_memory memory,
 		vb->index = q->num_buffers + buffer;
 		vb->type = q->type;
 		vb->memory = memory;
+		INIT_WORK(&vb->qbuf_work, __qbuf_work);
 		for (plane = 0; plane < num_planes; ++plane) {
 			vb->planes[plane].length = plane_sizes[plane];

drivers/media/common/videobuf2/videobuf2-core.c +1 -10

 			vb->planes[plane].min_length = plane_sizes[plane];

@@ -932,21 +935,12 @@ void *vb2_plane_cookie(struct vb2_buffer *vb, unsigned
 int plane_no)
 }
 EXPORT_SYMBOL_GPL(vb2_plane_cookie);
 
-void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+static void vb2_process_buffer_done(struct vb2_buffer *vb, enum
 vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
 	unsigned int plane;
 
-	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
-		return;
-
-	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
-		    state != VB2_BUF_STATE_ERROR &&
-		    state != VB2_BUF_STATE_QUEUED &&
-		    state != VB2_BUF_STATE_REQUEUEING))
-		state = VB2_BUF_STATE_ERROR;
-
 #ifdef CONFIG_VIDEO_ADV_DEBUG

drivers/media/common/videobuf2/videobuf2-core.c +3 -0

 	/*
 	 * Although this is not a callback, it still does have to balance

@@ -962,6 +956,9 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
 vb2_buffer_state state)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 
 	spin_lock_irqsave(&q->done_lock, flags);
+	if (vb->state == VB2_BUF_STATE_ACTIVE)
+		atomic_dec(&q->owned_by_drv_count);
+
 	if (state == VB2_BUF_STATE_QUEUED ||
 	    state == VB2_BUF_STATE_REQUEUEING) {

drivers/media/common/videobuf2/videobuf2-core.c +1 -1

 		vb->state = VB2_BUF_STATE_QUEUED;

@@ -970,7 +967,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
 vb2_buffer_state state)
 		list_add_tail(&vb->done_entry, &q->done_list);
 		vb->state = state;
 	}
-	atomic_dec(&q->owned_by_drv_count);
+
 	spin_unlock_irqrestore(&q->done_lock, flags);
 

drivers/media/common/videobuf2/videobuf2-core.c +62 -0

 	trace_vb2_buf_done(q, vb);

@@ -987,6 +984,47 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum
 vb2_buffer_state state)
 		wake_up(&q->done_wq);
 		break;
 	}
+
+}
+
+void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
+{
+	struct vb2_buffer *next;
+	struct vb2_queue *q;
+
+	dprintk(4, "called done on buffer %d state: %d -> %d\n",
+			vb->index, vb->state, state);
+
+	if (vb->state == VB2_BUF_STATE_ERROR)
+		return;
+	if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
+		return;
+
+	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
+		    state != VB2_BUF_STATE_ERROR &&
+		    state != VB2_BUF_STATE_QUEUED &&
+		    state != VB2_BUF_STATE_REQUEUEING))
+		state = VB2_BUF_STATE_ERROR;
+
+	vb2_process_buffer_done(vb, state);
+
+	/*
+	 * Check if there is any buffer with an in-fence error in the next
+	 * position of the queue. Buffers whose in-fence signaled with error
+	 * are not queued to the driver and kept on the queue until the buffer
+	 * before them is done.
+	 * So here we process any existing buffers with in-fence errors and
+	 * wake up userspace.
+	 */
+	q = vb->vb2_queue;
+	next = list_next_entry(vb, queued_entry);
+	list_for_each_entry_from(next, &q->queued_list, queued_entry) {
+		if (!next->in_fence || !next->in_fence->error)
+			break;
+		if (next->in_fence && next->in_fence->error &&
+		    vb->state != VB2_BUF_STATE_ERROR)
+			vb2_process_buffer_done(next, VB2_BUF_STATE_ERROR);
+	}
 }
 EXPORT_SYMBOL_GPL(vb2_buffer_done);

@@ -1271,6 +1309,8 @@ static void __enqueue_in_driver(struct vb2_buffer
 *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 
+	if (WARN_ON(vb->state == VB2_BUF_STATE_ACTIVE))
+		return;
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->owned_by_drv_count);

@@ -1322,6 +1362,25 @@ static int __buf_prepare(struct vb2_buffer *vb, const
 void *pb)
 	return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)
+{
+	struct vb2_buffer *vb;
+	int ready_count = 0;
+
+	/*
+	 * Count num of buffers ready in front of the queued_list.
+	 * We want to stop counting when we find a buffer with an
+	 * unsignaled fence.
+	 */
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
+		ready_count++;
+	}
+
+	return ready_count;
+}
+
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void
 *pb)

{

drivers/media/common/videobuf2/videobuf2-core.c +1 -1

 	struct vb2_buffer *vb;

@@ -1338,7 +1397,7 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned
 int index, void *pb)
 	if (ret)
 		return ret;
 
-	/* Fill buffer information for the userspace */
+	/* Fill buffer information for userspace */
 	call_void_bufop(q, fill_user_buffer, vb, pb);
 

drivers/media/common/videobuf2/videobuf2-core.c +14 -4

 	dprintk(2, "prepare of buffer %d succeeded\n", vb->index);

@@ -1364,11 +1423,16 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	int ret;
 
 	/*
-	 * If any buffers were queued before streamon,
-	 * we can now pass them to driver for processing.
+	 * Activate all buffers currently queued,
+	 * until we get an unsignaled fence.
 	 */
-	list_for_each_entry(vb, &q->queued_list, queued_entry)
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (vb->state != VB2_BUF_STATE_QUEUED)
+			continue;
+		if (vb->in_fence && dma_fence_get_status(vb->in_fence) == 0)
+			break;
 		__enqueue_in_driver(vb);
+	}
 
 	/* Tell the driver to start streaming */
 	q->start_streaming_called = 1;

@@ -1410,7 +1474,41 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	return ret;
 }
 
-int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
+static void __qbuf_work(struct work_struct *work)
+{
+	struct vb2_buffer *vb;
+	struct vb2_queue *q;
+
+	vb = container_of(work, struct vb2_buffer, qbuf_work);