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

[Sagar Arun Kamble <sagar.a.kamble@intel.com>]

On Mon, 2014-07-28 at 23:07 +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>
> ---
>  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);
Above piece of code may not be at the right place since during suspend
operation there might be workload on wells. Added to make it analogous
to runtime suspend. Moving it below gem_suspend will be proper way. Is
my understanding correct?
> +
> +		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);
>