[Intel-gfx] [PATCH v2 07/14] drm/i915: Start moving the cdclk stuff into a distinct state structure

Ander Conselvan De Oliveira conselvan2 at gmail.com
Fri Dec 23 09:09:22 UTC 2016


On Thu, 2016-12-22 at 16:33 +0200, Ville Syrjälä wrote:
> On Thu, Dec 22, 2016 at 04:14:40PM +0200, Ander Conselvan De Oliveira wrote:
> > 
> > On Mon, 2016-12-19 at 19:28 +0200, ville.syrjala at linux.intel.com wrote:
> > > 
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > 
> > > Introduce intel_cdclk state which for now will track the cdclk
> > > frequency, the vco frequency and the reference frequency (not sure we
> > > want the last one, but I put it there anyway). We'll also make the
> > > .get_cdclk() function fill out this state structure rather than
> > > just returning the current cdclk frequency.
> > > 
> > > One immediate benefit is that calling .get_cdclk() will no longer
> > > clobber state stored under dev_priv unless ex[plicitly told to do
> > Typo: ex[plicity
> > 
> > > 
> > > so. Previously it clobbered the vco and reference clocks stored
> > > there on some platforms.
> > > 
> > > We'll expand the use of this structure to actually precomputing the
> > > state and whatnot later.
> > > 
> > > v2: Constify intel_cdclk_state_compare()
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > A couple more comments beloew, but either way,
> > 
> > Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>
> > 
> > > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c     |   2 +-
> > >  drivers/gpu/drm/i915/i915_drv.h         |  14 +-
> > >  drivers/gpu/drm/i915/intel_audio.c      |   2 +-
> > >  drivers/gpu/drm/i915/intel_cdclk.c      | 359 ++++++++++++++++++--------------
> > >  drivers/gpu/drm/i915/intel_display.c    |  18 +-
> > >  drivers/gpu/drm/i915/intel_dp.c         |   2 +-
> > >  drivers/gpu/drm/i915/intel_drv.h        |   3 +
> > >  drivers/gpu/drm/i915/intel_fbc.c        |   2 +-
> > >  drivers/gpu/drm/i915/intel_panel.c      |   4 +-
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c |   5 +-
> > >  10 files changed, 240 insertions(+), 171 deletions(-)
> > > 

[...]

@@ -629,12 +668,13 @@ static int skl_calc_cdclk(int max_pixclk, int vco)
> > >  }
> > >  
> > > -static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > > +static void skl_dpll0_update(struct drm_i915_private *dev_priv,
> > > +			     struct intel_cdclk_state *cdclk_state)
> > >  {
> > >  	u32 val;
> > >  
> > > -	dev_priv->cdclk_pll.ref = 24000;
> > > -	dev_priv->cdclk_pll.vco = 0;
> > > +	cdclk_state->ref = 24000;
> > > +	cdclk_state->vco = 0;
> > >  
> > >  	val = I915_READ(LCPLL1_CTL);
> > >  	if ((val & LCPLL_PLL_ENABLE) == 0)
> > > @@ -656,11 +696,11 @@ static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > >  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, SKL_DPLL0):
> > >  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1620, SKL_DPLL0):
> > >  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, SKL_DPLL0):
> > > -		dev_priv->cdclk_pll.vco = 8100000;
> > > +		cdclk_state->vco = 8100000;
> > >  		break;
> > >  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1080, SKL_DPLL0):
> > >  	case DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2160, SKL_DPLL0):
> > > -		dev_priv->cdclk_pll.vco = 8640000;
> > > +		cdclk_state->vco = 8640000;
> > >  		break;
> > >  	default:
> > >  		MISSING_CASE(val & DPLL_CTRL1_LINK_RATE_MASK(SKL_DPLL0));
> > This function is a bit funny now, since it sets up the cdclk state based on
> > current pll0 programming, instead of looking at an atomic state.
> We should probably rename it to somehting more descriptive. skl_get_dpll0()
> maybe. Or maybe skl_dpll0_get_config to match the .get_config()
> nomenclature used for eg. encoders.

Hmm, I think the problem is that this have a mix of get_hw_state() and
get_config(). We actually have skl_ddi_dpll0_get_hw_state(), but it wouldn't be
trivial to make that code reusable for the cdclk stuff.

