On Sep 5, 2016 8:43 PM, "Michel Dänzer" <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>> wrote: > > On 06/09/16 12:08 PM, Jason Ekstrand wrote: > > On Mon, Sep 5, 2016 at 7:39 PM, Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a> > > <mailto:<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>>> wrote: > > > > From: Michel Dänzer <<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a> > > <mailto:<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a>>> > > > > 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" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a> > > <mailto:<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>>> > > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=97549">https://bugs.freedesktop.org/show_bug.cgi?id=97549</a> > > <<a href="https://bugs.freedesktop.org/show_bug.cgi?id=97549">https://bugs.freedesktop.org/show_bug.cgi?id=97549</a>> > > Signed-off-by: Michel Dänzer <<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a> > > <mailto:<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a>>> > > --- > > > > 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 <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a> > > <mailto:<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>>> > > Thanks! > > > -- > Earthling Michel Dänzer | <a href="http://www.amd.com">http://www.amd.com</a> > Libre software enthusiast | Mesa and X developer