[PATCH v4 3/4] drm: Document variable refresh properties

[Michel Dänzer]

On 2018-10-29 8:11 p.m., Kazlauskas, Nicholas wrote:
> On 10/29/18 2:03 PM, Ville Syrjälä wrote:
>> On Mon, Oct 29, 2018 at 05:37:49PM +0100, Michel Dänzer wrote:
>>> On 2018-10-26 7:59 p.m., Ville Syrjälä wrote:
>>>> On Fri, Oct 26, 2018 at 05:34:15PM +0000, Kazlauskas, Nicholas wrote:
>>>>> On 10/26/18 10:53 AM, Ville Syrjälä wrote:
>>>>>>
>>>>>> Speaking of timestamps. What is the expected behaviour of vblank
>>>>>> timestamps when vrr is enabled?
>>>>>
>>>>> When vrr is enabled the duration of the vertical front porch will be
>>>>> extended until flip or timeout occurs. The vblank timestamp will vary
>>>>> based on duration of the vertical front porch. The min/max duration for
>>>>> the front porch can be specified by the driver via the min/max range.
>>>>>
>>>>> No changes to vblank timestamping handling should be necessary to
>>>>> accommodate variable refresh rate.
>>>>
>>>> The problem is that the timestamp is supposed to correspond to the first
>>>> active pixel. And since we don't know how long the front porch will be
>>>> we can't realistically report the true value. So I guess just assuming
>>>> min front porch length is as good as anything else?
>>>
>>> That (and documenting that the timestamp corresponds to the earliest
>>> possible first active pixel, not necessarily the actual one, with VRR)
>>> might be good enough for the actual vblank event timestamps.
>>>
>>>
>>> However, I'm not so sure about the timestamps of page flip completion
>>> events. Those could be very misleading if the flip completes towards the
>>> timeout, which could result in bad behaviour of applications which use
>>> them for animation timing.
>>>
>>> Maybe the timestamp could be updated appropriately (yes, I'm hand-waving
>>> :) in drm_crtc_send_vblank_event?
>>
>> [...]
> 
> I'm not sure any of this is necessary.
> 
> The vblank timestamp is determined (at least if you're using the 
> helpers) by using the scanout position.
> 
> The crtc_vtotal won't change when variable refresh rate is enabled. What 
> does change is the behavior of the scanout position.
> 
> The vpos will continue going will beyond the end of crtc_vtotal up until 
> whatever the vtotal would be for the minimum refresh rate. There's no 
> clamping performed for these calculations so it ends up working as expected.

I'm afraid not.

amdgpu_display_get_crtc_scanoutpos returns how many lines after "the end
of vblank" (which is calculated based on crtc_vtotal) the current
scanout position is. drm_calc_vbltimestamp_from_scanoutpos converts that
to a time delta from "the end of vblank", and subtracts that from the
current time (when the scanout position was sampled). The result is the
timestamp corresponding to "the end of vblank" based on crtc_vtotal.


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