On 01/30, Shayenne Moura wrote:
> kms_flip tests are breaking on vkms when simulate vblank because vblank
> event sequence count returns one extra frame after arm vblank event to
> make a page flip.
>
> When vblank interrupt happens, userspace processes the vblank event and
> issues the next page flip command. Kernel calls queue_work to call
> commit_planes and arm the new page flip. The next vblank picks up the
> newly armed vblank event and vblank interrupt happens again.
>
> The arm and vblank event are asynchronous, then, on the next vblank, we
> receive x+2 from `get_vblank_timestamp`, instead x+1, although timestamp
> and vblank seqno matches.
>
> Function `get_vblank_timestamp` is reached by 2 ways:
>
> - from `drm_mode_page_flip_ioctl`: driver is doing one atomic operation
> to synchronize planes in the same output. There is no vblank simulation,
> the `drm_crtc_arm_vblank_event` function adds 1 on vblank count, and the
> variable in_vblank_irq is false
> - from `vkms_vblank_simulate`: since the driver is doing a vblank
> simulation,
> the variable in_vblank_irq is true.
>
> Fix this problem subtracting one vblank period from vblank_time when
> `get_vblank_timestamp` is called from trace `drm_mode_page_flip_ioctl`,
> i.e., is not a real vblank interrupt, and getting the timestamp and vblank
> seqno when it is a real vblank interrupt.
>
> The reason for all this is that get_vblank_timestamp always supplies the
> timestamp for the next vblank event. The hrtimer is the vblank simulator,
> and it needs the correct previous value to present the next vblank. Since
> this is how hw timestamp registers work and what the vblank core expects.
>
> Signed-off-by: Shayenne Moura
> Signed-off-by: Daniel Vetter
>
> ---
> drivers/gpu/drm/vkms/vkms_crtc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index d44bfc392491..23146ff2a25b 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -87,6 +87,9 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev,
> unsigned int pipe,
>
> *vblank_time = output->vblank_hrtimer.node.expires;
>
> + if (!in_vblank_irq)
> + *vblank_time -= output->period_ns;
> +
> return true;
> }
>
> --
> 2.17.1
>
Reviewed-by: Rodrigo Siqueira
--
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo
signature.asc
Description: PGP signature