Maybe it should be two separate functions, one that gets the hardware state and
another for choosing the vco. Then

  vco = skl_cdclk_vco(skl_dpll0_get_hw_state());

directly in skl_get_cdclk() ? I'm not really sure, maybe just leave the way it
is for now.

>  Though if we go there then
> .get_cdclk() might need to be renamed to .cdclk_get_config or something.

.cdclk_get_hw_state() ?


Ander

> 
> > 
> > But so far it
> > is only used to read the current cdclk, so there's no harm.
> > 
> > > 
> > > @@ -668,46 +708,57 @@ static void skl_dpll0_update(struct drm_i915_private *dev_priv)
> > >  	}
> > >  }
> > >  
> > > -static int skl_get_cdclk(struct drm_i915_private *dev_priv)
> > > +static void skl_get_cdclk(struct drm_i915_private *dev_priv,
> > > +			  struct intel_cdclk_state *cdclk_state)
> > >  {
> > >  	u32 cdctl;
> > >  
> > > -	skl_dpll0_update(dev_priv);
> > > +	skl_dpll0_update(dev_priv, cdclk_state);
> > > +
> > > +	cdclk_state->cdclk = cdclk_state->ref;
> > >  
> > > -	if (dev_priv->cdclk_pll.vco == 0)
> > > -		return dev_priv->cdclk_pll.ref;
> > > +	if (cdclk_state->vco == 0)
> > > +		return;
> > >  
> > >  	cdctl = I915_READ(CDCLK_CTL);
> > >  
> > > -	if (dev_priv->cdclk_pll.vco == 8640000) {
> > > +	if (cdclk_state->vco == 8640000) {
> > >  		switch (cdctl & CDCLK_FREQ_SEL_MASK) {
> > >  		case CDCLK_FREQ_450_432:
> > > -			return 432000;
> > > +			cdclk_state->cdclk = 432000;
> > > +			break;
> > >  		case CDCLK_FREQ_337_308:
> > > -			return 308571;
> > > +			cdclk_state->cdclk = 308571;
> > > +			break;
> > >  		case CDCLK_FREQ_540:
> > > -			return 540000;
> > > +			cdclk_state->cdclk = 540000;
> > > +			break;
> > >  		case CDCLK_FREQ_675_617:
> > > -			return 617143;
> > > +			cdclk_state->cdclk = 617143;
> > > +			break;
> > >  		default:
> > >  			MISSING_CASE(cdctl & CDCLK_FREQ_SEL_MASK);
> > > +			break;
> > >  		}
> > >  	} else {
> > >  		switch (cdctl & CDCLK_FREQ_SEL_MASK) {
> > >  		case CDCLK_FREQ_450_432:
> > > -			return 450000;
> > > +			cdclk_state->cdclk = 450000;
> > > +			break;
> > >  		case CDCLK_FREQ_337_308:
> > > -			return 337500;
> > > +			cdclk_state->cdclk = 337500;
> > > +			break;
> > >  		case CDCLK_FREQ_540:
> > > -			return 540000;
> > > +			cdclk_state->cdclk = 540000;
> > > +			break;
> > >  		case CDCLK_FREQ_675_617:
> > > -			return 675000;
> > > +			cdclk_state->cdclk = 675000;
> > > +			break;
> > >  		default:
> > >  			MISSING_CASE(cdctl & CDCLK_FREQ_SEL_MASK);
> > > +			break;
> > >  		}
> > >  	}
> > > -
> > > -	return dev_priv->cdclk_pll.ref;
> > >  }
> > >  
> > >  /* convert from kHz to .1 fixpoint MHz with -1MHz offset */
> > > @@ -770,7 +821,7 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
> > >  				    5))
> > >  		DRM_ERROR("DPLL0 not locked\n");
> > >  
> > > -	dev_priv->cdclk_pll.vco = vco;
> > > +	dev_priv->cdclk.hw.vco = vco;
> > >  
> > >  	/* We'll want to keep using the current vco from now on. */
> > >  	skl_set_preferred_cdclk_vco(dev_priv, vco);
> > > @@ -784,7 +835,7 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
> > >  				   1))
> > >  		DRM_ERROR("Couldn't disable DPLL0\n");
> > >  
> > > -	dev_priv->cdclk_pll.vco = 0;
> > > +	dev_priv->cdclk.hw.vco = 0;
> > >  }
> > >  
> > >  static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> > > @@ -834,11 +885,11 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> > >  		break;
> > >  	}
> > >  
> > > -	if (dev_priv->cdclk_pll.vco != 0 &&
> > > -	    dev_priv->cdclk_pll.vco != vco)
> > > +	if (dev_priv->cdclk.hw.vco != 0 &&
> > > +	    dev_priv->cdclk.hw.vco != vco)
> > >  		skl_dpll0_disable(dev_priv);
> > >  
> > > -	if (dev_priv->cdclk_pll.vco != vco)
> > > +	if (dev_priv->cdclk.hw.vco != vco)
> > >  		skl_dpll0_enable(dev_priv, vco);
> > >  
> > >  	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk));
> > > @@ -866,8 +917,8 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  	intel_update_cdclk(dev_priv);
> > >  	/* Is PLL enabled and locked ? */
> > > -	if (dev_priv->cdclk_pll.vco == 0 ||
> > > -	    dev_priv->cdclk_freq == dev_priv->cdclk_pll.ref)
> > > +	if (dev_priv->cdclk.hw.vco == 0 ||
> > > +	    dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.ref)
> > >  		goto sanitize;
> > >  
> > >  	/* DPLL okay; verify the cdclock
> > > @@ -878,7 +929,7 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  	 */
> > >  	cdctl = I915_READ(CDCLK_CTL);
> > >  	expected = (cdctl & CDCLK_FREQ_SEL_MASK) |
> > > -		skl_cdclk_decimal(dev_priv->cdclk_freq);
> > > +		skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
> > >  	if (cdctl == expected)
> > >  		/* All well; nothing to sanitize */
> > >  		return;
> > > @@ -887,14 +938,14 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> > >  
> > >  	/* force cdclk programming */
> > > -	dev_priv->cdclk_freq = 0;
> > > +	dev_priv->cdclk.hw.cdclk = 0;
> > >  	/* force full PLL disable + enable */
> > > -	dev_priv->cdclk_pll.vco = -1;
> > > +	dev_priv->cdclk.hw.vco = -1;
> > >  }
> > >  
> > >  void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >  {
> > > -	skl_set_cdclk(dev_priv, dev_priv->cdclk_pll.ref, 0);
> > > +	skl_set_cdclk(dev_priv, dev_priv->cdclk.hw.ref, 0);
> > >  }
> > >  
> > >  void skl_init_cdclk(struct drm_i915_private *dev_priv)
> > > @@ -903,14 +954,15 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  	skl_sanitize_cdclk(dev_priv);
> > >  
> > > -	if (dev_priv->cdclk_freq != 0 && dev_priv->cdclk_pll.vco != 0) {
> > > +	if (dev_priv->cdclk.hw.cdclk != 0 &&
> > > +	    dev_priv->cdclk.hw.vco != 0) {
> > >  		/*
> > >  		 * Use the current vco as our initial
> > >  		 * guess as to what the preferred vco is.
> > >  		 */
> > >  		if (dev_priv->skl_preferred_vco_freq == 0)
> > >  			skl_set_preferred_cdclk_vco(dev_priv,
> > > -						    dev_priv->cdclk_pll.vco);
> > > +						    dev_priv->cdclk.hw.vco);
> > >  		return;
> > >  	}
> > >  
> > > @@ -950,7 +1002,7 @@ static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> > >  {
> > >  	int ratio;
> > >  
> > > -	if (cdclk == dev_priv->cdclk_pll.ref)
> > > +	if (cdclk == dev_priv->cdclk.hw.ref)
> > >  		return 0;
> > >  
> > >  	switch (cdclk) {
> > > @@ -967,14 +1019,14 @@ static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> > >  		break;
> > >  	}
> > >  
> > > -	return dev_priv->cdclk_pll.ref * ratio;
> > > +	return dev_priv->cdclk.hw.ref * ratio;
> > >  }
> > >  
> > >  static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> > >  {
> > >  	int ratio;
> > >  
> > > -	if (cdclk == dev_priv->cdclk_pll.ref)
> > > +	if (cdclk == dev_priv->cdclk.hw.ref)
> > >  		return 0;
> > >  
> > >  	switch (cdclk) {
> > > @@ -987,15 +1039,16 @@ static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> > >  		break;
> > >  	}
> > >  
> > > -	return dev_priv->cdclk_pll.ref * ratio;
> > > +	return dev_priv->cdclk.hw.ref * ratio;
> > >  }
> > >  
> > > -static void bxt_de_pll_update(struct drm_i915_private *dev_priv)
> > > +static void bxt_de_pll_update(struct drm_i915_private *dev_priv,
> > > +			      struct intel_cdclk_state *cdclk_state)
> > >  {
> > >  	u32 val;
> > >  
> > > -	dev_priv->cdclk_pll.ref = 19200;
> > > -	dev_priv->cdclk_pll.vco = 0;
> > > +	cdclk_state->ref = 19200;
> > > +	cdclk_state->vco = 0;
> > >  
> > >  	val = I915_READ(BXT_DE_PLL_ENABLE);
> > >  	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
> > > @@ -1005,20 +1058,21 @@ static void bxt_de_pll_update(struct drm_i915_private *dev_priv)
> > >  		return;
> > >  
> > >  	val = I915_READ(BXT_DE_PLL_CTL);
> > > -	dev_priv->cdclk_pll.vco = (val & BXT_DE_PLL_RATIO_MASK) *
> > > -		dev_priv->cdclk_pll.ref;
> > > +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
> > >  }
> > >  
> > > -static int bxt_get_cdclk(struct drm_i915_private *dev_priv)
> > > +static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> > > +			  struct intel_cdclk_state *cdclk_state)
> > >  {
> > >  	u32 divider;
> > > -	int div, vco;
> > > +	int div;
> > >  
> > > -	bxt_de_pll_update(dev_priv);
> > > +	bxt_de_pll_update(dev_priv, cdclk_state);
> > >  
> > > -	vco = dev_priv->cdclk_pll.vco;
> > > -	if (vco == 0)
> > > -		return dev_priv->cdclk_pll.ref;
> > > +	cdclk_state->cdclk = cdclk_state->ref;
> > > +
> > > +	if (cdclk_state->vco == 0)
> > > +		return;
> > >  
> > >  	divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
> > >  
> > > @@ -1038,10 +1092,10 @@ static int bxt_get_cdclk(struct drm_i915_private *dev_priv)
> > >  		break;
> > >  	default:
> > >  		MISSING_CASE(divider);
> > > -		return dev_priv->cdclk_pll.ref;
> > > +		return;
> > >  	}
> > >  
> > > -	return DIV_ROUND_CLOSEST(vco, div);
> > > +	cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
> > >  }
> > >  
> > >  static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> > > @@ -1054,12 +1108,12 @@ static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> > >  				    1))
> > >  		DRM_ERROR("timeout waiting for DE PLL unlock\n");
> > >  
> > > -	dev_priv->cdclk_pll.vco = 0;
> > > +	dev_priv->cdclk.hw.vco = 0;
> > >  }
> > >  
> > >  static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> > >  {
> > > -	int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk_pll.ref);
> > > +	int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk.hw.ref);
> > >  	u32 val;
> > >  
> > >  	val = I915_READ(BXT_DE_PLL_CTL);
> > > @@ -1077,7 +1131,7 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> > >  				    1))
> > >  		DRM_ERROR("timeout waiting for DE PLL lock\n");
> > >  
> > > -	dev_priv->cdclk_pll.vco = vco;
> > > +	dev_priv->cdclk.hw.vco = vco;
> > >  }
> > >  
> > >  static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > > @@ -1105,7 +1159,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > >  		divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> > >  		break;
> > >  	default:
> > > -		WARN_ON(cdclk != dev_priv->cdclk_pll.ref);
> > > +		WARN_ON(cdclk != dev_priv->cdclk.hw.ref);
> > >  		WARN_ON(vco != 0);
> > >  
> > >  		divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> > > @@ -1124,11 +1178,11 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > >  		return;
> > >  	}
> > >  
> > > -	if (dev_priv->cdclk_pll.vco != 0 &&
> > > -	    dev_priv->cdclk_pll.vco != vco)
> > > +	if (dev_priv->cdclk.hw.vco != 0 &&
> > > +	    dev_priv->cdclk.hw.vco != vco)
> > >  		bxt_de_pll_disable(dev_priv);
> > >  
> > > -	if (dev_priv->cdclk_pll.vco != vco)
> > > +	if (dev_priv->cdclk.hw.vco != vco)
> > >  		bxt_de_pll_enable(dev_priv, vco);
> > >  
> > >  	val = divider | skl_cdclk_decimal(cdclk);
> > > @@ -1165,8 +1219,8 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  	intel_update_cdclk(dev_priv);
> > >  
> > > -	if (dev_priv->cdclk_pll.vco == 0 ||
> > > -	    dev_priv->cdclk_freq == dev_priv->cdclk_pll.ref)
> > > +	if (dev_priv->cdclk.hw.vco == 0 ||
> > > +	    dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.ref)
> > >  		goto sanitize;
> > >  
> > >  	/* DPLL okay; verify the cdclock
> > > @@ -1184,12 +1238,12 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  	cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> > >  
> > >  	expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
> > > -		   skl_cdclk_decimal(dev_priv->cdclk_freq);
> > > +		skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
> > >  	/*
> > >  	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> > >  	 * enable otherwise.
> > >  	 */
> > > -	if (dev_priv->cdclk_freq >= 500000)
> > > +	if (dev_priv->cdclk.hw.cdclk >= 500000)
> > >  		expected |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> > >  
> > >  	if (cdctl == expected)
> > > @@ -1200,10 +1254,10 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > >  	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> > >  
> > >  	/* force cdclk programming */
> > > -	dev_priv->cdclk_freq = 0;
> > > +	dev_priv->cdclk.hw.cdclk = 0;
> > >  
> > >  	/* force full PLL disable + enable */
> > > -	dev_priv->cdclk_pll.vco = -1;
> > > +	dev_priv->cdclk.hw.vco = -1;
> > >  }
> > >  
> > >  void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> > > @@ -1212,7 +1266,8 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  	bxt_sanitize_cdclk(dev_priv);
> > >  
> > > -	if (dev_priv->cdclk_freq != 0 && dev_priv->cdclk_pll.vco != 0)
> > > +	if (dev_priv->cdclk.hw.cdclk != 0 &&
> > > +	    dev_priv->cdclk.hw.vco != 0)
> > >  		return;
> > >  
> > >  	/*
> > > @@ -1233,7 +1288,13 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >  {
> > > -	bxt_set_cdclk(dev_priv, dev_priv->cdclk_pll.ref, 0);
> > > +	bxt_set_cdclk(dev_priv, dev_priv->cdclk.hw.ref, 0);
> > > +}
> > > +
> > > +bool intel_cdclk_state_compare(const struct intel_cdclk_state *a,
> > > +			       const struct intel_cdclk_state *b)
> > > +{
> > > +	return memcmp(a, b, sizeof(*a)) == 0;
> > >  }
> > >  
> > >  static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > > @@ -1533,7 +1594,7 @@ void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> > >  		dev_priv->max_cdclk_freq = 400000;
> > >  	} else {
> > >  		/* otherwise assume cdclk is fixed */
> > > -		dev_priv->max_cdclk_freq = dev_priv->cdclk_freq;
> > > +		dev_priv->max_cdclk_freq = dev_priv->cdclk.hw.cdclk;
> > >  	}
> > >  
> > >  	dev_priv->max_dotclk_freq = intel_compute_max_dotclk(dev_priv);
> > > @@ -1547,15 +1608,11 @@ void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> > >  
> > >  void intel_update_cdclk(struct drm_i915_private *dev_priv)
> > >  {
> > > -	dev_priv->cdclk_freq = dev_priv->display.get_cdclk(dev_priv);
> > > +	dev_priv->display.get_cdclk(dev_priv, &dev_priv->cdclk.hw);
> > >  
> > > -	if (INTEL_GEN(dev_priv) >= 9)
> > > -		DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz, VCO: %d kHz, ref: %d kHz\n",
> > > -				 dev_priv->cdclk_freq, dev_priv->cdclk_pll.vco,
> > > -				 dev_priv->cdclk_pll.ref);
> > > -	else
> > > -		DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz\n",
> > > -				 dev_priv->cdclk_freq);
> > > +	DRM_DEBUG_DRIVER("Current CD clock rate: %d kHz, VCO: %d kHz, ref: %d kHz\n",
> > > +			 dev_priv->cdclk.hw.cdclk, dev_priv->cdclk.hw.vco,
> > > +			 dev_priv->cdclk.hw.ref);
> > >  
> > >  	/*
> > >  	 * 9:0 CMBUS [sic] CDCLK frequency (cdfreq):
> > > @@ -1565,7 +1622,7 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv)
> > >  	 */
> > >  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > >  		I915_WRITE(GMBUSFREQ_VLV,
> > > -			   DIV_ROUND_UP(dev_priv->cdclk_freq, 1000));
> > > +			   DIV_ROUND_UP(dev_priv->cdclk.hw.cdclk, 1000));
> > >  }
> > >  
> > >  static int pch_rawclk(struct drm_i915_private *dev_priv)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index 519e5d663c5f..6c38ab299506 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -136,7 +136,7 @@ struct intel_limit {
> > >  };
> > >  
> > >  /* returns HPLL frequency in kHz */
> > > -static int valleyview_get_vco(struct drm_i915_private *dev_priv)
> > > +int vlv_get_hpll_vco(struct drm_i915_private *dev_priv)
> > >  {
> > >  	int hpll_freq, vco_freq[] = { 800, 1600, 2000, 2400 };
> > >  
> > > @@ -172,7 +172,7 @@ int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> > >  			   const char *name, u32 reg)
> > >  {
> > >  	if (dev_priv->hpll_freq == 0)
> > > -		dev_priv->hpll_freq = valleyview_get_vco(dev_priv);
> > > +		dev_priv->hpll_freq = vlv_get_hpll_vco(dev_priv);
> > >  
> > >  	return vlv_get_cck_clock(dev_priv, name, reg,
> > >  				 dev_priv->hpll_freq);
> > > @@ -12436,7 +12436,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> > >  	 */
> > >  	if (dev_priv->display.modeset_calc_cdclk) {
> > >  		if (!intel_state->cdclk_pll_vco)
> > > -			intel_state->cdclk_pll_vco = dev_priv->cdclk_pll.vco;
> > > +			intel_state->cdclk_pll_vco = dev_priv->cdclk.hw.vco;
> > >  		if (!intel_state->cdclk_pll_vco)
> > >  			intel_state->cdclk_pll_vco = dev_priv->skl_preferred_vco_freq;
> > >  
> > > @@ -12456,8 +12456,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> > >  		}
> > >  
> > >  		/* All pipes must be switched off while we change the cdclk. */
> > > -		if (intel_state->dev_cdclk != dev_priv->cdclk_freq ||
> > > -		    intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco) {
> > > +		if (intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
> > > +		    intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco) {
> > >  			ret = intel_modeset_all_pipes(state);
> > >  			if (ret < 0)
> > >  				return ret;
> > > @@ -12874,8 +12874,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> > >  		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
> > >  
> > >  		if (dev_priv->display.modeset_commit_cdclk &&
> > > -		    (intel_state->dev_cdclk != dev_priv->cdclk_freq ||
> > > -		     intel_state->cdclk_pll_vco != dev_priv->cdclk_pll.vco))
> > > +		    (intel_state->dev_cdclk != dev_priv->cdclk.hw.cdclk ||
> > > +		     intel_state->cdclk_pll_vco != dev_priv->cdclk.hw.vco))
> > >  			dev_priv->display.modeset_commit_cdclk(state);
> > >  
> > >  		/*
> > > @@ -14724,7 +14724,7 @@ void intel_modeset_init_hw(struct drm_device *dev)
> > >  
> > >  	intel_update_cdclk(dev_priv);
> > >  
> > > -	dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
> > > +	dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
> > >  
> > >  	intel_init_clock_gating(dev_priv);
> > >  }
> > > @@ -14897,7 +14897,7 @@ int intel_modeset_init(struct drm_device *dev)
> > >  
> > >  	intel_update_czclk(dev_priv);
> > >  	intel_update_cdclk(dev_priv);
> > > -	dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq;
> > > +	dev_priv->atomic_cdclk_freq = dev_priv->cdclk.hw.cdclk;
> > >  
> > >  	intel_shared_dpll_init(dev);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 45ebc9632633..73a708b8d887 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -885,7 +885,7 @@ static uint32_t ilk_get_aux_clock_divider(struct intel_dp *intel_dp, int index)
> > >  	 * divide by 2000 and use that
> > >  	 */
> > >  	if (intel_dig_port->port == PORT_A)
> > > -		return DIV_ROUND_CLOSEST(dev_priv->cdclk_freq, 2000);
> > > +		return DIV_ROUND_CLOSEST(dev_priv->cdclk.hw.cdclk, 2000);
> > >  	else
> > >  		return DIV_ROUND_CLOSEST(dev_priv->rawclk_freq, 2000);
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 8d93b7bda3ff..cc0122282a7b 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1225,10 +1225,13 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
> > >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
> > >  void intel_update_cdclk(struct drm_i915_private *dev_priv);
> > >  void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > > +bool intel_cdclk_state_compare(const struct intel_cdclk_state *a,
> > > +			       const struct intel_cdclk_state *b);
> > >  
> > >  /* intel_display.c */
> > >  enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> > >  void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > > +int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
> > >  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
> > >  		      const char *name, u32 reg, int ref_freq);
> > >  int vlv_get_cck_clock_hpll(struct drm_i915_private *dev_priv,
> > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> > > index 463056d80f9b..385251e48c74 100644
> > > --- a/drivers/gpu/drm/i915/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/intel_fbc.c
> > > @@ -829,7 +829,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> > >  
> > >  	/* WaFbcExceedCdClockThreshold:hsw,bdw */
> > >  	if ((IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) &&
> > > -	    cache->crtc.hsw_bdw_pixel_rate >= dev_priv->cdclk_freq * 95 / 100) {
> > > +	    cache->crtc.hsw_bdw_pixel_rate >= dev_priv->cdclk.hw.cdclk * 95 / 100) {
> > >  		fbc->no_fbc_reason = "pixel rate is too big";
> > >  		return false;
> > >  	}
> > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > > index 1a6ff26dea20..cb50c527401f 100644
> > > --- a/drivers/gpu/drm/i915/intel_panel.c
> > > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > > @@ -1315,7 +1315,7 @@ static u32 i9xx_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> > >  	if (IS_PINEVIEW(dev_priv))
> > >  		clock = KHz(dev_priv->rawclk_freq);
> > >  	else
> > > -		clock = KHz(dev_priv->cdclk_freq);
> > > +		clock = KHz(dev_priv->cdclk.hw.cdclk);
> > >  
> > >  	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 32);
> > >  }
> > > @@ -1333,7 +1333,7 @@ static u32 i965_hz_to_pwm(struct intel_connector *connector, u32 pwm_freq_hz)
> > >  	if (IS_G4X(dev_priv))
> > >  		clock = KHz(dev_priv->rawclk_freq);
> > >  	else
> > > -		clock = KHz(dev_priv->cdclk_freq);
> > > +		clock = KHz(dev_priv->cdclk.hw.cdclk);
> > >  
> > >  	return DIV_ROUND_CLOSEST(clock, pwm_freq_hz * 128);
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 6d5efeb35823..14bbecc8f2ed 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -964,9 +964,12 @@ static void gen9_assert_dbuf_enabled(struct drm_i915_private *dev_priv)
> > >  static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> > >  					  struct i915_power_well *power_well)
> > >  {
> > > +	struct intel_cdclk_state cdclk_state = {};
> > > +
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  
> > > -	WARN_ON(dev_priv->cdclk_freq != dev_priv->display.get_cdclk(dev_priv));
> > > +	dev_priv->display.get_cdclk(dev_priv, &cdclk_state);
> > > +	WARN_ON(!intel_cdclk_state_compare(&dev_priv->cdclk.hw, &cdclk_state));
> > >  
> > >  	gen9_assert_dbuf_enabled(dev_priv);
> > >  


More information about the Intel-gfx mailing list