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

[Albert Freeman]

On 29 September 2015 at 07:25, Albert Freeman <albertwdfreeman at gmail.com> wrote:
> 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.
Also when I said "Can easily be reverted later if a better solution is
found. :)". It seemed to me that this was an issue better resolved in
the code outside of mesa that had the issue.