[Mesa-dev] [PATCH 0/5] dri3, gallium: Correctness and performance fixes

[Thomas Hellstrom]

On 06/30/2017 03:35 PM, Brian Paul wrote:
> On 06/28/2017 11:07 AM, Thomas Hellstrom wrote:
>> On 06/22/2017 12:42 PM, Thomas Hellstrom wrote:
>>> A patch series that deals with dri3 correctness- and performance fixes.
>>>
>>> The corectness fixes attempts to deal with the fact that we need to
>>> wait for
>>> all pending swapbuffers before we touch the front buffer. Otherwise a
>>> front buffer change may be overwritten by a pending swapbuffer when it
>>> was actually intended to be drawn *after* the swapbuffer. Also a post
>>> swapbuffer front read could actually occur *before* the swapbuffer.
>>>
>>> Patch 1 deals with the dri3 internal synchronization. All frontbuffer
>>> accesses introduce a "swapbuffer barrier" to order with respect to
>>> pending swapbuffers.
>>> The exception is _WaitX because if we call _WaitX we're ordering with
>>> respect to X rendering and if there are pending swapbuffers, an
>>> application
>>> would already have called _WaitGL to be able to do the X rendering
>>> correctly,
>>> and _WaitGL is ordering with respect to pending swapbuffers. This patch
>>> fixes the piglit copysubbuffer test.
>>>
>>> Patch 2 to 4 deals with having glFinish() order with respect to pending
>>> swapbuffers. The behaviour is actually not correct in that it doesn't
>>> wait
>>> for the pending swapbuffers to complete, but a user shouldn't be 
>>> able to
>>> tell the difference. This patch series is motivated by the fact that 
>>> the
>>> glXWaitGL man page states that glFinish() can be used instead of
>>> glXWaitGL,
>>> and without this series it can't. The functionality is only
>>> implemented for
>>> gallium. Other drivers need to provide their own implementation. 
>>> Ideally
>>> we should have accomplished this without the dri interface changes by
>>> calling an unconditional flush_frontbuffer, but at least the gallium
>>> flush_frontbuffer implementation is relying on us having a fake front
>>> which
>>> is not always the case.
>>>
>>> Patch 5 replaces the back-to-fake-front full buffer copies with a swap
>>> during swapbuffers. Should be saving a lot of work when we actually
>>> have a
>>> fake front. There were some conserns raised when this was posted as an
>>> RFC
>>> that the separate-server-gpu case would be broken as well as the buffer
>>> age functionality. I've audited the code and I think that's not the 
>>> case.
>>> The piglit buffer age test still reports a pass after this change. The
>>> separate-server-gpu case I guess needs additional testing.
>>>
>>> Tested wih piglit -quick without regressions.
>>
>> Hi!
>>
>> Patches 1,2 and 5 are still unreviewed. It would be good to have at
>> least a pair of extra eyes on them.
>
> Looks like Emil had an EGL concern with patch 5.  Otherwise, I'm not a 
> DRI3 expert but the code looks OK to me.
>
> Reviewed-by: Brian Paul <brianp at vmware.com>
>
>
>
Thanks, Brian.

I won't push patch 5 until after the vacation when I can be there to 
rapidly back it out if it breaks something. Also I need to fix up the 
EGL breakage.

/Thomas