Re: [PATCH 1/2] drm/vkms: Bugfix extra vblank frame

[Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]

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 <shayenneluzmoura@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 
> ---
>  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 <rodrigosiqueiramelo@gmail.com>

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]