Re: [PATCH] drm/i915: Reset forcewake before suspend

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

On Thu, Mar 13, 2014 at 11:05:02AM +0000, Chris Wilson wrote:
> Now that we regularly defer the forcewake dance to a timer func, it is
> likely to fire after we disable the device during suspend. This
> generates an oops as we detect inconsistency in the hardware state. So
> before suspend, we want to complete the outstanding dance and generally
> sanitize the registers before handing back to the BIOS.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

I was just looking a this a bit myself and came to the same conclusion.

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

There is however one slight issue which I missed when the reviewing the
forcewake timer patch. intel_uncore_fini() will cancel the timer before
calling intel_uncore_sanitize(), but if that one will ever do something
that needs forcewake, we're going to run into the same problem again.
Actually it looks like gen6_disable_rps() will result in a forcewake
get on BDW due to GEN6_RC_CONTROL no being shadowed. So it looks like we
should just move the del_timer_sync() to happen after
intel_uncore_sanitize() and this issue would be fixed.

> ---
>  drivers/gpu/drm/i915/i915_drv.c     | 1 +
>  drivers/gpu/drm/i915/intel_uncore.c | 4 ++++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 658fe24961eb..5a0d34c47885 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -467,6 +467,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>  	i915_save_state(dev);
>  
>  	intel_opregion_fini(dev);
> +	intel_uncore_fini(dev);
>  
>  	console_lock();
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 7861d97600e1..361d1eae3cfd 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -805,6 +805,10 @@ void intel_uncore_fini(struct drm_device *dev)
>  	/* Paranoia: make sure we have disabled everything before we exit. */
>  	intel_uncore_sanitize(dev);
>  	intel_uncore_forcewake_reset(dev);
> +
> +	dev_priv->uncore.forcewake_count = 0;
> +	dev_priv->uncore.fw_rendercount = 0;
> +	dev_priv->uncore.fw_mediacount = 0;
>  }
>  
>  static const struct register_whitelist {
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC