linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brian Starkey <brian.starkey@arm.com>
To: Gustavo Padovan <gustavo.padovan@collabora.com>
Cc: Gustavo Padovan <gustavo@padovan.org>,
	linux-media@vger.kernel.org, Hans Verkuil <hverkuil@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	linux-kernel@vger.kernel.org, Jonathan.Chai@arm.com
Subject: Re: [PATCH v3 00/15] V4L2 Explicit Synchronization support
Date: Fri, 13 Oct 2017 02:56:07 +0100	[thread overview]
Message-ID: <20171013015607.GA17049@e107564-lin.cambridge.arm.com> (raw)
In-Reply-To: <1507147735.2981.54.camel@collabora.com>

Hi,

On Wed, Oct 04, 2017 at 05:08:55PM -0300, Gustavo Padovan wrote:
>Hi Brian,
>
>On Mon, 2017-10-02 at 14:41 +0100, Brian Starkey wrote:
>> Hi Gustavo,
>>
>> On Thu, Sep 07, 2017 at 03:42:11PM -0300, Gustavo Padovan wrote:
>> > From: Gustavo Padovan <gustavo.padovan@collabora.com>
>> >
>> > Hi,
>> >
>> > Refer to the documentation on the first patch for the details. The
>> > previous
>> > iteration is here: https://www.mail-archive.com/linux-media@vger.ke
>> > rnel.org/msg118077.html
>> >
>> > The 2nd patch proposes an userspace API for fences, then on patch 3
>> > we
>> > prepare to the addition of in-fences in patch 4, by introducing the
>> > infrastructure on vb2 to wait on an in-fence signal before queueing
>> > the
>> > buffer in the driver.
>> >
>> > Patch 5 fix uvc v4l2 event handling and patch 6 configure q->dev
>> > for
>> > vivid drivers to enable to subscribe and dequeue events on it.
>> >
>> > Patches 7-9 enables support to notify BUF_QUEUED events, the event
>> > send
>> > to userspace the out-fence fd and the index of the buffer that was
>> > queued.
>> >
>> > Patches 10-11 add support to mark queues as ordered. Finally
>> > patches 12
>> > and 13 add more fence infrastructure to support out-fences, patch
>> > 13 exposes
>> > close_fd() and patch 14 adds support to out-fences.
>> >
>> > It only works for ordered queues for now, see open question at the
>> > end
>> > of the letter.
>> >
>> > Test tool can be found at:
>> > https://git.collabora.com/cgit/user/padovan/v4l2-test.git/
>> >
>> > Main Changes
>> > ------------
>> >
>> > * out-fences: change in behavior: the out-fence fd now comes out of
>> > the
>> > BUF_QUEUED event along with the buffer id.
>>
>> The more I think about this, the more unfortunate it seems.
>> Especially
>> for our use-case (m2m engine which sits in front of the display
>> processor to convert the format of some buffers), having to wait for
>> the in-fence to signal before we can get an out-fence removes a lot
>> of
>> the advantages of having fences at all.
>
>Does your m2m driver ensures ordering between the buffer queued to it?
>

I'm not so familiar with the code, how can I check that?

>>
>> Ideally, we'd like to queue up our m2m work (while the GPU is still
>> rendering that buffer, holding the in-fence), immediately get the
>> out-fence for the m2m work, and pass that to DRM as the in-fence for
>> display. With the current behaviour we need to wait in userspace
>> before we can pass the buffer to display.
>>
>> Wouldn't it be possible to enforce that the buffers aren't queued
>> out-of-order in VB2? An easy way might be to (in qbuf) set a buffer's
>> ->in_fence to be a fence_array of all the ->in_fences from the
>> buffers
>> before it in the queue (and its own). That would then naturally order
>> the enqueue-ing in the driver, and allow you to return the out-fence
>> immediately.
>>
>> This would also solve your output devices question from below - a
>> buffer can never get queued in the driver until all of the buffers
>> which were QBUF'd before it are queued in the driver.
>
>What you say makes sense, what this proposal lacks the most now is
>feedback regarding its usecases. We can create a control setting to
>enforce ordering in the queue, if it's set we create the fence arrays.
>For output devices this should be set by default.

Yeah that could work. I can see that in some cases queueing
out-of-order as the fences signal would be the right thing to do, so
makes sense to allow both.

Thanks,
-Brian

>
>Gustavo
>
>-- 
>Gustavo Padovan
>Principal Software Engineer
>Collabora Ltd.

      reply	other threads:[~2017-10-13  1:56 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-07 18:42 [PATCH v3 00/15] V4L2 Explicit Synchronization support Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 01/15] [media] v4l: Document explicit synchronization behaviour Gustavo Padovan
2017-09-08  2:39   ` Nicolas Dufresne
2017-09-11 10:50   ` Hans Verkuil
2017-09-11 11:01     ` Hans Verkuil
2017-09-11 13:18       ` Gustavo Padovan
2017-09-11 13:26         ` Hans Verkuil
2017-09-11 13:34           ` Gustavo Padovan
2017-09-11 13:35             ` Hans Verkuil
2017-09-07 18:42 ` [PATCH v3 02/15] [media] vb2: add explicit fence user API Gustavo Padovan
2017-09-11 10:55   ` Hans Verkuil
2017-09-11 11:03     ` Hans Verkuil
2017-10-02 13:42   ` Brian Starkey
2017-10-04 20:12     ` Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 03/15] [media] vb2: check earlier if stream can be started Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 04/15] [media] vb2: add in-fence support to QBUF Gustavo Padovan
2017-10-02 13:43   ` Brian Starkey
2017-09-07 18:42 ` [PATCH v3 05/15] [media] uvc: enable subscriptions to other events Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 06/15] [media] vivid: assign the specific device to the vb2_queue->dev Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 07/15] [media] v4l: add V4L2_EVENT_BUF_QUEUED event Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 08/15] [media] vb2: add .buffer_queued() to notify queueing in the driver Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 09/15] [media] v4l: add support to BUF_QUEUED event Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 10/15] [media] vb2: add 'ordered' property to queues Gustavo Padovan
2017-10-02 13:43   ` Brian Starkey
2017-10-04 20:18     ` Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 11/15] [media] vivid: mark vivid queues as ordered Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 12/15] [media] vb2: add videobuf2 dma-buf fence helpers Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 13/15] [media] vb2: add infrastructure to support out-fences Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 14/15] fs/files: export close_fd() symbol Gustavo Padovan
2017-09-07 18:51   ` Eric Biggers
2017-09-07 20:36   ` Al Viro
2017-09-07 21:22     ` Gustavo Padovan
2017-09-07 22:03       ` Al Viro
2017-09-07 22:09   ` Hans Verkuil
2017-09-07 22:18     ` Gustavo Padovan
2017-09-07 18:42 ` [PATCH v3 15/15] [media] vb2: add out-fence support to QBUF Gustavo Padovan
2017-10-02 13:41 ` [PATCH v3 00/15] V4L2 Explicit Synchronization support Brian Starkey
2017-10-04 20:08   ` Gustavo Padovan
2017-10-13  1:56     ` Brian Starkey [this message]

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=20171013015607.GA17049@e107564-lin.cambridge.arm.com \
    --to=brian.starkey@arm.com \
    --cc=Jonathan.Chai@arm.com \
    --cc=gustavo.padovan@collabora.com \
    --cc=gustavo@padovan.org \
    --cc=hverkuil@xs4all.nl \
    --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).