* [PATCH 1/8] drm/i915: Consolidate bxt/cnl/icl cdclk readout
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:27 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 2/8] drm/i915: Use literal representation of cdclk tables Matt Roper
` (12 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
Aside from a few minor register changes and some different clock values,
cdclk design hasn't changed much since gen9lp. Let's consolidate the
handlers for bxt, cnl, and icl to keep the codeflow consistent.
Also, while we're at it, s/bxt_de_pll_update/bxt_de_pll_readout/ since
"update" makes me think we should be writing to hardware rather than
reading from it.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 325 +++++++++------------
1 file changed, 138 insertions(+), 187 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index d3e56628af70..e07de3b84cec 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1190,6 +1190,36 @@ static u8 bxt_calc_voltage_level(int cdclk)
return DIV_ROUND_UP(cdclk, 25000);
}
+static u8 cnl_calc_voltage_level(int cdclk)
+{
+ if (cdclk > 336000)
+ return 2;
+ else if (cdclk > 168000)
+ return 1;
+ else
+ return 0;
+}
+
+static u8 icl_calc_voltage_level(int cdclk)
+{
+ if (cdclk > 556800)
+ return 2;
+ else if (cdclk > 326400)
+ return 1;
+ else
+ return 0;
+}
+
+static u8 ehl_calc_voltage_level(int cdclk)
+{
+ if (cdclk > 326400)
+ return 2;
+ else if (cdclk > 180000)
+ return 1;
+ else
+ return 0;
+}
+
static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
int ratio;
@@ -1236,23 +1266,69 @@ static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
return dev_priv->cdclk.hw.ref * ratio;
}
-static void bxt_de_pll_update(struct drm_i915_private *dev_priv,
- struct intel_cdclk_state *cdclk_state)
+static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
+ struct intel_cdclk_state *cdclk_state)
{
- u32 val;
+ if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
+ cdclk_state->ref = 24000;
+ else
+ cdclk_state->ref = 19200;
+}
- cdclk_state->ref = 19200;
- cdclk_state->vco = 0;
+static void icl_readout_refclk(struct drm_i915_private *dev_priv,
+ struct intel_cdclk_state *cdclk_state)
+{
+ u32 dssm = I915_READ(SKL_DSSM) & ICL_DSSM_CDCLK_PLL_REFCLK_MASK;
+
+ switch (dssm) {
+ default:
+ MISSING_CASE(dssm);
+ /* fall through */
+ 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;
+ }
+}
+
+static void bxt_de_pll_readout(struct drm_i915_private *dev_priv,
+ struct intel_cdclk_state *cdclk_state)
+{
+ u32 val, ratio;
+
+ if (INTEL_GEN(dev_priv) >= 11)
+ icl_readout_refclk(dev_priv, cdclk_state);
+ else if (IS_CANNONLAKE(dev_priv))
+ cnl_readout_refclk(dev_priv, cdclk_state);
+ else
+ cdclk_state->ref = 19200;
val = I915_READ(BXT_DE_PLL_ENABLE);
- if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
+ 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;
return;
+ }
- if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
- return;
+ /*
+ * CNL+ have the ratio directly in the PLL enable register, gen9lp had
+ * it in a separate PLL control register.
+ */
+ if (INTEL_GEN(dev_priv) >= 10)
+ ratio = val & BXT_DE_PLL_RATIO_MASK;
+ else
+ ratio = I915_READ(BXT_DE_PLL_CTL) & BXT_DE_PLL_RATIO_MASK;
- val = I915_READ(BXT_DE_PLL_CTL);
- cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
+ cdclk_state->vco = ratio * cdclk_state->ref;
}
static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
@@ -1261,12 +1337,18 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
u32 divider;
int div;
- bxt_de_pll_update(dev_priv, cdclk_state);
-
- cdclk_state->cdclk = cdclk_state->bypass = cdclk_state->ref;
+ if (INTEL_GEN(dev_priv) >= 12)
+ cdclk_state->bypass = cdclk_state->ref / 2;
+ else if (INTEL_GEN(dev_priv) >= 11)
+ cdclk_state->bypass = 50000;
+ else
+ cdclk_state->bypass = cdclk_state->ref;
- if (cdclk_state->vco == 0)
+ bxt_de_pll_readout(dev_priv, cdclk_state);
+ if (cdclk_state->vco == 0) {
+ cdclk_state->cdclk = cdclk_state->bypass;
goto out;
+ }
divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
@@ -1275,13 +1357,15 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
div = 2;
break;
case BXT_CDCLK_CD2X_DIV_SEL_1_5:
- WARN(IS_GEMINILAKE(dev_priv), "Unsupported divider\n");
+ WARN(IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10,
+ "Unsupported divider\n");
div = 3;
break;
case BXT_CDCLK_CD2X_DIV_SEL_2:
div = 4;
break;
case BXT_CDCLK_CD2X_DIV_SEL_4:
+ WARN(INTEL_GEN(dev_priv) >= 10, "Unsupported divider\n");
div = 8;
break;
default:
@@ -1296,8 +1380,18 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
* Can't read this out :( Let's assume it's
* at least what the CDCLK frequency requires.
*/
- cdclk_state->voltage_level =
- bxt_calc_voltage_level(cdclk_state->cdclk);
+ if (IS_ELKHARTLAKE(dev_priv))
+ cdclk_state->voltage_level =
+ ehl_calc_voltage_level(cdclk_state->cdclk);
+ else if (INTEL_GEN(dev_priv) >= 11)
+ cdclk_state->voltage_level =
+ icl_calc_voltage_level(cdclk_state->cdclk);
+ else if (INTEL_GEN(dev_priv) >= 10)
+ cdclk_state->voltage_level =
+ cnl_calc_voltage_level(cdclk_state->cdclk);
+ else
+ cdclk_state->voltage_level =
+ bxt_calc_voltage_level(cdclk_state->cdclk);
}
static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
@@ -1515,76 +1609,6 @@ static int cnl_calc_cdclk(int min_cdclk)
return 168000;
}
-static u8 cnl_calc_voltage_level(int cdclk)
-{
- if (cdclk > 336000)
- return 2;
- else if (cdclk > 168000)
- return 1;
- else
- return 0;
-}
-
-static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv,
- struct intel_cdclk_state *cdclk_state)
-{
- u32 val;
-
- if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
- cdclk_state->ref = 24000;
- else
- cdclk_state->ref = 19200;
-
- cdclk_state->vco = 0;
-
- val = I915_READ(BXT_DE_PLL_ENABLE);
- if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
- return;
-
- if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
- return;
-
- cdclk_state->vco = (val & CNL_CDCLK_PLL_RATIO_MASK) * cdclk_state->ref;
-}
-
-static void cnl_get_cdclk(struct drm_i915_private *dev_priv,
- struct intel_cdclk_state *cdclk_state)
-{
- u32 divider;
- int div;
-
- cnl_cdclk_pll_update(dev_priv, cdclk_state);
-
- cdclk_state->cdclk = cdclk_state->bypass = cdclk_state->ref;
-
- if (cdclk_state->vco == 0)
- goto out;
-
- divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
-
- switch (divider) {
- case BXT_CDCLK_CD2X_DIV_SEL_1:
- div = 2;
- break;
- case BXT_CDCLK_CD2X_DIV_SEL_2:
- div = 4;
- break;
- default:
- MISSING_CASE(divider);
- return;
- }
-
- cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
-
- out:
- /*
- * Can't read this out :( Let's assume it's
- * at least what the CDCLK frequency requires.
- */
- cdclk_state->voltage_level =
- cnl_calc_voltage_level(cdclk_state->cdclk);
-}
-
static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
{
u32 val;
@@ -1830,91 +1854,6 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
return dev_priv->cdclk.hw.ref * ratio;
}
-static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int cdclk)
-{
- if (IS_ELKHARTLAKE(dev_priv)) {
- if (cdclk > 312000)
- return 2;
- else if (cdclk > 180000)
- return 1;
- else
- return 0;
- } else {
- if (cdclk > 556800)
- return 2;
- else if (cdclk > 312000)
- return 1;
- else
- return 0;
- }
-}
-
-static void icl_get_cdclk(struct drm_i915_private *dev_priv,
- struct intel_cdclk_state *cdclk_state)
-{
- u32 val;
- int div;
-
- val = I915_READ(SKL_DSSM);
- switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
- default:
- MISSING_CASE(val);
- /* fall through */
- 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;
- }
-
- if (INTEL_GEN(dev_priv) >= 12)
- cdclk_state->bypass = cdclk_state->ref / 2;
- else
- cdclk_state->bypass = 50000;
-
- 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 = cdclk_state->bypass;
- goto out;
- }
-
- cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
-
- val = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
- switch (val) {
- case BXT_CDCLK_CD2X_DIV_SEL_1:
- div = 2;
- break;
- case BXT_CDCLK_CD2X_DIV_SEL_2:
- div = 4;
- break;
- default:
- MISSING_CASE(val);
- div = 2;
- break;
- }
-
- cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
-
-out:
- /*
- * Can't read this out :( Let's assume it's
- * at least what the CDCLK frequency requires.
- */
- cdclk_state->voltage_level =
- icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
-}
-
static void icl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state sanitized_state;
@@ -1946,9 +1885,12 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
sanitized_state.cdclk);
- sanitized_state.voltage_level =
- icl_calc_voltage_level(dev_priv,
- sanitized_state.cdclk);
+ if (IS_ELKHARTLAKE(dev_priv))
+ sanitized_state.voltage_level =
+ ehl_calc_voltage_level(sanitized_state.cdclk);
+ else
+ sanitized_state.voltage_level =
+ icl_calc_voltage_level(sanitized_state.cdclk);
cnl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
@@ -1959,8 +1901,12 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = cdclk_state.bypass;
cdclk_state.vco = 0;
- cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
- cdclk_state.cdclk);
+ if (IS_ELKHARTLAKE(dev_priv))
+ cdclk_state.voltage_level =
+ ehl_calc_voltage_level(cdclk_state.cdclk);
+ else
+ cdclk_state.voltage_level =
+ icl_calc_voltage_level(cdclk_state.cdclk);
cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
@@ -2561,9 +2507,14 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
- state->cdclk.logical.voltage_level =
- max(icl_calc_voltage_level(dev_priv, cdclk),
- cnl_compute_min_voltage_level(state));
+ if (IS_ELKHARTLAKE(dev_priv))
+ state->cdclk.logical.voltage_level =
+ max(ehl_calc_voltage_level(cdclk),
+ cnl_compute_min_voltage_level(state));
+ else
+ state->cdclk.logical.voltage_level =
+ max(icl_calc_voltage_level(cdclk),
+ cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
@@ -2571,8 +2522,12 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
- state->cdclk.actual.voltage_level =
- icl_calc_voltage_level(dev_priv, cdclk);
+ if (IS_ELKHARTLAKE(dev_priv))
+ state->cdclk.actual.voltage_level =
+ ehl_calc_voltage_level(cdclk);
+ else
+ state->cdclk.actual.voltage_level =
+ icl_calc_voltage_level(cdclk);
} else {
state->cdclk.actual = state->cdclk.logical;
}
@@ -2819,11 +2774,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
}
- if (INTEL_GEN(dev_priv) >= 11)
- 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_LP(dev_priv))
+ else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
dev_priv->display.get_cdclk = bxt_get_cdclk;
else if (IS_GEN9_BC(dev_priv))
dev_priv->display.get_cdclk = skl_get_cdclk;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 1/8] drm/i915: Consolidate bxt/cnl/icl cdclk readout
2019-09-07 0:21 ` [PATCH 1/8] drm/i915: Consolidate bxt/cnl/icl cdclk readout Matt Roper
@ 2019-09-10 12:27 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:27 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:36PM -0700, Matt Roper wrote:
> Aside from a few minor register changes and some different clock values,
> cdclk design hasn't changed much since gen9lp. Let's consolidate the
> handlers for bxt, cnl, and icl to keep the codeflow consistent.
>
> Also, while we're at it, s/bxt_de_pll_update/bxt_de_pll_readout/ since
> "update" makes me think we should be writing to hardware rather than
> reading from it.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 325 +++++++++------------
> 1 file changed, 138 insertions(+), 187 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index d3e56628af70..e07de3b84cec 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1190,6 +1190,36 @@ static u8 bxt_calc_voltage_level(int cdclk)
> return DIV_ROUND_UP(cdclk, 25000);
> }
>
> +static u8 cnl_calc_voltage_level(int cdclk)
> +{
> + if (cdclk > 336000)
> + return 2;
> + else if (cdclk > 168000)
> + return 1;
> + else
> + return 0;
> +}
> +
> +static u8 icl_calc_voltage_level(int cdclk)
> +{
> + if (cdclk > 556800)
> + return 2;
> + else if (cdclk > 326400)
Not the same numbers as the old function.
> + return 1;
> + else
> + return 0;
> +}
> +
> +static u8 ehl_calc_voltage_level(int cdclk)
> +{
> + if (cdclk > 326400)
> + return 2;
> + else if (cdclk > 180000)
> + return 1;
> + else
> + return 0;
> +}
> +
> static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> {
> int ratio;
> @@ -1236,23 +1266,69 @@ static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> return dev_priv->cdclk.hw.ref * ratio;
> }
>
> -static void bxt_de_pll_update(struct drm_i915_private *dev_priv,
> - struct intel_cdclk_state *cdclk_state)
> +static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
> + struct intel_cdclk_state *cdclk_state)
> {
> - u32 val;
> + if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
> + cdclk_state->ref = 24000;
> + else
> + cdclk_state->ref = 19200;
> +}
>
> - cdclk_state->ref = 19200;
> - cdclk_state->vco = 0;
> +static void icl_readout_refclk(struct drm_i915_private *dev_priv,
> + struct intel_cdclk_state *cdclk_state)
> +{
> + u32 dssm = I915_READ(SKL_DSSM) & ICL_DSSM_CDCLK_PLL_REFCLK_MASK;
> +
> + switch (dssm) {
> + default:
> + MISSING_CASE(dssm);
> + /* fall through */
> + 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;
> + }
> +}
> +
> +static void bxt_de_pll_readout(struct drm_i915_private *dev_priv,
> + struct intel_cdclk_state *cdclk_state)
> +{
> + u32 val, ratio;
> +
> + if (INTEL_GEN(dev_priv) >= 11)
> + icl_readout_refclk(dev_priv, cdclk_state);
> + else if (IS_CANNONLAKE(dev_priv))
> + cnl_readout_refclk(dev_priv, cdclk_state);
> + else
> + cdclk_state->ref = 19200;
>
> val = I915_READ(BXT_DE_PLL_ENABLE);
> - if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
> + 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;
> return;
> + }
>
> - if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
> - return;
> + /*
> + * CNL+ have the ratio directly in the PLL enable register, gen9lp had
> + * it in a separate PLL control register.
> + */
> + if (INTEL_GEN(dev_priv) >= 10)
> + ratio = val & BXT_DE_PLL_RATIO_MASK;a
CNL_CDCLK_PLL_RATIO_MASK would be more correct. Same bits, but maybe
a bit less confusing if we don't share the define across registers.
With the wrong numbers in calc_voltage_level() fixed:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + else
> + ratio = I915_READ(BXT_DE_PLL_CTL) & BXT_DE_PLL_RATIO_MASK;
>
> - val = I915_READ(BXT_DE_PLL_CTL);
> - cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
> + cdclk_state->vco = ratio * cdclk_state->ref;
> }
>
> static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> @@ -1261,12 +1337,18 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> u32 divider;
> int div;
>
> - bxt_de_pll_update(dev_priv, cdclk_state);
> -
> - cdclk_state->cdclk = cdclk_state->bypass = cdclk_state->ref;
> + if (INTEL_GEN(dev_priv) >= 12)
> + cdclk_state->bypass = cdclk_state->ref / 2;
> + else if (INTEL_GEN(dev_priv) >= 11)
> + cdclk_state->bypass = 50000;
> + else
> + cdclk_state->bypass = cdclk_state->ref;
>
> - if (cdclk_state->vco == 0)
> + bxt_de_pll_readout(dev_priv, cdclk_state);
> + if (cdclk_state->vco == 0) {
> + cdclk_state->cdclk = cdclk_state->bypass;
> goto out;
> + }
>
> divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
>
> @@ -1275,13 +1357,15 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> div = 2;
> break;
> case BXT_CDCLK_CD2X_DIV_SEL_1_5:
> - WARN(IS_GEMINILAKE(dev_priv), "Unsupported divider\n");
> + WARN(IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10,
> + "Unsupported divider\n");
> div = 3;
> break;
> case BXT_CDCLK_CD2X_DIV_SEL_2:
> div = 4;
> break;
> case BXT_CDCLK_CD2X_DIV_SEL_4:
> + WARN(INTEL_GEN(dev_priv) >= 10, "Unsupported divider\n");
> div = 8;
> break;
> default:
> @@ -1296,8 +1380,18 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> * Can't read this out :( Let's assume it's
> * at least what the CDCLK frequency requires.
> */
> - cdclk_state->voltage_level =
> - bxt_calc_voltage_level(cdclk_state->cdclk);
> + if (IS_ELKHARTLAKE(dev_priv))
> + cdclk_state->voltage_level =
> + ehl_calc_voltage_level(cdclk_state->cdclk);
> + else if (INTEL_GEN(dev_priv) >= 11)
> + cdclk_state->voltage_level =
> + icl_calc_voltage_level(cdclk_state->cdclk);
> + else if (INTEL_GEN(dev_priv) >= 10)
> + cdclk_state->voltage_level =
> + cnl_calc_voltage_level(cdclk_state->cdclk);
> + else
> + cdclk_state->voltage_level =
> + bxt_calc_voltage_level(cdclk_state->cdclk);
> }
>
> static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> @@ -1515,76 +1609,6 @@ static int cnl_calc_cdclk(int min_cdclk)
> return 168000;
> }
>
> -static u8 cnl_calc_voltage_level(int cdclk)
> -{
> - if (cdclk > 336000)
> - return 2;
> - else if (cdclk > 168000)
> - return 1;
> - else
> - return 0;
> -}
> -
> -static void cnl_cdclk_pll_update(struct drm_i915_private *dev_priv,
> - struct intel_cdclk_state *cdclk_state)
> -{
> - u32 val;
> -
> - if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
> - cdclk_state->ref = 24000;
> - else
> - cdclk_state->ref = 19200;
> -
> - cdclk_state->vco = 0;
> -
> - val = I915_READ(BXT_DE_PLL_ENABLE);
> - if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
> - return;
> -
> - if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
> - return;
> -
> - cdclk_state->vco = (val & CNL_CDCLK_PLL_RATIO_MASK) * cdclk_state->ref;
> -}
> -
> -static void cnl_get_cdclk(struct drm_i915_private *dev_priv,
> - struct intel_cdclk_state *cdclk_state)
> -{
> - u32 divider;
> - int div;
> -
> - cnl_cdclk_pll_update(dev_priv, cdclk_state);
> -
> - cdclk_state->cdclk = cdclk_state->bypass = cdclk_state->ref;
> -
> - if (cdclk_state->vco == 0)
> - goto out;
> -
> - divider = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
> -
> - switch (divider) {
> - case BXT_CDCLK_CD2X_DIV_SEL_1:
> - div = 2;
> - break;
> - case BXT_CDCLK_CD2X_DIV_SEL_2:
> - div = 4;
> - break;
> - default:
> - MISSING_CASE(divider);
> - return;
> - }
> -
> - cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
> -
> - out:
> - /*
> - * Can't read this out :( Let's assume it's
> - * at least what the CDCLK frequency requires.
> - */
> - cdclk_state->voltage_level =
> - cnl_calc_voltage_level(cdclk_state->cdclk);
> -}
> -
> static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
> {
> u32 val;
> @@ -1830,91 +1854,6 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> return dev_priv->cdclk.hw.ref * ratio;
> }
>
> -static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int cdclk)
> -{
> - if (IS_ELKHARTLAKE(dev_priv)) {
> - if (cdclk > 312000)
> - return 2;
> - else if (cdclk > 180000)
> - return 1;
> - else
> - return 0;
> - } else {
> - if (cdclk > 556800)
> - return 2;
> - else if (cdclk > 312000)
> - return 1;
> - else
> - return 0;
> - }
> -}
> -
> -static void icl_get_cdclk(struct drm_i915_private *dev_priv,
> - struct intel_cdclk_state *cdclk_state)
> -{
> - u32 val;
> - int div;
> -
> - val = I915_READ(SKL_DSSM);
> - switch (val & ICL_DSSM_CDCLK_PLL_REFCLK_MASK) {
> - default:
> - MISSING_CASE(val);
> - /* fall through */
> - 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;
> - }
> -
> - if (INTEL_GEN(dev_priv) >= 12)
> - cdclk_state->bypass = cdclk_state->ref / 2;
> - else
> - cdclk_state->bypass = 50000;
> -
> - 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 = cdclk_state->bypass;
> - goto out;
> - }
> -
> - cdclk_state->vco = (val & BXT_DE_PLL_RATIO_MASK) * cdclk_state->ref;
> -
> - val = I915_READ(CDCLK_CTL) & BXT_CDCLK_CD2X_DIV_SEL_MASK;
> - switch (val) {
> - case BXT_CDCLK_CD2X_DIV_SEL_1:
> - div = 2;
> - break;
> - case BXT_CDCLK_CD2X_DIV_SEL_2:
> - div = 4;
> - break;
> - default:
> - MISSING_CASE(val);
> - div = 2;
> - break;
> - }
> -
> - cdclk_state->cdclk = DIV_ROUND_CLOSEST(cdclk_state->vco, div);
> -
> -out:
> - /*
> - * Can't read this out :( Let's assume it's
> - * at least what the CDCLK frequency requires.
> - */
> - cdclk_state->voltage_level =
> - icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
> -}
> -
> static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state sanitized_state;
> @@ -1946,9 +1885,12 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
> sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> sanitized_state.cdclk);
> - sanitized_state.voltage_level =
> - icl_calc_voltage_level(dev_priv,
> - sanitized_state.cdclk);
> + if (IS_ELKHARTLAKE(dev_priv))
> + sanitized_state.voltage_level =
> + ehl_calc_voltage_level(sanitized_state.cdclk);
> + else
> + sanitized_state.voltage_level =
> + icl_calc_voltage_level(sanitized_state.cdclk);
>
> cnl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> }
> @@ -1959,8 +1901,12 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
>
> cdclk_state.cdclk = cdclk_state.bypass;
> cdclk_state.vco = 0;
> - cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
> - cdclk_state.cdclk);
> + if (IS_ELKHARTLAKE(dev_priv))
> + cdclk_state.voltage_level =
> + ehl_calc_voltage_level(cdclk_state.cdclk);
> + else
> + cdclk_state.voltage_level =
> + icl_calc_voltage_level(cdclk_state.cdclk);
>
> cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
> @@ -2561,9 +2507,14 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> - state->cdclk.logical.voltage_level =
> - max(icl_calc_voltage_level(dev_priv, cdclk),
> - cnl_compute_min_voltage_level(state));
> + if (IS_ELKHARTLAKE(dev_priv))
> + state->cdclk.logical.voltage_level =
> + max(ehl_calc_voltage_level(cdclk),
> + cnl_compute_min_voltage_level(state));
> + else
> + state->cdclk.logical.voltage_level =
> + max(icl_calc_voltage_level(cdclk),
> + cnl_compute_min_voltage_level(state));
>
> if (!state->active_pipes) {
> cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
> @@ -2571,8 +2522,12 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> - state->cdclk.actual.voltage_level =
> - icl_calc_voltage_level(dev_priv, cdclk);
> + if (IS_ELKHARTLAKE(dev_priv))
> + state->cdclk.actual.voltage_level =
> + ehl_calc_voltage_level(cdclk);
> + else
> + state->cdclk.actual.voltage_level =
> + icl_calc_voltage_level(cdclk);
> } else {
> state->cdclk.actual = state->cdclk.logical;
> }
> @@ -2819,11 +2774,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
> }
>
> - if (INTEL_GEN(dev_priv) >= 11)
> - 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_LP(dev_priv))
> + else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> dev_priv->display.get_cdclk = bxt_get_cdclk;
> else if (IS_GEN9_BC(dev_priv))
> dev_priv->display.get_cdclk = skl_get_cdclk;
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
2019-09-07 0:21 ` [PATCH 1/8] drm/i915: Consolidate bxt/cnl/icl cdclk readout Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-08 2:57 ` Matt Roper
2019-09-07 0:21 ` [PATCH 3/8] drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk Matt Roper
` (11 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 319 ++++++++-------------
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h | 4 +
3 files changed, 126 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..8ac31f8775f0 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,97 @@ static void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 180000, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (ref == 19200 && table[i].ratio_19 != 0)
+ best_cdclk = table[i].cdclk;
+ else if (ref == 24000 && table[i].ratio_24 != 0)
+ best_cdclk = table[i].cdclk;
+ else if (ref == 38400 && table[i].ratio_38 != 0)
+ best_cdclk = table[i].cdclk;
+
+ if (table[i].cdclk < min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+ cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1289,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 144000:
- case 288000:
- case 384000:
- case 576000:
- ratio = 60;
- break;
- case 624000:
- ratio = 65;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
-static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 79200:
- case 158400:
- case 316800:
- ratio = 33;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
struct intel_cdclk_state *cdclk_state)
{
@@ -1576,13 +1599,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
* - The initial CDCLK needs to be read from VBT.
* Need to make this change after VBT has changes for BXT.
*/
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk_state.cdclk = glk_calc_cdclk(0);
- cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk);
- } else {
- cdclk_state.cdclk = bxt_calc_cdclk(0);
- cdclk_state.vco = bxt_de_pll_vco(dev_priv, cdclk_state.cdclk);
- }
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -1599,16 +1617,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int cnl_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 336000)
- return 528000;
- else if (min_cdclk > 168000)
- return 336000;
- else
- return 168000;
-}
-
static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
{
u32 val;
@@ -1718,29 +1726,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
}
-static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 168000:
- case 336000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28;
- break;
- case 528000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 55 : 44;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
{
u32 cdctl, expected;
@@ -1783,77 +1768,6 @@ 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)
-{
- static const int ranges_24[] = { 180000, 192000, 312000, 324000,
- 552000, 648000 };
- static const int ranges_19_38[] = { 172800, 192000, 307200, 326400,
- 556800, 652800 };
- const int *ranges;
- int len, i;
-
- switch (ref) {
- default:
- MISSING_CASE(ref);
- /* fall through */
- case 24000:
- ranges = ranges_24;
- len = ARRAY_SIZE(ranges_24);
- break;
- case 19200:
- case 38400:
- ranges = ranges_19_38;
- len = ARRAY_SIZE(ranges_19_38);
- break;
- }
-
- for (i = 0; i < len; i++) {
- if (min_cdclk <= ranges[i])
- return ranges[i];
- }
-
- WARN_ON(min_cdclk > ranges[len - 1]);
- return ranges[len - 1];
-}
-
-static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 172800:
- case 307200:
- case 326400:
- case 556800:
- case 652800:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400);
- break;
- case 180000:
- case 312000:
- case 324000:
- case 552000:
- case 648000:
- WARN_ON(dev_priv->cdclk.hw.ref != 24000);
- break;
- case 192000:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400 &&
- dev_priv->cdclk.hw.ref != 24000);
- break;
- }
-
- ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void icl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state sanitized_state;
@@ -1882,9 +1796,9 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
sanitized_state.ref = dev_priv->cdclk.hw.ref;
- sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
- sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
- sanitized_state.cdclk);
+ sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
+ sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
+ sanitized_state.cdclk);
if (IS_ELKHARTLAKE(dev_priv))
sanitized_state.voltage_level =
ehl_calc_voltage_level(sanitized_state.cdclk);
@@ -1923,8 +1837,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state = dev_priv->cdclk.hw;
- cdclk_state.cdclk = cnl_calc_cdclk(0);
- cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -2426,13 +2340,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2440,13 +2349,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
bxt_calc_voltage_level(cdclk);
if (!state->active_pipes) {
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2468,8 +2372,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- cdclk = cnl_calc_cdclk(min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2478,8 +2382,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = cnl_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2495,15 +2399,14 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- unsigned int ref = 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);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2517,8 +2420,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
- vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2754,12 +2657,18 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = icl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
} else if (IS_CANNONLAKE(dev_priv)) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = cnl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
} 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->cdclk.table = bxt_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
} 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;
@@ -2774,7 +2683,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
}
- else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
+ if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
dev_priv->display.get_cdclk = bxt_get_cdclk;
else if (IS_GEN9_BC(dev_priv))
dev_priv->display.get_cdclk = skl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 4d6f7f5f8930..80dc17a07aa2 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct intel_atomic_state;
struct intel_cdclk_state;
struct intel_crtc_state;
+struct intel_cdclk_vals {
+ int cdclk;
+ int divider; /* CD2X divider * 2 */
+ int ratio_19;
+ int ratio_24;
+ int ratio_38;
+};
+
int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
void intel_cdclk_init(struct drm_i915_private *i915);
void intel_cdclk_uninit(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index db7480831e52..a4d249193dc8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1420,6 +1420,10 @@ struct drm_i915_private {
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+ /* cdclk, divider, and ratio table from bspec */
+ const struct intel_cdclk_vals *table;
+ int table_size;
+
int force_min_cdclk;
} cdclk;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
2019-09-07 0:21 ` [PATCH 2/8] drm/i915: Use literal representation of cdclk tables Matt Roper
@ 2019-09-08 2:57 ` Matt Roper
2019-09-08 4:05 ` Matt Roper
0 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-08 2:57 UTC (permalink / raw)
To: intel-gfx
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
v2: Fix comparison when finding best cdclk.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 318 ++++++++-------------
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h | 4 +
3 files changed, 125 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..90efc82b8a94 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 180000, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (ref == 19200 && table[i].ratio_19 == 0 ||
+ ref == 24000 && table[i].ratio_24 == 0 ||
+ ref == 38400 && table[i].ratio_38 == 0)
+ continue;
+
+ best_cdclk = table[i].cdclk;
+ if (table[i].cdclk >= min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+ cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1288,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 144000:
- case 288000:
- case 384000:
- case 576000:
- ratio = 60;
- break;
- case 624000:
- ratio = 65;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
-static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 79200:
- case 158400:
- case 316800:
- ratio = 33;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
struct intel_cdclk_state *cdclk_state)
{
@@ -1576,13 +1598,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
* - The initial CDCLK needs to be read from VBT.
* Need to make this change after VBT has changes for BXT.
*/
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk_state.cdclk = glk_calc_cdclk(0);
- cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk);
- } else {
- cdclk_state.cdclk = bxt_calc_cdclk(0);
- cdclk_state.vco = bxt_de_pll_vco(dev_priv, cdclk_state.cdclk);
- }
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -1599,16 +1616,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int cnl_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 336000)
- return 528000;
- else if (min_cdclk > 168000)
- return 336000;
- else
- return 168000;
-}
-
static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
{
u32 val;
@@ -1718,29 +1725,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
}
-static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 168000:
- case 336000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28;
- break;
- case 528000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 55 : 44;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
{
u32 cdctl, expected;
@@ -1783,77 +1767,6 @@ 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)
-{
- static const int ranges_24[] = { 180000, 192000, 312000, 324000,
- 552000, 648000 };
- static const int ranges_19_38[] = { 172800, 192000, 307200, 326400,
- 556800, 652800 };
- const int *ranges;
- int len, i;
-
- switch (ref) {
- default:
- MISSING_CASE(ref);
- /* fall through */
- case 24000:
- ranges = ranges_24;
- len = ARRAY_SIZE(ranges_24);
- break;
- case 19200:
- case 38400:
- ranges = ranges_19_38;
- len = ARRAY_SIZE(ranges_19_38);
- break;
- }
-
- for (i = 0; i < len; i++) {
- if (min_cdclk <= ranges[i])
- return ranges[i];
- }
-
- WARN_ON(min_cdclk > ranges[len - 1]);
- return ranges[len - 1];
-}
-
-static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 172800:
- case 307200:
- case 326400:
- case 556800:
- case 652800:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400);
- break;
- case 180000:
- case 312000:
- case 324000:
- case 552000:
- case 648000:
- WARN_ON(dev_priv->cdclk.hw.ref != 24000);
- break;
- case 192000:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400 &&
- dev_priv->cdclk.hw.ref != 24000);
- break;
- }
-
- ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void icl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state sanitized_state;
@@ -1882,9 +1795,9 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
sanitized_state.ref = dev_priv->cdclk.hw.ref;
- sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
- sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
- sanitized_state.cdclk);
+ sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
+ sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
+ sanitized_state.cdclk);
if (IS_ELKHARTLAKE(dev_priv))
sanitized_state.voltage_level =
ehl_calc_voltage_level(sanitized_state.cdclk);
@@ -1923,8 +1836,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state = dev_priv->cdclk.hw;
- cdclk_state.cdclk = cnl_calc_cdclk(0);
- cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -2426,13 +2339,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2440,13 +2348,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
bxt_calc_voltage_level(cdclk);
if (!state->active_pipes) {
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2468,8 +2371,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- cdclk = cnl_calc_cdclk(min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2478,8 +2381,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = cnl_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2495,15 +2398,14 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- unsigned int ref = 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);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2517,8 +2419,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
- vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2754,12 +2656,18 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = icl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
} else if (IS_CANNONLAKE(dev_priv)) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = cnl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
} 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->cdclk.table = bxt_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
} 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;
@@ -2774,7 +2682,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
}
- else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
+ if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
dev_priv->display.get_cdclk = bxt_get_cdclk;
else if (IS_GEN9_BC(dev_priv))
dev_priv->display.get_cdclk = skl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 4d6f7f5f8930..80dc17a07aa2 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct intel_atomic_state;
struct intel_cdclk_state;
struct intel_crtc_state;
+struct intel_cdclk_vals {
+ int cdclk;
+ int divider; /* CD2X divider * 2 */
+ int ratio_19;
+ int ratio_24;
+ int ratio_38;
+};
+
int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
void intel_cdclk_init(struct drm_i915_private *i915);
void intel_cdclk_uninit(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index db7480831e52..a4d249193dc8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1420,6 +1420,10 @@ struct drm_i915_private {
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+ /* cdclk, divider, and ratio table from bspec */
+ const struct intel_cdclk_vals *table;
+ int table_size;
+
int force_min_cdclk;
} cdclk;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
2019-09-08 2:57 ` Matt Roper
@ 2019-09-08 4:05 ` Matt Roper
2019-09-10 12:56 ` Ville Syrjälä
0 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-08 4:05 UTC (permalink / raw)
To: intel-gfx
The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
dividers in an easy-to-read table for most recent platforms. We've been
translating the data from that table into platform-specific code logic,
but it's easy to overlook an area we need to update when adding new
cdclk values or enabling new platforms. Let's just add a form of the
bspec table to the code and then adjust our functions to pull what they
need directly out of the table.
v2: Fix comparison when finding best cdclk.
v3: Another logic fix for calc_cdclk.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 318 ++++++++-------------
drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
drivers/gpu/drm/i915/i915_drv.h | 4 +
3 files changed, 125 insertions(+), 205 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index e07de3b84cec..ca64143b6d7e 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int bxt_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 576000)
- return 624000;
- else if (min_cdclk > 384000)
- return 576000;
- else if (min_cdclk > 288000)
- return 384000;
- else if (min_cdclk > 144000)
- return 288000;
- else
- return 144000;
+static const struct intel_cdclk_vals bxt_cdclk_table[] = {
+ { 144000, 8, 60 },
+ { 288000, 4, 60 },
+ { 384000, 3, 60 },
+ { 576000, 2, 60 },
+ { 624000, 2, 65 },
+};
+
+static const struct intel_cdclk_vals glk_cdclk_table[] = {
+ { 79200, 8, 33 },
+ { 158400, 4, 33 },
+ { 316800, 2, 33 },
+};
+
+static const struct intel_cdclk_vals cnl_cdclk_table[] = {
+ { 168000, 4, 35, 28 },
+ { 336000, 2, 35, 28 },
+ { 528000, 2, 55, 44 },
+};
+
+static const struct intel_cdclk_vals icl_cdclk_table[] = {
+ { 172800, 2, 18, 0, 9 },
+ { 180000, 2, 0, 15, 0 },
+ { 192000, 2, 20, 16, 10 },
+ { 307200, 2, 32, 0, 16 },
+ { 312000, 2, 0, 26, 0 },
+ { 324000, 4, 0, 54, 0 },
+ { 326400, 4, 68, 0, 34 },
+ { 552000, 2, 0, 46, 0 },
+ { 556800, 2, 58, 0, 29 },
+ { 648000, 2, 0, 54, 0 },
+ { 652800, 2, 68, 0, 34 },
+};
+
+static int calc_cdclk(struct drm_i915_private *dev_priv,
+ int min_cdclk)
+{
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ unsigned int ref = dev_priv->cdclk.hw.ref;
+ int best_cdclk = 0;
+ int i;
+
+ if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
+ ref = 19200;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if ((ref == 19200 && table[i].ratio_19 == 0) ||
+ (ref == 24000 && table[i].ratio_24 == 0) ||
+ (ref == 38400 && table[i].ratio_38 == 0))
+ continue;
+
+ best_cdclk = table[i].cdclk;
+ if (table[i].cdclk >= min_cdclk)
+ return best_cdclk;
+ }
+
+ WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
+ return best_cdclk;
}
-static int glk_calc_cdclk(int min_cdclk)
+static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
{
- if (min_cdclk > 158400)
- return 316800;
- else if (min_cdclk > 79200)
- return 158400;
- else
- return 79200;
+ const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
+ int ratio, best_ratio, i;
+
+ if (cdclk == dev_priv->cdclk.hw.bypass)
+ return 0;
+
+ for (i = 0; i < dev_priv->cdclk.table_size; i++) {
+ if (dev_priv->cdclk.hw.ref == 19200)
+ ratio = table[i].ratio_19;
+ else if (dev_priv->cdclk.hw.ref == 24000)
+ ratio = table[i].ratio_24;
+ else
+ ratio = table[i].ratio_38;
+
+ if (ratio == 0)
+ continue;
+ else
+ best_ratio = ratio;
+
+ if (table[i].cdclk == cdclk ||
+ WARN_ON(table[i].cdclk > cdclk))
+ return dev_priv->cdclk.hw.ref * ratio;
+ }
+
+ WARN(1, "cdclk %d not valid for refclk %d\n",
+ cdclk, dev_priv->cdclk.hw.ref);
+
+ return dev_priv->cdclk.hw.ref * best_ratio;
}
static u8 bxt_calc_voltage_level(int cdclk)
@@ -1220,52 +1288,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
return 0;
}
-static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 144000:
- case 288000:
- case 384000:
- case 576000:
- ratio = 60;
- break;
- case 624000:
- ratio = 65;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
-static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 79200:
- case 158400:
- case 316800:
- ratio = 33;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
struct intel_cdclk_state *cdclk_state)
{
@@ -1576,13 +1598,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
* - The initial CDCLK needs to be read from VBT.
* Need to make this change after VBT has changes for BXT.
*/
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk_state.cdclk = glk_calc_cdclk(0);
- cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk);
- } else {
- cdclk_state.cdclk = bxt_calc_cdclk(0);
- cdclk_state.vco = bxt_de_pll_vco(dev_priv, cdclk_state.cdclk);
- }
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -1599,16 +1616,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static int cnl_calc_cdclk(int min_cdclk)
-{
- if (min_cdclk > 336000)
- return 528000;
- else if (min_cdclk > 168000)
- return 336000;
- else
- return 168000;
-}
-
static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
{
u32 val;
@@ -1718,29 +1725,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
}
-static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 168000:
- case 336000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28;
- break;
- case 528000:
- ratio = dev_priv->cdclk.hw.ref == 19200 ? 55 : 44;
- break;
- }
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
{
u32 cdctl, expected;
@@ -1783,77 +1767,6 @@ 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)
-{
- static const int ranges_24[] = { 180000, 192000, 312000, 324000,
- 552000, 648000 };
- static const int ranges_19_38[] = { 172800, 192000, 307200, 326400,
- 556800, 652800 };
- const int *ranges;
- int len, i;
-
- switch (ref) {
- default:
- MISSING_CASE(ref);
- /* fall through */
- case 24000:
- ranges = ranges_24;
- len = ARRAY_SIZE(ranges_24);
- break;
- case 19200:
- case 38400:
- ranges = ranges_19_38;
- len = ARRAY_SIZE(ranges_19_38);
- break;
- }
-
- for (i = 0; i < len; i++) {
- if (min_cdclk <= ranges[i])
- return ranges[i];
- }
-
- WARN_ON(min_cdclk > ranges[len - 1]);
- return ranges[len - 1];
-}
-
-static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
-{
- int ratio;
-
- if (cdclk == dev_priv->cdclk.hw.bypass)
- return 0;
-
- switch (cdclk) {
- default:
- MISSING_CASE(cdclk);
- /* fall through */
- case 172800:
- case 307200:
- case 326400:
- case 556800:
- case 652800:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400);
- break;
- case 180000:
- case 312000:
- case 324000:
- case 552000:
- case 648000:
- WARN_ON(dev_priv->cdclk.hw.ref != 24000);
- break;
- case 192000:
- WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
- dev_priv->cdclk.hw.ref != 38400 &&
- dev_priv->cdclk.hw.ref != 24000);
- break;
- }
-
- ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
-
- return dev_priv->cdclk.hw.ref * ratio;
-}
-
static void icl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state sanitized_state;
@@ -1882,9 +1795,9 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
sanitized_state.ref = dev_priv->cdclk.hw.ref;
- sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
- sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
- sanitized_state.cdclk);
+ sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
+ sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
+ sanitized_state.cdclk);
if (IS_ELKHARTLAKE(dev_priv))
sanitized_state.voltage_level =
ehl_calc_voltage_level(sanitized_state.cdclk);
@@ -1923,8 +1836,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state = dev_priv->cdclk.hw;
- cdclk_state.cdclk = cnl_calc_cdclk(0);
- cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
+ cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
+ cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
@@ -2426,13 +2339,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2440,13 +2348,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
bxt_calc_voltage_level(cdclk);
if (!state->active_pipes) {
- if (IS_GEMINILAKE(dev_priv)) {
- cdclk = glk_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = glk_de_pll_vco(dev_priv, cdclk);
- } else {
- cdclk = bxt_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = bxt_de_pll_vco(dev_priv, cdclk);
- }
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2468,8 +2371,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
if (min_cdclk < 0)
return min_cdclk;
- cdclk = cnl_calc_cdclk(min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2478,8 +2381,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = cnl_calc_cdclk(state->cdclk.force_min_cdclk);
- vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2495,15 +2398,14 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
{
struct drm_i915_private *dev_priv = to_i915(state->base.dev);
- unsigned int ref = 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);
+ cdclk = calc_cdclk(dev_priv, min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
@@ -2517,8 +2419,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
- cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
- vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
+ cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
@@ -2754,12 +2656,18 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = icl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
} else if (IS_CANNONLAKE(dev_priv)) {
dev_priv->display.set_cdclk = cnl_set_cdclk;
dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
+ dev_priv->cdclk.table = cnl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
} 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->cdclk.table = bxt_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
} 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;
@@ -2774,7 +2682,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
}
- else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
+ if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
dev_priv->display.get_cdclk = bxt_get_cdclk;
else if (IS_GEN9_BC(dev_priv))
dev_priv->display.get_cdclk = skl_get_cdclk;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 4d6f7f5f8930..80dc17a07aa2 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct intel_atomic_state;
struct intel_cdclk_state;
struct intel_crtc_state;
+struct intel_cdclk_vals {
+ int cdclk;
+ int divider; /* CD2X divider * 2 */
+ int ratio_19;
+ int ratio_24;
+ int ratio_38;
+};
+
int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
void intel_cdclk_init(struct drm_i915_private *i915);
void intel_cdclk_uninit(struct drm_i915_private *i915);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index db7480831e52..a4d249193dc8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1420,6 +1420,10 @@ struct drm_i915_private {
/* The current hardware cdclk state */
struct intel_cdclk_state hw;
+ /* cdclk, divider, and ratio table from bspec */
+ const struct intel_cdclk_vals *table;
+ int table_size;
+
int force_min_cdclk;
} cdclk;
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 2/8] drm/i915: Use literal representation of cdclk tables
2019-09-08 4:05 ` Matt Roper
@ 2019-09-10 12:56 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:56 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Sat, Sep 07, 2019 at 09:05:00PM -0700, Matt Roper wrote:
> The bspec lays out legal cdclk frequencies, PLL ratios, and CD2X
> dividers in an easy-to-read table for most recent platforms. We've been
> translating the data from that table into platform-specific code logic,
> but it's easy to overlook an area we need to update when adding new
> cdclk values or enabling new platforms. Let's just add a form of the
> bspec table to the code and then adjust our functions to pull what they
> need directly out of the table.
>
> v2: Fix comparison when finding best cdclk.
>
> v3: Another logic fix for calc_cdclk.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 318 ++++++++-------------
> drivers/gpu/drm/i915/display/intel_cdclk.h | 8 +
> drivers/gpu/drm/i915/i915_drv.h | 4 +
> 3 files changed, 125 insertions(+), 205 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index e07de3b84cec..ca64143b6d7e 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1161,28 +1161,96 @@ static void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static int bxt_calc_cdclk(int min_cdclk)
> -{
> - if (min_cdclk > 576000)
> - return 624000;
> - else if (min_cdclk > 384000)
> - return 576000;
> - else if (min_cdclk > 288000)
> - return 384000;
> - else if (min_cdclk > 144000)
> - return 288000;
> - else
> - return 144000;
> +static const struct intel_cdclk_vals bxt_cdclk_table[] = {
> + { 144000, 8, 60 },
> + { 288000, 4, 60 },
> + { 384000, 3, 60 },
> + { 576000, 2, 60 },
> + { 624000, 2, 65 },
Named initializers might make this number soup digestible.
> +};
> +
> +static const struct intel_cdclk_vals glk_cdclk_table[] = {
> + { 79200, 8, 33 },
> + { 158400, 4, 33 },
> + { 316800, 2, 33 },
> +};
> +
> +static const struct intel_cdclk_vals cnl_cdclk_table[] = {
> + { 168000, 4, 35, 28 },
> + { 336000, 2, 35, 28 },
> + { 528000, 2, 55, 44 },
> +};
> +
> +static const struct intel_cdclk_vals icl_cdclk_table[] = {
> + { 172800, 2, 18, 0, 9 },
> + { 180000, 2, 0, 15, 0 },
> + { 192000, 2, 20, 16, 10 },
> + { 307200, 2, 32, 0, 16 },
> + { 312000, 2, 0, 26, 0 },
> + { 324000, 4, 0, 54, 0 },
> + { 326400, 4, 68, 0, 34 },
> + { 552000, 2, 0, 46, 0 },
> + { 556800, 2, 58, 0, 29 },
> + { 648000, 2, 0, 54, 0 },
> + { 652800, 2, 68, 0, 34 },
> +};
> +
> +static int calc_cdclk(struct drm_i915_private *dev_priv,
> + int min_cdclk)
> +{
> + const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
> + unsigned int ref = dev_priv->cdclk.hw.ref;
> + int best_cdclk = 0;
> + int i;
> +
> + if (WARN_ON(ref != 19200 && ref != 24000 && ref != 38400))
> + ref = 19200;
Feels a bit like dead code.
> +
> + for (i = 0; i < dev_priv->cdclk.table_size; i++) {
> + if ((ref == 19200 && table[i].ratio_19 == 0) ||
> + (ref == 24000 && table[i].ratio_24 == 0) ||
> + (ref == 38400 && table[i].ratio_38 == 0))
> + continue;
> +
> + best_cdclk = table[i].cdclk;
> + if (table[i].cdclk >= min_cdclk)
> + return best_cdclk;
> + }
> +
> + WARN(1, "Cannot satisfy minimum cdclk %d\n", min_cdclk);
> + return best_cdclk;
Same for the best_cdclk thing. I'd probably just MISSING_CASE()
and return 0, or something.
> }
>
> -static int glk_calc_cdclk(int min_cdclk)
> +static int calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
We still use these things for bxt+ only, so would be nice if
the names kept reflecting that fact.
Though I suppose we should be able to extend this more
declarative approach backwards to older platforms as well.
> {
> - if (min_cdclk > 158400)
> - return 316800;
> - else if (min_cdclk > 79200)
> - return 158400;
> - else
> - return 79200;
> + const struct intel_cdclk_vals *table = dev_priv->cdclk.table;
> + int ratio, best_ratio, i;
> +
> + if (cdclk == dev_priv->cdclk.hw.bypass)
> + return 0;
> +
> + for (i = 0; i < dev_priv->cdclk.table_size; i++) {
> + if (dev_priv->cdclk.hw.ref == 19200)
> + ratio = table[i].ratio_19;
> + else if (dev_priv->cdclk.hw.ref == 24000)
> + ratio = table[i].ratio_24;
> + else
> + ratio = table[i].ratio_38;
> +
> + if (ratio == 0)
> + continue;
> + else
> + best_ratio = ratio;
> +
> + if (table[i].cdclk == cdclk ||
> + WARN_ON(table[i].cdclk > cdclk))
> + return dev_priv->cdclk.hw.ref * ratio;
> + }
> +
> + WARN(1, "cdclk %d not valid for refclk %d\n",
> + cdclk, dev_priv->cdclk.hw.ref);
> +
> + return dev_priv->cdclk.hw.ref * best_ratio;
> }
>
> static u8 bxt_calc_voltage_level(int cdclk)
> @@ -1220,52 +1288,6 @@ static u8 ehl_calc_voltage_level(int cdclk)
> return 0;
> }
>
> -static int bxt_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> -{
> - int ratio;
> -
> - if (cdclk == dev_priv->cdclk.hw.bypass)
> - return 0;
> -
> - switch (cdclk) {
> - default:
> - MISSING_CASE(cdclk);
> - /* fall through */
> - case 144000:
> - case 288000:
> - case 384000:
> - case 576000:
> - ratio = 60;
> - break;
> - case 624000:
> - ratio = 65;
> - break;
> - }
> -
> - return dev_priv->cdclk.hw.ref * ratio;
> -}
> -
> -static int glk_de_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> -{
> - int ratio;
> -
> - if (cdclk == dev_priv->cdclk.hw.bypass)
> - return 0;
> -
> - switch (cdclk) {
> - default:
> - MISSING_CASE(cdclk);
> - /* fall through */
> - case 79200:
> - case 158400:
> - case 316800:
> - ratio = 33;
> - break;
> - }
> -
> - return dev_priv->cdclk.hw.ref * ratio;
> -}
> -
> static void cnl_readout_refclk(struct drm_i915_private *dev_priv,
> struct intel_cdclk_state *cdclk_state)
> {
> @@ -1576,13 +1598,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> * - The initial CDCLK needs to be read from VBT.
> * Need to make this change after VBT has changes for BXT.
> */
> - if (IS_GEMINILAKE(dev_priv)) {
> - cdclk_state.cdclk = glk_calc_cdclk(0);
> - cdclk_state.vco = glk_de_pll_vco(dev_priv, cdclk_state.cdclk);
> - } else {
> - cdclk_state.cdclk = bxt_calc_cdclk(0);
> - cdclk_state.vco = bxt_de_pll_vco(dev_priv, cdclk_state.cdclk);
> - }
> + cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
> + cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> @@ -1599,16 +1616,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static int cnl_calc_cdclk(int min_cdclk)
> -{
> - if (min_cdclk > 336000)
> - return 528000;
> - else if (min_cdclk > 168000)
> - return 336000;
> - else
> - return 168000;
> -}
> -
> static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
> {
> u32 val;
> @@ -1718,29 +1725,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
> }
>
> -static int cnl_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> -{
> - int ratio;
> -
> - if (cdclk == dev_priv->cdclk.hw.bypass)
> - return 0;
> -
> - switch (cdclk) {
> - default:
> - MISSING_CASE(cdclk);
> - /* fall through */
> - case 168000:
> - case 336000:
> - ratio = dev_priv->cdclk.hw.ref == 19200 ? 35 : 28;
> - break;
> - case 528000:
> - ratio = dev_priv->cdclk.hw.ref == 19200 ? 55 : 44;
> - break;
> - }
> -
> - return dev_priv->cdclk.hw.ref * ratio;
> -}
> -
> static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> {
> u32 cdctl, expected;
> @@ -1783,77 +1767,6 @@ 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)
> -{
> - static const int ranges_24[] = { 180000, 192000, 312000, 324000,
> - 552000, 648000 };
> - static const int ranges_19_38[] = { 172800, 192000, 307200, 326400,
> - 556800, 652800 };
> - const int *ranges;
> - int len, i;
> -
> - switch (ref) {
> - default:
> - MISSING_CASE(ref);
> - /* fall through */
> - case 24000:
> - ranges = ranges_24;
> - len = ARRAY_SIZE(ranges_24);
> - break;
> - case 19200:
> - case 38400:
> - ranges = ranges_19_38;
> - len = ARRAY_SIZE(ranges_19_38);
> - break;
> - }
> -
> - for (i = 0; i < len; i++) {
> - if (min_cdclk <= ranges[i])
> - return ranges[i];
> - }
> -
> - WARN_ON(min_cdclk > ranges[len - 1]);
> - return ranges[len - 1];
> -}
> -
> -static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> -{
> - int ratio;
> -
> - if (cdclk == dev_priv->cdclk.hw.bypass)
> - return 0;
> -
> - switch (cdclk) {
> - default:
> - MISSING_CASE(cdclk);
> - /* fall through */
> - case 172800:
> - case 307200:
> - case 326400:
> - case 556800:
> - case 652800:
> - WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> - dev_priv->cdclk.hw.ref != 38400);
> - break;
> - case 180000:
> - case 312000:
> - case 324000:
> - case 552000:
> - case 648000:
> - WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> - break;
> - case 192000:
> - WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> - dev_priv->cdclk.hw.ref != 38400 &&
> - dev_priv->cdclk.hw.ref != 24000);
> - break;
> - }
> -
> - ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> -
> - return dev_priv->cdclk.hw.ref * ratio;
> -}
> -
> static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state sanitized_state;
> @@ -1882,9 +1795,9 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
>
> sanitized_state.ref = dev_priv->cdclk.hw.ref;
> - sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
> - sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> - sanitized_state.cdclk);
> + sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
> + sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
> + sanitized_state.cdclk);
> if (IS_ELKHARTLAKE(dev_priv))
> sanitized_state.voltage_level =
> ehl_calc_voltage_level(sanitized_state.cdclk);
> @@ -1923,8 +1836,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>
> cdclk_state = dev_priv->cdclk.hw;
>
> - cdclk_state.cdclk = cnl_calc_cdclk(0);
> - cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> + cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
> + cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>
> cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> @@ -2426,13 +2339,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
> if (min_cdclk < 0)
> return min_cdclk;
>
> - if (IS_GEMINILAKE(dev_priv)) {
> - cdclk = glk_calc_cdclk(min_cdclk);
> - vco = glk_de_pll_vco(dev_priv, cdclk);
> - } else {
> - cdclk = bxt_calc_cdclk(min_cdclk);
> - vco = bxt_de_pll_vco(dev_priv, cdclk);
> - }
> + cdclk = calc_cdclk(dev_priv, min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> @@ -2440,13 +2348,8 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
> bxt_calc_voltage_level(cdclk);
>
> if (!state->active_pipes) {
> - if (IS_GEMINILAKE(dev_priv)) {
> - cdclk = glk_calc_cdclk(state->cdclk.force_min_cdclk);
> - vco = glk_de_pll_vco(dev_priv, cdclk);
> - } else {
> - cdclk = bxt_calc_cdclk(state->cdclk.force_min_cdclk);
> - vco = bxt_de_pll_vco(dev_priv, cdclk);
> - }
> + cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> @@ -2468,8 +2371,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
> if (min_cdclk < 0)
> return min_cdclk;
>
> - cdclk = cnl_calc_cdclk(min_cdclk);
> - vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
> + cdclk = calc_cdclk(dev_priv, min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> @@ -2478,8 +2381,8 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
> cnl_compute_min_voltage_level(state));
>
> if (!state->active_pipes) {
> - cdclk = cnl_calc_cdclk(state->cdclk.force_min_cdclk);
> - vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
> + cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> @@ -2495,15 +2398,14 @@ static int cnl_modeset_calc_cdclk(struct intel_atomic_state *state)
> static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
> {
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> - unsigned int ref = 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);
> + cdclk = calc_cdclk(dev_priv, min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> @@ -2517,8 +2419,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
> cnl_compute_min_voltage_level(state));
>
> if (!state->active_pipes) {
> - cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
> - vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> + cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
>
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> @@ -2754,12 +2656,18 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> if (INTEL_GEN(dev_priv) >= 11) {
> dev_priv->display.set_cdclk = cnl_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> + dev_priv->cdclk.table = icl_cdclk_table;
> + dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
> } else if (IS_CANNONLAKE(dev_priv)) {
> dev_priv->display.set_cdclk = cnl_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
> + dev_priv->cdclk.table = cnl_cdclk_table;
> + dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
> } 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->cdclk.table = bxt_cdclk_table;
> + dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
> } 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;
> @@ -2774,7 +2682,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> dev_priv->display.modeset_calc_cdclk = vlv_modeset_calc_cdclk;
> }
>
> - else if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> + if (IS_GEN9_LP(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> dev_priv->display.get_cdclk = bxt_get_cdclk;
> else if (IS_GEN9_BC(dev_priv))
> dev_priv->display.get_cdclk = skl_get_cdclk;
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 4d6f7f5f8930..80dc17a07aa2 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -15,6 +15,14 @@ struct intel_atomic_state;
> struct intel_cdclk_state;
> struct intel_crtc_state;
>
> +struct intel_cdclk_vals {
> + int cdclk;
> + int divider; /* CD2X divider * 2 */
> + int ratio_19;
> + int ratio_24;
> + int ratio_38;
I think a bunch of these can easily fit smaller types.
A more general approach might be just
struct ... {
cdclk
ref
divider
ratio
};
Ie. add a unique entry for each cdclk+ref combo. that way we don't have
to encode the set of possible ref clocks here at all, and thus can use
this still when the set of ref clocks changes on some future platform.
> +};
> +
> int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
> void intel_cdclk_init(struct drm_i915_private *i915);
> void intel_cdclk_uninit(struct drm_i915_private *i915);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index db7480831e52..a4d249193dc8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1420,6 +1420,10 @@ struct drm_i915_private {
> /* The current hardware cdclk state */
> struct intel_cdclk_state hw;
>
> + /* cdclk, divider, and ratio table from bspec */
> + const struct intel_cdclk_vals *table;
> + int table_size;
Or we terminate the tables with {} ?
> +
> int force_min_cdclk;
> } cdclk;
>
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 3/8] drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
2019-09-07 0:21 ` [PATCH 1/8] drm/i915: Consolidate bxt/cnl/icl cdclk readout Matt Roper
2019-09-07 0:21 ` [PATCH 2/8] drm/i915: Use literal representation of cdclk tables Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:35 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 4/8] drm/i915: Kill cnl_sanitize_cdclk() Matt Roper
` (10 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
We'd previously combined ICL/TGL logic into the cnl_set_cdclk function,
but BXT is pretty similar as well. Roll the cnl/icl/tgl logic back into
the bxt function; the only things we really need to handle separately
are punit notification and calling different functions to enable/disable
the cdclk PLL.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 267 +++++++++------------
1 file changed, 119 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 8ac31f8775f0..6b5b1328a3fa 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1449,6 +1449,39 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
dev_priv->cdclk.hw.vco = vco;
}
+static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
+{
+ u32 val;
+
+ val = I915_READ(BXT_DE_PLL_ENABLE);
+ val &= ~BXT_DE_PLL_PLL_ENABLE;
+ I915_WRITE(BXT_DE_PLL_ENABLE, val);
+
+ /* Timeout 200us */
+ if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) == 0, 1))
+ DRM_ERROR("timeout waiting for CDCLK PLL unlock\n");
+
+ dev_priv->cdclk.hw.vco = 0;
+}
+
+static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
+{
+ int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk.hw.ref);
+ u32 val;
+
+ val = CNL_CDCLK_PLL_RATIO(ratio);
+ I915_WRITE(BXT_DE_PLL_ENABLE, val);
+
+ val |= BXT_DE_PLL_PLL_ENABLE;
+ I915_WRITE(BXT_DE_PLL_ENABLE, val);
+
+ /* Timeout 200us */
+ if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) != 0, 1))
+ DRM_ERROR("timeout waiting for CDCLK PLL lock\n");
+
+ dev_priv->cdclk.hw.vco = vco;
+}
+
static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
const struct intel_cdclk_state *cdclk_state,
enum pipe pipe)
@@ -1458,6 +1491,27 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
u32 val, divider;
int ret;
+ /* Inform power controller of upcoming frequency change. */
+ if (INTEL_GEN(dev_priv) >= 10)
+ 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);
+ else
+ /*
+ * BSpec requires us to wait up to 150usec, but that leads to
+ * timeouts; the 2ms used here is based on experiment.
+ */
+ ret = sandybridge_pcode_write_timeout(dev_priv,
+ HSW_PCODE_DE_WRITE_FREQ_REQ,
+ 0x80000000, 150, 2);
+
+ if (ret) {
+ DRM_ERROR("Failed to inform PCU about cdclk change (err %d, freq %d)\n",
+ ret, cdclk);
+ return;
+ }
+
/* cdclk = vco / 2 / div{1,1.5,2,4} */
switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
default:
@@ -1468,63 +1522,82 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
divider = BXT_CDCLK_CD2X_DIV_SEL_1;
break;
case 3:
- WARN(IS_GEMINILAKE(dev_priv), "Unsupported divider\n");
+ WARN(IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10,
+ "Unsupported divider\n");
divider = BXT_CDCLK_CD2X_DIV_SEL_1_5;
break;
case 4:
divider = BXT_CDCLK_CD2X_DIV_SEL_2;
break;
case 8:
+ WARN(INTEL_GEN(dev_priv) >= 10, "Unsupported divider\n");
divider = BXT_CDCLK_CD2X_DIV_SEL_4;
break;
}
- /*
- * Inform power controller of upcoming frequency change. BSpec
- * requires us to wait up to 150usec, but that leads to timeouts;
- * the 2ms used here is based on experiment.
- */
- ret = sandybridge_pcode_write_timeout(dev_priv,
- HSW_PCODE_DE_WRITE_FREQ_REQ,
- 0x80000000, 150, 2);
- if (ret) {
- DRM_ERROR("PCode CDCLK freq change notify failed (err %d, freq %d)\n",
- ret, cdclk);
- return;
- }
+ if (INTEL_GEN(dev_priv) >= 10) {
+ 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 != 0 &&
- dev_priv->cdclk.hw.vco != vco)
- bxt_de_pll_disable(dev_priv);
+ if (dev_priv->cdclk.hw.vco != vco)
+ cnl_cdclk_pll_enable(dev_priv, vco);
- if (dev_priv->cdclk.hw.vco != vco)
- bxt_de_pll_enable(dev_priv, vco);
+ } else {
+ if (dev_priv->cdclk.hw.vco != 0 &&
+ dev_priv->cdclk.hw.vco != vco)
+ bxt_de_pll_disable(dev_priv);
+
+ if (dev_priv->cdclk.hw.vco != vco)
+ bxt_de_pll_enable(dev_priv, vco);
+ }
val = divider | skl_cdclk_decimal(cdclk);
- if (pipe == INVALID_PIPE)
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
- else
- val |= BXT_CDCLK_CD2X_PIPE(pipe);
+
+ if (INTEL_GEN(dev_priv) >= 12) {
+ if (pipe == INVALID_PIPE)
+ val |= TGL_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= TGL_CDCLK_CD2X_PIPE(pipe);
+ } else if (INTEL_GEN(dev_priv) >= 11) {
+ if (pipe == INVALID_PIPE)
+ val |= ICL_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= ICL_CDCLK_CD2X_PIPE(pipe);
+ } else {
+ if (pipe == INVALID_PIPE)
+ val |= BXT_CDCLK_CD2X_PIPE_NONE;
+ else
+ val |= BXT_CDCLK_CD2X_PIPE(pipe);
+ }
+
/*
* Disable SSA Precharge when CD clock frequency < 500 MHz,
* enable otherwise.
*/
- if (cdclk >= 500000)
+ if (IS_GEN9_LP(dev_priv) && cdclk >= 500000)
val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
I915_WRITE(CDCLK_CTL, val);
if (pipe != INVALID_PIPE)
intel_wait_for_vblank(dev_priv, pipe);
- /*
- * The timeout isn't specified, the 2ms used here is based on
- * experiment.
- * FIXME: Waiting for the request completion could be delayed until
- * the next PCODE request based on BSpec.
- */
- ret = sandybridge_pcode_write_timeout(dev_priv,
- HSW_PCODE_DE_WRITE_FREQ_REQ,
- cdclk_state->voltage_level, 150, 2);
+ if (INTEL_GEN(dev_priv) >= 10) {
+ ret = sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+ cdclk_state->voltage_level);
+ } else {
+ /*
+ * The timeout isn't specified, the 2ms used here is based on
+ * experiment.
+ * FIXME: Waiting for the request completion could be delayed
+ * until the next PCODE request based on BSpec.
+ */
+ ret = sandybridge_pcode_write_timeout(dev_priv,
+ HSW_PCODE_DE_WRITE_FREQ_REQ,
+ cdclk_state->voltage_level,
+ 150, 2);
+ }
+
if (ret) {
DRM_ERROR("PCode CDCLK freq set failed, (err %d, freq %d)\n",
ret, cdclk);
@@ -1532,6 +1605,13 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
}
intel_update_cdclk(dev_priv);
+
+ if (INTEL_GEN(dev_priv) >= 10)
+ /*
+ * Can't read out the voltage level :(
+ * Let's just assume everything is as expected.
+ */
+ dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
}
static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
@@ -1617,115 +1697,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
-{
- u32 val;
-
- val = I915_READ(BXT_DE_PLL_ENABLE);
- val &= ~BXT_DE_PLL_PLL_ENABLE;
- I915_WRITE(BXT_DE_PLL_ENABLE, val);
-
- /* Timeout 200us */
- if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) == 0, 1))
- DRM_ERROR("timeout waiting for CDCLK PLL unlock\n");
-
- dev_priv->cdclk.hw.vco = 0;
-}
-
-static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
-{
- int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk.hw.ref);
- u32 val;
-
- val = CNL_CDCLK_PLL_RATIO(ratio);
- I915_WRITE(BXT_DE_PLL_ENABLE, val);
-
- val |= BXT_DE_PLL_PLL_ENABLE;
- I915_WRITE(BXT_DE_PLL_ENABLE, val);
-
- /* Timeout 200us */
- if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) != 0, 1))
- DRM_ERROR("timeout waiting for CDCLK PLL lock\n");
-
- dev_priv->cdclk.hw.vco = vco;
-}
-
-static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
- const struct intel_cdclk_state *cdclk_state,
- enum pipe pipe)
-{
- int cdclk = cdclk_state->cdclk;
- int vco = cdclk_state->vco;
- u32 val, divider;
- int ret;
-
- 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);
- if (ret) {
- DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
- ret);
- return;
- }
-
- /* cdclk = vco / 2 / div{1,2} */
- switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
- default:
- WARN_ON(cdclk != dev_priv->cdclk.hw.bypass);
- WARN_ON(vco != 0);
- /* fall through */
- case 2:
- divider = BXT_CDCLK_CD2X_DIV_SEL_1;
- break;
- case 4:
- divider = BXT_CDCLK_CD2X_DIV_SEL_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);
-
- val = divider | skl_cdclk_decimal(cdclk);
-
- if (INTEL_GEN(dev_priv) >= 12) {
- if (pipe == INVALID_PIPE)
- val |= TGL_CDCLK_CD2X_PIPE_NONE;
- else
- val |= TGL_CDCLK_CD2X_PIPE(pipe);
- } else if (INTEL_GEN(dev_priv) >= 11) {
- if (pipe == INVALID_PIPE)
- val |= ICL_CDCLK_CD2X_PIPE_NONE;
- else
- val |= ICL_CDCLK_CD2X_PIPE(pipe);
- } else {
- if (pipe == INVALID_PIPE)
- val |= BXT_CDCLK_CD2X_PIPE_NONE;
- else
- val |= BXT_CDCLK_CD2X_PIPE(pipe);
- }
- I915_WRITE(CDCLK_CTL, val);
-
- if (pipe != INVALID_PIPE)
- intel_wait_for_vblank(dev_priv, pipe);
-
- /* inform PCU of the change */
- sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
- cdclk_state->voltage_level);
-
- intel_update_cdclk(dev_priv);
-
- /*
- * Can't read out the voltage level :(
- * Let's just assume everything is as expected.
- */
- dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
-}
-
static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
{
u32 cdctl, expected;
@@ -1806,7 +1777,7 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
sanitized_state.voltage_level =
icl_calc_voltage_level(sanitized_state.cdclk);
- cnl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
+ bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
@@ -1822,7 +1793,7 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.voltage_level =
icl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
@@ -1841,7 +1812,7 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
@@ -1852,7 +1823,7 @@ static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.vco = 0;
cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
- cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
+ bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
/**
@@ -2655,12 +2626,12 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
{
if (INTEL_GEN(dev_priv) >= 11) {
- dev_priv->display.set_cdclk = cnl_set_cdclk;
+ dev_priv->display.set_cdclk = bxt_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
dev_priv->cdclk.table = icl_cdclk_table;
dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
} else if (IS_CANNONLAKE(dev_priv)) {
- dev_priv->display.set_cdclk = cnl_set_cdclk;
+ dev_priv->display.set_cdclk = bxt_set_cdclk;
dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
dev_priv->cdclk.table = cnl_cdclk_table;
dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
2019-09-07 0:21 ` [PATCH 3/8] drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk Matt Roper
@ 2019-09-10 12:35 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:35 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:38PM -0700, Matt Roper wrote:
> We'd previously combined ICL/TGL logic into the cnl_set_cdclk function,
> but BXT is pretty similar as well. Roll the cnl/icl/tgl logic back into
> the bxt function; the only things we really need to handle separately
> are punit notification and calling different functions to enable/disable
> the cdclk PLL.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Not sure if all the ifs are getting out of hand or not. But I guess
it's still legible.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 267 +++++++++------------
> 1 file changed, 119 insertions(+), 148 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 8ac31f8775f0..6b5b1328a3fa 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1449,6 +1449,39 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> dev_priv->cdclk.hw.vco = vco;
> }
>
> +static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
> +{
> + u32 val;
> +
> + val = I915_READ(BXT_DE_PLL_ENABLE);
> + val &= ~BXT_DE_PLL_PLL_ENABLE;
> + I915_WRITE(BXT_DE_PLL_ENABLE, val);
> +
> + /* Timeout 200us */
> + if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) == 0, 1))
> + DRM_ERROR("timeout waiting for CDCLK PLL unlock\n");
> +
> + dev_priv->cdclk.hw.vco = 0;
> +}
> +
> +static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
> +{
> + int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk.hw.ref);
> + u32 val;
> +
> + val = CNL_CDCLK_PLL_RATIO(ratio);
> + I915_WRITE(BXT_DE_PLL_ENABLE, val);
> +
> + val |= BXT_DE_PLL_PLL_ENABLE;
> + I915_WRITE(BXT_DE_PLL_ENABLE, val);
> +
> + /* Timeout 200us */
> + if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) != 0, 1))
> + DRM_ERROR("timeout waiting for CDCLK PLL lock\n");
> +
> + dev_priv->cdclk.hw.vco = vco;
> +}
> +
> static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> const struct intel_cdclk_state *cdclk_state,
> enum pipe pipe)
> @@ -1458,6 +1491,27 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> u32 val, divider;
> int ret;
>
> + /* Inform power controller of upcoming frequency change. */
> + if (INTEL_GEN(dev_priv) >= 10)
> + 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);
> + else
> + /*
> + * BSpec requires us to wait up to 150usec, but that leads to
> + * timeouts; the 2ms used here is based on experiment.
> + */
> + ret = sandybridge_pcode_write_timeout(dev_priv,
> + HSW_PCODE_DE_WRITE_FREQ_REQ,
> + 0x80000000, 150, 2);
> +
> + if (ret) {
> + DRM_ERROR("Failed to inform PCU about cdclk change (err %d, freq %d)\n",
> + ret, cdclk);
> + return;
> + }
> +
> /* cdclk = vco / 2 / div{1,1.5,2,4} */
> switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
> default:
> @@ -1468,63 +1522,82 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> break;
> case 3:
> - WARN(IS_GEMINILAKE(dev_priv), "Unsupported divider\n");
> + WARN(IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10,
> + "Unsupported divider\n");
> divider = BXT_CDCLK_CD2X_DIV_SEL_1_5;
> break;
> case 4:
> divider = BXT_CDCLK_CD2X_DIV_SEL_2;
> break;
> case 8:
> + WARN(INTEL_GEN(dev_priv) >= 10, "Unsupported divider\n");
> divider = BXT_CDCLK_CD2X_DIV_SEL_4;
> break;
> }
>
> - /*
> - * Inform power controller of upcoming frequency change. BSpec
> - * requires us to wait up to 150usec, but that leads to timeouts;
> - * the 2ms used here is based on experiment.
> - */
> - ret = sandybridge_pcode_write_timeout(dev_priv,
> - HSW_PCODE_DE_WRITE_FREQ_REQ,
> - 0x80000000, 150, 2);
> - if (ret) {
> - DRM_ERROR("PCode CDCLK freq change notify failed (err %d, freq %d)\n",
> - ret, cdclk);
> - return;
> - }
> + if (INTEL_GEN(dev_priv) >= 10) {
> + 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 != 0 &&
> - dev_priv->cdclk.hw.vco != vco)
> - bxt_de_pll_disable(dev_priv);
> + if (dev_priv->cdclk.hw.vco != vco)
> + cnl_cdclk_pll_enable(dev_priv, vco);
>
> - if (dev_priv->cdclk.hw.vco != vco)
> - bxt_de_pll_enable(dev_priv, vco);
> + } else {
> + if (dev_priv->cdclk.hw.vco != 0 &&
> + dev_priv->cdclk.hw.vco != vco)
> + bxt_de_pll_disable(dev_priv);
> +
> + if (dev_priv->cdclk.hw.vco != vco)
> + bxt_de_pll_enable(dev_priv, vco);
> + }
>
> val = divider | skl_cdclk_decimal(cdclk);
> - if (pipe == INVALID_PIPE)
> - val |= BXT_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= BXT_CDCLK_CD2X_PIPE(pipe);
> +
> + if (INTEL_GEN(dev_priv) >= 12) {
> + if (pipe == INVALID_PIPE)
> + val |= TGL_CDCLK_CD2X_PIPE_NONE;
> + else
> + val |= TGL_CDCLK_CD2X_PIPE(pipe);
> + } else if (INTEL_GEN(dev_priv) >= 11) {
> + if (pipe == INVALID_PIPE)
> + val |= ICL_CDCLK_CD2X_PIPE_NONE;
> + else
> + val |= ICL_CDCLK_CD2X_PIPE(pipe);
> + } else {
> + if (pipe == INVALID_PIPE)
> + val |= BXT_CDCLK_CD2X_PIPE_NONE;
> + else
> + val |= BXT_CDCLK_CD2X_PIPE(pipe);
> + }
> +
> /*
> * Disable SSA Precharge when CD clock frequency < 500 MHz,
> * enable otherwise.
> */
> - if (cdclk >= 500000)
> + if (IS_GEN9_LP(dev_priv) && cdclk >= 500000)
> val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> I915_WRITE(CDCLK_CTL, val);
>
> if (pipe != INVALID_PIPE)
> intel_wait_for_vblank(dev_priv, pipe);
>
> - /*
> - * The timeout isn't specified, the 2ms used here is based on
> - * experiment.
> - * FIXME: Waiting for the request completion could be delayed until
> - * the next PCODE request based on BSpec.
> - */
> - ret = sandybridge_pcode_write_timeout(dev_priv,
> - HSW_PCODE_DE_WRITE_FREQ_REQ,
> - cdclk_state->voltage_level, 150, 2);
> + if (INTEL_GEN(dev_priv) >= 10) {
> + ret = sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> + cdclk_state->voltage_level);
> + } else {
> + /*
> + * The timeout isn't specified, the 2ms used here is based on
> + * experiment.
> + * FIXME: Waiting for the request completion could be delayed
> + * until the next PCODE request based on BSpec.
> + */
> + ret = sandybridge_pcode_write_timeout(dev_priv,
> + HSW_PCODE_DE_WRITE_FREQ_REQ,
> + cdclk_state->voltage_level,
> + 150, 2);
> + }
> +
> if (ret) {
> DRM_ERROR("PCode CDCLK freq set failed, (err %d, freq %d)\n",
> ret, cdclk);
> @@ -1532,6 +1605,13 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> }
>
> intel_update_cdclk(dev_priv);
> +
> + if (INTEL_GEN(dev_priv) >= 10)
> + /*
> + * Can't read out the voltage level :(
> + * Let's just assume everything is as expected.
> + */
> + dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
> }
>
> static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> @@ -1617,115 +1697,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static void cnl_cdclk_pll_disable(struct drm_i915_private *dev_priv)
> -{
> - u32 val;
> -
> - val = I915_READ(BXT_DE_PLL_ENABLE);
> - val &= ~BXT_DE_PLL_PLL_ENABLE;
> - I915_WRITE(BXT_DE_PLL_ENABLE, val);
> -
> - /* Timeout 200us */
> - if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) == 0, 1))
> - DRM_ERROR("timeout waiting for CDCLK PLL unlock\n");
> -
> - dev_priv->cdclk.hw.vco = 0;
> -}
> -
> -static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
> -{
> - int ratio = DIV_ROUND_CLOSEST(vco, dev_priv->cdclk.hw.ref);
> - u32 val;
> -
> - val = CNL_CDCLK_PLL_RATIO(ratio);
> - I915_WRITE(BXT_DE_PLL_ENABLE, val);
> -
> - val |= BXT_DE_PLL_PLL_ENABLE;
> - I915_WRITE(BXT_DE_PLL_ENABLE, val);
> -
> - /* Timeout 200us */
> - if (wait_for((I915_READ(BXT_DE_PLL_ENABLE) & BXT_DE_PLL_LOCK) != 0, 1))
> - DRM_ERROR("timeout waiting for CDCLK PLL lock\n");
> -
> - dev_priv->cdclk.hw.vco = vco;
> -}
> -
> -static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> - const struct intel_cdclk_state *cdclk_state,
> - enum pipe pipe)
> -{
> - int cdclk = cdclk_state->cdclk;
> - int vco = cdclk_state->vco;
> - u32 val, divider;
> - int ret;
> -
> - 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);
> - if (ret) {
> - DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
> - ret);
> - return;
> - }
> -
> - /* cdclk = vco / 2 / div{1,2} */
> - switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
> - default:
> - WARN_ON(cdclk != dev_priv->cdclk.hw.bypass);
> - WARN_ON(vco != 0);
> - /* fall through */
> - case 2:
> - divider = BXT_CDCLK_CD2X_DIV_SEL_1;
> - break;
> - case 4:
> - divider = BXT_CDCLK_CD2X_DIV_SEL_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);
> -
> - val = divider | skl_cdclk_decimal(cdclk);
> -
> - if (INTEL_GEN(dev_priv) >= 12) {
> - if (pipe == INVALID_PIPE)
> - val |= TGL_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= TGL_CDCLK_CD2X_PIPE(pipe);
> - } else if (INTEL_GEN(dev_priv) >= 11) {
> - if (pipe == INVALID_PIPE)
> - val |= ICL_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= ICL_CDCLK_CD2X_PIPE(pipe);
> - } else {
> - if (pipe == INVALID_PIPE)
> - val |= BXT_CDCLK_CD2X_PIPE_NONE;
> - else
> - val |= BXT_CDCLK_CD2X_PIPE(pipe);
> - }
> - I915_WRITE(CDCLK_CTL, val);
> -
> - if (pipe != INVALID_PIPE)
> - intel_wait_for_vblank(dev_priv, pipe);
> -
> - /* inform PCU of the change */
> - sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> - cdclk_state->voltage_level);
> -
> - intel_update_cdclk(dev_priv);
> -
> - /*
> - * Can't read out the voltage level :(
> - * Let's just assume everything is as expected.
> - */
> - dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
> -}
> -
> static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> {
> u32 cdctl, expected;
> @@ -1806,7 +1777,7 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> sanitized_state.voltage_level =
> icl_calc_voltage_level(sanitized_state.cdclk);
>
> - cnl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> + bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> }
>
> static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> @@ -1822,7 +1793,7 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> cdclk_state.voltage_level =
> icl_calc_voltage_level(cdclk_state.cdclk);
>
> - cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> + bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> @@ -1841,7 +1812,7 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>
> - cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> + bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> @@ -1852,7 +1823,7 @@ static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> cdclk_state.vco = 0;
> cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>
> - cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> + bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> /**
> @@ -2655,12 +2626,12 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> {
> if (INTEL_GEN(dev_priv) >= 11) {
> - dev_priv->display.set_cdclk = cnl_set_cdclk;
> + dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> dev_priv->cdclk.table = icl_cdclk_table;
> dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
> } else if (IS_CANNONLAKE(dev_priv)) {
> - dev_priv->display.set_cdclk = cnl_set_cdclk;
> + dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
> dev_priv->cdclk.table = cnl_cdclk_table;
> dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/8] drm/i915: Kill cnl_sanitize_cdclk()
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (2 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 3/8] drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:37 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk Matt Roper
` (9 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
The CNL variant of this function is identical to the BXT variant aside
from not needing to handle SSA precharge.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 46 +---------------------
1 file changed, 2 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 6b5b1328a3fa..f8c2a706990b 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1645,7 +1645,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
* Disable SSA Precharge when CD clock frequency < 500 MHz,
* enable otherwise.
*/
- if (dev_priv->cdclk.hw.cdclk >= 500000)
+ if (IS_GEN9_LP(dev_priv) && dev_priv->cdclk.hw.cdclk >= 500000)
expected |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
if (cdctl == expected)
@@ -1697,48 +1697,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
-{
- u32 cdctl, expected;
-
- intel_update_cdclk(dev_priv);
- intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
-
- if (dev_priv->cdclk.hw.vco == 0 ||
- dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.bypass)
- goto sanitize;
-
- /* DPLL okay; verify the cdclock
- *
- * Some BIOS versions leave an incorrect decimal frequency value and
- * set reserved MBZ bits in CDCLK_CTL at least during exiting from S4,
- * so sanitize this register.
- */
- cdctl = I915_READ(CDCLK_CTL);
- /*
- * Let's ignore the pipe field, since BIOS could have configured the
- * dividers both synching to an active pipe, or asynchronously
- * (PIPE_NONE).
- */
- cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
-
- expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
- skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
-
- if (cdctl == expected)
- /* All well; nothing to sanitize */
- return;
-
-sanitize:
- DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
-
- /* force cdclk programming */
- dev_priv->cdclk.hw.cdclk = 0;
-
- /* force full PLL disable + enable */
- dev_priv->cdclk.hw.vco = -1;
-}
-
static void icl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state sanitized_state;
@@ -1800,7 +1758,7 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state cdclk_state;
- cnl_sanitize_cdclk(dev_priv);
+ bxt_sanitize_cdclk(dev_priv);
if (dev_priv->cdclk.hw.cdclk != 0 &&
dev_priv->cdclk.hw.vco != 0)
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 4/8] drm/i915: Kill cnl_sanitize_cdclk()
2019-09-07 0:21 ` [PATCH 4/8] drm/i915: Kill cnl_sanitize_cdclk() Matt Roper
@ 2019-09-10 12:37 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:37 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:39PM -0700, Matt Roper wrote:
> The CNL variant of this function is identical to the BXT variant aside
> from not needing to handle SSA precharge.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 46 +---------------------
> 1 file changed, 2 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 6b5b1328a3fa..f8c2a706990b 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1645,7 +1645,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> * Disable SSA Precharge when CD clock frequency < 500 MHz,
> * enable otherwise.
> */
> - if (dev_priv->cdclk.hw.cdclk >= 500000)
> + if (IS_GEN9_LP(dev_priv) && dev_priv->cdclk.hw.cdclk >= 500000)
> expected |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>
> if (cdctl == expected)
> @@ -1697,48 +1697,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> -{
> - u32 cdctl, expected;
> -
> - intel_update_cdclk(dev_priv);
> - intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
> -
> - if (dev_priv->cdclk.hw.vco == 0 ||
> - dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.bypass)
> - goto sanitize;
> -
> - /* DPLL okay; verify the cdclock
> - *
> - * Some BIOS versions leave an incorrect decimal frequency value and
> - * set reserved MBZ bits in CDCLK_CTL at least during exiting from S4,
> - * so sanitize this register.
> - */
> - cdctl = I915_READ(CDCLK_CTL);
> - /*
> - * Let's ignore the pipe field, since BIOS could have configured the
> - * dividers both synching to an active pipe, or asynchronously
> - * (PIPE_NONE).
> - */
> - cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
> -
> - expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
> - skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
> -
> - if (cdctl == expected)
> - /* All well; nothing to sanitize */
> - return;
> -
> -sanitize:
> - DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> -
> - /* force cdclk programming */
> - dev_priv->cdclk.hw.cdclk = 0;
> -
> - /* force full PLL disable + enable */
> - dev_priv->cdclk.hw.vco = -1;
> -}
> -
> static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state sanitized_state;
> @@ -1800,7 +1758,7 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state cdclk_state;
>
> - cnl_sanitize_cdclk(dev_priv);
> + bxt_sanitize_cdclk(dev_priv);
>
> if (dev_priv->cdclk.hw.cdclk != 0 &&
> dev_priv->cdclk.hw.vco != 0)
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (3 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 4/8] drm/i915: Kill cnl_sanitize_cdclk() Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:39 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 6/8] drm/i915: Add calc_voltage_level display vfunc Matt Roper
` (8 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
The uninitialize flow is the same on all of these platforms, aside from
calculating a different frequency level.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 48 +++++++---------------
1 file changed, 14 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index f8c2a706990b..a70fec82d2bc 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1692,7 +1692,18 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = cdclk_state.bypass;
cdclk_state.vco = 0;
- cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
+ if (IS_ELKHARTLAKE(dev_priv))
+ cdclk_state.voltage_level =
+ ehl_calc_voltage_level(cdclk_state.cdclk);
+ else if (INTEL_GEN(dev_priv) >= 11)
+ cdclk_state.voltage_level =
+ icl_calc_voltage_level(cdclk_state.cdclk);
+ else if (INTEL_GEN(dev_priv) >= 10)
+ cdclk_state.voltage_level =
+ cnl_calc_voltage_level(cdclk_state.cdclk);
+ else
+ cdclk_state.voltage_level =
+ bxt_calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
@@ -1738,22 +1749,6 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
-static 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.bypass;
- cdclk_state.vco = 0;
- if (IS_ELKHARTLAKE(dev_priv))
- cdclk_state.voltage_level =
- ehl_calc_voltage_level(cdclk_state.cdclk);
- else
- cdclk_state.voltage_level =
- icl_calc_voltage_level(cdclk_state.cdclk);
-
- bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
-}
-
static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
{
struct intel_cdclk_state cdclk_state;
@@ -1773,17 +1768,6 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
-{
- struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
-
- cdclk_state.cdclk = cdclk_state.bypass;
- cdclk_state.vco = 0;
- cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
-
- bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
-}
-
/**
* intel_cdclk_init - Initialize CDCLK
* @i915: i915 device
@@ -1814,14 +1798,10 @@ void intel_cdclk_init(struct drm_i915_private *i915)
*/
void intel_cdclk_uninit(struct drm_i915_private *i915)
{
- if (INTEL_GEN(i915) >= 11)
- icl_uninit_cdclk(i915);
- else if (IS_CANNONLAKE(i915))
- cnl_uninit_cdclk(i915);
+ if (IS_GEN9_LP(i915) || INTEL_GEN(i915) >= 10)
+ bxt_uninit_cdclk(i915);
else if (IS_GEN9_BC(i915))
skl_uninit_cdclk(i915);
- else if (IS_GEN9_LP(i915))
- bxt_uninit_cdclk(i915);
}
/**
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
2019-09-07 0:21 ` [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk Matt Roper
@ 2019-09-10 12:39 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:39 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:40PM -0700, Matt Roper wrote:
> The uninitialize flow is the same on all of these platforms, aside from
> calculating a different frequency level.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 48 +++++++---------------
> 1 file changed, 14 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index f8c2a706990b..a70fec82d2bc 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1692,7 +1692,18 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
>
> cdclk_state.cdclk = cdclk_state.bypass;
> cdclk_state.vco = 0;
> - cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> + if (IS_ELKHARTLAKE(dev_priv))
> + cdclk_state.voltage_level =
> + ehl_calc_voltage_level(cdclk_state.cdclk);
> + else if (INTEL_GEN(dev_priv) >= 11)
> + cdclk_state.voltage_level =
> + icl_calc_voltage_level(cdclk_state.cdclk);
> + else if (INTEL_GEN(dev_priv) >= 10)
> + cdclk_state.voltage_level =
> + cnl_calc_voltage_level(cdclk_state.cdclk);
> + else
> + cdclk_state.voltage_level =
> + bxt_calc_voltage_level(cdclk_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
> @@ -1738,22 +1749,6 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> }
>
> -static 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.bypass;
> - cdclk_state.vco = 0;
> - if (IS_ELKHARTLAKE(dev_priv))
> - cdclk_state.voltage_level =
> - ehl_calc_voltage_level(cdclk_state.cdclk);
> - else
> - cdclk_state.voltage_level =
> - icl_calc_voltage_level(cdclk_state.cdclk);
> -
> - bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> -}
> -
> static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> {
> struct intel_cdclk_state cdclk_state;
> @@ -1773,17 +1768,6 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> -{
> - struct intel_cdclk_state cdclk_state = dev_priv->cdclk.hw;
> -
> - cdclk_state.cdclk = cdclk_state.bypass;
> - cdclk_state.vco = 0;
> - cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> -
> - bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> -}
> -
> /**
> * intel_cdclk_init - Initialize CDCLK
> * @i915: i915 device
> @@ -1814,14 +1798,10 @@ void intel_cdclk_init(struct drm_i915_private *i915)
> */
> void intel_cdclk_uninit(struct drm_i915_private *i915)
> {
> - if (INTEL_GEN(i915) >= 11)
> - icl_uninit_cdclk(i915);
> - else if (IS_CANNONLAKE(i915))
> - cnl_uninit_cdclk(i915);
> + if (IS_GEN9_LP(i915) || INTEL_GEN(i915) >= 10)
I think the general concensus has been to list platforms
from new to old. Might be good to stick to that even
within conditions like this. There may have been other cases like this
in these patches that I didn't spot.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> + bxt_uninit_cdclk(i915);
> else if (IS_GEN9_BC(i915))
> skl_uninit_cdclk(i915);
> - else if (IS_GEN9_LP(i915))
> - bxt_uninit_cdclk(i915);
> }
>
> /**
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/8] drm/i915: Add calc_voltage_level display vfunc
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (4 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 5/8] drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:41 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 7/8] drm/i915: Enhance cdclk sanitization Matt Roper
` (7 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
With all of the cdclk function consolidation, we can cut down on a lot
of platform if/else logic by creating a vfunc that's initialized at
startup.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 76 ++++++++--------------
drivers/gpu/drm/i915/i915_drv.h | 1 +
2 files changed, 28 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index a70fec82d2bc..a6696697a09f 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1403,18 +1403,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
* Can't read this out :( Let's assume it's
* at least what the CDCLK frequency requires.
*/
- if (IS_ELKHARTLAKE(dev_priv))
- cdclk_state->voltage_level =
- ehl_calc_voltage_level(cdclk_state->cdclk);
- else if (INTEL_GEN(dev_priv) >= 11)
- cdclk_state->voltage_level =
- icl_calc_voltage_level(cdclk_state->cdclk);
- else if (INTEL_GEN(dev_priv) >= 10)
- cdclk_state->voltage_level =
- cnl_calc_voltage_level(cdclk_state->cdclk);
- else
- cdclk_state->voltage_level =
- bxt_calc_voltage_level(cdclk_state->cdclk);
+ cdclk_state->voltage_level =
+ dev_priv->display.calc_voltage_level(cdclk_state->cdclk);
}
static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
@@ -1681,7 +1671,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
*/
cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
- cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
+ cdclk_state.voltage_level =
+ dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
@@ -1692,18 +1683,8 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = cdclk_state.bypass;
cdclk_state.vco = 0;
- if (IS_ELKHARTLAKE(dev_priv))
- cdclk_state.voltage_level =
- ehl_calc_voltage_level(cdclk_state.cdclk);
- else if (INTEL_GEN(dev_priv) >= 11)
- cdclk_state.voltage_level =
- icl_calc_voltage_level(cdclk_state.cdclk);
- else if (INTEL_GEN(dev_priv) >= 10)
- cdclk_state.voltage_level =
- cnl_calc_voltage_level(cdclk_state.cdclk);
- else
- cdclk_state.voltage_level =
- bxt_calc_voltage_level(cdclk_state.cdclk);
+ cdclk_state.voltage_level =
+ dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
@@ -1739,12 +1720,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
sanitized_state.cdclk);
- if (IS_ELKHARTLAKE(dev_priv))
- sanitized_state.voltage_level =
- ehl_calc_voltage_level(sanitized_state.cdclk);
- else
- sanitized_state.voltage_level =
- icl_calc_voltage_level(sanitized_state.cdclk);
+ sanitized_state.voltage_level =
+ dev_priv->display.calc_voltage_level(sanitized_state.cdclk);
bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
}
@@ -1763,7 +1740,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
- cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
+ cdclk_state.voltage_level =
+ dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
@@ -2255,7 +2233,7 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
state->cdclk.logical.voltage_level =
- bxt_calc_voltage_level(cdclk);
+ dev_priv->display.calc_voltage_level(cdclk);
if (!state->active_pipes) {
cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
@@ -2264,7 +2242,7 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
state->cdclk.actual.voltage_level =
- bxt_calc_voltage_level(cdclk);
+ dev_priv->display.calc_voltage_level(cdclk);
} else {
state->cdclk.actual = state->cdclk.logical;
}
@@ -2319,14 +2297,9 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.logical.vco = vco;
state->cdclk.logical.cdclk = cdclk;
- if (IS_ELKHARTLAKE(dev_priv))
- state->cdclk.logical.voltage_level =
- max(ehl_calc_voltage_level(cdclk),
- cnl_compute_min_voltage_level(state));
- else
- state->cdclk.logical.voltage_level =
- max(icl_calc_voltage_level(cdclk),
- cnl_compute_min_voltage_level(state));
+ state->cdclk.logical.voltage_level =
+ max(dev_priv->display.calc_voltage_level(cdclk),
+ cnl_compute_min_voltage_level(state));
if (!state->active_pipes) {
cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
@@ -2334,12 +2307,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
state->cdclk.actual.vco = vco;
state->cdclk.actual.cdclk = cdclk;
- if (IS_ELKHARTLAKE(dev_priv))
- state->cdclk.actual.voltage_level =
- ehl_calc_voltage_level(cdclk);
- else
- state->cdclk.actual.voltage_level =
- icl_calc_voltage_level(cdclk);
+ state->cdclk.actual.voltage_level =
+ dev_priv->display.calc_voltage_level(cdclk);
} else {
state->cdclk.actual = state->cdclk.logical;
}
@@ -2563,19 +2532,28 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
*/
void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
{
- if (INTEL_GEN(dev_priv) >= 11) {
+ if (IS_ELKHARTLAKE(dev_priv)) {
+ dev_priv->display.set_cdclk = bxt_set_cdclk;
+ dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+ dev_priv->display.calc_voltage_level = ehl_calc_voltage_level;
+ dev_priv->cdclk.table = icl_cdclk_table;
+ dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
+ } else if (INTEL_GEN(dev_priv) >= 11) {
dev_priv->display.set_cdclk = bxt_set_cdclk;
dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
+ dev_priv->display.calc_voltage_level = icl_calc_voltage_level;
dev_priv->cdclk.table = icl_cdclk_table;
dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
} else if (IS_CANNONLAKE(dev_priv)) {
dev_priv->display.set_cdclk = bxt_set_cdclk;
dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
+ dev_priv->display.calc_voltage_level = cnl_calc_voltage_level;
dev_priv->cdclk.table = cnl_cdclk_table;
dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
} 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.calc_voltage_level = bxt_calc_voltage_level;
dev_priv->cdclk.table = bxt_cdclk_table;
dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
} else if (IS_GEN9_BC(dev_priv)) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a4d249193dc8..a56da431bfe3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -276,6 +276,7 @@ struct drm_i915_display_funcs {
int (*compute_global_watermarks)(struct intel_atomic_state *state);
void (*update_wm)(struct intel_crtc *crtc);
int (*modeset_calc_cdclk)(struct intel_atomic_state *state);
+ u8 (*calc_voltage_level)(int cdclk);
/* Returns the active state of the crtc, and if the crtc is active,
* fills out the pipe-config with the hw state. */
bool (*get_pipe_config)(struct intel_crtc *,
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 6/8] drm/i915: Add calc_voltage_level display vfunc
2019-09-07 0:21 ` [PATCH 6/8] drm/i915: Add calc_voltage_level display vfunc Matt Roper
@ 2019-09-10 12:41 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:41 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:41PM -0700, Matt Roper wrote:
> With all of the cdclk function consolidation, we can cut down on a lot
> of platform if/else logic by creating a vfunc that's initialized at
> startup.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 76 ++++++++--------------
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> 2 files changed, 28 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a70fec82d2bc..a6696697a09f 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1403,18 +1403,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> * Can't read this out :( Let's assume it's
> * at least what the CDCLK frequency requires.
> */
> - if (IS_ELKHARTLAKE(dev_priv))
> - cdclk_state->voltage_level =
> - ehl_calc_voltage_level(cdclk_state->cdclk);
> - else if (INTEL_GEN(dev_priv) >= 11)
> - cdclk_state->voltage_level =
> - icl_calc_voltage_level(cdclk_state->cdclk);
> - else if (INTEL_GEN(dev_priv) >= 10)
> - cdclk_state->voltage_level =
> - cnl_calc_voltage_level(cdclk_state->cdclk);
> - else
> - cdclk_state->voltage_level =
> - bxt_calc_voltage_level(cdclk_state->cdclk);
> + cdclk_state->voltage_level =
> + dev_priv->display.calc_voltage_level(cdclk_state->cdclk);
> }
>
> static void bxt_de_pll_disable(struct drm_i915_private *dev_priv)
> @@ -1681,7 +1671,8 @@ static void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> */
> cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
> cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> - cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> + cdclk_state.voltage_level =
> + dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
> @@ -1692,18 +1683,8 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
>
> cdclk_state.cdclk = cdclk_state.bypass;
> cdclk_state.vco = 0;
> - if (IS_ELKHARTLAKE(dev_priv))
> - cdclk_state.voltage_level =
> - ehl_calc_voltage_level(cdclk_state.cdclk);
> - else if (INTEL_GEN(dev_priv) >= 11)
> - cdclk_state.voltage_level =
> - icl_calc_voltage_level(cdclk_state.cdclk);
> - else if (INTEL_GEN(dev_priv) >= 10)
> - cdclk_state.voltage_level =
> - cnl_calc_voltage_level(cdclk_state.cdclk);
> - else
> - cdclk_state.voltage_level =
> - bxt_calc_voltage_level(cdclk_state.cdclk);
> + cdclk_state.voltage_level =
> + dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
> @@ -1739,12 +1720,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
> sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
> sanitized_state.cdclk);
> - if (IS_ELKHARTLAKE(dev_priv))
> - sanitized_state.voltage_level =
> - ehl_calc_voltage_level(sanitized_state.cdclk);
> - else
> - sanitized_state.voltage_level =
> - icl_calc_voltage_level(sanitized_state.cdclk);
> + sanitized_state.voltage_level =
> + dev_priv->display.calc_voltage_level(sanitized_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> }
> @@ -1763,7 +1740,8 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>
> cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
> cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> - cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> + cdclk_state.voltage_level =
> + dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
>
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
> @@ -2255,7 +2233,7 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> state->cdclk.logical.voltage_level =
> - bxt_calc_voltage_level(cdclk);
> + dev_priv->display.calc_voltage_level(cdclk);
>
> if (!state->active_pipes) {
> cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
> @@ -2264,7 +2242,7 @@ static int bxt_modeset_calc_cdclk(struct intel_atomic_state *state)
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> state->cdclk.actual.voltage_level =
> - bxt_calc_voltage_level(cdclk);
> + dev_priv->display.calc_voltage_level(cdclk);
> } else {
> state->cdclk.actual = state->cdclk.logical;
> }
> @@ -2319,14 +2297,9 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>
> state->cdclk.logical.vco = vco;
> state->cdclk.logical.cdclk = cdclk;
> - if (IS_ELKHARTLAKE(dev_priv))
> - state->cdclk.logical.voltage_level =
> - max(ehl_calc_voltage_level(cdclk),
> - cnl_compute_min_voltage_level(state));
> - else
> - state->cdclk.logical.voltage_level =
> - max(icl_calc_voltage_level(cdclk),
> - cnl_compute_min_voltage_level(state));
> + state->cdclk.logical.voltage_level =
> + max(dev_priv->display.calc_voltage_level(cdclk),
> + cnl_compute_min_voltage_level(state));
>
> if (!state->active_pipes) {
> cdclk = calc_cdclk(dev_priv, state->cdclk.force_min_cdclk);
> @@ -2334,12 +2307,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>
> state->cdclk.actual.vco = vco;
> state->cdclk.actual.cdclk = cdclk;
> - if (IS_ELKHARTLAKE(dev_priv))
> - state->cdclk.actual.voltage_level =
> - ehl_calc_voltage_level(cdclk);
> - else
> - state->cdclk.actual.voltage_level =
> - icl_calc_voltage_level(cdclk);
> + state->cdclk.actual.voltage_level =
> + dev_priv->display.calc_voltage_level(cdclk);
> } else {
> state->cdclk.actual = state->cdclk.logical;
> }
> @@ -2563,19 +2532,28 @@ void intel_update_rawclk(struct drm_i915_private *dev_priv)
> */
> void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
> {
> - if (INTEL_GEN(dev_priv) >= 11) {
> + if (IS_ELKHARTLAKE(dev_priv)) {
> + dev_priv->display.set_cdclk = bxt_set_cdclk;
> + dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> + dev_priv->display.calc_voltage_level = ehl_calc_voltage_level;
> + dev_priv->cdclk.table = icl_cdclk_table;
> + dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
> + } else if (INTEL_GEN(dev_priv) >= 11) {
> dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = icl_modeset_calc_cdclk;
> + dev_priv->display.calc_voltage_level = icl_calc_voltage_level;
> dev_priv->cdclk.table = icl_cdclk_table;
> dev_priv->cdclk.table_size = ARRAY_SIZE(icl_cdclk_table);
> } else if (IS_CANNONLAKE(dev_priv)) {
> dev_priv->display.set_cdclk = bxt_set_cdclk;
> dev_priv->display.modeset_calc_cdclk = cnl_modeset_calc_cdclk;
> + dev_priv->display.calc_voltage_level = cnl_calc_voltage_level;
> dev_priv->cdclk.table = cnl_cdclk_table;
> dev_priv->cdclk.table_size = ARRAY_SIZE(cnl_cdclk_table);
> } 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.calc_voltage_level = bxt_calc_voltage_level;
> dev_priv->cdclk.table = bxt_cdclk_table;
> dev_priv->cdclk.table_size = ARRAY_SIZE(bxt_cdclk_table);
> } else if (IS_GEN9_BC(dev_priv)) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index a4d249193dc8..a56da431bfe3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -276,6 +276,7 @@ struct drm_i915_display_funcs {
> int (*compute_global_watermarks)(struct intel_atomic_state *state);
> void (*update_wm)(struct intel_crtc *crtc);
> int (*modeset_calc_cdclk)(struct intel_atomic_state *state);
> + u8 (*calc_voltage_level)(int cdclk);
> /* Returns the active state of the crtc, and if the crtc is active,
> * fills out the pipe-config with the hw state. */
> bool (*get_pipe_config)(struct intel_crtc *,
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/8] drm/i915: Enhance cdclk sanitization
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (5 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 6/8] drm/i915: Add calc_voltage_level display vfunc Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:42 ` Ville Syrjälä
2019-09-07 0:21 ` [PATCH 8/8] drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk Matt Roper
` (6 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
When reading out the BIOS-programmed cdclk state, let's make sure that
the cdclk value is on the valid list for the platform, ensure that the
VCO matches the cdclk, and ensure that the CD2X divider was set
properly.
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 34 ++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index a6696697a09f..356495591cf9 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1607,6 +1607,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
{
u32 cdctl, expected;
+ int cdclk, vco;
intel_update_cdclk(dev_priv);
intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
@@ -1629,8 +1630,37 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
*/
cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
- expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
- skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
+ /* Make sure this is a legal cdclk value for the platform */
+ cdclk = calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk);
+ if (cdclk != dev_priv->cdclk.hw.cdclk)
+ goto sanitize;
+
+ /* Make sure the VCO is correct for the cdclk */
+ vco = calc_cdclk_pll_vco(dev_priv, cdclk);
+ if (vco != dev_priv->cdclk.hw.vco)
+ goto sanitize;
+
+ expected = skl_cdclk_decimal(cdclk);
+
+ /* Figure out what CD2X divider we should be using for this cdclk */
+ switch (DIV_ROUND_CLOSEST(dev_priv->cdclk.hw.vco,
+ dev_priv->cdclk.hw.cdclk)) {
+ case 2:
+ expected |= BXT_CDCLK_CD2X_DIV_SEL_1;
+ break;
+ case 3:
+ expected |= BXT_CDCLK_CD2X_DIV_SEL_1_5;
+ break;
+ case 4:
+ expected |= BXT_CDCLK_CD2X_DIV_SEL_2;
+ break;
+ case 8:
+ expected |= BXT_CDCLK_CD2X_DIV_SEL_4;
+ break;
+ default:
+ goto sanitize;
+ }
+
/*
* Disable SSA Precharge when CD clock frequency < 500 MHz,
* enable otherwise.
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 7/8] drm/i915: Enhance cdclk sanitization
2019-09-07 0:21 ` [PATCH 7/8] drm/i915: Enhance cdclk sanitization Matt Roper
@ 2019-09-10 12:42 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:42 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:42PM -0700, Matt Roper wrote:
> When reading out the BIOS-programmed cdclk state, let's make sure that
> the cdclk value is on the valid list for the platform, ensure that the
> VCO matches the cdclk, and ensure that the CD2X divider was set
> properly.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 34 ++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index a6696697a09f..356495591cf9 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1607,6 +1607,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> {
> u32 cdctl, expected;
> + int cdclk, vco;
>
> intel_update_cdclk(dev_priv);
> intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
> @@ -1629,8 +1630,37 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv)
> */
> cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE;
>
> - expected = (cdctl & BXT_CDCLK_CD2X_DIV_SEL_MASK) |
> - skl_cdclk_decimal(dev_priv->cdclk.hw.cdclk);
> + /* Make sure this is a legal cdclk value for the platform */
> + cdclk = calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk);
> + if (cdclk != dev_priv->cdclk.hw.cdclk)
> + goto sanitize;
> +
> + /* Make sure the VCO is correct for the cdclk */
> + vco = calc_cdclk_pll_vco(dev_priv, cdclk);
> + if (vco != dev_priv->cdclk.hw.vco)
> + goto sanitize;
> +
> + expected = skl_cdclk_decimal(cdclk);
> +
> + /* Figure out what CD2X divider we should be using for this cdclk */
> + switch (DIV_ROUND_CLOSEST(dev_priv->cdclk.hw.vco,
> + dev_priv->cdclk.hw.cdclk)) {
> + case 2:
> + expected |= BXT_CDCLK_CD2X_DIV_SEL_1;
> + break;
> + case 3:
> + expected |= BXT_CDCLK_CD2X_DIV_SEL_1_5;
> + break;
> + case 4:
> + expected |= BXT_CDCLK_CD2X_DIV_SEL_2;
> + break;
> + case 8:
> + expected |= BXT_CDCLK_CD2X_DIV_SEL_4;
> + break;
> + default:
> + goto sanitize;
> + }
> +
> /*
> * Disable SSA Precharge when CD clock frequency < 500 MHz,
> * enable otherwise.
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 8/8] drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (6 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 7/8] drm/i915: Enhance cdclk sanitization Matt Roper
@ 2019-09-07 0:21 ` Matt Roper
2019-09-10 12:44 ` Ville Syrjälä
2019-09-07 0:43 ` ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL Patchwork
` (5 subsequent siblings)
13 siblings, 1 reply; 25+ messages in thread
From: Matt Roper @ 2019-09-07 0:21 UTC (permalink / raw)
To: intel-gfx
The BXT and CNL functions were already basically identical, whereas
ICL's function tried to do its own sanitization rather than calling
bxt_sanitize_cdclk.
This should actually fix a bug in our ICL initialization where it would
consider the /2 CD2X divider invalid and force an unnecessary
sanitization (we now have valid clock frequencies that use this
divider).
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
drivers/gpu/drm/i915/display/intel_cdclk.c | 65 +---------------------
1 file changed, 2 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 356495591cf9..0ad83d67932d 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1719,63 +1719,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
}
-static void icl_init_cdclk(struct drm_i915_private *dev_priv)
-{
- struct intel_cdclk_state sanitized_state;
- u32 val;
-
- /* This sets dev_priv->cdclk.hw. */
- intel_update_cdclk(dev_priv);
- intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
-
- /* This means CDCLK disabled. */
- if (dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.bypass)
- 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");
-
- sanitized_state.ref = dev_priv->cdclk.hw.ref;
- sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
- sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
- sanitized_state.cdclk);
- sanitized_state.voltage_level =
- dev_priv->display.calc_voltage_level(sanitized_state.cdclk);
-
- bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
-}
-
-static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
-{
- struct intel_cdclk_state cdclk_state;
-
- bxt_sanitize_cdclk(dev_priv);
-
- if (dev_priv->cdclk.hw.cdclk != 0 &&
- dev_priv->cdclk.hw.vco != 0)
- return;
-
- cdclk_state = dev_priv->cdclk.hw;
-
- cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
- cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
- cdclk_state.voltage_level =
- dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
-
- bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
-}
-
/**
* intel_cdclk_init - Initialize CDCLK
* @i915: i915 device
@@ -1787,14 +1730,10 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
*/
void intel_cdclk_init(struct drm_i915_private *i915)
{
- if (INTEL_GEN(i915) >= 11)
- icl_init_cdclk(i915);
- else if (IS_CANNONLAKE(i915))
- cnl_init_cdclk(i915);
+ if (IS_GEN9_LP(i915) || INTEL_GEN(i915) >= 10)
+ bxt_init_cdclk(i915);
else if (IS_GEN9_BC(i915))
skl_init_cdclk(i915);
- else if (IS_GEN9_LP(i915))
- bxt_init_cdclk(i915);
}
/**
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 8/8] drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
2019-09-07 0:21 ` [PATCH 8/8] drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk Matt Roper
@ 2019-09-10 12:44 ` Ville Syrjälä
0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2019-09-10 12:44 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
On Fri, Sep 06, 2019 at 05:21:43PM -0700, Matt Roper wrote:
> The BXT and CNL functions were already basically identical, whereas
> ICL's function tried to do its own sanitization rather than calling
> bxt_sanitize_cdclk.
>
> This should actually fix a bug in our ICL initialization where it would
> consider the /2 CD2X divider invalid and force an unnecessary
> sanitization (we now have valid clock frequencies that use this
> divider).
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_cdclk.c | 65 +---------------------
> 1 file changed, 2 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 356495591cf9..0ad83d67932d 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1719,63 +1719,6 @@ static void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> }
>
> -static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> -{
> - struct intel_cdclk_state sanitized_state;
> - u32 val;
> -
> - /* This sets dev_priv->cdclk.hw. */
> - intel_update_cdclk(dev_priv);
> - intel_dump_cdclk_state(&dev_priv->cdclk.hw, "Current CDCLK");
> -
> - /* This means CDCLK disabled. */
> - if (dev_priv->cdclk.hw.cdclk == dev_priv->cdclk.hw.bypass)
> - 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");
> -
> - sanitized_state.ref = dev_priv->cdclk.hw.ref;
> - sanitized_state.cdclk = calc_cdclk(dev_priv, 0);
> - sanitized_state.vco = calc_cdclk_pll_vco(dev_priv,
> - sanitized_state.cdclk);
> - sanitized_state.voltage_level =
> - dev_priv->display.calc_voltage_level(sanitized_state.cdclk);
> -
> - bxt_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> -}
> -
> -static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> -{
> - struct intel_cdclk_state cdclk_state;
> -
> - bxt_sanitize_cdclk(dev_priv);
> -
> - if (dev_priv->cdclk.hw.cdclk != 0 &&
> - dev_priv->cdclk.hw.vco != 0)
> - return;
> -
> - cdclk_state = dev_priv->cdclk.hw;
> -
> - cdclk_state.cdclk = calc_cdclk(dev_priv, 0);
> - cdclk_state.vco = calc_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> - cdclk_state.voltage_level =
> - dev_priv->display.calc_voltage_level(cdclk_state.cdclk);
> -
> - bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> -}
> -
> /**
> * intel_cdclk_init - Initialize CDCLK
> * @i915: i915 device
> @@ -1787,14 +1730,10 @@ static void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> */
> void intel_cdclk_init(struct drm_i915_private *i915)
> {
> - if (INTEL_GEN(i915) >= 11)
> - icl_init_cdclk(i915);
> - else if (IS_CANNONLAKE(i915))
> - cnl_init_cdclk(i915);
> + if (IS_GEN9_LP(i915) || INTEL_GEN(i915) >= 10)
> + bxt_init_cdclk(i915);
> else if (IS_GEN9_BC(i915))
> skl_init_cdclk(i915);
> - else if (IS_GEN9_LP(i915))
> - bxt_init_cdclk(i915);
> }
>
> /**
> --
> 2.20.1
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (7 preceding siblings ...)
2019-09-07 0:21 ` [PATCH 8/8] drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk Matt Roper
@ 2019-09-07 0:43 ` Patchwork
2019-09-07 1:31 ` ✗ Fi.CI.BAT: failure " Patchwork
` (4 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-07 0:43 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL
URL : https://patchwork.freedesktop.org/series/66365/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
637dedda4030 drm/i915: Consolidate bxt/cnl/icl cdclk readout
-:72: CHECK:CAMELCASE: Avoid CamelCase: <CNL_DSSM_CDCLK_PLL_REFCLK_24MHz>
#72: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1272:
+ if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
total: 0 errors, 0 warnings, 1 checks, 412 lines checked
1ccb1492f096 drm/i915: Use literal representation of cdclk tables
4cf0293d973c drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
93848eb9fbee drm/i915: Kill cnl_sanitize_cdclk()
c3c5cd83bf04 drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
7bf020857c41 drm/i915: Add calc_voltage_level display vfunc
8d07a457135f drm/i915: Enhance cdclk sanitization
f59e1b13e248 drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread* ✗ Fi.CI.BAT: failure for cdclk consolidation and rework for BXT-TGL
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (8 preceding siblings ...)
2019-09-07 0:43 ` ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL Patchwork
@ 2019-09-07 1:31 ` Patchwork
2019-09-08 3:17 ` ✗ Fi.CI.BUILD: failure for cdclk consolidation and rework for BXT-TGL (rev2) Patchwork
` (3 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-07 1:31 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL
URL : https://patchwork.freedesktop.org/series/66365/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_6848 -> Patchwork_14308
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_14308 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_14308, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_14308:
### IGT changes ###
#### Possible regressions ####
* igt@runner@aborted:
- fi-bxt-dsi: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-bxt-dsi/igt@runner@aborted.html
- fi-apl-guc: NOTRUN -> [FAIL][2]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-apl-guc/igt@runner@aborted.html
Known issues
------------
Here are the changes found in Patchwork_14308 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6848/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
* igt@kms_chamelium@dp-crc-fast:
- fi-cml-u2: [PASS][5] -> [FAIL][6] ([fdo#110627])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6848/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: [PASS][7] -> [DMESG-WARN][8] ([fdo#102614])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6848/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
#### Possible fixes ####
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-7500u: [FAIL][9] ([fdo#111096]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6848/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
* igt@prime_vgem@basic-fence-flip:
- fi-ilk-650: [DMESG-WARN][11] ([fdo#106387]) -> [PASS][12] +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6848/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
[fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
Participating hosts (50 -> 44)
------------------------------
Additional (2): fi-kbl-soraka fi-tgl-u
Missing (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6848 -> Patchwork_14308
CI-20190529: 20190529
CI_DRM_6848: a1769d05ffa7fe6e4481131e97215f37b8f5ed4e @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5172: 073caf4acb7cac63abe7a5e1409ea27a764db5fd @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14308: f59e1b13e248d599f0c365b9d05a4b9a0f095268 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
f59e1b13e248 drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
8d07a457135f drm/i915: Enhance cdclk sanitization
7bf020857c41 drm/i915: Add calc_voltage_level display vfunc
c3c5cd83bf04 drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
93848eb9fbee drm/i915: Kill cnl_sanitize_cdclk()
4cf0293d973c drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
1ccb1492f096 drm/i915: Use literal representation of cdclk tables
637dedda4030 drm/i915: Consolidate bxt/cnl/icl cdclk readout
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14308/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread* ✗ Fi.CI.BUILD: failure for cdclk consolidation and rework for BXT-TGL (rev2)
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (9 preceding siblings ...)
2019-09-07 1:31 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-09-08 3:17 ` Patchwork
2019-09-08 4:23 ` ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL (rev3) Patchwork
` (2 subsequent siblings)
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-08 3:17 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL (rev2)
URL : https://patchwork.freedesktop.org/series/66365/
State : failure
== Summary ==
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
DESCEND objtool
CHK include/generated/compile.h
AR drivers/gpu/drm/i915/built-in.a
CC [M] drivers/gpu/drm/i915/display/intel_cdclk.o
drivers/gpu/drm/i915/display/intel_cdclk.c: In function ‘calc_cdclk’:
drivers/gpu/drm/i915/display/intel_cdclk.c:1210:20: error: suggest parentheses around ‘&&’ within ‘||’ [-Werror=parentheses]
if (ref == 19200 && table[i].ratio_19 == 0 ||
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/display/intel_cdclk.c:1212:20: error: suggest parentheses around ‘&&’ within ‘||’ [-Werror=parentheses]
ref == 38400 && table[i].ratio_38 == 0)
~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
scripts/Makefile.build:280: recipe for target 'drivers/gpu/drm/i915/display/intel_cdclk.o' failed
make[4]: *** [drivers/gpu/drm/i915/display/intel_cdclk.o] Error 1
scripts/Makefile.build:497: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:497: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:497: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1083: recipe for target 'drivers' failed
make: *** [drivers] Error 2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread* ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL (rev3)
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (10 preceding siblings ...)
2019-09-08 3:17 ` ✗ Fi.CI.BUILD: failure for cdclk consolidation and rework for BXT-TGL (rev2) Patchwork
@ 2019-09-08 4:23 ` Patchwork
2019-09-08 4:48 ` ✓ Fi.CI.BAT: success " Patchwork
2019-09-08 5:57 ` ✓ Fi.CI.IGT: " Patchwork
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-08 4:23 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL (rev3)
URL : https://patchwork.freedesktop.org/series/66365/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
27511b1a3ffa drm/i915: Consolidate bxt/cnl/icl cdclk readout
-:72: CHECK:CAMELCASE: Avoid CamelCase: <CNL_DSSM_CDCLK_PLL_REFCLK_24MHz>
#72: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1272:
+ if (I915_READ(SKL_DSSM) & CNL_DSSM_CDCLK_PLL_REFCLK_24MHz)
total: 0 errors, 0 warnings, 1 checks, 412 lines checked
dc7baa4686ad drm/i915: Use literal representation of cdclk tables
93991cb9176b drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
339a12c3ee06 drm/i915: Kill cnl_sanitize_cdclk()
6ed3fd26d46a drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
619860c282c5 drm/i915: Add calc_voltage_level display vfunc
31291bba865d drm/i915: Enhance cdclk sanitization
a09151689f9d drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread* ✓ Fi.CI.BAT: success for cdclk consolidation and rework for BXT-TGL (rev3)
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (11 preceding siblings ...)
2019-09-08 4:23 ` ✗ Fi.CI.CHECKPATCH: warning for cdclk consolidation and rework for BXT-TGL (rev3) Patchwork
@ 2019-09-08 4:48 ` Patchwork
2019-09-08 5:57 ` ✓ Fi.CI.IGT: " Patchwork
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-08 4:48 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL (rev3)
URL : https://patchwork.freedesktop.org/series/66365/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6850 -> Patchwork_14318
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/
Known issues
------------
Here are the changes found in Patchwork_14318 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_chamelium@dp-crc-fast:
- fi-cml-u2: [PASS][1] -> [FAIL][2] ([fdo#110627])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-icl-u2: [PASS][3] -> [FAIL][4] ([fdo#109483])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- {fi-icl-u4}: [INCOMPLETE][5] ([fdo#107713] / [fdo#109100]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u4/igt@gem_ctx_create@basic-files.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-icl-u4/igt@gem_ctx_create@basic-files.html
* igt@kms_busy@basic-flip-c:
- fi-kbl-7500u: [SKIP][7] ([fdo#109271] / [fdo#109278]) -> [PASS][8] +2 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-kbl-7500u/igt@kms_busy@basic-flip-c.html
* igt@kms_chamelium@dp-edid-read:
- fi-cml-u2: [FAIL][9] ([fdo#109483]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-cml-u2/igt@kms_chamelium@dp-edid-read.html
* igt@kms_chamelium@dp-hpd-fast:
- fi-icl-u2: [DMESG-WARN][11] ([fdo#110595]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109483]: https://bugs.freedesktop.org/show_bug.cgi?id=109483
[fdo#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
[fdo#110464]: https://bugs.freedesktop.org/show_bug.cgi?id=110464
[fdo#110595]: https://bugs.freedesktop.org/show_bug.cgi?id=110595
[fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
Participating hosts (52 -> 44)
------------------------------
Missing (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-icl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6850 -> Patchwork_14318
CI-20190529: 20190529
CI_DRM_6850: edc92c96939102c2a59e389195e8d04049e9a022 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14318: a09151689f9d2ec127d1464285f1f6a3bf019970 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
a09151689f9d drm/i915: Consolidate {bxt, cnl, icl}_init_cdclk
31291bba865d drm/i915: Enhance cdclk sanitization
619860c282c5 drm/i915: Add calc_voltage_level display vfunc
6ed3fd26d46a drm/i915: Consolidate {bxt, cnl, icl}_uninit_cdclk
339a12c3ee06 drm/i915: Kill cnl_sanitize_cdclk()
93991cb9176b drm/i915: Combine bxt_set_cdclk and cnl_set_cdclk
dc7baa4686ad drm/i915: Use literal representation of cdclk tables
27511b1a3ffa drm/i915: Consolidate bxt/cnl/icl cdclk readout
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread* ✓ Fi.CI.IGT: success for cdclk consolidation and rework for BXT-TGL (rev3)
2019-09-07 0:21 [PATCH 0/8] cdclk consolidation and rework for BXT-TGL Matt Roper
` (12 preceding siblings ...)
2019-09-08 4:48 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-09-08 5:57 ` Patchwork
13 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2019-09-08 5:57 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx
== Series Details ==
Series: cdclk consolidation and rework for BXT-TGL (rev3)
URL : https://patchwork.freedesktop.org/series/66365/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6850_full -> Patchwork_14318_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_14318_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_exec@basic-invalid-context-vcs1:
- shard-iclb: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb1/igt@gem_ctx_exec@basic-invalid-context-vcs1.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb7/igt@gem_ctx_exec@basic-invalid-context-vcs1.html
* igt@gem_ctx_isolation@rcs0-s3:
- shard-apl: [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +5 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-apl4/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_eio@reset-stress:
- shard-apl: [PASS][5] -> [FAIL][6] ([fdo#109661])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl8/igt@gem_eio@reset-stress.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-apl7/igt@gem_eio@reset-stress.html
* igt@gem_exec_capture@capture-bsd2:
- shard-iclb: [PASS][7] -> [SKIP][8] ([fdo#109276]) +14 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb4/igt@gem_exec_capture@capture-bsd2.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb7/igt@gem_exec_capture@capture-bsd2.html
* igt@gem_exec_schedule@deep-bsd:
- shard-iclb: [PASS][9] -> [SKIP][10] ([fdo#111325]) +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb7/igt@gem_exec_schedule@deep-bsd.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb2/igt@gem_exec_schedule@deep-bsd.html
* igt@i915_pm_rpm@legacy-planes:
- shard-skl: [PASS][11] -> [INCOMPLETE][12] ([fdo#107807])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl2/igt@i915_pm_rpm@legacy-planes.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-skl5/igt@i915_pm_rpm@legacy-planes.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
- shard-iclb: [PASS][13] -> [FAIL][14] ([fdo#103167]) +5 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
* igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
- shard-skl: [PASS][15] -> [FAIL][16] ([fdo#108145] / [fdo#110403])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-skl4/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
* igt@kms_psr2_su@frontbuffer:
- shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109642] / [fdo#111068])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb5/igt@kms_psr2_su@frontbuffer.html
* igt@kms_psr@psr2_sprite_mmap_gtt:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441]) +2 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb5/igt@kms_psr@psr2_sprite_mmap_gtt.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][21] -> [FAIL][22] ([fdo#99912])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl1/igt@kms_setmode@basic.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-apl3/igt@kms_setmode@basic.html
* igt@testdisplay:
- shard-glk: [PASS][23] -> [DMESG-WARN][24] ([fdo#105763] / [fdo#106538])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-glk1/igt@testdisplay.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-glk2/igt@testdisplay.html
#### Possible fixes ####
* igt@gem_exec_schedule@preemptive-hang-bsd:
- shard-iclb: [SKIP][25] ([fdo#111325]) -> [PASS][26] +3 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@gem_exec_schedule@preemptive-hang-bsd.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb5/igt@gem_exec_schedule@preemptive-hang-bsd.html
* igt@i915_suspend@fence-restore-tiled2untiled:
- shard-apl: [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28] +8 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-apl7/igt@i915_suspend@fence-restore-tiled2untiled.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: [FAIL][29] ([fdo#105363]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-glk5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_frontbuffer_tracking@basic:
- shard-iclb: [FAIL][31] ([fdo#103167]) -> [PASS][32] +3 similar issues
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_frontbuffer_tracking@basic.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb1/igt@kms_frontbuffer_tracking@basic.html
* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [FAIL][33] ([fdo#103166]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb3/igt@kms_plane_lowres@pipe-a-tiling-y.html
* igt@kms_psr@psr2_no_drrs:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36] +2 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb7/igt@kms_psr@psr2_no_drrs.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb2/igt@kms_psr@psr2_no_drrs.html
* igt@prime_busy@hang-bsd2:
- shard-iclb: [SKIP][37] ([fdo#109276]) -> [PASS][38] +11 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb8/igt@prime_busy@hang-bsd2.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb4/igt@prime_busy@hang-bsd2.html
#### Warnings ####
* igt@gem_ctx_isolation@vcs1-nonpriv:
- shard-iclb: [SKIP][39] ([fdo#109276]) -> [FAIL][40] ([fdo#111329])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb6/igt@gem_ctx_isolation@vcs1-nonpriv.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html
* igt@gem_mocs_settings@mocs-settings-bsd2:
- shard-iclb: [SKIP][41] ([fdo#109276]) -> [FAIL][42] ([fdo#111330])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb8/igt@gem_mocs_settings@mocs-settings-bsd2.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb4/igt@gem_mocs_settings@mocs-settings-bsd2.html
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: [DMESG-WARN][43] ([fdo#107724]) -> [SKIP][44] ([fdo#109349])
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-iclb6/igt@kms_dp_dsc@basic-dsc-enable-edp.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- shard-skl: [FAIL][45] ([fdo#103167]) -> [FAIL][46] ([fdo#108040])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6850/shard-skl5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/shard-skl9/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-gtt.html
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
[fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
[fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329
[fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 10)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_6850 -> Patchwork_14318
CI-20190529: 20190529
CI_DRM_6850: edc92c96939102c2a59e389195e8d04049e9a022 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5173: 3fb0f227d8856008f89a797879e27094745ce97e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_14318: a09151689f9d2ec127d1464285f1f6a3bf019970 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14318/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread