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

[Tomasz Figa]

Hi everyone,

On Fri, Dec 14, 2018 at 5:17 PM Tomasz Figa <tfiga at chromium.org> 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.
>
> One could make the driver always keep the reference to the last fence,
> but:
>
>  - the driver seems to explicitly destroy the fence whenever a rendering
>    pass completes and changing that would require a significant functional
>    change to the code. (Specifically, in lp_scene_end_rasterization().)
>
>  - it 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.
>
> Therefore, the simple approach of always creating a fence is taken,
> similarly to other drivers, such as radeonsi.
>
> Tested with piglit llvmpipe suite with no regressions and following
> tests fixed:
>
> egl_khr_fence_sync
>  conformance
>   eglclientwaitsynckhr_flag_sync_flush
>   eglclientwaitsynckhr_nonzero_timeout
>   eglclientwaitsynckhr_zero_timeout
>   eglcreatesynckhr_default_attributes
>   eglgetsyncattribkhr_invalid_attrib
>   eglgetsyncattribkhr_sync_status
>
> v2:
>  - remove the useless lp_fence_reference() dance (Nicolai),
>  - explain why creating the dummy fence is the right approach.
>
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
>  src/gallium/drivers/llvmpipe/lp_setup.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index b087369473..e72e119c8a 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -361,6 +361,8 @@ lp_setup_flush( struct lp_setup_context *setup,
>
>     if (fence) {
>        lp_fence_reference((struct lp_fence **)fence, setup->last_fence);
> +      if (!*fence)
> +         *fence = (struct pipe_fence_handle *)lp_fence_create(0);
>     }
>  }
>
> --
> 2.20.0.405.gbc1bbc6f85-goog
>

Gentle ping. :)

Best regards,
Tomasz