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

[Roland Scheidegger]

Sorry but I had to revert this, as we've seen lots of assertion failures
(src/gallium/drivers/llvmpipe/lp_fence.c:120: lp_fence_wait: Assertion
`f->issued' failed.). For instance with libgl_xlib state tracker and piglit.
I'm not entirely sure if it's really safe to just remove the assert or
if it needs some more work, and I don't have time right now for a
thorough investigation, but I'll happily take new patches...

Roland


Am 09.01.19 um 02:09 schrieb 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%7Ccb06f4e1c9164a7871cb08d675cf20c7%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636825929820495738&sdata=6hmZk%2BXWaQk%2B5XAKjxFSybOSpCVwzvKemYgZQ1rtpvg%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);
>>>>     }
>>>>  }
>>>>
>>>>
>>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7Ccb06f4e1c9164a7871cb08d675cf20c7%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636825929820495738&sdata=FeFqwQi9AUVDWaUw7lLMtAci6wWjE44vqwwjVwysY3o%3D&reserved=0
>