[igt-dev] [PATCH i-g-t] tests/kms_async_flips: Fix flip timestamp checks

[Karthik B S]

On 4/29/2021 9:02 PM, Ville Syrjälä wrote:
> On Thu, Apr 29, 2021 at 11:07:53AM +0530, Karthik B S wrote:
>> On 4/28/2021 3:55 AM, Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>
>>> Check the flip timestamp from the event (not the time we got the
>>> event) against the vblank timestamps. Also if things are going
>>> well we should expect the flip timestamp to in fact match the
>>> first vblank timestamp, so allow them to be equal.
>> Hi,
>>
>> During the initial implementation, the timestamp returned by the kernel
>> was vblank timestamp and we were not sending back the flip timestamp.
>>
>> So we were calculating the time in the event rather than using the event
>> timestamp.
> That approach doesn't make too much sense since the two vblank
> timestamps we're comparing against could both be in the future.
> That happens if the flip is completed already during the first vblank.
> But I haven't actually checked when the hw completes the flip when
> scheduled during a vblank.

I think we are seeing this case on CI sporadically, where both the 
vblanks are in the future. How should we handle this in IGT?

>
>> Is this now updated to return the flip timestamp? If not, we would
>> always have flip time stamp same as the previous vblank time stamp?
> Looks like it just returns the last stored timestamp as before.
> Which doesn't really make sense. At the very least we should
> make sure the timestam and seq number are updated to reflect
> the current frame. So if we do something like
>
> 1. wait for vblank
> 2. wait for several frames
> 3. async flip
>
> then the event would now carry the timestamp from that ancient
> vblank wait. Well, at least that's how it looks from the code,
> still need to double check it. And the test should probably be
> extended to catch stuff like that.

In the initial kernel patches enabling async flip, we tried to update 
this using the frame counter register. But we received review comments 
telling that the seq number and timestamp should correspond to the 
vblank and should not update with flip. Any other way we can achieve this?

Agreed that the test should be extended to catch this once we have 
kernel support. Also, we can remove the monotonic timestamps which we 
are currently using in the test, which would fix the issue mentioned 
above also.

Thanks,

Karthik.B.S

>