Re: [RFC v5 09/11] [media] vb2: add infrastructure to support out-fences

[Gustavo Padovan <gustavo@padovan.org>]

2017-11-17 Alexandre Courbot <acourbot@chromium.org>:

> On Thursday, November 16, 2017 2:10:55 AM JST, Gustavo Padovan wrote:
> > From: Gustavo Padovan <gustavo.padovan@collabora.com>
> > 
> > Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.
> > 
> > v3:
> > 	- Do not hold yet another ref to the out_fence (Brian Starkey)
> > 
> > v2:	- change it to reflect fd_install at DQEVENT
> > 	- add fence context for out-fences
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com>
> > ---
> >  drivers/media/v4l2-core/videobuf2-core.c | 28 ++++++++++++++++++++++++++++
> >  include/media/videobuf2-core.h           | 20 ++++++++++++++++++++
> >  2 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> > b/drivers/media/v4l2-core/videobuf2-core.c
> > index 26de4c80717d..8b4f0e9bcb36 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -24,8 +24,10 @@
> >  #include <linux/freezer.h>
> >  #include <linux/kthread.h>
> >  #include <linux/dma-fence-array.h>
> > +#include <linux/sync_file.h>
> >  #include <media/videobuf2-core.h>
> > +#include <media/videobuf2-fence.h>
> >  #include <media/v4l2-mc.h>
> >  #include <trace/events/vb2.h>
> > @@ -1320,6 +1322,32 @@ int vb2_core_prepare_buf(struct vb2_queue *q,
> > unsigned int index, void *pb)
> >  }
> >  EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
> > +int vb2_setup_out_fence(struct vb2_queue *q, unsigned int index)
> > +{
> > +	struct vb2_buffer *vb;
> > +
> > +	vb = q->bufs[index];
> > +
> > +	vb->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
> 
> out_fence_fd is allocated in this patch but not used anywhere for the
> moment.
> For consistency, maybe move its allocation to the next patch, or move the
> call
> to fd_install() here if that is possible? In both cases, the call to
> get_unused_fd() can be moved right before fd_install() so you don't need to
> call put_unused_fd() in the error paths below.
> 
> ... same thing for sync_file too. Maybe this patch can just be merged into
> the next one? The current patch just creates an incomplete version of
> vb2_setup_out_fence() for which no user exist yet.

It turned out that out-fences patch is not big at all, so I can merge
them both. I think it will be cleaner, thanks for the suggestion.

Gustavo