Re: [Mesa-dev] [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Hi Emil: Thanks very much. Yes, the previous patches (V1) is superseded now. -Original Message- From: Emil Velikov [mailto:emil.l.veli...@gmail.com] Sent: Friday, March 16, 2018 22:14 To: Wu, Zhongmin Cc: Tomasz Figa ; ML mesa-dev ; Rob Herring ; Liu, Zhiquan ; Long, Zhifang ; Kondapally, Kalyan ; Palli, Tapani ; Xu, Randy ; Eric Engestrom ; Bhardwaj, MunishX ; Kps, Harish Krupo ; Chad Versace Subject: Re: [PATCH v2] egl/android: Implement the eglSwapinterval for Android. On 16 March 2018 at 08:51, Wu, Zhongmin wrote: > Thanks very much Tomasz. > > So, who can help to commit such patch. > Fixed with s/dpy/disp/ (otherwise it won't even build) and pushed. Please go through the patches list and update it. I believe that all of those are superseded, do check them over. https://patchwork.freedesktop.org/project/mesa/patches/?submitter=17018&state=&q=&archive=&delegate= Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Thanks very much Tomasz.
So, who can help to commit such patch.
-Original Message-
From: Tomasz Figa [mailto:tf...@chromium.org]
Sent: Thursday, March 15, 2018 16:07
To: Wu, Zhongmin
Cc: ML mesa-dev ; Rob Herring
; Emil Velikov ; Liu, Zhiquan
; Long, Zhifang ; Kondapally,
Kalyan ; Palli, Tapani ;
Xu, Randy ; Eric Engestrom ; Bhardwaj,
MunishX ; Kps, Harish Krupo
; Chad Versace
Subject: Re: [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
On Thu, Jan 18, 2018 at 4:39 PM, Zhongmin Wu wrote:
> Implement the eglSwapinterval for Android platform to enable the async
> mode for some GFX benchmarks such as Daimler C217, CityBench.
>
> Signed-off-by: Zhongmin Wu
> ---
> src/egl/drivers/dri2/platform_android.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index f6a24cd..3a64689 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -476,6 +476,20 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> return EGL_TRUE;
> }
>
> +static EGLBoolean
> +droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> + _EGLSurface *surf, EGLint interval) {
> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> + struct ANativeWindow *window = dri2_surf->window;
> +
> + if (window->setSwapInterval(window, interval))
> + return EGL_FALSE;
> +
> + surf->SwapInterval = interval;
> + return EGL_TRUE;
> +}
> +
> static int
> update_buffers(struct dri2_egl_surface *dri2_surf) { @@ -1300,6
> +1314,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = {
> .swap_buffers = droid_swap_buffers,
> .swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /*
> Android implements the function */
> .swap_buffers_region = dri2_fallback_swap_buffers_region,
> + .swap_interval = droid_swap_interval,
> #if ANDROID_API_LEVEL >= 23
> .set_damage_region = droid_set_damage_region, #else @@ -1443,6
> +1458,12 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
>
> dri2_setup_screen(dpy);
>
> + /* we set the maximum swap interval as 1 for Android platform, Since
> + it is the maximum value supported by Android according to the
> + value of ANativeWindow::maxSwapInterval.
> + */
nit: Please use comment style consistent with rest of the file. Also, please
fix up typography issues, as below:
/* We set the maximum swap interval as 1 for Android platform, since it is
* the maximum value supported by Android according to the value of
* ANativeWindow::maxSwapInterval.
*/
I guess whoever committing this patch could just fix this up without a resend.
With the above:
Reviewed-by: Tomasz Figa
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Friendly ping
-Original Message-
From: Wu, Zhongmin
Sent: Wednesday, March 7, 2018 9:16
To: Eric Engestrom ; Emil Velikov
Cc: ML mesa-dev ; Tomasz Figa
; Rob Herring ; Liu, Zhiquan
; Long, Zhifang ; Kondapally,
Kalyan ; Palli, Tapani ;
Xu, Randy ; Bhardwaj, MunishX ;
Kps, Harish Krupo ; Chad Versace
Subject: RE: [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Hi Eric:
Sorry, but what is the status about this patch. I saw you said you was going
to help to modify the comments and push it in the last mail. Is this patch
merged in to lasts codes ?
Or do I miss something?
-Original Message-
From: Eric Engestrom [mailto:e...@engestrom.ch]
Sent: Friday, January 19, 2018 3:46
To: Emil Velikov
Cc: Wu, Zhongmin ; ML mesa-dev
; Tomasz Figa ; Rob Herring
; Liu, Zhiquan ; Long, Zhifang
; Kondapally, Kalyan ;
Palli, Tapani ; Xu, Randy ;
Bhardwaj, MunishX ; Kps, Harish Krupo
; Chad Versace
Subject: Re: [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
On Thursday, 2018-01-18 19:07:06 +, Emil Velikov wrote:
> On 18 January 2018 at 17:43, Eric Engestrom wrote:
> > On Thursday, 2018-01-18 07:52:42 +, Zhongmin Wu wrote:
> >> Implement the eglSwapinterval for Android platform to enable the
> >> async mode for some GFX benchmarks such as Daimler C217, CityBench.
> >>
> >> Signed-off-by: Zhongmin Wu
> >> ---
> >> src/egl/drivers/dri2/platform_android.c | 21 +
> >> 1 file changed, 21 insertions(+)
> >>
> >> diff --git a/src/egl/drivers/dri2/platform_android.c
> >> b/src/egl/drivers/dri2/platform_android.c
> >> index f6a24cd..3a64689 100644
> >> --- a/src/egl/drivers/dri2/platform_android.c
> >> +++ b/src/egl/drivers/dri2/platform_android.c
> >> @@ -476,6 +476,20 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> >> *disp, _EGLSurface *surf)
> >> return EGL_TRUE;
> >> }
> >>
> >> +static EGLBoolean
> >> +droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> >> + _EGLSurface *surf, EGLint interval) {
> >> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> >> + struct ANativeWindow *window = dri2_surf->window;
> >> +
> >> + if (window->setSwapInterval(window, interval))
> >> + return EGL_FALSE;
> >> +
> >> + surf->SwapInterval = interval;
> >> + return EGL_TRUE;
> >> +}
> >> +
> >> static int
> >> update_buffers(struct dri2_egl_surface *dri2_surf) { @@ -1300,6
> >> +1314,7 @@ static const struct dri2_egl_display_vtbl
> >> +droid_display_vtbl = {
> >> .swap_buffers = droid_swap_buffers,
> >> .swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /*
> >> Android implements the function */
> >> .swap_buffers_region = dri2_fallback_swap_buffers_region,
> >> + .swap_interval = droid_swap_interval,
> >> #if ANDROID_API_LEVEL >= 23
> >> .set_damage_region = droid_set_damage_region, #else @@ -1443,6
> >> +1458,12 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay
> >> *dpy)
> >>
> >> dri2_setup_screen(dpy);
> >>
> >> + /* we set the maximum swap interval as 1 for Android platform, Since
> >> + it is the maximum value supported by Android according to the
> >> + value of ANativeWindow::maxSwapInterval.
> >> + */
> >> + dri2_setup_swap_interval(dpy, 1);
> >
> > My C<->C++ interaction skills are more than rusty, but is it
> > possible to use `ANativeWindow.maxSwapInterval` or something like this?
> >
> > Given that the dEQP tests all pass, whether with the current comment
> > or the class field directly used in the code, this patch is:
> > Reviewed-by: Eric Engestrom
> >
> > I'll push it for you in a couple days if no one objects :)
> >
> My current checkout shows the struct is nicely separated for inclusion
> in both C and C++ sources.
> The C++ vfuncs are wrapped in a ifdef _cplusplus block, so the ABI
> should be identical across both.
>
> I'm suspecting that maxSwapInterval > 1 will require additional
> changes, so I'd keep it as-is
That's actually a good point; this should indeed stay hard coded, with this
comment.
> (nit the comment style and adding dEQP stats in the commit message).
I was going to do this before pushing, but you're right to point it out :)
> Reviewed-by: Emil Velikov
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Hi Eric:
Sorry, but what is the status about this patch. I saw you said you was going
to help to modify the comments and push it in the last mail. Is this patch
merged in to lasts codes ?
Or do I miss something?
-Original Message-
From: Eric Engestrom [mailto:e...@engestrom.ch]
Sent: Friday, January 19, 2018 3:46
To: Emil Velikov
Cc: Wu, Zhongmin ; ML mesa-dev
; Tomasz Figa ; Rob Herring
; Liu, Zhiquan ; Long, Zhifang
; Kondapally, Kalyan ;
Palli, Tapani ; Xu, Randy ;
Bhardwaj, MunishX ; Kps, Harish Krupo
; Chad Versace
Subject: Re: [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
On Thursday, 2018-01-18 19:07:06 +, Emil Velikov wrote:
> On 18 January 2018 at 17:43, Eric Engestrom wrote:
> > On Thursday, 2018-01-18 07:52:42 +, Zhongmin Wu wrote:
> >> Implement the eglSwapinterval for Android platform to enable the
> >> async mode for some GFX benchmarks such as Daimler C217, CityBench.
> >>
> >> Signed-off-by: Zhongmin Wu
> >> ---
> >> src/egl/drivers/dri2/platform_android.c | 21 +
> >> 1 file changed, 21 insertions(+)
> >>
> >> diff --git a/src/egl/drivers/dri2/platform_android.c
> >> b/src/egl/drivers/dri2/platform_android.c
> >> index f6a24cd..3a64689 100644
> >> --- a/src/egl/drivers/dri2/platform_android.c
> >> +++ b/src/egl/drivers/dri2/platform_android.c
> >> @@ -476,6 +476,20 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> >> *disp, _EGLSurface *surf)
> >> return EGL_TRUE;
> >> }
> >>
> >> +static EGLBoolean
> >> +droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> >> + _EGLSurface *surf, EGLint interval) {
> >> + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> >> + struct ANativeWindow *window = dri2_surf->window;
> >> +
> >> + if (window->setSwapInterval(window, interval))
> >> + return EGL_FALSE;
> >> +
> >> + surf->SwapInterval = interval;
> >> + return EGL_TRUE;
> >> +}
> >> +
> >> static int
> >> update_buffers(struct dri2_egl_surface *dri2_surf) { @@ -1300,6
> >> +1314,7 @@ static const struct dri2_egl_display_vtbl droid_display_vtbl = {
> >> .swap_buffers = droid_swap_buffers,
> >> .swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /*
> >> Android implements the function */
> >> .swap_buffers_region = dri2_fallback_swap_buffers_region,
> >> + .swap_interval = droid_swap_interval,
> >> #if ANDROID_API_LEVEL >= 23
> >> .set_damage_region = droid_set_damage_region, #else @@ -1443,6
> >> +1458,12 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay
> >> *dpy)
> >>
> >> dri2_setup_screen(dpy);
> >>
> >> + /* we set the maximum swap interval as 1 for Android platform, Since
> >> + it is the maximum value supported by Android according to the
> >> + value of ANativeWindow::maxSwapInterval.
> >> + */
> >> + dri2_setup_swap_interval(dpy, 1);
> >
> > My C<->C++ interaction skills are more than rusty, but is it
> > possible to use `ANativeWindow.maxSwapInterval` or something like this?
> >
> > Given that the dEQP tests all pass, whether with the current comment
> > or the class field directly used in the code, this patch is:
> > Reviewed-by: Eric Engestrom
> >
> > I'll push it for you in a couple days if no one objects :)
> >
> My current checkout shows the struct is nicely separated for inclusion
> in both C and C++ sources.
> The C++ vfuncs are wrapped in a ifdef _cplusplus block, so the ABI
> should be identical across both.
>
> I'm suspecting that maxSwapInterval > 1 will require additional
> changes, so I'd keep it as-is
That's actually a good point; this should indeed stay hard coded, with this
comment.
> (nit the comment style and adding dEQP stats in the commit message).
I was going to do this before pushing, but you're right to point it out :)
> Reviewed-by: Emil Velikov
>
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Below is the dEQP result for all the dEQP-EGL.*swap_interval tests
11-11 18:22:44.164 12737 12737 I dEQP: Device has at least 832.00 MiB total
system memory per Android CDD
11-11 18:22:44.164 12737 12737 I dEQP: Writing test log into
/sdcard/dEQP-Log.qpa
11-11 18:22:44.165 12737 12737 I dEQP: dEQP Core unknown (0xcafebabe)
starting..
11-11 18:22:44.165 12737 12737 I dEQP: target implementation = 'android'
11-11 18:22:44.329 12737 12760 I dEQP:
11-11 18:22:44.329 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.query_config.get_config_attrib.max_swap_interval'..
11-11 18:22:44.391 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.392 12737 12760 I dEQP:
11-11 18:22:44.392 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.query_config.get_config_attrib.min_swap_interval'..
11-11 18:22:44.421 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.421 12737 12760 I dEQP:
11-11 18:22:44.421 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.choose_config.simple.selection_only.max_swap_interval'..
11-11 18:22:44.429 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.429 12737 12760 I dEQP:
11-11 18:22:44.429 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.choose_config.simple.selection_only.min_swap_interval'..
11-11 18:22:44.437 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.437 12737 12760 I dEQP:
11-11 18:22:44.437 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.choose_config.simple.selection_and_sort.max_swap_interval'..
11-11 18:22:44.444 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.444 12737 12760 I dEQP:
11-11 18:22:44.444 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.choose_config.simple.selection_and_sort.min_swap_interval'..
11-11 18:22:44.451 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.618 12737 12760 I dEQP:
11-11 18:22:44.618 12737 12760 I dEQP: Test case
'dEQP-EGL.functional.negative_api.swap_interval'..
11-11 18:22:44.621 12737 12760 I dEQP: Pass (Pass)
11-11 18:22:44.645 12737 12760 I dEQP:
11-11 18:22:44.645 12737 12760 I dEQP: DONE!
11-11 18:22:44.645 12737 12760 I dEQP:
11-11 18:22:44.645 12737 12760 I dEQP: Test run totals:
11-11 18:22:44.645 12737 12760 I dEQP: Passed:7/7 (100.0%)
11-11 18:22:44.645 12737 12760 I dEQP: Failed:0/7 (0.0%)
11-11 18:22:44.645 12737 12760 I dEQP: Not supported: 0/7 (0.0%)
11-11 18:22:44.645 12737 12760 I dEQP: Warnings: 0/7 (0.0%)
11-11 18:22:45.097 12737 12737 I dEQP: Done, killing process
-Original Message-
From: Wu, Zhongmin
Sent: Thursday, January 18, 2018 15:39
To: mesa-dev@lists.freedesktop.org
Cc: tf...@chromium.org; r...@kernel.org; emil.l.veli...@gmail.com; Liu, Zhiquan
; Long, Zhifang ; Kondapally,
Kalyan ; Palli, Tapani ;
Xu, Randy ; Wu, Zhongmin ; Eric
Engestrom ; Bhardwaj, MunishX ;
Kps, Harish Krupo ; Chad Versace
Subject: [PATCH v2] egl/android: Implement the eglSwapinterval for Android.
Implement the eglSwapinterval for Android platform to enable the async mode for
some GFX benchmarks such as Daimler C217, CityBench.
Signed-off-by: Zhongmin Wu
---
src/egl/drivers/dri2/platform_android.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index f6a24cd..3a64689 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -476,6 +476,20 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *surf)
return EGL_TRUE;
}
+static EGLBoolean
+droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
+ _EGLSurface *surf, EGLint interval) {
+ struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+ struct ANativeWindow *window = dri2_surf->window;
+
+ if (window->setSwapInterval(window, interval))
+ return EGL_FALSE;
+
+ surf->SwapInterval = interval;
+ return EGL_TRUE;
+}
+
static int
update_buffers(struct dri2_egl_surface *dri2_surf) { @@ -1300,6 +1314,7 @@
static const struct dri2_egl_display_vtbl droid_display_vtbl = {
.swap_buffers = droid_swap_buffers,
.swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /*
Android implements the function */
.swap_buffers_region = dri2_fallback_swap_buffers_region,
+ .swap_interval = droid_swap_interval,
#if ANDROID_API_LEVEL >= 23
.set_damage_region = droid_set_damage_region, #else @@ -1443,6 +1458,12 @@
dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
dri2_setup_screen(dpy);
+ /* we set the maximum swap interval as 1 for Android platform, Since
+ it is the maximum value supported by Android according to the
+ value of ANativeWindow::maxSwapInterval.
+ */
+ dri2_setup_swap_interval(dpy, 1);
+
if (!droid_add_configs_for_vi
Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for Android] egl/android: Implement the eglSwapinterval for Android.
Hi Tomasz:
Thanks very much for your reply, I will re-submit the patch.
And I did not do the deqp testing yet, I am going to have a try.
-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of
Tomasz Figa
Sent: Tuesday, January 16, 2018 15:17
To: Wu, Zhongmin
Cc: Long, Zhifang ; Kps, Harish Krupo
; Xu, Randy ; Chad Versace
; Eric Engestrom ; Emil Velikov
; Kondapally, Kalyan ;
Bhardwaj, MunishX ; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for
Android] egl/android: Implement the eglSwapinterval for Android.
Hi Zhongmin,
On Tue, Jan 16, 2018 at 4:07 PM, Wu, Zhongmin wrote:
> Sorry, is there any comment about the below patch, Thanks very much! Or did I
> miss something ?
I assumed this was sent by mistake. The subject doesn't look like a patch for
review - it should have [PATCH] prefix. There was even a follow-up email
(presumably generated by your mailing client) to cancel sending it.
Also please remove internal annotations, such as gerrit Change-Id, since they
do not have any meaning for upstream purposes.
As for the change itself, it looks fine to me, +/- some style nitpicks, which I
listed inline. Have you checked if there are no dEQP regressions (at least for
the EGL suite)?
Best regards,
Tomasz
>
> -Original Message-
> From: Wu, Zhongmin
> Sent: Wednesday, January 3, 2018 10:11
> To: mesa-dev@lists.freedesktop.org
> Cc: Kondapally, Kalyan ; Palli, Tapani
> ; Xu, Randy ; Long,
> Zhifang ; Wu, Zhongmin
> ; Rob Herring ; Tomasz Figa
> ; Eric Engestrom ; Emil Velikov
> ; Bhardwaj, MunishX
> ; Kps, Harish Krupo
> ; Chad Versace
> Subject: [egl/android: Implement the eglSwapinterval for Android]
> egl/android: Implement the eglSwapinterval for Android.
>
> From: Zhongmin Wu
>
> Implement the eglSwapinterval for Android platform to enable the async mode
> for some GFX benchmarks.
>
> Change-Id: I3576d8b92862719dae11c31e2adc2d77cb5a0b64
> Signed-off-by: Zhongmin Wu
> ---
> src/egl/drivers/dri2/platform_android.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index f6a24cd..f9c74ee 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -476,6 +476,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay
> *disp, _EGLSurface *surf)
> return EGL_TRUE;
> }
>
> +static EGLBoolean droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
> +_EGLSurface *surf, EGLint interval) {
Please move the function name to new line, align the arguments with the top
lines, if there is a need to wrap the lines and move the opening brace to new
line, to match the coding style already used in the file.
> +
No need for this blank line.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [egl/android: Implement the eglSwapinterval for Android] egl/android: Implement the eglSwapinterval for Android.
Sorry, is there any comment about the below patch, Thanks very much! Or did I
miss something ?
-Original Message-
From: Wu, Zhongmin
Sent: Wednesday, January 3, 2018 10:11
To: mesa-dev@lists.freedesktop.org
Cc: Kondapally, Kalyan ; Palli, Tapani
; Xu, Randy ; Long, Zhifang
; Wu, Zhongmin ; Rob Herring
; Tomasz Figa ; Eric Engestrom
; Emil Velikov ; Bhardwaj, MunishX
; Kps, Harish Krupo ;
Chad Versace
Subject: [egl/android: Implement the eglSwapinterval for Android] egl/android:
Implement the eglSwapinterval for Android.
From: Zhongmin Wu
Implement the eglSwapinterval for Android platform to enable the async mode for
some GFX benchmarks.
Change-Id: I3576d8b92862719dae11c31e2adc2d77cb5a0b64
Signed-off-by: Zhongmin Wu
---
src/egl/drivers/dri2/platform_android.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index f6a24cd..f9c74ee 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -476,6 +476,18 @@ droid_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp,
_EGLSurface *surf)
return EGL_TRUE;
}
+static EGLBoolean droid_swap_interval(_EGLDriver *drv, _EGLDisplay *dpy,
+_EGLSurface *surf, EGLint interval) {
+
+ struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+ struct ANativeWindow *window = dri2_surf->window;
+ if (window->setSwapInterval(window, interval)) {
+ return EGL_FALSE;
+ }
+ surf->SwapInterval = interval;
+ return EGL_TRUE;
+}
+
static int
update_buffers(struct dri2_egl_surface *dri2_surf) { @@ -1300,6 +1312,7 @@
static const struct dri2_egl_display_vtbl droid_display_vtbl = {
.swap_buffers = droid_swap_buffers,
.swap_buffers_with_damage = dri2_fallback_swap_buffers_with_damage, /*
Android implements the function */
.swap_buffers_region = dri2_fallback_swap_buffers_region,
+ .swap_interval = droid_swap_interval,
#if ANDROID_API_LEVEL >= 23
.set_damage_region = droid_set_damage_region, #else @@ -1443,6 +1456,8 @@
dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *dpy)
dri2_setup_screen(dpy);
+ dri2_setup_swap_interval(dpy, 1);
+
if (!droid_add_configs_for_visuals(drv, dpy)) {
err = "DRI2: failed to add configs";
goto cleanup;
--
2.7.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [EGL android: accquire fence implementation 1/2] i965: Return the last fence if the batch buffer is empty and nothing to be flushed when _intel_batchbuffer_flush_fence.
Hi Yogesh:
http://oorja.iind.intel.com/mediawiki/index.php/Flatland
can you also try the flatland on this page.
For AOSP flatland, yes, the EGL patch may solve the issue.
However, I met one case that the batch buffer is empty just at the swapbuffer
(glfush is just called before that), then you might not get a fence fd. So I
had to record the last fence fd.
I am not sure, you had better try it on the board If you can get a valid fd on
any situation.
-Original Message-
From: Marathe, Yogesh
Sent: Monday, August 7, 2017 13:25
To: Tomasz Figa ; Wu, Zhongmin
Cc: Gao, Shuo ; Antognolli, Rafael
; Emil Velikov ; Kenneth
Graunke ; ML mesa-dev ;
Kondapally, Kalyan ; Timothy Arceri
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation 1/2] i965:
Return the last fence if the batch buffer is empty and nothing to be flushed
when _intel_batchbuffer_flush_fence.
This can be dropped. I'm running with egl patch alone and things seem fine.
Zhongmin, please comment if you don’t think so.
> -Original Message-
> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
> Behalf Of Marathe, Yogesh
> Sent: Friday, August 4, 2017 9:18 PM
>
> Tomasz,
>
> > -Original Message-
> > From: Tomasz Figa [mailto:tf...@chromium.org]
> > Sent: Friday, August 4, 2017 3:48 PM
> > To: Marathe, Yogesh
> > Cc: Emil Velikov ; Wu, Zhongmin
> > ; Gao, Shuo ; Antognolli,
> > Rafael ; Timothy Arceri
> > ; Kenneth Graunke ;
> > Kondapally, Kalyan ; ML mesa-dev > d...@lists.freedesktop.org>
> > Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation
> > 1/2]
> i965:
> > Return the last fence if the batch buffer is empty and nothing to be
> > flushed when _intel_batchbuffer_flush_fence.
> >
> > Hi Yogesh,
> >
> > On Fri, Aug 4, 2017 at 7:12 PM, Marathe, Yogesh
> >
> > wrote:
> > > Hi Emil,
> > >
> > >> -Original Message-
> > >> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
> > >> Behalf Of Emil Velikov
> > >> Sent: Tuesday, July 25, 2017 8:19 PM
> > >> To: Wu, Zhongmin
> > >> Cc: Gao, Shuo ; Antognolli, Rafael
> > >> ; Timothy Arceri
> > >> ; Marathe, Yogesh
> > >> ; Tomasz Figa ;
> > >> Kenneth Graunke ; Kondapally, Kalyan
> > >> ; ML mesa-dev > >> d...@lists.freedesktop.org>
> > >> Subject: Re: [Mesa-dev] [EGL android: accquire fence
> > >> implementation 1/2]
> > i965:
> > >> Return the last fence if the batch buffer is empty and nothing to
> > >> be flushed when _intel_batchbuffer_flush_fence.
> > >>
> > >> Hi Zhongmin,
> > >>
> > >> Is the issue resolved by the EGL patch alone? Worth sticking with
> > >> that for
> > now?
> > >>
> > >> I think this patch will cause some noticeable overhead - see
> > >> below for
> details.
> > >>
> > >>
> > >> On 21 July 2017 at 04:08, Zhongmin Wu wrote:
> > >> > Always save the last fence in the brw context when flushing buffer.
> > >> > If the buffer is nothing to be flushed, then return the last
> > >> > fence when asked for.
> > >> >
> > >> > Change-Id: Ic47035bcd1a27e402609afd9e2d1e3972548b97d
> > >> > Signed-off-by: Zhongmin Wu
> > >> > ---
> > >> > src/mesa/drivers/dri/i965/brw_context.c |5 +
> > >> > src/mesa/drivers/dri/i965/brw_context.h |1 +
> > >> > src/mesa/drivers/dri/i965/intel_batchbuffer.c | 16 ++--
> > >> > 3 files changed, 20 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> > >> > b/src/mesa/drivers/dri/i965/brw_context.c
> > >> > index 5433f90..ed0b056 100644
> > >> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > >> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > >> > @@ -1086,6 +1086,8 @@ brwCreateContext(gl_api api,
> > >> > ctx->VertexProgram._MaintainTnlProgram = true;
> > >> > ctx->FragmentProgram._MaintainTexEnvProgram = true;
> > >> >
> > >> > + brw->out_fence_fd = -1;
> > >> > +
> > >> > brw_draw_init( brw );
> > >> >
> > >> > if ((flags & __DRI_CTX_FLAG_DEBUG) != 0) { @@ -1169,6
> > >> > +1171,9 @@ intelDestro
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v3.1)
Hi Tomasz:
Thanks very much for your comments, I read it carefully, but still have some
questions below. Could you please have a look. Thanks again.
-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of
Tomasz Figa
Sent: Wednesday, July 19, 2017 12:42
To: Wu, Zhongmin
Cc: Gao, Shuo ; Liu, Zhiquan ;
Daniel Stone ; Antognolli, Rafael
; Emil Velikov ; Chad
Versace ; Eric Engestrom ;
Marathe, Yogesh ; Kenneth Graunke
; Kondapally, Kalyan ;
Rainer Hochecker ; ML mesa-dev
; Nicolai Hähnle ;
Varad Gautam
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS (v3.1)
Hi Zhongmin,
Thanks for the patch. Please see my comments inline.
On Wed, Jul 19, 2017 at 12:22 PM, Zhongmin Wu wrote:
> Before we queued the buffer with a invalid fence (-1), it will make
> some benchmarks failed to test such as flatland.
>
> Now we get the out fence during the flushing buffer and then pass it
> to SurfaceFlinger in eglSwapbuffer function.
>
> v2: a) Also implement the fence in cancelBuffer.
> b) The last sync fence is stored in drawable object
>rather than brw context.
> c) format clear.
>
> v3: a) Save the last fence fd in DRI Context object.
> b) Return the last fence if the batch buffer is empty and
>nothing to be flushed when _intel_batchbuffer_flush_fence
> c) Add the new interface in vbtl to set the retrieve fence
>in the egl surface. This is just for cancelBuffer.
> d) For queueBuffer, the fence is get with DRI fence interface.
>For cancelBuffer, the fence is get before the context is
>reset, and the fence is then moved to its surface.
> v3.1 a) close fd in the new vbtl interface on none Android platform
>
> Change-Id: Ic0773c19788d612a98d1402f5b5619dab64c1bc2
> Tracked-On: https://jira01.devtools.intel.com/browse/OAM-43936
> Reported-On: https://bugs.freedesktop.org/show_bug.cgi?id=101655
> Signed-off-by: Zhongmin Wu
> Reported-by: Li, Guangli
> Tested-by: Marathe, Yogesh
> ---
> src/egl/drivers/dri2/egl_dri2.c | 17 +-
> src/egl/drivers/dri2/egl_dri2.h |3 +++
> src/egl/drivers/dri2/platform_android.c | 30
> ++---
> src/egl/drivers/dri2/platform_drm.c |1 +
> src/egl/drivers/dri2/platform_surfaceless.c |1 +
> src/egl/drivers/dri2/platform_wayland.c |2 ++
> src/egl/drivers/dri2/platform_x11.c |2 ++
> src/egl/drivers/dri2/platform_x11_dri3.c |1 +
> src/mesa/drivers/dri/common/dri_util.c|3 +++
> src/mesa/drivers/dri/common/dri_util.h|2 ++
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 25 ++---
> 11 files changed, 80 insertions(+), 7 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c
> b/src/egl/drivers/dri2/egl_dri2.c index 020a0bc..19d346d 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1351,9 +1351,17 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp,
> _EGLSurface *dsurf,
> ddraw = (dsurf) ? dri2_dpy->vtbl->get_dri_drawable(dsurf) : NULL;
> rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
> cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
> -
> + int fence_fd = -1;
> if (old_ctx) {
>__DRIcontext *old_cctx =
> dri2_egl_context(old_ctx)->dri_context;
> + if (old_dsurf) {
> + void * fence = dri2_dpy->fence->create_fence_fd(old_cctx, -1);
> + if (fence) {
> +fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
> fence);
> +dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
> + }
> + dri2_dpy->vtbl->set_retrieve_fence(old_dsurf, fence_fd);
I think we don't need this callback. We can just set the fence in old_dsurf
directly, i.e.
dri2_surf_set_release_fence(surf, fence_fd) {
if (surf->fence_fd >= 0)
close(surf->fence_fd);
surf->fence_fd = fence_fd;
}
// ...
dri2_make_current()
{
// ...
int fence_fd = -1;
if (old_dsurf) {
void * fence = dri2_dpy->fence->create_fence_fd(old_cctx, -1);
if (fence) {
fence_fd =
dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen, fence);
dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
}
dri2_surf_set_release_fence(old_dsurf, fence_fd);
}
// ...
}
wzm :==
I am a little confused here, you mean we can add the fd f
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)
sa/drivers/dri/common/dri_util.h
b/src/mesa/drivers/dri/common/dri_util.h
index 8fcd632..f723a90 100644
--- a/src/mesa/drivers/dri/common/dri_util.h
+++ b/src/mesa/drivers/dri/common/dri_util.h
@@ -218,6 +218,8 @@ struct __DRIcontextRec {
int draw_stamp;
int read_stamp;
} dri2;
+
+int retrieve_fd;
};
/**
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 62d2fe8..77bf8f6 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -648,9 +648,25 @@ do_flush_locked(struct brw_context *brw, int in_fence_fd,
int *out_fence_fd)
/* Add the batch itself to the end of the validation list */
add_exec_bo(batch, batch->bo);
+ if (brw->driContext &&
+brw->driContext->retrieve_fd != -1) {
+close(brw->driContext->retrieve_fd);
+brw->driContext->retrieve_fd = -1;
+ }
+
+ int fd = -1;
ret = execbuffer(dri_screen->fd, batch, hw_ctx,
4 * USED_BATCH(*batch),
- in_fence_fd, out_fence_fd, flags);
+ in_fence_fd, &fd, flags);
+
+ if (out_fence_fd != NULL) {
+*out_fence_fd = fd;
+if (brw->driContext)
+ brw->driContext->retrieve_fd = dup(fd);
+ } else {
+if (brw->driContext)
+ brw->driContext->retrieve_fd = fd;
+ }
}
throttle(brw);
@@ -684,8 +700,11 @@ _intel_batchbuffer_flush_fence(struct brw_context *brw,
{
int ret;
- if (USED_BATCH(brw->batch) == 0)
- return 0;
+ if (USED_BATCH(brw->batch) == 0) {
+ if (out_fence_fd && brw->driContext->retrieve_fd != -1)
+ *out_fence_fd = dup(brw->driContext->retrieve_fd);
+ return 0;
+ }
if (brw->throttle_batch[0] == NULL) {
brw->throttle_batch[0] = brw->batch.bo;
--
1.7.9.5
From: Tomasz Figa [tf...@chromium.org]
Sent: Friday, July 14, 2017 9:58 AM
To: Antognolli, Rafael
Cc: Chris Wilson; ML mesa-dev; Marathe, Yogesh; Wu, Zhongmin; Liu, Zhiquan;
Kondapally, Kalyan; Chad Versace; Eric Engestrom; Emil Velikov; Rob Clark;
Kenneth Graunke; Widawsky, Benjamin; Kristian H . Kristensen; Timothy Arceri;
Dominik Behr
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS (v2)
[resend from right email address, without bouncing recipients and with
all the people involved in v1... Zhongmin, please remember to add all
people participating in the discussion to CC next time.]
On Sat, Jul 15, 2017 at 1:45 AM, Rafael Antognolli
wrote:
> On Fri, Jul 14, 2017 at 09:13:49AM +0100, Chris Wilson wrote:
>> Quoting Zhongmin Wu (2017-07-14 07:55:45)
[snip]
>> > extern uint32_t
>> > diff --git a/src/mesa/drivers/dri/i915/intel_screen.c
>> > b/src/mesa/drivers/dri/i915/intel_screen.c
>> > index cba5434..38d0e63 100644
>> > --- a/src/mesa/drivers/dri/i915/intel_screen.c
>> > +++ b/src/mesa/drivers/dri/i915/intel_screen.c
>> > @@ -182,6 +182,7 @@ static const struct __DRI2flushExtensionRec
>> > intelFlushExtension = {
>> >
>> > .flush = intelDRI2Flush,
>> > .invalidate = dri2InvalidateDrawable,
>> > +.get_retrieve_fd= NULL,
>> > };
>> >
>> > static struct intel_image_format intel_image_formats[] = {
>> > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> > b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> > index 62d2fe8..9813c8c 100644
>> > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> > @@ -648,9 +648,25 @@ do_flush_locked(struct brw_context *brw, int
>> > in_fence_fd, int *out_fence_fd)
>> > /* Add the batch itself to the end of the validation list */
>> > add_exec_bo(batch, batch->bo);
>> >
>> > + if (brw->driContext->driDrawablePriv &&
>> > +brw->driContext->driDrawablePriv->retrieve_fd != -1) {
>> > +close(brw->driContext->driDrawablePriv->retrieve_fd);
>> > +brw->driContext->driDrawablePriv->retrieve_fd = -1;
>> > + }
>>
>> No. Use the correct interfaces for creating a fence, stop imposing the
>> cost upon everyone else.
>
> Yeah, that's correct, it definitely shouldn't be inside intel_batchbuffer.
>
> Chris, the main problem that Zhongmin raised was that on
> droid_window_cancel_buffer the conte
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
Add Chris Wilson
-Original Message-
From: Wu, Zhongmin
Sent: Thursday, July 13, 2017 16:31
To: 'Tomasz Figa' ; 'Michel Dänzer'
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ;
'Chad Versace' ; 'Eric Engestrom'
; 'Emil Velikov' ; 'Rob Clark'
; 'Kenneth Graunke' ;
Widawsky, Benjamin ; 'ML mesa-dev'
; 'Kristian H . Kristensen'
; 'Timothy Arceri'
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
By the way, we can't use create_fence_fd in cancelBuffer even if we
implemented the last fence feature as radeonsi driver , since the context is
reset before cancelBuffer, and we cannot depend on the last fence in context
object either...
So things may be tricky on Android
We will try to modify the previous patch and save the last fence in the
drawable object (contained in EGLSurface) rather than context object to support
the cancelBuffer...
As for the using of last fence when the batch buffer is empty for
create_fence_fd, I suggest it can be another story and we will try to optimize
it in the future...
-Original Message-
From: Wu, Zhongmin
Sent: Thursday, July 13, 2017 12:23
To: Tomasz Figa ; Michel Dänzer
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ; Chad
Versace ; Eric Engestrom ; Emil
Velikov ; Rob Clark ;
Kenneth Graunke ; Widawsky, Benjamin
; ML mesa-dev ;
Kristian H . Kristensen ; Timothy Arceri
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Tomasz:
That is right, what we needed is just the "last fence", and then where to
record it and how to get it.
We will check the solutions on radeonsi by Michel, if create_fence_fd can also
get the last fence, of course we can use such DRI2 fence extension API in
queuebuffer and canclebuffer.
-Original Message-
From: Tomasz Figa [mailto:tf...@chromium.org]
Sent: Thursday, July 13, 2017 12:08
To: Michel Dänzer ; Wu, Zhongmin
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ; Chad
Versace ; Eric Engestrom ; Emil
Velikov ; Rob Clark ;
Kenneth Graunke ; Widawsky, Benjamin
; ML mesa-dev ;
Kristian H . Kristensen ; Timothy Arceri
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Zhongmin, Michel,
On Thu, Jul 13, 2017 at 12:21 PM, Michel Dänzer wrote:
> On 13/07/17 09:28 AM, Wu, Zhongmin wrote:
>> Hi Tomasz
>>
>> Thanks, but I am afraid we have to use the last fence from the last buffer
>> flushing , According to my understanding:
>>
>> If the glFlush was called before swapbuffer ,there would be no new fence
>> after that since the batch buffer may be flushed out, no batch flushing, no
>> fence generated.
>>
>>
>> You can check the function "_intel_batchbuffer_flush_fence", at the very
>> beginning, it will check the content of the buffer, if it is empty, it will
>> return and do nothing.
>> =
>>
>> So, as the same reason, it may not work if we use
>> dri2_dpy->fence->create_fence_fd, the process is below.
>>
>> Swapbuffer (==> flush batch buffer) ==> create_fence_fd(===> flush
>> batch buffer and get fd)
>>
>> You can see, the second buffer flushing may get nothing (not to
>> mention the first buffer flushing may get nothing either)
First of all, the sequence of calls is like this:
- brw_dri_create_fence_fd()
- brw_fence_insert_locked()
- brw_emit_mi_flush() <--- this emits commands to the command buffer...
- intel_batchbuffer_flush_fence() <--- this should flush the buffer and get
a fence
but...
>
> Apologies for jumping into this discussion, and possibly missing some
> context.
>
> FWIW, it should be possible to re-use the previous fence when there is
> no new work to flush. See e.g. commits
>
> f1be3d8cdde1 ("radeonsi: don't flush an empty IB if the only thing we
>need is a fence")
> 800efb0690e9 ("radeonsi: Flush when we're asked to return a fence but
>don't have one yet")
>
> for how the radeonsi driver handles this.
I agree with Michel that it would make more sense if Intel driver kept the last
fence and returned it using when DRI2 fence extension is called.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS (v2)
Hi Chris Wilson,
Thanks for your advice, but we have some discussions on another thread, and
now we do meet some tricky things about how to implement such fence needed by
Android, and why current API in codes cannot meet the requirement.
I will add you in that loop and hope U can help us to find ways to handle it.
Thanks again !
-Original Message-
From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of
Chris Wilson
Sent: Friday, July 14, 2017 16:14
To: Wu, Zhongmin ; mesa-dev@lists.freedesktop.org
Cc: b...@freedesktop.org; Marathe, Yogesh
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS (v2)
Quoting Zhongmin Wu (2017-07-14 07:55:45)
> Before we queued the buffer with a invalid fence (-1), it will make
> some benchmarks failed to test such as flatland.
>
> Now we get the out fence during the flushing buffer and then pass it
> to SurfaceFlinger in eglSwapbuffer function.
>
> v2: a) Also implement the fence in cancelBuffer.
> b) The last sync fence is stored in drawable object
>rather than brw context.
> c) format clear.
>
> Change-Id: Ic0773c19788d612a98d1402f5b5619dab64c1bc2
> Tracked-On: https://jira01.devtools.intel.com/browse/OAM-43936
> Reported-On: https://bugs.freedesktop.org/show_bug.cgi?id=101655
> Signed-off-by: Zhongmin Wu
> Reported-by: Li, Guangli
> Tested-by: Marathe, Yogesh
> ---
> include/GL/internal/dri_interface.h |9 +
> src/egl/drivers/dri2/platform_android.c | 20 +---
> src/mesa/drivers/dri/common/dri_util.c|3 +++
> src/mesa/drivers/dri/common/dri_util.h|2 ++
> src/mesa/drivers/dri/i915/intel_screen.c |1 +
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 18 +-
> src/mesa/drivers/dri/i965/intel_screen.c |6 ++
> src/mesa/drivers/dri/nouveau/nouveau_screen.c |1 +
> src/mesa/drivers/dri/radeon/radeon_screen.c |1 +
> 9 files changed, 53 insertions(+), 8 deletions(-)
>
> diff --git a/include/GL/internal/dri_interface.h
> b/include/GL/internal/dri_interface.h
> index fc2d4bb..7c6b08b 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -287,6 +287,15 @@ struct __DRI2flushExtensionRec {
> void (*flush)(__DRIdrawable *drawable);
>
> /**
> +* This function enables retrieving fence during enqueue / cancel buffer
> operations
> +*
> +* \param drawable the drawable to invalidate
> +*
> +* \since 3
> +*/
> +int (*get_retrieve_fd)(__DRIdrawable *drawable);
> +
> +/**
> * Ask the driver to call getBuffers/getBuffersWithFormat before
> * it starts rendering again.
> *
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index cc2e4a6..aa99ed3 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -305,10 +305,11 @@ droid_window_enqueue_buffer(_EGLDisplay *disp, struct
> dri2_egl_surface *dri2_sur
> *is passed to queueBuffer, and the ANativeWindow implementation
> *is responsible for closing it.
> */
> - int fence_fd = -1;
> + int fd = -1;
> + if (dri2_dpy->flush->get_retrieve_fd)
> + fd = dri2_dpy->flush->get_retrieve_fd(dri2_surf->dri_drawable);
> dri2_surf->window->queueBuffer(dri2_surf->window, dri2_surf->buffer,
> - fence_fd);
> -
> + fd);
> dri2_surf->buffer->common.decRef(&dri2_surf->buffer->common);
> dri2_surf->buffer = NULL;
> dri2_surf->back = NULL;
> @@ -324,11 +325,16 @@ droid_window_enqueue_buffer(_EGLDisplay *disp,
> struct dri2_egl_surface *dri2_sur }
>
> static void
> -droid_window_cancel_buffer(struct dri2_egl_surface *dri2_surf)
> +droid_window_cancel_buffer(_EGLDisplay *disp,
> + struct dri2_egl_surface *dri2_surf)
> {
> int ret;
> -
> - ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
> dri2_surf->buffer, -1);
> + int fd = -1;
> + struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
> + if (dri2_dpy->flush->get_retrieve_fd)
> + fd = dri2_dpy->flush->get_retrieve_fd(dri2_surf->dri_drawable);
> + ret = dri2_surf->window->cancelBuffer(dri2_surf->window,
> + dri2_surf->buffer, fd);
> if (ret < 0) {
>_eglLog(_EGL_WARNING, "ANativeWindow::cancelBuffer failed");
>dri2_surf->base.Lost
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
By the way, we can't use create_fence_fd in cancelBuffer even if we
implemented the last fence feature as radeonsi driver , since the context is
reset before cancelBuffer, and we cannot depend on the last fence in context
object either...
So things may be tricky on Android
We will try to modify the previous patch and save the last fence in the
drawable object (contained in EGLSurface) rather than context object to support
the cancelBuffer...
As for the using of last fence when the batch buffer is empty for
create_fence_fd, I suggest it can be another story and we will try to optimize
it in the future...
-Original Message-
From: Wu, Zhongmin
Sent: Thursday, July 13, 2017 12:23
To: Tomasz Figa ; Michel Dänzer
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ; Chad
Versace ; Eric Engestrom ; Emil
Velikov ; Rob Clark ;
Kenneth Graunke ; Widawsky, Benjamin
; ML mesa-dev ;
Kristian H . Kristensen ; Timothy Arceri
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Tomasz:
That is right, what we needed is just the "last fence", and then where to
record it and how to get it.
We will check the solutions on radeonsi by Michel, if create_fence_fd can also
get the last fence, of course we can use such DRI2 fence extension API in
queuebuffer and canclebuffer.
-Original Message-
From: Tomasz Figa [mailto:tf...@chromium.org]
Sent: Thursday, July 13, 2017 12:08
To: Michel Dänzer ; Wu, Zhongmin
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ; Chad
Versace ; Eric Engestrom ; Emil
Velikov ; Rob Clark ;
Kenneth Graunke ; Widawsky, Benjamin
; ML mesa-dev ;
Kristian H . Kristensen ; Timothy Arceri
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Zhongmin, Michel,
On Thu, Jul 13, 2017 at 12:21 PM, Michel Dänzer wrote:
> On 13/07/17 09:28 AM, Wu, Zhongmin wrote:
>> Hi Tomasz
>>
>> Thanks, but I am afraid we have to use the last fence from the last buffer
>> flushing , According to my understanding:
>>
>> If the glFlush was called before swapbuffer ,there would be no new fence
>> after that since the batch buffer may be flushed out, no batch flushing, no
>> fence generated.
>>
>>
>> You can check the function "_intel_batchbuffer_flush_fence", at the very
>> beginning, it will check the content of the buffer, if it is empty, it will
>> return and do nothing.
>> =
>>
>> So, as the same reason, it may not work if we use
>> dri2_dpy->fence->create_fence_fd, the process is below.
>>
>> Swapbuffer (==> flush batch buffer) ==> create_fence_fd(===> flush
>> batch buffer and get fd)
>>
>> You can see, the second buffer flushing may get nothing (not to
>> mention the first buffer flushing may get nothing either)
First of all, the sequence of calls is like this:
- brw_dri_create_fence_fd()
- brw_fence_insert_locked()
- brw_emit_mi_flush() <--- this emits commands to the command buffer...
- intel_batchbuffer_flush_fence() <--- this should flush the buffer and get
a fence
but...
>
> Apologies for jumping into this discussion, and possibly missing some
> context.
>
> FWIW, it should be possible to re-use the previous fence when there is
> no new work to flush. See e.g. commits
>
> f1be3d8cdde1 ("radeonsi: don't flush an empty IB if the only thing we
>need is a fence")
> 800efb0690e9 ("radeonsi: Flush when we're asked to return a fence but
>don't have one yet")
>
> for how the radeonsi driver handles this.
I agree with Michel that it would make more sense if Intel driver kept the last
fence and returned it using when DRI2 fence extension is called.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
Hi Tomasz:
That is right, what we needed is just the "last fence", and then where to
record it and how to get it.
We will check the solutions on radeonsi by Michel, if create_fence_fd can also
get the last fence, of course we can use such DRI2 fence extension API in
queuebuffer and canclebuffer.
-Original Message-
From: Tomasz Figa [mailto:tf...@chromium.org]
Sent: Thursday, July 13, 2017 12:08
To: Michel Dänzer ; Wu, Zhongmin
Cc: Liu, Zhiquan ; Kondapally, Kalyan
; Marathe, Yogesh ; Chad
Versace ; Eric Engestrom ; Emil
Velikov ; Rob Clark ;
Kenneth Graunke ; Widawsky, Benjamin
; ML mesa-dev ;
Kristian H . Kristensen ; Timothy Arceri
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Zhongmin, Michel,
On Thu, Jul 13, 2017 at 12:21 PM, Michel Dänzer wrote:
> On 13/07/17 09:28 AM, Wu, Zhongmin wrote:
>> Hi Tomasz
>>
>> Thanks, but I am afraid we have to use the last fence from the last buffer
>> flushing , According to my understanding:
>>
>> If the glFlush was called before swapbuffer ,there would be no new fence
>> after that since the batch buffer may be flushed out, no batch flushing, no
>> fence generated.
>>
>>
>> You can check the function "_intel_batchbuffer_flush_fence", at the very
>> beginning, it will check the content of the buffer, if it is empty, it will
>> return and do nothing.
>> =
>>
>> So, as the same reason, it may not work if we use
>> dri2_dpy->fence->create_fence_fd, the process is below.
>>
>> Swapbuffer (==> flush batch buffer) ==> create_fence_fd(===> flush
>> batch buffer and get fd)
>>
>> You can see, the second buffer flushing may get nothing (not to
>> mention the first buffer flushing may get nothing either)
First of all, the sequence of calls is like this:
- brw_dri_create_fence_fd()
- brw_fence_insert_locked()
- brw_emit_mi_flush() <--- this emits commands to the command buffer...
- intel_batchbuffer_flush_fence() <--- this should flush the buffer and get
a fence
but...
>
> Apologies for jumping into this discussion, and possibly missing some
> context.
>
> FWIW, it should be possible to re-use the previous fence when there is
> no new work to flush. See e.g. commits
>
> f1be3d8cdde1 ("radeonsi: don't flush an empty IB if the only thing we
>need is a fence")
> 800efb0690e9 ("radeonsi: Flush when we're asked to return a fence but
>don't have one yet")
>
> for how the radeonsi driver handles this.
I agree with Michel that it would make more sense if Intel driver kept the last
fence and returned it using when DRI2 fence extension is called.
Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
Hi Tomasz Thanks, but I am afraid we have to use the last fence from the last buffer flushing , According to my understanding: If the glFlush was called before swapbuffer ,there would be no new fence after that since the batch buffer may be flushed out, no batch flushing, no fence generated. You can check the function "_intel_batchbuffer_flush_fence", at the very beginning, it will check the content of the buffer, if it is empty, it will return and do nothing. = So, as the same reason, it may not work if we use dri2_dpy->fence->create_fence_fd, the process is below. Swapbuffer (==> flush batch buffer) ==> create_fence_fd(===> flush batch buffer and get fd) You can see, the second buffer flushing may get nothing (not to mention the first buffer flushing may get nothing either) -Original Message- From: Tomasz Figa [mailto:tf...@chromium.org] Sent: Wednesday, July 12, 2017 19:24 To: Wu, Zhongmin Cc: Emil Velikov ; Liu, Zhiquan ; ML mesa-dev ; Chad Versace ; Eric Engestrom ; Marathe, Yogesh ; Rob Clark ; Kenneth Graunke ; Widawsky, Benjamin ; Kondapally, Kalyan ; Kristian H . Kristensen ; Timothy Arceri Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS Hi Zhongmin, On Wed, Jul 12, 2017 at 9:55 AM, Wu, Zhongmin 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.veli...@gmail.com] > Sent: Wednesday, July 12, 2017 1:41 > To: Tomasz Figa > Cc: Wu, Zhongmin ; Marathe, Yogesh > ; Widawsky, Benjamin > ; Liu, Zhiquan ; > Eric Engestrom ; Rob Clark > ; Kenneth Graunke ; > Kondapally, Kalyan ; ML mesa-dev > ; Timothy Arceri > ; Kristian H . Kristensen > ; Chad Versace ; > db...@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 wrote: >> Hi Zhongmin, >> >> On Tue, Jul 11, 2017 at 11:02 AM, Wu, Zhongmin 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 >>&
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
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. ) 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. -Original Message- From: Emil Velikov [mailto:emil.l.veli...@gmail.com] Sent: Wednesday, July 12, 2017 1:41 To: Tomasz Figa Cc: Wu, Zhongmin ; Marathe, Yogesh ; Widawsky, Benjamin ; Liu, Zhiquan ; Eric Engestrom ; Rob Clark ; Kenneth Graunke ; Kondapally, Kalyan ; ML mesa-dev ; Timothy Arceri ; Kristian H . Kristensen ; Chad Versace ; db...@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 wrote: > Hi Zhongmin, > > On Tue, Jul 11, 2017 at 11:02 AM, Wu, Zhongmin 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
Add Gao, Shuo
-Original Message-
From: Wu, Zhongmin
Sent: Tuesday, July 11, 2017 10:03
To: 'Emil Velikov' ; Marathe, Yogesh
Cc: Widawsky, Benjamin ; Liu, Zhiquan
; 'Eric Engestrom' ; 'Rob Clark'
; 'Tomasz Figa' ; 'Kenneth
Graunke' ; Kondapally, Kalyan
; 'ML mesa-dev' ;
'Timothy Arceri' ; 'Chuanbo Weng'
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
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.
-Original Message-
From: Wu, Zhongmin
Sent: Tuesday, July 11, 2017 8:40
To: 'Emil Velikov' ; Marathe, Yogesh
Cc: Widawsky, Benjamin ; Liu, Zhiquan
; Eric Engestrom ; Rob Clark
; Tomasz Figa ; Kenneth Graunke
; Kondapally, Kalyan ; ML
mesa-dev ; Timothy Arceri
; Chuanbo Weng
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Emil and Yogesh
Thank you for your comments, and thanks Yogesh for giving the detailed
explanations
And according to the document of Android below
(https://source.android.com/devices/graphics/arch-bq-gralloc):
Recent Android devices support the sync framework, which enables the system to
do nifty things when combined with hardware components that can manipulate
graphics data asynchronously. For example, a producer can submit a series of
OpenGL ES drawing commands and then enqueue the output buffer before rendering
completes. The buffer is accompanied by a fence that signals when the contents
are ready.
I think the things is very clear, that is if the rendering is completed already
when we call queueBuffer() in mesa ? If not, we should queue the buffer with
a fence which will be signaled when the buffer is ready.
-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
Sent: Tuesday, July 11, 2017 1:18
To: Marathe, Yogesh
Cc: Wu, Zhongmin ; Widawsky, Benjamin
; Liu, Zhiquan ; Eric
Engestrom ; Rob Clark ; Tomasz
Figa ; Kenneth Graunke ; Kondapally,
Kalyan ; ML mesa-dev
; Timothy Arceri ;
Chuanbo Weng
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
On 10 July 2017 at 15:26, Marathe, Yogesh wrote:
> Hello Emil, My two cents since I too spent some time on this.
>
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
>> Behalf Of Emil Velikov
>> Sent: Monday, July 10, 2017 4:41 PM
>> To: Wu, Zhongmin
>> Cc: Widawsky, Benjamin ; Liu, Zhiquan
>> ; Eric Engestrom ; Rob
>> Clark ; Tomasz Figa ;
>> Kenneth Graunke ; Kondapally, Kalyan
>> ; ML mesa-dev > d...@lists.freedesktop.org>; Timothy Arceri ;
>> Chuanbo Weng
>> Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
>> Queue the buffer with a sync fence for Android OS
>>
>> Hi Zhongmin Wu,
>>
>> Above all, a bit of a disclaimer: I'm by no means an expert on the
>> topic so take the following with a pinch of salt.
>>
>> On 10 July 2017 at 03:11, Zhongmin Wu wrote:
>> > Before we queued the buffer with a invalid fence (-1), it will make
>> > some benchmarks failed to test such as flatland.
>> >
>> > Now we get the out fence during the flushing buffer and then pass
>> > it to SurfaceFlinger in eglSwapbuffer function.
>> >
>> Having a closer look it seems that the issue can be summarised as follows:
>> - flatland intercepts/interacts ANativeWindow::{de,}queueBuffer (how
>> about
>> ANativeWindow::cancelBuffer?)
>> - the program expects that a sync fd is available for both dequeue
>> and queue
>>
>> At the same time:
>> - the ANativeWindow documentation does _not_ state such requirement
>> - even if it did, that will be somewhat wrong, since
>> ANativeWindow::queueBuffer is called by eglSwapBuffers() Where the
>> latter documentation clearly states - "... performs an implicit flush ...
>> glFlush ...
>> vgFlush"
>>
>> My take is that if flatland/Android framework does want an explicit
>> sync point it should insert one with the EGL API.
>> There could be alternative solutions, but the proposed patch seems
>> wrong IMHO.
>
> In fact, I could work
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
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.
-Original Message-
From: Wu, Zhongmin
Sent: Tuesday, July 11, 2017 8:40
To: 'Emil Velikov' ; Marathe, Yogesh
Cc: Widawsky, Benjamin ; Liu, Zhiquan
; Eric Engestrom ; Rob Clark
; Tomasz Figa ; Kenneth Graunke
; Kondapally, Kalyan ; ML
mesa-dev ; Timothy Arceri
; Chuanbo Weng
Subject: RE: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
Hi Emil and Yogesh
Thank you for your comments, and thanks Yogesh for giving the detailed
explanations
And according to the document of Android below
(https://source.android.com/devices/graphics/arch-bq-gralloc):
Recent Android devices support the sync framework, which enables the system to
do nifty things when combined with hardware components that can manipulate
graphics data asynchronously. For example, a producer can submit a series of
OpenGL ES drawing commands and then enqueue the output buffer before rendering
completes. The buffer is accompanied by a fence that signals when the contents
are ready.
I think the things is very clear, that is if the rendering is completed already
when we call queueBuffer() in mesa ? If not, we should queue the buffer with
a fence which will be signaled when the buffer is ready.
-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
Sent: Tuesday, July 11, 2017 1:18
To: Marathe, Yogesh
Cc: Wu, Zhongmin ; Widawsky, Benjamin
; Liu, Zhiquan ; Eric
Engestrom ; Rob Clark ; Tomasz
Figa ; Kenneth Graunke ; Kondapally,
Kalyan ; ML mesa-dev
; Timothy Arceri ;
Chuanbo Weng
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
On 10 July 2017 at 15:26, Marathe, Yogesh wrote:
> Hello Emil, My two cents since I too spent some time on this.
>
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
>> Behalf Of Emil Velikov
>> Sent: Monday, July 10, 2017 4:41 PM
>> To: Wu, Zhongmin
>> Cc: Widawsky, Benjamin ; Liu, Zhiquan
>> ; Eric Engestrom ; Rob
>> Clark ; Tomasz Figa ;
>> Kenneth Graunke ; Kondapally, Kalyan
>> ; ML mesa-dev > d...@lists.freedesktop.org>; Timothy Arceri ;
>> Chuanbo Weng
>> Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
>> Queue the buffer with a sync fence for Android OS
>>
>> Hi Zhongmin Wu,
>>
>> Above all, a bit of a disclaimer: I'm by no means an expert on the
>> topic so take the following with a pinch of salt.
>>
>> On 10 July 2017 at 03:11, Zhongmin Wu wrote:
>> > Before we queued the buffer with a invalid fence (-1), it will make
>> > some benchmarks failed to test such as flatland.
>> >
>> > Now we get the out fence during the flushing buffer and then pass
>> > it to SurfaceFlinger in eglSwapbuffer function.
>> >
>> Having a closer look it seems that the issue can be summarised as follows:
>> - flatland intercepts/interacts ANativeWindow::{de,}queueBuffer (how
>> about
>> ANativeWindow::cancelBuffer?)
>> - the program expects that a sync fd is available for both dequeue
>> and queue
>>
>> At the same time:
>> - the ANativeWindow documentation does _not_ state such requirement
>> - even if it did, that will be somewhat wrong, since
>> ANativeWindow::queueBuffer is called by eglSwapBuffers() Where the
>> latter documentation clearly states - "... performs an implicit flush ...
>> glFlush ...
>> vgFlush"
>>
>> My take is that if flatland/Android framework does want an explicit
>> sync point it should insert one with the EGL API.
>> There could be alternative solutions, but the proposed patch seems
>> wrong IMHO.
>
> In fact, I could work this around in producer (Surface::queueBuffer)
> by ignoring the (-1) passed and by creating a sync using egl APIs. I see two
> problems with that.
>
> - Before getting a fd using eglDupNativeFenceFDANDROID(), you need a
> glFlush(),
>this costs additional cycles for each queueBuffer transaction on each
> BufferItem and
>I believe fd is also signaled due to this. (so I don’t know what we'll get
> by waiting on
>that fd on consumer side).
> - AFAIK, the whole idea o
Re: [Mesa-dev] [EGL android: accquire fence implementation] i965: Queue the buffer with a sync fence for Android OS
Hi Emil and Yogesh
Thank you for your comments, and thanks Yogesh for giving the detailed
explanations
And according to the document of Android below
(https://source.android.com/devices/graphics/arch-bq-gralloc):
Recent Android devices support the sync framework, which enables the system to
do nifty things when combined with hardware components that can manipulate
graphics data asynchronously. For example, a producer can submit a series of
OpenGL ES drawing commands and then enqueue the output buffer before rendering
completes. The buffer is accompanied by a fence that signals when the contents
are ready.
I think the things is very clear, that is if the rendering is completed already
when we call queueBuffer() in mesa ? If not, we should queue the buffer with
a fence which will be signaled when the buffer is ready.
-Original Message-
From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
Sent: Tuesday, July 11, 2017 1:18
To: Marathe, Yogesh
Cc: Wu, Zhongmin ; Widawsky, Benjamin
; Liu, Zhiquan ; Eric
Engestrom ; Rob Clark ; Tomasz
Figa ; Kenneth Graunke ; Kondapally,
Kalyan ; ML mesa-dev
; Timothy Arceri ;
Chuanbo Weng
Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
Queue the buffer with a sync fence for Android OS
On 10 July 2017 at 15:26, Marathe, Yogesh wrote:
> Hello Emil, My two cents since I too spent some time on this.
>
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
>> Behalf Of Emil Velikov
>> Sent: Monday, July 10, 2017 4:41 PM
>> To: Wu, Zhongmin
>> Cc: Widawsky, Benjamin ; Liu, Zhiquan
>> ; Eric Engestrom ; Rob
>> Clark ; Tomasz Figa ;
>> Kenneth Graunke ; Kondapally, Kalyan
>> ; ML mesa-dev > d...@lists.freedesktop.org>; Timothy Arceri ;
>> Chuanbo Weng
>> Subject: Re: [Mesa-dev] [EGL android: accquire fence implementation] i965:
>> Queue the buffer with a sync fence for Android OS
>>
>> Hi Zhongmin Wu,
>>
>> Above all, a bit of a disclaimer: I'm by no means an expert on the
>> topic so take the following with a pinch of salt.
>>
>> On 10 July 2017 at 03:11, Zhongmin Wu wrote:
>> > Before we queued the buffer with a invalid fence (-1), it will make
>> > some benchmarks failed to test such as flatland.
>> >
>> > Now we get the out fence during the flushing buffer and then pass
>> > it to SurfaceFlinger in eglSwapbuffer function.
>> >
>> Having a closer look it seems that the issue can be summarised as follows:
>> - flatland intercepts/interacts ANativeWindow::{de,}queueBuffer (how
>> about
>> ANativeWindow::cancelBuffer?)
>> - the program expects that a sync fd is available for both dequeue
>> and queue
>>
>> At the same time:
>> - the ANativeWindow documentation does _not_ state such requirement
>> - even if it did, that will be somewhat wrong, since
>> ANativeWindow::queueBuffer is called by eglSwapBuffers() Where the
>> latter documentation clearly states - "... performs an implicit flush ...
>> glFlush ...
>> vgFlush"
>>
>> My take is that if flatland/Android framework does want an explicit
>> sync point it should insert one with the EGL API.
>> There could be alternative solutions, but the proposed patch seems
>> wrong IMHO.
>
> In fact, I could work this around in producer (Surface::queueBuffer)
> by ignoring the (-1) passed and by creating a sync using egl APIs. I see two
> problems with that.
>
> - Before getting a fd using eglDupNativeFenceFDANDROID(), you need a
> glFlush(),
>this costs additional cycles for each queueBuffer transaction on each
> BufferItem and
>I believe fd is also signaled due to this. (so I don’t know what we'll get
> by waiting on
>that fd on consumer side).
> - AFAIK, the whole idea of explicit sync revolves around being able to pass
> fds created
> by driver between processes and this one breaks that chain. If we work this
> around in
> upper layers, explicit sync feature will have to be fixed for every other
> lib that may use
> lib mesa underneath.
>
> For these reasons, I still believe we should fix it here. Of course,
> you and Rob have very valid points on cancelBuffer and about not
> breaking gallium respectively, those need to be taken care of.
>
What I'm saying is - seems like the app/framework does something silly or at
least undocumented.
Fixing things in Mesa may be the right thing to do, but without more
information, its everyone's guess who's got it wrong.
As Rob asked earlier - can we get an a simple test case or some pseudo code
illustrating the whole thing?
Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
