Re: [PATCH 03/12] [media] vb2: add in-fence support to QBUF

[Gustavo Padovan <gustavo@padovan.org>]

2017-07-07 Hans Verkuil <hverkuil@xs4all.nl>:

> On 07/07/2017 03:53 AM, Gustavo Padovan wrote:
> > > 
> > > >   	help
> > > >   	  If you want to use Webcams, Video grabber devices and/or TV devices
> > > >   	  enable this option and other options below.
> > > > diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> > > > index ea83126..29aa9d4 100644
> > 
> > > > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > > > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > > > @@ -1279,6 +1279,22 @@ 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 */
> > > > +	list_for_each_entry(vb, &q->queued_list, queued_entry) {
> > > > +		if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))
> > > > +			break;
> > > 
> > > Obviously the break is wrong as Mauro mentioned.
> > 
> > I replied this in the other email to Mauro, if a fence is not signaled
> > it is not ready te be queued by the driver nor is all buffers following
> > it. Hence the break. They need all to be in order and in front of the
> > queue.
> > 
> > In any case I'll check this again as now there is two people saying I'm
> > wrong! :)
> 
> I think this comes back to the 'ordered' requirement and what that means
> exactly. In this particular case if I have buffers queued up in vb2 without
> a fence (or the fence was signaled), why shouldn't it possible to queue them
> up to the driver right away?
> 
> Of course, if all buffers are waiting for a fence, then __get_num_ready_buffers
> returns 0 and nothing happens.
> 
> My understanding is that the ordered requirement is for the hardware,
> i.e. queueing buffers A, B, C to ordered hardware requires that they come
> out in the same order.

That is correct. I thought I had to queue to the hardware in the order we
receive from userspace. So that makes that loop indeed wrong, as we
should queue the buffers right away.

The ordered requirement is for the OUT_FENCE side because after we queue
the buffer to the hardware and send the BUF_QUEUED event out userspace
might just go ahead and issue an DRM Atomic Request containing that
buffer and the out-fence fd received. DRM then needs to wait on that
fence before any scanout, but it may wait after the scanout is not
allowed to fail anymore.

Thus if a buffer requeuing happens at buffer_done() the fence won't
signal and DRM/KMS won't have a buffer to display. That is reason behind
it.

Alternatively we can ignore this and live with the fact that sometimes a
requeuing may affect the scanout pipeline.

	Gustavo