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

[Michel Dänzer]

On 06/09/16 01:59 PM, Jason Ekstrand wrote:
> On Sep 5, 2016 8:43 PM, "Michel Dänzer" <michel at daenzer.net
> <mailto: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>
>> > <mailto: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>
>> >     <mailto: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>
>> >     <mailto: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>
>> >     <mailto: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

As I explained, there was really no quad-buffering in the first place.


> and the second was to single-buffer in the swapinterval==0 case.
> This effectively reverts the second functional change while leaving
> the first intact.

That's not the only remaining change either. E.g. before my changes,
flipping with swap interval > 0 was using only two buffers, now it's
three. (That was actually the main motivation for my first change)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer