Re: [PATCH] drm/vblank: Fix vbl time for IRQ fired at the end of display active region

[Liu Ying <victor.liu@nxp.com>]

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