[PATCH v2] wayland: Add support for eglSwapInterval

[Pekka Paalanen]

On Fri, 13 Sep 2013 16:10:25 +0100
Neil Roberts <neil at linux.intel.com> wrote:

> Thanks for the explanation. I wasn't considering the fullscreen case
> where the compositor is directly scanning out from the client's buffers.
> I think for the non-fullscreen case the compositor would only hold on to
> a single buffer, right? In that case the sync request is enough.

It depends. If a client commits a buffer while the compositor is already
waiting for the previous repaint and pageflip to complete, the
compositor cannot release the old buffer, because the GPU might still
be compositing from it, even in the GL case (texturing). (I don't know
if DRM internally has any ordering guarantees here. If it has, it
should be explicitly written out in comments in the relevant Mesa code.
Developers of other EGL stacks look into Mesa for examples.)

I am also not sure what the current Weston implementation does in this
situation.

Also, there are platforms (rpi) where all non-shm buffers are scanout
buffers. Would be nice to get some guidelines here, where and what
implementation are usually expected to wait on. And on rpi we really do
not have too much memory available.

> I think in an ideal world you would still only need three buffers in the
> fullscreen case. Ie, you have one buffer that is currently used for
> scanout, one buffer that is pending and one buffer that you are
> rendering to. When you are finished rendering to that last buffer
> ideally the compositor would be able to cancel that pending buffer and
> instead queue the new buffer. As far as I understand, there is currently
> no way to cancel that pending buffer in KMS so that is why we need a
> fourth buffer.

Right. (Ignoring the additional case of synchronized sub-surface.)

> I'm not sure whether using four or more buffers is a good idea because
> that seems like quite a lot considering these are fullscreen buffers.
> Maybe in that case we should just give up and block. I guess it depends
> on whether we expect to be able to change KMS to allow cancelling the
> pending buffer, but it sounds like that could be quite hard.

It depends on how far you want to go with the swapinterval=0 case. If
an application is indeed going to max out the fps at any cost, then
I think we should do that. And if an application is using
swapinterval=0 only as means to avoid blocking in EGL while throttling
itself to frame callbacks or something, it probably won't ever allocate
so many buffers.

On a PC platform, I'd easily tend to say let's have 10 buffers slots,
allocate and free them as needed, and only block when we run out.
Anyone requesting swapiterval=0 would get exactly what they want, since
rendering a lot more frames than a compositor can show in one output
frame time is silly to begin with. It is quite convoluted to figure out
the minimum required number of buffer slots to avoid blocking on
compositor refresh, and it can differ between compositors.

You might also be able to free extra buffers, that is shrink and not
only grow the buffer pool. Does that happen in Mesa btw?

But, that's just hand-waving. T think I'd prefer testing with different
numbers in different scanout situations.

> As for flushing the delayed buffer release event, maybe we could add a
> way to disable the event queuing if Weston knows that the client doesn't
> have a frame callback installed? I am about to attach two patches that
> would implement that.

I can't say if that is a good heuristic or not, to assume that surfaces
where a client is not interested in frame callbacks need buffer
releases ASAP. Apart from an explicit request, I have no better
suggestions.


Thanks,
pq

> Pekka Paalanen <ppaalanen at gmail.com> writes:
> 
> > No, you generally don't get the release event as an immediate reply to a
> > commit request.
> >
> > 1. When you commit a new buffer, compositor schedules a repaint, and
> > continues processing your requests. If you simply asked for one
> > wl_display.sync right after the commit, it is acked now.
> >
> > 2. Some time later, the compositor actually enters repaint. It submits
> > a command stream to the GPU, and schedules a page flip onto the screen.
> > At this point the compositor sends the frame callbacks, if you
> > requested one. Then the compositor continues processing requests in the
> > mean time.
> >
> > 3. Some more time later, the GPU is done, and the page flip is done.
> > The compositor gets an async notification that the page flip is
> > complete. Only at *this point* is the compositor able to send a release
> > for the old buffers.
> >
> > So you see, step 3 comes much later than your sync for the commit. The
> > compositor cannot signal the release of any buffers before the page
> > flip is done, because the GPU or scanout may be using the buffers
> > still, and you risk re-using a buffer while it being read.
> >
> > The step 3 is pratically guaranteed to come, the wait for it is not
> > indefinite, but it is not immediate either. It is not synchronized to
> > the Wayland protocol, like many other replies are.
> >
> > The only time when you could get the release for the old buffer as an
> > immediate reply is, when the old buffer has never been on screen yet.
> > Provided you also did a wl_display.sync after the commit.
> >
> > Also don't forget about sub-surfaces, which may hold one additional
> > buffer indefinitely (until the parent surface is committed).
> >
> > Unfortunately, I do not have any good suggestions how to solve this,
> > other than sending the release with 'post', not 'queue'. Could we
> > perhaps use the hypothetical presentation time stamp event, that would
> > be fired at step 3, to flush out the release events for a client?
> >
> > (To complement the frame callback which tells when a client should
> > start preparing a new frame, there was a plan to add a presentation
> > timestamp callback to be used for AV-sync etc. telling the time the
> > frame actually turned into light. I don't know what the status of that
> > proposal is nowadays.)
> >
> >
> > Hmm, or could we simply rely on the "never been on screen" behaviour?
> > Does that make the wl_display.sync proposition work after all?
> > I'm a little sceptical, but can't think far enough...
> >
> > Re-thinking again, I would need to re-check the code whether the "never
> > been on screen" actually applies for non-sub-surfaces. In theory it
> > should.
> >
> >
> > Thanks,
> > pq
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ
> VAT No: 860 2173 47
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel