[Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

[Jonas Ådahl]

On Thu, Nov 10, 2016 at 10:38:51AM +0000, Daniel Stone wrote:
> Hi,
> 
> On 10 November 2016 at 06:08, Jonas Ådahl <jadahl at gmail.com> wrote:
> > On Mon, Oct 24, 2016 at 08:42:59PM +0100, Daniel Stone wrote:
> >> The intent of moving blocking from SwapBuffers to get_back_bo, was to
> >> avoid unnecessary triple-buffering by ensuring that the compositor had
> >> fully processed the previous frame before we started rendering. This
> >> means that the only time we would have to resort to triple-buffering
> >> would be when the buffer is directly scanned out, thus saving an extra
> >> buffer for composition anyway.
> >>
> >> The 'repaint window' changes introduced in Weston since then, however,
> >> have narrowed the window of time between the frame event being sent and
> >> the repaint loop needing to conclude, to 7ms by default, in order to
> >> reduce latency. This means however that blocking in get_back_bo gives a
> >> maximum of 7ms for the entire GL submission to begin and complete.
> >>
> >> Not only this, but if a client is using buffer_age to avoid full
> >> repaints, the buffer-age request will stall in get_back_bo until the
> >> frame callback completes, meaning that the client cannot even calculate
> >> the repaint area before the 7ms window.
> >>
> >> The combination of the two meant that WebKit-GTK+ was failing to
> >> achieve full framerate on a Minnowboard, due to spending a great deal of
> >> its time attempting to query the age of the next buffer before redraw.
> >>
> >> Revert to the previous behaviour of allowing rendering to begin but
> >> delaying SwapBuffers, unless and until we can find a more gentle
> >> behaviour.
> >>
> >> Signed-off-by: Daniel Stone <daniels at collabora.com>
> >> Cc: Neil Roberts <neil at linux.intel.com>
> >> Cc: Kristian Høgsberg <krh at bitplanet.net>
> >
> > The explanations sounds sane to me, and even though it's a revert, I
> > looked at the patch and it seems correct, so this is
> >
> > Reviewed-by: Jonas Ådahl <jadahl at gmail.com>
> >
> > I suspect we can't avoid tripple buffering if the client asks for
> > information about the next buffer early enough, since mesa won't have a
> > clue what the future will be regarding any buffer being released or not
> > before the frame callback.
> 
> Thanks both for the review/testing - pushed now.
> 
> > Or maybe we could possibly fake the buffer age, pretending the buffer is
> > a fresh one if the client queries "too early", but then eventually
> > re-use the buffer had it been released before the frame callback. We'd
> > at least avoid the extra buffer being allocated, while instead only
> > wasting resources painting the whole buffer which we would do anyway if
> > we allocated the third buffer.
> 
> That relies on the compositor releasing buffers in a strictly FIFO
> manner though, which is ... dangerous.

Is FIFO an issue here? Lets assume it looks like this:

get_next_fb_age() {
  fb = get_released_fb();
  if (fb)
    return fb.age;
  else
    return 0;
}

get_next_fb() {
  fb = get_released_fb();
  if (fb)
    return fb;
  else
    return create_new_buffer();
}

In other terms, if we don't have a buffer released at the point we
query, we say its a new buffer, making the application assume it needs
to redraw everything. When we then actually need the buffer for doing
things, we see if there is any buffer to reuse. What buffer was actually
released doesn't matter, since the application will redraw everything
anyway, and if we actually did reuse a buffer, the previously returned
buffer age was a lie, but a harmless one, and if we eventually actually
did allocate a new one, it wouldn't even be a lie.


Jonas

> 
> If you're interested in buffer age though, here's another one:
> https://lists.freedesktop.org/archives/mesa-dev/2016-November/134787.html
> 
> Cheers,
> Daniel