[Mesa-dev] [PATCH] [AMD] dri3: Add adaptive_sync_enable driconf option

[Michel Dänzer]

On 17/10/17 12:29 PM, Nicolai Hähnle wrote:
> On 17.10.2017 12:07, Michel Dänzer wrote:
>> On 17/10/17 11:33 AM, Nicolai Hähnle wrote:
>>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>>
>>> When enabled, this will request FreeSync via the hybrid amdgpu DDX's
>>> AMDGPU X11 protocol extension.
>>>
>>> Due to limitations in the DDX this will only work for applications
>>> that cover the entire X screen (which is important to keep in mind when
>>> you have a multi-monitor setup).
>>
>> This limitation already applies to page flipping in general, it's not
>> specific to FreeSync.
> 
> Maybe it shouldn't apply in general? After all, what if you have one
> monitor with 60Hz and one with 90Hz?

Page flipping can be used in that case. The application's buffer swaps
will be synchronized to one or the other CRTC, by default the one where
the largest part of the window is visible on. On the other CRTC(s), each
flip may take effect slightly earlier or later than on the
synchronization CRTC.


>>> @@ -767,20 +820,25 @@ loader_dri3_swap_buffers_msc(struct
>>> loader_dri3_drawable *draw,
>>>                                bool force_copy)
>>>   {
>>>      struct loader_dri3_buffer *back;
>>>      int64_t ret = 0;
>>>      uint32_t options = XCB_PRESENT_OPTION_NONE;
>>>        draw->vtable->flush_drawable(draw, flush_flags);
>>>        back = dri3_find_back_alloc(draw);
>>>   +   if (draw->adaptive_sync) {
>>> +      if (!loader_dri3_amdgpu_freesync_enable(draw->conn,
>>> draw->drawable))
>>> +         draw->adaptive_sync = false;
>>> +   }
>>
>> The AMDGPUFreesyncCapability request only has to be submitted once per
>> window, not for every buffer swap.
> 
> Unfortunately, that's not true due to the implementation in the DDX.
> 
> For one,  we *definitely* have to re-submit the request at least each
> time the window is moved or resized, because the request doesn't
> register unless the window is already fullscreen.
> 
> For another, some applications create multiple windows, but the DDX can
> only keep track of one.
> 
> There's perhaps an argument to be made that both of these issues would
> better be fixed in the DDX. However, since this is a non-upstream
> interim solution anyway, and the final upstream solution will likely
> also do something in swap_buffers, I think this approach is reasonable.
> 
> Correct me if I got some of the xcb code wrong, but from what I
> understand there should be no stalls associated with it, just a bunch of
> additional bytes sent over the socket on each swap buffers, which
> doesn't seem too bad to me.
> 
> (For what it's worth, the closed GL driver sends this request on
> glXMakeCurrent, which makes even less sense to me :-))

I agree with your analysis on all accounts. Really, this should have
simply used a per-window property (part of the original X11 protocol)
instead of creating an extension...


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer