Re: [PATCH 11/16] drm/i915/bxt: Don't toggle power well 1 on-demand

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

On Fri, Apr 01, 2016 at 04:02:42PM +0300, Imre Deak wrote:
> Power well 1 is managed by the DMC firmware so don't toggle it on-demand
> from the driver. This means we need to follow the BSpec display
> initialization sequence during driver loading and resuming (both system
> and runtime) and enable power well 1 only once there. Afterwards DMC
> will toggle power well 1 whenever entering/exiting DC5.
> 
> For this to work we also need to do away getting the PLL power domain,
> since that just kept runtime PM disabled for good.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Make it more like SKL (which also needs more work in this area,
but that's another matter).

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

> ---
>  drivers/gpu/drm/i915/i915_drv.c         | 15 +------
>  drivers/gpu/drm/i915/intel_display.c    | 17 --------
>  drivers/gpu/drm/i915/intel_dpll_mgr.c   |  5 +--
>  drivers/gpu/drm/i915/intel_drv.h        |  2 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 75 +++++++++++++++++++++++++++------
>  5 files changed, 66 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3998f6a..3f56ddf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1070,10 +1070,7 @@ static int hsw_suspend_complete(struct drm_i915_private *dev_priv)
>  
>  static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
>  {
> -	/* TODO: when DC5 support is added disable DC5 here. */
> -
> -	broxton_ddi_phy_uninit(dev_priv);
> -	broxton_uninit_cdclk(dev_priv);
> +	bxt_display_core_uninit(dev_priv);
>  	bxt_enable_dc9(dev_priv);
>  
>  	return 0;
> @@ -1081,16 +1078,8 @@ static int bxt_suspend_complete(struct drm_i915_private *dev_priv)
>  
>  static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>  {
> -	/* TODO: when CSR FW support is added make sure the FW is loaded */
> -
>  	bxt_disable_dc9(dev_priv);
> -
> -	/*
> -	 * TODO: when DC5 support is added enable DC5 here if the CSR FW
> -	 * is available.
> -	 */
> -	broxton_init_cdclk(dev_priv);
> -	broxton_ddi_phy_init(dev_priv);
> +	bxt_display_core_init(dev_priv, true);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d9da89d..1fbe619 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5442,21 +5442,6 @@ static void broxton_set_cdclk(struct drm_i915_private *dev_priv, int frequency)
>  
>  void broxton_init_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	uint32_t val;
> -
> -	/*
> -	 * NDE_RSTWRN_OPT RST PCH Handshake En must always be 0b on BXT
> -	 * or else the reset will hang because there is no PCH to respond.
> -	 * Move the handshake programming to initialization sequence.
> -	 * Previously was left up to BIOS.
> -	 */
> -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> -	val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> -
> -	/* Enable PG1 for cdclk */
> -	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> -
>  	/* check if cd clock is enabled */
>  	if (I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_PLL_ENABLE) {
>  		DRM_DEBUG_KMS("Display already initialized\n");
> @@ -5493,8 +5478,6 @@ void broxton_uninit_cdclk(struct drm_i915_private *dev_priv)
>  
>  	/* Set minimum (bypass) frequency, in effect turning off the DE PLL */
>  	broxton_set_cdclk(dev_priv, 19200);
> -
> -	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
>  static const struct skl_cdclk_entry {
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index fbe88b8..a060b67 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -1644,10 +1644,7 @@ static void intel_ddi_pll_init(struct drm_device *dev)
>  			DRM_DEBUG_KMS("Sanitized cdclk programmed by pre-os\n");
>  		if (!(I915_READ(LCPLL1_CTL) & LCPLL_PLL_ENABLE))
>  			DRM_ERROR("LCPLL1 is disabled\n");
> -	} else if (IS_BROXTON(dev)) {
> -		broxton_init_cdclk(dev_priv);
> -		broxton_ddi_phy_init(dev_priv);
> -	} else {
> +	} else if (!IS_BROXTON(dev_priv)) {
>  		/*
>  		 * The LCPLL register should be turned on by the BIOS. For now
>  		 * let's just check its state and print errors in case
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index e8843a7..4c2083d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1460,6 +1460,8 @@ int intel_power_domains_init(struct drm_i915_private *);
>  void intel_power_domains_fini(struct drm_i915_private *);
>  void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume);
>  void intel_power_domains_suspend(struct drm_i915_private *dev_priv);
> +void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
> +void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
>  const char *
>  intel_display_power_domain_str(enum intel_display_power_domain domain);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 58ed8bc..0c30635 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -419,25 +419,13 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_VGA) |				\
>  	BIT(POWER_DOMAIN_GMBUS) |			\
>  	BIT(POWER_DOMAIN_INIT))
> -#define BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
> -	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> -	BIT(POWER_DOMAIN_PIPE_A) |			\
> -	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> -	BIT(POWER_DOMAIN_TRANSCODER_DSI_A) |		\
> -	BIT(POWER_DOMAIN_TRANSCODER_DSI_C) |		\
> -	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> -	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> -	BIT(POWER_DOMAIN_PORT_DSI) |			\
> -	BIT(POWER_DOMAIN_AUX_A) |			\
> -	BIT(POWER_DOMAIN_PLLS) |			\
> -	BIT(POWER_DOMAIN_INIT))
>  #define BXT_DISPLAY_DC_OFF_POWER_DOMAINS (		\
>  	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
>  	BIT(POWER_DOMAIN_MODESET) |			\
>  	BIT(POWER_DOMAIN_AUX_A) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  #define BXT_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
> -	(POWER_DOMAIN_MASK & ~(BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
> +	(POWER_DOMAIN_MASK & ~(				\
>  	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS)) |	\
>  	BIT(POWER_DOMAIN_INIT))
>  
> @@ -1930,7 +1918,7 @@ static struct i915_power_well bxt_power_wells[] = {
>  	},
>  	{
>  		.name = "power well 1",
> -		.domains = BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS,
> +		.domains = 0,
>  		.ops = &skl_power_well_ops,
>  		.data = SKL_DISP_PW_1,
>  	},
> @@ -2166,6 +2154,61 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&power_domains->lock);
>  }
>  
> +void bxt_display_core_init(struct drm_i915_private *dev_priv,
> +			   bool resume)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *well;
> +	uint32_t val;
> +
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
> +	/*
> +	 * NDE_RSTWRN_OPT RST PCH Handshake En must always be 0b on BXT
> +	 * or else the reset will hang because there is no PCH to respond.
> +	 * Move the handshake programming to initialization sequence.
> +	 * Previously was left up to BIOS.
> +	 */
> +	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +	val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> +
> +	/* Enable PG1 */
> +	mutex_lock(&power_domains->lock);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_enable(dev_priv, well);
> +
> +	mutex_unlock(&power_domains->lock);
> +
> +	broxton_init_cdclk(dev_priv);
> +	broxton_ddi_phy_init(dev_priv);
> +
> +	if (resume && dev_priv->csr.dmc_payload)
> +		intel_csr_load_program(dev_priv);
> +}
> +
> +void bxt_display_core_uninit(struct drm_i915_private *dev_priv)
> +{
> +	struct i915_power_domains *power_domains = &dev_priv->power_domains;
> +	struct i915_power_well *well;
> +
> +	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
> +	broxton_ddi_phy_uninit(dev_priv);
> +	broxton_uninit_cdclk(dev_priv);
> +
> +	/* The spec doesn't call for removing the reset handshake flag */
> +
> +	/* Disable PG1 */
> +	mutex_lock(&power_domains->lock);
> +
> +	well = lookup_power_well(dev_priv, SKL_DISP_PW_1);
> +	intel_power_well_disable(dev_priv, well);
> +
> +	mutex_unlock(&power_domains->lock);
> +}
> +
>  static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_well *cmn_bc =
> @@ -2297,6 +2340,8 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) {
>  		skl_display_core_init(dev_priv, resume);
> +	} else if (IS_BROXTON(dev)) {
> +		bxt_display_core_init(dev_priv, resume);
>  	} else if (IS_CHERRYVIEW(dev)) {
>  		mutex_lock(&power_domains->lock);
>  		chv_phy_control_init(dev_priv);
> @@ -2334,6 +2379,8 @@ void intel_power_domains_suspend(struct drm_i915_private *dev_priv)
>  
>  	if (IS_SKYLAKE(dev_priv) || IS_KABYLAKE(dev_priv))
>  		skl_display_core_uninit(dev_priv);
> +	else if (IS_BROXTON(dev_priv))
> +		bxt_display_core_uninit(dev_priv);
>  }
>  
>  /**
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx