[Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS

[Tomasz Figa]

Hi Zhongmin,

On Wed, Jul 12, 2017 at 9:55 AM, Wu, Zhongmin <zhongmin.wu at intel.com> wrote:
> Thanks very much for Emil and Tomasz's suggestions.
>
> We can see that the out fence needed in queue buffer is get from batch buffer flushing.
>
> Now we  may flush bath buffer at below places:
>
> 1.   Create sync     ===> flush buffer and get out fence from driver
> 2.    Do glFLush()   ====>    flush buffer  ( not ask for out fence from driver)
> 3.    swapbuffer     ====>  flush buffer ( not ask for  out fence from drvier,   queue buffer with fd -1 )
> ...
>
>
> So,   in my opinion,  we don't need to get the out fence with help of other paths in swapbuffer. we just need to save the last out fence in the latest batch buffer flushing, and give it to Android with queue buffer in each swapbuffer.
>
> ( for example,   if glflush is just  called before swapbuffer,   the following buffer flushing will returned directly as the batch buffer is empty now, we can use the out fence got in glflush, and using create sync API may still not get the fence. )

Do we really need to use the same fence? If glFlush() was called just
before eglSwapBuffers(), there were no commands inserted in between
and the new fence would just be inserted directly after the one used
for glFlush(). I don't think it's an issue. (And the client is
actually doing something strange, because there is no need to call
glFlush() directly before eglSwapBuffers().)

>
> But you are right, we need to change the method in the patch, the patch is ill-considered.  As Emil said,  the canclebuffer is missed.
> We can't save the latest fence fd in the context object, as the context is reset yet before calling canclebuffer.  Then it is not good to add the call back in __DRI2flushExtensionRec,   We do need a better design for this.

How about my suggestion to use the DRI2 flush extension
(dri2_dpy->fence->create_fence_fd)? I think the following in
droid_swap_buffers could work:

   if (dri2_dpy->fence && dri2_dpy->fence->create_fence_fd)
      fence_fd = dri2_dpy->fence->create_fence_fd(ctx, -1);

I think you might also need to bypass flush drawable, as AFAICT it
would defeat the purpose of out-fence by waiting synchronously for the
GPU to finish.

Best regards,
Tomasz

>
> -----Original Message-----
> From: Emil Velikov [mailto:emil.l.velikov at gmail.com]
> Sent: Wednesday, July 12, 2017 1:41
> To: Tomasz Figa <tfiga at chromium.org>
> Cc: Wu, Zhongmin <zhongmin.wu at intel.com>; Marathe, Yogesh <yogesh.marathe at intel.com>; Widawsky, Benjamin <benjamin.widawsky at intel.com>; Liu, Zhiquan <zhiquan.liu at intel.com>; Eric Engestrom <eric at engestrom.ch>; Rob Clark <robclark at freedesktop.org>; Kenneth Graunke <kenneth at whitecape.org>; Kondapally, Kalyan <kalyan.kondapally at intel.com>; ML mesa-dev <mesa-dev at lists.freedesktop.org>; Timothy Arceri <tarceri at itsqueeze.com>; Kristian H . Kristensen <hoegsberg at chromium.org>; Chad Versace <chadversary at chromium.org>; dbehr at chromium.org
> Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
>
> On 11 July 2017 at 16:23, Tomasz Figa <tfiga at chromium.org> wrote:
>> Hi Zhongmin,
>>
>> On Tue, Jul 11, 2017 at 11:02 AM, Wu, Zhongmin <zhongmin.wu at intel.com> wrote:
>>> By the way,
>>>
>>> For cancelBuffer, sorry I forget such function, thanks for notice. It should also pass the same fence fd as the queuebuffer.
>>>
>>> And Yogesh, you mentioned the gallium,   is it another platform supported by mesa ?  I am sorry I have no idea about this,  could you please help to check this ?
>>>
>>> I think we can co-work with mesa team to work out an acceptable fix which can meet the requirement of Android without any break on other platforms.
>>
>> One thing needs clarifying here. Release fences from EGL are _not_ a
>> requirement. It is an optional feature. Android compliance suites pass
>> fully without Android sync fence support in Mesa at all.
>>
>> Other than that, it's been taking long enough and I agree that we
>> should finally wire both acquire and release fence support in EGL and
>> related drivers. Otherwise we can forget about getting good user
>> experience on Android.
>>
> Right, I'm not trying to say otherwise.
>
> The strange part, IMHO, is that now flatland has a hard requirement on both fences, where the [developer-side of the] documentation does not say anything about this.
> This sounds a bit backwards. I believe documentation update is in order?
>
> FWIW I was under the impression that EGL_ANDROID_native_fence_sync can be used in flatland. Although as Rob mentioned... not sure if the extension is available since the EGL meta seems to block/strip it out.
>
>
>> On a technical side, the EGL change needs to take into account that
>> not all drivers support fences and so it needs to have a fallback to
>> old behavior for those which don't.
>>
>
>> Other than that, correct me if I'm wrong, but could we just use the
>> DRI2 fence extension instead of adding some custom callbacks? I can
>> see that a normal client request to create a sync fence would end up
>> calling dri2_dpy->fence->create_fence_fd() (if it's present) [1].
>> Could we do the same?
>>
> Reusing existing API would be ideal.
>
> If not, Zhongmin/Yogesh please note:
>  - when extending the interface, the version number must be bumped
>  - user should check the version and the function pointer prior to use, falling back to the old scheme
>  - get_retrive_fd [barring the typo - retrieve], should have at least the fd ownership documented
>
> Thanks
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev