On 2018-04-10 08:45 AM, Christian König wrote: > Am 09.04.2018 um 23:45 schrieb Manasi Navare: >> Thanks for initiating the discussion. Find my comments below: >> On Mon, Apr 09, 2018 at 04:00:21PM -0400, Harry Wentland wrote: >>> On 2018-04-09 03:56 PM, Harry Wentland wrote: >>>> >>>> === A DRM render API to support variable refresh rates === >>>> >>>> In order to benefit from adaptive sync and VRR userland needs a way >>>> to let us know whether to vary frame timings or to target a >>>> different frame time. These can be provided as atomic properties on >>>> a CRTC: >>>>  * bool   variable_refresh_compatible >>>>  * int   target_frame_duration_ns (nanosecond frame duration) >>>> >>>> This gives us the following cases: >>>> >>>> variable_refresh_compatible = 0, target_frame_duration_ns = 0 >>>>  * drive monitor at timing's normal refresh rate >>>> >>>> variable_refresh_compatible = 1, target_frame_duration_ns = 0 >>>>  * send new frame to monitor as soon as it's available, if within >>>> min/max of monitor's reported capabilities >>>> >>>> variable_refresh_compatible = 0/1, target_frame_duration_ns = > 0 >>>>  * send new frame to monitor with the specified >>>> target_frame_duration_ns >>>> >>>> When a target_frame_duration_ns or variable_refresh_compatible >>>> cannot be supported the atomic check will reject the commit. >>>> >> What I would like is two sets of properties on a CRTC or preferably on >> a connector: >> >> KMD properties that UMD can query: >> * vrr_capable - This will be an immutable property for exposing >> hardware's capability of supporting VRR. This will be set by the >> kernel after >> reading the EDID mode information and monitor range capabilities. >> * vrr_vrefresh_max, vrr_vrefresh_min - To expose the min and max >> refresh rates supported. >> These properties are optional and will be created and attached to the >> DP/eDP connector when the connector >> is getting intialized. > > Mhm, aren't those properties actually per mode and not per CRTC/connector? > >> Properties that you mentioned above that the UMD can set before kernel >> can enable VRR functionality >> *bool vrr_enable or vrr_compatible >> target_frame_duration_ns > > Yeah, that certainly makes sense. But target_frame_duration_ns is a bad > name/semantics. > > We should use an absolute timestamp where the frame should be presented, > otherwise you could run into a bunch of trouble with IOCTL restarts or > missed blanks. Also, a fixed target frame duration isn't suitable even for video playback, due to drift between the video and audio clocks. Time-based presentation seems to be the right approach for preventing micro-stutter in games as well, Croteam developers have been researching this. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer