[Mesa-dev] [PATCH] loader/dri3: Always use at least two back buffers

[Jason Ekstrand]

On Sep 5, 2016 8:43 PM, "Michel Dänzer" <michel at daenzer.net> wrote:
>
> On 06/09/16 12:08 PM, Jason Ekstrand wrote:
> > On Mon, Sep 5, 2016 at 7:39 PM, Michel Dänzer <michel at daenzer.net
> > <mailto:michel at daenzer.net>> wrote:
> >
> >     From: Michel Dänzer <michel.daenzer at amd.com
> >     <mailto:michel.daenzer at amd.com>>
> >
> >     This can make a significant difference for performance with some
extreme
> >     test cases such as vblank_mode=0 glxgears.
> >
> >     Fixes: 1e3218bc5ba2 ("loader/dri3: Overhaul dri3_update_num_back")
> >     Cc: "12.0 11.2" <mesa-stable at lists.freedesktop.org
> >     <mailto:mesa-stable at lists.freedesktop.org>>
> >     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97549
> >     <https://bugs.freedesktop.org/show_bug.cgi?id=97549>
> >     Signed-off-by: Michel Dänzer <michel.daenzer at amd.com
> >     <mailto:michel.daenzer at amd.com>>
> >     ---
> >
> >     I could swear I tested vblank_mode=0 glxgears with my previous
change
> >     and couldn't measure any difference, but I can now, so I must have
> >     messed up my previous testing somehow... Apologies for any
inconvenience
> >     this caused.
> >
> >      src/loader/loader_dri3_helper.c | 4 +---
> >      1 file changed, 1 insertion(+), 3 deletions(-)
> >
> >     diff --git a/src/loader/loader_dri3_helper.c
> >     b/src/loader/loader_dri3_helper.c
> >     index 86ae5ae..3ce0352 100644
> >     --- a/src/loader/loader_dri3_helper.c
> >     +++ b/src/loader/loader_dri3_helper.c
> >     @@ -70,10 +70,8 @@ dri3_update_num_back(struct loader_dri3_drawable
> >     *draw)
> >      {
> >         if (draw->flipping)
> >            draw->num_back = 3;
> >     -   else if (draw->vtable->get_swap_interval(draw) != 0)
> >     -      draw->num_back = 2;
> >         else
> >     -      draw->num_back = 1;
> >     +      draw->num_back = 2;
> >
> >
> > With this change, the function is logically identical to the old
> > function with the async flipping case removed.
>
> Not sure what you mean exactly, but I hope we can agree that the code is
> easier to understand and reason about now. :)

I mean that you original patch made two functional changes: One was to drop
quadbuffering and the second was to single-buffer in the swapinterval==0
case.  This effectively reverts the second functional change while leaving
the first intact.

Yes, it's much easier to read now.  One could argue that the "counting
buffers" approach of the original code had it's merits, but it is much
easier to look at the new code and see what it will do in any particular
case.

> > I'm still not sure why we aren't getting a stall with 3 butters in the
> > swapinterval=0 flipping case, but if it doesn't have a perf impact, it
must
> > not be a problem.
>
> Right, there is a stall, but it doesn't seem significant, at least on my
> setup.
>
>
> > Reviewed-by: Jason Ekstrand <jason at jlekstrand.net
> > <mailto:jason at jlekstrand.net>>
>
> Thanks!
>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160905/c2c359a6/attachment.html>