Re: [RFC v4 16/17] [media] vb2: add out-fence support to QBUF
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
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
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