Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC

From: "Michel Dänzer" <michel-otUistvHUpPR7s880joybQ@public.gmane.org>
To: "Ville Syrjälä"
	<ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	"Kazlauskas,
	Nicholas" <nicholas.kazlauskas-5C7GfCeVMHo@public.gmane.org>
Cc: nicolai.haehnle-5C7GfCeVMHo@public.gmane.org,
	Marek.Olsak-5C7GfCeVMHo@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	manasi.d.navare-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	Alexander.Deucher-5C7GfCeVMHo@public.gmane.org,
	Christian.Koenig-5C7GfCeVMHo@public.gmane.org
Subject: Re: [PATCH v2 2/3] drm: Add variable refresh property to DRM CRTC
Date: Tue, 25 Sep 2018 15:28:28 +0200	[thread overview]
Message-ID: <2158aa72-9156-5592-822a-c815f373fd53@daenzer.net> (raw)
In-Reply-To: <20180924202655.GA9144-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

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.

-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx