All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/19] ddi: respin of runtime PM for DPMS
@ 2014-06-25 19:01 Imre Deak
  2014-06-25 19:01 ` [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll Imre Deak
                   ` (20 more replies)
  0 siblings, 21 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx

This is a respin of the unmerged part of Daniel's runtime PM for DPMS
patchset [1]. The original one also included a refactoring of the DDI
PCH/CRT encoder modesetting path, I left the corresponding patches out
from this series. This is because there hasn't been yet an agreement on
those parts, but people would like to see the RPM DPMS support already
applied.

Some patches needed to be updated/rebased because of the above omission,
but these weren't anywhere significant so I just marked the fact
with my s-o-b line. I also added two minor change to keep the
modeset sequence at its current order and collected all the reviewed-by
lines.

Tested on HSW DP/VGA, with basic DPMS on/off and igt/pm_rpm.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2014-April/044179.html

Daniel Vetter (17):
  drm/i915: Check hw state in assert_can_disable_lcpll
  drm/i915: Remove spll_refcount for hsw
  drm/i915: Clean up WRPLL/SPLL #defines
  drm/i915: Move the SPLL enabling into hsw_crt_pre_enable
  drm/i915: Move SPLL disabling into hsw_crt_post_disable
  drm/i915: Add a debugfs file for the shared dpll state
  drm/i915: Move ddi_pll_sel into the pipe config
  drm/i915: State readout and cross-checking for ddi_pll_sel
  drm/i915: Precompute static ddi_pll_sel values in encoders
  drm/i915: Basic shared dpll support for WRPLLs
  drm/i915: Document that the pll->mode_set hook is optional
  drm/i915: State readout support for WRPLLs
  drm/i915: ->disable hook for WRPLLs
  drm/i915: ->enable hook for WRPLLs
  drm/i915: Switch to common shared dpll framework for WRPLLs
  drm/i915: Only touch WRPLL hw state in enable/disable hooks
  drm/i915: ddi: enable runtime pm during dpms

Imre Deak (2):
  drm/i915: ddi: move pch setup after encoder->pre_enable
  drm/i915: ddi: move pch cleanup before encoder->post_disable

 drivers/gpu/drm/i915/i915_debugfs.c  |  27 +++
 drivers/gpu/drm/i915/i915_drv.h      |  16 +-
 drivers/gpu/drm/i915/i915_reg.h      |  10 +-
 drivers/gpu/drm/i915/intel_crt.c     |  32 +++-
 drivers/gpu/drm/i915/intel_ddi.c     | 340 +++++++----------------------------
 drivers/gpu/drm/i915/intel_display.c | 157 +++++++++-------
 drivers/gpu/drm/i915/intel_dp.c      |  23 ++-
 drivers/gpu/drm/i915/intel_drv.h     |  14 +-
 8 files changed, 258 insertions(+), 361 deletions(-)

-- 
1.8.4

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

* [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-30 20:59   ` Paulo Zanoni
  2014-06-25 19:01 ` [PATCH 02/19] drm/i915: Remove spll_refcount for hsw Imre Deak
                   ` (19 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

All the other checks also check hw state, so checking our software
refcounts for the plls looks a bit odd. Also this will simplify the
conversion over to the shared dpll framework, which itself has massive
amounts of checks to make sure that we never leave a display pll
enabled when we shouldn't.

So after that conversion we should stil have a good enough coverage of
asserts for entering pc8/runtime pm on hsw/bdw.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 065984d..2442a88 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7322,7 +7322,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
-	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	struct intel_crtc *crtc;
 
 	for_each_intel_crtc(dev, crtc)
@@ -7330,9 +7329,9 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
 		     pipe_name(crtc->pipe));
 
 	WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
-	WARN(plls->spll_refcount, "SPLL enabled\n");
-	WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
-	WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
+	WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL enabled\n");
+	WARN(I915_READ(WRPLL_CTL1) & WRPLL_PLL_ENABLE, "WRPLL1 enabled\n");
+	WARN(I915_READ(WRPLL_CTL2) & WRPLL_PLL_ENABLE, "WRPLL2 enabled\n");
 	WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
 	WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
 	     "CPU PWM1 enabled\n");
-- 
1.8.4

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

* [PATCH 02/19] drm/i915: Remove spll_refcount for hsw
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
  2014-06-25 19:01 ` [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 03/19] drm/i915: Clean up WRPLL/SPLL #defines Imre Deak
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

SPLL would be a reference clock we could potentially share,
especially if we want to use the SSC mode. But currently we
don't, so let's rip out this complexity for a simpler conversion
to the new display pll framework.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_ddi.c | 41 +++++++++++++---------------------------
 2 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8cea596..bdc578b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -229,7 +229,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
 			    struct intel_link_m_n *m_n);
 
 struct intel_ddi_plls {
-	int spll_refcount;
 	int wrpll1_refcount;
 	int wrpll2_refcount;
 };
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ded6013..9f02281 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -394,14 +394,11 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 
 	switch (intel_crtc->ddi_pll_sel) {
 	case PORT_CLK_SEL_SPLL:
-		plls->spll_refcount--;
-		if (plls->spll_refcount == 0) {
-			DRM_DEBUG_KMS("Disabling SPLL\n");
-			val = I915_READ(SPLL_CTL);
-			WARN_ON(!(val & SPLL_PLL_ENABLE));
-			I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
-			POSTING_READ(SPLL_CTL);
-		}
+		DRM_DEBUG_KMS("Disabling SPLL\n");
+		val = I915_READ(SPLL_CTL);
+		WARN_ON(!(val & SPLL_PLL_ENABLE));
+		I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
+		POSTING_READ(SPLL_CTL);
 		break;
 	case PORT_CLK_SEL_WRPLL1:
 		plls->wrpll1_refcount--;
@@ -425,7 +422,6 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 		break;
 	}
 
-	WARN(plls->spll_refcount < 0, "Invalid SPLL refcount\n");
 	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
 	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
 
@@ -821,16 +817,9 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 		}
 
 	} else if (type == INTEL_OUTPUT_ANALOG) {
-		if (plls->spll_refcount == 0) {
-			DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
-				      pipe_name(pipe));
-			plls->spll_refcount++;
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
-		} else {
-			DRM_ERROR("SPLL already in use\n");
-			return false;
-		}
-
+		DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
+			      pipe_name(pipe));
+		intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
 	} else {
 		WARN(1, "Invalid DDI encoder type %d\n", type);
 		return false;
@@ -869,13 +858,13 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 		return;
 
 	case PORT_CLK_SEL_SPLL:
-		pll_name = "SPLL";
-		reg = SPLL_CTL;
-		refcount = plls->spll_refcount;
 		new_val = SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz |
 			  SPLL_PLL_SSC;
-		break;
-
+		WARN(I915_READ(SPLL_CTL) & enable_bit, "SPLL already enabled\n");
+		I915_WRITE(SPLL_CTL, new_val);
+		POSTING_READ(SPLL_CTL);
+		udelay(20);
+		return;
 	case PORT_CLK_SEL_WRPLL1:
 	case PORT_CLK_SEL_WRPLL2:
 		if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
@@ -1186,7 +1175,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 	enum pipe pipe;
 	struct intel_crtc *intel_crtc;
 
-	dev_priv->ddi_plls.spll_refcount = 0;
 	dev_priv->ddi_plls.wrpll1_refcount = 0;
 	dev_priv->ddi_plls.wrpll2_refcount = 0;
 
@@ -1203,9 +1191,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 								 pipe);
 
 		switch (intel_crtc->ddi_pll_sel) {
-		case PORT_CLK_SEL_SPLL:
-			dev_priv->ddi_plls.spll_refcount++;
-			break;
 		case PORT_CLK_SEL_WRPLL1:
 			dev_priv->ddi_plls.wrpll1_refcount++;
 			break;
-- 
1.8.4

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

* [PATCH 03/19] drm/i915: Clean up WRPLL/SPLL #defines
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
  2014-06-25 19:01 ` [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll Imre Deak
  2014-06-25 19:01 ` [PATCH 02/19] drm/i915: Remove spll_refcount for hsw Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 04/19] drm/i915: ddi: move pch setup after encoder->pre_enable Imre Deak
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Luckily the bit definitions match, but it's still confusing
to use one when handling the other. So sprinkle some OCD over
the #defines to make them match and use the right version in
each place.

Maybe we should unify these definitions completely, but that
can always be done sometime in the future.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  7 ++++---
 drivers/gpu/drm/i915/intel_ddi.c | 12 ++++++------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3488567..5db1959 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,9 +5900,10 @@ enum punit_power_well {
 #define WRPLL_CTL1			0x46040
 #define WRPLL_CTL2			0x46060
 #define  WRPLL_PLL_ENABLE		(1<<31)
-#define  WRPLL_PLL_SELECT_SSC		(0x01<<28)
-#define  WRPLL_PLL_SELECT_NON_SSC	(0x02<<28)
-#define  WRPLL_PLL_SELECT_LCPLL_2700	(0x03<<28)
+#define  WRPLL_PLL_SSC			(1<<28)
+#define  WRPLL_PLL_NON_SSC		(2<<28)
+#define  WRPLL_PLL_LCPLL		(3<<28)
+#define  WRPLL_PLL_REF_MASK		(3<<28)
 /* WRPLL divider programming */
 #define  WRPLL_DIVIDER_REFERENCE(x)	((x)<<0)
 #define  WRPLL_DIVIDER_REF_MASK		(0xff)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9f02281..16c9163 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -588,9 +588,9 @@ static int intel_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
 	u32 wrpll;
 
 	wrpll = I915_READ(reg);
-	switch (wrpll & SPLL_PLL_REF_MASK) {
-	case SPLL_PLL_SSC:
-	case SPLL_PLL_NON_SSC:
+	switch (wrpll & WRPLL_PLL_REF_MASK) {
+	case WRPLL_PLL_SSC:
+	case WRPLL_PLL_NON_SSC:
 		/*
 		 * We could calculate spread here, but our checking
 		 * code only cares about 5% accuracy, and spread is a max of
@@ -598,7 +598,7 @@ static int intel_ddi_calc_wrpll_link(struct drm_i915_private *dev_priv,
 		 */
 		refclk = 135;
 		break;
-	case SPLL_PLL_LCPLL:
+	case WRPLL_PLL_LCPLL:
 		refclk = LC_FREQ;
 		break;
 	default:
@@ -780,7 +780,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 
 		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
 
-		val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
+		val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
 		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
 		      WRPLL_DIVIDER_POST(p);
 
@@ -879,7 +879,7 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 
 		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
 
-		new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_SELECT_LCPLL_2700 |
+		new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
 			  WRPLL_DIVIDER_REFERENCE(r2) |
 			  WRPLL_DIVIDER_FEEDBACK(n2) | WRPLL_DIVIDER_POST(p);
 
-- 
1.8.4

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

* [PATCH 04/19] drm/i915: ddi: move pch setup after encoder->pre_enable
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (2 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 03/19] drm/i915: Clean up WRPLL/SPLL #defines Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 05/19] drm/i915: ddi: move pch cleanup before encoder->post_disable Imre Deak
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx

This is needed by an upcoming patch that moves the PCH/CRT PLL enabling
into the pre_enable hook, after which we want to keep the modeset
sequence at its current state. At this point this won't have an effect
since the PCH/CRT pre_enable hook is atm a NOP.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2442a88..b480d86 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4145,15 +4145,14 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc->active = true;
 
 	intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
-	if (intel_crtc->config.has_pch_encoder)
-		intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		if (encoder->pre_enable)
+			encoder->pre_enable(encoder);
 
-	if (intel_crtc->config.has_pch_encoder)
+	if (intel_crtc->config.has_pch_encoder) {
+		intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
 		dev_priv->display.fdi_link_train(crtc);
-
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->pre_enable)
-			encoder->pre_enable(encoder);
+	}
 
 	intel_ddi_enable_pipe_clock(intel_crtc);
 
-- 
1.8.4

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

* [PATCH 05/19] drm/i915: ddi: move pch cleanup before encoder->post_disable
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (3 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 04/19] drm/i915: ddi: move pch setup after encoder->pre_enable Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 06/19] drm/i915: Move the SPLL enabling into hsw_crt_pre_enable Imre Deak
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx

This is needed by an upcoming patch that moves the PCH/CRT PLL disabling
into the post_disable hook, after which we want to keep the modeset
sequence at its current state. At this point this won't have an effect
since the PCH/CRT post_disable hook is atm a NOP.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b480d86..006b41b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4291,16 +4291,16 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 
 	intel_ddi_disable_pipe_clock(intel_crtc);
 
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		if (encoder->post_disable)
-			encoder->post_disable(encoder);
-
 	if (intel_crtc->config.has_pch_encoder) {
 		lpt_disable_pch_transcoder(dev_priv);
 		intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
 		intel_ddi_fdi_disable(crtc);
 	}
 
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		if (encoder->post_disable)
+			encoder->post_disable(encoder);
+
 	intel_crtc->active = false;
 	intel_update_watermarks(crtc);
 
-- 
1.8.4

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

* [PATCH 06/19] drm/i915: Move the SPLL enabling into hsw_crt_pre_enable
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (4 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 05/19] drm/i915: ddi: move pch cleanup before encoder->post_disable Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 07/19] drm/i915: Move SPLL disabling into hsw_crt_post_disable Imre Deak
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

The call to intel_ddi_pll_enable in haswell_crtc_mode_set is the only
function that still touches the hardware state from the crtc mode_set
callback on hsw. Since the SPLL isn't ever shared we can easily take
it out into the hsw crt encoder functions.

Temporarily we'll loose a bit of WARN_ON coverage with this, but once
the WRPLLs are switched over that will be restored. For the SPLL
selection add a WARN in the hsw fdi link training code.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 13 +++++++++++++
 drivers/gpu/drm/i915/intel_ddi.c | 19 +------------------
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 8da5ef9..a4bdf257 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -137,6 +137,18 @@ static void hsw_crt_get_config(struct intel_encoder *encoder,
 	pipe_config->adjusted_mode.flags |= intel_crt_get_flags(encoder);
 }
 
+static void hsw_crt_pre_enable(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL already enabled\n");
+	I915_WRITE(SPLL_CTL,
+		   SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC);
+	POSTING_READ(SPLL_CTL);
+	udelay(20);
+}
+
 /* Note: The caller is required to filter out dpms modes not supported by the
  * platform. */
 static void intel_crt_set_dpms(struct intel_encoder *encoder, int mode)
@@ -860,6 +872,7 @@ void intel_crt_init(struct drm_device *dev)
 	if (HAS_DDI(dev)) {
 		crt->base.get_config = hsw_crt_get_config;
 		crt->base.get_hw_state = intel_ddi_get_hw_state;
+		crt->base.pre_enable = hsw_crt_pre_enable;
 	} else {
 		crt->base.get_config = intel_crt_get_config;
 		crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 16c9163..accacb9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -278,6 +278,7 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 
 	/* Configure Port Clock Select */
 	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
+	WARN_ON(intel_crtc->ddi_pll_sel != PORT_CLK_SEL_SPLL);
 
 	/* Start the training iterating through available voltages and emphasis,
 	 * testing each value twice. */
@@ -848,23 +849,6 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 	BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
 
 	switch (crtc->ddi_pll_sel) {
-	case PORT_CLK_SEL_LCPLL_2700:
-	case PORT_CLK_SEL_LCPLL_1350:
-	case PORT_CLK_SEL_LCPLL_810:
-		/*
-		 * LCPLL should always be enabled at this point of the mode set
-		 * sequence, so nothing to do.
-		 */
-		return;
-
-	case PORT_CLK_SEL_SPLL:
-		new_val = SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz |
-			  SPLL_PLL_SSC;
-		WARN(I915_READ(SPLL_CTL) & enable_bit, "SPLL already enabled\n");
-		I915_WRITE(SPLL_CTL, new_val);
-		POSTING_READ(SPLL_CTL);
-		udelay(20);
-		return;
 	case PORT_CLK_SEL_WRPLL1:
 	case PORT_CLK_SEL_WRPLL2:
 		if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
@@ -889,7 +873,6 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 		WARN(1, "Bad selected pll: PORT_CLK_SEL_NONE\n");
 		return;
 	default:
-		WARN(1, "Bad selected pll: 0x%08x\n", crtc->ddi_pll_sel);
 		return;
 	}
 
-- 
1.8.4

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

* [PATCH 07/19] drm/i915: Move SPLL disabling into hsw_crt_post_disable
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (5 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 06/19] drm/i915: Move the SPLL enabling into hsw_crt_pre_enable Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 08/19] drm/i915: Add a debugfs file for the shared dpll state Imre Deak
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Similar to how the ->crtc_mode_set hook should touch the hardware to
enable anything the ->crtc_off hook should disable anything in the
hardware. Otherwise runtime pm for dpms will not work.

Currently the only things left int the haswell_crtc_off hook is
disabling the ddi plls. We can't move the WRPLL enabling out yet
because the current ddi pll sharing code used by the haswell code
doesn't separately track active users and overall users. This must be
fixed by porting it to the generic shared display pll framework, which
is powerful enough.

But the SPLL source is only used by the crt encoder and so can be
moved already. We only need to make sure that the ddi port E is
already off, which hsw_fdi_disable does by calling
intel_ddi_post_disable.

With this the code reorg to shuffle hsw fdi/lpt specific code into a
new hsw-specific crt encoder type is now finally complete.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/intel_ddi.c |  7 -------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index a4bdf257..76ffa2c 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -206,6 +206,20 @@ static void intel_disable_crt(struct intel_encoder *encoder)
 	intel_crt_set_dpms(encoder, DRM_MODE_DPMS_OFF);
 }
 
+
+static void hsw_crt_post_disable(struct intel_encoder *encoder)
+{
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t val;
+
+	DRM_DEBUG_KMS("Disabling SPLL\n");
+	val = I915_READ(SPLL_CTL);
+	WARN_ON(!(val & SPLL_PLL_ENABLE));
+	I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
+	POSTING_READ(SPLL_CTL);
+}
+
 static void intel_enable_crt(struct intel_encoder *encoder)
 {
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
@@ -873,6 +887,7 @@ void intel_crt_init(struct drm_device *dev)
 		crt->base.get_config = hsw_crt_get_config;
 		crt->base.get_hw_state = intel_ddi_get_hw_state;
 		crt->base.pre_enable = hsw_crt_pre_enable;
+		crt->base.post_disable = hsw_crt_post_disable;
 	} else {
 		crt->base.get_config = intel_crt_get_config;
 		crt->base.get_hw_state = intel_crt_get_hw_state;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index accacb9..3582270 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -394,13 +394,6 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 	uint32_t val;
 
 	switch (intel_crtc->ddi_pll_sel) {
-	case PORT_CLK_SEL_SPLL:
-		DRM_DEBUG_KMS("Disabling SPLL\n");
-		val = I915_READ(SPLL_CTL);
-		WARN_ON(!(val & SPLL_PLL_ENABLE));
-		I915_WRITE(SPLL_CTL, val & ~SPLL_PLL_ENABLE);
-		POSTING_READ(SPLL_CTL);
-		break;
 	case PORT_CLK_SEL_WRPLL1:
 		plls->wrpll1_refcount--;
 		if (plls->wrpll1_refcount == 0) {
-- 
1.8.4

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

* [PATCH 08/19] drm/i915: Add a debugfs file for the shared dpll state
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (6 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 07/19] drm/i915: Move SPLL disabling into hsw_crt_post_disable Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 09/19] drm/i915: Move ddi_pll_sel into the pipe config Imre Deak
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a93b3bf..a0d8733 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2388,6 +2388,31 @@ static int i915_display_info(struct seq_file *m, void *unused)
 	return 0;
 }
 
+static int i915_shared_dplls_info(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = (struct drm_info_node *) m->private;
+	struct drm_device *dev = node->minor->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
+
+	drm_modeset_lock_all(dev);
+	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
+
+		seq_printf(m, "DPLL%i: %s, id: %i\n", i, pll->name, pll->id);
+		seq_printf(m, " refcount: %i, active: %i, on: %s\n", pll->refcount,
+			   pll->active, yesno(pll->on));
+		seq_printf(m, " tracked hardware state:\n");
+		seq_printf(m, " dpll:    0x%08x\n", pll->hw_state.dpll);
+		seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
+		seq_printf(m, " fp0:     0x%08x\n", pll->hw_state.fp0);
+		seq_printf(m, " fp1:     0x%08x\n", pll->hw_state.fp1);
+	}
+	drm_modeset_unlock_all(dev);
+
+	return 0;
+}
+
 struct pipe_crc_info {
 	const char *name;
 	struct drm_device *dev;
@@ -3839,6 +3864,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_pc8_status", i915_pc8_status, 0},
 	{"i915_power_domain_info", i915_power_domain_info, 0},
 	{"i915_display_info", i915_display_info, 0},
+	{"i915_shared_dplls_info", i915_shared_dplls_info, 0},
 };
 #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)
 
-- 
1.8.4

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

* [PATCH 09/19] drm/i915: Move ddi_pll_sel into the pipe config
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (7 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 08/19] drm/i915: Add a debugfs file for the shared dpll state Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 10/19] drm/i915: State readout and cross-checking for ddi_pll_sel Imre Deak
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Just boring sed job for preparation.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 34 +++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_drv.h |  5 +++--
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 3582270..c60d92a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -277,8 +277,8 @@ void hsw_fdi_link_train(struct drm_crtc *crtc)
 	I915_WRITE(_FDI_RXA_CTL, rx_ctl_val);
 
 	/* Configure Port Clock Select */
-	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->ddi_pll_sel);
-	WARN_ON(intel_crtc->ddi_pll_sel != PORT_CLK_SEL_SPLL);
+	I915_WRITE(PORT_CLK_SEL(PORT_E), intel_crtc->config.ddi_pll_sel);
+	WARN_ON(intel_crtc->config.ddi_pll_sel != PORT_CLK_SEL_SPLL);
 
 	/* Start the training iterating through available voltages and emphasis,
 	 * testing each value twice. */
@@ -393,7 +393,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t val;
 
