diff mbox series

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

Message ID 171e6e1c239cbca0c3df7183ed8acdfeeace9cf4.1548856186.git.shayenneluzmoura@gmail.com (mailing list archive)
State New, archived
Headers show
Series drm/vkms: Bugfix for igt-tests | expand

Commit Message

Shayenne Moura Jan. 30, 2019, 4:06 p.m. UTC
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(+)

Comments

Rodrigo Siqueira Feb. 3, 2019, 8:57 p.m. UTC | #1
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>
diff mbox series

Patch

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;
 }