Re: [RFC v4 16/17] [media] vb2: add out-fence support to QBUF

2017-11-02 Thread Gustavo Padovan
Hi Brian,

2017-10-27 Brian Starkey :

> Hi Gustavo,
> 
> On Fri, Oct 20, 2017 at 07:50:11PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
> > an out_fence and send to userspace on the V4L2_EVENT_OUT_FENCE when
> > the buffer is queued to the driver, or right away if the queue is ordered
> > both in VB2 and in the driver.
> > 
> > The fence is signaled on buffer_done(), when the job on the buffer is
> > finished.
> > 
> > v5:
> > - delay fd_install to DQ_EVENT (Hans)
> > - if queue is fully ordered send OUT_FENCE event right away
> > (Brian)
> > - rename 'q->ordered' to 'q->ordered_in_driver'
> > - merge change to implement OUT_FENCE event here
> > 
> > v4:
> > - return the out_fence_fd in the BUF_QUEUED event(Hans)
> > 
> > v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
> > - set the OUT_FENCE flag if there is a fence pending (Hans)
> > - call fd_install() after vb2_core_qbuf() (Hans)
> > - clean up fence if vb2_core_qbuf() fails (Hans)
> > - add list to store sync_file and fence for the next queued buffer
> > 
> > v2: check if the queue is ordered.
> > 
> > Signed-off-by: Gustavo Padovan 
> > ---
> > drivers/media/v4l2-core/v4l2-event.c |  2 ++
> > drivers/media/v4l2-core/videobuf2-core.c | 25 +++
> > drivers/media/v4l2-core/videobuf2-v4l2.c | 55 
> > 
> > 3 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-event.c 
> > b/drivers/media/v4l2-core/v4l2-event.c
> > index 6274e3e174e0..275da224ace4 100644
> > --- a/drivers/media/v4l2-core/v4l2-event.c
> > +++ b/drivers/media/v4l2-core/v4l2-event.c
> > @@ -385,6 +385,8 @@ int v4l2_subscribe_event_v4l2(struct v4l2_fh *fh,
> > switch (sub->type) {
> > case V4L2_EVENT_CTRL:
> > return v4l2_ctrl_subscribe_event(fh, sub);
> > +   case V4L2_EVENT_OUT_FENCE:
> > +   return v4l2_event_subscribe(fh, sub, VIDEO_MAX_FRAME, NULL);
> > }
> > return -EINVAL;
> > }
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> > b/drivers/media/v4l2-core/videobuf2-core.c
> > index c7ba67bda5ac..21e2052776c1 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
> > vb2_memory memory,
> > vb->planes[plane].length = plane_sizes[plane];
> > vb->planes[plane].min_length = plane_sizes[plane];
> > }
> > +   vb->out_fence_fd = -1;
> > q->bufs[vb->index] = vb;
> > 
> > /* Allocate video buffer memory for the MMAP type */
> > @@ -934,10 +935,24 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
> > vb2_buffer_state state)
> > case VB2_BUF_STATE_QUEUED:
> > return;
> > case VB2_BUF_STATE_REQUEUEING:
> > +   /*
> > +* Explicit synchronization requires ordered queues for now,
> > +* so WARN_ON if we are requeuing on an ordered queue.
> > +*/
> > +   if (vb->out_fence)
> > +   WARN_ON_ONCE(q->ordered_in_driver);
> > +
> > if (q->start_streaming_called)
> > __enqueue_in_driver(vb);
> > return;
> > default:
> > +   if (state == VB2_BUF_STATE_ERROR)
> > +   dma_fence_set_error(vb->out_fence, -ENOENT);
> > +   dma_fence_signal(vb->out_fence);
> > +   dma_fence_put(vb->out_fence);
> > +   vb->out_fence = NULL;
> > +   vb->out_fence_fd = -1;
> > +
> > /* Inform any processes that may be waiting for buffers */
> > wake_up(&q->done_wq);
> > break;
> > @@ -1235,6 +1250,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> > trace_vb2_buf_queue(q, vb);
> > 
> > call_void_vb_qop(vb, buf_queue, vb);
> > +
> > +   if (!(q->is_output || q->ordered_in_vb2))
> > +   call_void_bufop(q, send_out_fence, vb);
> > }
> > 
> > static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
> > @@ -1451,6 +1469,7 @@ static struct dma_fence *__set_in_fence(struct 
> > vb2_queue *q,
> > }
> > 
> > q->last_fence = dma_fence_get(fence);
> > +   call_void_bufop(q, send_out_fence, vb);
> > }
> > 
> > return fence;
> > @@ -1840,6 +1859,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> > }
> > 
> > /*
> > +* Renew out-fence context.
> > +*/
> 
> Why is that? I don't think I understand the nuances of fence contexts.

Because inside each context we should maintain ordering of the fences.
If we cancel the stream and restart it with we need a new context. Look
at ordering between two different streams doesn't make sense.

> 
> > +   q->out_fence_context = dma_fence_context_alloc(1);
> > +
> > +   /*
> >  * Remove all buffers from videobuf's 

Re: [RFC v4 16/17] [media] vb2: add out-fence support to QBUF

2017-10-27 Thread Brian Starkey

Hi Gustavo,

On Fri, Oct 20, 2017 at 07:50:11PM -0200, Gustavo Padovan wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send to userspace on the V4L2_EVENT_OUT_FENCE when
the buffer is queued to the driver, or right away if the queue is ordered
both in VB2 and in the driver.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
drivers/media/v4l2-core/v4l2-event.c |  2 ++
drivers/media/v4l2-core/videobuf2-core.c | 25 +++
drivers/media/v4l2-core/videobuf2-v4l2.c | 55 
3 files changed, 82 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-event.c 
b/drivers/media/v4l2-core/v4l2-event.c
index 6274e3e174e0..275da224ace4 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -385,6 +385,8 @@ int v4l2_subscribe_event_v4l2(struct v4l2_fh *fh,
switch (sub->type) {
case V4L2_EVENT_CTRL:
return v4l2_ctrl_subscribe_event(fh, sub);
+   case V4L2_EVENT_OUT_FENCE:
+   return v4l2_event_subscribe(fh, sub, VIDEO_MAX_FRAME, NULL);
}
return -EINVAL;
}
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index c7ba67bda5ac..21e2052776c1 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;

/* Allocate video buffer memory for the MMAP type */
@@ -934,10 +935,24 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /*
+* Explicit synchronization requires ordered queues for now,
+* so WARN_ON if we are requeuing on an ordered queue.
+*/
+   if (vb->out_fence)
+   WARN_ON_ONCE(q->ordered_in_driver);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -ENOENT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(&q->done_wq);
break;
@@ -1235,6 +1250,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
trace_vb2_buf_queue(q, vb);

call_void_vb_qop(vb, buf_queue, vb);
+
+   if (!(q->is_output || q->ordered_in_vb2))
+   call_void_bufop(q, send_out_fence, vb);
}

static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
@@ -1451,6 +1469,7 @@ static struct dma_fence *__set_in_fence(struct vb2_queue 
*q,
}

q->last_fence = dma_fence_get(fence);
+   call_void_bufop(q, send_out_fence, vb);
}

return fence;
@@ -1840,6 +1859,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
}

/*
+* Renew out-fence context.
+*/


Why is that? I don't think I understand the nuances of fence contexts.


+   q->out_fence_context = dma_fence_context_alloc(1);
+
+   /*
 * Remove all buffers from videobuf's list...
 */
INIT_LIST_HEAD(&q->queued_list);
@@ -2171,6 +2195,7 @@ int vb2_core_queue_init(struct vb2_queue *q)
spin_lock_init(&q->done_lock);
mutex_init(&q->mmap_lock);
init_waitqueue_head(&q->done_wq);
+   q->out_fence_context = dma_fence_context_alloc(1);

if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-co

[RFC v4 16/17] [media] vb2: add out-fence support to QBUF

2017-10-20 Thread Gustavo Padovan
From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send to userspace on the V4L2_EVENT_OUT_FENCE when
the buffer is queued to the driver, or right away if the queue is ordered
both in VB2 and in the driver.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

v5:
- delay fd_install to DQ_EVENT (Hans)
- if queue is fully ordered send OUT_FENCE event right away
(Brian)
- rename 'q->ordered' to 'q->ordered_in_driver'
- merge change to implement OUT_FENCE event here

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/v4l2-event.c |  2 ++
 drivers/media/v4l2-core/videobuf2-core.c | 25 +++
 drivers/media/v4l2-core/videobuf2-v4l2.c | 55 
 3 files changed, 82 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-event.c 
b/drivers/media/v4l2-core/v4l2-event.c
index 6274e3e174e0..275da224ace4 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -385,6 +385,8 @@ int v4l2_subscribe_event_v4l2(struct v4l2_fh *fh,
switch (sub->type) {
case V4L2_EVENT_CTRL:
return v4l2_ctrl_subscribe_event(fh, sub);
+   case V4L2_EVENT_OUT_FENCE:
+   return v4l2_event_subscribe(fh, sub, VIDEO_MAX_FRAME, NULL);
}
return -EINVAL;
 }
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index c7ba67bda5ac..21e2052776c1 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
/* Allocate video buffer memory for the MMAP type */
@@ -934,10 +935,24 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /*
+* Explicit synchronization requires ordered queues for now,
+* so WARN_ON if we are requeuing on an ordered queue.
+*/
+   if (vb->out_fence)
+   WARN_ON_ONCE(q->ordered_in_driver);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -ENOENT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(&q->done_wq);
break;
@@ -1235,6 +1250,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
trace_vb2_buf_queue(q, vb);
 
call_void_vb_qop(vb, buf_queue, vb);
+
+   if (!(q->is_output || q->ordered_in_vb2))
+   call_void_bufop(q, send_out_fence, vb);
 }
 
 static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
@@ -1451,6 +1469,7 @@ static struct dma_fence *__set_in_fence(struct vb2_queue 
*q,
}
 
q->last_fence = dma_fence_get(fence);
+   call_void_bufop(q, send_out_fence, vb);
}
 
return fence;
@@ -1840,6 +1859,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
}
 
/*
+* Renew out-fence context.
+*/
+   q->out_fence_context = dma_fence_context_alloc(1);
+
+   /*
 * Remove all buffers from videobuf's list...
 */
INIT_LIST_HEAD(&q->queued_list);
@@ -2171,6 +2195,7 @@ int vb2_core_queue_init(struct vb2_queue *q)
spin_lock_init(&q->done_lock);
mutex_init(&q->mmap_lock);
init_waitqueue_head(&q->done_wq);
+   q->out_fence_context = dma_fence_context_alloc(1);
 
if (q->buf_struct_size == 0)
q->buf_struct_size = sizeof(struct vb2_buffer);
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 4c09ea007d90..9fb01ddefdc9 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/vide