LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Brian Starkey <brian.starkey@arm.com>
To: Ezequiel Garcia <ezequiel@collabora.com>
Cc: linux-media@vger.kernel.org, kernel@collabora.com,
	Hans Verkuil <hverkuil@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Shuah Khan <shuahkh@osg.samsung.com>,
	Pawel Osciak <pawel@osciak.com>,
	Alexandre Courbot <acourbot@chromium.org>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	linux-kernel@vger.kernel.org,
	Gustavo Padovan <gustavo.padovan@collabora.com>
Subject: Re: [PATCH v9 11/15] vb2: add in-fence support to QBUF
Date: Wed, 9 May 2018 17:45:05 +0100	[thread overview]
Message-ID: <20180509164504.GB23664@e107564-lin.cambridge.arm.com> (raw)
In-Reply-To: <fca78137c285eff268b0319ca752014c9f4e76fc.camel@collabora.com>

On Wed, May 09, 2018 at 01:03:15PM -0300, Ezequiel Garcia wrote:
>On Wed, 2018-05-09 at 11:36 +0100, Brian Starkey wrote:
>
>[..]
>> > @@ -203,9 +215,14 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
>> > 	b->timestamp = ns_to_timeval(vb->timestamp);
>> > 	b->timecode = vbuf->timecode;
>> > 	b->sequence = vbuf->sequence;
>> > -	b->fence_fd = 0;
>> > 	b->reserved = 0;
>> >
>> > +	b->fence_fd = 0;
>>
>> I didn't understand why we're returning 0 instead of -1. Actually the
>> doc in patch 10 seems to say it will be -1 or 0 depending on whether
>> we set one of the fence flags? I'm not sure:
>>
>>     For all other ioctls V4L2 sets this field to -1 if
>>     ``V4L2_BUF_FLAG_IN_FENCE`` and/or ``V4L2_BUF_FLAG_OUT_FENCE`` are set,
>>     otherwise this field is set to 0 for backward compatibility.
>>
>
>Well, I think that for backwards compatibility (userspace not knowing
>about fence_fd field), we should return 0, unless the flags are explicitly
>set.
>
>That is what the doc says and it sounds sane.

On the line below where you snipped, is this:

+      if (vb->in_fence)
+              b->flags |= V4L2_BUF_FLAG_IN_FENCE;
+      else
+              b->flags &= ~V4L2_BUF_FLAG_IN_FENCE;

If the "if (vb->in_fence)" condition is true, then the flag is set,
and the fence_fd field is 0. I think that's the opposite of what the
doc says:

    For all other ioctls V4L2 sets this field to -1 if
    ``V4L2_BUF_FLAG_IN_FENCE`` and/or ``V4L2_BUF_FLAG_OUT_FENCE`` are set,
    otherwise this field is set to 0 for backward compatibility.

V4L2_BUF_FLAG_IN_FENCE is set, therefore the doc says V4L2 will set
this field to -1. (Or at least the comment should be made less
ambiguous).

>
>The bits are implemented in patch 12, but as I mentioned in my reply to
>patch 10, I will move it to patch 10, for consistency.

Yeah as you say, it looks like you change this behaviour in path 12,
so I'm not totally sure which is right or expected. But consistency is
good :-)

-Brian

>
>Thanks,
>Eze

  reply	other threads:[~2018-05-09 16:45 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-04 20:05 [PATCH v9 00/15] V4L2 Explicit Synchronization Ezequiel Garcia
2018-05-04 20:05 ` [PATCH v9 01/15] xilinx: regroup caps on querycap Ezequiel Garcia
2018-05-04 20:05 ` [PATCH v9 02/15] hackrf: group device capabilities Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 03/15] omap3isp: " Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 04/15] vb2: move vb2_ops functions to videobuf2-core.[ch] Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 05/15] vb2: add unordered vb2_queue property for drivers Ezequiel Garcia
2018-05-07 11:03   ` Hans Verkuil
2018-05-04 20:06 ` [PATCH v9 06/15] v4l: add unordered flag to format description ioctl Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 07/15] v4l: mark unordered formats Ezequiel Garcia
2018-05-07 13:45   ` Hans Verkuil
2018-05-04 20:06 ` [PATCH v9 08/15] cobalt: set queue as unordered Ezequiel Garcia
2018-05-07 11:04   ` Hans Verkuil
2018-05-04 20:06 ` [PATCH v9 09/15] vb2: mark codec drivers " Ezequiel Garcia
2018-05-07 11:02   ` Hans Verkuil
2018-05-04 20:06 ` [PATCH v9 10/15] vb2: add explicit fence user API Ezequiel Garcia
2018-05-07 11:30   ` Hans Verkuil
2018-05-09 10:33   ` Brian Starkey
2018-05-09 15:52     ` Ezequiel Garcia
2018-05-09 16:33       ` Brian Starkey
2018-05-09 19:10         ` Ezequiel Garcia
2018-05-09 19:40         ` Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 11/15] vb2: add in-fence support to QBUF Ezequiel Garcia
2018-05-07 12:07   ` Hans Verkuil
2018-05-08 19:16     ` Ezequiel Garcia
2018-05-09  7:04       ` Hans Verkuil
2018-05-08 23:18     ` Gustavo Padovan
2018-05-09 10:35       ` Brian Starkey
2018-05-09 10:36   ` Brian Starkey
2018-05-09 10:52     ` Hans Verkuil
2018-05-09 16:03     ` Ezequiel Garcia
2018-05-09 16:45       ` Brian Starkey [this message]
2018-05-04 20:06 ` [PATCH v9 12/15] vb2: add out-fence " Ezequiel Garcia
2018-05-07 12:29   ` Hans Verkuil
2018-05-09 10:37   ` Brian Starkey
2018-05-04 20:06 ` [PATCH v9 13/15] v4l: introduce the fences capability Ezequiel Garcia
2018-05-04 20:06 ` [PATCH v9 14/15] v4l: Add V4L2_CAP_FENCES to drivers Ezequiel Garcia
2018-05-07 12:42   ` Hans Verkuil
2018-05-04 20:06 ` [PATCH v9 15/15] v4l: Document explicit synchronization behavior Ezequiel Garcia
2018-05-07 12:51   ` Hans Verkuil

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=20180509164504.GB23664@e107564-lin.cambridge.arm.com \
    --to=brian.starkey@arm.com \
    --cc=acourbot@chromium.org \
    --cc=ezequiel@collabora.com \
    --cc=gustavo.padovan@collabora.com \
    --cc=hverkuil@xs4all.nl \
    --cc=kernel@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@osg.samsung.com \
    --cc=pawel@osciak.com \
    --cc=sakari.ailus@iki.fi \
    --cc=shuahkh@osg.samsung.com \
    --subject='Re: [PATCH v9 11/15] vb2: add in-fence support to QBUF' \
    /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

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).