Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Mauro Rossi]

2017-04-04 0:54 GMT+02:00 Mauro Rossi :

> Hi Tomasz,
>
>
> 2017-04-03 10:00 GMT+02:00 Tomasz Figa :
>
>> Hi Mauro,
>>
>> On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi 
>> wrote:
>> >
>> >
>> > 2017-03-31 13:05 GMT+02:00 Tapani Pälli :
>> >>
>> >>
>> >>
>> >> On 03/31/2017 10:12 AM, Tapani Pälli wrote:
>> >>>
>> >>>
>> >>>
>> >>> On 03/31/2017 09:06 AM, Tapani Pälli wrote:
>> 
>> 
>> 
>>  On 03/31/2017 08:24 AM, Rob Clark wrote:
>> >
>> > On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
>> >  wrote:
>> >>
>> >>
>> >>
>> >> On 03/30/2017 05:57 PM, Emil Velikov wrote:
>> >>>
>> >>>
>> >>> On 30 March 2017 at 15:30, Tomasz Figa 
>> wrote:
>> 
>> 
>>  On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
>>  
>>  wrote:
>> >
>> >
>> >
>> > On 30 March 2017 at 11:55, Tomasz Figa 
>> wrote:
>> >>
>> >>
>> >> Android buffer queues can be abandoned, which results in
>> failing
>> >> to
>> >> dequeue next buffer. Currently this would fail somewhere deep
>> >> within
>> >> the DRI stack calling loader's getBuffers*(), without any error
>> >> reporting to the client app. However Android framework code
>> >> relies on
>> >> proper signaling of this event, so we move buffer dequeue to
>> >> createWindowSurface() and swapBuffers() call, which can
>> generate
>> >> proper
>> >> EGL errors. To keep the performance benefits of delayed buffer
>> >> handling,
>> >> if any, fence wait and DRI image creation is kept delayed until
>> >> getBuffers*() is called by the DRI driver.
>> >>
>> > Thank you Tomasz.
>> >
>> > I'm fairly confident that this should resolve the crash [in
>> > swap_buffers] that Mauro was seeing.
>> > Mauro can you give it a test ?
>> 
>> 
>> 
>>  Ah, I actually noticed a problem with existing code, supposedly
>>  fixed
>>  by [1], but I'm afraid it's still wrong.
>> 
>>  Current swap_buffers calls get_back_bo(), but doesn't call
>>  update_buffers(), which is the function that should be called
>> before
>>  to actually dequeue a buffer from Android's buffer queue. Given
>>  that,
>>  get_back_bo() would simply fail with !dri2_surf->buffer, because
>> no
>>  buffer was dequeued.
>> 
>> >>> Right - I was wondering why we don't hit that on EGL/GBM or
>> >>> EGL/Wayland.
>> >>> From a quick look - may be because EGL/Android drops the dpy
>> mutex in
>> >>> droid_window_enqueue_buffer().
>> >>>
>>  My patch removes update_buffers() and changes the buffer
>>  management so
>>  that there is always a buffer dequeued, starting from surface
>>  creation, unless there was an error somewhere.
>> 
>> >>> Of the top of your head - is there something stopping us from
>> using
>> >>> the same method on $other platforms?
>> >>>
>>  [1]
>> 
>>  https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/driver
>> s/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597
>> 
>> 
>> 
>> >
>> >
>> > Not that huge of an expert on the Android specifics, so just a
>> > humble
>> > request:
>> > Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
>> >>>
>> >>>
>> >>> Oops silly typo - s/seek/split/.
>> >>>
>> > other?) separate from the functionality changes ?
>> 
>> 
>> 
>>  Sure. Thanks for suggestion.
>> 
>> >>> Please give it a day or two for others to comment.
>> >>
>> >>
>> >>
>> >> I'm trying to debug why this causes our homescreen (wallpaper) to
>> be
>> >> black.
>> >> Otherwise I haven't seen any issues with these changes.
>> >>
>> >
>> > wallpaper seems to be a special sorta hell..  I wonder if there is
>> > somehow some sort of interaction with what I fixed / worked-around
>> in
>> > a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
>> >
>> > Maybe at least try commenting out the temp-pbuffer thing to get max
>> > texture size, and see if that "fixes" things
>> 
>> 
>>  Can you give more details, I still live in la la land and don't know
>>  about 'temp-pbuffer thing'?
>> 
>> >>>
>> >>> aa I did not recall the problem, you mean the 'dummy pbuffer' in
>> >>> SurfaceFlinger .. yes I will check if this is related.
>> >>>
>> >>
>> >> If I take away that dummy pbuffer usage (which is useless anyway),
>> couple
>> >> of errors 

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Mauro Rossi]

Hi Tomasz,

2017-04-03 10:00 GMT+02:00 Tomasz Figa :

> Hi Mauro,
>
> On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi  wrote:
> >
> >
> > 2017-03-31 13:05 GMT+02:00 Tapani Pälli :
> >>
> >>
> >>
> >> On 03/31/2017 10:12 AM, Tapani Pälli wrote:
> >>>
> >>>
> >>>
> >>> On 03/31/2017 09:06 AM, Tapani Pälli wrote:
> 
> 
> 
>  On 03/31/2017 08:24 AM, Rob Clark wrote:
> >
> > On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
> >  wrote:
> >>
> >>
> >>
> >> On 03/30/2017 05:57 PM, Emil Velikov wrote:
> >>>
> >>>
> >>> On 30 March 2017 at 15:30, Tomasz Figa  wrote:
> 
> 
>  On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
>  
>  wrote:
> >
> >
> >
> > On 30 March 2017 at 11:55, Tomasz Figa 
> wrote:
> >>
> >>
> >> Android buffer queues can be abandoned, which results in failing
> >> to
> >> dequeue next buffer. Currently this would fail somewhere deep
> >> within
> >> the DRI stack calling loader's getBuffers*(), without any error
> >> reporting to the client app. However Android framework code
> >> relies on
> >> proper signaling of this event, so we move buffer dequeue to
> >> createWindowSurface() and swapBuffers() call, which can generate
> >> proper
> >> EGL errors. To keep the performance benefits of delayed buffer
> >> handling,
> >> if any, fence wait and DRI image creation is kept delayed until
> >> getBuffers*() is called by the DRI driver.
> >>
> > Thank you Tomasz.
> >
> > I'm fairly confident that this should resolve the crash [in
> > swap_buffers] that Mauro was seeing.
> > Mauro can you give it a test ?
> 
> 
> 
>  Ah, I actually noticed a problem with existing code, supposedly
>  fixed
>  by [1], but I'm afraid it's still wrong.
> 
>  Current swap_buffers calls get_back_bo(), but doesn't call
>  update_buffers(), which is the function that should be called
> before
>  to actually dequeue a buffer from Android's buffer queue. Given
>  that,
>  get_back_bo() would simply fail with !dri2_surf->buffer, because
> no
>  buffer was dequeued.
> 
> >>> Right - I was wondering why we don't hit that on EGL/GBM or
> >>> EGL/Wayland.
> >>> From a quick look - may be because EGL/Android drops the dpy mutex
> in
> >>> droid_window_enqueue_buffer().
> >>>
>  My patch removes update_buffers() and changes the buffer
>  management so
>  that there is always a buffer dequeued, starting from surface
>  creation, unless there was an error somewhere.
> 
> >>> Of the top of your head - is there something stopping us from using
> >>> the same method on $other platforms?
> >>>
>  [1]
> 
>  https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/
> drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581
> 149dbe1597
> 
> 
> 
> >
> >
> > Not that huge of an expert on the Android specifics, so just a
> > humble
> > request:
> > Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
> >>>
> >>>
> >>> Oops silly typo - s/seek/split/.
> >>>
> > other?) separate from the functionality changes ?
> 
> 
> 
>  Sure. Thanks for suggestion.
> 
> >>> Please give it a day or two for others to comment.
> >>
> >>
> >>
> >> I'm trying to debug why this causes our homescreen (wallpaper) to be
> >> black.
> >> Otherwise I haven't seen any issues with these changes.
> >>
> >
> > wallpaper seems to be a special sorta hell..  I wonder if there is
> > somehow some sort of interaction with what I fixed / worked-around in
> > a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
> >
> > Maybe at least try commenting out the temp-pbuffer thing to get max
> > texture size, and see if that "fixes" things
> 
> 
>  Can you give more details, I still live in la la land and don't know
>  about 'temp-pbuffer thing'?
> 
> >>>
> >>> aa I did not recall the problem, you mean the 'dummy pbuffer' in
> >>> SurfaceFlinger .. yes I will check if this is related.
> >>>
> >>
> >> If I take away that dummy pbuffer usage (which is useless anyway),
> couple
> >> of errors disappear from the log. They are:
> >>
> >> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
> >> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
> >>
> >> but otherwise the desktop still stays black, 

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 03/31/2017 04:04 PM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 2:06 AM, Tapani Pälli  wrote:



On 03/31/2017 08:24 AM, Rob Clark wrote:


On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli 
wrote:




On 03/30/2017 05:57 PM, Emil Velikov wrote:



On 30 March 2017 at 15:30, Tomasz Figa  wrote:



On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov

wrote:




On 30 March 2017 at 11:55, Tomasz Figa  wrote:



Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate
proper
EGL errors. To keep the performance benefits of delayed buffer
handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?




Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.


Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().


My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.


Of the top of your head - is there something stopping us from using
the same method on $other platforms?


[1]

https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597




Not that huge of an expert on the Android specifics, so just a humble
request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,



Oops silly typo - s/seek/split/.


other?) separate from the functionality changes ?




Sure. Thanks for suggestion.


Please give it a day or two for others to comment.




I'm trying to debug why this causes our homescreen (wallpaper) to be
black.
Otherwise I haven't seen any issues with these changes.



wallpaper seems to be a special sorta hell..  I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things



Can you give more details, I still live in la la land and don't know about
'temp-pbuffer thing'?



Sorry, this is something in the wallpaper app java code.. I forget
exactly where it lives now, but there was some code matching what I
pasted in the commit msg which was creating and then destroying a
temporary pbuffer..



OK yeah found it, it's very similar to what surfaceflinger does also on 
startup before initializing it's rendering engine. It creates temporary 
pbuffer only to query different things before actual init.


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

Hi Mauro,

On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi  wrote:
>
>
> 2017-03-31 13:05 GMT+02:00 Tapani Pälli :
>>
>>
>>
>> On 03/31/2017 10:12 AM, Tapani Pälli wrote:
>>>
>>>
>>>
>>> On 03/31/2017 09:06 AM, Tapani Pälli wrote:



 On 03/31/2017 08:24 AM, Rob Clark wrote:
>
> On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
>  wrote:
>>
>>
>>
>> On 03/30/2017 05:57 PM, Emil Velikov wrote:
>>>
>>>
>>> On 30 March 2017 at 15:30, Tomasz Figa  wrote:


 On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
 
 wrote:
>
>
>
> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>>
>>
>> Android buffer queues can be abandoned, which results in failing
>> to
>> dequeue next buffer. Currently this would fail somewhere deep
>> within
>> the DRI stack calling loader's getBuffers*(), without any error
>> reporting to the client app. However Android framework code
>> relies on
>> proper signaling of this event, so we move buffer dequeue to
>> createWindowSurface() and swapBuffers() call, which can generate
>> proper
>> EGL errors. To keep the performance benefits of delayed buffer
>> handling,
>> if any, fence wait and DRI image creation is kept delayed until
>> getBuffers*() is called by the DRI driver.
>>
> Thank you Tomasz.
>
> I'm fairly confident that this should resolve the crash [in
> swap_buffers] that Mauro was seeing.
> Mauro can you give it a test ?



 Ah, I actually noticed a problem with existing code, supposedly
 fixed
 by [1], but I'm afraid it's still wrong.

 Current swap_buffers calls get_back_bo(), but doesn't call
 update_buffers(), which is the function that should be called before
 to actually dequeue a buffer from Android's buffer queue. Given
 that,
 get_back_bo() would simply fail with !dri2_surf->buffer, because no
 buffer was dequeued.

>>> Right - I was wondering why we don't hit that on EGL/GBM or
>>> EGL/Wayland.
>>> From a quick look - may be because EGL/Android drops the dpy mutex in
>>> droid_window_enqueue_buffer().
>>>
 My patch removes update_buffers() and changes the buffer
 management so
 that there is always a buffer dequeued, starting from surface
 creation, unless there was an error somewhere.

>>> Of the top of your head - is there something stopping us from using
>>> the same method on $other platforms?
>>>
 [1]

 https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597



>
>
> Not that huge of an expert on the Android specifics, so just a
> humble
> request:
> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
>>>
>>>
>>> Oops silly typo - s/seek/split/.
>>>
> other?) separate from the functionality changes ?



 Sure. Thanks for suggestion.

>>> Please give it a day or two for others to comment.
>>
>>
>>
>> I'm trying to debug why this causes our homescreen (wallpaper) to be
>> black.
>> Otherwise I haven't seen any issues with these changes.
>>
>
> wallpaper seems to be a special sorta hell..  I wonder if there is
> somehow some sort of interaction with what I fixed / worked-around in
> a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
>
> Maybe at least try commenting out the temp-pbuffer thing to get max
> texture size, and see if that "fixes" things


 Can you give more details, I still live in la la land and don't know
 about 'temp-pbuffer thing'?

>>>
>>> aa I did not recall the problem, you mean the 'dummy pbuffer' in
>>> SurfaceFlinger .. yes I will check if this is related.
>>>
>>
>> If I take away that dummy pbuffer usage (which is useless anyway), couple
>> of errors disappear from the log. They are:
>>
>> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
>> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
>>
>> but otherwise the desktop still stays black, live wallpapers seem to work
>> so there is something special about this default wallpaper. Will continue
>> digging ..
>>
>> // Tapani
>
>
>
> Hi,
>
> about the black wallpaper the only sign found in logcat is the following:
>
> - beginning of main
> 04-02 15:09:43.148
> ...
> 04-02 15:10:41.710  1414  1414 E Layer   :
> 

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

On Mon, Apr 3, 2017 at 2:42 PM, Tapani Pälli  wrote:
>
>
> On 04/02/2017 08:12 PM, Tomasz Figa wrote:
>>
>> Sorry for replying to myself, just got enlightened...
>>
>> On Mon, Apr 3, 2017 at 2:07 AM, Tomasz Figa  wrote:
>>>
>>> Hi Mauro,
>>>
>>> On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi 
>>> wrote:



 2017-03-30 16:17 GMT+02:00 Emil Velikov :
>
>
> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>>
>> Android buffer queues can be abandoned, which results in failing to
>> dequeue next buffer. Currently this would fail somewhere deep within
>> the DRI stack calling loader's getBuffers*(), without any error
>> reporting to the client app. However Android framework code relies on
>> proper signaling of this event, so we move buffer dequeue to
>> createWindowSurface() and swapBuffers() call, which can generate
>> proper
>> EGL errors. To keep the performance benefits of delayed buffer
>> handling,
>> if any, fence wait and DRI image creation is kept delayed until
>> getBuffers*() is called by the DRI driver.
>>
> Thank you Tomasz.
>
> I'm fairly confident that this should resolve the crash [in
> swap_buffers] that Mauro was seeing.
> Mauro can you give it a test ?



 After applying last version of Tomasz patch,
 I could not boot nougat-x86, the same way as per Tapani get_back_bo()
 throwing and EGL_BAD_ALLOC
 which is a show stopper for surfaceflinger
>>>
>>>
>>> Hmm, must be something I missed in the code, because with my patch
>>> applied, there should be no condition that could make get_back_bo()
>>> fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
>>> or there is something wrong with the first buffer being dequeued in
>>> create_surface(). Would you be able to check where exactly
>>> get_back_bo() fails with your setup?
>>
>>
>> Ah, wait, I just realized that get_back_bo() is valid only when the
>> image loader is used, which is only on DMA-buf FD-based systems. No
>> wonder that it fails on android-x86.
>>
>> Tapani, would you be able to give a bit more details on the crash
>> being observed without that call? AFAICT, without my patch, even with
>> the call, the code is still not fully correct, because on the first
>> call to swap_buffers() without any rendering the dri2_surf->buffer
>> would be NULL and get_back_bo() would simply fail (but not crash
>> indeed). With my patch, it won't fail, because there is always a
>> buffer dequeued, so it should be the closest to correct behavior.
>
>
> Reason why that crash started to happen (observed only in one app so far!)
> were changes done in 'EGL/Android: Add EGL_EXT_buffer_age extension'. That
> patch adds 'age' for buffers and modifies droid_swap_buffers to set back
> buffer age to value 1. My original patch was to set the age only if there is
> a back buffer but then discussed with Emil and decided to unify approach
> with other backends and call get_back_bo.
>
> You are right that in the crash case dri2_surf->buffer is NULL as well.
>
> for long time flow looks like this:
>
> 01-01 00:00:54.087  2771  2791 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0xc8c88788 dri2_surf->back 0xca35ede8
> 01-01 00:00:54.104  2771  2791 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0xc8e84e08 dri2_surf->back 0xca35edd8
> 01-01 00:00:54.120 2771 2791 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0
>
> then all of sudden there is a frame where buffer and back are 0:
>
> 01-01 00:00:55.386  2771  2791 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0
> 01-01 00:00:55.390  2771  2831 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0xc8c8b2a8 dri2_surf->back 0xca360098
> 01-01 00:00:55.391  2771  2831 D EGL-DRI2: droid_swap_buffers:
> dri2_surf->buffer 0x0 dri2_surf->back 0x0

I guess it means that there was no rendering happening between the two
swaps above, so my patch should take care of this case indeed and the
call to get_back_bo() is not needed anymore (actually it should have
been update_buffers(), not get_back_bo()).

Thanks for really helpful explanation.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 04/02/2017 08:12 PM, Tomasz Figa wrote:

Sorry for replying to myself, just got enlightened...

On Mon, Apr 3, 2017 at 2:07 AM, Tomasz Figa  wrote:

Hi Mauro,

On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi  wrote:



2017-03-30 16:17 GMT+02:00 Emil Velikov :


On 30 March 2017 at 11:55, Tomasz Figa  wrote:

Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate proper
EGL errors. To keep the performance benefits of delayed buffer handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?



After applying last version of Tomasz patch,
I could not boot nougat-x86, the same way as per Tapani get_back_bo()
throwing and EGL_BAD_ALLOC
which is a show stopper for surfaceflinger


Hmm, must be something I missed in the code, because with my patch
applied, there should be no condition that could make get_back_bo()
fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
or there is something wrong with the first buffer being dequeued in
create_surface(). Would you be able to check where exactly
get_back_bo() fails with your setup?


Ah, wait, I just realized that get_back_bo() is valid only when the
image loader is used, which is only on DMA-buf FD-based systems. No
wonder that it fails on android-x86.

Tapani, would you be able to give a bit more details on the crash
being observed without that call? AFAICT, without my patch, even with
the call, the code is still not fully correct, because on the first
call to swap_buffers() without any rendering the dri2_surf->buffer
would be NULL and get_back_bo() would simply fail (but not crash
indeed). With my patch, it won't fail, because there is always a
buffer dequeued, so it should be the closest to correct behavior.


Reason why that crash started to happen (observed only in one app so 
far!) were changes done in 'EGL/Android: Add EGL_EXT_buffer_age 
extension'. That patch adds 'age' for buffers and modifies 
droid_swap_buffers to set back buffer age to value 1. My original patch 
was to set the age only if there is a back buffer but then discussed 
with Emil and decided to unify approach with other backends and call 
get_back_bo.


You are right that in the crash case dri2_surf->buffer is NULL as well.

for long time flow looks like this:

01-01 00:00:54.087  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c88788 dri2_surf->back 0xca35ede8
01-01 00:00:54.104  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8e84e08 dri2_surf->back 0xca35edd8
01-01 00:00:54.120  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0


then all of sudden there is a frame where buffer and back are 0:

01-01 00:00:55.386  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0
01-01 00:00:55.390  2771  2831 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c8b2a8 dri2_surf->back 0xca360098
01-01 00:00:55.391  2771  2831 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0x0 dri2_surf->back 0x0
01-01 00:00:55.395  1943  2088 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0x7c449f5ed610 dri2_surf->back 0x7c44b2cfc010


then we crash:

01-01 00:00:56.101  2836  2836 F DEBUG   : backtrace:
01-01 00:00:56.101  2836  2836 F DEBUG   : #00 pc 00014046 
/system/lib/egl/libGLES_mesa.so (droid_swap_buffers+166)
01-01 00:00:56.101  2836  2836 F DEBUG   : #01 pc 00011832 
/system/lib/egl/libGLES_mesa.so (dri2_swap_buffers+50)
01-01 00:00:56.101  2836  2836 F DEBUG   : #02 pc 58c2 
/system/lib/egl/libGLES_mesa.so (eglSwapBuffers+386)
01-01 00:00:56.101  2836  2836 F DEBUG   : #03 pc 00011329 
/system/lib/libEGL.so (eglSwapBuffersWithDamageKHR+553)
01-01 00:00:56.101  2836  2836 F DEBUG   : #04 pc 000118e7 
/system/lib/libEGL.so (eglSwapBuffers+55)
01-01 00:00:56.101  2836  2836 F DEBUG   : #05 pc 000754dc 
/system/lib/libandroid_runtime.so
01-01 00:00:56.101  2836  2836 F DEBUG   : #06 pc 0249217c 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(com.google.android.gles_jni.EGLImpl.eglSwapBuffers+168)
01-01 00:00:56.101  2836  2836 F DEBUG   : #07 pc 01cc8dec 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(android.opengl.GLSurfaceView$EglHelper.swap+72)
01-01 00:00:56.101  2836  2836 F DEBUG   : #08 pc 01cc9ebe 
/system/framework/x86/boot-framework.oat (offset 

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 04/02/2017 07:38 PM, Mauro Rossi wrote:



2017-03-30 16:17 GMT+02:00 Emil Velikov >:

On 30 March 2017 at 11:55, Tomasz Figa > wrote:
> Android buffer queues can be abandoned, which results in failing to
> dequeue next buffer. Currently this would fail somewhere deep within
> the DRI stack calling loader's getBuffers*(), without any error
> reporting to the client app. However Android framework code relies on
> proper signaling of this event, so we move buffer dequeue to
> createWindowSurface() and swapBuffers() call, which can generate
proper
> EGL errors. To keep the performance benefits of delayed buffer
handling,
> if any, fence wait and DRI image creation is kept delayed until
> getBuffers*() is called by the DRI driver.
>
Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?


After applying last version of Tomasz patch,
I could not boot nougat-x86, the same way as per Tapani get_back_bo()
throwing and EGL_BAD_ALLOC
which is a show stopper for surfaceflinger

So I reverted [1] and now I can boot and I also see the black wallpaper
like Tapani.
dumpsys SurfaceFlinger output shows a buffer allocated,
but for some reason both HWC and GLES composition (used in nougat-x86)
show black wallpaper.


h/w composer state:
  h/w composer not present and enabled
Allocated buffers:
...
0x7b60f301e380: 29440.00 KiB | 2880 (2944) x 2560 |5 |
0x0900 | com.android.systemui.ImageWallpaper
...
Total allocated (estimate): 54728.50 KB




Not that huge of an expert on the Android specifics, so just a
humble request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
other?) separate from the functionality changes ?

-Emil


I'd also kindly request to confirm the test environment used to verify
Tomasz patch v2,
which in my understanding has been the following, common between
ChomiumOS and Android-IA:

  * minigbm based gralloc
  * dma FDs for buffers
  * kernel based explicit fences with FDs
  * HWC2 for compositing
  * (?) Render nodes - but I don't know if/when they are used

In android-x86 (nougat-x86) situation is the following:

  * drm_gralloc based gralloc
  * buffer handles
  * Not 100% sure about sync/fences, but I don't recall about using
explicit fences with FDs
  * GLES for compositing
  * we don't use render nodes

Pardon me if this seems a long checklist or if it's not 100% accurate,
but I would assume that this patch should just work Out-Of-The-Box with
android-x86 (nougat-x86)
even if most of CrOS/Android-IA optimizations are not (yet) used there.

Is this assumption correct?

In this moment the only way to boot nougat-x86 is to revert [1]
but besides this and black wallpaper, which both require investigation,
I've not seen any particular regression.

Thanks for feeedbacks
Mauro

PS: Question for Tapani: if you apply Tomasz patch and revert [1], do
you still see the segfault in Android-IA?


No, that patch is not needed anymore as it seems now there is always a 
back buffer available when coming to droid_swap_buffers. I tested with 
the 3DMark app that caused that crash.




[1]
 
https://cgit.freedesktop.org/mesa/mesa/commit/?id=4d4558411db166d2d66f8cec9cb581149dbe1597






// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Mauro Rossi]

2017-03-31 13:05 GMT+02:00 Tapani Pälli :

>
>
> On 03/31/2017 10:12 AM, Tapani Pälli wrote:
>
>>
>>
>> On 03/31/2017 09:06 AM, Tapani Pälli wrote:
>>
>>>
>>>
>>> On 03/31/2017 08:24 AM, Rob Clark wrote:
>>>
 On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
  wrote:

>
>
> On 03/30/2017 05:57 PM, Emil Velikov wrote:
>
>>
>> On 30 March 2017 at 15:30, Tomasz Figa  wrote:
>>
>>>
>>> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
>>> 
>>> wrote:
>>>


 On 30 March 2017 at 11:55, Tomasz Figa  wrote:

>
> Android buffer queues can be abandoned, which results in failing to
> dequeue next buffer. Currently this would fail somewhere deep
> within
> the DRI stack calling loader's getBuffers*(), without any error
> reporting to the client app. However Android framework code
> relies on
> proper signaling of this event, so we move buffer dequeue to
> createWindowSurface() and swapBuffers() call, which can generate
> proper
> EGL errors. To keep the performance benefits of delayed buffer
> handling,
> if any, fence wait and DRI image creation is kept delayed until
> getBuffers*() is called by the DRI driver.
>
> Thank you Tomasz.

 I'm fairly confident that this should resolve the crash [in
 swap_buffers] that Mauro was seeing.
 Mauro can you give it a test ?

>>>
>>>
>>> Ah, I actually noticed a problem with existing code, supposedly fixed
>>> by [1], but I'm afraid it's still wrong.
>>>
>>> Current swap_buffers calls get_back_bo(), but doesn't call
>>> update_buffers(), which is the function that should be called before
>>> to actually dequeue a buffer from Android's buffer queue. Given that,
>>> get_back_bo() would simply fail with !dri2_surf->buffer, because no
>>> buffer was dequeued.
>>>
>>> Right - I was wondering why we don't hit that on EGL/GBM or
>> EGL/Wayland.
>> From a quick look - may be because EGL/Android drops the dpy mutex in
>> droid_window_enqueue_buffer().
>>
>> My patch removes update_buffers() and changes the buffer
>>> management so
>>> that there is always a buffer dequeued, starting from surface
>>> creation, unless there was an error somewhere.
>>>
>>> Of the top of your head - is there something stopping us from using
>> the same method on $other platforms?
>>
>> [1]
>>> https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/driver
>>> s/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb5811
>>> 49dbe1597
>>>
>>>
>>>
>>>

 Not that huge of an expert on the Android specifics, so just a
 humble
 request:
 Can we seek the code resuffle (droid_{alloc,free}_local_buffer,

>>>
>> Oops silly typo - s/seek/split/.
>>
>> other?) separate from the functionality changes ?

>>>
>>>
>>> Sure. Thanks for suggestion.
>>>
>>> Please give it a day or two for others to comment.
>>
>
>
> I'm trying to debug why this causes our homescreen (wallpaper) to be
> black.
> Otherwise I haven't seen any issues with these changes.
>
>
 wallpaper seems to be a special sorta hell..  I wonder if there is
 somehow some sort of interaction with what I fixed / worked-around in
 a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

 Maybe at least try commenting out the temp-pbuffer thing to get max
 texture size, and see if that "fixes" things

>>>
>>> Can you give more details, I still live in la la land and don't know
>>> about 'temp-pbuffer thing'?
>>>
>>>
>> aa I did not recall the problem, you mean the 'dummy pbuffer' in
>> SurfaceFlinger .. yes I will check if this is related.
>>
>>
> If I take away that dummy pbuffer usage (which is useless anyway), couple
> of errors disappear from the log. They are:
>
> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
> SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
>
> but otherwise the desktop still stays black, live wallpapers seem to work
> so there is something special about this default wallpaper. Will continue
> digging ..
>
> // Tapani
>


Hi,

about the black wallpaper the only sign found in logcat is the following:

- beginning of main
04-02 15:09:43.148
...
04-02 15:10:41.710  1414  1414 E Layer   :
[com.android.systemui.ImageWallpaper] rejecting buffer: bufWidth=1024,
bufHeight=768, front.active.{w=1, h=1}
...
04-02 15:10:41.726  1414  1414 E Layer   :
[com.android.systemui.ImageWallpaper] rejecting buffer: bufWidth=1024,
bufHeight=768, front.active.{w=1, h=1}

Are the expected width and 

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

Sorry for replying to myself, just got enlightened...

On Mon, Apr 3, 2017 at 2:07 AM, Tomasz Figa  wrote:
> Hi Mauro,
>
> On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi  wrote:
>>
>>
>> 2017-03-30 16:17 GMT+02:00 Emil Velikov :
>>>
>>> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>>> > Android buffer queues can be abandoned, which results in failing to
>>> > dequeue next buffer. Currently this would fail somewhere deep within
>>> > the DRI stack calling loader's getBuffers*(), without any error
>>> > reporting to the client app. However Android framework code relies on
>>> > proper signaling of this event, so we move buffer dequeue to
>>> > createWindowSurface() and swapBuffers() call, which can generate proper
>>> > EGL errors. To keep the performance benefits of delayed buffer handling,
>>> > if any, fence wait and DRI image creation is kept delayed until
>>> > getBuffers*() is called by the DRI driver.
>>> >
>>> Thank you Tomasz.
>>>
>>> I'm fairly confident that this should resolve the crash [in
>>> swap_buffers] that Mauro was seeing.
>>> Mauro can you give it a test ?
>>
>>
>> After applying last version of Tomasz patch,
>> I could not boot nougat-x86, the same way as per Tapani get_back_bo()
>> throwing and EGL_BAD_ALLOC
>> which is a show stopper for surfaceflinger
>
> Hmm, must be something I missed in the code, because with my patch
> applied, there should be no condition that could make get_back_bo()
> fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
> or there is something wrong with the first buffer being dequeued in
> create_surface(). Would you be able to check where exactly
> get_back_bo() fails with your setup?

Ah, wait, I just realized that get_back_bo() is valid only when the
image loader is used, which is only on DMA-buf FD-based systems. No
wonder that it fails on android-x86.

Tapani, would you be able to give a bit more details on the crash
being observed without that call? AFAICT, without my patch, even with
the call, the code is still not fully correct, because on the first
call to swap_buffers() without any rendering the dri2_surf->buffer
would be NULL and get_back_bo() would simply fail (but not crash
indeed). With my patch, it won't fail, because there is always a
buffer dequeued, so it should be the closest to correct behavior.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

Hi Mauro,

On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi  wrote:
>
>
> 2017-03-30 16:17 GMT+02:00 Emil Velikov :
>>
>> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>> > Android buffer queues can be abandoned, which results in failing to
>> > dequeue next buffer. Currently this would fail somewhere deep within
>> > the DRI stack calling loader's getBuffers*(), without any error
>> > reporting to the client app. However Android framework code relies on
>> > proper signaling of this event, so we move buffer dequeue to
>> > createWindowSurface() and swapBuffers() call, which can generate proper
>> > EGL errors. To keep the performance benefits of delayed buffer handling,
>> > if any, fence wait and DRI image creation is kept delayed until
>> > getBuffers*() is called by the DRI driver.
>> >
>> Thank you Tomasz.
>>
>> I'm fairly confident that this should resolve the crash [in
>> swap_buffers] that Mauro was seeing.
>> Mauro can you give it a test ?
>
>
> After applying last version of Tomasz patch,
> I could not boot nougat-x86, the same way as per Tapani get_back_bo()
> throwing and EGL_BAD_ALLOC
> which is a show stopper for surfaceflinger

Hmm, must be something I missed in the code, because with my patch
applied, there should be no condition that could make get_back_bo()
fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
or there is something wrong with the first buffer being dequeued in
create_surface(). Would you be able to check where exactly
get_back_bo() fails with your setup?

>
> So I reverted [1] and now I can boot and I also see the black wallpaper like
> Tapani.
> dumpsys SurfaceFlinger output shows a buffer allocated,
> but for some reason both HWC and GLES composition (used in nougat-x86) show
> black wallpaper.
>
> 
> h/w composer state:
>   h/w composer not present and enabled
> Allocated buffers:
> ...
> 0x7b60f301e380: 29440.00 KiB | 2880 (2944) x 2560 |5 | 0x0900 |
> com.android.systemui.ImageWallpaper
> ...
> Total allocated (estimate): 54728.50 KB
> 
>
>>
>>
>> Not that huge of an expert on the Android specifics, so just a humble
>> request:
>> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
>> other?) separate from the functionality changes ?
>>
>> -Emil
>
>
> I'd also kindly request to confirm the test environment used to verify
> Tomasz patch v2,
> which in my understanding has been the following, common between ChomiumOS
> and Android-IA:
>
> minigbm based gralloc

On ChromeOS it's still a drm_gralloc-based gralloc, but heavily
rewritten to use DMA-buf FDs and satisfy some other requirements for
production.

> dma FDs for buffers
> kernel based explicit fences with FDs

There is no support for explicit fences in Mesa at the moment, at
least in Android EGL backend. It always waits synchronously on the
acquire fence and returns -1 as release fence FD.

> HWC2 for compositing
> (?) Render nodes - but I don't know if/when they are used

We don't use control nodes from the Android running in ChromeOS at
all. We use the most suitable render node (i.e. i965 on Intel
platforms) for allocating buffers and rendering.

>
> In android-x86 (nougat-x86) situation is the following:
>
> drm_gralloc based gralloc
> buffer handles
> Not 100% sure about sync/fences, but I don't recall about using explicit
> fences with FDs
> GLES for compositing
> we don't use render nodes
>
> Pardon me if this seems a long checklist or if it's not 100% accurate,
> but I would assume that this patch should just work Out-Of-The-Box with
> android-x86 (nougat-x86)
> even if most of CrOS/Android-IA optimizations are not (yet) used there.
>
> Is this assumption correct?

Yeah, I believe so. Although I might have missed something within the
BO name/handle code path, as it can be quite tricky.

>
> In this moment the only way to boot nougat-x86 is to revert [1]
> but besides this and black wallpaper, which both require investigation,
> I've not seen any particular regression.

Thanks for testing!

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Mauro Rossi]

2017-03-30 16:17 GMT+02:00 Emil Velikov :

> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
> > Android buffer queues can be abandoned, which results in failing to
> > dequeue next buffer. Currently this would fail somewhere deep within
> > the DRI stack calling loader's getBuffers*(), without any error
> > reporting to the client app. However Android framework code relies on
> > proper signaling of this event, so we move buffer dequeue to
> > createWindowSurface() and swapBuffers() call, which can generate proper
> > EGL errors. To keep the performance benefits of delayed buffer handling,
> > if any, fence wait and DRI image creation is kept delayed until
> > getBuffers*() is called by the DRI driver.
> >
> Thank you Tomasz.
>
> I'm fairly confident that this should resolve the crash [in
> swap_buffers] that Mauro was seeing.
> Mauro can you give it a test ?
>

After applying last version of Tomasz patch,
I could not boot nougat-x86, the same way as per Tapani get_back_bo()
throwing and EGL_BAD_ALLOC
which is a show stopper for surfaceflinger

So I reverted [1] and now I can boot and I also see the black wallpaper
like Tapani.
dumpsys SurfaceFlinger output shows a buffer allocated,
but for some reason both HWC and GLES composition (used in nougat-x86) show
black wallpaper.


h/w composer state:
  h/w composer not present and enabled
Allocated buffers:
...
0x7b60f301e380: 29440.00 KiB | 2880 (2944) x 2560 |5 | 0x0900 |
com.android.systemui.ImageWallpaper
...
Total allocated (estimate): 54728.50 KB



>
> Not that huge of an expert on the Android specifics, so just a humble
> request:
> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
> other?) separate from the functionality changes ?
>
> -Emil
>

I'd also kindly request to confirm the test environment used to verify
Tomasz patch v2,
which in my understanding has been the following, common between ChomiumOS
and Android-IA:

   - minigbm based gralloc
   - dma FDs for buffers
   - kernel based explicit fences with FDs
   - HWC2 for compositing
   - (?) Render nodes - but I don't know if/when they are used

In android-x86 (nougat-x86) situation is the following:

   - drm_gralloc based gralloc
   - buffer handles
   - Not 100% sure about sync/fences, but I don't recall about using
   explicit fences with FDs
   - GLES for compositing
   - we don't use render nodes

Pardon me if this seems a long checklist or if it's not 100% accurate,
but I would assume that this patch should just work Out-Of-The-Box with
android-x86 (nougat-x86)
even if most of CrOS/Android-IA optimizations are not (yet) used there.

Is this assumption correct?

In this moment the only way to boot nougat-x86 is to revert [1]
but besides this and black wallpaper, which both require investigation,
I've not seen any particular regression.

Thanks for feeedbacks
Mauro

PS: Question for Tapani: if you apply Tomasz patch and revert [1], do you
still see the segfault in Android-IA?

[1]  https://cgit.freedesktop.org/mesa/mesa/commit/?id=
4d4558411db166d2d66f8cec9cb581149dbe1597
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Rob Clark]

On Fri, Mar 31, 2017 at 2:06 AM, Tapani Pälli  wrote:
>
>
> On 03/31/2017 08:24 AM, Rob Clark wrote:
>>
>> On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli 
>> wrote:
>>>
>>>
>>>
>>> On 03/30/2017 05:57 PM, Emil Velikov wrote:


 On 30 March 2017 at 15:30, Tomasz Figa  wrote:
>
>
> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov
> 
> wrote:
>>
>>
>>
>> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>>>
>>>
>>> Android buffer queues can be abandoned, which results in failing to
>>> dequeue next buffer. Currently this would fail somewhere deep within
>>> the DRI stack calling loader's getBuffers*(), without any error
>>> reporting to the client app. However Android framework code relies on
>>> proper signaling of this event, so we move buffer dequeue to
>>> createWindowSurface() and swapBuffers() call, which can generate
>>> proper
>>> EGL errors. To keep the performance benefits of delayed buffer
>>> handling,
>>> if any, fence wait and DRI image creation is kept delayed until
>>> getBuffers*() is called by the DRI driver.
>>>
>> Thank you Tomasz.
>>
>> I'm fairly confident that this should resolve the crash [in
>> swap_buffers] that Mauro was seeing.
>> Mauro can you give it a test ?
>
>
>
> Ah, I actually noticed a problem with existing code, supposedly fixed
> by [1], but I'm afraid it's still wrong.
>
> Current swap_buffers calls get_back_bo(), but doesn't call
> update_buffers(), which is the function that should be called before
> to actually dequeue a buffer from Android's buffer queue. Given that,
> get_back_bo() would simply fail with !dri2_surf->buffer, because no
> buffer was dequeued.
>
 Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
 From a quick look - may be because EGL/Android drops the dpy mutex in
 droid_window_enqueue_buffer().

> My patch removes update_buffers() and changes the buffer management so
> that there is always a buffer dequeued, starting from surface
> creation, unless there was an error somewhere.
>
 Of the top of your head - is there something stopping us from using
 the same method on $other platforms?

> [1]
>
> https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597
>
>>
>>
>> Not that huge of an expert on the Android specifics, so just a humble
>> request:
>> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,


 Oops silly typo - s/seek/split/.

>> other?) separate from the functionality changes ?
>
>
>
> Sure. Thanks for suggestion.
>
 Please give it a day or two for others to comment.
>>>
>>>
>>>
>>> I'm trying to debug why this causes our homescreen (wallpaper) to be
>>> black.
>>> Otherwise I haven't seen any issues with these changes.
>>>
>>
>> wallpaper seems to be a special sorta hell..  I wonder if there is
>> somehow some sort of interaction with what I fixed / worked-around in
>> a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
>>
>> Maybe at least try commenting out the temp-pbuffer thing to get max
>> texture size, and see if that "fixes" things
>
>
> Can you give more details, I still live in la la land and don't know about
> 'temp-pbuffer thing'?


Sorry, this is something in the wallpaper app java code.. I forget
exactly where it lives now, but there was some code matching what I
pasted in the commit msg which was creating and then destroying a
temporary pbuffer..

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 03/31/2017 10:12 AM, Tapani Pälli wrote:



On 03/31/2017 09:06 AM, Tapani Pälli wrote:



On 03/31/2017 08:24 AM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
 wrote:



On 03/30/2017 05:57 PM, Emil Velikov wrote:


On 30 March 2017 at 15:30, Tomasz Figa  wrote:


On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov

wrote:



On 30 March 2017 at 11:55, Tomasz Figa  wrote:


Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep
within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code
relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate
proper
EGL errors. To keep the performance benefits of delayed buffer
handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?



Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.


Right - I was wondering why we don't hit that on EGL/GBM or
EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().


My patch removes update_buffers() and changes the buffer
management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.


Of the top of your head - is there something stopping us from using
the same method on $other platforms?


[1]
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597






Not that huge of an expert on the Android specifics, so just a
humble
request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,


Oops silly typo - s/seek/split/.


other?) separate from the functionality changes ?



Sure. Thanks for suggestion.


Please give it a day or two for others to comment.



I'm trying to debug why this causes our homescreen (wallpaper) to be
black.
Otherwise I haven't seen any issues with these changes.



wallpaper seems to be a special sorta hell..  I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things


Can you give more details, I still live in la la land and don't know
about 'temp-pbuffer thing'?



aa I did not recall the problem, you mean the 'dummy pbuffer' in
SurfaceFlinger .. yes I will check if this is related.



If I take away that dummy pbuffer usage (which is useless anyway), 
couple of errors disappear from the log. They are:


SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)

but otherwise the desktop still stays black, live wallpapers seem to 
work so there is something special about this default wallpaper. Will 
continue digging ..


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 03/31/2017 09:06 AM, Tapani Pälli wrote:



On 03/31/2017 08:24 AM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli
 wrote:



On 03/30/2017 05:57 PM, Emil Velikov wrote:


On 30 March 2017 at 15:30, Tomasz Figa  wrote:


On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov

wrote:



On 30 March 2017 at 11:55, Tomasz Figa  wrote:


Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code
relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate
proper
EGL errors. To keep the performance benefits of delayed buffer
handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?



Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.


Right - I was wondering why we don't hit that on EGL/GBM or
EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().


My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.


Of the top of your head - is there something stopping us from using
the same method on $other platforms?


[1]
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597





Not that huge of an expert on the Android specifics, so just a humble
request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,


Oops silly typo - s/seek/split/.


other?) separate from the functionality changes ?



Sure. Thanks for suggestion.


Please give it a day or two for others to comment.



I'm trying to debug why this causes our homescreen (wallpaper) to be
black.
Otherwise I haven't seen any issues with these changes.



wallpaper seems to be a special sorta hell..  I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things


Can you give more details, I still live in la la land and don't know
about 'temp-pbuffer thing'?



aa I did not recall the problem, you mean the 'dummy pbuffer' in 
SurfaceFlinger .. yes I will check if this is related.


// Tapani

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 03/31/2017 08:24 AM, Rob Clark wrote:

On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli  wrote:



On 03/30/2017 05:57 PM, Emil Velikov wrote:


On 30 March 2017 at 15:30, Tomasz Figa  wrote:


On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov 
wrote:



On 30 March 2017 at 11:55, Tomasz Figa  wrote:


Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate proper
EGL errors. To keep the performance benefits of delayed buffer
handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?



Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.


Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().


My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.


Of the top of your head - is there something stopping us from using
the same method on $other platforms?


[1]
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597




Not that huge of an expert on the Android specifics, so just a humble
request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,


Oops silly typo - s/seek/split/.


other?) separate from the functionality changes ?



Sure. Thanks for suggestion.


Please give it a day or two for others to comment.



I'm trying to debug why this causes our homescreen (wallpaper) to be black.
Otherwise I haven't seen any issues with these changes.



wallpaper seems to be a special sorta hell..  I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things


Can you give more details, I still live in la la land and don't know 
about 'temp-pbuffer thing'?


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Rob Clark]

On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli  wrote:
>
>
> On 03/30/2017 05:57 PM, Emil Velikov wrote:
>>
>> On 30 March 2017 at 15:30, Tomasz Figa  wrote:
>>>
>>> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov 
>>> wrote:


 On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>
> Android buffer queues can be abandoned, which results in failing to
> dequeue next buffer. Currently this would fail somewhere deep within
> the DRI stack calling loader's getBuffers*(), without any error
> reporting to the client app. However Android framework code relies on
> proper signaling of this event, so we move buffer dequeue to
> createWindowSurface() and swapBuffers() call, which can generate proper
> EGL errors. To keep the performance benefits of delayed buffer
> handling,
> if any, fence wait and DRI image creation is kept delayed until
> getBuffers*() is called by the DRI driver.
>
 Thank you Tomasz.

 I'm fairly confident that this should resolve the crash [in
 swap_buffers] that Mauro was seeing.
 Mauro can you give it a test ?
>>>
>>>
>>> Ah, I actually noticed a problem with existing code, supposedly fixed
>>> by [1], but I'm afraid it's still wrong.
>>>
>>> Current swap_buffers calls get_back_bo(), but doesn't call
>>> update_buffers(), which is the function that should be called before
>>> to actually dequeue a buffer from Android's buffer queue. Given that,
>>> get_back_bo() would simply fail with !dri2_surf->buffer, because no
>>> buffer was dequeued.
>>>
>> Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
>> From a quick look - may be because EGL/Android drops the dpy mutex in
>> droid_window_enqueue_buffer().
>>
>>> My patch removes update_buffers() and changes the buffer management so
>>> that there is always a buffer dequeued, starting from surface
>>> creation, unless there was an error somewhere.
>>>
>> Of the top of your head - is there something stopping us from using
>> the same method on $other platforms?
>>
>>> [1]
>>> https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597
>>>


 Not that huge of an expert on the Android specifics, so just a humble
 request:
 Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
>>
>> Oops silly typo - s/seek/split/.
>>
 other?) separate from the functionality changes ?
>>>
>>>
>>> Sure. Thanks for suggestion.
>>>
>> Please give it a day or two for others to comment.
>
>
> I'm trying to debug why this causes our homescreen (wallpaper) to be black.
> Otherwise I haven't seen any issues with these changes.
>

wallpaper seems to be a special sorta hell..  I wonder if there is
somehow some sort of interaction with what I fixed / worked-around in
a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??

Maybe at least try commenting out the temp-pbuffer thing to get max
texture size, and see if that "fixes" things

BR,
-R
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tapani Pälli]

On 03/30/2017 05:57 PM, Emil Velikov wrote:

On 30 March 2017 at 15:30, Tomasz Figa  wrote:

On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov  wrote:


On 30 March 2017 at 11:55, Tomasz Figa  wrote:

Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate proper
EGL errors. To keep the performance benefits of delayed buffer handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.


Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?


Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.


Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().


My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.


Of the top of your head - is there something stopping us from using
the same method on $other platforms?


[1] 
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597




Not that huge of an expert on the Android specifics, so just a humble request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,

Oops silly typo - s/seek/split/.


other?) separate from the functionality changes ?


Sure. Thanks for suggestion.


Please give it a day or two for others to comment.


I'm trying to debug why this causes our homescreen (wallpaper) to be 
black. Otherwise I haven't seen any issues with these changes.


Thanks;

// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Emil Velikov]

On 30 March 2017 at 15:30, Tomasz Figa  wrote:
> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov  
> wrote:
>>
>> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
>> > Android buffer queues can be abandoned, which results in failing to
>> > dequeue next buffer. Currently this would fail somewhere deep within
>> > the DRI stack calling loader's getBuffers*(), without any error
>> > reporting to the client app. However Android framework code relies on
>> > proper signaling of this event, so we move buffer dequeue to
>> > createWindowSurface() and swapBuffers() call, which can generate proper
>> > EGL errors. To keep the performance benefits of delayed buffer handling,
>> > if any, fence wait and DRI image creation is kept delayed until
>> > getBuffers*() is called by the DRI driver.
>> >
>> Thank you Tomasz.
>>
>> I'm fairly confident that this should resolve the crash [in
>> swap_buffers] that Mauro was seeing.
>> Mauro can you give it a test ?
>
> Ah, I actually noticed a problem with existing code, supposedly fixed
> by [1], but I'm afraid it's still wrong.
>
> Current swap_buffers calls get_back_bo(), but doesn't call
> update_buffers(), which is the function that should be called before
> to actually dequeue a buffer from Android's buffer queue. Given that,
> get_back_bo() would simply fail with !dri2_surf->buffer, because no
> buffer was dequeued.
>
Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().

> My patch removes update_buffers() and changes the buffer management so
> that there is always a buffer dequeued, starting from surface
> creation, unless there was an error somewhere.
>
Of the top of your head - is there something stopping us from using
the same method on $other platforms?

> [1] 
> https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597
>
>>
>>
>> Not that huge of an expert on the Android specifics, so just a humble 
>> request:
>> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
Oops silly typo - s/seek/split/.

>> other?) separate from the functionality changes ?
>
> Sure. Thanks for suggestion.
>
Please give it a day or two for others to comment.

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov  wrote:
>
> On 30 March 2017 at 11:55, Tomasz Figa  wrote:
> > Android buffer queues can be abandoned, which results in failing to
> > dequeue next buffer. Currently this would fail somewhere deep within
> > the DRI stack calling loader's getBuffers*(), without any error
> > reporting to the client app. However Android framework code relies on
> > proper signaling of this event, so we move buffer dequeue to
> > createWindowSurface() and swapBuffers() call, which can generate proper
> > EGL errors. To keep the performance benefits of delayed buffer handling,
> > if any, fence wait and DRI image creation is kept delayed until
> > getBuffers*() is called by the DRI driver.
> >
> Thank you Tomasz.
>
> I'm fairly confident that this should resolve the crash [in
> swap_buffers] that Mauro was seeing.
> Mauro can you give it a test ?

Ah, I actually noticed a problem with existing code, supposedly fixed
by [1], but I'm afraid it's still wrong.

Current swap_buffers calls get_back_bo(), but doesn't call
update_buffers(), which is the function that should be called before
to actually dequeue a buffer from Android's buffer queue. Given that,
get_back_bo() would simply fail with !dri2_surf->buffer, because no
buffer was dequeued.

My patch removes update_buffers() and changes the buffer management so
that there is always a buffer dequeued, starting from surface
creation, unless there was an error somewhere.

[1] 
https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597

>
>
> Not that huge of an expert on the Android specifics, so just a humble request:
> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
> other?) separate from the functionality changes ?

Sure. Thanks for suggestion.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Emil Velikov]

On 30 March 2017 at 11:55, Tomasz Figa  wrote:
> Android buffer queues can be abandoned, which results in failing to
> dequeue next buffer. Currently this would fail somewhere deep within
> the DRI stack calling loader's getBuffers*(), without any error
> reporting to the client app. However Android framework code relies on
> proper signaling of this event, so we move buffer dequeue to
> createWindowSurface() and swapBuffers() call, which can generate proper
> EGL errors. To keep the performance benefits of delayed buffer handling,
> if any, fence wait and DRI image creation is kept delayed until
> getBuffers*() is called by the DRI driver.
>
Thank you Tomasz.

I'm fairly confident that this should resolve the crash [in
swap_buffers] that Mauro was seeing.
Mauro can you give it a test ?

Not that huge of an expert on the Android specifics, so just a humble request:
Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
other?) separate from the functionality changes ?

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

Uhh, last minute rebase without compile re-testing, sorry. 

On Thu, Mar 30, 2017 at 7:55 PM, Tomasz Figa  wrote:
[snip]
> diff --git a/src/egl/drivers/dri2/platform_android.c 
> b/src/egl/drivers/dri2/platform_android.c
> index 6db4015cc9..571afa8004 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
[snip]
> @@ -195,15 +224,36 @@ droid_window_dequeue_buffer(struct dri2_egl_surface 
> *dri2_surf)
>  *any value except -1) then the caller is responsible for closing the
>  *file descriptor.
>  */
> -if (fence_fd >= 0) {
> +if (dri2_surf->acquire_fence_fd >= 0) {
> /* From the SYNC_IOC_WAIT documentation in :
>  *
>  *Waits indefinitely if timeout < 0.
>  */
>  int timeout = -1;
> -sync_wait(fence_fd, timeout);
> -close(fence_fd);
> +sync_wait(dri2_surf->acquire_fence_fd, timeout);
> +close(dri2_surf->acquire_fence_fd);
> +dri2_surf->acquire_fence_fd = -1;
> }
> +}
> +
> +static EGLBoolean
> +droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)

Should be:
+droid_window_dequeue_buffer(_EGLDisplay *disp, struct
dri2_egl_surface *dri2_surf)

> +{
> +   int err;
> +
> +   /* To avoid blocking other EGL calls, release the display mutex before
> +* we enter droid_window_enqueue_buffer() and re-acquire the mutex upon
> +* return.
> +*/
> +   mtx_unlock(>Mutex);
> +
> +   err = dri2_surf->window->dequeueBuffer(dri2_surf->window, 
> _surf->buffer,
> +  _surf->acquire_fence_fd);
> +
> +   mtx_lock(>Mutex);
> +
> +   if (err)
> +  return EGL_FALSE;
>
> dri2_surf->buffer->common.incRef(_surf->buffer->common);
>
[snip]
> @@ -336,6 +362,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
> EGLint type,
>_eglError(EGL_BAD_ALLOC, "droid_create_surface");
>return NULL;
> }
> +   dri2_surf->acquire_fence_fd = -1;
>
> if (!_eglInitSurface(_surf->base, disp, type, conf, attrib_list))
>goto cleanup_surface;
> @@ -377,10 +404,18 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay 
> *disp, EGLint type,
> if (window) {
>window->common.incRef(>common);
>dri2_surf->window = window;
> +  if (!droid_window_dequeue_buffer(dri2_surf)) {

Should be:
+  if (!droid_window_dequeue_buffer(disp, dri2_surf)) {

> + _eglError(EGL_BAD_SURFACE, "Could not dequeue buffer from native 
> window");
> + goto cleanup_window;
> +  }
> }
>
> return _surf->base;
>
> +cleanup_window:
> +   window->common.decRef(>common);
> +   (*dri2_dpy->core->destroyDrawable)(dri2_surf->dri_drawable);
> +
>  cleanup_surface:
> free(dri2_surf);
>
[snip]
> @@ -651,6 +664,12 @@ droid_swap_buffers(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *draw)
>
> dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
>
> +   /* try to dequeue the next back buffer */
> +   if (!droid_window_dequeue_buffer(dri2_surf)) {

Should be:
+   if (!droid_window_dequeue_buffer(disp, dri2_surf)) {

Sorry for the noise. Will resend correct patch anyway.

Best regards,
Tomasz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

[Tomasz Figa]

Android buffer queues can be abandoned, which results in failing to
dequeue next buffer. Currently this would fail somewhere deep within
the DRI stack calling loader's getBuffers*(), without any error
reporting to the client app. However Android framework code relies on
proper signaling of this event, so we move buffer dequeue to
createWindowSurface() and swapBuffers() call, which can generate proper
EGL errors. To keep the performance benefits of delayed buffer handling,
if any, fence wait and DRI image creation is kept delayed until
getBuffers*() is called by the DRI driver.

Fixes following CTS run with i965 driver:
 cts run --module CtsCameraTest -t 
android.hardware.camera2.cts.RobustnessTest#testAbandonRepeatingRequestSurface

Signed-off-by: Tomasz Figa 
---
 src/egl/drivers/dri2/egl_dri2.h |   1 +
 src/egl/drivers/dri2/platform_android.c | 167 ++--
 2 files changed, 94 insertions(+), 74 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 2773079c99..dd9e500eb6 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -288,6 +288,7 @@ struct dri2_egl_surface
 #ifdef HAVE_ANDROID_PLATFORM
struct ANativeWindow *window;
struct ANativeWindowBuffer *buffer;
+   int acquire_fence_fd;
__DRIimage *dri_image_back;
__DRIimage *dri_image_front;
 
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index 6db4015cc9..571afa8004 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -169,15 +169,44 @@ get_native_buffer_name(struct ANativeWindowBuffer *buf)
return gralloc_drm_get_gem_handle(buf->handle);
 }
 
-static EGLBoolean
-droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
+static __DRIbuffer *
+droid_alloc_local_buffer(struct dri2_egl_surface *dri2_surf,
+ unsigned int att, unsigned int format)
 {
-   int fence_fd;
+   struct dri2_egl_display *dri2_dpy =
+  dri2_egl_display(dri2_surf->base.Resource.Display);
 
-   if (dri2_surf->window->dequeueBuffer(dri2_surf->window, _surf->buffer,
-_fd))
-  return EGL_FALSE;
+   if (att >= ARRAY_SIZE(dri2_surf->local_buffers))
+  return NULL;
+
+   if (!dri2_surf->local_buffers[att]) {
+  dri2_surf->local_buffers[att] =
+ dri2_dpy->dri2->allocateBuffer(dri2_dpy->dri_screen, att, format,
+   dri2_surf->base.Width, dri2_surf->base.Height);
+   }
+
+   return dri2_surf->local_buffers[att];
+}
+
+static void
+droid_free_local_buffers(struct dri2_egl_surface *dri2_surf)
+{
+   struct dri2_egl_display *dri2_dpy =
+  dri2_egl_display(dri2_surf->base.Resource.Display);
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(dri2_surf->local_buffers); i++) {
+  if (dri2_surf->local_buffers[i]) {
+ dri2_dpy->dri2->releaseBuffer(dri2_dpy->dri_screen,
+   dri2_surf->local_buffers[i]);
+ dri2_surf->local_buffers[i] = NULL;
+  }
+   }
+}
 
+static void
+wait_and_close_acquire_fence(struct dri2_egl_surface *dri2_surf)
+{
/* If access to the buffer is controlled by a sync fence, then block on the
 * fence.
 *
@@ -195,15 +224,36 @@ droid_window_dequeue_buffer(struct dri2_egl_surface 
*dri2_surf)
 *any value except -1) then the caller is responsible for closing the
 *file descriptor.
 */
-if (fence_fd >= 0) {
+if (dri2_surf->acquire_fence_fd >= 0) {
/* From the SYNC_IOC_WAIT documentation in :
 *
 *Waits indefinitely if timeout < 0.
 */
 int timeout = -1;
-sync_wait(fence_fd, timeout);
-close(fence_fd);
+sync_wait(dri2_surf->acquire_fence_fd, timeout);
+close(dri2_surf->acquire_fence_fd);
+dri2_surf->acquire_fence_fd = -1;
}
+}
+
+static EGLBoolean
+droid_window_dequeue_buffer(struct dri2_egl_surface *dri2_surf)
+{
+   int err;
+
+   /* To avoid blocking other EGL calls, release the display mutex before
+* we enter droid_window_enqueue_buffer() and re-acquire the mutex upon
+* return.
+*/
+   mtx_unlock(>Mutex);
+
+   err = dri2_surf->window->dequeueBuffer(dri2_surf->window, 
_surf->buffer,
+  _surf->acquire_fence_fd);
+
+   mtx_lock(>Mutex);
+
+   if (err)
+  return EGL_FALSE;
 
dri2_surf->buffer->common.incRef(_surf->buffer->common);
 
@@ -234,6 +284,14 @@ droid_window_dequeue_buffer(struct dri2_egl_surface 
*dri2_surf)
   dri2_surf->back = _surf->color_buffers[0];
}
 
+   /* free outdated buffers and update the surface size */
+   if (dri2_surf->base.Width != dri2_surf->buffer->width ||
+   dri2_surf->base.Height != dri2_surf->buffer->height) {
+  droid_free_local_buffers(dri2_surf);
+  dri2_surf->base.Width = dri2_surf->buffer->width;
+  dri2_surf->base.Height = dri2_surf->buffer->height;
+   }
+