Helper drm_crtc_vblank_helper_get_vblank_timestamp_internal() aims to
return the timestamp for end of vblank from the vblank_time pointer.
For vblank interrupt fired at the end of vblank, it's fine to subtract
time delta(as a positive value) according to scanout position from
etime to get the timestamp.  However, for vblank interrupt fired at
the end of display active region, the read-out scanout position could
be at the horizontal blank region of the last display active line,
which causes the calculated timestamp to be one frame eariler.  To fix
this, subtract the time duration of an entire frame from the time delta
for the problematic case so that the final timestamp moves forward for
one frame.

This fixes weston assertion on backward timestamp when comparing
timestamps for consecutive frames.  The issue can be seen with a display
controller which fires off vblank interrupt at the end of display active
region.  The display controller driver is not yet upstreamed though.

Signed-off-by: Liu Ying <victor.liu@nxp.com>
---
 drivers/gpu/drm/drm_vblank.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 46f59883183d..c228d327bf17 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -782,6 +782,15 @@ drm_crtc_vblank_helper_get_vblank_timestamp_internal(
 	delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos),
 			   mode->crtc_clock);
 
+	/*
+	 * For vblank interrupt fired off at the end of display active region,
+	 * subtract time duration of an entire frame if vpos happens to be the
+	 * display active lines(hpos is in the horizontal blank region).
+	 */
+	if (in_vblank_irq && vpos == mode->crtc_vdisplay)
+		delta_ns -= div_u64(1000000LL * mode->crtc_htotal * mode->crtc_vtotal,
+				    mode->crtc_clock);
+
 	/* Subtract time delta from raw timestamp to get final
 	 * vblank_time timestamp for end of vblank.
 	 */
-- 
2.34.1

On 06/18/2025, Liu Ying wrote:
> Helper drm_crtc_vblank_helper_get_vblank_timestamp_internal() aims to
> return the timestamp for end of vblank from the vblank_time pointer.
> For vblank interrupt fired at the end of vblank, it's fine to subtract
> time delta(as a positive value) according to scanout position from
> etime to get the timestamp.  However, for vblank interrupt fired at
> the end of display active region, the read-out scanout position could
> be at the horizontal blank region of the last display active line,
> which causes the calculated timestamp to be one frame eariler.  To fix
> this, subtract the time duration of an entire frame from the time delta
> for the problematic case so that the final timestamp moves forward for
> one frame.
> 
> This fixes weston assertion on backward timestamp when comparing
> timestamps for consecutive frames.  The issue can be seen with a display
> controller which fires off vblank interrupt at the end of display active
> region.  The display controller driver is not yet upstreamed though.

Gentle ping.

There are about a dozen of drivers that set the get_scanout_position callback.
If their backing display controllers fire off vblank interrupt at the end
of display active region, this patch should fix vblank time for them too.
Land this?

> 
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  drivers/gpu/drm/drm_vblank.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 46f59883183d..c228d327bf17 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -782,6 +782,15 @@ drm_crtc_vblank_helper_get_vblank_timestamp_internal(
>  	delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos),
>  			   mode->crtc_clock);
>  
> +	/*
> +	 * For vblank interrupt fired off at the end of display active region,
> +	 * subtract time duration of an entire frame if vpos happens to be the
> +	 * display active lines(hpos is in the horizontal blank region).
> +	 */
> +	if (in_vblank_irq && vpos == mode->crtc_vdisplay)
> +		delta_ns -= div_u64(1000000LL * mode->crtc_htotal * mode->crtc_vtotal,
> +				    mode->crtc_clock);
> +
>  	/* Subtract time delta from raw timestamp to get final
>  	 * vblank_time timestamp for end of vblank.
>  	 */

-- 
Regards,
Liu Ying