On 2018-09-25 4:04 p.m., Ville Syrjälä wrote:
> On Tue, Sep 25, 2018 at 03:28:28PM +0200, Michel Dänzer wrote:
>> On 2018-09-24 10:26 p.m., Ville Syrjälä wrote:
>>> On Mon, Sep 24, 2018 at 03:06:02PM -0400, Kazlauskas, Nicholas wrote:
>>>> On 09/24/2018 02:38 PM, Ville Syrjälä wrote:
>>>>>
>>>>> Actually I don't understand why this per-crtc thing is being added at
>>>>> all. You already have the prop on the connector. Why do we want to
>>>>> make life more difficult by requiring the thing to be set on both the
>>>>> crtc and connector?
>>>>
>>>> It doesn't make much sense without both.
>>>>
>>>> The user can globally enable or disable the variable_refresh_enabled on
>>>> the connector. This is the user's preference.
>>>>
>>>> What they don't control is the variable_refresh - the content hint that
>>>> userspace specifies when the CRTC contents are suitable for enabling
>>>> variable refresh features (like adaptive sync). This is userspace's
>>>> preference.
>>>
>>> By userspace I guess you mean the compositor/display server.
>>
>> Actually rather the application, see the corresponding Mesa and
>> xf86-video-amdgpu patches.
>>
>>
>>> I don't really see why the kernel has to be involved like this in a
>>> userspace policy matter. If the compositor doesn't think vrr is a good
>>> idea then it could simply choose to disable the prop on the connector
>>> even when requested by its clients.
>>
>> Connector properties are exposed directly to X11 clients as RandR output
>> properties. With only the connector property, the user running e.g.
>>
>> xrandr --output <name> --set variable_refresh_enabled 1
>>
>> would result in variable refresh being enabled regardless of whether a
>> variable refresh compatible client is currently using page flipping,
>> which can result in flickering or getting stuck at the minimum refresh rate.
>
> in ddx:
>
> configure_vrr()
> {
> kms_vrr = client_vrr && is_vrr_a_good_idea();
> kms_setprop(kms_vrr);
> }
>
> set_prop()
> {
> if (is_vrr_prop)
> configure_vrr();
> else
> kms_setprop();
> }
>
> other_stuff()
> {
> /* some stuff */
> ...
>
> if (vrr_related_stuff_changed)
> configure_vrr();
> }
>
> or something along those lines?
>
Sure, that's not the problem, which is that if the Xorg driver doesn't
actively hide the property from clients (e.g. because it's a currently
released version), we get the issue I described above.
Also, if the Xorg driver were to hide the connector property from
clients, there's no way for the user to completely disable variable
refresh for a display (unless the Xorg driver exposes another fake
property for that, which seems a bit silly).
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
