Hi Tomasz, 2017-04-03 10:00 GMT+02:00 Tomasz Figa <[email protected]>:
> Hi Mauro, > > On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi <[email protected]> wrote: > > > > > > 2017-03-31 13:05 GMT+02:00 Tapani Pälli <[email protected]>: > >> > >> > >> > >> 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 > >>>>> <[email protected]> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 03/30/2017 05:57 PM, Emil Velikov wrote: > >>>>>>> > >>>>>>> > >>>>>>> On 30 March 2017 at 15:30, Tomasz Figa <[email protected]> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov > >>>>>>>> <[email protected]> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> On 30 March 2017 at 11:55, Tomasz Figa <[email protected]> > 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, 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 > > 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 > With this change applied the wallpaper is still black Mauro
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
