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

[Michel Dänzer]

On 06/09/16 03:14 PM, Michel Dänzer wrote:
> 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)

To be precise, this change only applies when the X driver supports async
flips, otherwise it was already using three buffers before. (Note that
async flips are never used with swap interval > 0)


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