Re: [Intel-gfx] [PATCH 3/6] drm/i915/pm: drop intel_suspend_hw()

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Mar 01, 2023 at 03:54:17PM +0200, Jani Nikula wrote:
> All intel_suspend_hw() does is clear PCH_LP_PARTITION_LEVEL_DISABLE bit
> in SOUTH_DSPCLK_GATE_D for LPT LP. intel_suspend_hw() gets called from
> i915_drm_suspend().
> 
> However, i915_drm_suspend_late() calls
> intel_display_power_suspend_late(), which in turn calls hsw_enable_pc8()
> on HSW and BDW. The first thing that does is clear
> PCH_LP_PARTITION_LEVEL_DISABLE bit in SOUTH_DSPCLK_GATE_D.

For a moment I thought that the if HSW || BDW on the other call would
make some difference, but then I confirmed on intel_pch.c that only
HSW and BDW has PCH_LPT anyway. So,


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>


> 
> Remove the duplicated clearing of the bit, effectively delaying it from
> i915_drm_suspend() to i915_drm_suspend_late(), and remove the
> unnecessary intel_suspend_hw() function altogether.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_driver.c |  2 --
>  drivers/gpu/drm/i915/intel_pm.c    | 16 ----------------
>  drivers/gpu/drm/i915/intel_pm.h    |  1 -
>  3 files changed, 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 171ff4edabd6..a53fd339e2cc 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -1079,8 +1079,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>  
>  	intel_suspend_encoders(dev_priv);
>  
> -	intel_suspend_hw(dev_priv);
> -
>  	/* Must be called before GGTT is suspended. */
>  	intel_dpt_suspend(dev_priv);
>  	i915_ggtt_suspend(to_gt(dev_priv)->ggtt);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8b02af531e82..c45af0d981fd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -320,16 +320,6 @@ static void lpt_init_clock_gating(struct drm_i915_private *dev_priv)
>  			 0, TRANS_CHICKEN1_DP0UNIT_GC_DISABLE);
>  }
>  
> -static void lpt_suspend_hw(struct drm_i915_private *dev_priv)
> -{
> -	if (HAS_PCH_LPT_LP(dev_priv)) {
> -		u32 val = intel_uncore_read(&dev_priv->uncore, SOUTH_DSPCLK_GATE_D);
> -
> -		val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
> -		intel_uncore_write(&dev_priv->uncore, SOUTH_DSPCLK_GATE_D, val);
> -	}
> -}
> -
>  static void gen8_set_l3sqc_credits(struct drm_i915_private *dev_priv,
>  				   int general_prio_credits,
>  				   int high_prio_credits)
> @@ -789,12 +779,6 @@ void intel_init_clock_gating(struct drm_i915_private *dev_priv)
>  	dev_priv->clock_gating_funcs->init_clock_gating(dev_priv);
>  }
>  
> -void intel_suspend_hw(struct drm_i915_private *dev_priv)
> -{
> -	if (HAS_PCH_LPT(dev_priv))
> -		lpt_suspend_hw(dev_priv);
> -}
> -
>  static void nop_init_clock_gating(struct drm_i915_private *dev_priv)
>  {
>  	drm_dbg_kms(&dev_priv->drm,
> diff --git a/drivers/gpu/drm/i915/intel_pm.h b/drivers/gpu/drm/i915/intel_pm.h
> index 1dd464d2d186..f774bddcdca6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.h
> +++ b/drivers/gpu/drm/i915/intel_pm.h
> @@ -13,7 +13,6 @@ struct intel_crtc_state;
>  struct intel_plane_state;
>  
>  void intel_init_clock_gating(struct drm_i915_private *dev_priv);
> -void intel_suspend_hw(struct drm_i915_private *dev_priv);
>  void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv);
>  
>  #endif /* __INTEL_PM_H__ */
> -- 
> 2.39.1
>