2017-09-02 Hans Verkuil <hverkuil@xxxxxxxxx>: > On 09/01/2017 08:21 PM, Gustavo Padovan wrote: > > Hi Hans, > > > > 2017-09-01 Hans Verkuil <hverkuil@xxxxxxxxx>: > > > >> Hi Gustavo, > >> > >> I think I concentrate on this last patch first. Once I fully understand this > >> I can review the code. Remember, it's been a while for me since I last looked > >> at fences, so forgive me if I ask stupid questions. On the other hand, others > >> with a similar lack of understanding of fences probably have similar questions, > >> so it is a good indication where the documentation needs improvement :-) > > > > Please ask as many as you want, those are the best questions. :) > > > >> > >> On 01/09/17 03:50, Gustavo Padovan wrote: > >>> From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxx> > >>> > >>> Add section to VIDIOC_QBUF about it > >>> > >>> Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxx> > >>> ---q > >>> Documentation/media/uapi/v4l/vidioc-qbuf.rst | 30 ++++++++++++++++++++++++++++ > >>> 1 file changed, 30 insertions(+) > >>> > >>> diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst b/Documentation/media/uapi/v4l/vidioc-qbuf.rst > >>> index 1f3612637200..6bd960d3972b 100644 > >>> --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst > >>> +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst > >>> @@ -117,6 +117,36 @@ immediately with an ``EAGAIN`` error code when no buffer is available. > >>> The struct :c:type:`v4l2_buffer` structure is specified in > >>> :ref:`buffer`. > >>> > >>> +Explicit Synchronization > >>> +------------------------ > >>> + > >>> +Explicit Synchronization allows us to control the synchronization of > >>> +shared buffers from userspace by passing fences to the kernel and/or > >>> +receiving them from it. Fences passed to the kernel are named in-fences and > >>> +the kernel should wait them to signal before using the buffer, i.e., queueing > >>> +it to the driver. On the other side, the kernel can create out-fences for the > >>> +buffers it queues to the drivers, out-fences signal when the driver is > >>> +finished with buffer, that is the buffer is ready. > >> > >> This should add a line explaining that fences are represented by file descriptors. > > > > Okay. > > > >> > >>> + > >>> +The in-fences and out-fences are communicated at the ``VIDIOC_QBUF`` ioctl > >>> +using the ``V4L2_BUF_FLAG_IN_FENCE`` and ``V4L2_BUF_FLAG_OUT_FENCE`` buffer > >>> +flags and the `fence_fd` field. If an in-fence needs to be passed to the kernel, > >>> +`fence_fd` should be set to the fence file descriptor number and the > >>> +``V4L2_BUF_FLAG_IN_FENCE`` should be set as well. > >> > >> Is it possible to have both flags at the same time? Or are they mutually exclusive? > >> > >> If only V4L2_BUF_FLAG_IN_FENCE is set, then what is the value of fence_fd after > >> the QBUF call? -1? > > > > Yes, it is -1. > > > >> > >>> + > >>> +To get a out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag should > >>> +be set and the `fence_fd` field will be returned with the out-fence file > >>> +descriptor related to the next buffer to be queued internally to the V4L2 > >>> +driver. That means the out-fence may not be associated with the buffer in the > >>> +current ``VIDIOC_QBUF`` ioctl call because the ordering in which videobuf2 core > >>> +queues the buffers to the drivers can't be guaranteed. To become aware of the > >>> +buffer with which the out-fence will be attached the ``V4L2_EVENT_BUF_QUEUED`` > >>> +should be used. It will trigger an event for every buffer queued to the V4L2 > >>> +driver. > >> > >> General question: does it even make sense to use an out-fence if you don't know with > >> what buffer is it associated? I mean, QBUF returns an out fence, but only some > >> time later are you informed about a buffer being queued. It also looks like userspace > >> has to keep track of the issued out-fences and the queued buffers and map them > >> accordingly. > >> > >> If the out-fence cannot be used until you know the buffer as well, then wouldn't > >> it make more sense if the out-fence and the buffer index are both sent by the > >> event? Of course, in that case the event can only be sent to the owner file handle > >> of the streaming device node, but that's OK, we have that. > >> > >> Setting the OUT_FENCE flag will just cause this event to be sent whenever that > >> buffer is queued internally. > >> > >> Sorry if this just shows a complete lack of understanding of fences on my side, > >> that's perfectly possible. > > > > Right, I can not think of anything that prevents what you are saying to > > work. I built it this way initially because on my lack of understanding > > of V4L2 (seems we complement each other here :) because I thought we > > needed to keep the QBUF ordering. In the last review you talked me away > > of this misconception but I really didn't took that to the > > implementation. > > > > If there is no care about ordering, there is no need to receive the > > fence before and we could just do as you say. That makes sense to me. > > We could do it that way but I have one question, maybe a stupid one. :) > > > > If a specific driver can guarantee the ordering of vb2 buffer, and > > userspace has a way to become aware of that. In this case we will > > receive the out-fence in QBUF knowing the buffer already (it is > > ordered!). Thus we can proceed right away and send it to the next > > driver. Does that makes sense? Is this a optmization we need? In your > > proposal we will have the timeframe between the queueing to the driver > > and buffer_done() to make the out_fence arrive on the next driver. If > > that is sufficient then we can just do as you propose. > > If it is ordered, then you can just send the event immediately from the > vb2 qbuf function. But you shouldn't return the out fence in the fence_fd > field. > > The fence_fd field can be renamed to in_fence_fd. The field in the event > struct can then be out_fence_fd. > > This is much cleaner since the fence_fd field is no longer used for both > in and out fence (that was very confusing!). That makes sense. I'll work on a new version with this modification and send it out as soon as possible. Thanks for reviewing! Gustavo
