On Mon, Apr 3, 2017 at 2:42 PM, Tapani Pälli <tapani.pa...@intel.com> 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 <tf...@chromium.org> wrote: >>> >>> Hi Mauro, >>> >>> On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi <issor.or...@gmail.com> >>> wrote: >>>> >>>> >>>> >>>> 2017-03-30 16:17 GMT+02:00 Emil Velikov <emil.l.veli...@gmail.com>: >>>>> >>>>> >>>>> On 30 March 2017 at 11:55, Tomasz Figa <tf...@chromium.org> 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
