From: Shuah Khan <shuahkhan@gmail.com>
To: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
Gustavo Padovan <gustavo@padovan.org>,
linux-media@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
LKML <linux-kernel@vger.kernel.org>,
Gustavo Padovan <gustavo.padovan@collabora.com>,
shuahkh@osg.samsung.com
Subject: Re: [RFC 00/10] V4L2 explicit synchronization support
Date: Mon, 3 Apr 2017 14:48:32 -0600 [thread overview]
Message-ID: <CAKocOOPAvkDpNXiFqsr=jxOvq2589piRV+HTMOoSy8gmZ-M67Q@mail.gmail.com> (raw)
In-Reply-To: <bafe3f9d-cdf4-bb22-b9c8-fe3f677d289c@osg.samsung.com>
Hi Gustavo,
On Mon, Apr 3, 2017 at 1:46 PM, Javier Martinez Canillas
<javier@osg.samsung.com> wrote:
> Hello Mauro and Gustavo,
>
> On 04/03/2017 07:16 AM, Mauro Carvalho Chehab wrote:
>> Hi Gustavo,
>>
>> Em Mon, 13 Mar 2017 16:20:25 -0300
>> Gustavo Padovan <gustavo@padovan.org> escreveu:
>>
>>> From: Gustavo Padovan <gustavo.padovan@collabora.com>
>>>
>>> Hi,
>>>
>>> This RFC adds support for Explicit Synchronization of shared buffers in V4L2.
>>> It uses the Sync File Framework[1] as vector to communicate the fences
>>> between kernel and userspace.
>>
>> Thanks for your work!
>>
>> I looked on your patchset, and I didn't notice anything really weird
>> there. So, instead on reviewing patch per patch, I prefer to discuss
>> about the requirements and API, as depending on it, the code base will
>> change a lot.
>>
>
> Agree that's better to first set on an uAPI and then implement based on that.
Yes. Agreeing on uAPI first would work well.
>
>> I'd like to do some tests with it on devices with mem2mem drivers.
>> My plan is to use an Exynos board for such thing, but I guess that
>> the DRM driver for it currently doesn't. I'm seeing internally if someone
>> could be sure that Exynos driver upstream will become ready for such
>> tests.
>>
>
> Not sure if you should try to do testing before agreeing on an uAPI and
> implementation.
Running some tests might give you a better feel for m2m - export buf - drm
cases without fences on exynos. This could help understand the performance
gains with fences.
>
>> Javier wrote some patches last year meant to implement implicit
>> fences support. What we noticed is that, while his mechanism worked
>> fine for pure capture and pure output devices, when we added a mem2mem
>> device, on a DMABUF+fences pipeline, e. g.:
>>
>> sensor -> [m2m] -> DRM
>>
>> End everything using fences/DMABUF, the fences mechanism caused dead
>> locks on existing userspace apps.
>>
>> A m2m device has both capture and output devnodes. Both should be
>> queued/dequeued. The capture queue is synchronized internally at the
>> driver with the output buffer[1].
>>
>> [1] The names here are counter-intuitive: "capture" is a devnode
>> where userspace receives a video stream; "output" is a devnode where
>> userspace feeds a video stream.
>>
>> The problem is that adding implicit fences changed the behavior of
>> the ioctls, causing gstreamer to wait forever for buffers to be ready.
>>
>
> The problem was related to trying to make user-space unaware of the implicit
> fences support, and so it tried to QBUF a buffer that had already a pending
> fence. A workaround was to block the second QBUF ioctl if the buffer had a
> pending fence, but this caused the mentioned deadlock since GStreamer wasn't
> expecting the QBUF ioctl to block.
>
>> I suspect that, even with explicit fences, the behavior of Q/DQ
>> will be incompatible with the current behavior (or will require some
>> dirty hacks to make it identical).
One big difference between implicit and explicit is that use-space is aware
of fences in the case of explicit. Can that knowledge be helpful in avoiding
the the problems we have seen with explicit?
>>
>> So, IMHO, the best would be to use a new set of ioctls, when fences are
>> used (like VIDIOC_QFENCE/DQFENCE).
>>
>
> For explicit you can check if there's an input-fence so is different than
> implicit, but still I agree that it would be better to have specific ioctls.
>
It would be nice if we can avoid adding more ioctls. As I mentioned earlier,
user-space is aware that fences are in use. Can we key off of that and make
it a queue property to be used to change the user-space action for fence vs.
no fence case?
<snip.>
thanks,
-- Shuah
next prev parent reply other threads:[~2017-04-03 20:48 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-13 19:20 [RFC 00/10] V4L2 explicit synchronization support Gustavo Padovan
2017-03-13 19:20 ` [RFC 01/10] [media] vb2: add explicit fence user API Gustavo Padovan
2017-04-03 9:48 ` Philipp Zabel
2017-04-05 14:08 ` Gustavo Padovan
2017-03-13 19:20 ` [RFC 02/10] [media] vb2: split out queueing from vb_core_qbuf() Gustavo Padovan
2017-03-13 19:20 ` [RFC 03/10] [media] vb2: add in-fence support to QBUF Gustavo Padovan
2017-04-03 18:27 ` Javier Martinez Canillas
2017-03-13 19:20 ` [RFC 04/10] [media] uvc: enable subscriptions to other events Gustavo Padovan
2017-03-13 19:20 ` [RFC 05/10] [media] vivid: assign the specific device to the vb2_queue->dev Gustavo Padovan
2017-03-13 19:20 ` [RFC 06/10] [media] v4l: add V4L2_EVENT_BUF_QUEUED event Gustavo Padovan
2017-03-13 19:20 ` [RFC 07/10] [media] v4l: add support to BUF_QUEUED event Gustavo Padovan
2017-03-13 19:20 ` [RFC 08/10] [media] vb2: add videobuf2 dma-buf fence helpers Gustavo Padovan
2017-03-13 19:20 ` [RFC 09/10] [media] vb2: add infrastructure to support out-fences Gustavo Padovan
2017-03-13 19:20 ` [RFC 10/10] [media] vb2: add out-fence support to QBUF Gustavo Padovan
2017-04-03 11:16 ` [RFC 00/10] V4L2 explicit synchronization support Mauro Carvalho Chehab
2017-04-03 19:46 ` Javier Martinez Canillas
2017-04-03 20:48 ` Shuah Khan [this message]
2017-04-05 15:09 ` Gustavo Padovan
2017-04-05 17:12 ` Javier Martinez Canillas
2017-04-06 14:08 ` Gustavo Padovan
2017-04-06 14:35 ` Javier Martinez Canillas
2017-06-09 15:38 ` Nicolas Dufresne
2017-04-04 11:34 ` Sakari Ailus
2017-04-05 15:24 ` Gustavo Padovan
2017-04-05 20:43 ` Sakari Ailus
2017-05-25 0:31 ` Gustavo Padovan
2017-06-08 20:17 ` Mauro Carvalho Chehab
2017-06-08 21:36 ` Shuah Khan
2017-06-09 6:25 ` Gustavo Padovan
2017-06-09 16:09 ` Shuah Khan
2017-06-09 6:15 ` Gustavo Padovan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKocOOPAvkDpNXiFqsr=jxOvq2589piRV+HTMOoSy8gmZ-M67Q@mail.gmail.com' \
--to=shuahkhan@gmail.com \
--cc=gustavo.padovan@collabora.com \
--cc=gustavo@padovan.org \
--cc=hverkuil@xs4all.nl \
--cc=javier@osg.samsung.com \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=shuahkh@osg.samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).