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:

quoted

On 4/28/2021 3:55 AM, Ville Syrjala wrote:

quoted

From: Ville Syrjälä <redacted>

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?

quoted

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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev