All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] CNL DVFS
@ 2017-09-30  0:08 Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 01/12] drm/i915: Let's use more enum intel_dpll_id pll_id Rodrigo Vivi
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

This is a new attempt of fixing the DVFS on CNL.

After I got the CI nack on the initial attempt I went down
finding and fixing some issues.
But also I decided to rework the existent port clock
functions to make sure we don't duplicate existent code
but also make sure we address HDMI case.

Another difference is that this series I don't extend
dvfs functions to SKL yet. I'd like to first discuss
and fix this CNL before we go and address SKL one.

Along with extending to SKL my plan is to document
these dvfs functions.

So, please let me know all your thoughts about patches here.

Thanks in advance,
Rodrigo.

Kahola, Mika (3):
  drm/i915/cnl: Expose DVFS change functions
  drm/i915/cnl: DVFS for PLL enabling
  drm/i915/cnl: DVFS for PLL disabling

Paulo Zanoni (1):
  drm/i915/cnl: extract cnl_dvfs_{pre,post}_change

Rodrigo Vivi (8):
  drm/i915: Let's use more enum intel_dpll_id pll_id.
  drm/i915/cnl: Extract cnl_calc_pll_link following bxt style.
  drm/i915/skl: Extract cnl_calc_pll_link following bxt,cnl style.
  drm/i915: Unify and export gen9+ port_clock calculation.
  drm/i915/cnl: Only request voltage frequency switching when needed.
  drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement.
  drm/i915/cnl: Invert dvfs default level.
  drm/i915/cnl: Unify dvfs level selection.

 drivers/gpu/drm/i915/intel_cdclk.c    | 66 +++++++++++++++++---------
 drivers/gpu/drm/i915/intel_ddi.c      | 87 +++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 42 ++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h      |  7 ++-
 4 files changed, 133 insertions(+), 69 deletions(-)

-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/12] drm/i915: Let's use more enum intel_dpll_id pll_id.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 02/12] drm/i915/cnl: Extract cnl_calc_pll_link following bxt style Rodrigo Vivi
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

No functional change expected. Just let's use this enum
when possible and also same standard pll_id name
so we can rework gen9+ port clock later.

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 93cbbcbbc193..b5dd82a0e357 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1102,14 +1102,14 @@ static int hsw_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
 }
 
 static int skl_calc_wrpll_link(struct drm_i915_private *dev_priv,
-			       uint32_t dpll)
+			       enum intel_dpll_id pll_id)
 {
 	i915_reg_t cfgcr1_reg, cfgcr2_reg;
 	uint32_t cfgcr1_val, cfgcr2_val;
 	uint32_t p0, p1, p2, dco_freq;
 
-	cfgcr1_reg = DPLL_CFGCR1(dpll);
-	cfgcr2_reg = DPLL_CFGCR2(dpll);
+	cfgcr1_reg = DPLL_CFGCR1(pll_id);
+	cfgcr2_reg = DPLL_CFGCR2(pll_id);
 
 	cfgcr1_val = I915_READ(cfgcr1_reg);
 	cfgcr2_val = I915_READ(cfgcr2_reg);
@@ -1162,7 +1162,7 @@ static int skl_calc_wrpll_link(struct drm_i915_private *dev_priv,
 }
 
 static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
-			       uint32_t pll_id)
+			       enum intel_dpll_id pll_id)
 {
 	uint32_t cfgcr0, cfgcr1;
 	uint32_t p0, p1, p2, dco_freq, ref_clock;
@@ -1246,7 +1246,8 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int link_clock = 0;
-	uint32_t cfgcr0, pll_id;
+	uint32_t cfgcr0;
+	enum intel_dpll_id pll_id;
 
 	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
@@ -1299,17 +1300,18 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int link_clock = 0;
-	uint32_t dpll_ctl1, dpll;
+	uint32_t dpll_ctl1;
+	enum intel_dpll_id pll_id;
 
-	dpll = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
+	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
 	dpll_ctl1 = I915_READ(DPLL_CTRL1);
 
-	if (dpll_ctl1 & DPLL_CTRL1_HDMI_MODE(dpll)) {
-		link_clock = skl_calc_wrpll_link(dev_priv, dpll);
+	if (dpll_ctl1 & DPLL_CTRL1_HDMI_MODE(pll_id)) {
+		link_clock = skl_calc_wrpll_link(dev_priv, pll_id);
 	} else {
-		link_clock = dpll_ctl1 & DPLL_CTRL1_LINK_RATE_MASK(dpll);
-		link_clock >>= DPLL_CTRL1_LINK_RATE_SHIFT(dpll);
+		link_clock = dpll_ctl1 & DPLL_CTRL1_LINK_RATE_MASK(pll_id);
+		link_clock >>= DPLL_CTRL1_LINK_RATE_SHIFT(pll_id);
 
 		switch (link_clock) {
 		case DPLL_CTRL1_LINK_RATE_810:
@@ -1390,17 +1392,17 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
 }
 
 static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
-				enum intel_dpll_id dpll)
+			     enum intel_dpll_id pll_id)
 {
 	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state *state;
 	struct dpll clock;
 
 	/* For DDI ports we always use a shared PLL. */
-	if (WARN_ON(dpll == DPLL_ID_PRIVATE))
+	if (WARN_ON(pll_id == DPLL_ID_PRIVATE))
 		return 0;
 
-	pll = &dev_priv->shared_dplls[dpll];
+	pll = &dev_priv->shared_dplls[pll_id];
 	state = &pll->state.hw_state;
 
 	clock.m1 = 2;
@@ -1419,9 +1421,9 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	enum port port = intel_ddi_get_encoder_port(encoder);
-	uint32_t dpll = port;
+	enum intel_dpll_id pll_id = port;
 
-	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, dpll);
+	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/12] drm/i915/cnl: Extract cnl_calc_pll_link following bxt style.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 01/12] drm/i915: Let's use more enum intel_dpll_id pll_id Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 03/12] drm/i915/skl: Extract cnl_calc_pll_link following bxt, cnl style Rodrigo Vivi
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

No functional change. Just spliting the function for
better port clock handling later.

Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b5dd82a0e357..0412035aaa45 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1241,15 +1241,11 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
 	pipe_config->base.adjusted_mode.crtc_clock = dotclock;
 }
 
-static void cnl_ddi_clock_get(struct intel_encoder *encoder,
-			      struct intel_crtc_state *pipe_config)
+static int cnl_calc_pll_link(struct drm_i915_private *dev_priv,
+			     enum intel_dpll_id pll_id)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int link_clock = 0;
 	uint32_t cfgcr0;
-	enum intel_dpll_id pll_id;
-
-	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
 	cfgcr0 = I915_READ(CNL_DPLL_CFGCR0(pll_id));
 
@@ -1287,10 +1283,20 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
 			WARN(1, "Unsupported link rate\n");
 			break;
 		}
-		link_clock *= 2;
 	}
 
-	pipe_config->port_clock = link_clock;
+	return link_clock;
+}
+
+static void cnl_ddi_clock_get(struct intel_encoder *encoder,
+			      struct intel_crtc_state *pipe_config)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum intel_dpll_id pll_id;
+
+	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
+
+	pipe_config->port_clock = 2 * cnl_calc_pll_link(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/12] drm/i915/skl: Extract cnl_calc_pll_link following bxt, cnl style.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 01/12] drm/i915: Let's use more enum intel_dpll_id pll_id Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 02/12] drm/i915/cnl: Extract cnl_calc_pll_link following bxt style Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 04/12] drm/i915: Unify and export gen9+ port_clock calculation Rodrigo Vivi
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

No functional change. Just spliting the function for
better port clock handling later.

Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0412035aaa45..593fd18ffbb2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1301,15 +1301,11 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
 	ddi_dotclock_get(pipe_config);
 }
 
-static void skl_ddi_clock_get(struct intel_encoder *encoder,
-				struct intel_crtc_state *pipe_config)
+static int skl_calc_pll_link(struct drm_i915_private *dev_priv,
+			     enum intel_dpll_id pll_id)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	int link_clock = 0;
 	uint32_t dpll_ctl1;
-	enum intel_dpll_id pll_id;
-
-	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
 	dpll_ctl1 = I915_READ(DPLL_CTRL1);
 
@@ -1342,10 +1338,20 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 			WARN(1, "Unsupported link rate\n");
 			break;
 		}
-		link_clock *= 2;
 	}
 
-	pipe_config->port_clock = link_clock;
+	return link_clock;
+}
+
+static void skl_ddi_clock_get(struct intel_encoder *encoder,
+				struct intel_crtc_state *pipe_config)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	enum intel_dpll_id pll_id;
+
+	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
+
+	pipe_config->port_clock = 2 * skl_calc_pll_link(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/12] drm/i915: Unify and export gen9+ port_clock calculation.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 03/12] drm/i915/skl: Extract cnl_calc_pll_link following bxt, cnl style Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 05/12] drm/i915/cnl: extract cnl_dvfs_{pre, post}_change Rodrigo Vivi
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

On Cannonlake the DVFS level selection depends on the
port clock.

So let's re-org in a way that we can easily export without
duplicating any code.

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h |  3 ++-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 593fd18ffbb2..a359ce2d9ada 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1296,7 +1296,7 @@ static void cnl_ddi_clock_get(struct intel_encoder *encoder,
 
 	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
-	pipe_config->port_clock = 2 * cnl_calc_pll_link(dev_priv, pll_id);
+	pipe_config->port_clock = intel_ddi_port_clock(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
@@ -1351,7 +1351,7 @@ static void skl_ddi_clock_get(struct intel_encoder *encoder,
 
 	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
 
-	pipe_config->port_clock = 2 * skl_calc_pll_link(dev_priv, pll_id);
+	pipe_config->port_clock = intel_ddi_port_clock(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
@@ -1435,11 +1435,26 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	enum intel_dpll_id pll_id = port;
 
-	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, pll_id);
+	pipe_config->port_clock = intel_ddi_port_clock(dev_priv, pll_id);
 
 	ddi_dotclock_get(pipe_config);
 }
 
+int intel_ddi_port_clock(struct drm_i915_private *dev_priv,
+			 enum intel_dpll_id pll_id)
+{
+	if (IS_GEN9_BC(dev_priv)) {
+		return 2 * skl_calc_pll_link(dev_priv, pll_id);
+	} else if (IS_GEN9_LP(dev_priv)) {
+		return bxt_calc_pll_link(dev_priv, pll_id);
+	} else if (IS_CANNONLAKE(dev_priv)) {
+		return 2 * cnl_calc_pll_link(dev_priv, pll_id);
+	} else {
+		MISSING_CASE(INTEL_GEN(dev_priv));
+		return 0;
+	}
+}
+
 void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0cab667fff57..fe4650d6db03 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1293,7 +1293,8 @@ bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
 				 struct intel_crtc *intel_crtc);
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config);
-
+int intel_ddi_port_clock(struct drm_i915_private *dev_priv,
+			 enum intel_dpll_id pll_id);
 void intel_ddi_clock_get(struct intel_encoder *encoder,
 			 struct intel_crtc_state *pipe_config);
 void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/12] drm/i915/cnl: extract cnl_dvfs_{pre, post}_change
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (3 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 04/12] drm/i915: Unify and export gen9+ port_clock calculation Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 06/12] drm/i915/cnl: Expose DVFS change functions Rodrigo Vivi
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

From: Paulo Zanoni <paulo.r.zanoni@intel.com>

These functions even have their own page in our spec,
so extract them from cnl_set_cdclk().

v2: (By Rodrigo) Fixed inverted logic on error return of
    cnl_dvfs_pre_change.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 87fc42b19336..b35eb145d66e 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1510,12 +1510,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
 	dev_priv->cdclk.hw.vco = vco;
 }
 
-static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+static int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv)
 {
-	int cdclk = cdclk_state->cdclk;
-	int vco = cdclk_state->vco;
-	u32 val, divider, pcu_ack;
 	int ret;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
@@ -1524,11 +1520,30 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 				SKL_CDCLK_READY_FOR_CHANGE,
 				SKL_CDCLK_READY_FOR_CHANGE, 3);
 	mutex_unlock(&dev_priv->rps.hw_lock);
-	if (ret) {
+
+	if (ret)
 		DRM_ERROR("Failed to inform PCU about cdclk change (%d)\n",
 			  ret);
+
+	return ret;
+}
+
+static void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level)
+{
+	mutex_lock(&dev_priv->rps.hw_lock);
+	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, level);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+}
+
+static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
+			  const struct intel_cdclk_state *cdclk_state)
+{
+	int cdclk = cdclk_state->cdclk;
+	int vco = cdclk_state->vco;
+	u32 val, divider, pcu_ack;
+
+	if (cnl_dvfs_pre_change(dev_priv))
 		return;
-	}
 
 	/* cdclk = vco / 2 / div{1,2} */
 	switch (DIV_ROUND_CLOSEST(vco, cdclk)) {
@@ -1575,9 +1590,7 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 	I915_WRITE(CDCLK_CTL, val);
 
 	/* inform PCU of the change */
-	mutex_lock(&dev_priv->rps.hw_lock);
-	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, pcu_ack);
-	mutex_unlock(&dev_priv->rps.hw_lock);
+	cnl_dvfs_post_change(dev_priv, pcu_ack);
 
 	intel_update_cdclk(dev_priv);
 }
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/12] drm/i915/cnl: Expose DVFS change functions
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (4 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 05/12] drm/i915/cnl: extract cnl_dvfs_{pre, post}_change Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 07/12] drm/i915/cnl: DVFS for PLL enabling Rodrigo Vivi
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Kahola, Rodrigo Vivi

From: "Kahola, Mika" <mika.kahola@intel.com>

DVFS computation needs cnl_dvfs_{pre,post}_change() functions to be exposed.
These functions will be used when computing DVFS levels in intel_dpll_mgr.c

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Kahola, Mika <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 4 ++--
 drivers/gpu/drm/i915/intel_drv.h   | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index b35eb145d66e..af8411c2a6b9 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1510,7 +1510,7 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
 	dev_priv->cdclk.hw.vco = vco;
 }
 
-static int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv)
+int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv)
 {
 	int ret;
 
@@ -1528,7 +1528,7 @@ static int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-static void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level)
+void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level)
 {
 	mutex_lock(&dev_priv->rps.hw_lock);
 	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL, level);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fe4650d6db03..934ccf17f8ab 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1323,6 +1323,8 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
 void cnl_init_cdclk(struct drm_i915_private *dev_priv);
 void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
+int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv);
+void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level);
 void bxt_init_cdclk(struct drm_i915_private *dev_priv);
 void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/12] drm/i915/cnl: DVFS for PLL enabling
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (5 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 06/12] drm/i915/cnl: Expose DVFS change functions Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 08/12] drm/i915/cnl: DVFS for PLL disabling Rodrigo Vivi
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Kahola, Paulo Zanoni, Rodrigo Vivi

From: "Kahola, Mika" <mika.kahola@intel.com>

Display Voltage and Frequency Switching (DVFS) is used to adjust the
display voltage to match the display clock frequencies. If voltage is
set too low, it will break functionality. If voltage is set too high,
it will waste power. Voltage level is selected based on CD clock and
DDI clock.

The sequence before frequency change is the following and it requests
the power controller to raise voltage to maximum

- Ensure any previous GT Driver Mailbox transaction is complete.
- Write GT Driver Mailbox Data Low = 0x3.
- Write GT Driver Mailbox Data High = 0x0.
- Write GT Driver Mailbox Interface = 0x80000007.
- Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 = 0).
- Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart the sequence.
  Timeout after 3ms

The sequence after frequency change is the following and it requests
the port controller to raise voltage to the requested level.

- Write GT Driver Mailbox Data Low
 * For level 0, write 0x0
 * For level 1, write 0x1
 * For level 2, write 0x2
 * For level 3, write 0x3
   - Write GT Driver Mailbox Data High = 0x0.
   - Write GT Driver Mailbox Interface = 0x80000007.

For Cannonlake, the level 3 is not used and it aliases to level 2.

v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
    [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
v3: (By Rodrigo): Remove duplicated commend and fix typo on Paulo's name.
v4: (By Rodrigo): Rebase on top "Unify and export gen9+ port_clock calculation"
    The port clock calculation here was only addressing DP, so let's reuse
    the current port calculation that is already in place without any duplication.
    Alos fix portclk <= 594000 instead of portclk < 594000.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Kahola, Mika <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index a2a3d93d67bd..6030fbafa580 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1966,10 +1966,23 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
 	.dump_hw_state = bxt_dump_hw_state,
 };
 
+static int cnl_get_dvfs_level(int cdclk, int portclk)
+{
+	if (cdclk == 168000 && portclk <= 594000)
+		return 0;
+	else if (cdclk == 336000 && portclk <= 594000)
+		return 1;
+	else
+		return 2;
+}
+
 static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
 	uint32_t val;
+	int ret;
+	int level;
+	int cdclk, portclk;
 
 	/* 1. Enable DPLL power in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2006,11 +2019,9 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	/*
 	 * 5. If the frequency will result in a change to the voltage
 	 * requirement, follow the Display Voltage Frequency Switching
-	 * Sequence Before Frequency Change
-	 *
-	 * FIXME: (DVFS) is used to adjust the display voltage to match the
-	 * display clock frequencies
+	 * (DVFS) Sequence Before Frequency Change
 	 */
+	ret = cnl_dvfs_pre_change(dev_priv);
 
 	/* 6. Enable DPLL in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2028,11 +2039,14 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	/*
 	 * 8. If the frequency will result in a change to the voltage
 	 * requirement, follow the Display Voltage Frequency Switching
-	 * Sequence After Frequency Change
-	 *
-	 * FIXME: (DVFS) is used to adjust the display voltage to match the
-	 * display clock frequencies
+	 * (DVFS) Sequence After Frequency Change
 	 */
+	if (ret == 0) {
+		cdclk = dev_priv->cdclk.hw.cdclk;
+		portclk = intel_ddi_port_clock(dev_priv, pll->id);
+		level = cnl_get_dvfs_level(cdclk, portclk);
+		cnl_dvfs_post_change(dev_priv, level);
+	}
 
 	/*
 	 * 9. turn on the clock for the DDI and map the DPLL to the DDI
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/12] drm/i915/cnl: DVFS for PLL disabling
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (6 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 07/12] drm/i915/cnl: DVFS for PLL enabling Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 09/12] drm/i915/cnl: Only request voltage frequency switching when needed Rodrigo Vivi
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Kahola, Paulo Zanoni, Rodrigo Vivi

From: "Kahola, Mika" <mika.kahola@intel.com>

Display Voltage and Frequency Switching (DVFS) is used to adjust the
display voltage to match the display clock frequencies. To save power the
voltage is set to minimum when disabling PLL.

The sequence before frequency change is the following and it requests
the power controller to raise voltage to maximum

- Ensure any previous GT Driver Mailbox transaction is complete.
- Write GT Driver Mailbox Data Low = 0x3.
- Write GT Driver Mailbox Data High = 0x0.
- Write GT Driver Mailbox Interface = 0x80000007.
- Poll GT Driver Mailbox Interface for Run/Busy indication cleared (bit 31 = 0).
- Read GT Driver Mailbox Data Low, if bit 0 is 0x1, continue, else restart the sequence.
  Timeout after 3ms

The sequence after frequency change is the following and it requests
the port controller to lower voltage to the minimum.

- Write GT Driver Mailbox Data Low = 0x0
- Write GT Driver Mailbox Data High = 0x0.
- Write GT Driver Mailbox Interface = 0x80000007.

v2: reuse Paulo's work on cdclk. This patch depends on Paulo's patch
    [PATCH 02/12] drm/i915/cnl: extract cnl_dvfs_{pre,post}_change
v3: (By Rodrigo): Fix typo on Paulo's name.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Kahola, Mika <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 6030fbafa580..a71a6c396bbd 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2058,6 +2058,7 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll)
 {
 	uint32_t val;
+	int ret;
 
 	/*
 	 * 1. Configure DPCLKA_CFGCR0 to turn off the clock for the DDI.
@@ -2067,11 +2068,9 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	/*
 	 * 2. If the frequency will result in a change to the voltage
 	 * requirement, follow the Display Voltage Frequency Switching
-	 * Sequence Before Frequency Change
-	 *
-	 * FIXME: (DVFS) is used to adjust the display voltage to match the
-	 * display clock frequencies
+	 * (DVFS) Sequence Before Frequency Change
 	 */
+	ret = cnl_dvfs_pre_change(dev_priv);
 
 	/* 3. Disable DPLL through DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2089,11 +2088,10 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	/*
 	 * 5. If the frequency will result in a change to the voltage
 	 * requirement, follow the Display Voltage Frequency Switching
-	 * Sequence After Frequency Change
-	 *
-	 * FIXME: (DVFS) is used to adjust the display voltage to match the
-	 * display clock frequencies
+	 * (DVFS) Sequence After Frequency Change
 	 */
+	if (ret == 0)
+		cnl_dvfs_post_change(dev_priv, 0);
 
 	/* 6. Disable DPLL power in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/12] drm/i915/cnl: Only request voltage frequency switching when needed.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (7 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 08/12] drm/i915/cnl: DVFS for PLL disabling Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 10/12] drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement Rodrigo Vivi
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

At cdclk initialization at spec they tell us to always use dvfs
or avoid change cdclk if it fails.

Here on pll sequence, spec tells us to only use dvfs "if the
frequency will result in a change to the voltage requirement."

So in order to respect that and avoid necessary interactions with
PCODE we compare our current needed level with the current set level
and only perform dvfs sequences when we need to change that level.

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c    |  9 +++++++++
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 16 +++++++++-------
 drivers/gpu/drm/i915/intel_drv.h      |  1 +
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index af8411c2a6b9..516cd4f920b5 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1535,6 +1535,15 @@ void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level)
 	mutex_unlock(&dev_priv->rps.hw_lock);
 }
 
+bool cnl_dvfs_need_change(struct drm_i915_private *dev_priv, int level)
+{
+	int old_level;
+	mutex_lock(&dev_priv->rps.hw_lock);
+	sandybridge_pcode_read(dev_priv, SKL_PCODE_CDCLK_CONTROL, &old_level);
+	mutex_unlock(&dev_priv->rps.hw_lock);
+	return level != old_level;
+}
+
 static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 			  const struct intel_cdclk_state *cdclk_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index a71a6c396bbd..ca1ea475c517 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1980,9 +1980,10 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
 	uint32_t val;
-	int ret;
+	int ret = 0;
 	int level;
 	int cdclk, portclk;
+	bool change_level;
 
 	/* 1. Enable DPLL power in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2021,7 +2022,12 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	 * requirement, follow the Display Voltage Frequency Switching
 	 * (DVFS) Sequence Before Frequency Change
 	 */
-	ret = cnl_dvfs_pre_change(dev_priv);
+	cdclk = dev_priv->cdclk.hw.cdclk;
+	portclk = intel_ddi_port_clock(dev_priv, pll->id);
+	level = cnl_get_dvfs_level(cdclk, portclk);
+	change_level = cnl_dvfs_need_change(dev_priv, level);
+	if (change_level)
+		ret = cnl_dvfs_pre_change(dev_priv);
 
 	/* 6. Enable DPLL in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2041,12 +2047,8 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	 * requirement, follow the Display Voltage Frequency Switching
 	 * (DVFS) Sequence After Frequency Change
 	 */
-	if (ret == 0) {
-		cdclk = dev_priv->cdclk.hw.cdclk;
-		portclk = intel_ddi_port_clock(dev_priv, pll->id);
-		level = cnl_get_dvfs_level(cdclk, portclk);
+	if (change_level && ret == 0)
 		cnl_dvfs_post_change(dev_priv, level);
-	}
 
 	/*
 	 * 9. turn on the clock for the DDI and map the DPLL to the DDI
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 934ccf17f8ab..200061def48e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1325,6 +1325,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv);
 void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
 int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv);
 void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level);
+bool cnl_dvfs_need_change(struct drm_i915_private *dev_priv, int level);
 void bxt_init_cdclk(struct drm_i915_private *dev_priv);
 void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/12] drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (8 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 09/12] drm/i915/cnl: Only request voltage frequency switching when needed Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 11/12] drm/i915/cnl: Invert dvfs default level Rodrigo Vivi
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

Spec tells us to change the level "if the frequency will result
in a change to the voltage requirement."

When we don't have pll enabled yet we only base our level
calculation on cdclk. So let's do the same when disabling the
pll instead of forcing randomly to zero.

Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index ca1ea475c517..f6b58879ac6a 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2060,7 +2060,9 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll)
 {
 	uint32_t val;
-	int ret;
+	int ret = 0;
+	int cdclk, level;
+	bool change_level;
 
 	/*
 	 * 1. Configure DPCLKA_CFGCR0 to turn off the clock for the DDI.
@@ -2072,7 +2074,22 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 * requirement, follow the Display Voltage Frequency Switching
 	 * (DVFS) Sequence Before Frequency Change
 	 */
-	ret = cnl_dvfs_pre_change(dev_priv);
+	cdclk = dev_priv->cdclk.hw.cdclk;
+	switch (cdclk) {
+	case 528000:
+		level = 2;
+		break;
+	case 336000:
+		level = 1;
+		break;
+	case 168000:
+	default:
+		level = 0;
+		break;
+	}
+	change_level = cnl_dvfs_need_change(dev_priv, level);
+	if (change_level)
+		ret = cnl_dvfs_pre_change(dev_priv);
 
 	/* 3. Disable DPLL through DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
@@ -2092,8 +2109,8 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 * requirement, follow the Display Voltage Frequency Switching
 	 * (DVFS) Sequence After Frequency Change
 	 */
-	if (ret == 0)
-		cnl_dvfs_post_change(dev_priv, 0);
+	if (change_level && ret == 0)
+		cnl_dvfs_post_change(dev_priv, level);
 
 	/* 6. Disable DPLL power in DPLL_ENABLE. */
 	val = I915_READ(CNL_DPLL_ENABLE(pll->id));
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/12] drm/i915/cnl: Invert dvfs default level.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (9 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 10/12] drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:08 ` [PATCH 12/12] drm/i915/cnl: Unify dvfs level selection Rodrigo Vivi
  2017-09-30  0:31 ` ✗ Fi.CI.BAT: failure for CNL DVFS Patchwork
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

According to spec "If voltage is set too low,
it will break functionality. If voltage is set too high,
 it will waste power."

So, let's prefer the waste of power instead of breaking
functionality.

But also the logic of deciding the level on spec
tells "Else, use level 2."
So, default is actually "2", not "0".

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c    | 8 ++++----
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 516cd4f920b5..f2629dbec763 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1571,15 +1571,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 
 	switch (cdclk) {
-	case 528000:
-		pcu_ack = 2;
+	case 168000:
+		pcu_ack = 0;
 		break;
 	case 336000:
 		pcu_ack = 1;
 		break;
-	case 168000:
+	case 528000:
 	default:
-		pcu_ack = 0;
+		pcu_ack = 2;
 		break;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index f6b58879ac6a..6bbc3d718e78 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2076,15 +2076,15 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 */
 	cdclk = dev_priv->cdclk.hw.cdclk;
 	switch (cdclk) {
-	case 528000:
-		level = 2;
+	case 168000:
+		level = 0;
 		break;
 	case 336000:
 		level = 1;
 		break;
-	case 168000:
+	case 528000:
 	default:
-		level = 0;
+		level = 2;
 		break;
 	}
 	change_level = cnl_dvfs_need_change(dev_priv, level);
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 12/12] drm/i915/cnl: Unify dvfs level selection.
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (10 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 11/12] drm/i915/cnl: Invert dvfs default level Rodrigo Vivi
@ 2017-09-30  0:08 ` Rodrigo Vivi
  2017-09-30  0:31 ` ✗ Fi.CI.BAT: failure for CNL DVFS Patchwork
  12 siblings, 0 replies; 14+ messages in thread
From: Rodrigo Vivi @ 2017-09-30  0:08 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Rodrigo Vivi

When port clock is zero or undefined we base our
calculation on cdclk. So, same function can be used
for port clock == 0 now that we have the same default "2".

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c    | 28 +++++++++++++---------------
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 25 ++-----------------------
 drivers/gpu/drm/i915/intel_drv.h      |  1 +
 3 files changed, 16 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index f2629dbec763..b2a1c0c7c938 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1544,12 +1544,22 @@ bool cnl_dvfs_need_change(struct drm_i915_private *dev_priv, int level)
 	return level != old_level;
 }
 
+int cnl_dvfs_get_level(int cdclk, int portclk)
+{
+	if (cdclk == 168000 && portclk <= 594000)
+		return 0;
+	else if (cdclk == 336000 && portclk <= 594000)
+		return 1;
+	else
+		return 2;
+}
+
 static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 			  const struct intel_cdclk_state *cdclk_state)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
-	u32 val, divider, pcu_ack;
+	u32 val, divider, level;
 
 	if (cnl_dvfs_pre_change(dev_priv))
 		return;
@@ -1570,19 +1580,6 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 		break;
 	}
 
-	switch (cdclk) {
-	case 168000:
-		pcu_ack = 0;
-		break;
-	case 336000:
-		pcu_ack = 1;
-		break;
-	case 528000:
-	default:
-		pcu_ack = 2;
-		break;
-	}
-
 	if (dev_priv->cdclk.hw.vco != 0 &&
 	    dev_priv->cdclk.hw.vco != vco)
 		cnl_cdclk_pll_disable(dev_priv);
@@ -1599,7 +1596,8 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 	I915_WRITE(CDCLK_CTL, val);
 
 	/* inform PCU of the change */
-	cnl_dvfs_post_change(dev_priv, pcu_ack);
+	level = cnl_dvfs_get_level(cdclk, 0);
+	cnl_dvfs_post_change(dev_priv, level);
 
 	intel_update_cdclk(dev_priv);
 }
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 6bbc3d718e78..1006c2853999 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -1966,16 +1966,6 @@ static const struct intel_dpll_mgr bxt_pll_mgr = {
 	.dump_hw_state = bxt_dump_hw_state,
 };
 
-static int cnl_get_dvfs_level(int cdclk, int portclk)
-{
-	if (cdclk == 168000 && portclk <= 594000)
-		return 0;
-	else if (cdclk == 336000 && portclk <= 594000)
-		return 1;
-	else
-		return 2;
-}
-
 static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
@@ -2024,7 +2014,7 @@ static void cnl_ddi_pll_enable(struct drm_i915_private *dev_priv,
 	 */
 	cdclk = dev_priv->cdclk.hw.cdclk;
 	portclk = intel_ddi_port_clock(dev_priv, pll->id);
-	level = cnl_get_dvfs_level(cdclk, portclk);
+	level = cnl_dvfs_get_level(cdclk, portclk);
 	change_level = cnl_dvfs_need_change(dev_priv, level);
 	if (change_level)
 		ret = cnl_dvfs_pre_change(dev_priv);
@@ -2075,18 +2065,7 @@ static void cnl_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	 * (DVFS) Sequence Before Frequency Change
 	 */
 	cdclk = dev_priv->cdclk.hw.cdclk;
-	switch (cdclk) {
-	case 168000:
-		level = 0;
-		break;
-	case 336000:
-		level = 1;
-		break;
-	case 528000:
-	default:
-		level = 2;
-		break;
-	}
+	level = cnl_dvfs_get_level(cdclk, 0);
 	change_level = cnl_dvfs_need_change(dev_priv, level);
 	if (change_level)
 		ret = cnl_dvfs_pre_change(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 200061def48e..c1395e8a6ba6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1326,6 +1326,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv);
 int cnl_dvfs_pre_change(struct drm_i915_private *dev_priv);
 void cnl_dvfs_post_change(struct drm_i915_private *dev_priv, int level);
 bool cnl_dvfs_need_change(struct drm_i915_private *dev_priv, int level);
+int cnl_dvfs_get_level(int cdclk, int portclk);
 void bxt_init_cdclk(struct drm_i915_private *dev_priv);
 void bxt_uninit_cdclk(struct drm_i915_private *dev_priv);
 void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
-- 
2.13.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.BAT: failure for CNL DVFS
  2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
                   ` (11 preceding siblings ...)
  2017-09-30  0:08 ` [PATCH 12/12] drm/i915/cnl: Unify dvfs level selection Rodrigo Vivi
@ 2017-09-30  0:31 ` Patchwork
  12 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2017-09-30  0:31 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: CNL DVFS
URL   : https://patchwork.freedesktop.org/series/31211/
State : failure

== Summary ==

Series 31211v1 CNL DVFS
https://patchwork.freedesktop.org/api/1.0/series/31211/revisions/1/mbox/

Test chamelium:
        Subgroup dp-hpd-fast:
                skip       -> INCOMPLETE (fi-skl-6700k)
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
Test kms_busy:
        Subgroup basic-flip-a:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-b:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-c:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
                incomplete -> PASS       (fi-bxt-j4205) fdo#102035
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-busy-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-after-cursor-atomic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-after-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-before-cursor-atomic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-before-cursor-legacy:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-before-cursor-varying-size:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup hang-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-b:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-c:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6260u)
                pass       -> DMESG-WARN (fi-skl-gvtdvm)
        Subgroup read-crc-pipe-a-frame-sequence:
WARNING: Long output truncated

e650b9cdaff616445f38986379b1c0c4b03a4282 drm-tip: 2017y-09m-29d-11h-52m-41s UTC integration manifest
01f486f724a0 drm/i915/cnl: Unify dvfs level selection.
5c8e9f3d2c27 drm/i915/cnl: Invert dvfs default level.
8a2290d9eb61 drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement.
3d5ee32da8c1 drm/i915/cnl: Only request voltage frequency switching when needed.
87400176df23 drm/i915/cnl: DVFS for PLL disabling
d304cffe3c57 drm/i915/cnl: DVFS for PLL enabling
fe6fa00697d3 drm/i915/cnl: Expose DVFS change functions
c8013e5efe27 drm/i915/cnl: extract cnl_dvfs_{pre, post}_change
3d80f9e4830b drm/i915: Unify and export gen9+ port_clock calculation.
a1f590338947 drm/i915/skl: Extract cnl_calc_pll_link following bxt, cnl style.
f47c443cf27c drm/i915/cnl: Extract cnl_calc_pll_link following bxt style.
fd11eec0b2b8 drm/i915: Let's use more enum intel_dpll_id pll_id.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5865/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-09-30  0:31 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-30  0:08 [PATCH 00/12] CNL DVFS Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 01/12] drm/i915: Let's use more enum intel_dpll_id pll_id Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 02/12] drm/i915/cnl: Extract cnl_calc_pll_link following bxt style Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 03/12] drm/i915/skl: Extract cnl_calc_pll_link following bxt, cnl style Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 04/12] drm/i915: Unify and export gen9+ port_clock calculation Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 05/12] drm/i915/cnl: extract cnl_dvfs_{pre, post}_change Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 06/12] drm/i915/cnl: Expose DVFS change functions Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 07/12] drm/i915/cnl: DVFS for PLL enabling Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 08/12] drm/i915/cnl: DVFS for PLL disabling Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 09/12] drm/i915/cnl: Only request voltage frequency switching when needed Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 10/12] drm/i915/cnl: When disabling pll put dvfs back to cdclk requirement Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 11/12] drm/i915/cnl: Invert dvfs default level Rodrigo Vivi
2017-09-30  0:08 ` [PATCH 12/12] drm/i915/cnl: Unify dvfs level selection Rodrigo Vivi
2017-09-30  0:31 ` ✗ Fi.CI.BAT: failure for CNL DVFS Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.