Re: [PATCH 1/4] drm/i915: Use intel_de_read64_2x32_volatile_fw() for IVB_TIMESTAMP_CTR+PIPE_FRMTMSTMP query

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 22 Oct 2026, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Switch the combined IVB_TIMESTAMP_CTR+PIPE_FRMTMSTMP read to
> use intel_de_read64_2x32_volatile_fw(). It basically does the
> same thing that we were doing by hand, and both are already
> using the intel_de_read_fw().
>
> This has a few functional changes:
> - intel_de_read64_2x32_volatile_fw() has a retry limit, the
>   old code did not

This change is probably a good thing?

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> - when retrying intel_de_read64_2x32_volatile_fw() reads each
>   register once, the old code always read PIPE_FRMTMSTMP twice
>   and IVB_TIMESTAMP_CTR once
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vblank.c | 34 +++++++--------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 99cb648cab73..1d4753605675 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -142,34 +142,22 @@ static u32 intel_crtc_scanlines_since_frame_timestamp(struct intel_crtc *crtc)
>  	const struct drm_display_mode *mode = &vblank->hwmode;
>  	u32 htotal = mode->crtc_htotal;
>  	u32 clock = mode->crtc_clock;
> -	u32 scan_prev_time, scan_curr_time, scan_post_time;
> +	u32 curr_time, frame_time;
> +	u64 result;
>  
>  	/*
> -	 * To avoid the race condition where we might cross into the
> -	 * next vblank just between the PIPE_FRMTMSTMP and TIMESTAMP_CTR
> -	 * reads. We make sure we read PIPE_FRMTMSTMP and TIMESTAMP_CTR
> -	 * during the same frame.
> +	 * IVB_TIMESTAMP_CTR is the current timestamp, PIPE_FRMTMSTMP
> +	 * is the sampled timestamp at the start of the previous
> +	 * delayed vblank. intel_de_read64_2x32_volatile_fw() gives
> +	 * a race free snapshot of both registers.
>  	 */
> -	do {
> -		/*
> -		 * This field provides read back of the display
> -		 * pipe frame time stamp. The time stamp value
> -		 * is sampled at every start of vertical blank.
> -		 */
> -		scan_prev_time = intel_de_read_fw(display,
> +	result = intel_de_read64_2x32_volatile_fw(display,
> +						  IVB_TIMESTAMP_CTR,
>  						  PIPE_FRMTMSTMP(crtc->pipe));
> +	curr_time = lower_32_bits(result);
> +	frame_time = upper_32_bits(result);
>  
> -		/*
> -		 * The TIMESTAMP_CTR register has the current
> -		 * time stamp value.
> -		 */
> -		scan_curr_time = intel_de_read_fw(display, IVB_TIMESTAMP_CTR);
> -
> -		scan_post_time = intel_de_read_fw(display,
> -						  PIPE_FRMTMSTMP(crtc->pipe));
> -	} while (scan_post_time != scan_prev_time);
> -
> -	return div_u64(mul_u32_u32(scan_curr_time - scan_prev_time,
> +	return div_u64(mul_u32_u32(curr_time - frame_time,
>  				   clock), 1000 * htotal);
>  }

-- 
Jani Nikula, Intel