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

[Roland Scheidegger]

Am 07.01.19 um 09:54 schrieb Tomasz Figa:
> On Sun, Dec 23, 2018 at 12:55 AM Roland Scheidegger <sroland at vmware.com> wrote:
>>
>> Alright, I guess it should work...
>>
>> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>>
> 
> Thanks!
> 
> Would we have anyone who could help to commit it?

Pushed (albeit I forgot the R-b on it, ah well...)

Roland

> 
> (I know that I was supposed to apply for commit rights, but I expect
> my contribution rate to be relatively low, due to a shift to different
> areas, so I don't think I'm a good candidate for a committer anymore.)
> 
> Best regards,
> Tomasz
> 
>>
>> Am 14.12.18 um 09:17 schrieb Tomasz Figa:
>>> 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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fandroid.googlesource.com%2Fplatform%2Fframeworks%2Fbase%2F%2B%2Fmaster%2Flibs%2Fhwui%2Fpipeline%2Fskia%2FSkiaOpenGLPipeline.cpp%23427&data=02%7C01%7Csroland%40vmware.com%7C578e1621b8de4eeca24508d6747dd1a1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636824481094747485&sdata=dEjvlyeiOlGCBnegZS%2FXsNRf8OX21qss9d59D0RGqZk%3D&reserved=0
>>>
>>> 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);
>>>     }
>>>  }
>>>
>>>
>>