diff mbox

[01/17] drm/i915/icl: add the main CDCLK functions

Message ID 20180123190536.11208-2-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Jan. 23, 2018, 7:05 p.m. UTC
This commit adds the basic CDCLK functions, but it's still missing
pieces of the display initialization sequence.

v2:
 - Implement the voltage levels.
 - Rebase.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  10 +-
 drivers/gpu/drm/i915/intel_cdclk.c | 253 ++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_drv.h   |   2 +
 3 files changed, 261 insertions(+), 4 deletions(-)

Comments

James Ausmus Jan. 26, 2018, 11:14 p.m. UTC | #1
On Tue, Jan 23, 2018 at 05:05:20PM -0200, Paulo Zanoni wrote:
> This commit adds the basic CDCLK functions, but it's still missing
> pieces of the display initialization sequence.
> 
> v2:
>  - Implement the voltage levels.
>  - Rebase.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  10 +-
>  drivers/gpu/drm/i915/intel_cdclk.c | 253 ++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h   |   2 +
>  3 files changed, 261 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index abd9ee876186..d72e206b2b9f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7113,8 +7113,12 @@ enum {
>  #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
>  #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
>  
> -#define SKL_DSSM			_MMIO(0x51004)
> -#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
> +#define SKL_DSSM				_MMIO(0x51004)
> +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
>  
>  #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
>  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
> @@ -8760,6 +8764,8 @@ enum skl_power_gate {
>  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
>  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
>  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)

This isn't right - pipe A is (0 << 19), but pipe B is (2 << 19), and C
is (6 << 19).

> +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
>  
>  /* LCPLL_CTL */
>  #define LCPLL1_CTL		_MMIO(0x46010)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index c4392ea34a3d..d867956d5a9f 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1766,6 +1766,215 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	dev_priv->cdclk.hw.vco = -1;
>  }
>  
> +static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> +{
> +	int ranges_24[] = { 312000, 552000, 648000 };
> +	int ranges_19_38[] = { 307200, 556800, 652800 };
> +	int *ranges;
> +
> +	switch (ref) {
> +	default:
> +		MISSING_CASE(ref);
> +	case 24000:
> +		ranges = ranges_24;
> +		break;
> +	case 19200:
> +	case 38400:
> +		ranges = ranges_19_38;
> +		break;
> +	}
> +
> +	if (min_cdclk > ranges[1])
> +		return ranges[2];
> +	else if (min_cdclk > ranges[0])
> +		return ranges[1];
> +	else
> +		return ranges[0];
> +}
> +
> +static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> +{
> +	int ratio;
> +
> +	/* 50MHz == CDCLK PLL disabled. */
> +	if (cdclk == 50000)
> +		return 0;
> +
> +	switch (cdclk) {
> +	default:
> +		MISSING_CASE(cdclk);
> +	case 307200:
> +	case 556800:
> +	case 652800:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> +			dev_priv->cdclk.hw.ref != 38400);
> +		break;
> +	case 312000:
> +	case 552000:
> +	case 648000:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> +	}
> +
> +	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> +
> +	return dev_priv->cdclk.hw.ref * ratio;
> +}
> +
> +static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> +			  const struct intel_cdclk_state *cdclk_state)
> +{
> +	unsigned int cdclk = cdclk_state->cdclk;
> +	unsigned int vco = cdclk_state->vco;
> +	int ret;
> +	u32 voltage_level;
> +
> +	mutex_lock(&dev_priv->pcu_lock);
> +	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +				SKL_CDCLK_PREPARE_FOR_CHANGE,
> +				SKL_CDCLK_READY_FOR_CHANGE,
> +				SKL_CDCLK_READY_FOR_CHANGE, 3);
> +	mutex_unlock(&dev_priv->pcu_lock);
> +	if (ret) {
> +		DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
> +			  ret);
> +		return;
> +	}
> +
> +	/* FIXME: We should also consider the DDI clock here. */
> +	switch (cdclk) {
> +	case 307200:
> +	case 312000:
> +		voltage_level = 0;
> +		break;
> +	case 556800:
> +	case 552000:
> +		voltage_level = 1;
> +		break;
> +	default:
> +		MISSING_CASE(cdclk);
> +	case 652800:
> +	case 648000:
> +		voltage_level = 2;
> +		break;
> +	}
> +
> +	if (dev_priv->cdclk.hw.vco != 0 &&
> +	    dev_priv->cdclk.hw.vco != vco)
> +		cnl_cdclk_pll_disable(dev_priv);
> +
> +	if (dev_priv->cdclk.hw.vco != vco)
> +		cnl_cdclk_pll_enable(dev_priv, vco);
> +
> +	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> +			      skl_cdclk_decimal(cdclk));
> +
> +	mutex_lock(&dev_priv->pcu_lock);
> +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +				voltage_level);
> +	mutex_unlock(&dev_priv->pcu_lock);
> +
> +	intel_update_cdclk(dev_priv);
> +}
> +
> +static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> +			  struct intel_cdclk_state *cdclk_state)
> +{
> +	u32 val;
> +
> +	val = I915_READ(SKL_DSSM);
> +	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> +	default:
> +		MISSING_CASE(val);
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
> +		cdclk_state->ref = 24000;
> +		break;
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
> +		cdclk_state->ref = 19200;
> +		break;
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
> +		cdclk_state->ref = 38400;
> +		break;
> +	}
> +
> +	val = I915_READ(BXT_DE_PLL_ENABLE);
> +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
> +	    (val & BXT_DE_PLL_LOCK) == 0) {
> +		/* CDCLK PLL is disabled, the VCO/ratio doesn't matter, but
> +		 * setting it to zero is a way to signal that. */
> +		cdclk_state->vco = 0;
> +		cdclk_state->cdclk = 50000;
> +		return;
> +	}
> +
> +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
> +
> +	val = I915_READ(CDCLK_CTL);
> +	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
> +
> +	cdclk_state->cdclk = cdclk_state->vco / 2;
> +}
> +
> +/**
> + * icl_init_cdclk - Initialize CDCLK on ICL
> + * @dev_priv: i915 device
> + *
> + * Initialize CDCLK for ICL. This consists mainly of initializing
> + * dev_priv->cdclk.hw and sanitizing the state of the hardware if needed. This
> + * is generally done only during the display core initialization sequence, after
> + * which the DMC will take care of turning CDCLK off/on as needed.
> + */
> +void icl_init_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_cdclk_state cdclk_state;
> +	u32 val;
> +
> +	/* This sets dev_priv->cdclk.hw. */
> +	intel_update_cdclk(dev_priv);
> +
> +	cdclk_state = dev_priv->cdclk.hw;
> +
> +	/* This means CDCLK disabled. */
> +	if (cdclk_state.cdclk == 50000)
> +		goto sanitize;
> +
> +	val = I915_READ(CDCLK_CTL);
> +
> +	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
> +		goto sanitize;
> +
> +	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
> +	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
> +		goto sanitize;
> +
> +	return;
> +
> +sanitize:
> +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> +
> +	cdclk_state.ref = dev_priv->cdclk.hw.ref;
> +	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
> +	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> +
> +	icl_set_cdclk(dev_priv, &cdclk_state);
> +}
> +
> +/**
> + * icl_uninit_cdclk - Uninitialize CDCLK on ICL
> + * @dev_priv: i915 device
> + *
> + * Uninitialize CDCLK for ICL. This is done only during the display core
> + * uninitialization sequence.
> + */
> +void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> +
> +	cdclk_state.cdclk = cdclk_state.ref;
> +	cdclk_state.vco = 0;
> +
> +	icl_set_cdclk(dev_priv, &cdclk_state);
> +}
> +
>  /**
>   * cnl_init_cdclk - Initialize CDCLK on CNL
>   * @dev_priv: i915 device
> @@ -2204,6 +2413,36 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	return 0;
>  }
>  
> +static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	unsigned int ref = intel_state->cdclk.logical.ref;
> +	int min_cdclk, cdclk, vco;
> +
> +	min_cdclk = intel_compute_min_cdclk(state);
> +	if (min_cdclk < 0)
> +		return min_cdclk;
> +
> +	cdclk = icl_calc_cdclk(min_cdclk, ref);
> +	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> +
> +	intel_state->cdclk.logical.vco = vco;
> +	intel_state->cdclk.logical.cdclk = cdclk;
> +
> +	if (!intel_state->active_crtcs) {
> +		cdclk = icl_calc_cdclk(0, ref);
> +		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> +
> +		intel_state->cdclk.actual.vco = vco;
> +		intel_state->cdclk.actual.cdclk = cdclk;
> +	} else {
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
> +	}
> +
> +	return 0;
> +}
> +
>  static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  {
>  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> @@ -2237,7 +2476,12 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>   */
>  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_CANNONLAKE(dev_priv)) {
> +	if (IS_ICELAKE(dev_priv)) {
> +		if (dev_priv->cdclk.hw.ref == 24000)
> +			dev_priv->max_cdclk_freq = 648000;
> +		else
> +			dev_priv->max_cdclk_freq = 652800;
> +	} else if (IS_CANNONLAKE(dev_priv)) {
>  		dev_priv->max_cdclk_freq = 528000;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> @@ -2461,9 +2705,14 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.set_cdclk = cnl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			cnl_modeset_calc_cdclk;
> +	} else if (IS_ICELAKE(dev_priv)) {
> +		dev_priv->display.set_cdclk = icl_set_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>  	}
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		dev_priv->display.get_cdclk = icl_get_cdclk;
> +	else if (IS_CANNONLAKE(dev_priv))
>  		dev_priv->display.get_cdclk = cnl_get_cdclk;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dev_priv->display.get_cdclk = skl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3cee54bc0352..c5d6092aca41 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1403,6 +1403,8 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv);
>  void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
>  void bxt_init_cdclk(struct drm_i915_private *dev_priv);
>  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> +void icl_init_cdclk(struct drm_i915_private *dev_priv);
> +void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
>  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);
> -- 
> 2.14.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak Jan. 29, 2018, 10:51 a.m. UTC | #2
On Tue, Jan 23, 2018 at 05:05:20PM -0200, Paulo Zanoni wrote:
> This commit adds the basic CDCLK functions, but it's still missing
> pieces of the display initialization sequence.
> 
> v2:
>  - Implement the voltage levels.
>  - Rebase.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  10 +-
>  drivers/gpu/drm/i915/intel_cdclk.c | 253 ++++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_drv.h   |   2 +
>  3 files changed, 261 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index abd9ee876186..d72e206b2b9f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7113,8 +7113,12 @@ enum {
>  #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
>  #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
>  
> -#define SKL_DSSM			_MMIO(0x51004)
> -#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
> +#define SKL_DSSM				_MMIO(0x51004)
> +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> +#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
>  
>  #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
>  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
> @@ -8760,6 +8764,8 @@ enum skl_power_gate {
>  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
>  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
>  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
>  
>  /* LCPLL_CTL */
>  #define LCPLL1_CTL		_MMIO(0x46010)
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index c4392ea34a3d..d867956d5a9f 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1766,6 +1766,215 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	dev_priv->cdclk.hw.vco = -1;
>  }
>  
> +static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> +{
> +	int ranges_24[] = { 312000, 552000, 648000 };
> +	int ranges_19_38[] = { 307200, 556800, 652800 };
> +	int *ranges;
> +
> +	switch (ref) {
> +	default:
> +		MISSING_CASE(ref);
> +	case 24000:
> +		ranges = ranges_24;
> +		break;
> +	case 19200:
> +	case 38400:
> +		ranges = ranges_19_38;
> +		break;
> +	}
> +
> +	if (min_cdclk > ranges[1])
> +		return ranges[2];
> +	else if (min_cdclk > ranges[0])
> +		return ranges[1];
> +	else
> +		return ranges[0];
> +}
> +
> +static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> +{
> +	int ratio;
> +
> +	/* 50MHz == CDCLK PLL disabled. */
> +	if (cdclk == 50000)

Here and everywhere else cdclk.hw.bypass should be used instead of hard
coding it.

> +		return 0;
> +
> +	switch (cdclk) {
> +	default:
> +		MISSING_CASE(cdclk);
> +	case 307200:
> +	case 556800:
> +	case 652800:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> +			dev_priv->cdclk.hw.ref != 38400);
> +		break;
> +	case 312000:
> +	case 552000:
> +	case 648000:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> +	}
> +
> +	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> +
> +	return dev_priv->cdclk.hw.ref * ratio;
> +}
> +
> +static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> +			  const struct intel_cdclk_state *cdclk_state)
> +{
> +	unsigned int cdclk = cdclk_state->cdclk;
> +	unsigned int vco = cdclk_state->vco;
> +	int ret;
> +	u32 voltage_level;
> +
> +	mutex_lock(&dev_priv->pcu_lock);
> +	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +				SKL_CDCLK_PREPARE_FOR_CHANGE,
> +				SKL_CDCLK_READY_FOR_CHANGE,
> +				SKL_CDCLK_READY_FOR_CHANGE, 3);
> +	mutex_unlock(&dev_priv->pcu_lock);
> +	if (ret) {
> +		DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
> +			  ret);
> +		return;
> +	}
> +
> +	/* FIXME: We should also consider the DDI clock here. */
> +	switch (cdclk) {
> +	case 307200:
> +	case 312000:
> +		voltage_level = 0;
> +		break;
> +	case 556800:
> +	case 552000:
> +		voltage_level = 1;
> +		break;
> +	default:
> +		MISSING_CASE(cdclk);
> +	case 652800:
> +	case 648000:
> +		voltage_level = 2;
> +		break;
> +	}
> +
> +	if (dev_priv->cdclk.hw.vco != 0 &&
> +	    dev_priv->cdclk.hw.vco != vco)
> +		cnl_cdclk_pll_disable(dev_priv);
> +
> +	if (dev_priv->cdclk.hw.vco != vco)
> +		cnl_cdclk_pll_enable(dev_priv, vco);
> +
> +	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> +			      skl_cdclk_decimal(cdclk));
> +
> +	mutex_lock(&dev_priv->pcu_lock);
> +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> +				voltage_level);
> +	mutex_unlock(&dev_priv->pcu_lock);
> +
> +	intel_update_cdclk(dev_priv);
> +}
> +
> +static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> +			  struct intel_cdclk_state *cdclk_state)
> +{
> +	u32 val;
> +
> +	val = I915_READ(SKL_DSSM);
> +	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> +	default:
> +		MISSING_CASE(val);
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
> +		cdclk_state->ref = 24000;
> +		break;
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
> +		cdclk_state->ref = 19200;
> +		break;
> +	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
> +		cdclk_state->ref = 38400;
> +		break;
> +	}
> +
> +	val = I915_READ(BXT_DE_PLL_ENABLE);
> +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
> +	    (val & BXT_DE_PLL_LOCK) == 0) {
> +		/* CDCLK PLL is disabled, the VCO/ratio doesn't matter, but
> +		 * setting it to zero is a way to signal that. */
> +		cdclk_state->vco = 0;
> +		cdclk_state->cdclk = 50000;
> +		return;
> +	}
> +
> +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
> +
> +	val = I915_READ(CDCLK_CTL);
> +	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
> +
> +	cdclk_state->cdclk = cdclk_state->vco / 2;
> +}
> +
> +/**
> + * icl_init_cdclk - Initialize CDCLK on ICL
> + * @dev_priv: i915 device
> + *
> + * Initialize CDCLK for ICL. This consists mainly of initializing
> + * dev_priv->cdclk.hw and sanitizing the state of the hardware if needed. This
> + * is generally done only during the display core initialization sequence, after
> + * which the DMC will take care of turning CDCLK off/on as needed.
> + */
> +void icl_init_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_cdclk_state cdclk_state;
> +	u32 val;
> +
> +	/* This sets dev_priv->cdclk.hw. */
> +	intel_update_cdclk(dev_priv);
> +
> +	cdclk_state = dev_priv->cdclk.hw;
> +
> +	/* This means CDCLK disabled. */
> +	if (cdclk_state.cdclk == 50000)
> +		goto sanitize;
> +
> +	val = I915_READ(CDCLK_CTL);
> +
> +	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
> +		goto sanitize;
> +
> +	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
> +	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
> +		goto sanitize;
> +
> +	return;
> +
> +sanitize:
> +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> +
> +	cdclk_state.ref = dev_priv->cdclk.hw.ref;
> +	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
> +	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> +
> +	icl_set_cdclk(dev_priv, &cdclk_state);
> +}
> +
> +/**
> + * icl_uninit_cdclk - Uninitialize CDCLK on ICL
> + * @dev_priv: i915 device
> + *
> + * Uninitialize CDCLK for ICL. This is done only during the display core
> + * uninitialization sequence.
> + */
> +void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> +
> +	cdclk_state.cdclk = cdclk_state.ref;

On ICL it's the slow clock (croclock) used for the bypass clock source,
so have to set cdclk_state.bypass here.

> +	cdclk_state.vco = 0;
> +
> +	icl_set_cdclk(dev_priv, &cdclk_state);
> +}
> +
>  /**
>   * cnl_init_cdclk - Initialize CDCLK on CNL
>   * @dev_priv: i915 device
> @@ -2204,6 +2413,36 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	return 0;
>  }
>  
> +static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	unsigned int ref = intel_state->cdclk.logical.ref;
> +	int min_cdclk, cdclk, vco;
> +
> +	min_cdclk = intel_compute_min_cdclk(state);
> +	if (min_cdclk < 0)
> +		return min_cdclk;
> +
> +	cdclk = icl_calc_cdclk(min_cdclk, ref);
> +	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> +
> +	intel_state->cdclk.logical.vco = vco;
> +	intel_state->cdclk.logical.cdclk = cdclk;
> +
> +	if (!intel_state->active_crtcs) {
> +		cdclk = icl_calc_cdclk(0, ref);
> +		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> +
> +		intel_state->cdclk.actual.vco = vco;
> +		intel_state->cdclk.actual.cdclk = cdclk;
> +	} else {
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
> +	}
> +
> +	return 0;
> +}
> +
>  static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  {
>  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> @@ -2237,7 +2476,12 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>   */
>  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_CANNONLAKE(dev_priv)) {
> +	if (IS_ICELAKE(dev_priv)) {
> +		if (dev_priv->cdclk.hw.ref == 24000)
> +			dev_priv->max_cdclk_freq = 648000;
> +		else
> +			dev_priv->max_cdclk_freq = 652800;
> +	} else if (IS_CANNONLAKE(dev_priv)) {
>  		dev_priv->max_cdclk_freq = 528000;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
> @@ -2461,9 +2705,14 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.set_cdclk = cnl_set_cdclk;
>  		dev_priv->display.modeset_calc_cdclk =
>  			cnl_modeset_calc_cdclk;
> +	} else if (IS_ICELAKE(dev_priv)) {
> +		dev_priv->display.set_cdclk = icl_set_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
>  	}
>  
> -	if (IS_CANNONLAKE(dev_priv))
> +	if (IS_ICELAKE(dev_priv))
> +		dev_priv->display.get_cdclk = icl_get_cdclk;
> +	else if (IS_CANNONLAKE(dev_priv))
>  		dev_priv->display.get_cdclk = cnl_get_cdclk;
>  	else if (IS_GEN9_BC(dev_priv))
>  		dev_priv->display.get_cdclk = skl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3cee54bc0352..c5d6092aca41 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1403,6 +1403,8 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv);
>  void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
>  void bxt_init_cdclk(struct drm_i915_private *dev_priv);
>  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> +void icl_init_cdclk(struct drm_i915_private *dev_priv);
> +void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
>  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);
> -- 
> 2.14.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Feb. 1, 2018, 8:08 p.m. UTC | #3
Em Seg, 2018-01-29 às 12:51 +0200, Imre Deak escreveu:
> On Tue, Jan 23, 2018 at 05:05:20PM -0200, Paulo Zanoni wrote:
> > This commit adds the basic CDCLK functions, but it's still missing
> > pieces of the display initialization sequence.
> > 
> > v2:
> >  - Implement the voltage levels.
> >  - Rebase.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  10 +-
> >  drivers/gpu/drm/i915/intel_cdclk.c | 253
> > ++++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_drv.h   |   2 +
> >  3 files changed, 261 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index abd9ee876186..d72e206b2b9f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7113,8 +7113,12 @@ enum {
> >  #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
> >  #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
> >  
> > -#define SKL_DSSM			_MMIO(0x51004)
> > -#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
> > +#define SKL_DSSM				_MMIO(0x51004)
> > +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
> >  
> >  #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
> >  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
> > @@ -8760,6 +8764,8 @@ enum skl_power_gate {
> >  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> >  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
> >  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> > +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
> >  
> >  /* LCPLL_CTL */
> >  #define LCPLL1_CTL		_MMIO(0x46010)
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > b/drivers/gpu/drm/i915/intel_cdclk.c
> > index c4392ea34a3d..d867956d5a9f 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1766,6 +1766,215 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >  	dev_priv->cdclk.hw.vco = -1;
> >  }
> >  
> > +static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > +{
> > +	int ranges_24[] = { 312000, 552000, 648000 };
> > +	int ranges_19_38[] = { 307200, 556800, 652800 };
> > +	int *ranges;
> > +
> > +	switch (ref) {
> > +	default:
> > +		MISSING_CASE(ref);
> > +	case 24000:
> > +		ranges = ranges_24;
> > +		break;
> > +	case 19200:
> > +	case 38400:
> > +		ranges = ranges_19_38;
> > +		break;
> > +	}
> > +
> > +	if (min_cdclk > ranges[1])
> > +		return ranges[2];
> > +	else if (min_cdclk > ranges[0])
> > +		return ranges[1];
> > +	else
> > +		return ranges[0];
> > +}
> > +
> > +static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > +{
> > +	int ratio;
> > +
> > +	/* 50MHz == CDCLK PLL disabled. */
> > +	if (cdclk == 50000)
> 
> Here and everywhere else cdclk.hw.bypass should be used instead of
> hard
> coding it.

I really don't understand your comment, and I also looked at "drm/i915:
Add tracking for CDCLK bypass frequency" and don't understand it
either.

On our documentation I can't find anything named "bypass clock", and
there's no explanation in our code nor the commit message. The 50MHz is
the CDCLK value (not "bypass clock") when the CDCLK PLL is disabled.

In the driver, I see the "bypass" clock is set to the refclk we read
from the hardware, and that has nothing to do with the 50MHz we're
talking about at that point. We can't even read that 50MHz value from
the hardware. On ICL, the refclk is either 24, 19.2 or 38.4, we can't
read the 50 anywhere.

I'll really need a better explanation on what exactly you meant with
this "bypass" clock thing if I need to implement it on ICL. I just
can't see those "50000" references going away. I can't see what's wrong
with this code.

> 
> > +		return 0;
> > +
> > +	switch (cdclk) {
> > +	default:
> > +		MISSING_CASE(cdclk);
> > +	case 307200:
> > +	case 556800:
> > +	case 652800:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > +			dev_priv->cdclk.hw.ref != 38400);
> > +		break;
> > +	case 312000:
> > +	case 552000:
> > +	case 648000:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > +	}
> > +
> > +	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > +
> > +	return dev_priv->cdclk.hw.ref * ratio;
> > +}
> > +
> > +static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > +			  const struct intel_cdclk_state
> > *cdclk_state)
> > +{
> > +	unsigned int cdclk = cdclk_state->cdclk;
> > +	unsigned int vco = cdclk_state->vco;
> > +	int ret;
> > +	u32 voltage_level;
> > +
> > +	mutex_lock(&dev_priv->pcu_lock);
> > +	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > +				SKL_CDCLK_PREPARE_FOR_CHANGE,
> > +				SKL_CDCLK_READY_FOR_CHANGE,
> > +				SKL_CDCLK_READY_FOR_CHANGE, 3);
> > +	mutex_unlock(&dev_priv->pcu_lock);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to inform PCU about cdclk change
> > (%d)\n",
> > +			  ret);
> > +		return;
> > +	}
> > +
> > +	/* FIXME: We should also consider the DDI clock here. */
> > +	switch (cdclk) {
> > +	case 307200:
> > +	case 312000:
> > +		voltage_level = 0;
> > +		break;
> > +	case 556800:
> > +	case 552000:
> > +		voltage_level = 1;
> > +		break;
> > +	default:
> > +		MISSING_CASE(cdclk);
> > +	case 652800:
> > +	case 648000:
> > +		voltage_level = 2;
> > +		break;
> > +	}
> > +
> > +	if (dev_priv->cdclk.hw.vco != 0 &&
> > +	    dev_priv->cdclk.hw.vco != vco)
> > +		cnl_cdclk_pll_disable(dev_priv);
> > +
> > +	if (dev_priv->cdclk.hw.vco != vco)
> > +		cnl_cdclk_pll_enable(dev_priv, vco);
> > +
> > +	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> > +			      skl_cdclk_decimal(cdclk));
> > +
> > +	mutex_lock(&dev_priv->pcu_lock);
> > +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > +				voltage_level);
> > +	mutex_unlock(&dev_priv->pcu_lock);
> > +
> > +	intel_update_cdclk(dev_priv);
> > +}
> > +
> > +static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> > +			  struct intel_cdclk_state *cdclk_state)
> > +{
> > +	u32 val;
> > +
> > +	val = I915_READ(SKL_DSSM);
> > +	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> > +	default:
> > +		MISSING_CASE(val);
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
> > +		cdclk_state->ref = 24000;
> > +		break;
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
> > +		cdclk_state->ref = 19200;
> > +		break;
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
> > +		cdclk_state->ref = 38400;
> > +		break;
> > +	}
> > +
> > +	val = I915_READ(BXT_DE_PLL_ENABLE);
> > +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
> > +	    (val & BXT_DE_PLL_LOCK) == 0) {
> > +		/* CDCLK PLL is disabled, the VCO/ratio doesn't
> > matter, but
> > +		 * setting it to zero is a way to signal that. */
> > +		cdclk_state->vco = 0;
> > +		cdclk_state->cdclk = 50000;
> > +		return;
> > +	}
> > +
> > +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) *
> > cdclk_state->ref;
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
> > +
> > +	cdclk_state->cdclk = cdclk_state->vco / 2;
> > +}
> > +
> > +/**
> > + * icl_init_cdclk - Initialize CDCLK on ICL
> > + * @dev_priv: i915 device
> > + *
> > + * Initialize CDCLK for ICL. This consists mainly of initializing
> > + * dev_priv->cdclk.hw and sanitizing the state of the hardware if
> > needed. This
> > + * is generally done only during the display core initialization
> > sequence, after
> > + * which the DMC will take care of turning CDCLK off/on as needed.
> > + */
> > +void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_cdclk_state cdclk_state;
> > +	u32 val;
> > +
> > +	/* This sets dev_priv->cdclk.hw. */
> > +	intel_update_cdclk(dev_priv);
> > +
> > +	cdclk_state = dev_priv->cdclk.hw;
> > +
> > +	/* This means CDCLK disabled. */
> > +	if (cdclk_state.cdclk == 50000)
> > +		goto sanitize;
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +
> > +	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
> > +		goto sanitize;
> > +
> > +	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
> > +	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
> > +		goto sanitize;
> > +
> > +	return;
> > +
> > +sanitize:
> > +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> > +
> > +	cdclk_state.ref = dev_priv->cdclk.hw.ref;
> > +	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
> > +	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> > cdclk_state.cdclk);
> > +
> > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > +}
> > +
> > +/**
> > + * icl_uninit_cdclk - Uninitialize CDCLK on ICL
> > + * @dev_priv: i915 device
> > + *
> > + * Uninitialize CDCLK for ICL. This is done only during the
> > display core
> > + * uninitialization sequence.
> > + */
> > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> > +
> > +	cdclk_state.cdclk = cdclk_state.ref;
> 
> On ICL it's the slow clock (croclock) used for the bypass clock
> source,
> so have to set cdclk_state.bypass here.
> 
> > +	cdclk_state.vco = 0;
> > +
> > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > +}
> > +
> >  /**
> >   * cnl_init_cdclk - Initialize CDCLK on CNL
> >   * @dev_priv: i915 device
> > @@ -2204,6 +2413,36 @@ static int cnl_modeset_calc_cdclk(struct
> > drm_atomic_state *state)
> >  	return 0;
> >  }
> >  
> > +static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct intel_atomic_state *intel_state =
> > to_intel_atomic_state(state);
> > +	unsigned int ref = intel_state->cdclk.logical.ref;
> > +	int min_cdclk, cdclk, vco;
> > +
> > +	min_cdclk = intel_compute_min_cdclk(state);
> > +	if (min_cdclk < 0)
> > +		return min_cdclk;
> > +
> > +	cdclk = icl_calc_cdclk(min_cdclk, ref);
> > +	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > +
> > +	intel_state->cdclk.logical.vco = vco;
> > +	intel_state->cdclk.logical.cdclk = cdclk;
> > +
> > +	if (!intel_state->active_crtcs) {
> > +		cdclk = icl_calc_cdclk(0, ref);
> > +		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > +
> > +		intel_state->cdclk.actual.vco = vco;
> > +		intel_state->cdclk.actual.cdclk = cdclk;
> > +	} else {
> > +		intel_state->cdclk.actual = intel_state-
> > >cdclk.logical;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int intel_compute_max_dotclk(struct drm_i915_private
> > *dev_priv)
> >  {
> >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > @@ -2237,7 +2476,12 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >   */
> >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > -	if (IS_CANNONLAKE(dev_priv)) {
> > +	if (IS_ICELAKE(dev_priv)) {
> > +		if (dev_priv->cdclk.hw.ref == 24000)
> > +			dev_priv->max_cdclk_freq = 648000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 652800;
> > +	} else if (IS_CANNONLAKE(dev_priv)) {
> >  		dev_priv->max_cdclk_freq = 528000;
> >  	} else if (IS_GEN9_BC(dev_priv)) {
> >  		u32 limit = I915_READ(SKL_DFSM) &
> > SKL_DFSM_CDCLK_LIMIT_MASK;
> > @@ -2461,9 +2705,14 @@ void intel_init_cdclk_hooks(struct
> > drm_i915_private *dev_priv)
> >  		dev_priv->display.set_cdclk = cnl_set_cdclk;
> >  		dev_priv->display.modeset_calc_cdclk =
> >  			cnl_modeset_calc_cdclk;
> > +	} else if (IS_ICELAKE(dev_priv)) {
> > +		dev_priv->display.set_cdclk = icl_set_cdclk;
> > +		dev_priv->display.modeset_calc_cdclk =
> > icl_modeset_calc_cdclk;
> >  	}
> >  
> > -	if (IS_CANNONLAKE(dev_priv))
> > +	if (IS_ICELAKE(dev_priv))
> > +		dev_priv->display.get_cdclk = icl_get_cdclk;
> > +	else if (IS_CANNONLAKE(dev_priv))
> >  		dev_priv->display.get_cdclk = cnl_get_cdclk;
> >  	else if (IS_GEN9_BC(dev_priv))
> >  		dev_priv->display.get_cdclk = skl_get_cdclk;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 3cee54bc0352..c5d6092aca41 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1403,6 +1403,8 @@ void cnl_init_cdclk(struct drm_i915_private
> > *dev_priv);
> >  void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
> >  void bxt_init_cdclk(struct drm_i915_private *dev_priv);
> >  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> > +void icl_init_cdclk(struct drm_i915_private *dev_priv);
> > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
> >  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);
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zanoni, Paulo R Feb. 1, 2018, 8:09 p.m. UTC | #4
Em Sex, 2018-01-26 às 15:14 -0800, James Ausmus escreveu:
> On Tue, Jan 23, 2018 at 05:05:20PM -0200, Paulo Zanoni wrote:
> > This commit adds the basic CDCLK functions, but it's still missing
> > pieces of the display initialization sequence.
> > 
> > v2:
> >  - Implement the voltage levels.
> >  - Rebase.
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |  10 +-
> >  drivers/gpu/drm/i915/intel_cdclk.c | 253
> > ++++++++++++++++++++++++++++++++++++-
> >  drivers/gpu/drm/i915/intel_drv.h   |   2 +
> >  3 files changed, 261 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index abd9ee876186..d72e206b2b9f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -7113,8 +7113,12 @@ enum {
> >  #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
> >  #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
> >  
> > -#define SKL_DSSM			_MMIO(0x51004)
> > -#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
> > +#define SKL_DSSM				_MMIO(0x51004)
> > +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> > +#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
> >  
> >  #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
> >  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
> > @@ -8760,6 +8764,8 @@ enum skl_power_gate {
> >  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> >  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
> >  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> 
> This isn't right - pipe A is (0 << 19), but pipe B is (2 << 19), and
> C
> is (6 << 19).

Right. I'll fix that once the other review comments on this patch are
clarified.

> 
> > +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
> >  
> >  /* LCPLL_CTL */
> >  #define LCPLL1_CTL		_MMIO(0x46010)
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > b/drivers/gpu/drm/i915/intel_cdclk.c
> > index c4392ea34a3d..d867956d5a9f 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1766,6 +1766,215 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >  	dev_priv->cdclk.hw.vco = -1;
> >  }
> >  
> > +static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > +{
> > +	int ranges_24[] = { 312000, 552000, 648000 };
> > +	int ranges_19_38[] = { 307200, 556800, 652800 };
> > +	int *ranges;
> > +
> > +	switch (ref) {
> > +	default:
> > +		MISSING_CASE(ref);
> > +	case 24000:
> > +		ranges = ranges_24;
> > +		break;
> > +	case 19200:
> > +	case 38400:
> > +		ranges = ranges_19_38;
> > +		break;
> > +	}
> > +
> > +	if (min_cdclk > ranges[1])
> > +		return ranges[2];
> > +	else if (min_cdclk > ranges[0])
> > +		return ranges[1];
> > +	else
> > +		return ranges[0];
> > +}
> > +
> > +static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > +{
> > +	int ratio;
> > +
> > +	/* 50MHz == CDCLK PLL disabled. */
> > +	if (cdclk == 50000)
> > +		return 0;
> > +
> > +	switch (cdclk) {
> > +	default:
> > +		MISSING_CASE(cdclk);
> > +	case 307200:
> > +	case 556800:
> > +	case 652800:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > +			dev_priv->cdclk.hw.ref != 38400);
> > +		break;
> > +	case 312000:
> > +	case 552000:
> > +	case 648000:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > +	}
> > +
> > +	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > +
> > +	return dev_priv->cdclk.hw.ref * ratio;
> > +}
> > +
> > +static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > +			  const struct intel_cdclk_state
> > *cdclk_state)
> > +{
> > +	unsigned int cdclk = cdclk_state->cdclk;
> > +	unsigned int vco = cdclk_state->vco;
> > +	int ret;
> > +	u32 voltage_level;
> > +
> > +	mutex_lock(&dev_priv->pcu_lock);
> > +	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > +				SKL_CDCLK_PREPARE_FOR_CHANGE,
> > +				SKL_CDCLK_READY_FOR_CHANGE,
> > +				SKL_CDCLK_READY_FOR_CHANGE, 3);
> > +	mutex_unlock(&dev_priv->pcu_lock);
> > +	if (ret) {
> > +		DRM_ERROR("Failed to inform PCU about cdclk change
> > (%d)\n",
> > +			  ret);
> > +		return;
> > +	}
> > +
> > +	/* FIXME: We should also consider the DDI clock here. */
> > +	switch (cdclk) {
> > +	case 307200:
> > +	case 312000:
> > +		voltage_level = 0;
> > +		break;
> > +	case 556800:
> > +	case 552000:
> > +		voltage_level = 1;
> > +		break;
> > +	default:
> > +		MISSING_CASE(cdclk);
> > +	case 652800:
> > +	case 648000:
> > +		voltage_level = 2;
> > +		break;
> > +	}
> > +
> > +	if (dev_priv->cdclk.hw.vco != 0 &&
> > +	    dev_priv->cdclk.hw.vco != vco)
> > +		cnl_cdclk_pll_disable(dev_priv);
> > +
> > +	if (dev_priv->cdclk.hw.vco != vco)
> > +		cnl_cdclk_pll_enable(dev_priv, vco);
> > +
> > +	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> > +			      skl_cdclk_decimal(cdclk));
> > +
> > +	mutex_lock(&dev_priv->pcu_lock);
> > +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > +				voltage_level);
> > +	mutex_unlock(&dev_priv->pcu_lock);
> > +
> > +	intel_update_cdclk(dev_priv);
> > +}
> > +
> > +static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> > +			  struct intel_cdclk_state *cdclk_state)
> > +{
> > +	u32 val;
> > +
> > +	val = I915_READ(SKL_DSSM);
> > +	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> > +	default:
> > +		MISSING_CASE(val);
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
> > +		cdclk_state->ref = 24000;
> > +		break;
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
> > +		cdclk_state->ref = 19200;
> > +		break;
> > +	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
> > +		cdclk_state->ref = 38400;
> > +		break;
> > +	}
> > +
> > +	val = I915_READ(BXT_DE_PLL_ENABLE);
> > +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
> > +	    (val & BXT_DE_PLL_LOCK) == 0) {
> > +		/* CDCLK PLL is disabled, the VCO/ratio doesn't
> > matter, but
> > +		 * setting it to zero is a way to signal that. */
> > +		cdclk_state->vco = 0;
> > +		cdclk_state->cdclk = 50000;
> > +		return;
> > +	}
> > +
> > +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) *
> > cdclk_state->ref;
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
> > +
> > +	cdclk_state->cdclk = cdclk_state->vco / 2;
> > +}
> > +
> > +/**
> > + * icl_init_cdclk - Initialize CDCLK on ICL
> > + * @dev_priv: i915 device
> > + *
> > + * Initialize CDCLK for ICL. This consists mainly of initializing
> > + * dev_priv->cdclk.hw and sanitizing the state of the hardware if
> > needed. This
> > + * is generally done only during the display core initialization
> > sequence, after
> > + * which the DMC will take care of turning CDCLK off/on as needed.
> > + */
> > +void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_cdclk_state cdclk_state;
> > +	u32 val;
> > +
> > +	/* This sets dev_priv->cdclk.hw. */
> > +	intel_update_cdclk(dev_priv);
> > +
> > +	cdclk_state = dev_priv->cdclk.hw;
> > +
> > +	/* This means CDCLK disabled. */
> > +	if (cdclk_state.cdclk == 50000)
> > +		goto sanitize;
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +
> > +	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
> > +		goto sanitize;
> > +
> > +	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
> > +	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
> > +		goto sanitize;
> > +
> > +	return;
> > +
> > +sanitize:
> > +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> > +
> > +	cdclk_state.ref = dev_priv->cdclk.hw.ref;
> > +	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
> > +	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> > cdclk_state.cdclk);
> > +
> > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > +}
> > +
> > +/**
> > + * icl_uninit_cdclk - Uninitialize CDCLK on ICL
> > + * @dev_priv: i915 device
> > + *
> > + * Uninitialize CDCLK for ICL. This is done only during the
> > display core
> > + * uninitialization sequence.
> > + */
> > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> > +
> > +	cdclk_state.cdclk = cdclk_state.ref;
> > +	cdclk_state.vco = 0;
> > +
> > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > +}
> > +
> >  /**
> >   * cnl_init_cdclk - Initialize CDCLK on CNL
> >   * @dev_priv: i915 device
> > @@ -2204,6 +2413,36 @@ static int cnl_modeset_calc_cdclk(struct
> > drm_atomic_state *state)
> >  	return 0;
> >  }
> >  
> > +static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > +	struct intel_atomic_state *intel_state =
> > to_intel_atomic_state(state);
> > +	unsigned int ref = intel_state->cdclk.logical.ref;
> > +	int min_cdclk, cdclk, vco;
> > +
> > +	min_cdclk = intel_compute_min_cdclk(state);
> > +	if (min_cdclk < 0)
> > +		return min_cdclk;
> > +
> > +	cdclk = icl_calc_cdclk(min_cdclk, ref);
> > +	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > +
> > +	intel_state->cdclk.logical.vco = vco;
> > +	intel_state->cdclk.logical.cdclk = cdclk;
> > +
> > +	if (!intel_state->active_crtcs) {
> > +		cdclk = icl_calc_cdclk(0, ref);
> > +		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > +
> > +		intel_state->cdclk.actual.vco = vco;
> > +		intel_state->cdclk.actual.cdclk = cdclk;
> > +	} else {
> > +		intel_state->cdclk.actual = intel_state-
> > >cdclk.logical;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static int intel_compute_max_dotclk(struct drm_i915_private
> > *dev_priv)
> >  {
> >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > @@ -2237,7 +2476,12 @@ static int intel_compute_max_dotclk(struct
> > drm_i915_private *dev_priv)
> >   */
> >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> >  {
> > -	if (IS_CANNONLAKE(dev_priv)) {
> > +	if (IS_ICELAKE(dev_priv)) {
> > +		if (dev_priv->cdclk.hw.ref == 24000)
> > +			dev_priv->max_cdclk_freq = 648000;
> > +		else
> > +			dev_priv->max_cdclk_freq = 652800;
> > +	} else if (IS_CANNONLAKE(dev_priv)) {
> >  		dev_priv->max_cdclk_freq = 528000;
> >  	} else if (IS_GEN9_BC(dev_priv)) {
> >  		u32 limit = I915_READ(SKL_DFSM) &
> > SKL_DFSM_CDCLK_LIMIT_MASK;
> > @@ -2461,9 +2705,14 @@ void intel_init_cdclk_hooks(struct
> > drm_i915_private *dev_priv)
> >  		dev_priv->display.set_cdclk = cnl_set_cdclk;
> >  		dev_priv->display.modeset_calc_cdclk =
> >  			cnl_modeset_calc_cdclk;
> > +	} else if (IS_ICELAKE(dev_priv)) {
> > +		dev_priv->display.set_cdclk = icl_set_cdclk;
> > +		dev_priv->display.modeset_calc_cdclk =
> > icl_modeset_calc_cdclk;
> >  	}
> >  
> > -	if (IS_CANNONLAKE(dev_priv))
> > +	if (IS_ICELAKE(dev_priv))
> > +		dev_priv->display.get_cdclk = icl_get_cdclk;
> > +	else if (IS_CANNONLAKE(dev_priv))
> >  		dev_priv->display.get_cdclk = cnl_get_cdclk;
> >  	else if (IS_GEN9_BC(dev_priv))
> >  		dev_priv->display.get_cdclk = skl_get_cdclk;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 3cee54bc0352..c5d6092aca41 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1403,6 +1403,8 @@ void cnl_init_cdclk(struct drm_i915_private
> > *dev_priv);
> >  void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
> >  void bxt_init_cdclk(struct drm_i915_private *dev_priv);
> >  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> > +void icl_init_cdclk(struct drm_i915_private *dev_priv);
> > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
> >  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);
> > -- 
> > 2.14.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Imre Deak Feb. 1, 2018, 8:40 p.m. UTC | #5
On Thu, Feb 01, 2018 at 06:08:29PM -0200, Paulo Zanoni wrote:
> Em Seg, 2018-01-29 às 12:51 +0200, Imre Deak escreveu:
> > On Tue, Jan 23, 2018 at 05:05:20PM -0200, Paulo Zanoni wrote:
> > > This commit adds the basic CDCLK functions, but it's still missing
> > > pieces of the display initialization sequence.
> > > 
> > > v2:
> > >  - Implement the voltage levels.
> > >  - Rebase.
> > > 
> > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h    |  10 +-
> > >  drivers/gpu/drm/i915/intel_cdclk.c | 253
> > > ++++++++++++++++++++++++++++++++++++-
> > >  drivers/gpu/drm/i915/intel_drv.h   |   2 +
> > >  3 files changed, 261 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index abd9ee876186..d72e206b2b9f 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7113,8 +7113,12 @@ enum {
> > >  #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
> > >  #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
> > >  
> > > -#define SKL_DSSM			_MMIO(0x51004)
> > > -#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
> > > +#define SKL_DSSM				_MMIO(0x51004)
> > > +#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
> > > +#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
> > > +#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
> > > +#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
> > > +#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
> > >  
> > >  #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
> > >  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
> > > @@ -8760,6 +8764,8 @@ enum skl_power_gate {
> > >  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> > >  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
> > >  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > > +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> > > +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
> > >  
> > >  /* LCPLL_CTL */
> > >  #define LCPLL1_CTL		_MMIO(0x46010)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index c4392ea34a3d..d867956d5a9f 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1766,6 +1766,215 @@ static void cnl_sanitize_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >  	dev_priv->cdclk.hw.vco = -1;
> > >  }
> > >  
> > > +static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > > +{
> > > +	int ranges_24[] = { 312000, 552000, 648000 };
> > > +	int ranges_19_38[] = { 307200, 556800, 652800 };
> > > +	int *ranges;
> > > +
> > > +	switch (ref) {
> > > +	default:
> > > +		MISSING_CASE(ref);
> > > +	case 24000:
> > > +		ranges = ranges_24;
> > > +		break;
> > > +	case 19200:
> > > +	case 38400:
> > > +		ranges = ranges_19_38;
> > > +		break;
> > > +	}
> > > +
> > > +	if (min_cdclk > ranges[1])
> > > +		return ranges[2];
> > > +	else if (min_cdclk > ranges[0])
> > > +		return ranges[1];
> > > +	else
> > > +		return ranges[0];
> > > +}
> > > +
> > > +static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > > +{
> > > +	int ratio;
> > > +
> > > +	/* 50MHz == CDCLK PLL disabled. */
> > > +	if (cdclk == 50000)
> > 
> > Here and everywhere else cdclk.hw.bypass should be used instead of
> > hard
> > coding it.
> 
> I really don't understand your comment, and I also looked at "drm/i915:
> Add tracking for CDCLK bypass frequency" and don't understand it
> either.
> 
> On our documentation I can't find anything named "bypass clock", and
> there's no explanation in our code nor the commit message. The 50MHz is
> the CDCLK value (not "bypass clock") when the CDCLK PLL is disabled.

cdclk.hw.bypass is the CDCLK frequency when the CDCLK PLL is disabled.
The clock source for this has different names on different platforms, on
ICL it's called croclk or slow-clock by BSpec which is a 100MHz clock.
This is divided by 2, resulting in the 50MHz bypass frequency, as shown
under "Icelake Clocks"/"CD Clock" in BSpec.

> In the driver, I see the "bypass" clock is set to the refclk we read
> from the hardware, and that has nothing to do with the 50MHz we're
> talking about at that point. We can't even read that 50MHz value from
> the hardware. On ICL, the refclk is either 24, 19.2 or 38.4, we can't
> read the 50 anywhere.

Yes, on ICL the bypass frequency is not refclk based on the above, but
the croclk/2 or 50MHz, so that's what cdclk.hw.bypass needs to be set
to.

> I'll really need a better explanation on what exactly you meant with
> this "bypass" clock thing if I need to implement it on ICL. I just
> can't see those "50000" references going away. I can't see what's wrong
> with this code.

We want to handle this part of the code similarly to all other
platforms, that is by using cdclk.hw.bypass instead of a hard-coded
value. Note that on some platform this frequency can be selected that is
you will have to read it out from the HW the same way you read the
refclk value.

icl_uninit_cdclk() is also using refclk incorrectly, see my comment
below.

--Imre

> 
> > 
> > > +		return 0;
> > > +
> > > +	switch (cdclk) {
> > > +	default:
> > > +		MISSING_CASE(cdclk);
> > > +	case 307200:
> > > +	case 556800:
> > > +	case 652800:
> > > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > +			dev_priv->cdclk.hw.ref != 38400);
> > > +		break;
> > > +	case 312000:
> > > +	case 552000:
> > > +	case 648000:
> > > +		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > +	}
> > > +
> > > +	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > +
> > > +	return dev_priv->cdclk.hw.ref * ratio;
> > > +}
> > > +
> > > +static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > > +			  const struct intel_cdclk_state
> > > *cdclk_state)
> > > +{
> > > +	unsigned int cdclk = cdclk_state->cdclk;
> > > +	unsigned int vco = cdclk_state->vco;
> > > +	int ret;
> > > +	u32 voltage_level;
> > > +
> > > +	mutex_lock(&dev_priv->pcu_lock);
> > > +	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > > +				SKL_CDCLK_PREPARE_FOR_CHANGE,
> > > +				SKL_CDCLK_READY_FOR_CHANGE,
> > > +				SKL_CDCLK_READY_FOR_CHANGE, 3);
> > > +	mutex_unlock(&dev_priv->pcu_lock);
> > > +	if (ret) {
> > > +		DRM_ERROR("Failed to inform PCU about cdclk change
> > > (%d)\n",
> > > +			  ret);
> > > +		return;
> > > +	}
> > > +
> > > +	/* FIXME: We should also consider the DDI clock here. */
> > > +	switch (cdclk) {
> > > +	case 307200:
> > > +	case 312000:
> > > +		voltage_level = 0;
> > > +		break;
> > > +	case 556800:
> > > +	case 552000:
> > > +		voltage_level = 1;
> > > +		break;
> > > +	default:
> > > +		MISSING_CASE(cdclk);
> > > +	case 652800:
> > > +	case 648000:
> > > +		voltage_level = 2;
> > > +		break;
> > > +	}
> > > +
> > > +	if (dev_priv->cdclk.hw.vco != 0 &&
> > > +	    dev_priv->cdclk.hw.vco != vco)
> > > +		cnl_cdclk_pll_disable(dev_priv);
> > > +
> > > +	if (dev_priv->cdclk.hw.vco != vco)
> > > +		cnl_cdclk_pll_enable(dev_priv, vco);
> > > +
> > > +	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> > > +			      skl_cdclk_decimal(cdclk));
> > > +
> > > +	mutex_lock(&dev_priv->pcu_lock);
> > > +	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > > +				voltage_level);
> > > +	mutex_unlock(&dev_priv->pcu_lock);
> > > +
> > > +	intel_update_cdclk(dev_priv);
> > > +}
> > > +
> > > +static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> > > +			  struct intel_cdclk_state *cdclk_state)
> > > +{
> > > +	u32 val;
> > > +
> > > +	val = I915_READ(SKL_DSSM);
> > > +	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> > > +	default:
> > > +		MISSING_CASE(val);
> > > +	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
> > > +		cdclk_state->ref = 24000;
> > > +		break;
> > > +	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
> > > +		cdclk_state->ref = 19200;
> > > +		break;
> > > +	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
> > > +		cdclk_state->ref = 38400;
> > > +		break;
> > > +	}
> > > +
> > > +	val = I915_READ(BXT_DE_PLL_ENABLE);
> > > +	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
> > > +	    (val & BXT_DE_PLL_LOCK) == 0) {
> > > +		/* CDCLK PLL is disabled, the VCO/ratio doesn't
> > > matter, but
> > > +		 * setting it to zero is a way to signal that. */
> > > +		cdclk_state->vco = 0;
> > > +		cdclk_state->cdclk = 50000;
> > > +		return;
> > > +	}
> > > +
> > > +	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) *
> > > cdclk_state->ref;
> > > +
> > > +	val = I915_READ(CDCLK_CTL);
> > > +	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
> > > +
> > > +	cdclk_state->cdclk = cdclk_state->vco / 2;
> > > +}
> > > +
> > > +/**
> > > + * icl_init_cdclk - Initialize CDCLK on ICL
> > > + * @dev_priv: i915 device
> > > + *
> > > + * Initialize CDCLK for ICL. This consists mainly of initializing
> > > + * dev_priv->cdclk.hw and sanitizing the state of the hardware if
> > > needed. This
> > > + * is generally done only during the display core initialization
> > > sequence, after
> > > + * which the DMC will take care of turning CDCLK off/on as needed.
> > > + */
> > > +void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > > +{
> > > +	struct intel_cdclk_state cdclk_state;
> > > +	u32 val;
> > > +
> > > +	/* This sets dev_priv->cdclk.hw. */
> > > +	intel_update_cdclk(dev_priv);
> > > +
> > > +	cdclk_state = dev_priv->cdclk.hw;
> > > +
> > > +	/* This means CDCLK disabled. */
> > > +	if (cdclk_state.cdclk == 50000)
> > > +		goto sanitize;
> > > +
> > > +	val = I915_READ(CDCLK_CTL);
> > > +
> > > +	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
> > > +		goto sanitize;
> > > +
> > > +	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
> > > +	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
> > > +		goto sanitize;
> > > +
> > > +	return;
> > > +
> > > +sanitize:
> > > +	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> > > +
> > > +	cdclk_state.ref = dev_priv->cdclk.hw.ref;
> > > +	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
> > > +	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> > > cdclk_state.cdclk);
> > > +
> > > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > > +}
> > > +
> > > +/**
> > > + * icl_uninit_cdclk - Uninitialize CDCLK on ICL
> > > + * @dev_priv: i915 device
> > > + *
> > > + * Uninitialize CDCLK for ICL. This is done only during the
> > > display core
> > > + * uninitialization sequence.
> > > + */
> > > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > > +{
> > > +	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> > > +
> > > +	cdclk_state.cdclk = cdclk_state.ref;
> > 
> > On ICL it's the slow clock (croclock) used for the bypass clock
> > source,
> > so have to set cdclk_state.bypass here.
> > 
> > > +	cdclk_state.vco = 0;
> > > +
> > > +	icl_set_cdclk(dev_priv, &cdclk_state);
> > > +}
> > > +
> > >  /**
> > >   * cnl_init_cdclk - Initialize CDCLK on CNL
> > >   * @dev_priv: i915 device
> > > @@ -2204,6 +2413,36 @@ static int cnl_modeset_calc_cdclk(struct
> > > drm_atomic_state *state)
> > >  	return 0;
> > >  }
> > >  
> > > +static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
> > > +{
> > > +	struct drm_i915_private *dev_priv = to_i915(state->dev);
> > > +	struct intel_atomic_state *intel_state =
> > > to_intel_atomic_state(state);
> > > +	unsigned int ref = intel_state->cdclk.logical.ref;
> > > +	int min_cdclk, cdclk, vco;
> > > +
> > > +	min_cdclk = intel_compute_min_cdclk(state);
> > > +	if (min_cdclk < 0)
> > > +		return min_cdclk;
> > > +
> > > +	cdclk = icl_calc_cdclk(min_cdclk, ref);
> > > +	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > > +
> > > +	intel_state->cdclk.logical.vco = vco;
> > > +	intel_state->cdclk.logical.cdclk = cdclk;
> > > +
> > > +	if (!intel_state->active_crtcs) {
> > > +		cdclk = icl_calc_cdclk(0, ref);
> > > +		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> > > +
> > > +		intel_state->cdclk.actual.vco = vco;
> > > +		intel_state->cdclk.actual.cdclk = cdclk;
> > > +	} else {
> > > +		intel_state->cdclk.actual = intel_state-
> > > >cdclk.logical;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static int intel_compute_max_dotclk(struct drm_i915_private
> > > *dev_priv)
> > >  {
> > >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > > @@ -2237,7 +2476,12 @@ static int intel_compute_max_dotclk(struct
> > > drm_i915_private *dev_priv)
> > >   */
> > >  void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
> > >  {
> > > -	if (IS_CANNONLAKE(dev_priv)) {
> > > +	if (IS_ICELAKE(dev_priv)) {
> > > +		if (dev_priv->cdclk.hw.ref == 24000)
> > > +			dev_priv->max_cdclk_freq = 648000;
> > > +		else
> > > +			dev_priv->max_cdclk_freq = 652800;
> > > +	} else if (IS_CANNONLAKE(dev_priv)) {
> > >  		dev_priv->max_cdclk_freq = 528000;
> > >  	} else if (IS_GEN9_BC(dev_priv)) {
> > >  		u32 limit = I915_READ(SKL_DFSM) &
> > > SKL_DFSM_CDCLK_LIMIT_MASK;
> > > @@ -2461,9 +2705,14 @@ void intel_init_cdclk_hooks(struct
> > > drm_i915_private *dev_priv)
> > >  		dev_priv->display.set_cdclk = cnl_set_cdclk;
> > >  		dev_priv->display.modeset_calc_cdclk =
> > >  			cnl_modeset_calc_cdclk;
> > > +	} else if (IS_ICELAKE(dev_priv)) {
> > > +		dev_priv->display.set_cdclk = icl_set_cdclk;
> > > +		dev_priv->display.modeset_calc_cdclk =
> > > icl_modeset_calc_cdclk;
> > >  	}
> > >  
> > > -	if (IS_CANNONLAKE(dev_priv))
> > > +	if (IS_ICELAKE(dev_priv))
> > > +		dev_priv->display.get_cdclk = icl_get_cdclk;
> > > +	else if (IS_CANNONLAKE(dev_priv))
> > >  		dev_priv->display.get_cdclk = cnl_get_cdclk;
> > >  	else if (IS_GEN9_BC(dev_priv))
> > >  		dev_priv->display.get_cdclk = skl_get_cdclk;
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h
> > > b/drivers/gpu/drm/i915/intel_drv.h
> > > index 3cee54bc0352..c5d6092aca41 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -1403,6 +1403,8 @@ void cnl_init_cdclk(struct drm_i915_private
> > > *dev_priv);
> > >  void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
> > >  void bxt_init_cdclk(struct drm_i915_private *dev_priv);
> > >  void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
> > > +void icl_init_cdclk(struct drm_i915_private *dev_priv);
> > > +void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
> > >  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);
> > > -- 
> > > 2.14.3
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index abd9ee876186..d72e206b2b9f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7113,8 +7113,12 @@  enum {
 #define SKL_DFSM_PIPE_B_DISABLE		(1 << 21)
 #define SKL_DFSM_PIPE_C_DISABLE		(1 << 28)
 
-#define SKL_DSSM			_MMIO(0x51004)
-#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz	(1 << 31)
+#define SKL_DSSM				_MMIO(0x51004)
+#define CNL_DSSM_CDCLK_PLL_REFCLK_24MHz		(1 << 31)
+#define ICL_DSSM_CDCLK_PLL_REFCLK_MASK		(7 << 29)
+#define ICL_DSSM_CDCLK_PLL_REFCLK_24MHz		(0 << 29)
+#define ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz	(1 << 29)
+#define ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz	(2 << 29)
 
 #define GEN7_FF_SLICE_CS_CHICKEN1	_MMIO(0x20e0)
 #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL	(1<<14)
@@ -8760,6 +8764,8 @@  enum skl_power_gate {
 #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
 #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
 #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
+#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
+#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
 
 /* LCPLL_CTL */
 #define LCPLL1_CTL		_MMIO(0x46010)
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index c4392ea34a3d..d867956d5a9f 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1766,6 +1766,215 @@  static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	dev_priv->cdclk.hw.vco = -1;
 }
 
+static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
+{
+	int ranges_24[] = { 312000, 552000, 648000 };
+	int ranges_19_38[] = { 307200, 556800, 652800 };
+	int *ranges;
+
+	switch (ref) {
+	default:
+		MISSING_CASE(ref);
+	case 24000:
+		ranges = ranges_24;
+		break;
+	case 19200:
+	case 38400:
+		ranges = ranges_19_38;
+		break;
+	}
+
+	if (min_cdclk > ranges[1])
+		return ranges[2];
+	else if (min_cdclk > ranges[0])
+		return ranges[1];
+	else
+		return ranges[0];
+}
+
+static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
+{
+	int ratio;
+
+	/* 50MHz == CDCLK PLL disabled. */
+	if (cdclk == 50000)
+		return 0;
+
+	switch (cdclk) {
+	default:
+		MISSING_CASE(cdclk);
+	case 307200:
+	case 556800:
+	case 652800:
+		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
+			dev_priv->cdclk.hw.ref != 38400);
+		break;
+	case 312000:
+	case 552000:
+	case 648000:
+		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
+	}
+
+	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
+
+	return dev_priv->cdclk.hw.ref * ratio;
+}
+
+static void icl_set_cdclk(struct drm_i915_private *dev_priv,
+			  const struct intel_cdclk_state *cdclk_state)
+{
+	unsigned int cdclk = cdclk_state->cdclk;
+	unsigned int vco = cdclk_state->vco;
+	int ret;
+	u32 voltage_level;
+
+	mutex_lock(&dev_priv->pcu_lock);
+	ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+				SKL_CDCLK_PREPARE_FOR_CHANGE,
+				SKL_CDCLK_READY_FOR_CHANGE,
+				SKL_CDCLK_READY_FOR_CHANGE, 3);
+	mutex_unlock(&dev_priv->pcu_lock);
+	if (ret) {
+		DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
+			  ret);
+		return;
+	}
+
+	/* FIXME: We should also consider the DDI clock here. */
+	switch (cdclk) {
+	case 307200:
+	case 312000:
+		voltage_level = 0;
+		break;
+	case 556800:
+	case 552000:
+		voltage_level = 1;
+		break;
+	default:
+		MISSING_CASE(cdclk);
+	case 652800:
+	case 648000:
+		voltage_level = 2;
+		break;
+	}
+
+	if (dev_priv->cdclk.hw.vco != 0 &&
+	    dev_priv->cdclk.hw.vco != vco)
+		cnl_cdclk_pll_disable(dev_priv);
+
+	if (dev_priv->cdclk.hw.vco != vco)
+		cnl_cdclk_pll_enable(dev_priv, vco);
+
+	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
+			      skl_cdclk_decimal(cdclk));
+
+	mutex_lock(&dev_priv->pcu_lock);
+	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+				voltage_level);
+	mutex_unlock(&dev_priv->pcu_lock);
+
+	intel_update_cdclk(dev_priv);
+}
+
+static void icl_get_cdclk(struct drm_i915_private *dev_priv,
+			  struct intel_cdclk_state *cdclk_state)
+{
+	u32 val;
+
+	val = I915_READ(SKL_DSSM);
+	switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
+	default:
+		MISSING_CASE(val);
+	case ICL_DSSM_CDCLK_PLL_REFCLK_24MHz:
+		cdclk_state->ref = 24000;
+		break;
+	case ICL_DSSM_CDCLK_PLL_REFCLK_19_2MHz:
+		cdclk_state->ref = 19200;
+		break;
+	case ICL_DSSM_CDCLK_PLL_REFCLK_38_4MHz:
+		cdclk_state->ref = 38400;
+		break;
+	}
+
+	val = I915_READ(BXT_DE_PLL_ENABLE);
+	if ((val & BXT_DE_PLL_PLL_ENABLE) == 0 ||
+	    (val & BXT_DE_PLL_LOCK) == 0) {
+		/* CDCLK PLL is disabled, the VCO/ratio doesn't matter, but
+		 * setting it to zero is a way to signal that. */
+		cdclk_state->vco = 0;
+		cdclk_state->cdclk = 50000;
+		return;
+	}
+
+	cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
+
+	val = I915_READ(CDCLK_CTL);
+	WARN_ON((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0);
+
+	cdclk_state->cdclk = cdclk_state->vco / 2;
+}
+
+/**
+ * icl_init_cdclk - Initialize CDCLK on ICL
+ * @dev_priv: i915 device
+ *
+ * Initialize CDCLK for ICL. This consists mainly of initializing
+ * dev_priv->cdclk.hw and sanitizing the state of the hardware if needed. This
+ * is generally done only during the display core initialization sequence, after
+ * which the DMC will take care of turning CDCLK off/on as needed.
+ */
+void icl_init_cdclk(struct drm_i915_private *dev_priv)
+{
+	struct intel_cdclk_state cdclk_state;
+	u32 val;
+
+	/* This sets dev_priv->cdclk.hw. */
+	intel_update_cdclk(dev_priv);
+
+	cdclk_state = dev_priv->cdclk.hw;
+
+	/* This means CDCLK disabled. */
+	if (cdclk_state.cdclk == 50000)
+		goto sanitize;
+
+	val = I915_READ(CDCLK_CTL);
+
+	if ((val & BXT_CDCLK_CD2X_DIV_SEL_MASK) != 0)
+		goto sanitize;
+
+	if ((val & CDCLK_FREQ_DECIMAL_MASK) !=
+	    skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk))
+		goto sanitize;
+
+	return;
+
+sanitize:
+	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
+
+	cdclk_state.ref = dev_priv->cdclk.hw.ref;
+	cdclk_state.cdclk = icl_calc_cdclk(0, cdclk_state.ref);
+	cdclk_state.vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
+
+	icl_set_cdclk(dev_priv, &cdclk_state);
+}
+
+/**
+ * icl_uninit_cdclk - Uninitialize CDCLK on ICL
+ * @dev_priv: i915 device
+ *
+ * Uninitialize CDCLK for ICL. This is done only during the display core
+ * uninitialization sequence.
+ */
+void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
+{
+	struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
+
+	cdclk_state.cdclk = cdclk_state.ref;
+	cdclk_state.vco = 0;
+
+	icl_set_cdclk(dev_priv, &cdclk_state);
+}
+
 /**
  * cnl_init_cdclk - Initialize CDCLK on CNL
  * @dev_priv: i915 device
@@ -2204,6 +2413,36 @@  static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	return 0;
 }
 
+static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->dev);
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	unsigned int ref = intel_state->cdclk.logical.ref;
+	int min_cdclk, cdclk, vco;
+
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
+
+	cdclk = icl_calc_cdclk(min_cdclk, ref);
+	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
+
+	intel_state->cdclk.logical.vco = vco;
+	intel_state->cdclk.logical.cdclk = cdclk;
+
+	if (!intel_state->active_crtcs) {
+		cdclk = icl_calc_cdclk(0, ref);
+		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
+
+		intel_state->cdclk.actual.vco = vco;
+		intel_state->cdclk.actual.cdclk = cdclk;
+	} else {
+		intel_state->cdclk.actual = intel_state->cdclk.logical;
+	}
+
+	return 0;
+}
+
 static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 {
 	int max_cdclk_freq = dev_priv->max_cdclk_freq;
@@ -2237,7 +2476,12 @@  static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
  */
 void intel_update_max_cdclk(struct drm_i915_private *dev_priv)
 {
-	if (IS_CANNONLAKE(dev_priv)) {
+	if (IS_ICELAKE(dev_priv)) {
+		if (dev_priv->cdclk.hw.ref == 24000)
+			dev_priv->max_cdclk_freq = 648000;
+		else
+			dev_priv->max_cdclk_freq = 652800;
+	} else if (IS_CANNONLAKE(dev_priv)) {
 		dev_priv->max_cdclk_freq = 528000;
 	} else if (IS_GEN9_BC(dev_priv)) {
 		u32 limit = I915_READ(SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
@@ -2461,9 +2705,14 @@  void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.set_cdclk = cnl_set_cdclk;
 		dev_priv->display.modeset_calc_cdclk =
 			cnl_modeset_calc_cdclk;
+	} else if (IS_ICELAKE(dev_priv)) {
+		dev_priv->display.set_cdclk = icl_set_cdclk;
+		dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
 	}
 
-	if (IS_CANNONLAKE(dev_priv))
+	if (IS_ICELAKE(dev_priv))
+		dev_priv->display.get_cdclk = icl_get_cdclk;
+	else if (IS_CANNONLAKE(dev_priv))
 		dev_priv->display.get_cdclk = cnl_get_cdclk;
 	else if (IS_GEN9_BC(dev_priv))
 		dev_priv->display.get_cdclk = skl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3cee54bc0352..c5d6092aca41 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1403,6 +1403,8 @@  void cnl_init_cdclk(struct drm_i915_private *dev_priv);
 void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
 void bxt_init_cdclk(struct drm_i915_private *dev_priv);
 void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
+void icl_init_cdclk(struct drm_i915_private *dev_priv);
+void icl_uninit_cdclk(struct drm_i915_private *dev_priv);
 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);