Re: [PATCH 11/21] drm/i915: Move SKL+ DBUF enable/disable to display core init/uninit

[Imre Deak <imre.deak@intel.com>]

On pe, 2016-05-13 at 23:41 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> SKL and BXT have the same snippets of code for enabling disabling the
> DBUF. Extract those into helpers and move the calls from
> init/unit_cdclk() to the display core init/init since this stuff isn't
> really about cdclk. Also doing the enable twice shouldn't hurt since
> you're just setting the request bit again when it was already set.
> 
> We can also toss in a few WARNs about the register values into
> skl_get_dpll0_vco() now that we know that things should always be
> sane there.
> 
> Flatten skl_init_cdclk() while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c    | 58 ++++-----------------------------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 32 ++++++++++++++++++
>  2 files changed, 38 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index da903b718c11..e908f360da74 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5480,18 +5480,6 @@ static bool broxton_cdclk_is_enabled(struct drm_i915_private *dev_priv)
>  
>  	/* TODO: Check for a valid CDCLK rate */
>  
> -	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_REQUEST)) {
> -		DRM_DEBUG_DRIVER("CDCLK enabled, but DBUF power not requested\n");
> -
> -		return false;
> -	}
> -
> -	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE)) {
> -		DRM_DEBUG_DRIVER("CDCLK enabled, but DBUF power hasn't settled\n");
> -
> -		return false;
> -	}
> -
>  	return true;
>  }
>  
> @@ -5518,26 +5506,10 @@ void broxton_init_cdclk(struct drm_i915_private *dev_priv)
>  	 *   here, it belongs to modeset time
>  	 */
>  	broxton_set_cdclk(dev_priv, 624000);
> -
> -	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> -	POSTING_READ(DBUF_CTL);
> -
> -	udelay(10);
> -
> -	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
> -		DRM_ERROR("DBuf power enable timeout!\n");
>  }
>  
>  void broxton_uninit_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
> -	POSTING_READ(DBUF_CTL);
> -
> -	udelay(10);
> -
> -	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> -		DRM_ERROR("DBuf power disable timeout!\n");
> -
>  	/* Set minimum (bypass) frequency, in effect turning off the DE PLL */
>  	broxton_set_cdclk(dev_priv, 19200);
>  }
> @@ -5759,15 +5731,6 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv);
>  
>  void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	/* disable DBUF power */
> -	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
> -	POSTING_READ(DBUF_CTL);
> -
> -	udelay(10);
> -
> -	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> -		DRM_ERROR("DBuf power disable timeout\n");
> -
>  	skl_set_cdclk(dev_priv, 24000, 0);
>  }
>  
> @@ -5785,24 +5748,15 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>  		if (dev_priv->skl_preferred_vco_freq == 0)
>  			skl_set_preferred_cdclk_vco(dev_priv,
>  						    dev_priv->skl_vco_freq);
> -	} else {
> -		/* set CDCLK to the lowest frequency, Modeset follows */
> -		vco = dev_priv->skl_preferred_vco_freq;
> -		if (vco == 0)
> -			vco = 8100;
> -		cdclk = skl_calc_cdclk(0, vco);
> -
> -		skl_set_cdclk(dev_priv, cdclk, vco);
> +		return;
>  	}
>  
> -	/* enable DBUF power */
> -	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> -	POSTING_READ(DBUF_CTL);
> -
> -	udelay(10);
> +	vco = dev_priv->skl_preferred_vco_freq;
> +	if (vco == 0)
> +		vco = 8100;
> +	cdclk = skl_calc_cdclk(0, vco);
>  
> -	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
> -		DRM_ERROR("DBuf power enable timeout\n");
> +	skl_set_cdclk(dev_priv, cdclk, vco);
>  }
>  
>  static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index fefe22c3c163..6817a3cb5fbc 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2176,6 +2176,28 @@ static void intel_power_domains_sync_hw(struct drm_i915_private *dev_priv)
>  	mutex_unlock(&power_domains->lock);
>  }
>  
> +static void skl_dbuf_enable(struct drm_i915_private *dev_priv)

I would've used gen9_ for these, but either way:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +{
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) | DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (!(I915_READ(DBUF_CTL) & DBUF_POWER_STATE))
> +		DRM_ERROR("DBuf power enable timeout\n");
> +}
> +
> +static void skl_dbuf_disable(struct drm_i915_private *dev_priv)
> +{
> +	I915_WRITE(DBUF_CTL, I915_READ(DBUF_CTL) & ~DBUF_POWER_REQUEST);
> +	POSTING_READ(DBUF_CTL);
> +
> +	udelay(10);
> +
> +	if (I915_READ(DBUF_CTL) & DBUF_POWER_STATE)
> +		DRM_ERROR("DBuf power disable timeout!\n");
> +}
> +
>  static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  				   bool resume)
>  {
> @@ -2202,6 +2224,8 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  
>  	skl_init_cdclk(dev_priv);
>  
> +	skl_dbuf_enable(dev_priv);
> +
>  	if (resume && dev_priv->csr.dmc_payload)
>  		intel_csr_load_program(dev_priv);
>  }
> @@ -2213,6 +2237,8 @@ static void skl_display_core_uninit(struct drm_i915_private *dev_priv)
>  
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
> +	skl_dbuf_disable(dev_priv);
> +
>  	skl_uninit_cdclk(dev_priv);
>  
>  	/* The spec doesn't call for removing the reset handshake flag */
> @@ -2257,6 +2283,9 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
>  	mutex_unlock(&power_domains->lock);
>  
>  	broxton_init_cdclk(dev_priv);
> +
> +	skl_dbuf_enable(dev_priv);
> +
>  	broxton_ddi_phy_init(dev_priv);
>  
>  	broxton_cdclk_verify_state(dev_priv);
> @@ -2274,6 +2303,9 @@ void bxt_display_core_uninit(struct drm_i915_private *dev_priv)
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	broxton_ddi_phy_uninit(dev_priv);
> +
> +	skl_dbuf_disable(dev_priv);
> +
>  	broxton_uninit_cdclk(dev_priv);
>  
>  	/* The spec doesn't call for removing the reset handshake flag */
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx