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

[Kazlauskas, Nicholas]

On 10/30/18 5:29 AM, Michel Dänzer wrote:
> On 2018-10-29 7:03 p.m., 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?
>>
>> Hmm. Updated how? Whether it's a page flip event or vblank even we won't
>> know when the first active pixel will come. Although I suppose if
>> there is some kind of vrr slew rate limit we could at least account
>> for that to report a more correct "this is the earliest you migth be
>> able to see your frame" timestamp.
>>
>> Oh, or are you actually saying that shceduling a new flip before the
>> timeout is actually going to latch that flip immediately? I figured
>> that the flip would get latched on the next start of vblank regardless,
>> and the act of scheduling a flip will just kick the hardware to start
>> scanning the previously latched frame earlier.
>>
>> scanout A | ... vblank | scanout A | ... vblank | scanout B | ... vblank
>>                    ^                ^        ^               ^
>>                    |                |        flip C          latch C
>>                    flip B           latch B
> 
> This would kind of defeat the point of VRR, wouldn't it? If a flip was
> scheduled after the start of vblank, the vblank would always time out,
> resulting in the minimum refresh rate.
> 
> 
>> scanout A | ... vblank | scanout B | ... vblank | scanout C | ... vblank
>>                    ^ ^                      ^ ^
>>                    | latch B                | latch C
>>                    flip B                   flip C
> 
> So this is what happens.
> 
> Regardless, when the flip is latched, AMD hardware generates a "pflip"
> interrupt, and its handler calls drm_crtc_send_vblank_event (and in the
> case of DC drm_crtc_accurate_vblank_count before that). So the time when
> drm_crtc_send_vblank_event is called might be a good approximation of
> when scanout of the next frame starts.
> 
> Another possibility might be to wait for the hardware vline counter to
> wrap around to 0 before calling drm_crtc_accurate_vblank_count, then the
> calculations should be based on 0 instead of crtc_vtotal.
> 
> 


I understand the issue you're describing now. The timestamp is supposed 
to signify the end of the current vblank. The call to get scanout 
position is supposed to return the number of lines until scanout (a 
negative value) or the number of lines since the next scanout began (a 
positive value).

The amdgpu driver calculates the number of lines based on a hardware 
register status position which returns an increasing value from 0 that 
indicates the current vpos/hpos for the display.

For any vpos below vbl_start we know the value is correct since the next 
vblank hasn't begun yet. But for anythign about vbl_start it's probably 
wrong since it applies a corrective offset based on the fixed value of 
crtc_vtotal. It's even worse when the value is above crtc_vtotal since 
it'll be calculating the number of lines since the last scanout since 
it'll be a positive value.

So the issue becomes determining when the vfront porch will end.

When the flip address gets written the vfront porch will end at the 
start of the next line leaving only the back porch plus part of the 
line. But we don't know when the flip will occur, if at all. It hasn't 
occurred yet in this case.

Waiting for the wrap around to 0 might be the best choice here since 
there's no guarantee the flip will occur.

Nicholas Kazlauskas