-	switch (intel_crtc->ddi_pll_sel) {
+	switch (intel_crtc->config.ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 		plls->wrpll1_refcount--;
 		if (plls->wrpll1_refcount == 0) {
@@ -419,7 +419,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
 	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
 
-	intel_crtc->ddi_pll_sel = PORT_CLK_SEL_NONE;
+	intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 }
 
 #define LC_FREQ 2700
@@ -754,13 +754,13 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 
 		switch (intel_dp->link_bw) {
 		case DP_LINK_BW_1_62:
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
 			break;
 		case DP_LINK_BW_2_7:
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
 			break;
 		case DP_LINK_BW_5_4:
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
 			break;
 		default:
 			DRM_ERROR("Link bandwidth %d unsupported\n",
@@ -804,16 +804,16 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 
 		if (reg == WRPLL_CTL1) {
 			plls->wrpll1_refcount++;
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
 		} else {
 			plls->wrpll2_refcount++;
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
 		}
 
 	} else if (type == INTEL_OUTPUT_ANALOG) {
 		DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
 			      pipe_name(pipe));
-		intel_crtc->ddi_pll_sel = PORT_CLK_SEL_SPLL;
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
 	} else {
 		WARN(1, "Invalid DDI encoder type %d\n", type);
 		return false;
@@ -841,10 +841,10 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 	BUILD_BUG_ON(enable_bit != SPLL_PLL_ENABLE);
 	BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
 
-	switch (crtc->ddi_pll_sel) {
+	switch (crtc->config.ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 	case PORT_CLK_SEL_WRPLL2:
-		if (crtc->ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
+		if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
 			pll_name = "WRPLL1";
 			reg = WRPLL_CTL1;
 			refcount = plls->wrpll1_refcount;
@@ -1159,14 +1159,14 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 			to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
 
 		if (!intel_crtc->active) {
-			intel_crtc->ddi_pll_sel = PORT_CLK_SEL_NONE;
+			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 			continue;
 		}
 
-		intel_crtc->ddi_pll_sel = intel_ddi_get_crtc_pll(dev_priv,
+		intel_crtc->config.ddi_pll_sel = intel_ddi_get_crtc_pll(dev_priv,
 								 pipe);
 
-		switch (intel_crtc->ddi_pll_sel) {
+		switch (intel_crtc->config.ddi_pll_sel) {
 		case PORT_CLK_SEL_WRPLL1:
 			dev_priv->ddi_plls.wrpll1_refcount++;
 			break;
@@ -1222,8 +1222,8 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 		intel_edp_panel_on(intel_dp);
 	}
 
-	WARN_ON(crtc->ddi_pll_sel == PORT_CLK_SEL_NONE);
-	I915_WRITE(PORT_CLK_SEL(port), crtc->ddi_pll_sel);
+	WARN_ON(crtc->config.ddi_pll_sel == PORT_CLK_SEL_NONE);
+	I915_WRITE(PORT_CLK_SEL(port), crtc->config.ddi_pll_sel);
 
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..482bc2d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -307,6 +307,9 @@ struct intel_crtc_config {
 	/* Selected dpll when shared or DPLL_ID_PRIVATE. */
 	enum intel_dpll_id shared_dpll;
 
+	/* PORT_CLK_SEL for DDI ports. */
+	uint32_t ddi_pll_sel;
+
 	/* Actual register state of the dpll, for shared dpll cross-checking. */
 	struct intel_dpll_hw_state dpll_hw_state;
 
@@ -398,8 +401,6 @@ struct intel_crtc {
 	struct intel_crtc_config *new_config;
 	bool new_enabled;
 
-	uint32_t ddi_pll_sel;
-
 	/* reset counter value when the last flip was submitted */
 	unsigned int reset_counter;
 
-- 
1.8.4

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

* [PATCH 10/19] drm/i915: State readout and cross-checking for ddi_pll_sel
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (8 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 09/19] drm/i915: Move ddi_pll_sel into the pipe config Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Imre Deak
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

To make things a bit more manageable extract a new function for
reading out common ddi port state. This means a bit of duplication
between encoders and the core since both look at the same registers,
but doesn't seem worth to make a fuzz about.

We can also remove the state readout code in intel_ddi_setup_hw_pll_state.
That code is only called from the hardware take over and not the cross
check code, and only after the crtc state is reconstructed. So we can
rely on an accurate value of crtc->config.ddi_pll_sel already.

Compared to the old code also trust the hw state more and don't
special-case port A - we want to cross-check the actual-state, not
bake in our own assumptions about how this is supposed to all be
linked up.

v2: Make use of the read-out ddi_pll_sel in intel_ddi_clock_get.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_ddi.c     | 40 +-----------------------------
 drivers/gpu/drm/i915/intel_display.c | 48 ++++++++++++++++++++++++------------
 3 files changed, 34 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5db1959..3d61a53 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5772,6 +5772,7 @@ enum punit_power_well {
 #define  TRANS_DDI_FUNC_ENABLE		(1<<31)
 /* Those bits are ignored by pipe EDP since it can only connect to DDI A */
 #define  TRANS_DDI_PORT_MASK		(7<<28)
+#define  TRANS_DDI_PORT_SHIFT		28
 #define  TRANS_DDI_SELECT_PORT(x)	((x)<<28)
 #define  TRANS_DDI_PORT_NONE		(0<<28)
 #define  TRANS_DDI_MODE_SELECT_MASK	(7<<24)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c60d92a..5356e3e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -612,11 +612,10 @@ static void intel_ddi_clock_get(struct intel_encoder *encoder,
 				struct intel_crtc_config *pipe_config)
 {
 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
-	enum port port = intel_ddi_get_encoder_port(encoder);
 	int link_clock = 0;
 	u32 val, pll;
 
-	val = I915_READ(PORT_CLK_SEL(port));
+	val = pipe_config->ddi_pll_sel;
 	switch (val & PORT_CLK_SEL_MASK) {
 	case PORT_CLK_SEL_LCPLL_810:
 		link_clock = 81000;
@@ -1111,40 +1110,6 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	return false;
 }
 
-static uint32_t intel_ddi_get_crtc_pll(struct drm_i915_private *dev_priv,
-				       enum pipe pipe)
-{
-	uint32_t temp, ret;
-	enum port port = I915_MAX_PORTS;
-	enum transcoder cpu_transcoder = intel_pipe_to_cpu_transcoder(dev_priv,
-								      pipe);
-	int i;
-
-	if (cpu_transcoder == TRANSCODER_EDP) {
-		port = PORT_A;
-	} else {
-		temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-		temp &= TRANS_DDI_PORT_MASK;
-
-		for (i = PORT_B; i <= PORT_E; i++)
-			if (temp == TRANS_DDI_SELECT_PORT(i))
-				port = i;
-	}
-
-	if (port == I915_MAX_PORTS) {
-		WARN(1, "Pipe %c enabled on an unknown port\n",
-		     pipe_name(pipe));
-		ret = PORT_CLK_SEL_NONE;
-	} else {
-		ret = I915_READ(PORT_CLK_SEL(port));
-		DRM_DEBUG_KMS("Pipe %c connected to port %c using clock "
-			      "0x%08x\n", pipe_name(pipe), port_name(port),
-			      ret);
-	}
-
-	return ret;
-}
-
 void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1163,9 +1128,6 @@ void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
 			continue;
 		}
 
-		intel_crtc->config.ddi_pll_sel = intel_ddi_get_crtc_pll(dev_priv,
-								 pipe);
-
 		switch (intel_crtc->config.ddi_pll_sel) {
 		case PORT_CLK_SEL_WRPLL1:
 			dev_priv->ddi_plls.wrpll1_refcount++;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 006b41b..ea8cc58 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7569,6 +7569,35 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	return 0;
 }
 
+static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
+				       struct intel_crtc_config *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum port port;
+	uint32_t tmp;
+
+	tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
+
+	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
+
+	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
+	/*
+	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
+	 * DDI E. So just check whether this pipe is wired to DDI E and whether
+	 * the PCH transcoder is on.
+	 */
+	if ((port == PORT_E) && I915_READ(LPT_TRANSCONF) & TRANS_ENABLE) {
+		pipe_config->has_pch_encoder = true;
+
+		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
+		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
+					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
+
+		ironlake_get_fdi_m_n_config(crtc, pipe_config);
+	}
+}
+
 static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 				    struct intel_crtc_config *pipe_config)
 {
@@ -7614,22 +7643,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	if (!(tmp & PIPECONF_ENABLE))
 		return false;
 
-	/*
-	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
-	 * DDI E. So just check whether this pipe is wired to DDI E and whether
-	 * the PCH transcoder is on.
-	 */
-	tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config->cpu_transcoder));
-	if ((tmp & TRANS_DDI_PORT_MASK) == TRANS_DDI_SELECT_PORT(PORT_E) &&
-	    I915_READ(LPT_TRANSCONF) & TRANS_ENABLE) {
-		pipe_config->has_pch_encoder = true;
-
-		tmp = I915_READ(FDI_RX_CTL(PIPE_A));
-		pipe_config->fdi_lanes = ((FDI_DP_PORT_WIDTH_MASK & tmp) >>
-					  FDI_DP_PORT_WIDTH_SHIFT) + 1;
-
-		ironlake_get_fdi_m_n_config(crtc, pipe_config);
-	}
+	haswell_get_ddi_port_state(crtc, pipe_config);
 
 	intel_get_pipe_timings(crtc, pipe_config);
 
@@ -10392,6 +10406,8 @@ intel_pipe_config_compare(struct drm_device *dev,
 
 	PIPE_CONF_CHECK_I(double_wide);
 
+	PIPE_CONF_CHECK_X(ddi_pll_sel);
+
 	PIPE_CONF_CHECK_I(shared_dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll);
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
-- 
1.8.4

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

* [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (9 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 10/19] drm/i915: State readout and cross-checking for ddi_pll_sel Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-07-02 17:17   ` Paulo Zanoni
  2014-07-04 14:26   ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Paulo Zanoni
  2014-06-25 19:01 ` [PATCH 12/19] drm/i915: Basic shared dpll support for WRPLLs Imre Deak
                   ` (9 subsequent siblings)
  20 siblings, 2 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This way only the dynamic WRPLL selection for hdmi ddi mode is
done in intel_ddi_pll_select.

v2: Don't clobber the precomputed values when selecting clocks fro
hdmi encoders.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_crt.c |  4 +++-
 drivers/gpu/drm/i915/intel_ddi.c | 34 +++-------------------------------
 drivers/gpu/drm/i915/intel_dp.c  | 23 ++++++++++++++++++++---
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 76ffa2c..88db4b6 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -315,8 +315,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
 		pipe_config->pipe_bpp = 24;
 
 	/* FDI must always be 2.7 GHz */
-	if (HAS_DDI(dev))
+	if (HAS_DDI(dev)) {
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
 		pipe_config->port_clock = 135000 * 2;
+	}
 
 	return true;
 }
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5356e3e..6e976ba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -403,6 +403,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 			I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
 			POSTING_READ(WRPLL_CTL1);
 		}
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
 	case PORT_CLK_SEL_WRPLL2:
 		plls->wrpll2_refcount--;
@@ -413,13 +414,12 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 			I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
 			POSTING_READ(WRPLL_CTL2);
 		}
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
 	}
 
 	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
 	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
-
-	intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 }
 
 #define LC_FREQ 2700
@@ -739,7 +739,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
-	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	int type = intel_encoder->type;
@@ -748,26 +747,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 
 	intel_ddi_put_crtc_pll(crtc);
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
-		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
-		switch (intel_dp->link_bw) {
-		case DP_LINK_BW_1_62:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
-			break;
-		case DP_LINK_BW_2_7:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
-			break;
-		case DP_LINK_BW_5_4:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
-			break;
-		default:
-			DRM_ERROR("Link bandwidth %d unsupported\n",
-				  intel_dp->link_bw);
-			return false;
-		}
-
-	} else if (type == INTEL_OUTPUT_HDMI) {
+	if (type == INTEL_OUTPUT_HDMI) {
 		uint32_t reg, val;
 		unsigned p, n2, r2;
 
@@ -808,14 +788,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 			plls->wrpll2_refcount++;
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
 		}
-
-	} else if (type == INTEL_OUTPUT_ANALOG) {
-		DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
-			      pipe_name(pipe));
-		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
-	} else {
-		WARN(1, "Invalid DDI encoder type %d\n", type);
-		return false;
 	}
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..9a2ede0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -746,6 +746,22 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
 }
 
 static void
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
+{
+	switch (link_bw) {
+	case DP_LINK_BW_1_62:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+		break;
+	case DP_LINK_BW_2_7:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
+		break;
+	case DP_LINK_BW_5_4:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
+		break;
+	}
+}
+
+static void
 intel_dp_set_clock(struct intel_encoder *encoder,
 		   struct intel_crtc_config *pipe_config, int link_bw)
 {
@@ -756,8 +772,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 	if (IS_G4X(dev)) {
 		divisor = gen4_dpll;
 		count = ARRAY_SIZE(gen4_dpll);
-	} else if (IS_HASWELL(dev)) {
-		/* Haswell has special-purpose DP DDI clocks. */
 	} else if (HAS_PCH_SPLIT(dev)) {
 		divisor = pch_dpll;
 		count = ARRAY_SIZE(pch_dpll);
@@ -928,7 +942,10 @@ found:
 				&pipe_config->dp_m2_n2);
 	}
 
-	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
+	if (IS_HASWELL(dev))
+		hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
+	else
+		intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
 
 	return true;
 }
-- 
1.8.4

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

* [PATCH 12/19] drm/i915: Basic shared dpll support for WRPLLs
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (10 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 13/19] drm/i915: Document that the pll->mode_set hook is optional Imre Deak
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Just filing in names and ids, but not yet officially registering them
so that the hw state cross checker doesn't completely freak out about
them. Still since we do already read out and cross check
config->shared_dpll the basics are now there to flesh out the wrpll
shared dpll implementation.

The idea is now to roll out all the callbacks step-by-step and then at
the end switch to the shared dpll framework. This way hw and sw
changes are clearly separated.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
[imre: added const to hsw_ddi_pll_names (Damien)]
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++--
 drivers/gpu/drm/i915/intel_ddi.c     | 17 +++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 21 +++++++++++++--------
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bdc578b..7b0cbe4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -184,8 +184,10 @@ struct i915_mmu_object;
 enum intel_dpll_id {
 	DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */
 	/* real shared dpll ids must be >= 0 */
-	DPLL_ID_PCH_PLL_A,
-	DPLL_ID_PCH_PLL_B,
+	DPLL_ID_PCH_PLL_A = 0,
+	DPLL_ID_PCH_PLL_B = 1,
+	DPLL_ID_WRPLL1 = 0,
+	DPLL_ID_WRPLL2 = 1,
 };
 #define I915_NUM_PLLS 2
 
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 6e976ba..fae73d3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -784,9 +784,11 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 		if (reg == WRPLL_CTL1) {
 			plls->wrpll1_refcount++;
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
+			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL1;
 		} else {
 			plls->wrpll2_refcount++;
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
+			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
 		}
 	}
 
@@ -1313,10 +1315,25 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
+static char *hsw_ddi_pll_names[] = {
+	"WRPLL 1",
+	"WRPLL 2",
+};
+
 void intel_ddi_pll_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t val = I915_READ(LCPLL_CTL);
+	int i;
+
+	/* Dummy setup until everything is moved over to avoid upsetting the hw
+	 * state cross checker. */
+	dev_priv->num_shared_dpll = 0;
+
+	for (i = 0; i < 2; i++) {
+		dev_priv->shared_dplls[i].id = i;
+		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+	}
 
 	/* The LCPLL register should be turned on by the BIOS. For now let's
 	 * just check its state and print errors in case something is wrong.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ea8cc58..2b83c07 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7582,6 +7582,16 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
 
 	pipe_config->ddi_pll_sel = I915_READ(PORT_CLK_SEL(port));
+
+	switch (pipe_config->ddi_pll_sel) {
+	case PORT_CLK_SEL_WRPLL1:
+		pipe_config->shared_dpll = DPLL_ID_WRPLL1;
+		break;
+	case PORT_CLK_SEL_WRPLL2:
+		pipe_config->shared_dpll = DPLL_ID_WRPLL2;
+		break;
+	}
+
 	/*
 	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
 	 * DDI E. So just check whether this pipe is wired to DDI E and whether
@@ -11273,12 +11283,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.page_flip = intel_crtc_page_flip,
 };
 
-static void intel_cpu_pll_init(struct drm_device *dev)
-{
-	if (HAS_DDI(dev))
-		intel_ddi_pll_init(dev);
-}
-
 static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
 				      struct intel_shared_dpll *pll,
 				      struct intel_dpll_hw_state *hw_state)
@@ -11366,7 +11370,9 @@ static void intel_shared_dpll_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
+	if (HAS_DDI(dev))
+		intel_ddi_pll_init(dev);
+	else if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
 		ibx_pch_dpll_init(dev);
 	else
 		dev_priv->num_shared_dpll = 0;
@@ -12494,7 +12500,6 @@ void intel_modeset_init(struct drm_device *dev)
 	intel_init_dpio(dev);
 	intel_reset_dpio(dev);
 
-	intel_cpu_pll_init(dev);
 	intel_shared_dpll_init(dev);
 
 	/* Just disable it once at startup */
-- 
1.8.4

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

* [PATCH 13/19] drm/i915: Document that the pll->mode_set hook is optional
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (11 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 12/19] drm/i915: Basic shared dpll support for WRPLLs Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-06-25 19:01 ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Imre Deak
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

The WRPLLs won't use them.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7b0cbe4..650a5ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -206,6 +206,8 @@ struct intel_shared_dpll {
 	/* should match the index in the dev_priv->shared_dplls array */
 	enum intel_dpll_id id;
 	struct intel_dpll_hw_state hw_state;
+	/* The mode_set hook is optional and should be used together with the
+	 * intel_prepare_shared_dpll function. */
 	void (*mode_set)(struct drm_i915_private *dev_priv,
 			 struct intel_shared_dpll *pll);
 	void (*enable)(struct drm_i915_private *dev_priv,
-- 
1.8.4

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

* [PATCH 14/19] drm/i915: State readout support for WRPLLs
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (12 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 13/19] drm/i915: Document that the pll->mode_set hook is optional Imre Deak
@ 2014-06-25 19:01 ` Imre Deak
  2014-07-04 14:27   ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Paulo Zanoni
  2014-06-25 19:02 ` [PATCH 15/19] drm/i915: ->disable hook for WRPLLs Imre Deak
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:01 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Still tacked onto the side, but slowly getting there.

v2: Don't forget the debugfs file.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  1 +
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_ddi.c     | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 5 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a0d8733..2dee463 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2407,6 +2407,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 		seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
 		seq_printf(m, " fp0:     0x%08x\n", pll->hw_state.fp0);
 		seq_printf(m, " fp1:     0x%08x\n", pll->hw_state.fp1);
+		seq_printf(m, " wrpll:   0x%08x\n", pll->hw_state.wrpll);
 	}
 	drm_modeset_unlock_all(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 650a5ad..855f055 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -196,6 +196,7 @@ struct intel_dpll_hw_state {
 	uint32_t dpll_md;
 	uint32_t fp0;
 	uint32_t fp1;
+	uint32_t wrpll;
 };
 
 struct intel_shared_dpll {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3d61a53..654417e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,6 +5900,7 @@ enum punit_power_well {
 /* WRPLL */
 #define WRPLL_CTL1			0x46040
 #define WRPLL_CTL2			0x46060
+#define WRPLL_CTL(pll)			(pll == 0 ? WRPLL_CTL1 : WRPLL_CTL2)
 #define  WRPLL_PLL_ENABLE		(1<<31)
 #define  WRPLL_PLL_SSC			(1<<28)
 #define  WRPLL_PLL_NON_SSC		(2<<28)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fae73d3..9539405 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -790,6 +790,8 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
 			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
 		}
+
+		intel_crtc->config.dpll_hw_state.wrpll = val;
 	}
 
 	return true;
@@ -1315,6 +1317,18 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
+static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
+				     struct intel_shared_dpll *pll,
+				     struct intel_dpll_hw_state *hw_state)
+{
+	uint32_t val;
+
+	val = I915_READ(WRPLL_CTL(pll->id));
+	hw_state->wrpll = val;
+
+	return val & WRPLL_PLL_ENABLE;
+}
+
 static char *hsw_ddi_pll_names[] = {
 	"WRPLL 1",
 	"WRPLL 2",
@@ -1333,6 +1347,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	for (i = 0; i < 2; i++) {
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+		dev_priv->shared_dplls[i].get_hw_state =
+			hsw_ddi_pll_get_hw_state;
 	}
 
 	/* The LCPLL register should be turned on by the BIOS. For now let's
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2b83c07..d8277a1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7574,6 +7574,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_shared_dpll *pll;
 	enum port port;
 	uint32_t tmp;
 
@@ -7592,6 +7593,13 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 		break;
 	}
 
+	if (pipe_config->shared_dpll >= 0) {
+		pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
+
+		WARN_ON(!pll->get_hw_state(dev_priv, pll,
+					   &pipe_config->dpll_hw_state));
+	}
+
 	/*
 	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
 	 * DDI E. So just check whether this pipe is wired to DDI E and whether
@@ -10423,6 +10431,7 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
+	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
 
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
-- 
1.8.4

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

* [PATCH 15/19] drm/i915: ->disable hook for WRPLLs
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (13 preceding siblings ...)
  2014-06-25 19:01 ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Imre Deak
@ 2014-06-25 19:02 ` Imre Deak
  2014-06-25 19:02 ` [PATCH 16/19] drm/i915: ->enable " Imre Deak
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:02 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Currently still with a redudant WARN_ON in there, the common shared
dpll code will take care of this in the future.

Also we need to flip the switch for the transitional hack now to make
sure that we disable the right pll.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 26 +++++++++++++++-----------
 drivers/gpu/drm/i915/intel_display.c |  8 +++++---
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9539405..61c2471 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -391,28 +391,20 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t val;
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
 
 	switch (intel_crtc->config.ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 		plls->wrpll1_refcount--;
 		if (plls->wrpll1_refcount == 0) {
-			DRM_DEBUG_KMS("Disabling WRPLL 1\n");
-			val = I915_READ(WRPLL_CTL1);
-			WARN_ON(!(val & WRPLL_PLL_ENABLE));
-			I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
-			POSTING_READ(WRPLL_CTL1);
+			pll->disable(dev_priv, pll);
 		}
 		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
 	case PORT_CLK_SEL_WRPLL2:
 		plls->wrpll2_refcount--;
 		if (plls->wrpll2_refcount == 0) {
-			DRM_DEBUG_KMS("Disabling WRPLL 2\n");
-			val = I915_READ(WRPLL_CTL2);
-			WARN_ON(!(val & WRPLL_PLL_ENABLE));
-			I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
-			POSTING_READ(WRPLL_CTL2);
+			pll->disable(dev_priv, pll);
 		}
 		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
@@ -1317,6 +1309,17 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
+				struct intel_shared_dpll *pll)
+{
+	uint32_t val;
+
+	val = I915_READ(WRPLL_CTL(pll->id));
+	WARN_ON(!(val & WRPLL_PLL_ENABLE));
+	I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
+	POSTING_READ(WRPLL_CTL(pll->id));
+}
+
 static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				     struct intel_shared_dpll *pll,
 				     struct intel_dpll_hw_state *hw_state)
@@ -1347,6 +1350,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	for (i = 0; i < 2; i++) {
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
 		dev_priv->shared_dplls[i].get_hw_state =
 			hsw_ddi_pll_get_hw_state;
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d8277a1..1fab569 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5245,9 +5245,11 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
 	if (HAS_IPS(dev))
 		hsw_compute_ips_config(crtc, pipe_config);
 
-	/* XXX: PCH clock sharing is done in ->mode_set, so make sure the old
-	 * clock survives for now. */
-	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev))
+	/*
+	 * XXX: PCH/WRPLL clock sharing is done in ->mode_set, so make sure the
+	 * old clock survives for now.
+	 */
+	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev) || HAS_DDI(dev))
 		pipe_config->shared_dpll = crtc->config.shared_dpll;
 
 	if (pipe_config->has_pch_encoder)
-- 
1.8.4

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

* [PATCH 16/19] drm/i915: ->enable hook for WRPLLs
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (14 preceding siblings ...)
  2014-06-25 19:02 ` [PATCH 15/19] drm/i915: ->disable hook for WRPLLs Imre Deak
@ 2014-06-25 19:02 ` Imre Deak
  2014-06-25 19:02 ` [PATCH 17/19] drm/i915: Switch to common shared dpll framework " Imre Deak
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:02 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This time around another cute hack to pre-fill the pll->hw_state with
the right values. And also remove a bunch of checks which will be
replaced by lots more checks in the common framework.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 51 +++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 61c2471..d1569d0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -740,6 +740,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 	intel_ddi_put_crtc_pll(crtc);
 
 	if (type == INTEL_OUTPUT_HDMI) {
+		struct intel_shared_dpll *pll;
 		uint32_t reg, val;
 		unsigned p, n2, r2;
 
@@ -784,6 +785,9 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 		}
 
 		intel_crtc->config.dpll_hw_state.wrpll = val;
+
+		pll = &dev_priv->shared_dplls[intel_crtc->config.shared_dpll];
+		pll->hw_state.wrpll = val;
 	}
 
 	return true;
@@ -798,54 +802,24 @@ void intel_ddi_pll_enable(struct intel_crtc *crtc)
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
-	int clock = crtc->config.port_clock;
-	uint32_t reg, cur_val, new_val;
 	int refcount;
-	const char *pll_name;
-	uint32_t enable_bit = (1 << 31);
-	unsigned int p, n2, r2;
-
-	BUILD_BUG_ON(enable_bit != SPLL_PLL_ENABLE);
-	BUILD_BUG_ON(enable_bit != WRPLL_PLL_ENABLE);
+	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
 	switch (crtc->config.ddi_pll_sel) {
 	case PORT_CLK_SEL_WRPLL1:
 	case PORT_CLK_SEL_WRPLL2:
 		if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
-			pll_name = "WRPLL1";
-			reg = WRPLL_CTL1;
 			refcount = plls->wrpll1_refcount;
 		} else {
-			pll_name = "WRPLL2";
-			reg = WRPLL_CTL2;
 			refcount = plls->wrpll2_refcount;
 		}
-
-		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
-
-		new_val = WRPLL_PLL_ENABLE | WRPLL_PLL_LCPLL |
-			  WRPLL_DIVIDER_REFERENCE(r2) |
-			  WRPLL_DIVIDER_FEEDBACK(n2) | WRPLL_DIVIDER_POST(p);
-
 		break;
-
-	case PORT_CLK_SEL_NONE:
-		WARN(1, "Bad selected pll: PORT_CLK_SEL_NONE\n");
-		return;
 	default:
 		return;
 	}
 
-	cur_val = I915_READ(reg);
-
-	WARN(refcount < 1, "Bad %s refcount: %d\n", pll_name, refcount);
 	if (refcount == 1) {
-		WARN(cur_val & enable_bit, "%s already enabled\n", pll_name);
-		I915_WRITE(reg, new_val);
-		POSTING_READ(reg);
-		udelay(20);
-	} else {
-		WARN((cur_val & enable_bit) == 0, "%s disabled\n", pll_name);
+		pll->enable(dev_priv, pll);
 	}
 }
 
@@ -1309,6 +1283,18 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
+			       struct intel_shared_dpll *pll)
+{
+	uint32_t cur_val;
+
+	cur_val = I915_READ(WRPLL_CTL(pll->id));
+	WARN(cur_val & WRPLL_PLL_ENABLE, "%s already enabled\n", pll->name);
+	I915_WRITE(WRPLL_CTL(pll->id), pll->hw_state.wrpll);
+	POSTING_READ(WRPLL_CTL(pll->id));
+	udelay(20);
+}
+
 static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll)
 {
@@ -1351,6 +1337,7 @@ void intel_ddi_pll_init(struct drm_device *dev)
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
 		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
+		dev_priv->shared_dplls[i].enable = hsw_ddi_pll_enable;
 		dev_priv->shared_dplls[i].get_hw_state =
 			hsw_ddi_pll_get_hw_state;
 	}
-- 
1.8.4

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

* [PATCH 17/19] drm/i915: Switch to common shared dpll framework for WRPLLs
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (15 preceding siblings ...)
  2014-06-25 19:02 ` [PATCH 16/19] drm/i915: ->enable " Imre Deak
@ 2014-06-25 19:02 ` Imre Deak
  2014-06-25 19:02 ` [PATCH 18/19] drm/i915: Only touch WRPLL hw state in enable/disable hooks Imre Deak
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:02 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Mostly this patch is one big excersize in deleting code and asserts
which are no longer needed. Note that we still abuse the shared dpll
framework a bit since we call the enable/disable functions from the
crtc mode_set and off hooks. But changing the actual hardware sequence
will be done in the next step.

Note that besides the massive amount of changes in this patch the
places and order in which the low-level WRPLL code is called is
absolutely unchanged.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
[imre: rebased on patchset version w/o pch/crt/fdi refactoring]
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   6 --
 drivers/gpu/drm/i915/i915_reg.h      |   1 +
 drivers/gpu/drm/i915/intel_ddi.c     | 143 ++++-------------------------------
 drivers/gpu/drm/i915/intel_display.c |  14 ++--
 drivers/gpu/drm/i915/intel_drv.h     |   9 ++-
 5 files changed, 28 insertions(+), 145 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 855f055..bf88f2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -233,11 +233,6 @@ void intel_link_compute_m_n(int bpp, int nlanes,
 			    int pixel_clock, int link_clock,
 			    struct intel_link_m_n *m_n);
 
-struct intel_ddi_plls {
-	int wrpll1_refcount;
-	int wrpll2_refcount;
-};
-
 /* Interface history:
  *
  * 1.1: Original.
@@ -1485,7 +1480,6 @@ struct drm_i915_private {
 
 	int num_shared_dpll;
 	struct intel_shared_dpll shared_dplls[I915_NUM_PLLS];
-	struct intel_ddi_plls ddi_plls;
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
 	/* Reclocking support */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 654417e..a7e2ec8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5924,6 +5924,7 @@ enum punit_power_well {
 #define  PORT_CLK_SEL_LCPLL_1350	(1<<29)
 #define  PORT_CLK_SEL_LCPLL_810		(2<<29)
 #define  PORT_CLK_SEL_SPLL		(3<<29)
+#define  PORT_CLK_SEL_WRPLL(pll)	(((pll)+4)<<29)
 #define  PORT_CLK_SEL_WRPLL1		(4<<29)
 #define  PORT_CLK_SEL_WRPLL2		(5<<29)
 #define  PORT_CLK_SEL_NONE		(7<<29)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index d1569d0..4619ee2 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -388,30 +388,12 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
 
 void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
-	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(intel_crtc);
 
-	switch (intel_crtc->config.ddi_pll_sel) {
-	case PORT_CLK_SEL_WRPLL1:
-		plls->wrpll1_refcount--;
-		if (plls->wrpll1_refcount == 0) {
-			pll->disable(dev_priv, pll);
-		}
-		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
-		break;
-	case PORT_CLK_SEL_WRPLL2:
-		plls->wrpll2_refcount--;
-		if (plls->wrpll2_refcount == 0) {
-			pll->disable(dev_priv, pll);
-		}
-		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
-		break;
-	}
+	if (intel_crtc_to_shared_dpll(intel_crtc))
+		intel_disable_shared_dpll(intel_crtc);
 
-	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
-	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
+	intel_put_shared_dpll(intel_crtc);
 }
 
 #define LC_FREQ 2700
@@ -731,17 +713,14 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
-	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
-	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	int type = intel_encoder->type;
-	enum pipe pipe = intel_crtc->pipe;
 	int clock = intel_crtc->config.port_clock;
 
 	intel_ddi_put_crtc_pll(crtc);
 
 	if (type == INTEL_OUTPUT_HDMI) {
 		struct intel_shared_dpll *pll;
-		uint32_t reg, val;
+		uint32_t val;
 		unsigned p, n2, r2;
 
 		intel_ddi_calculate_wrpll(clock * 1000, &r2, &n2, &p);
@@ -750,79 +729,21 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
 		      WRPLL_DIVIDER_POST(p);
 
-		if (val == I915_READ(WRPLL_CTL1)) {
-			DRM_DEBUG_KMS("Reusing WRPLL 1 on pipe %c\n",
-				      pipe_name(pipe));
-			reg = WRPLL_CTL1;
-		} else if (val == I915_READ(WRPLL_CTL2)) {
-			DRM_DEBUG_KMS("Reusing WRPLL 2 on pipe %c\n",
-				      pipe_name(pipe));
-			reg = WRPLL_CTL2;
-		} else if (plls->wrpll1_refcount == 0) {
-			DRM_DEBUG_KMS("Using WRPLL 1 on pipe %c\n",
-				      pipe_name(pipe));
-			reg = WRPLL_CTL1;
-		} else if (plls->wrpll2_refcount == 0) {
-			DRM_DEBUG_KMS("Using WRPLL 2 on pipe %c\n",
-				      pipe_name(pipe));
-			reg = WRPLL_CTL2;
-		} else {
-			DRM_ERROR("No WRPLLs available!\n");
-			return false;
-		}
-
-		DRM_DEBUG_KMS("WRPLL: %dKHz refresh rate with p=%d, n2=%d r2=%d\n",
-			      clock, p, n2, r2);
-
-		if (reg == WRPLL_CTL1) {
-			plls->wrpll1_refcount++;
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL1;
-			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL1;
-		} else {
-			plls->wrpll2_refcount++;
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
-			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
-		}
-
 		intel_crtc->config.dpll_hw_state.wrpll = val;
 
-		pll = &dev_priv->shared_dplls[intel_crtc->config.shared_dpll];
-		pll->hw_state.wrpll = val;
+		pll = intel_get_shared_dpll(intel_crtc);
+		if (pll == NULL) {
+			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
+					 pipe_name(intel_crtc->pipe));
+			return false;
+		}
+
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
 	}
 
 	return true;
 }
 
-/*
- * To be called after intel_ddi_pll_select(). That one selects the PLL to be
- * used, this one actually enables the PLL.
- */
-void intel_ddi_pll_enable(struct intel_crtc *crtc)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
-	int refcount;
-	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
-
-	switch (crtc->config.ddi_pll_sel) {
-	case PORT_CLK_SEL_WRPLL1:
-	case PORT_CLK_SEL_WRPLL2:
-		if (crtc->config.ddi_pll_sel == PORT_CLK_SEL_WRPLL1) {
-			refcount = plls->wrpll1_refcount;
-		} else {
-			refcount = plls->wrpll2_refcount;
-		}
-		break;
-	default:
-		return;
-	}
-
-	if (refcount == 1) {
-		pll->enable(dev_priv, pll);
-	}
-}
-
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
@@ -1052,35 +973,6 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	return false;
 }
 
-void intel_ddi_setup_hw_pll_state(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe;
-	struct intel_crtc *intel_crtc;
-
-	dev_priv->ddi_plls.wrpll1_refcount = 0;
-	dev_priv->ddi_plls.wrpll2_refcount = 0;
-
-	for_each_pipe(pipe) {
-		intel_crtc =
-			to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
-
-		if (!intel_crtc->active) {
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
-			continue;
-		}
-
-		switch (intel_crtc->config.ddi_pll_sel) {
-		case PORT_CLK_SEL_WRPLL1:
-			dev_priv->ddi_plls.wrpll1_refcount++;
-			break;
-		case PORT_CLK_SEL_WRPLL2:
-			dev_priv->ddi_plls.wrpll2_refcount++;
-			break;
-		}
-	}
-}
-
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
@@ -1286,10 +1178,6 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 static void hsw_ddi_pll_enable(struct drm_i915_private *dev_priv,
 			       struct intel_shared_dpll *pll)
 {
-	uint32_t cur_val;
-
-	cur_val = I915_READ(WRPLL_CTL(pll->id));
-	WARN(cur_val & WRPLL_PLL_ENABLE, "%s already enabled\n", pll->name);
 	I915_WRITE(WRPLL_CTL(pll->id), pll->hw_state.wrpll);
 	POSTING_READ(WRPLL_CTL(pll->id));
 	udelay(20);
@@ -1301,7 +1189,6 @@ static void hsw_ddi_pll_disable(struct drm_i915_private *dev_priv,
 	uint32_t val;
 
 	val = I915_READ(WRPLL_CTL(pll->id));
-	WARN_ON(!(val & WRPLL_PLL_ENABLE));
 	I915_WRITE(WRPLL_CTL(pll->id), val & ~WRPLL_PLL_ENABLE);
 	POSTING_READ(WRPLL_CTL(pll->id));
 }
@@ -1329,11 +1216,9 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	uint32_t val = I915_READ(LCPLL_CTL);
 	int i;
 
-	/* Dummy setup until everything is moved over to avoid upsetting the hw
-	 * state cross checker. */
-	dev_priv->num_shared_dpll = 0;
+	dev_priv->num_shared_dpll = 2;
 
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
 		dev_priv->shared_dplls[i].disable = hsw_ddi_pll_disable;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1fab569..1688913 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1842,7 +1842,7 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	pll->on = true;
 }
 
-static void intel_disable_shared_dpll(struct intel_crtc *crtc)
+void intel_disable_shared_dpll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3635,7 +3635,7 @@ static void lpt_pch_enable(struct drm_crtc *crtc)
 	lpt_enable_pch_transcoder(dev_priv, cpu_transcoder);
 }
 
-static void intel_put_shared_dpll(struct intel_crtc *crtc)
+void intel_put_shared_dpll(struct intel_crtc *crtc)
 {
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
@@ -3655,7 +3655,7 @@ static void intel_put_shared_dpll(struct intel_crtc *crtc)
 	crtc->config.shared_dpll = DPLL_ID_PRIVATE;
 }
 
-static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
+struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
@@ -7564,7 +7564,9 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	if (!intel_ddi_pll_select(intel_crtc))
 		return -EINVAL;
-	intel_ddi_pll_enable(intel_crtc);
+
+	if (intel_crtc_to_shared_dpll(intel_crtc))
+		intel_enable_shared_dpll(intel_crtc);
 
 	intel_crtc->lowfreq_avail = false;
 
@@ -12819,10 +12821,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			      crtc->active ? "enabled" : "disabled");
 	}
 
-	/* FIXME: Smash this into the new shared dpll infrastructure. */
-	if (HAS_DDI(dev))
-		intel_ddi_setup_hw_pll_state(dev);
-
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 482bc2d..8a31428 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -707,9 +707,7 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
 				       enum transcoder cpu_transcoder);
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
 void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
-void intel_ddi_setup_hw_pll_state(struct drm_device *dev);
 bool intel_ddi_pll_select(struct intel_crtc *crtc);
-void intel_ddi_pll_enable(struct intel_crtc *crtc);
 void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
@@ -794,12 +792,19 @@ __intel_framebuffer_create(struct drm_device *dev,
 void intel_prepare_page_flip(struct drm_device *dev, int plane);
 void intel_finish_page_flip(struct drm_device *dev, int pipe);
 void intel_finish_page_flip_plane(struct drm_device *dev, int plane);
+
+/* shared dpll functions */
 struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			struct intel_shared_dpll *pll,
 			bool state);
 #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
 #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
+void intel_disable_shared_dpll(struct intel_crtc *crtc);
+struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
+void intel_put_shared_dpll(struct intel_crtc *crtc);
+
+/* modesetting asserts */
 void assert_pll(struct drm_i915_private *dev_priv,
 		enum pipe pipe, bool state);
 #define assert_pll_enabled(d, p) assert_pll(d, p, true)
-- 
1.8.4

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

* [PATCH 18/19] drm/i915: Only touch WRPLL hw state in enable/disable hooks
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (16 preceding siblings ...)
  2014-06-25 19:02 ` [PATCH 17/19] drm/i915: Switch to common shared dpll framework " Imre Deak
@ 2014-06-25 19:02 ` Imre Deak
  2014-06-25 19:02 ` [PATCH 19/19] drm/i915: ddi: enable runtime pm during dpms Imre Deak
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:02 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

To be able to do this we need to separately keep track of how many
crtcs need a given WRPLL and how many actually actively use it. The
common shared dpll framework already has all this, including massive
state readout and cross checking. Which allows us to do this switch in
a fairly small patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     | 12 +-----------
 drivers/gpu/drm/i915/intel_display.c | 15 +++++++--------
 drivers/gpu/drm/i915/intel_drv.h     |  2 --
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 4619ee2..64721e5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -386,16 +386,6 @@ intel_ddi_get_crtc_encoder(struct drm_crtc *crtc)
 	return ret;
 }
 
-void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	if (intel_crtc_to_shared_dpll(intel_crtc))
-		intel_disable_shared_dpll(intel_crtc);
-
-	intel_put_shared_dpll(intel_crtc);
-}
-
 #define LC_FREQ 2700
 #define LC_FREQ_2K (LC_FREQ * 2000)
 
@@ -716,7 +706,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 	int type = intel_encoder->type;
 	int clock = intel_crtc->config.port_clock;
 
-	intel_ddi_put_crtc_pll(crtc);
+	intel_put_shared_dpll(intel_crtc);
 
 	if (type == INTEL_OUTPUT_HDMI) {
 		struct intel_shared_dpll *pll;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1688913..62b6896 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4121,6 +4121,9 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->active)
 		return;
 
+	if (intel_crtc_to_shared_dpll(intel_crtc))
+		intel_enable_shared_dpll(intel_crtc);
+
 	if (intel_crtc->config.has_dp_encoder)
 		intel_dp_set_m_n(intel_crtc);
 
@@ -4307,6 +4310,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	mutex_lock(&dev->struct_mutex);
 	intel_update_fbc(dev);
 	mutex_unlock(&dev->struct_mutex);
+
+	if (intel_crtc_to_shared_dpll(intel_crtc))
+		intel_disable_shared_dpll(intel_crtc);
 }
 
 static void ironlake_crtc_off(struct drm_crtc *crtc)
@@ -4315,10 +4321,6 @@ static void ironlake_crtc_off(struct drm_crtc *crtc)
 	intel_put_shared_dpll(intel_crtc);
 }
 
-static void haswell_crtc_off(struct drm_crtc *crtc)
-{
-	intel_ddi_put_crtc_pll(crtc);
-}
 
 static void i9xx_pfit_enable(struct intel_crtc *crtc)
 {
@@ -7565,9 +7567,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	if (!intel_ddi_pll_select(intel_crtc))
 		return -EINVAL;
 
-	if (intel_crtc_to_shared_dpll(intel_crtc))
-		intel_enable_shared_dpll(intel_crtc);
-
 	intel_crtc->lowfreq_avail = false;
 
 	return 0;
@@ -12170,7 +12169,7 @@ static void intel_init_display(struct drm_device *dev)
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
 		dev_priv->display.crtc_enable = haswell_crtc_enable;
 		dev_priv->display.crtc_disable = haswell_crtc_disable;
-		dev_priv->display.off = haswell_crtc_off;
+		dev_priv->display.off = ironlake_crtc_off;
 		dev_priv->display.update_primary_plane =
 			ironlake_update_primary_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8a31428..57cbecd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -708,7 +708,6 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
 void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
 bool intel_ddi_pll_select(struct intel_crtc *crtc);
-void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
 void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
 bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector);
@@ -800,7 +799,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 			bool state);
 #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
 #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
-void intel_disable_shared_dpll(struct intel_crtc *crtc);
 struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc);
 void intel_put_shared_dpll(struct intel_crtc *crtc);
 
-- 
1.8.4

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

* [PATCH 19/19] drm/i915: ddi: enable runtime pm during dpms
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (17 preceding siblings ...)
  2014-06-25 19:02 ` [PATCH 18/19] drm/i915: Only touch WRPLL hw state in enable/disable hooks Imre Deak
@ 2014-06-25 19:02 ` Imre Deak
  2014-07-01 21:33 ` [PATCH 00/19] ddi: respin of runtime PM for DPMS Paulo Zanoni
  2014-07-04 14:30 ` [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT Paulo Zanoni
  20 siblings, 0 replies; 34+ messages in thread
From: Imre Deak @ 2014-06-25 19:02 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 62b6896..35e7c89 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4920,12 +4920,10 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
 			 * yet, so do runtime PM for DPMS only for all other
 			 * platforms for now.
 			 */
-			if (!HAS_DDI(dev)) {
-				domains = get_crtc_power_domains(crtc);
-				for_each_power_domain(domain, domains)
-					intel_display_power_get(dev_priv, domain);
-				intel_crtc->enabled_power_domains = domains;
-			}
+			domains = get_crtc_power_domains(crtc);
+			for_each_power_domain(domain, domains)
+				intel_display_power_get(dev_priv, domain);
+			intel_crtc->enabled_power_domains = domains;
 
 			dev_priv->display.crtc_enable(crtc);
 		}
@@ -4933,12 +4931,10 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
 		if (intel_crtc->active) {
 			dev_priv->display.crtc_disable(crtc);
 
-			if (!HAS_DDI(dev)) {
-				domains = intel_crtc->enabled_power_domains;
-				for_each_power_domain(domain, domains)
-					intel_display_power_put(dev_priv, domain);
-				intel_crtc->enabled_power_domains = 0;
-			}
+			domains = intel_crtc->enabled_power_domains;
+			for_each_power_domain(domain, domains)
+				intel_display_power_put(dev_priv, domain);
+			intel_crtc->enabled_power_domains = 0;
 		}
 	}
 
-- 
1.8.4

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

* Re: [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll
  2014-06-25 19:01 ` [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll Imre Deak
@ 2014-06-30 20:59   ` Paulo Zanoni
  0 siblings, 0 replies; 34+ messages in thread
From: Paulo Zanoni @ 2014-06-30 20:59 UTC (permalink / raw
  To: Imre Deak; +Cc: Daniel Vetter, Intel Graphics Development

2014-06-25 16:01 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> All the other checks also check hw state, so checking our software
> refcounts for the plls looks a bit odd. Also this will simplify the
> conversion over to the shared dpll framework, which itself has massive
> amounts of checks to make sure that we never leave a display pll
> enabled when we shouldn't.
>
> So after that conversion we should stil have a good enough coverage of
> asserts for entering pc8/runtime pm on hsw/bdw.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 065984d..2442a88 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7322,7 +7322,6 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
>  {
>         struct drm_device *dev = dev_priv->dev;
> -       struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
>         struct intel_crtc *crtc;
>
>         for_each_intel_crtc(dev, crtc)
> @@ -7330,9 +7329,9 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
>                      pipe_name(crtc->pipe));
>
>         WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
> -       WARN(plls->spll_refcount, "SPLL enabled\n");
> -       WARN(plls->wrpll1_refcount, "WRPLL1 enabled\n");
> -       WARN(plls->wrpll2_refcount, "WRPLL2 enabled\n");
> +       WARN(I915_READ(SPLL_CTL) & SPLL_PLL_ENABLE, "SPLL enabled\n");
> +       WARN(I915_READ(WRPLL_CTL1) & WRPLL_PLL_ENABLE, "WRPLL1 enabled\n");
> +       WARN(I915_READ(WRPLL_CTL2) & WRPLL_PLL_ENABLE, "WRPLL2 enabled\n");
>         WARN(I915_READ(PCH_PP_STATUS) & PP_ON, "Panel power on\n");
>         WARN(I915_READ(BLC_PWM_CPU_CTL2) & BLM_PWM_ENABLE,
>              "CPU PWM1 enabled\n");
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

* Re: [PATCH 00/19] ddi: respin of runtime PM for DPMS
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (18 preceding siblings ...)
  2014-06-25 19:02 ` [PATCH 19/19] drm/i915: ddi: enable runtime pm during dpms Imre Deak
@ 2014-07-01 21:33 ` Paulo Zanoni
  2014-07-10 20:15   ` Daniel Vetter
  2014-07-04 14:30 ` [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT Paulo Zanoni
  20 siblings, 1 reply; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-01 21:33 UTC (permalink / raw
  To: Imre Deak; +Cc: Intel Graphics Development

2014-06-25 16:01 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> This is a respin of the unmerged part of Daniel's runtime PM for DPMS
> patchset [1]. The original one also included a refactoring of the DDI
> PCH/CRT encoder modesetting path, I left the corresponding patches out
> from this series. This is because there hasn't been yet an agreement on
> those parts, but people would like to see the RPM DPMS support already
> applied.
>
> Some patches needed to be updated/rebased because of the above omission,
> but these weren't anywhere significant so I just marked the fact
> with my s-o-b line. I also added two minor change to keep the
> modeset sequence at its current order and collected all the reviewed-by
> lines.
>
> Tested on HSW DP/VGA, with basic DPMS on/off and igt/pm_rpm.

For patches 2, 4, 5, 6, 7, 19: Reviewed-by: Paulo Zanoni
<paulo.r.zanoni@intel.com>

However, I tested these patches on a HSW Machine with eDP+HDMI, and
there are WARNs on dmesg for the dpms-non-lpsp subtest. I found at
least two problems:
1 - Function hsw_ddi_pll_get_hw_state() reads registers while the
device is suspended.
2 - When _intel_set_mode() calls intel_crtc_disable(), it calls
assert_plane() which reads register 0x71180, which triggers an
"Unclaimed register" error. I didn't investigate this deeply, so I
don't have a suggestion for a solution.

I can reproduce these errors 100% of the time.

>
> [1]
> http://lists.freedesktop.org/archives/intel-gfx/2014-April/044179.html
>
> Daniel Vetter (17):
>   drm/i915: Check hw state in assert_can_disable_lcpll
>   drm/i915: Remove spll_refcount for hsw
>   drm/i915: Clean up WRPLL/SPLL #defines
>   drm/i915: Move the SPLL enabling into hsw_crt_pre_enable
>   drm/i915: Move SPLL disabling into hsw_crt_post_disable
>   drm/i915: Add a debugfs file for the shared dpll state
>   drm/i915: Move ddi_pll_sel into the pipe config
>   drm/i915: State readout and cross-checking for ddi_pll_sel
>   drm/i915: Precompute static ddi_pll_sel values in encoders
>   drm/i915: Basic shared dpll support for WRPLLs
>   drm/i915: Document that the pll->mode_set hook is optional
>   drm/i915: State readout support for WRPLLs
>   drm/i915: ->disable hook for WRPLLs
>   drm/i915: ->enable hook for WRPLLs
>   drm/i915: Switch to common shared dpll framework for WRPLLs
>   drm/i915: Only touch WRPLL hw state in enable/disable hooks
>   drm/i915: ddi: enable runtime pm during dpms
>
> Imre Deak (2):
>   drm/i915: ddi: move pch setup after encoder->pre_enable
>   drm/i915: ddi: move pch cleanup before encoder->post_disable
>
>  drivers/gpu/drm/i915/i915_debugfs.c  |  27 +++
>  drivers/gpu/drm/i915/i915_drv.h      |  16 +-
>  drivers/gpu/drm/i915/i915_reg.h      |  10 +-
>  drivers/gpu/drm/i915/intel_crt.c     |  32 +++-
>  drivers/gpu/drm/i915/intel_ddi.c     | 340 +++++++----------------------------
>  drivers/gpu/drm/i915/intel_display.c | 157 +++++++++-------
>  drivers/gpu/drm/i915/intel_dp.c      |  23 ++-
>  drivers/gpu/drm/i915/intel_drv.h     |  14 +-
>  8 files changed, 258 insertions(+), 361 deletions(-)
>
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

* Re: [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders
  2014-06-25 19:01 ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Imre Deak
@ 2014-07-02 17:17   ` Paulo Zanoni
  2014-07-04 14:26   ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Paulo Zanoni
  1 sibling, 0 replies; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-02 17:17 UTC (permalink / raw
  To: Imre Deak; +Cc: Daniel Vetter, Intel Graphics Development

2014-06-25 16:01 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> This way only the dynamic WRPLL selection for hdmi ddi mode is
> done in intel_ddi_pll_select.
>
> v2: Don't clobber the precomputed values when selecting clocks fro
> hdmi encoders.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_crt.c |  4 +++-
>  drivers/gpu/drm/i915/intel_ddi.c | 34 +++-------------------------------
>  drivers/gpu/drm/i915/intel_dp.c  | 23 ++++++++++++++++++++---
>  3 files changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 76ffa2c..88db4b6 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -315,8 +315,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
>                 pipe_config->pipe_bpp = 24;
>
>         /* FDI must always be 2.7 GHz */
> -       if (HAS_DDI(dev))
> +       if (HAS_DDI(dev)) {
> +               pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
>                 pipe_config->port_clock = 135000 * 2;
> +       }
>
>         return true;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5356e3e..6e976ba 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -403,6 +403,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
>                         I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
>                         POSTING_READ(WRPLL_CTL1);
>                 }
> +               intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
>                 break;
>         case PORT_CLK_SEL_WRPLL2:
>                 plls->wrpll2_refcount--;
> @@ -413,13 +414,12 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
>                         I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
>                         POSTING_READ(WRPLL_CTL2);
>                 }
> +               intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
>                 break;
>         }
>
>         WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
>         WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
> -
> -       intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
>  }
>
>  #define LC_FREQ 2700
> @@ -739,7 +739,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>  {
>         struct drm_crtc *crtc = &intel_crtc->base;
>         struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
> -       struct drm_encoder *encoder = &intel_encoder->base;
>         struct drm_i915_private *dev_priv = crtc->dev->dev_private;
>         struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
>         int type = intel_encoder->type;
> @@ -748,26 +747,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>
>         intel_ddi_put_crtc_pll(crtc);
>
> -       if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
> -               struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -
> -               switch (intel_dp->link_bw) {
> -               case DP_LINK_BW_1_62:
> -                       intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
> -                       break;
> -               case DP_LINK_BW_2_7:
> -                       intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
> -                       break;
> -               case DP_LINK_BW_5_4:
> -                       intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
> -                       break;
> -               default:
> -                       DRM_ERROR("Link bandwidth %d unsupported\n",
> -                                 intel_dp->link_bw);
> -                       return false;
> -               }
> -
> -       } else if (type == INTEL_OUTPUT_HDMI) {
> +       if (type == INTEL_OUTPUT_HDMI) {
>                 uint32_t reg, val;
>                 unsigned p, n2, r2;
>
> @@ -808,14 +788,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>                         plls->wrpll2_refcount++;
>                         intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
>                 }
> -
> -       } else if (type == INTEL_OUTPUT_ANALOG) {
> -               DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
> -                             pipe_name(pipe));
> -               intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
> -       } else {
> -               WARN(1, "Invalid DDI encoder type %d\n", type);
> -               return false;
>         }
>
>         return true;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b5ec489..9a2ede0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -746,6 +746,22 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
>  }
>
>  static void
> +hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
> +{
> +       switch (link_bw) {
> +       case DP_LINK_BW_1_62:
> +               pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
> +               break;
> +       case DP_LINK_BW_2_7:
> +               pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
> +               break;
> +       case DP_LINK_BW_5_4:
> +               pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
> +               break;
> +       }
> +}
> +
> +static void
>  intel_dp_set_clock(struct intel_encoder *encoder,
>                    struct intel_crtc_config *pipe_config, int link_bw)
>  {
> @@ -756,8 +772,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
>         if (IS_G4X(dev)) {
>                 divisor = gen4_dpll;
>                 count = ARRAY_SIZE(gen4_dpll);
> -       } else if (IS_HASWELL(dev)) {
> -               /* Haswell has special-purpose DP DDI clocks. */

Didn't this code also need an IS_BROADWELL or HAS_DDI?

>         } else if (HAS_PCH_SPLIT(dev)) {
>                 divisor = pch_dpll;
>                 count = ARRAY_SIZE(pch_dpll);
> @@ -928,7 +942,10 @@ found:
>                                 &pipe_config->dp_m2_n2);
>         }
>
> -       intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
> +       if (IS_HASWELL(dev))

Don't we need an IS_BROADWELL or HAS_DDI here?


> +               hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
> +       else
> +               intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
>
>         return true;
>  }
> --
> 1.8.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

* [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks
  2014-06-25 19:01 ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Imre Deak
  2014-07-02 17:17   ` Paulo Zanoni
@ 2014-07-04 14:26   ` Paulo Zanoni
  2014-07-04 14:26     ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Paulo Zanoni
  2014-07-04 14:30     ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Damien Lespiau
  1 sibling, 2 replies; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-04 14:26 UTC (permalink / raw
  To: intel-gfx; +Cc: Paulo Zanoni

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

Don't let it fall in the HAS_PCH_SPLIT() case.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..aedce65 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -756,7 +756,7 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 	if (IS_G4X(dev)) {
 		divisor = gen4_dpll;
 		count = ARRAY_SIZE(gen4_dpll);
-	} else if (IS_HASWELL(dev)) {
+	} else if (HAS_DDI(dev)) {
 		/* Haswell has special-purpose DP DDI clocks. */
 	} else if (HAS_PCH_SPLIT(dev)) {
 		divisor = pch_dpll;
-- 
2.0.0

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

* [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders
  2014-07-04 14:26   ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Paulo Zanoni
@ 2014-07-04 14:26     ` Paulo Zanoni
  2014-07-04 14:30     ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Damien Lespiau
  1 sibling, 0 replies; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-04 14:26 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This way only the dynamic WRPLL selection for hdmi ddi mode is
done in intel_ddi_pll_select.

v2: Don't clobber the precomputed values when selecting clocks fro
hdmi encoders.
v3 (from Paulo): Rebase on top of the s/IS_HASWELL/HAS_DDI/ patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_crt.c |  4 +++-
 drivers/gpu/drm/i915/intel_ddi.c | 34 +++-------------------------------
 drivers/gpu/drm/i915/intel_dp.c  | 23 ++++++++++++++++++++---
 3 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 76ffa2c..88db4b6 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -315,8 +315,10 @@ static bool intel_crt_compute_config(struct intel_encoder *encoder,
 		pipe_config->pipe_bpp = 24;
 
 	/* FDI must always be 2.7 GHz */
-	if (HAS_DDI(dev))
+	if (HAS_DDI(dev)) {
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_SPLL;
 		pipe_config->port_clock = 135000 * 2;
+	}
 
 	return true;
 }
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 5356e3e..6e976ba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -403,6 +403,7 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 			I915_WRITE(WRPLL_CTL1, val & ~WRPLL_PLL_ENABLE);
 			POSTING_READ(WRPLL_CTL1);
 		}
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
 	case PORT_CLK_SEL_WRPLL2:
 		plls->wrpll2_refcount--;
@@ -413,13 +414,12 @@ void intel_ddi_put_crtc_pll(struct drm_crtc *crtc)
 			I915_WRITE(WRPLL_CTL2, val & ~WRPLL_PLL_ENABLE);
 			POSTING_READ(WRPLL_CTL2);
 		}
+		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 		break;
 	}
 
 	WARN(plls->wrpll1_refcount < 0, "Invalid WRPLL1 refcount\n");
 	WARN(plls->wrpll2_refcount < 0, "Invalid WRPLL2 refcount\n");
-
-	intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_NONE;
 }
 
 #define LC_FREQ 2700
@@ -739,7 +739,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
-	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_ddi_plls *plls = &dev_priv->ddi_plls;
 	int type = intel_encoder->type;
@@ -748,26 +747,7 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 
 	intel_ddi_put_crtc_pll(crtc);
 
-	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
-		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
-		switch (intel_dp->link_bw) {
-		case DP_LINK_BW_1_62:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
-			break;
-		case DP_LINK_BW_2_7:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
-			break;
-		case DP_LINK_BW_5_4:
-			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
-			break;
-		default:
-			DRM_ERROR("Link bandwidth %d unsupported\n",
-				  intel_dp->link_bw);
-			return false;
-		}
-
-	} else if (type == INTEL_OUTPUT_HDMI) {
+	if (type == INTEL_OUTPUT_HDMI) {
 		uint32_t reg, val;
 		unsigned p, n2, r2;
 
@@ -808,14 +788,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 			plls->wrpll2_refcount++;
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
 		}
-
-	} else if (type == INTEL_OUTPUT_ANALOG) {
-		DRM_DEBUG_KMS("Using SPLL on pipe %c\n",
-			      pipe_name(pipe));
-		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_SPLL;
-	} else {
-		WARN(1, "Invalid DDI encoder type %d\n", type);
-		return false;
 	}
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index aedce65..69db121 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -746,6 +746,22 @@ intel_dp_connector_unregister(struct intel_connector *intel_connector)
 }
 
 static void
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_config *pipe_config, int link_bw)
+{
+	switch (link_bw) {
+	case DP_LINK_BW_1_62:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_810;
+		break;
+	case DP_LINK_BW_2_7:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_1350;
+		break;
+	case DP_LINK_BW_5_4:
+		pipe_config->ddi_pll_sel = PORT_CLK_SEL_LCPLL_2700;
+		break;
+	}
+}
+
+static void
 intel_dp_set_clock(struct intel_encoder *encoder,
 		   struct intel_crtc_config *pipe_config, int link_bw)
 {
@@ -756,8 +772,6 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 	if (IS_G4X(dev)) {
 		divisor = gen4_dpll;
 		count = ARRAY_SIZE(gen4_dpll);
-	} else if (HAS_DDI(dev)) {
-		/* Haswell has special-purpose DP DDI clocks. */
 	} else if (HAS_PCH_SPLIT(dev)) {
 		divisor = pch_dpll;
 		count = ARRAY_SIZE(pch_dpll);
@@ -928,7 +942,10 @@ found:
 				&pipe_config->dp_m2_n2);
 	}
 
-	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
+	if (HAS_DDI(dev))
+		hsw_dp_set_ddi_pll_sel(pipe_config, intel_dp->link_bw);
+	else
+		intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
 
 	return true;
 }
-- 
2.0.0

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

* [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS
  2014-06-25 19:01 ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Imre Deak
@ 2014-07-04 14:27   ` Paulo Zanoni
  2014-07-04 14:27     ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Paulo Zanoni
  2014-07-10 14:33     ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Damien Lespiau
  0 siblings, 2 replies; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-04 14:27 UTC (permalink / raw
  To: intel-gfx; +Cc: Paulo Zanoni

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

And get/put it when needed. The special thing about this commit is
that it will now return false in ibx_pch_dpll_get_hw_state() in case
the power domain is not enabled. This will fix some WARNs we have when
we run pm_rpm on SNB.

Testcase: igt/pm_rpm
Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=80463
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
 drivers/gpu/drm/i915/intel_pm.c      |  1 +
 4 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a89cc7a..e79ddbf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2134,6 +2134,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
 		return "VGA";
 	case POWER_DOMAIN_AUDIO:
 		return "AUDIO";
+	case POWER_DOMAIN_PLLS:
+		return "PLLS";
 	case POWER_DOMAIN_INIT:
 		return "INIT";
 	default:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b1f1518..2ec7cb6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -129,6 +129,7 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_PORT_OTHER,
 	POWER_DOMAIN_VGA,
 	POWER_DOMAIN_AUDIO,
+	POWER_DOMAIN_PLLS,
 	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3bbc798..1d919ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1837,6 +1837,8 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
 	}
 	WARN_ON(pll->on);
 
+	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
+
 	DRM_DEBUG_KMS("enabling %s\n", pll->name);
 	pll->enable(dev_priv, pll);
 	pll->on = true;
@@ -1873,6 +1875,8 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
 	DRM_DEBUG_KMS("disabling %s\n", pll->name);
 	pll->disable(dev_priv, pll);
 	pll->on = false;
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
 }
 
 static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
@@ -11298,6 +11302,9 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
 {
 	uint32_t val;
 
+	if (!intel_display_power_enabled(dev_priv, POWER_DOMAIN_PLLS))
+		return false;
+
 	val = I915_READ(PCH_DPLL(pll->id));
 	hw_state->dpll = val;
 	hw_state->fp0 = I915_READ(PCH_FP0(pll->id));
@@ -12864,6 +12871,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		DRM_DEBUG_KMS("%s hw state readout: refcount %i, on %i\n",
 			      pll->name, pll->refcount, pll->on);
+
+		if (pll->refcount)
+			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
 	}
 
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 31ae2b4..cf4c521 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6310,6 +6310,7 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
 	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_CRT) |			\
+	BIT(POWER_DOMAIN_PLLS) |			\
 	BIT(POWER_DOMAIN_INIT))
 #define HSW_DISPLAY_POWER_DOMAINS (				\
 	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
-- 
2.0.0

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

* [PATCH 14/19] drm/i915: State readout support for WRPLLs
  2014-07-04 14:27   ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Paulo Zanoni
@ 2014-07-04 14:27     ` Paulo Zanoni
  2014-07-10 14:35       ` Damien Lespiau
  2014-07-10 14:33     ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Damien Lespiau
  1 sibling, 1 reply; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-04 14:27 UTC (permalink / raw
  To: intel-gfx; +Cc: Daniel Vetter, Paulo Zanoni

From: Daniel Vetter <daniel.vetter@ffwll.ch>

Still tacked onto the side, but slowly getting there.

v2: Don't forget the debugfs file.

v3 (from Paulo): Don't forget to check the power domains.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  1 +
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/i915_reg.h      |  1 +
 drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
 5 files changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e79ddbf..7d72768 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2409,6 +2409,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
 		seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
 		seq_printf(m, " fp0:     0x%08x\n", pll->hw_state.fp0);
 		seq_printf(m, " fp1:     0x%08x\n", pll->hw_state.fp1);
+		seq_printf(m, " wrpll:   0x%08x\n", pll->hw_state.wrpll);
 	}
 	drm_modeset_unlock_all(dev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2ec7cb6..c1fa561 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -197,6 +197,7 @@ struct intel_dpll_hw_state {
 	uint32_t dpll_md;
 	uint32_t fp0;
 	uint32_t fp1;
+	uint32_t wrpll;
 };
 
 struct intel_shared_dpll {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3d61a53..654417e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,6 +5900,7 @@ enum punit_power_well {
 /* WRPLL */
 #define WRPLL_CTL1			0x46040
 #define WRPLL_CTL2			0x46060
+#define WRPLL_CTL(pll)			(pll == 0 ? WRPLL_CTL1 : WRPLL_CTL2)
 #define  WRPLL_PLL_ENABLE		(1<<31)
 #define  WRPLL_PLL_SSC			(1<<28)
 #define  WRPLL_PLL_NON_SSC		(2<<28)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index fae73d3..79cbb5e 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -790,6 +790,8 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
 			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
 			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
 		}
+
+		intel_crtc->config.dpll_hw_state.wrpll = val;
 	}
 
 	return true;
@@ -1315,6 +1317,21 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
+static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
+				     struct intel_shared_dpll *pll,
+				     struct intel_dpll_hw_state *hw_state)
+{
+	uint32_t val;
+
+	if (!intel_display_power_enabled(dev_priv, POWER_DOMAIN_PLLS))
+		return false;
+
+	val = I915_READ(WRPLL_CTL(pll->id));
+	hw_state->wrpll = val;
+
+	return val & WRPLL_PLL_ENABLE;
+}
+
 static char *hsw_ddi_pll_names[] = {
 	"WRPLL 1",
 	"WRPLL 2",
@@ -1333,6 +1350,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
 	for (i = 0; i < 2; i++) {
 		dev_priv->shared_dplls[i].id = i;
 		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
+		dev_priv->shared_dplls[i].get_hw_state =
+			hsw_ddi_pll_get_hw_state;
 	}
 
 	/* The LCPLL register should be turned on by the BIOS. For now let's
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1d919ae..594a49f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7587,6 +7587,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_shared_dpll *pll;
 	enum port port;
 	uint32_t tmp;
 
@@ -7605,6 +7606,13 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 		break;
 	}
 
+	if (pipe_config->shared_dpll >= 0) {
+		pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
+
+		WARN_ON(!pll->get_hw_state(dev_priv, pll,
+					   &pipe_config->dpll_hw_state));
+	}
+
 	/*
 	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
 	 * DDI E. So just check whether this pipe is wired to DDI E and whether
@@ -10436,6 +10444,7 @@ intel_pipe_config_compare(struct drm_device *dev,
 	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
 	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
+	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
 
 	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
 		PIPE_CONF_CHECK_I(pipe_bpp);
-- 
2.0.0

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

* [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT
  2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
                   ` (19 preceding siblings ...)
  2014-07-01 21:33 ` [PATCH 00/19] ddi: respin of runtime PM for DPMS Paulo Zanoni
@ 2014-07-04 14:30 ` Paulo Zanoni
  2014-07-10 14:40   ` Damien Lespiau
  20 siblings, 1 reply; 34+ messages in thread
From: Paulo Zanoni @ 2014-07-04 14:30 UTC (permalink / raw
  To: intel-gfx; +Cc: Paulo Zanoni

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

Since we now have support for shared DPLLS.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d5861bf..5187341 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1094,11 +1094,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
 	bool cur_state;
 	struct intel_dpll_hw_state hw_state;
 
-	if (HAS_PCH_LPT(dev_priv->dev)) {
-		DRM_DEBUG_DRIVER("LPT detected: skipping PCH PLL test\n");
-		return;
-	}
-
 	if (WARN (!pll,
 		  "asserting DPLL %s with no DPLL\n", state_string(state)))
 		return;
-- 
2.0.0

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

* Re: [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks
  2014-07-04 14:26   ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Paulo Zanoni
  2014-07-04 14:26     ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Paulo Zanoni
@ 2014-07-04 14:30     ` Damien Lespiau
  1 sibling, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2014-07-04 14:30 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jul 04, 2014 at 11:26:03AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Don't let it fall in the HAS_PCH_SPLIT() case.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b5ec489..aedce65 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -756,7 +756,7 @@ intel_dp_set_clock(struct intel_encoder *encoder,
>  	if (IS_G4X(dev)) {
>  		divisor = gen4_dpll;
>  		count = ARRAY_SIZE(gen4_dpll);
> -	} else if (IS_HASWELL(dev)) {
> +	} else if (HAS_DDI(dev)) {
>  		/* Haswell has special-purpose DP DDI clocks. */
>  	} else if (HAS_PCH_SPLIT(dev)) {
>  		divisor = pch_dpll;
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS
  2014-07-04 14:27   ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Paulo Zanoni
  2014-07-04 14:27     ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Paulo Zanoni
@ 2014-07-10 14:33     ` Damien Lespiau
  1 sibling, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2014-07-10 14:33 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jul 04, 2014 at 11:27:38AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> And get/put it when needed. The special thing about this commit is
> that it will now return false in ibx_pch_dpll_get_hw_state() in case
> the power domain is not enabled. This will fix some WARNs we have when
> we run pm_rpm on SNB.
> 
> Testcase: igt/pm_rpm
> Bugzilla:https://bugs.freedesktop.org/show_bug.cgi?id=80463
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

(already reviewed this on in the SNB series, but might as well do it
here again):

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  2 ++
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
>  drivers/gpu/drm/i915/intel_pm.c      |  1 +
>  4 files changed, 14 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a89cc7a..e79ddbf 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2134,6 +2134,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>  		return "VGA";
>  	case POWER_DOMAIN_AUDIO:
>  		return "AUDIO";
> +	case POWER_DOMAIN_PLLS:
> +		return "PLLS";
>  	case POWER_DOMAIN_INIT:
>  		return "INIT";
>  	default:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b1f1518..2ec7cb6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -129,6 +129,7 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_PORT_OTHER,
>  	POWER_DOMAIN_VGA,
>  	POWER_DOMAIN_AUDIO,
> +	POWER_DOMAIN_PLLS,
>  	POWER_DOMAIN_INIT,
>  
>  	POWER_DOMAIN_NUM,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3bbc798..1d919ae 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1837,6 +1837,8 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc)
>  	}
>  	WARN_ON(pll->on);
>  
> +	intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
> +
>  	DRM_DEBUG_KMS("enabling %s\n", pll->name);
>  	pll->enable(dev_priv, pll);
>  	pll->on = true;
> @@ -1873,6 +1875,8 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
>  	DRM_DEBUG_KMS("disabling %s\n", pll->name);
>  	pll->disable(dev_priv, pll);
>  	pll->on = false;
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_PLLS);
>  }
>  
>  static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv,
> @@ -11298,6 +11302,9 @@ static bool ibx_pch_dpll_get_hw_state(struct drm_i915_private *dev_priv,
>  {
>  	uint32_t val;
>  
> +	if (!intel_display_power_enabled(dev_priv, POWER_DOMAIN_PLLS))
> +		return false;
> +
>  	val = I915_READ(PCH_DPLL(pll->id));
>  	hw_state->dpll = val;
>  	hw_state->fp0 = I915_READ(PCH_FP0(pll->id));
> @@ -12864,6 +12871,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		DRM_DEBUG_KMS("%s hw state readout: refcount %i, on %i\n",
>  			      pll->name, pll->refcount, pll->on);
> +
> +		if (pll->refcount)
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
>  	}
>  
>  	list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 31ae2b4..cf4c521 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6310,6 +6310,7 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
>  	BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_CRT) |			\
> +	BIT(POWER_DOMAIN_PLLS) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  #define HSW_DISPLAY_POWER_DOMAINS (				\
>  	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 14/19] drm/i915: State readout support for WRPLLs
  2014-07-04 14:27     ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Paulo Zanoni
@ 2014-07-10 14:35       ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2014-07-10 14:35 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: Daniel Vetter, intel-gfx, Paulo Zanoni

On Fri, Jul 04, 2014 at 11:27:39AM -0300, Paulo Zanoni wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Still tacked onto the side, but slowly getting there.
> 
> v2: Don't forget the debugfs file.
> 
> v3 (from Paulo): Don't forget to check the power domains.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

As always, there's nothing replacing testing:

for v3: Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  1 +
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/i915_reg.h      |  1 +
>  drivers/gpu/drm/i915/intel_ddi.c     | 19 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c |  9 +++++++++
>  5 files changed, 31 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e79ddbf..7d72768 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2409,6 +2409,7 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused)
>  		seq_printf(m, " dpll_md: 0x%08x\n", pll->hw_state.dpll_md);
>  		seq_printf(m, " fp0:     0x%08x\n", pll->hw_state.fp0);
>  		seq_printf(m, " fp1:     0x%08x\n", pll->hw_state.fp1);
> +		seq_printf(m, " wrpll:   0x%08x\n", pll->hw_state.wrpll);
>  	}
>  	drm_modeset_unlock_all(dev);
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2ec7cb6..c1fa561 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -197,6 +197,7 @@ struct intel_dpll_hw_state {
>  	uint32_t dpll_md;
>  	uint32_t fp0;
>  	uint32_t fp1;
> +	uint32_t wrpll;
>  };
>  
>  struct intel_shared_dpll {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 3d61a53..654417e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5900,6 +5900,7 @@ enum punit_power_well {
>  /* WRPLL */
>  #define WRPLL_CTL1			0x46040
>  #define WRPLL_CTL2			0x46060
> +#define WRPLL_CTL(pll)			(pll == 0 ? WRPLL_CTL1 : WRPLL_CTL2)
>  #define  WRPLL_PLL_ENABLE		(1<<31)
>  #define  WRPLL_PLL_SSC			(1<<28)
>  #define  WRPLL_PLL_NON_SSC		(2<<28)
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index fae73d3..79cbb5e 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -790,6 +790,8 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
>  			intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL2;
>  			intel_crtc->config.shared_dpll = DPLL_ID_WRPLL2;
>  		}
> +
> +		intel_crtc->config.dpll_hw_state.wrpll = val;
>  	}
>  
>  	return true;
> @@ -1315,6 +1317,21 @@ int intel_ddi_get_cdclk_freq(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static bool hsw_ddi_pll_get_hw_state(struct drm_i915_private *dev_priv,
> +				     struct intel_shared_dpll *pll,
> +				     struct intel_dpll_hw_state *hw_state)
> +{
> +	uint32_t val;
> +
> +	if (!intel_display_power_enabled(dev_priv, POWER_DOMAIN_PLLS))
> +		return false;
> +
> +	val = I915_READ(WRPLL_CTL(pll->id));
> +	hw_state->wrpll = val;
> +
> +	return val & WRPLL_PLL_ENABLE;
> +}
> +
>  static char *hsw_ddi_pll_names[] = {
>  	"WRPLL 1",
>  	"WRPLL 2",
> @@ -1333,6 +1350,8 @@ void intel_ddi_pll_init(struct drm_device *dev)
>  	for (i = 0; i < 2; i++) {
>  		dev_priv->shared_dplls[i].id = i;
>  		dev_priv->shared_dplls[i].name = hsw_ddi_pll_names[i];
> +		dev_priv->shared_dplls[i].get_hw_state =
> +			hsw_ddi_pll_get_hw_state;
>  	}
>  
>  	/* The LCPLL register should be turned on by the BIOS. For now let's
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1d919ae..594a49f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7587,6 +7587,7 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_shared_dpll *pll;
>  	enum port port;
>  	uint32_t tmp;
>  
> @@ -7605,6 +7606,13 @@ static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  		break;
>  	}
>  
> +	if (pipe_config->shared_dpll >= 0) {
> +		pll = &dev_priv->shared_dplls[pipe_config->shared_dpll];
> +
> +		WARN_ON(!pll->get_hw_state(dev_priv, pll,
> +					   &pipe_config->dpll_hw_state));
> +	}
> +
>  	/*
>  	 * Haswell has only FDI/PCH transcoder A. It is which is connected to
>  	 * DDI E. So just check whether this pipe is wired to DDI E and whether
> @@ -10436,6 +10444,7 @@ intel_pipe_config_compare(struct drm_device *dev,
>  	PIPE_CONF_CHECK_X(dpll_hw_state.dpll_md);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp0);
>  	PIPE_CONF_CHECK_X(dpll_hw_state.fp1);
> +	PIPE_CONF_CHECK_X(dpll_hw_state.wrpll);
>  
>  	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5)
>  		PIPE_CONF_CHECK_I(pipe_bpp);
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT
  2014-07-04 14:30 ` [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT Paulo Zanoni
@ 2014-07-10 14:40   ` Damien Lespiau
  0 siblings, 0 replies; 34+ messages in thread
From: Damien Lespiau @ 2014-07-10 14:40 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jul 04, 2014 at 11:30:28AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Since we now have support for shared DPLLS.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d5861bf..5187341 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1094,11 +1094,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
>  	bool cur_state;
>  	struct intel_dpll_hw_state hw_state;
>  
> -	if (HAS_PCH_LPT(dev_priv->dev)) {
> -		DRM_DEBUG_DRIVER("LPT detected: skipping PCH PLL test\n");
> -		return;
> -	}
> -
>  	if (WARN (!pll,
>  		  "asserting DPLL %s with no DPLL\n", state_string(state)))
>  		return;
> -- 
> 2.0.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 00/19] ddi: respin of runtime PM for DPMS
  2014-07-01 21:33 ` [PATCH 00/19] ddi: respin of runtime PM for DPMS Paulo Zanoni
@ 2014-07-10 20:15   ` Daniel Vetter
  2014-07-11 15:51     ` Daniel Vetter
  0 siblings, 1 reply; 34+ messages in thread
From: Daniel Vetter @ 2014-07-10 20:15 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: Intel Graphics Development

On Tue, Jul 01, 2014 at 06:33:50PM -0300, Paulo Zanoni wrote:
> 2014-06-25 16:01 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > This is a respin of the unmerged part of Daniel's runtime PM for DPMS
> > patchset [1]. The original one also included a refactoring of the DDI
> > PCH/CRT encoder modesetting path, I left the corresponding patches out
> > from this series. This is because there hasn't been yet an agreement on
> > those parts, but people would like to see the RPM DPMS support already
> > applied.
> >
> > Some patches needed to be updated/rebased because of the above omission,
> > but these weren't anywhere significant so I just marked the fact
> > with my s-o-b line. I also added two minor change to keep the
> > modeset sequence at its current order and collected all the reviewed-by
> > lines.
> >
> > Tested on HSW DP/VGA, with basic DPMS on/off and igt/pm_rpm.
> 
> For patches 2, 4, 5, 6, 7, 19: Reviewed-by: Paulo Zanoni
> <paulo.r.zanoni@intel.com>
> 
> However, I tested these patches on a HSW Machine with eDP+HDMI, and
> there are WARNs on dmesg for the dpms-non-lpsp subtest. I found at
> least two problems:
> 1 - Function hsw_ddi_pll_get_hw_state() reads registers while the
> device is suspended.
> 2 - When _intel_set_mode() calls intel_crtc_disable(), it calls
> assert_plane() which reads register 0x71180, which triggers an
> "Unclaimed register" error. I didn't investigate this deeply, so I
> don't have a suggestion for a solution.
> 
> I can reproduce these errors 100% of the time.

Ok, I've pulled in the series (hopefully all of it with all the right
fixup and in the right order), except for the last patch. I'll do that one
once your unclaimed register write fixes are in to avoid regressions.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 00/19] ddi: respin of runtime PM for DPMS
  2014-07-10 20:15   ` Daniel Vetter
@ 2014-07-11 15:51     ` Daniel Vetter
  0 siblings, 0 replies; 34+ messages in thread
From: Daniel Vetter @ 2014-07-11 15:51 UTC (permalink / raw
  To: Paulo Zanoni; +Cc: Intel Graphics Development

On Thu, Jul 10, 2014 at 10:15:11PM +0200, Daniel Vetter wrote:
> On Tue, Jul 01, 2014 at 06:33:50PM -0300, Paulo Zanoni wrote:
> > 2014-06-25 16:01 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > > This is a respin of the unmerged part of Daniel's runtime PM for DPMS
> > > patchset [1]. The original one also included a refactoring of the DDI
> > > PCH/CRT encoder modesetting path, I left the corresponding patches out
> > > from this series. This is because there hasn't been yet an agreement on
> > > those parts, but people would like to see the RPM DPMS support already
> > > applied.
> > >
> > > Some patches needed to be updated/rebased because of the above omission,
> > > but these weren't anywhere significant so I just marked the fact
> > > with my s-o-b line. I also added two minor change to keep the
> > > modeset sequence at its current order and collected all the reviewed-by
> > > lines.
> > >
> > > Tested on HSW DP/VGA, with basic DPMS on/off and igt/pm_rpm.
> > 
> > For patches 2, 4, 5, 6, 7, 19: Reviewed-by: Paulo Zanoni
> > <paulo.r.zanoni@intel.com>
> > 
> > However, I tested these patches on a HSW Machine with eDP+HDMI, and
> > there are WARNs on dmesg for the dpms-non-lpsp subtest. I found at
> > least two problems:
> > 1 - Function hsw_ddi_pll_get_hw_state() reads registers while the
> > device is suspended.
> > 2 - When _intel_set_mode() calls intel_crtc_disable(), it calls
> > assert_plane() which reads register 0x71180, which triggers an
> > "Unclaimed register" error. I didn't investigate this deeply, so I
> > don't have a suggestion for a solution.
> > 
> > I can reproduce these errors 100% of the time.
> 
> Ok, I've pulled in the series (hopefully all of it with all the right
> fixup and in the right order), except for the last patch. I'll do that one
> once your unclaimed register write fixes are in to avoid regressions.

Paulo clarified on irc that the remaining patches are just to make the
unclaimed register debug support more useful on bdw. So I've gone ahead
and pulled in patch 19, too.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

end of thread, other threads:[~2014-07-11 15:51 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-25 19:01 [PATCH 00/19] ddi: respin of runtime PM for DPMS Imre Deak
2014-06-25 19:01 ` [PATCH 01/19] drm/i915: Check hw state in assert_can_disable_lcpll Imre Deak
2014-06-30 20:59   ` Paulo Zanoni
2014-06-25 19:01 ` [PATCH 02/19] drm/i915: Remove spll_refcount for hsw Imre Deak
2014-06-25 19:01 ` [PATCH 03/19] drm/i915: Clean up WRPLL/SPLL #defines Imre Deak
2014-06-25 19:01 ` [PATCH 04/19] drm/i915: ddi: move pch setup after encoder->pre_enable Imre Deak
2014-06-25 19:01 ` [PATCH 05/19] drm/i915: ddi: move pch cleanup before encoder->post_disable Imre Deak
2014-06-25 19:01 ` [PATCH 06/19] drm/i915: Move the SPLL enabling into hsw_crt_pre_enable Imre Deak
2014-06-25 19:01 ` [PATCH 07/19] drm/i915: Move SPLL disabling into hsw_crt_post_disable Imre Deak
2014-06-25 19:01 ` [PATCH 08/19] drm/i915: Add a debugfs file for the shared dpll state Imre Deak
2014-06-25 19:01 ` [PATCH 09/19] drm/i915: Move ddi_pll_sel into the pipe config Imre Deak
2014-06-25 19:01 ` [PATCH 10/19] drm/i915: State readout and cross-checking for ddi_pll_sel Imre Deak
2014-06-25 19:01 ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Imre Deak
2014-07-02 17:17   ` Paulo Zanoni
2014-07-04 14:26   ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Paulo Zanoni
2014-07-04 14:26     ` [PATCH 11/19] drm/i915: Precompute static ddi_pll_sel values in encoders Paulo Zanoni
2014-07-04 14:30     ` [PATCH 10.5/19] drm/i915: BDW also has special-purpose DP DDI clocks Damien Lespiau
2014-06-25 19:01 ` [PATCH 12/19] drm/i915: Basic shared dpll support for WRPLLs Imre Deak
2014-06-25 19:01 ` [PATCH 13/19] drm/i915: Document that the pll->mode_set hook is optional Imre Deak
2014-06-25 19:01 ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Imre Deak
2014-07-04 14:27   ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Paulo Zanoni
2014-07-04 14:27     ` [PATCH 14/19] drm/i915: State readout support for WRPLLs Paulo Zanoni
2014-07-10 14:35       ` Damien Lespiau
2014-07-10 14:33     ` [PATCH 13.5/19] drm/i915: add POWER_DOMAIN_PLLS Damien Lespiau
2014-06-25 19:02 ` [PATCH 15/19] drm/i915: ->disable hook for WRPLLs Imre Deak
2014-06-25 19:02 ` [PATCH 16/19] drm/i915: ->enable " Imre Deak
2014-06-25 19:02 ` [PATCH 17/19] drm/i915: Switch to common shared dpll framework " Imre Deak
2014-06-25 19:02 ` [PATCH 18/19] drm/i915: Only touch WRPLL hw state in enable/disable hooks Imre Deak
2014-06-25 19:02 ` [PATCH 19/19] drm/i915: ddi: enable runtime pm during dpms Imre Deak
2014-07-01 21:33 ` [PATCH 00/19] ddi: respin of runtime PM for DPMS Paulo Zanoni
2014-07-10 20:15   ` Daniel Vetter
2014-07-11 15:51     ` Daniel Vetter
2014-07-04 14:30 ` [PATCH 20/19] drm/i915: don't skip shared DPLL assertion on LPT Paulo Zanoni
2014-07-10 14:40   ` Damien Lespiau

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.