[PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Kazlauskas, Nicholas]

On 09/12/2018 12:22 PM, Michel Dänzer wrote:
> On 2018-09-12 2:48 p.m., Kazlauskas, Nicholas wrote:
>> On 09/12/2018 04:13 AM, Michel Dänzer wrote:
>>> On 2018-09-11 6:18 p.m., Nicholas Kazlauskas wrote:
>>>> These patches are part of a proposed new interface for supporting
>>>> variable refresh rate via DRM properties.
>>>>
>>>> https://patchwork.freedesktop.org/series/49486/
>>>>
>>>> When notified of a window that is FreeSync capable via X these
>>>> patches help track when the window is fullscreen to manage the
>>>> variable_refresh property on the CRTC.
>>>
>>> I'm afraid the Xorg driver support will have to be more or less redone
>>> from scratch for upstreaming:
>>>
>>> Whether or not a client wants variable refresh rate enabled can be
>>> tracked via the window property mechanism supported by the core X11
>>> protocol, no need for a protocol extension.
>>>
>>> That should also allow simpler tracking of when variable refresh rate
>>> can actually be enabled: It can be enabled while a window is flipping,
>>> and its corresponding property allows it. This should be straightforward
>>> with the Present extension, because that also explicitly marks the end
>>> of a window flipping (via an "unflip"). DRI2 is trickier; it's probably
>>> okay not to support variable refresh rate with that, at least initially.
>>>
>>>
>>> I can look into this after the upcoming Xorg driver 18.1 releases. Or I
>>> can give guidance if one of you wants to look into it.
>>
>> I can a look into this. I agree that the extension method is less than
>> ideal - in being vendor specific mostly. It does have the nice property
>> that it remains independent of the application's render backend, though.
> 
> Not sure what you mean by that. Surely a window property is just as
> independent. :)

I was mostly referring to being independent from the backend specific 
code in the DDX driver - DRI2/DRI3/present etc. This change turns the 
notification from being a callback into polling.

> 
> 
>> I imagine you're suggesting specifying a window property hint like
>> _NET_WM_BYPASS_COMPOSITOR - maybe a define new one like
>> _NET_WM_VARIABLE_REFRESH (even though the two are closely related in
>> terms of fullscreen behavior). Then in each backend the property could
>> probably be checked before the present as appropriate.
> 
> Right. (Not sure the _NET_WM prefix is appropriate, as it's nothing to
> do with the window manager, but that's a detail)

I guess an application could operate independently from the window 
manager with variable refresh enabled but it's not certainly a common 
use case. I suppose this would probably need additional documentation 
patches for the extended window manager standards.

_NET_VARIABLE_REFRESH is probably fine as a name.

> 
> 
>> This patch series already has the problem you're describing about DRI2
>> where there's not a nice explicit end notification for flipping - it's
>> only disabled when the window no longer covers the CRTC or the window is
>> destroyed (which is the case for most applications but likely not all).
> 
> Right. I think it's better to just not bother with DRI2, at least until
> there's a specific scenario where variable refresh is really needed with
> DRI2.
> 
> 

Nicholas Kazlauskas