Re: [PATCH v3] drm/core: Add drm_accurate_vblank_count, v3.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Apr 12, 2016 at 04:32:19PM +0200, Maarten Lankhorst wrote:
> This function is useful for gen2 intel devices which have no frame
> counter, but need a way to determine the current vblank count without
> racing with the vblank interrupt handler.
> 
> intel_pipe_update_start checks if no vblank interrupt will occur
> during vblank evasion, but cannot check whether the vblank handler has
> run to completion. This function uses the timestamps to determine
> when the last vblank has happened, and interpolates from there.
> 
> Changes since v1:
> - Take vblank_time_lock and don't use drm_vblank_count_and_time.
> Changes since v2:
> - Don't return time of last vblank.
> 
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 3c1a6f18e71c..32bfa4bb8f41 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
>  	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
>  }
>  
> +/**
> + * drm_accurate_vblank_count - retrieve the master vblank counter
> + * @crtc: which counter to retrieve
> + * @tv_ret: last time counter was updated
> + *
> + * This function is similar to @drm_crtc_vblank_count but this
> + * function interpolates to handle a race with vblank irq's.
> + */
> +
> +u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	u32 vblank, pipe = drm_crtc_index(crtc);

pipe should be 'unsigned int' for consistency.

> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&dev->vblank_time_lock, flags);
> +
> +	drm_update_vblank_count(dev, pipe, 0);

One thing that came to mind is some callers might end up doing the
update twice if the irq wasn't yet enabled when drm_vblank_get() was
called. Eg. drm_wait_one_vblank() might do that. So it might be a bit
more efficient to add a drm_vblank_get_and_update() instead, or something
like that. But I don't really care too much.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	vblank = dev->vblank[pipe].count;
> +
> +	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
> +
> +	return vblank;
> +}
> +EXPORT_SYMBOL(drm_accurate_vblank_count);
> +
>  /*
>   * Disable vblank irq's on crtc, make sure that last vblank count
>   * of hardware and corresponding consistent software vblank counter
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 31483c2fef51..747c80da70e5 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
>  extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>  extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
>  extern void drm_vblank_cleanup(struct drm_device *dev);
> +extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
>  extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
>  
>  extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx