[Mesa-dev] [PATCH v5 4/5] loader/dri3: Enable adaptive_sync via _VARIABLE_REFRESH property

[Kazlauskas, Nicholas]

On 10/15/2018 11:18 AM, Michel Dänzer wrote:
> On 2018-10-12 6:48 p.m., Nicholas Kazlauskas wrote:
>> The DDX driver can be notified of adaptive sync suitability by
>> flagging the application's window with the _VARIABLE_REFRESH property.
>>
>> This property is set on the first swap the application performs
>> when adaptive_sync is set to true in the drirc.
>>
>> It's performed here instead of when the loader is initialized for
>> two reasons:
>>
>> (1) The window's drawable can be missing during loader init.
>>      This can be observed during the Unigine Superposition benchmark.
>>
>> (2) Adaptive sync will only be enabled closer to when the application
>>      actually begins rendering.
>>
>> If adaptive_sync is false then the _VARIABLE_REFRESH property
>> is deleted on loader init.
>>
>> The property is only managed on the glx DRI3 backend for now. This
>> should cover most common applications and games on modern hardware.
>>
>> Vulkan support can be implemented in a similar manner but would likely
>> require splitting the function out into a common helper function.
>>
>> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>>
>> [...]
>>   
>> @@ -331,16 +358,28 @@ loader_dri3_drawable_init(xcb_connection_t *conn,
>>      draw->have_back = 0;
>>      draw->have_fake_front = 0;
>>      draw->first_init = true;
>> +   draw->adaptive_sync = false;
>> +   draw->adaptive_sync_active = false;
>>   
>>      draw->cur_blit_source = -1;
>>      draw->back_format = __DRI_IMAGE_FORMAT_NONE;
>>      mtx_init(&draw->mtx, mtx_plain);
>>      cnd_init(&draw->event_cnd);
>>   
>> -   if (draw->ext->config)
>> +   if (draw->ext->config) {
>>         draw->ext->config->configQueryi(draw->dri_screen,
>>                                         "vblank_mode", &vblank_mode);
>>   
>> +      draw->ext->config->configQueryb(draw->dri_screen,
>> +                                      "adaptive_sync",
>> +                                      &adaptive_sync);
>> +
>> +      draw->adaptive_sync = adaptive_sync;
>> +
>> +      if (!adaptive_sync)
>> +         set_adaptive_sync_property(conn, draw->drawable, false);
>> +   }
> 
> The set_adaptive_sync_property call should be after the if
> (draw->ext->config) block here, so that the property is deleted even if
> draw->ext->config is NULL.
> 
> 
Makes sense to me, I don't mind fixing this.

I have to wonder when this is actually ever NULL, though. There's 
support for driver default config values so I don't think it's absence 
of a config file. Maybe just a memory allocation failure, then?

Nicholas Kazlauskas