Re: [PATCH v10 13/16] vb2: add out-fence support to QBUF

2018-05-25 Thread Brian Starkey 

Hi Ezequiel,

Not sure if this patch series is still relevant, but I spotted a
couple more things below.

On Mon, May 21, 2018 at 01:59:43PM -0300, Ezequiel Garcia wrote:

From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace in the fence_fd field as a
return arg for the QBUF call.

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

v12: - Pass the fence_fd in vb2_qbuf for clarity.
- Increase fence seqno if fence context if shared
- Check get_unused_fd return

v11: - Return fence_fd to userpace only in the QBUF ioctl.
- Rework implementation to avoid storing the sync_file
  as state, which is not really needed.

v10: - use -EIO for fence error (Hans Verkuil)
- add comment around fence context creation (Hans Verkuil)

v9: - remove in-fences changes from this patch (Alex Courbot)
   - improve fence context creation (Hans Verkuil)
   - clean up out fences if vb2_core_qbuf() fails (Hans Verkuil)

v8: - return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
   - fix crash when checking not using fences in vb2_buffer_done()

v7: - merge patch that add the infrastructure to out-fences into
 this one (Alex Courbot)
   - Do not install the fd if there is no fence. (Alex Courbot)
   - do not report error on requeueing, just WARN_ON_ONCE() (Hans)

v6: - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
 ordering in vb2 for queueing in the driver, so the event is not
 necessary anymore and the out_fence_fd is sent back to userspace
 on QBUF call return arg
   - do not allow requeueing with out-fences, instead mark the buffer
 with an error and wake up to userspace.
   - send the out_fence_fd back to userspace on the fence_fd field

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 
Signed-off-by: Ezequiel Garcia 
---
drivers/media/common/videobuf2/videobuf2-core.c | 113 +++-
drivers/media/common/videobuf2/videobuf2-v4l2.c |  10 ++-
drivers/media/dvb-core/dvb_vb2.c|   2 +-
include/media/videobuf2-core.h  |  20 -
4 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 86b5ebe25263..edc2fdaf56de 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -25,6 +25,7 @@
#include 
#include 
#include 
+#include 

#include 
#include 
@@ -380,6 +381,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 */
@@ -976,10 +978,22 @@ static void vb2_process_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /* Requeuing with explicit synchronization, spit warning */
+   WARN_ON_ONCE(vb->out_fence);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (vb->out_fence) {
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EIO);
+   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(>done_wq);
break;
@@ -1406,6 +1420,76 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
}
EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);

