Re: [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Jul 28, 2014 at 11:07:10PM +0530, sagar.a.kamble@intel.com wrote:
> From: Sagar Kamble <sagar.a.kamble@intel.com>
> 
> Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of gunit registers need to be followed in
> PM suspend and resume path similar to runtime suspend and resume.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Sagar Kamble <sagar.a.kamble@intel.com>

Whereever that code currently lives, I want it shared instead of runtime
pm and system s/r diverging even more. Runtime pm is fully platform
agnostic, so this should be possible without add a single IS_VLV check
anywhere in the code.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c  | 84 ++++++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_drv.h |  7 ++++
>  3 files changed, 85 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 6c4b25c..fdfeccf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -495,6 +495,26 @@ static int i915_drm_freeze(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc;
>  	pci_power_t opregion_target_state;
> +	u32 mask;
> +	int err = 0;
> +
> +	/* Following sequence from vlv_runtime_suspend */
> +	if (IS_VALLEYVIEW(dev)) {
> +		/*
> +		 * Bspec defines the following GT well on flags as debug only,
> +		 * so don't treat them as hard failures.
> +		 */
> +		(void)vlv_wait_for_gt_wells(dev_priv, false);
> +
> +		mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
> +		WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL) & mask) != mask);
> +
> +		vlv_check_no_gt_access(dev_priv);
> +
> +		err = vlv_force_gfx_clock(dev_priv, true);
> +		if (err)
> +			goto err1;
> +	}
>  
>  	/* ignore lid events during suspend */
>  	mutex_lock(&dev_priv->modeset_restore_lock);
> @@ -542,6 +562,10 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>  	i915_gem_suspend_gtt_mappings(dev);
>  
> +	/* Save Gunit State */
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_save_gunit_s0ix_state(dev_priv);
> +
>  	i915_save_state(dev);
>  
>  	opregion_target_state = PCI_D3cold;
> @@ -562,7 +586,27 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>  	intel_display_set_init_power(dev_priv, false);
>  
> -	return 0;
> +	/* Clear Allow Wake Bit so that none of the
> +	 * force/demand wake requests
> +	 */
> +	if (IS_VALLEYVIEW(dev)) {
> +		err = vlv_allow_gt_wake(dev_priv, false);
> +		if (err)
> +			goto err2;
> +
> +		/* Release graphics clocks */
> +		vlv_force_gfx_clock(dev_priv, false);
> +	}
> +	return err;
> +err2:
> +	/* For safety always re-enable waking and disable gfx clock forcing */
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_allow_gt_wake(dev_priv, true);
> +err1:
> +	if (IS_VALLEYVIEW(dev))
> +		vlv_force_gfx_clock(dev_priv, false);
> +
> +	return err;
>  }
>  
>  int i915_suspend(struct drm_device *dev, pm_message_t state)
> @@ -610,6 +654,26 @@ void intel_console_resume(struct work_struct *work)
>  static int i915_drm_thaw_early(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int ret = 0;
> +	int err;
> +
> +	/*
> +	 * Following sequence from vlv_runtime_resume. Clock is released
> +	 * in i915_drm_thaw.
> +	 * If any of the steps fail just try to continue, that's the best we
> +	 * can do at this point. Return the first error code (which will also
> +	 * leave RPM permanently disabled).
> +	 */
> +
> +	if (IS_VALLEYVIEW(dev)) {
> +		ret = vlv_force_gfx_clock(dev_priv, true);
> +
> +		vlv_restore_gunit_s0ix_state(dev_priv);
> +
> +		err = vlv_allow_gt_wake(dev_priv, true);
> +		if (!ret)
> +			ret = err;
> +	}
>  
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		hsw_disable_pc8(dev_priv);
> @@ -618,7 +682,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
>  	intel_uncore_sanitize(dev);
>  	intel_power_domains_init_hw(dev_priv);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> @@ -695,6 +759,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>  
>  	intel_opregion_notify_adapter(dev, PCI_D0);
>  
> +	/* Release graphics clocks turned on in thaw_early*/
> +	vlv_force_gfx_clock(dev_priv, false);
> +
>  	return 0;
>  }
>  
> @@ -1028,7 +1095,7 @@ static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
>   * a black-box for the driver. Further investigation is needed to reduce the
>   * saved/restored registers even further, by following the same 3 criteria.
>   */
> -static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
> +void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  {
>  	struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
>  	int i;
> @@ -1098,6 +1165,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  	s->gu_ctl0		= I915_READ(VLV_GU_CTL0);
>  	s->gu_ctl1		= I915_READ(VLV_GU_CTL1);
>  	s->clock_gate_dis2	= I915_READ(VLV_GUNIT_CLOCK_GATE2);
> +	s->dpio_cfg_data	= I915_READ(DPIO_CTL);
>  
>  	/*
>  	 * Not saving any of:
> @@ -1108,7 +1176,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  	 */
>  }
>  
> -static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
> +void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  {
>  	struct vlv_s0ix_state *s = &dev_priv->vlv_s0ix_state;
>  	u32 val;
> @@ -1192,6 +1260,8 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
>  	I915_WRITE(VLV_GU_CTL0,			s->gu_ctl0);
>  	I915_WRITE(VLV_GU_CTL1,			s->gu_ctl1);
>  	I915_WRITE(VLV_GUNIT_CLOCK_GATE2,	s->clock_gate_dis2);
> +	I915_WRITE(DPIO_CTL,			s->dpio_cfg_data);
> +
>  }
>  
>  int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
> @@ -1231,7 +1301,7 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  #undef COND
>  }
>  
> -static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
> +int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
>  {
>  	u32 val;
>  	int err = 0;
> @@ -1252,7 +1322,7 @@ static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
>  #undef COND
>  }
>  
> -static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
> +int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
>  				 bool wait_for_on)
>  {
>  	u32 mask;
> @@ -1282,7 +1352,7 @@ static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
>  #undef COND
>  }
>  
> -static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
> +void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
>  {
>  	if (!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEERR))
>  		return;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index ef38c3b..4167800 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -907,6 +907,7 @@ struct vlv_s0ix_state {
>  	u32 gu_ctl0;
>  	u32 gu_ctl1;
>  	u32 clock_gate_dis2;
> +	u32 dpio_cfg_data;
>  };
>  
>  struct intel_rps_ei {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bdcc4a1..63cbb06 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1074,6 +1074,13 @@ void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
>  void ilk_wm_get_hw_state(struct drm_device *dev);
>  
>  
> +int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
> +				 bool wait_for_on);
> +void vlv_check_no_gt_access(struct drm_i915_private *dev_priv);
> +void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv);
> +void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv);
> +int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow);
> +
>  /* intel_sdvo.c */
>  bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob);
>  
> -- 
> 1.8.5
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch