[Intel-gfx] [PATCH 2/2] drm/i915: Tighten timestamp around vblank sampling

[Chris Wilson]

Quoting Chris Wilson (2020-06-11 13:30:38)
> Tighten the timestamp queries before/after the register read so that we
> have less uncertainity for when the read actually took place. This is
> more apt for the older generations where it is not a simple single
> register read. Whether we are able to discern an improvement in our
> sampling accuracy remains to be seen.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 57 ++++++++++++++++++++++++---------
>  1 file changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 8e823ba25f5f..9c44df8ecce7 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -713,7 +713,9 @@ u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
>   * This function will use Framestamp and current
>   * timestamp registers to calculate the scanline.
>   */
> -static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
> +static u32
> +__intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc,
> +                                        ktime_t *stime, ktime_t *etime)
>  {
>         struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>         struct drm_vblank_crtc *vblank =
> @@ -737,6 +739,9 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
>                  * pipe frame time stamp. The time stamp value
>                  * is sampled at every start of vertical blank.
>                  */
> +               if (stime)
> +                       *stime = ktime_get();
> +
>                 scan_prev_time = intel_de_read_fw(dev_priv,
>                                                   PIPE_FRMTMSTMP(crtc->pipe));
>  
> @@ -746,6 +751,9 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
>                  */
>                 scan_curr_time = intel_de_read_fw(dev_priv, IVB_TIMESTAMP_CTR);
>  
> +               if (etime)
> +                       *etime = ktime_get();

I guess with PREEMPT_RT and sleeping spinlocks, these timestamps +
intel_de_read_fw deserve to be within preempt_disable().
-Chris