+static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence)
+{
+   return

Re: [PATCH v10 13/16] vb2: add out-fence support to QBUF

2018-05-22 Thread Hans Verkuil 
On 21/05/18 18:59, Ezequiel Garcia wrote:
> From: Gustavo Padovan 
> 
> If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
> an out_fence and send its fd to userspace in the fence_fd field as a
> return arg for the QBUF call.
> 
> The fence is signaled on buffer_done(), when the job on the buffer is
> finished.
> 
> v12: - Pass the fence_fd in vb2_qbuf for clarity.
>  - Increase fence seqno if fence context if shared
>  - Check get_unused_fd return

v12 while this this PATCH v10? That's a bit confusing.

> 
> v11: - Return fence_fd to userpace only in the QBUF ioctl.
>  - Rework implementation to avoid storing the sync_file
>as state, which is not really needed.
> 
> v10: - use -EIO for fence error (Hans Verkuil)
>  - add comment around fence context creation (Hans Verkuil)
> 
> v9: - remove in-fences changes from this patch (Alex Courbot)
> - improve fence context creation (Hans Verkuil)
> - clean up out fences if vb2_core_qbuf() fails (Hans Verkuil)
> 
> v8: - return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
> - fix crash when checking not using fences in vb2_buffer_done()
> 
> v7: - merge patch that add the infrastructure to out-fences into
>   this one (Alex Courbot)
> - Do not install the fd if there is no fence. (Alex Courbot)
> - do not report error on requeueing, just WARN_ON_ONCE() (Hans)
> 
> v6: - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
>   ordering in vb2 for queueing in the driver, so the event is not
>   necessary anymore and the out_fence_fd is sent back to userspace
>   on QBUF call return arg
> - do not allow requeueing with out-fences, instead mark the buffer
>   with an error and wake up to userspace.
> - send the out_fence_fd back to userspace on the fence_fd field
> 
> 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 
> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/media/common/videobuf2/videobuf2-core.c | 113 
> +++-
>  drivers/media/common/videobuf2/videobuf2-v4l2.c |  10 ++-
>  drivers/media/dvb-core/dvb_vb2.c|   2 +-
>  include/media/videobuf2-core.h  |  20 -
>  4 files changed, 136 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
> b/drivers/media/common/videobuf2/videobuf2-core.c
> index 86b5ebe25263..edc2fdaf56de 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -380,6 +381,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 */
> @@ -976,10 +978,22 @@ static void vb2_process_buffer_done(struct vb2_buffer 
> *vb, enum vb2_buffer_state
>   case VB2_BUF_STATE_QUEUED:
>   return;
>   case VB2_BUF_STATE_REQUEUEING:
> + /* Requeuing with explicit synchronization, spit warning */
> + WARN_ON_ONCE(vb->out_fence);
> +
>   if (q->start_streaming_called)
>   __enqueue_in_driver(vb);
>   return;
>   default:
> + if (vb->out_fence) {
> + if (state == VB2_BUF_STATE_ERROR)
> + dma_fence_set_error(vb->out_fence, -EIO);
> + 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(>done_wq);
>   break;
> @@ -1406,6 +1420,76 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
> int index, void *pb)
>  }
>  EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
>  
> +static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
>

[PATCH v10 13/16] vb2: add out-fence support to QBUF

2018-05-21 Thread Ezequiel Garcia 
From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and send its fd to userspace in the fence_fd field as a
return arg for the QBUF call.

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

v12: - Pass the fence_fd in vb2_qbuf for clarity.
 - Increase fence seqno if fence context if shared
 - Check get_unused_fd return

v11: - Return fence_fd to userpace only in the QBUF ioctl.
 - Rework implementation to avoid storing the sync_file
   as state, which is not really needed.

v10: - use -EIO for fence error (Hans Verkuil)
 - add comment around fence context creation (Hans Verkuil)

v9: - remove in-fences changes from this patch (Alex Courbot)
- improve fence context creation (Hans Verkuil)
- clean up out fences if vb2_core_qbuf() fails (Hans Verkuil)

v8: - return 0 as fence_fd if OUT_FENCE flag not used (Mauro)
- fix crash when checking not using fences in vb2_buffer_done()

v7: - merge patch that add the infrastructure to out-fences into
  this one (Alex Courbot)
- Do not install the fd if there is no fence. (Alex Courbot)
- do not report error on requeueing, just WARN_ON_ONCE() (Hans)

v6: - get rid of the V4L2_EVENT_OUT_FENCE event. We always keep the
  ordering in vb2 for queueing in the driver, so the event is not
  necessary anymore and the out_fence_fd is sent back to userspace
  on QBUF call return arg
- do not allow requeueing with out-fences, instead mark the buffer
  with an error and wake up to userspace.
- send the out_fence_fd back to userspace on the fence_fd field

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 
Signed-off-by: Ezequiel Garcia 
---
 drivers/media/common/videobuf2/videobuf2-core.c | 113 +++-
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  10 ++-
 drivers/media/dvb-core/dvb_vb2.c|   2 +-
 include/media/videobuf2-core.h  |  20 -
 4 files changed, 136 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 86b5ebe25263..edc2fdaf56de 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -380,6 +381,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 */
@@ -976,10 +978,22 @@ static void vb2_process_buffer_done(struct vb2_buffer 
*vb, enum vb2_buffer_state
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /* Requeuing with explicit synchronization, spit warning */
+   WARN_ON_ONCE(vb->out_fence);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (vb->out_fence) {
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -EIO);
+   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(>done_wq);
break;
@@ -1406,6 +1420,76 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence)
+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct