diff mbox

[1/4] drm/i915: kill {bdw, bxt}_modeset_calc_cdclk

Message ID 1487337727-9813-2-git-send-email-paulo.r.zanoni@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Zanoni, Paulo R Feb. 17, 2017, 1:22 p.m. UTC
The functions are pretty much the same, except for the CDCLK and VCO
calculations. Add BDW support to vlv_modeset_calc_cdclk() and add
BXT/GLK support to skl_modeset_calc_cdclk(). The two reamining
functions are still very similar, except for the fact that the vlv
version doesn't touch the VCO. Further patches could unify them even
more if that's desired.

While at it, merge some lines that can fit 80 columns in those
functions.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 120 ++++++++++---------------------------
 1 file changed, 30 insertions(+), 90 deletions(-)

Comments

Ville Syrjala Feb. 17, 2017, 1:51 p.m. UTC | #1
On Fri, Feb 17, 2017 at 11:22:04AM -0200, Paulo Zanoni wrote:
> The functions are pretty much the same, except for the CDCLK and VCO
> calculations. Add BDW support to vlv_modeset_calc_cdclk() and add
> BXT/GLK support to skl_modeset_calc_cdclk(). The two reamining
> functions are still very similar, except for the fact that the vlv
> version doesn't touch the VCO. Further patches could unify them even
> more if that's desired.
> 
> While at it, merge some lines that can fit 80 columns in those
> functions.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 120 ++++++++++---------------------------
>  1 file changed, 30 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index d643c0c..d505ff1 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1499,45 +1499,18 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
>  static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk;
> -
> -	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = vlv_calc_cdclk(dev_priv, 0);
> -
> -		intel_state->cdclk.actual.cdclk = cdclk;
> -	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> -	}
> -
> -	return 0;
> -}
> -
> -static int bdw_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);
>  	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk;
>  
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Broadwell should also account for plane ratio once 64bpp pixel
> +	 * formats are supported.

BTW these restrictions affect pretty much all platforms, so specifying the
platforms in the comment is rather redundant.

>  	 */
> -	cdclk = bdw_calc_cdclk(max_pixclk);
> +	if (IS_BROADWELL(dev_priv))
> +		cdclk = bdw_calc_cdclk(max_pixclk);
> +	else
> +		cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
>  		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -1548,12 +1521,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	intel_state->cdclk.logical.cdclk = cdclk;
>  
>  	if (!intel_state->active_crtcs) {
> -		cdclk = bdw_calc_cdclk(0);
> +		if (IS_BROADWELL(dev_priv))
> +			cdclk = bdw_calc_cdclk(0);
> +		else
> +			cdclk = vlv_calc_cdclk(dev_priv, 0);
>  
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1561,57 +1536,26 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
> -	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	const int max_pixclk = intel_max_pixel_rate(state);
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk, vco;
>  
> -	vco = intel_state->cdclk.logical.vco;
> -	if (!vco)
> -		vco = dev_priv->skl_preferred_vco_freq;
> -
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Skylake/Kabylake should also account for plane ratio once
> +	 * 64bpp pixel formats are supported.
>  	 */
> -	cdclk = skl_calc_cdclk(max_pixclk, vco);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.vco = vco;
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = skl_calc_cdclk(0, vco);
> -
> -		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 bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk, vco;
> -
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		cdclk = glk_calc_cdclk(max_pixclk);
>  		vco = glk_de_pll_vco(dev_priv, cdclk);
> -	} else {
> +	} else if (IS_BROXTON(dev_priv)) {
>  		cdclk = bxt_calc_cdclk(max_pixclk);
>  		vco = bxt_de_pll_vco(dev_priv, cdclk);
> +	} else {
> +		vco = intel_state->cdclk.logical.vco;
> +		if (!vco)
> +			vco = dev_priv->skl_preferred_vco_freq;
> +		cdclk = skl_calc_cdclk(max_pixclk, vco);
>  	}
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
> @@ -1627,16 +1571,17 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
>  		if (IS_GEMINILAKE(dev_priv)) {
>  			cdclk = glk_calc_cdclk(0);
>  			vco = glk_de_pll_vco(dev_priv, cdclk);
> -		} else {
> +		} else if (IS_BROXTON(dev_priv)) {
>  			cdclk = bxt_calc_cdclk(0);
>  			vco = bxt_de_pll_vco(dev_priv, cdclk);
> +		} else {
> +			cdclk = skl_calc_cdclk(0, vco);
>  		}
>  
>  		intel_state->cdclk.actual.vco = vco;
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1823,24 +1768,19 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = chv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = vlv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		dev_priv->display.set_cdclk = bdw_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bdw_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_GEN9_LP(dev_priv)) {
>  		dev_priv->display.set_cdclk = bxt_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bxt_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		dev_priv->display.set_cdclk = skl_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			skl_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	}
>  
>  	if (IS_GEN9_BC(dev_priv))
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
David Weinehall Feb. 18, 2017, 3:13 p.m. UTC | #2
On Fri, Feb 17, 2017 at 11:22:04AM -0200, Paulo Zanoni wrote:
> The functions are pretty much the same, except for the CDCLK and VCO
> calculations. Add BDW support to vlv_modeset_calc_cdclk() and add
> BXT/GLK support to skl_modeset_calc_cdclk(). The two reamining

s/reamining/remaining/

> functions are still very similar, except for the fact that the vlv
> version doesn't touch the VCO. Further patches could unify them even
> more if that's desired.
> 
> While at it, merge some lines that can fit 80 columns in those
> functions.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 120 ++++++++++---------------------------
>  1 file changed, 30 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index d643c0c..d505ff1 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1499,45 +1499,18 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
>  static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk;
> -
> -	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = vlv_calc_cdclk(dev_priv, 0);
> -
> -		intel_state->cdclk.actual.cdclk = cdclk;
> -	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> -	}
> -
> -	return 0;
> -}
> -
> -static int bdw_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);
>  	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk;
>  
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Broadwell should also account for plane ratio once 64bpp pixel
> +	 * formats are supported.
>  	 */
> -	cdclk = bdw_calc_cdclk(max_pixclk);
> +	if (IS_BROADWELL(dev_priv))
> +		cdclk = bdw_calc_cdclk(max_pixclk);
> +	else
> +		cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
>  		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> @@ -1548,12 +1521,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  	intel_state->cdclk.logical.cdclk = cdclk;
>  
>  	if (!intel_state->active_crtcs) {
> -		cdclk = bdw_calc_cdclk(0);
> +		if (IS_BROADWELL(dev_priv))
> +			cdclk = bdw_calc_cdclk(0);
> +		else
> +			cdclk = vlv_calc_cdclk(dev_priv, 0);
>  
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1561,57 +1536,26 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>  
>  static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>  {
> -	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	const int max_pixclk = intel_max_pixel_rate(state);
> +	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
> +	int max_pixclk = intel_max_pixel_rate(state);
>  	int cdclk, vco;
>  
> -	vco = intel_state->cdclk.logical.vco;
> -	if (!vco)
> -		vco = dev_priv->skl_preferred_vco_freq;
> -
>  	/*
> -	 * FIXME should also account for plane ratio
> -	 * once 64bpp pixel formats are supported.
> +	 * FIXME: Skylake/Kabylake should also account for plane ratio once
> +	 * 64bpp pixel formats are supported.
>  	 */
> -	cdclk = skl_calc_cdclk(max_pixclk, vco);
> -
> -	if (cdclk > dev_priv->max_cdclk_freq) {
> -		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
> -			      cdclk, dev_priv->max_cdclk_freq);
> -		return -EINVAL;
> -	}
> -
> -	intel_state->cdclk.logical.vco = vco;
> -	intel_state->cdclk.logical.cdclk = cdclk;
> -
> -	if (!intel_state->active_crtcs) {
> -		cdclk = skl_calc_cdclk(0, vco);
> -
> -		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 bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(state->dev);
> -	int max_pixclk = intel_max_pixel_rate(state);
> -	struct intel_atomic_state *intel_state =
> -		to_intel_atomic_state(state);
> -	int cdclk, vco;
> -
>  	if (IS_GEMINILAKE(dev_priv)) {
>  		cdclk = glk_calc_cdclk(max_pixclk);
>  		vco = glk_de_pll_vco(dev_priv, cdclk);
> -	} else {
> +	} else if (IS_BROXTON(dev_priv)) {
>  		cdclk = bxt_calc_cdclk(max_pixclk);
>  		vco = bxt_de_pll_vco(dev_priv, cdclk);
> +	} else {
> +		vco = intel_state->cdclk.logical.vco;
> +		if (!vco)
> +			vco = dev_priv->skl_preferred_vco_freq;
> +		cdclk = skl_calc_cdclk(max_pixclk, vco);
>  	}
>  
>  	if (cdclk > dev_priv->max_cdclk_freq) {
> @@ -1627,16 +1571,17 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
>  		if (IS_GEMINILAKE(dev_priv)) {
>  			cdclk = glk_calc_cdclk(0);
>  			vco = glk_de_pll_vco(dev_priv, cdclk);
> -		} else {
> +		} else if (IS_BROXTON(dev_priv)) {
>  			cdclk = bxt_calc_cdclk(0);
>  			vco = bxt_de_pll_vco(dev_priv, cdclk);
> +		} else {
> +			cdclk = skl_calc_cdclk(0, vco);
>  		}
>  
>  		intel_state->cdclk.actual.vco = vco;
>  		intel_state->cdclk.actual.cdclk = cdclk;
>  	} else {
> -		intel_state->cdclk.actual =
> -			intel_state->cdclk.logical;
> +		intel_state->cdclk.actual = intel_state->cdclk.logical;
>  	}
>  
>  	return 0;
> @@ -1823,24 +1768,19 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = chv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_VALLEYVIEW(dev_priv)) {
>  		dev_priv->display.set_cdclk = vlv_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			vlv_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_BROADWELL(dev_priv)) {
>  		dev_priv->display.set_cdclk = bdw_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bdw_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
>  	} else if (IS_GEN9_LP(dev_priv)) {
>  		dev_priv->display.set_cdclk = bxt_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			bxt_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	} else if (IS_GEN9_BC(dev_priv)) {
>  		dev_priv->display.set_cdclk = skl_set_cdclk;
> -		dev_priv->display.modeset_calc_cdclk =
> -			skl_modeset_calc_cdclk;
> +		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
>  	}
>  
>  	if (IS_GEN9_BC(dev_priv))
> -- 
> 2.7.4
> 
> _______________________________________________
> 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/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index d643c0c..d505ff1 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1499,45 +1499,18 @@  static int intel_max_pixel_rate(struct drm_atomic_state *state)
 static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk;
-
-	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
-
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		cdclk = vlv_calc_cdclk(dev_priv, 0);
-
-		intel_state->cdclk.actual.cdclk = cdclk;
-	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
-	}
-
-	return 0;
-}
-
-static int bdw_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);
 	int max_pixclk = intel_max_pixel_rate(state);
 	int cdclk;
 
 	/*
-	 * FIXME should also account for plane ratio
-	 * once 64bpp pixel formats are supported.
+	 * FIXME: Broadwell should also account for plane ratio once 64bpp pixel
+	 * formats are supported.
 	 */
-	cdclk = bdw_calc_cdclk(max_pixclk);
+	if (IS_BROADWELL(dev_priv))
+		cdclk = bdw_calc_cdclk(max_pixclk);
+	else
+		cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1548,12 +1521,14 @@  static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 	intel_state->cdclk.logical.cdclk = cdclk;
 
 	if (!intel_state->active_crtcs) {
-		cdclk = bdw_calc_cdclk(0);
+		if (IS_BROADWELL(dev_priv))
+			cdclk = bdw_calc_cdclk(0);
+		else
+			cdclk = vlv_calc_cdclk(dev_priv, 0);
 
 		intel_state->cdclk.actual.cdclk = cdclk;
 	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
+		intel_state->cdclk.actual = intel_state->cdclk.logical;
 	}
 
 	return 0;
@@ -1561,57 +1536,26 @@  static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
-	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	const int max_pixclk = intel_max_pixel_rate(state);
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	int max_pixclk = intel_max_pixel_rate(state);
 	int cdclk, vco;
 
-	vco = intel_state->cdclk.logical.vco;
-	if (!vco)
-		vco = dev_priv->skl_preferred_vco_freq;
-
 	/*
-	 * FIXME should also account for plane ratio
-	 * once 64bpp pixel formats are supported.
+	 * FIXME: Skylake/Kabylake should also account for plane ratio once
+	 * 64bpp pixel formats are supported.
 	 */
-	cdclk = skl_calc_cdclk(max_pixclk, vco);
-
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
-	intel_state->cdclk.logical.vco = vco;
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		cdclk = skl_calc_cdclk(0, vco);
-
-		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 bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk, vco;
-
 	if (IS_GEMINILAKE(dev_priv)) {
 		cdclk = glk_calc_cdclk(max_pixclk);
 		vco = glk_de_pll_vco(dev_priv, cdclk);
-	} else {
+	} else if (IS_BROXTON(dev_priv)) {
 		cdclk = bxt_calc_cdclk(max_pixclk);
 		vco = bxt_de_pll_vco(dev_priv, cdclk);
+	} else {
+		vco = intel_state->cdclk.logical.vco;
+		if (!vco)
+			vco = dev_priv->skl_preferred_vco_freq;
+		cdclk = skl_calc_cdclk(max_pixclk, vco);
 	}
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
@@ -1627,16 +1571,17 @@  static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 		if (IS_GEMINILAKE(dev_priv)) {
 			cdclk = glk_calc_cdclk(0);
 			vco = glk_de_pll_vco(dev_priv, cdclk);
-		} else {
+		} else if (IS_BROXTON(dev_priv)) {
 			cdclk = bxt_calc_cdclk(0);
 			vco = bxt_de_pll_vco(dev_priv, cdclk);
+		} else {
+			cdclk = skl_calc_cdclk(0, vco);
 		}
 
 		intel_state->cdclk.actual.vco = vco;
 		intel_state->cdclk.actual.cdclk = cdclk;
 	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
+		intel_state->cdclk.actual = intel_state->cdclk.logical;
 	}
 
 	return 0;
@@ -1823,24 +1768,19 @@  void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
 	if (IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.set_cdclk = chv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
+		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		dev_priv->display.set_cdclk = vlv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
+		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
 	} else if (IS_BROADWELL(dev_priv)) {
 		dev_priv->display.set_cdclk = bdw_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bdw_modeset_calc_cdclk;
+		dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
 	} else if (IS_GEN9_LP(dev_priv)) {
 		dev_priv->display.set_cdclk = bxt_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bxt_modeset_calc_cdclk;
+		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
 	} else if (IS_GEN9_BC(dev_priv)) {
 		dev_priv->display.set_cdclk = skl_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			skl_modeset_calc_cdclk;
+		dev_priv->display.modeset_calc_cdclk = skl_modeset_calc_cdclk;
 	}
 
 	if (IS_GEN9_BC(dev_priv))