[Mesa-dev] [PATCH] egl: wayland: bufferAge: only request a back buffer if there isn't one

[Lionel Landwerlin]

On 06/12/16 11:36, Eric Engestrom wrote:
> On Saturday, 2016-12-03 22:47:17 +0000, Lionel Landwerlin wrote:
>> Seeing gtk+ application lockup when they query the buffer age of a surface.
>>
>> Since we update the buffer age field only when creating buffers & swaping
>> them on the client side, there shouldn't be any need for requesting a new
>> back buffer if there is already one available.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84016
>> ---
>>   src/egl/drivers/dri2/platform_wayland.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
>> index 395f1e1..84d9ddd 100644
>> --- a/src/egl/drivers/dri2/platform_wayland.c
>> +++ b/src/egl/drivers/dri2/platform_wayland.c
>> @@ -798,7 +798,7 @@ dri2_wl_query_buffer_age(_EGLDriver *drv,
>>   {
>>      struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surface);
>>   
>> -   if (get_back_bo(dri2_surf) < 0) {
>> +   if (dri2_surf->back == NULL && get_back_bo(dri2_surf) < 0) {
> I'm not convinced this is the right fix: this change the behaviour in
> two ways:
> 1. the backing dri_image might not be allocated, which we don't care
>     about here because we only want to read dri2_surf->back->age.

I don't see anything in the specification that requires allocating the 
image when the buffer age is queried.
So this doesn't seem to be an issue right?

> 2. wl_display_dispatch_queue_pending() not being called, ie. we might
>     operate on stale data.

I don't really agree with this argument :

My understanding (correct me if I'm wrong) is that the age value of the 
back buffers is entirely managed by the client.
It is to be updated when a swap buffer request is issued (also in 
destructions and cases that don't really concern us here, like memory 
pressure events etc...).
The only information that the display server gives us is that a buffer 
is not used anymore and can therefore be reused as a back buffer, it 
does not change the buffer age value a any buffer.

Reading the dri3 backend for example, there is no communication between 
the client & server upon buffer age query.


Thanks a lot for looking this patch.

Cheers,

-
Lionel

>
> If I understand (2) correctly, it means after your patch, we might
> return the buffer age of a buffer about to be released, instead of
> processing the queue, letting it get released and getting a new one.
> This might fix your lockup, but I think this results in potentially
> invalid data being returned to the client.
>
> Cheers,
>    Eric
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev