Date: Thu, 20 Oct 2016 22:15:20 +0300
From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= <ville.syrjala@linux.intel.com>
To: Gustavo Padovan <gustavo.padovan@collabora.com>
Cc: marcheu@google.com, Daniel Stone <daniels@collabora.com>,
        seanpaul@google.com, Daniel Vetter <daniel.vetter@ffwll.ch>,
        linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
        laurent.pinchart@ideasonboard.com,
        Gustavo Padovan <gustavo.padovan@collabora.co.uk>,
        John Harrison <John.C.Harrison@Intel.com>, m.chehab@samsung.com
Subject: Re: [PATCH v5 4/4] drm/fence: add out-fences support
Message-ID: <20161020191520.GZ4329@intel.com>
References: <1476975005-30441-1-git-send-email-gustavo@padovan.org>
 <1476975005-30441-5-git-send-email-gustavo@padovan.org>
 <20161020153518.GX4329@intel.com>
 <20161020155538.GA10205@joana>
 <20161020163444.GY4329@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20161020163444.GY4329@intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3926
Lines: 108

On Thu, Oct 20, 2016 at 07:34:44PM +0300, Ville Syrj?l? wrote:
> On Thu, Oct 20, 2016 at 01:55:38PM -0200, Gustavo Padovan wrote:
> > 2016-10-20 Ville Syrj?l? <ville.syrjala@linux.intel.com>:
> > 
> > > On Thu, Oct 20, 2016 at 12:50:05PM -0200, Gustavo Padovan wrote:
> > > > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > > > 
> > > > Support DRM out-fences by creating a sync_file with a fence for each CRTC
> > > > that sets the OUT_FENCE_PTR property.
> > > 
> > > I still maintain the out fence should also be per fb (well, per plane
> > > since we can't add it to fbs). Otherwise it's not going to be at all
> > > useful if you do fps>vrefresh and don't include the same set of planes
> > > in every atomic ioctl, eg. if you only ever include ones that are
> > > somehow dirty.
> > 
> > How would the kernel signal these dirty planes then? Right now we signal
> > at the vblank.
> 
> So if I think about it in terms of the previous fbs something like this
> comes to mind:
> 
>  starting point:
>  plane a, fb 0
>  plane b, fb 1
> 
>  ioctl:
>  plane a, fb 2, fence A
>  plane b, fb 3, fence B
> 
>  ioctl:
>  plane a, fb 4, fence C
>  -> fb 2 can be reused, so fence C should signal immediately?
> 
>  vblank:
>  -> fb 0,1 can be reused, so fence A,B signal?
> 
> It feels a bit wonky since the fence is effectively associated with the
> previous fb after the previous fb was already submitted to the kernel.
> One might assume fence A to be the one signalled early, but that would
> give the impression that fb 0 would be free for reuse when it's not.

OK, so after hashing this out on irc with Gustavo and Brian, we came
to the conclusion that the per-crtc out fence should be sufficient
after all.

The way it can work is that the first ioctl will return the fence,
doesn't really matter how many of the planes are involved in this
ioctl. Subsequent ioctls that manage to get in before the next
vblank will get back a -1 as the fence, even if the set if planes
involved is not the same as in the first ioctl. From the -1
userspace can tell what happened, and can then consult its own
records to see which still pending flips were overwritten, and
thus knows which buffers can be reused immediately.

If userspace plans on sending out the now free buffers to someone
else accompanied by a fence, it can just create an already signalled
fence to send instead of sending the fence it got back from the
atomic ioctl since that one is still associated with the original
scanout buffers.

The fence returned by the atomic ioctl will only signal after the
vblank, at which point userspace will obviously know that the
orginal scanout buffers are now also ready for reuse, and that
the last buffer submitted for each plane is now being actively
scanned out. So if userspace wants to send out some of the
original buffers to someone else, it can send along the fence
returned from the atomic ioctl.

So requires a bit more work from userspace perhaps. But obviously
it only has to do this if it plans on submitting multiple operations
per frame.

So a slightly expanded version of my previous example might look like:
starting point:
 plane a, fb 0
 plane b, fb 1

ioctl:
 crtc: fence A
 plane a, fb 2
 plane b, fb 3

ioctl:
 crtc: fence -1
 plane a, fb 4
 -> the previously submitted fb for plane a (fb 2) can be reused

ioctl:
 crtc: fence -1
 plane a, fb 5
 -> the previously submitted fb for plane a (fb 4) can be reused

vblank:
 -> fence A signals, fb 0,1 can be reused
    fb 3 and 5 being scanned out now


We also had a quick side track w.r.t. variable refresh rate displays,
and I think the conclusion was that there the vblank may just happen
sooner or later. Nothing else should change. What's unclear is how
the refresh rate would be controlled. The two obvious options are
explicit control, and automagic based on the submit rate. But that's
a separate topic entirely.

-- 
Ville Syrj?l?
Intel OTC