[Mesa-dev] [PATCH] llvmpipe: Always return some fence in flush

[Tomasz Figa]

Hi Michel,

On Thu, Aug 16, 2018 at 6:43 PM Michel Dänzer <michel at daenzer.net> wrote:
>
> On 2018-08-16 11:34 AM, Tomasz Figa wrote:
> > If there is no last fence, due to no rendering happening yet, just
> > create a new signaled fence and return it, to match the expectations of
> > the EGL sync fence API.
> >
> > Fixes random "Could not create sync fence 0x3003" assertion failures from
> > Skia on Android, coming from the following code:
> >
> > https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp#427
> >
> > Reproducible especially with thread count >= 4.
> >
> > Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> > ---
> >  src/gallium/drivers/llvmpipe/lp_setup.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> > index b087369473..a6f1b54d69 100644
> > --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> > +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> > @@ -360,7 +360,13 @@ lp_setup_flush( struct lp_setup_context *setup,
> >     set_scene_state( setup, SETUP_FLUSHED, reason );
> >
> >     if (fence) {
> > -      lp_fence_reference((struct lp_fence **)fence, setup->last_fence);
> > +      struct lp_fence *lp_fence = NULL;
> > +
> > +      lp_fence_reference(&lp_fence, setup->last_fence);
> > +      if (!lp_fence)
> > +         lp_fence = lp_fence_create(0);
> > +      lp_fence_reference((struct lp_fence **)fence, lp_fence);
> > +      lp_fence_reference(&lp_fence, NULL);
> >     }
> >  }
> >
> >
>
> Other drivers keep around a reference to the last fence in the context,
> and return that if there's no new work to flush.

Thanks for taking a look.

Right, it sounds like a sane thing to do. LLVMpipe, however, seems to
explicitly destroy the fence whenever a rendering pass completes and I
didn't want to change that without understanding the intentions behind
that. Precisely, it's lp_scene_end_rasterization():
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/drivers/llvmpipe/lp_scene.c#L292

Also, this still wouldn't solve the problem of an EGL sync fence being
created and waited on without any rendering happening at all, which is
also likely to happen with Android code pointed to in the commit
message. Obviously that could be dealt with by creating a signaled
fence in lp_setup_create(), though.

Let me add Keith and Brian for more visibility.

Best regards,
Tomasz