diff mbox

[09/10] drm/vblank: Restoring vblank counts after device PM events.

Message ID 20180203051302.9974-9-dhinakaran.pandiyan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dhinakaran Pandiyan Feb. 3, 2018, 5:13 a.m. UTC
From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>

The HW frame counter can get reset if device enters a low power state after
vblank interrupts were disabled. This messes up any following vblank count
update as a negative diff (huge unsigned diff) is calculated from the HW
frame counter change. We cannot ignore negative diffs altogther as there
could be legitimate wrap arounds. So, allow drivers to update vblank->count
with missed vblanks for the time interrupts were disabled. This is similar
to _crtc_vblank_on() except that vblanks interrupts are not enabled at the
end as this function is expected to be called from the driver
_enable_vblank() vfunc.

v2: drm_crtc_vblank_restore should take crtc as arg. (Chris)
    Add docs and sprinkle some asserts.

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_vblank.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_vblank.h     |  2 ++
 2 files changed, 61 insertions(+)

Comments

Daniel Vetter Feb. 19, 2018, 10:39 p.m. UTC | #1
On Fri, Feb 02, 2018 at 09:13:01PM -0800, Dhinakaran Pandiyan wrote:
> From: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> 
> The HW frame counter can get reset if device enters a low power state after
> vblank interrupts were disabled. This messes up any following vblank count
> update as a negative diff (huge unsigned diff) is calculated from the HW
> frame counter change. We cannot ignore negative diffs altogther as there
> could be legitimate wrap arounds. So, allow drivers to update vblank->count
> with missed vblanks for the time interrupts were disabled. This is similar
> to _crtc_vblank_on() except that vblanks interrupts are not enabled at the
> end as this function is expected to be called from the driver
> _enable_vblank() vfunc.
> 
> v2: drm_crtc_vblank_restore should take crtc as arg. (Chris)
>     Add docs and sprinkle some asserts.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Dänzer <michel@daenzer.net>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

This now creates new warnings when building the docs. When changing any
kind of kernel doc please always run

$ make htmldocs

and make sure the parts you're touching are warning free, the results look
good (and consistent with existing docs) and that all the hyperlinks work
and point to the right places.

Please spin a follow up patch to fix this (since this one here is merged
already).

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_vblank.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_vblank.h     |  2 ++
>  2 files changed, 61 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 913954765d9e..c781cb426bf1 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -1237,6 +1237,65 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc)
>  }
>  EXPORT_SYMBOL(drm_crtc_vblank_on);
>  
> +/**
> + * drm_vblank_restore - estimated vblanks using timestamps and update it.
> + *
> + * Power manamement features can cause frame counter resets between vblank
> + * disable and enable. Drivers can then use this function in their
> + * &drm_crtc_funcs.enable_vblank implementation to estimate the vblanks since
> + * the last &drm_crtc_funcs.disable_vblank.
> + *
> + * This function is the legacy version of drm_crtc_vblank_restore().
> + */
> +void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
> +{
> +	ktime_t t_vblank;
> +	struct drm_vblank_crtc *vblank;
> +	int framedur_ns;
> +	u64 diff_ns;
> +	u32 cur_vblank, diff = 1;
> +	int count = DRM_TIMESTAMP_MAXRETRIES;
> +
> +	if (WARN_ON(pipe >= dev->num_crtcs))
> +		return;
> +
> +	assert_spin_locked(&dev->vbl_lock);
> +	assert_spin_locked(&dev->vblank_time_lock);
> +
> +	vblank = &dev->vblank[pipe];
> +	WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns,
> +		  "Cannot compute missed vblanks without frame duration\n");
> +	framedur_ns = vblank->framedur_ns;
> +
> +	do {
> +		cur_vblank = __get_vblank_counter(dev, pipe);
> +		drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false);
> +	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
> +
> +	diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time));
> +	if (framedur_ns)
> +		diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);
> +
> +
> +	DRM_DEBUG_VBL("missed %d vblanks in %lld ns, frame duration=%d ns, hw_diff=%d\n",
> +		      diff, diff_ns, framedur_ns, cur_vblank - vblank->last);
> +	store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
> +}
> +EXPORT_SYMBOL(drm_vblank_restore);
> +
> +/**
> + * drm_crtc_vblank_restore - estimate vblanks using timestamps and update it.
> + * Power manamement features can cause frame counter resets between vblank
> + * disable and enable. Drivers can then use this function in their
> + * &drm_crtc_funcs.enable_vblank implementation to estimate the vblanks since
> + * the last &drm_crtc_funcs.disable_vblank.
> + */
> +void drm_crtc_vblank_restore(struct drm_crtc *crtc)
> +{
> +	drm_vblank_restore(crtc->dev, drm_crtc_index(crtc));
> +}
> +EXPORT_SYMBOL(drm_crtc_vblank_restore);
> +
>  static void drm_legacy_vblank_pre_modeset(struct drm_device *dev,
>  					  unsigned int pipe)
>  {
> diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
> index a4c3b0a0a197..16d46e2a6854 100644
> --- a/include/drm/drm_vblank.h
> +++ b/include/drm/drm_vblank.h
> @@ -180,6 +180,8 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc);
>  void drm_crtc_vblank_reset(struct drm_crtc *crtc);
>  void drm_crtc_vblank_on(struct drm_crtc *crtc);
>  u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
> +void drm_vblank_restore(struct drm_device *dev, unsigned int pipe);
> +void drm_crtc_vblank_restore(struct drm_crtc *crtc);
>  
>  bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  					   unsigned int pipe, int *max_error,
> -- 
> 2.14.1
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 913954765d9e..c781cb426bf1 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -1237,6 +1237,65 @@  void drm_crtc_vblank_on(struct drm_crtc *crtc)
 }
 EXPORT_SYMBOL(drm_crtc_vblank_on);
 
+/**
+ * drm_vblank_restore - estimated vblanks using timestamps and update it.
+ *
+ * Power manamement features can cause frame counter resets between vblank
+ * disable and enable. Drivers can then use this function in their
+ * &drm_crtc_funcs.enable_vblank implementation to estimate the vblanks since
+ * the last &drm_crtc_funcs.disable_vblank.
+ *
+ * This function is the legacy version of drm_crtc_vblank_restore().
+ */
+void drm_vblank_restore(struct drm_device *dev, unsigned int pipe)
+{
+	ktime_t t_vblank;
+	struct drm_vblank_crtc *vblank;
+	int framedur_ns;
+	u64 diff_ns;
+	u32 cur_vblank, diff = 1;
+	int count = DRM_TIMESTAMP_MAXRETRIES;
+
+	if (WARN_ON(pipe >= dev->num_crtcs))
+		return;
+
+	assert_spin_locked(&dev->vbl_lock);
+	assert_spin_locked(&dev->vblank_time_lock);
+
+	vblank = &dev->vblank[pipe];
+	WARN_ONCE((drm_debug & DRM_UT_VBL) && !vblank->framedur_ns,
+		  "Cannot compute missed vblanks without frame duration\n");
+	framedur_ns = vblank->framedur_ns;
+
+	do {
+		cur_vblank = __get_vblank_counter(dev, pipe);
+		drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false);
+	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
+
+	diff_ns = ktime_to_ns(ktime_sub(t_vblank, vblank->time));
+	if (framedur_ns)
+		diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);
+
+
+	DRM_DEBUG_VBL("missed %d vblanks in %lld ns, frame duration=%d ns, hw_diff=%d\n",
+		      diff, diff_ns, framedur_ns, cur_vblank - vblank->last);
+	store_vblank(dev, pipe, diff, t_vblank, cur_vblank);
+}
+EXPORT_SYMBOL(drm_vblank_restore);
+
+/**
+ * drm_crtc_vblank_restore - estimate vblanks using timestamps and update it.
+ * Power manamement features can cause frame counter resets between vblank
+ * disable and enable. Drivers can then use this function in their
+ * &drm_crtc_funcs.enable_vblank implementation to estimate the vblanks since
+ * the last &drm_crtc_funcs.disable_vblank.
+ */
+void drm_crtc_vblank_restore(struct drm_crtc *crtc)
+{
+	drm_vblank_restore(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_restore);
+
 static void drm_legacy_vblank_pre_modeset(struct drm_device *dev,
 					  unsigned int pipe)
 {
diff --git a/include/drm/drm_vblank.h b/include/drm/drm_vblank.h
index a4c3b0a0a197..16d46e2a6854 100644
--- a/include/drm/drm_vblank.h
+++ b/include/drm/drm_vblank.h
@@ -180,6 +180,8 @@  void drm_crtc_vblank_off(struct drm_crtc *crtc);
 void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 void drm_crtc_vblank_on(struct drm_crtc *crtc);
 u64 drm_crtc_accurate_vblank_count(struct drm_crtc *crtc);
+void drm_vblank_restore(struct drm_device *dev, unsigned int pipe);
+void drm_crtc_vblank_restore(struct drm_crtc *crtc);
 
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe, int *max_error,