<div dir="ltr">Hi Tomasz, <div class="gmail_extra"> <div class="gmail_quote">2017-04-03 10:00 GMT+02:00 Tomasz Figa <<a href="mailto:tfiga@chromium.org" target="_blank">tfiga@chromium.org</a>>: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Mauro, <div><div class="h5"> On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi <<a href="mailto:issor.oruam@gmail.com">issor.oruam@gmail.com</a>> wrote: > > > 2017-03-31 13:05 GMT+02:00 Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>>: >> >> >> >> 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 >>>>> <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 03/30/2017 05:57 PM, Emil Velikov wrote: >>>>>>> >>>>>>> >>>>>>> On 30 March 2017 at 15:30, Tomasz Figa <<a href="mailto:tfiga@chromium.org">tfiga@chromium.org</a>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov >>>>>>>> <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 30 March 2017 at 11:55, Tomasz Figa <<a href="mailto:tfiga@chromium.org">tfiga@chromium.org</a>> 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] >>>>>>>> >>>>>>>> <a href="https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597" rel="noreferrer" target="_blank">https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597</a> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> 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 height correct? > > In dmesg at relative dmesg timestamp around 58 seconds there is no > signal/error, > just the selinux log (set to permissive): > > [ 58.271833] type=1400 audit(1491145841.554:192): avc: denied { ioctl } > for pid=2141 comm="ndroid.systemui" path="/dev/dri/card0" dev="tmpfs" > ino=7199 ioctlcmd=6467 scontext=u:r:platform_app:s0:c512,c768 > tcontext=u:object_r:device:s0 tclass=chr_file permissive=1 > [ 58.271879] type=1400 audit(1491145841.554:193): avc: denied { read write > } for pid=2141 comm="ndroid.systemui" path="/dev/dri/card0" dev="tmpfs" > ino=7199 scontext=u:r:platform_app:s0:c512,c768 > tcontext=u:object_r:device:s0 tclass=chr_file permissive=1 > > I hope these information may help </div></div>I found that the code was missing acquire fence wait in case of the DRI loader (droid_get_buffers_parse_attachments()). Would you be able to add the following chunk and check if the wallpaper is still broken? diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c index 35aba3a7f0..dc29cc24b2 100644 --- a/src/egl/drivers/dri2/platform_android.c +++ b/src/egl/drivers/dri2/platform_android.c @@ -917,6 +917,10 @@ droid_get_buffers_parse_attachments(struct dri2_egl_surface *dri2_surf, if (!dri2_surf->buffer) continue; + /* Android might have given us an acquire fence to wait for. If so, + * we need to wait for it and close the descriptor after that. */ + wait_and_close_acquire_fence(dri2_surf); + buf->attachment = attachments[i]; buf->name = get_native_buffer_name(dri2_surf->buffer); buf->cpp = get_format_bpp(dri2_surf->buffer->format); Best regards, Tomasz </blockquote></div> </div><div class="gmail_extra">With this change applied the wallpaper is still black</div><div class="gmail_extra">Mauro</div></div>