[Mesa-dev] [PATCH 2/2] egl/dri2: don't require a context for ClientWaitSync

[Albert Freeman]

On 28 September 2015 at 08:10, Marek Olšák <maraeo at gmail.com> wrote:
> On Mon, Sep 28, 2015 at 7:20 AM, Albert Freeman
> <albertwdfreeman at gmail.com> wrote:
>> On 28 September 2015 at 14:38, Albert Freeman <albertwdfreeman at gmail.com> wrote:
>>> On 28 September 2015 at 04:12, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>> On 26 September 2015 at 00:49, Marek Olšák <maraeo at gmail.com> wrote:
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> The spec doesn't require it. This fixes a crash on Android.
>>>>>
>>>> Nicely spotted Marek.
>>>>
>>>> A couple of related (not supposed to be part of this patch) notes:
>>>>  - This will just move the crash (from libEGL to i965_dri.so) for i965
>>>> hardware :)
>>> I rediscovered this before I read the email related to this patch
>>> (where Marek mentions it): "Re: [Mesa-dev] Pending issues of
>>> lollipop-x86".
>>> Who (if anyone) should we CC this to?
>>>>  - We really shouldn't be setting the flags if ctx is NULL, as per the spec.
>>>>
>>>>     "If no context is
>>>>     current for the bound API, the EGL_SYNC_FLUSH_COMMANDS_BIT_KHR bit
>>>>     is ignored."
>>> Maybe handle this in the driver (probably state_tracker in the case of
>>> gallium) level. Both here and drivers?
>> OpenGL ClientWaitSync is very clear about SYNC_FLUSH_COMMANDS_BIT only
>> flushing the GL context when the fence sync is created (it doesn’t
>> seem to require it at the moment wait sync is called (probably to
>> leave room for drivers to optimise)). EGL is a lot less clear, it
>> could be interpreted as behaving just like GL, but to someone writing
>> an EGL program it could be easily interpreted as "the flush() must
>> occur when eglClientWaitSync is called with SYNC_FLUSH_COMMANDS_BIT".
>>
>> The current behaviour of gallium (not sure about i965) is to always
>> flush() when creating the sync object (hence always behave as
>> SYNC_FLUSH_COMMANDS_BIT is set according to GL). However
>> implementations are allowed to behave as if flush was not required (in
>> GL and I am assuming EGL). So technically the current behaviour of the
>> gallium dri state tracker could reduce performance (since flushing
>> tends to do that). It however doesn’t actually violate the EGL spec.
>> Perhaps some heuristics on when to flush would be a good solution?
>
> Flushing is currently required for creating fences. This is a gallium
> design decision as well as radeon/amdgpu kernel driver decision. It's
> simply not possible to put a fence anywhere in the GPU command buffer.
> We can only submit a command buffer to the kernel and then get a fence
> tracking execution of the whole command buffer.
>
> Marek
Thanks for the clarification.