[PATCH 0/6] Enable PC8+ states

[PATCH 0/6] Enable PC8+ states

[Paulo Zanoni]

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

Hi

So this series is a second version of the patch I sent on April 16th. Daniel
asked to write some patches to properly initialize the interrupts we're touching
on the PC8+ patches, so the first 5 patches should do this.

The 6th patch is the big one enabling PC8+ and even though I still didn't
address all of Daniel's concerns from his first review, the patch already works:
we get into PC10 and when we get back to PC7 things are still working.

Cheers,
Paulo

Paulo Zanoni (6):
  drm/i915: add ibx_irq_preinstall
  drm/i915: initialize Haswell audio interrupts
  drm/i915: initialize FDI RX interrupts
  drm/i915: initialize the Haswell SRD interrupts
  drm/i915: initialize the PCH GTC interrupts
  drm/i915: implement HSW display sequences for package C8+

 drivers/gpu/drm/i915/i915_dma.c      |   4 +
 drivers/gpu/drm/i915/i915_drv.h      |  28 +++
 drivers/gpu/drm/i915/i915_irq.c      | 111 +++++++--
 drivers/gpu/drm/i915/i915_reg.h      |  27 +++
 drivers/gpu/drm/i915/intel_crt.c     |  26 ++-
 drivers/gpu/drm/i915/intel_display.c | 430 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c      |  11 +-
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 drivers/gpu/drm/i915/intel_hdmi.c    |   3 +
 9 files changed, 609 insertions(+), 34 deletions(-)

-- 
1.8.1.2

[PATCH 1/6] drm/i915: add ibx_irq_preinstall

[Paulo Zanoni]

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

So we can remove some duplicate code. All the PCHs are very similar
and right now the code is the same. I plan to add more code, so we
would have more duplicated code.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 63996aa..c482e8a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2472,6 +2472,25 @@ void i915_hangcheck_elapsed(unsigned long data)
 					   DRM_I915_HANGCHECK_JIFFIES));
 }
 
+static void ibx_irq_preinstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (HAS_PCH_NOP(dev))
+		return;
+
+	/* south display irq */
+	I915_WRITE(SDEIMR, 0xffffffff);
+	/*
+	 * SDEIER is also touched by the interrupt handler to work around missed
+	 * PCH interrupts. Hence we can't update it after the interrupt handler
+	 * is enabled - instead we unconditionally enable all PCH interrupt
+	 * sources here, but then only unmask them as needed with SDEIMR.
+	 */
+	I915_WRITE(SDEIER, 0xffffffff);
+	POSTING_READ(SDEIER);
+}
+
 /* drm_dma.h hooks
 */
 static void ironlake_irq_preinstall(struct drm_device *dev)
@@ -2493,16 +2512,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GTIER, 0x0);
 	POSTING_READ(GTIER);
 
-	/* south display irq */
-	I915_WRITE(SDEIMR, 0xffffffff);
-	/*
-	 * SDEIER is also touched by the interrupt handler to work around missed
-	 * PCH interrupts. Hence we can't update it after the interrupt handler
-	 * is enabled - instead we unconditionally enable all PCH interrupt
-	 * sources here, but then only unmask them as needed with SDEIMR.
-	 */
-	I915_WRITE(SDEIER, 0xffffffff);
-	POSTING_READ(SDEIER);
+	ibx_irq_preinstall(dev);
 }
 
 static void ivybridge_irq_preinstall(struct drm_device *dev)
@@ -2529,19 +2539,7 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN6_PMIER, 0x0);
 	POSTING_READ(GEN6_PMIER);
 
-	if (HAS_PCH_NOP(dev))
-		return;
-
-	/* south display irq */
-	I915_WRITE(SDEIMR, 0xffffffff);
-	/*
-	 * SDEIER is also touched by the interrupt handler to work around missed
-	 * PCH interrupts. Hence we can't update it after the interrupt handler
-	 * is enabled - instead we unconditionally enable all PCH interrupt
-	 * sources here, but then only unmask them as needed with SDEIMR.
-	 */
-	I915_WRITE(SDEIER, 0xffffffff);
-	POSTING_READ(SDEIER);
+	ibx_irq_preinstall(dev);
 }
 
 static void valleyview_irq_preinstall(struct drm_device *dev)
-- 
1.8.1.2

[PATCH 2/6] drm/i915: initialize Haswell audio interrupts

[Paulo Zanoni]

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

These interrupts are specific to Haswell and we don't use them
anywhere inside our code. If we want to allow package C8+ states we
need to make sure we don't have any pending interrupts, so let's
properly initialize the interrupt registers here, so when we allow C8+
states we'll only need to check if these interrupts are still
disabled.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  4 ++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c482e8a..6706d89 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2539,6 +2539,13 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN6_PMIER, 0x0);
 	POSTING_READ(GEN6_PMIER);
 
+	/* Audio */
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(AUDIMR, 0xffffffff);
+		I915_WRITE(AUDIER, 0x0);
+		POSTING_READ(AUDIER);
+	}
+
 	ibx_irq_preinstall(dev);
 }
 
@@ -2730,6 +2737,11 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		   (I915_READ(GEN6_PMIER) & GEN6_PM_RPS_EVENTS) | pm_irqs);
 	POSTING_READ(GEN6_PMIER);
 
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(AUDIIR, I915_READ(AUDIIR));
+		POSTING_READ(AUDIIR);
+	}
+
 	ibx_irq_postinstall(dev);
 
 	return 0;
@@ -2837,6 +2849,12 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	I915_WRITE(GTIER, 0x0);
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
 
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(AUDIMR, 0xffffffff);
+		I915_WRITE(AUDIER, 0x0);
+		I915_WRITE(AUDIIR, I915_READ(AUDIIR));
+	}
+
 	if (HAS_PCH_NOP(dev))
 		return;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 47a9de0..d7f272a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3733,6 +3733,10 @@
 #define GTIIR   0x44018
 #define GTIER   0x4401c
 
+#define AUDIMR	0x44084
+#define AUDIIR	0x44088
+#define AUDIER	0x4408c
+
 #define ILK_DISPLAY_CHICKEN2	0x42004
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
1.8.1.2

[PATCH 3/6] drm/i915: initialize FDI RX interrupts

[Paulo Zanoni]

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

Initialize (disable) the FDI RX interrupts when initializing the
driver. The code that uses those interrupts already takes care of
properly unmasking the needed ones before using them.

This patch will make sure we never get undesired FDI RX interrupts,
and then when we enable package C8+ states on Haswell all we'll need
to do is to check that we still don't have any pending interrupt,
since Haswell doesn't use the FDI RX interrupts.

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

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6706d89..33f404e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2475,6 +2475,7 @@ void i915_hangcheck_elapsed(unsigned long data)
 static void ibx_irq_preinstall(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe;
 
 	if (HAS_PCH_NOP(dev))
 		return;
@@ -2489,6 +2490,14 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 	 */
 	I915_WRITE(SDEIER, 0xffffffff);
 	POSTING_READ(SDEIER);
+
+	for_each_pipe(pipe) {
+		I915_WRITE(FDI_RX_IMR(pipe), 0xffffffff);
+		POSTING_READ(FDI_RX_IMR(pipe));
+		/* Only 1 PCH transcoder. */
+		if (HAS_PCH_LPT(dev))
+			break;
+	}
 }
 
 /* drm_dma.h hooks
@@ -2621,6 +2630,7 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	u32 mask;
+	enum pipe pipe;
 
 	if (HAS_PCH_NOP(dev))
 		return;
@@ -2636,6 +2646,14 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 
 	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 	I915_WRITE(SDEIMR, ~mask);
+
+	for_each_pipe(pipe) {
+		I915_WRITE(FDI_RX_IMR(pipe), 0xffffffff);
+		I915_WRITE(FDI_RX_IIR(pipe), I915_READ(FDI_RX_IIR(pipe)));
+		/* Only 1 PCH transcoder. */
+		if (HAS_PCH_LPT(dev))
+			break;
+	}
 }
 
 static int ironlake_irq_postinstall(struct drm_device *dev)
@@ -2831,6 +2849,7 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
 static void ironlake_irq_uninstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+	enum pipe pipe;
 
 	if (!dev_priv)
 		return;
@@ -2863,6 +2882,14 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 	I915_WRITE(SDEIIR, I915_READ(SDEIIR));
 	if (HAS_PCH_CPT(dev) || HAS_PCH_LPT(dev))
 		I915_WRITE(SERR_INT, I915_READ(SERR_INT));
+
+	for_each_pipe(pipe) {
+		I915_WRITE(FDI_RX_IMR(pipe), 0xffffffff);
+		I915_WRITE(FDI_RX_IIR(pipe), I915_READ(FDI_RX_IIR(pipe)));
+		/* Only 1 PCH transcoder. */
+		if (HAS_PCH_LPT(dev))
+			break;
+	}
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
-- 
1.8.1.2

[PATCH 4/6] drm/i915: initialize the Haswell SRD interrupts

[Paulo Zanoni]

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

Same reasons as the other ones: we're not using them so we don't want
them, and this is a preparation for when we enable package C8+ states.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 11 ++++++++++-
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 33f404e..c21055e 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2548,11 +2548,14 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(GEN6_PMIER, 0x0);
 	POSTING_READ(GEN6_PMIER);
 
-	/* Audio */
+	/* Audio and PSR */
 	if (IS_HASWELL(dev)) {
 		I915_WRITE(AUDIMR, 0xffffffff);
 		I915_WRITE(AUDIER, 0x0);
 		POSTING_READ(AUDIER);
+
+		I915_WRITE(SRDIMR, 0xffffffff);
+		POSTING_READ(SRDIMR);
 	}
 
 	ibx_irq_preinstall(dev);
@@ -2758,6 +2761,9 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 	if (IS_HASWELL(dev)) {
 		I915_WRITE(AUDIIR, I915_READ(AUDIIR));
 		POSTING_READ(AUDIIR);
+
+		I915_WRITE(SRDIIR, I915_READ(SRDIIR));
+		POSTING_READ(SRDIIR);
 	}
 
 	ibx_irq_postinstall(dev);
@@ -2872,6 +2878,9 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 		I915_WRITE(AUDIMR, 0xffffffff);
 		I915_WRITE(AUDIER, 0x0);
 		I915_WRITE(AUDIIR, I915_READ(AUDIIR));
+
+		I915_WRITE(SRDIMR, 0xffffffff);
+		I915_WRITE(SRDIIR, I915_READ(SRDIIR));
 	}
 
 	if (HAS_PCH_NOP(dev))
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d7f272a..f996e9f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3737,6 +3737,9 @@
 #define AUDIIR	0x44088
 #define AUDIER	0x4408c
 
+#define SRDIMR	0x64834
+#define SRDIIR	0x64838
+
 #define ILK_DISPLAY_CHICKEN2	0x42004
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
1.8.1.2

[PATCH 5/6] drm/i915: initialize the PCH GTC interrupts

[Paulo Zanoni]

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

These regsiters only exist on LPT and we still don't use them.
Initialize them for the same reason as we initialize the other
interrupts we don't use (SRD, FDI_RX, AUD).

Notice that we also have CPU GTC registers, but these registers are
disabled when the power well is disabled, so they must be handled
differently. Also, they don't affect the code for package C8+ since
we need the power well disabled to enter PC8+.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c21055e..dc2658c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2498,6 +2498,11 @@ static void ibx_irq_preinstall(struct drm_device *dev)
 		if (HAS_PCH_LPT(dev))
 			break;
 	}
+
+	if (HAS_PCH_LPT(dev)) {
+		I915_WRITE(PCH_GTCIMR, 0xffffffff);
+		POSTING_READ(PCH_GTCIMR);
+	}
 }
 
 /* drm_dma.h hooks
@@ -2657,6 +2662,11 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 		if (HAS_PCH_LPT(dev))
 			break;
 	}
+
+	if (HAS_PCH_LPT(dev)) {
+		I915_WRITE(PCH_GTCIMR, 0xffffffff);
+		I915_WRITE(PCH_GTCIIR, I915_READ(PCH_GTCIIR));
+	}
 }
 
 static int ironlake_irq_postinstall(struct drm_device *dev)
@@ -2899,6 +2909,11 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
 		if (HAS_PCH_LPT(dev))
 			break;
 	}
+
+	if (HAS_PCH_LPT(dev)) {
+		I915_WRITE(PCH_GTCIMR, 0xffffffff);
+		I915_WRITE(PCH_GTCIIR, I915_READ(PCH_GTCIIR));
+	}
 }
 
 static void i8xx_irq_preinstall(struct drm_device * dev)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f996e9f..6a977ce 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3740,6 +3740,9 @@
 #define SRDIMR	0x64834
 #define SRDIIR	0x64838
 
+#define PCH_GTCIMR	0xe7054
+#define PCH_GTCIIR	0xe7058
+
 #define ILK_DISPLAY_CHICKEN2	0x42004
 /* Required on all Ironlake and Sandybridge according to the B-Spec. */
 #define  ILK_ELPIN_409_SELECT	(1 << 25)
-- 
1.8.1.2

[PATCH 6/6] drm/i915: implement HSW display sequences for package C8+

[Paulo Zanoni]

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

This patch implements "Display Sequences for Package C8", from the
"Display Mode Set Sequence" section from the Haswell documentation.

Notice that even if we allow PC8+, there's no guarantee that we will
actually enter PC8+, since the other devices also need to allow it.
Also notice that we need i915.disable_power_well=1 in order to test
this feature: we don't allow PC8+ if the power well is still enabled.

v2: - Rebase
    - Implement many review comments form Daniel Vetter
    - Call intel_prepare_ddi and i915_gem_init_swizzling when
      returning from C8+ so we can actually see the screen contents

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |   4 +
 drivers/gpu/drm/i915/i915_drv.h      |  28 +++
 drivers/gpu/drm/i915/i915_reg.h      |  17 ++
 drivers/gpu/drm/i915/intel_crt.c     |  26 ++-
 drivers/gpu/drm/i915/intel_display.c | 430 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_dp.c      |  11 +-
 drivers/gpu/drm/i915/intel_drv.h     |   3 +
 drivers/gpu/drm/i915/intel_hdmi.c    |   3 +
 8 files changed, 510 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 22534c3..fe2bd7f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1639,10 +1639,14 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 	spin_lock_init(&dev_priv->rps.lock);
 	spin_lock_init(&dev_priv->backlight.lock);
 	mutex_init(&dev_priv->dpio_lock);
+	mutex_init(&dev_priv->c8_lock);
 
 	mutex_init(&dev_priv->rps.hw_lock);
 	mutex_init(&dev_priv->modeset_restore_lock);
 
+	dev_priv->allowing_package_c8 = false;
+	dev_priv->c8_forbid_refcnt = 1;
+
 	dev_priv->num_plane = 1;
 	if (IS_VALLEYVIEW(dev))
 		dev_priv->num_plane = 2;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 215aa63..46b1f70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -538,6 +538,29 @@ struct intel_gmbus {
 	struct drm_i915_private *dev_priv;
 };
 
+struct i915_c8_saved_registers {
+	u32 de_imr;
+	u32 de_ier;
+	u32 aud_imr;
+	u32 aud_ier;
+	u32 gt_imr;
+	u32 gt_ier;
+	u32 pm_imr;
+	u32 pm_ier;
+	u32 srd_imr;
+	u32 hotplug_ctl;
+	u32 err_int;
+
+	u32 sde_imr;
+	u32 sde_ier;
+	u32 fdirx_imr;
+	u32 gtcpch_imr;
+	u32 shotplug_ctl;
+	u32 serr_int;
+
+	u32 lcpll_freq;
+};
+
 struct i915_suspend_saved_registers {
 	u8 saveLBB;
 	u32 saveDSPACNTR;
@@ -1127,6 +1150,11 @@ typedef struct drm_i915_private {
 
 	struct i915_suspend_saved_registers regfile;
 
+	struct i915_c8_saved_registers c8_regfile;
+	bool allowing_package_c8;
+	int c8_forbid_refcnt;
+	struct mutex c8_lock;
+
 	/* Old dri1 support infrastructure, beware the dragons ya fools entering
 	 * here! */
 	struct i915_dri1_state dri1;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6a977ce..ba73e00 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2194,6 +2194,8 @@
 #define BLC_PWM_CPU_CTL2	0x48250
 #define BLC_PWM_CPU_CTL		0x48254
 
+#define HSW_BLC_PWM2_CTL	0x48350
+
 /* PCH CTL1 is totally different, all but the below bits are reserved. CTL2 is
  * like the normal CTL from gen4 and earlier. Hooray for confusing naming. */
 #define BLC_PWM_PCH_CTL1	0xc8250
@@ -2202,6 +2204,12 @@
 #define   BLM_PCH_POLARITY			(1 << 29)
 #define BLC_PWM_PCH_CTL2	0xc8254
 
+#define UTIL_PIN_CTL		0x48400
+#define   UTIL_PIN_ENABLE	(1 << 31)
+
+#define PCH_GTC_CTL		0xe7000
+#define   PCH_GTC_ENABLE	(1 << 31)
+
 /* TV port control */
 #define TV_CTL			0x68000
 /** Enables the TV encoder */
@@ -4877,6 +4885,8 @@
 #define   SBI_SSCAUXDIV_FINALDIV2SEL(x)		((x)<<4)
 #define  SBI_DBUFF0				0x2a00
 #define   SBI_DBUFF0_ENABLE			(1<<0)
+#define  SBI_GEN0				0x1f00
+#define   SBI_GEN0_ENABLE			(1<<0)
 
 /* LPT PIXCLK_GATE */
 #define PIXCLK_GATE			0xC6020
@@ -4942,7 +4952,14 @@
 #define  LCPLL_CLK_FREQ_450		(0<<26)
 #define  LCPLL_CD_CLOCK_DISABLE		(1<<25)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
+#define  LCPLL_POWER_DOWN_ALLOW		(1<<22)
 #define  LCPLL_CD_SOURCE_FCLK		(1<<21)
+#define  LCPLL_CD_SOURCE_FCLK_DONE	(1<<19)
+
+#define D_COMP				(MCHBAR_MIRROR_BASE_SNB + 0x5F0C)
+#define  D_COMP_RCOMP_IN_PROGRESS	(1<<9)
+#define  D_COMP_COMP_FORCE		(1<<8)
+#define  D_COMP_COMP_DISABLE		(1<<0)
 
 /* Pipe WM_LINETIME - watermark line time */
 #define PIPE_WM_LINETIME_A		0x45270
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 3acec8c..6808a31 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -609,10 +609,13 @@ static enum drm_connector_status
 intel_crt_detect(struct drm_connector *connector, bool force)
 {
 	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crt *crt = intel_attached_crt(connector);
 	enum drm_connector_status status;
 	struct intel_load_detect_pipe tmp;
 
+	intel_aux_display_shutdown_forbid(dev_priv);
+
 	if (I915_HAS_HOTPLUG(dev)) {
 		/* We can not rely on the HPD pin always being correctly wired
 		 * up, for example many KVM do not pass it through, and so
@@ -620,23 +623,30 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 		 */
 		if (intel_crt_detect_hotplug(connector)) {
 			DRM_DEBUG_KMS("CRT detected via hotplug\n");
-			return connector_status_connected;
+			status = connector_status_connected;
+			goto out;
 		} else
 			DRM_DEBUG_KMS("CRT not detected via hotplug\n");
 	}
 
-	if (intel_crt_detect_ddc(connector))
-		return connector_status_connected;
+	if (intel_crt_detect_ddc(connector)) {
+		status = connector_status_connected;
+		goto out;
+	}
 
 	/* Load detection is broken on HPD capable machines. Whoever wants a
 	 * broken monitor (without edid) to work behind a broken kvm (that fails
 	 * to have the right resistors for HP detection) needs to fix this up.
 	 * For now just bail out. */
-	if (I915_HAS_HOTPLUG(dev))
-		return connector_status_disconnected;
+	if (I915_HAS_HOTPLUG(dev)) {
+		status = connector_status_disconnected;
+		goto out;
+	}
 
-	if (!force)
-		return connector->status;
+	if (!force) {
+		status = connector->status;
+		goto out;
+	}
 
 	/* for pre-945g platforms use load detect */
 	if (intel_get_load_detect_pipe(connector, NULL, &tmp)) {
@@ -648,6 +658,8 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	} else
 		status = connector_status_unknown;
 
+out:
+	intel_aux_display_shutdown_allow(dev_priv);
 	return status;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 827d7ca..4c8fcec 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5284,6 +5284,66 @@ static void lpt_init_pch_refclk(struct drm_device *dev)
 	mutex_unlock(&dev_priv->dpio_lock);
 }
 
+/* Sequence to enable CLKOUT_DP */
+static void lpt_enable_clkout_dp(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	mutex_lock(&dev_priv->dpio_lock);
+
+	val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
+	val &= ~SBI_SSCCTL_DISABLE;
+	val |= SBI_SSCCTL_PATHALT;
+	intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
+
+	udelay(24);
+
+	val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
+	val &= ~SBI_SSCCTL_PATHALT;
+	intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
+
+	if (IS_ULT(dev_priv->dev)) {
+		val = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
+		val |= SBI_GEN0_ENABLE;
+		intel_sbi_write(dev_priv, SBI_GEN0, val, SBI_ICLK);
+	} else {
+		val = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
+		val |= SBI_DBUFF0_ENABLE;
+		intel_sbi_write(dev_priv, SBI_DBUFF0, val, SBI_ICLK);
+	}
+
+	mutex_unlock(&dev_priv->dpio_lock);
+}
+
+/* Sequence to disable CLKOUT_DP */
+static void lpt_disable_clkout_dp(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	mutex_lock(&dev_priv->dpio_lock);
+
+	if (IS_ULT(dev_priv->dev)) {
+		val = intel_sbi_read(dev_priv, SBI_GEN0, SBI_ICLK);
+		val &= ~SBI_GEN0_ENABLE;
+		intel_sbi_write(dev_priv, SBI_GEN0, val, SBI_ICLK);
+	} else {
+		val = intel_sbi_read(dev_priv, SBI_DBUFF0, SBI_ICLK);
+		val &= ~SBI_DBUFF0_ENABLE;
+		intel_sbi_write(dev_priv, SBI_DBUFF0, val, SBI_ICLK);
+	}
+
+	val = intel_sbi_read(dev_priv, SBI_SSCCTL, SBI_ICLK);
+	if (!(val & SBI_SSCCTL_PATHALT)) {
+		val |= SBI_SSCCTL_PATHALT;
+		intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
+		udelay(32);
+	}
+	val |= SBI_SSCCTL_DISABLE;
+	intel_sbi_write(dev_priv, SBI_SSCCTL, val, SBI_ICLK);
+
+	mutex_unlock(&dev_priv->dpio_lock);
+}
+
 /*
  * Initialize reference clocks when the driver loads
  */
@@ -5844,9 +5904,357 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	return true;
 }
 
+static void hsw_disable_lcpll(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	val = I915_READ(LCPLL_CTL);
+
+	dev_priv->c8_regfile.lcpll_freq = val & LCPLL_CLK_FREQ_MASK;
+
+	val |= LCPLL_CD_SOURCE_FCLK;
+	I915_WRITE(LCPLL_CTL, val);
+	POSTING_READ(LCPLL_CTL);
+
+	udelay(1);
+
+	val = I915_READ(LCPLL_CTL);
+	if (!(val & LCPLL_CD_SOURCE_FCLK_DONE))
+		DRM_ERROR("Switching to FCLK failed\n");
+
+	val |= LCPLL_PLL_DISABLE;
+	I915_WRITE(LCPLL_CTL, val);
+	POSTING_READ(LCPLL_CTL);
+
+	if (wait_for((I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK) == 0, 1))
+		DRM_ERROR("LCPLL still locked\n");
+
+	val = I915_READ(D_COMP);
+	val |= D_COMP_COMP_DISABLE;
+	I915_WRITE(D_COMP, val);
+	POSTING_READ(D_COMP);
+
+	udelay(2);
+
+	val = I915_READ(D_COMP);
+	if (val & D_COMP_RCOMP_IN_PROGRESS)
+		DRM_ERROR("D_COMP RCOMP still in progress\n");
+
+	val = I915_READ(LCPLL_CTL);
+	val |= LCPLL_POWER_DOWN_ALLOW;
+	I915_WRITE(LCPLL_CTL, val);
+	POSTING_READ(LCPLL_CTL);
+}
+
+static void hsw_restore_lcpll(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	val = I915_READ(LCPLL_CTL);
+
+	if ((val & LCPLL_CLK_FREQ_MASK) != dev_priv->c8_regfile.lcpll_freq)
+		DRM_ERROR("LCPLL frequency changed\n");
+
+	if (val & LCPLL_POWER_DOWN_ALLOW) {
+		val &= ~LCPLL_POWER_DOWN_ALLOW;
+		I915_WRITE(LCPLL_CTL, val);
+	}
+
+	if (val & LCPLL_CD_SOURCE_FCLK) {
+		val = I915_READ(D_COMP);
+		val |= D_COMP_COMP_FORCE;
+		val &= ~D_COMP_COMP_DISABLE;
+		I915_WRITE(D_COMP, val);
+
+		val = I915_READ(LCPLL_CTL);
+		val &= ~LCPLL_PLL_DISABLE;
+		I915_WRITE(LCPLL_CTL, val);
+		POSTING_READ(LCPLL_CTL);
+
+		if (wait_for(I915_READ(LCPLL_CTL) & LCPLL_PLL_LOCK, 5))
+			DRM_ERROR("LCPLL not locked yet\n");
+
+		val = I915_READ(LCPLL_CTL);
+		val &= ~LCPLL_CD_SOURCE_FCLK;
+		I915_WRITE(LCPLL_CTL, val);
+		POSTING_READ(LCPLL_CTL);
+
+		udelay(1);
+
+		val = I915_READ(LCPLL_CTL);
+		if (val & LCPLL_CD_SOURCE_FCLK_DONE)
+			DRM_ERROR("Switching back to LCPLL failed\n");
+	}
+}
+
+static void hsw_disable_interrupts(struct drm_i915_private *dev_priv)
+{
+	struct i915_c8_saved_registers *c8_regfile = &dev_priv->c8_regfile;
+	unsigned long irqflags;
+	uint32_t val, deier, sdeier, hotplug;
+
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+	/* TODO: The spec says we need to clear all pending graphics interrupts,
+	 * but what's the best way to guarantee this? For now let's just print
+	 * some error messages and assume everything will work. We may have to
+	 * manually zero all the IIR registers if we discover they're a real
+	 * requirement to reach PC8+. */
+	val = I915_READ(DEIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: DEIIR 0x%08x\n", val);
+
+	val = I915_READ(AUDIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: AUDIIR 0x%08x\n", val);
+
+	val = I915_READ(GTIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: GTIIR 0x%08x\n", val);
+
+	val = I915_READ(GEN6_PMIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: PMIIR 0x%08x\n", val);
+
+	val = I915_READ(SRDIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: SRDIIR 0x%08x\n", val);
+
+	val = I915_READ(SDEIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: SDEIIR 0x%08x\n", val);
+
+	val = I915_READ(_FDI_RXA_IIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: FDIRXAIIR 0x%08x\n", val);
+
+	val = I915_READ(PCH_GTCIIR);
+	if (val)
+		DRM_ERROR("Pending interrupt: GTCPCHIIR 0x%08x\n", val);
+
+	c8_regfile->de_imr = I915_READ(DEIMR);
+	c8_regfile->de_ier = I915_READ(DEIER);
+	c8_regfile->aud_imr = I915_READ(AUDIMR);
+	c8_regfile->aud_ier = I915_READ(AUDIER);
+	c8_regfile->gt_imr = I915_READ(GTIMR);
+	c8_regfile->gt_ier = I915_READ(GTIER);
+	c8_regfile->pm_imr = I915_READ(GEN6_PMIMR);
+	c8_regfile->pm_ier = I915_READ(GEN6_PMIER);
+	c8_regfile->srd_imr = I915_READ(SRDIMR);
+	c8_regfile->hotplug_ctl = I915_READ(DIGITAL_PORT_HOTPLUG_CNTRL);
+	c8_regfile->err_int = I915_READ(GEN7_ERR_INT);
+
+	c8_regfile->sde_imr = I915_READ(SDEIMR);
+	c8_regfile->sde_ier = I915_READ(SDEIER);
+	c8_regfile->fdirx_imr = I915_READ(_FDI_RXA_IMR);
+	c8_regfile->gtcpch_imr = I915_READ(PCH_GTCIMR);
+	c8_regfile->shotplug_ctl = I915_READ(PCH_PORT_HOTPLUG);
+	c8_regfile->serr_int = I915_READ(SERR_INT);
+
+	deier = DE_MASTER_IRQ_CONTROL | DE_PCH_EVENT_IVB;
+	I915_WRITE(DEIMR, ~deier);
+	I915_WRITE(DEIER, deier);
+
+	I915_WRITE(AUDIMR, 0xFFFFFFFF);
+	I915_WRITE(AUDIER, 0);
+
+	I915_WRITE(GTIMR, 0xFFFFFFFF);
+	I915_WRITE(GTIER, 0);
+
+	I915_WRITE(GEN6_PMIMR, 0xFFFFFFFF);
+	I915_WRITE(GEN6_PMIER, 0);
+
+	I915_WRITE(SRDIMR, 0xFFFFFFFF);
+
+	I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, 0);
+
+	I915_WRITE(GEN7_ERR_INT, 0xFFFFFFFF);
+
+	sdeier = (SDE_PORTD_HOTPLUG_CPT | SDE_PORTC_HOTPLUG_CPT |
+		  SDE_PORTB_HOTPLUG_CPT | SDE_CRT_HOTPLUG_CPT) &
+		  c8_regfile->sde_ier;
+	I915_WRITE(SDEIMR, ~sdeier);
+	I915_WRITE(SDEIER, sdeier);
+
+	I915_WRITE(_FDI_RXA_IMR, 0xFFFFFFFF);
+	I915_WRITE(PCH_GTCIMR, 0xFFFFFFFF);
+
+	hotplug = (PORTD_HOTPLUG_ENABLE | PORTC_HOTPLUG_ENABLE |
+		   PORTB_HOTPLUG_ENABLE) & c8_regfile->hotplug_ctl;
+	I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
+	I915_WRITE(SERR_INT, 0xFFFFFFFF);
+
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
+static void hsw_restore_interrupts(struct drm_i915_private *dev_priv)
+{
+	struct i915_c8_saved_registers *c8_regfile = &dev_priv->c8_regfile;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+	I915_WRITE(DEIMR, c8_regfile->de_imr);
+	I915_WRITE(DEIER, c8_regfile->de_ier);
+	I915_WRITE(AUDIMR, c8_regfile->aud_imr);
+	I915_WRITE(AUDIER, c8_regfile->aud_ier);
+	I915_WRITE(GTIMR, c8_regfile->gt_imr);
+	I915_WRITE(GTIER, c8_regfile->gt_ier);
+	I915_WRITE(GEN6_PMIMR, c8_regfile->pm_imr);
+	I915_WRITE(GEN6_PMIER, c8_regfile->pm_ier);
+	I915_WRITE(SRDIMR, c8_regfile->srd_imr);
+	I915_WRITE(DIGITAL_PORT_HOTPLUG_CNTRL, c8_regfile->hotplug_ctl);
+	I915_WRITE(GEN7_ERR_INT, c8_regfile->err_int);
+
+	I915_WRITE(SDEIMR, c8_regfile->sde_imr);
+	I915_WRITE(SDEIER, c8_regfile->sde_ier);
+	I915_WRITE(_FDI_RXA_IMR, c8_regfile->fdirx_imr);
+	I915_WRITE(PCH_GTCIMR, c8_regfile->gtcpch_imr);
+	I915_WRITE(PCH_PORT_HOTPLUG, c8_regfile->shotplug_ctl);
+	I915_WRITE(SERR_INT, c8_regfile->serr_int);
+
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
+static void hsw_allow_package_c8(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->c8_lock));
+	WARN(dev_priv->c8_forbid_refcnt < 1,
+	     "c8_forbid_refcnt: %d\n", dev_priv->c8_forbid_refcnt);
+
+	dev_priv->c8_forbid_refcnt--;
+	if (dev_priv->c8_forbid_refcnt != 0)
+		return;
+
+	DRM_DEBUG_KMS("Allowing package C8+\n");
+
+	if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+		val = I915_READ(SOUTH_DSPCLK_GATE_D);
+		val &= ~PCH_LP_PARTITION_LEVEL_DISABLE;
+		I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
+	}
+
+	lpt_disable_clkout_dp(dev_priv);
+	hsw_disable_interrupts(dev_priv);
+	hsw_disable_lcpll(dev_priv);
+}
+
+static void hsw_disallow_package_c8(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	WARN_ON(!mutex_is_locked(&dev_priv->c8_lock));
+	WARN(dev_priv->c8_forbid_refcnt < 0,
+	     "c8_forbid_refcnt: %d\n", dev_priv->c8_forbid_refcnt);
+
+	dev_priv->c8_forbid_refcnt++;
+	if (dev_priv->c8_forbid_refcnt != 1)
+		return;
+
+	DRM_DEBUG_KMS("Disallowing package C8+\n");
+
+	hsw_restore_lcpll(dev_priv);
+	hsw_restore_interrupts(dev_priv);
+	lpt_enable_clkout_dp(dev_priv);
+
+	if (dev_priv->pch_id == INTEL_PCH_LPT_LP_DEVICE_ID_TYPE) {
+		val = I915_READ(SOUTH_DSPCLK_GATE_D);
+		val |= PCH_LP_PARTITION_LEVEL_DISABLE;
+		I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
+	}
+
+	intel_prepare_ddi(dev_priv->dev);
+	i915_gem_init_swizzling(dev_priv->dev);
+}
+
+static bool hsw_can_allow_package_c8(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;
+	uint32_t val;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
+		if (crtc->base.enabled)
+			return false;
+
+	/* This case is still possible since we have the i915.disable_power_well
+	 * parameter and also the KVMr or something else might be requesting the
+	 * power well. */
+	val = I915_READ(HSW_PWR_WELL_DRIVER);
+	if (val != 0) {
+		DRM_DEBUG_KMS("Not allowing PC8: power well on\n");
+		return false;
+	}
+
+	if (WARN(plls->spll_refcount, "Not allowing PC8: SPLL enabled\n"))
+		return false;
+
+	if (WARN(plls->wrpll1_refcount, "Not allowing PC8: WRPLL1 enabled\n"))
+		return false;
+
+	if (WARN(plls->wrpll2_refcount, "Not allowing PC8: WRPLL2 enabled\n"))
+		return false;
+
+	val = I915_READ(PCH_PP_STATUS);
+	if (WARN(val & PP_ON, "Not allowing PC8: panel power on\n"))
+		return false;
+
+	val = I915_READ(BLC_PWM_CPU_CTL2);
+	if (WARN(val & BLM_PWM_ENABLE, "Not allowing PC8: CPU PWM1 enabled\n"))
+		return false;
+
+	val = I915_READ(HSW_BLC_PWM2_CTL);
+	if (WARN(val & BLM_PWM_ENABLE, "Not allowing PC8: CPU PWM2 enabled\n"))
+		return false;
+
+	val = I915_READ(BLC_PWM_PCH_CTL1);
+	if (WARN(val & BLM_PCH_PWM_ENABLE,
+		 "Not allowing PC8: PCH PWM1 enabled\n"))
+		return false;
+
+	val = I915_READ(UTIL_PIN_CTL);
+	if (WARN(val & UTIL_PIN_ENABLE,
+		 "Not allowing PC8: utility pin enabled\n"))
+		return false;
+
+	val = I915_READ(PCH_GTC_CTL);
+	if (WARN(val & PCH_GTC_ENABLE, "Not allowing PC8: PCH GTC enabled\n"))
+		return false;
+
+	return true;
+}
+
+/* Since we're called from modeset_global_resources there's no way to
+ * symmetrically increase and decrease the refcnt, so we use
+ * dev_priv->allowing_package_c8 to track whether we already have the refcnt or
+ * not. */
+static void hsw_set_package_c8(struct drm_i915_private *dev_priv)
+{
+	bool allow = hsw_can_allow_package_c8(dev_priv);
+
+	mutex_lock(&dev_priv->c8_lock);
+
+	if (allow == dev_priv->allowing_package_c8)
+		goto done;
+
+	dev_priv->allowing_package_c8 = allow;
+
+	if (allow)
+		hsw_allow_package_c8(dev_priv);
+	else
+		hsw_disallow_package_c8(dev_priv);
+
+done:
+	mutex_unlock(&dev_priv->c8_lock);
+}
+
+
 static void haswell_modeset_global_resources(struct drm_device *dev)
 {
-	bool enable = false;
+	bool enable_power_well = false;
 	struct intel_crtc *crtc;
 
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) {
@@ -5855,10 +6263,26 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
 
 		if (crtc->pipe != PIPE_A || crtc->config.pch_pfit.size ||
 		    crtc->config.cpu_transcoder != TRANSCODER_EDP)
-			enable = true;
+			enable_power_well = true;
 	}
 
-	intel_set_power_well(dev, enable);
+	intel_set_power_well(dev, enable_power_well);
+
+	hsw_set_package_c8(dev->dev_private);
+}
+
+void intel_aux_display_shutdown_forbid(struct drm_i915_private *dev_priv)
+{
+	mutex_lock(&dev_priv->c8_lock);
+	hsw_disallow_package_c8(dev_priv);
+	mutex_unlock(&dev_priv->c8_lock);
+}
+
+void intel_aux_display_shutdown_allow(struct drm_i915_private *dev_priv)
+{
+	mutex_lock(&dev_priv->c8_lock);
+	hsw_allow_package_c8(dev_priv);
+	mutex_unlock(&dev_priv->c8_lock);
 }
 
 static int haswell_crtc_mode_set(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 759a1c5..0de82bb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2519,9 +2519,12 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = connector->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum drm_connector_status status;
 	struct edid *edid = NULL;
 
+	intel_aux_display_shutdown_forbid(dev_priv);
+
 	intel_dp->has_audio = false;
 
 	if (HAS_PCH_SPLIT(dev))
@@ -2530,7 +2533,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		status = g4x_dp_detect(intel_dp);
 
 	if (status != connector_status_connected)
-		return status;
+		goto out;
 
 	intel_dp_probe_oui(intel_dp);
 
@@ -2546,7 +2549,11 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
-	return connector_status_connected;
+	status = connector_status_connected;
+
+out:
+	intel_aux_display_shutdown_allow(dev_priv);
+	return status;
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index eae3dbc..50ca1bf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -767,6 +767,9 @@ extern void intel_update_fbc(struct drm_device *dev);
 extern void intel_gpu_ips_init(struct drm_i915_private *dev_priv);
 extern void intel_gpu_ips_teardown(void);
 
+extern void
+intel_aux_display_shutdown_forbid(struct drm_i915_private *dev_priv);
+extern void intel_aux_display_shutdown_allow(struct drm_i915_private *dev_priv);
 extern bool intel_display_power_enabled(struct drm_device *dev,
 					enum intel_display_power_domain domain);
 extern void intel_init_power_well(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index bc12518..7fd186d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -866,6 +866,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 	struct edid *edid;
 	enum drm_connector_status status = connector_status_disconnected;
 
+	intel_aux_display_shutdown_forbid(dev_priv);
+
 	intel_hdmi->has_hdmi_sink = false;
 	intel_hdmi->has_audio = false;
 	intel_hdmi->rgb_quant_range_selectable = false;
@@ -893,6 +895,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 		intel_encoder->type = INTEL_OUTPUT_HDMI;
 	}
 
+	intel_aux_display_shutdown_allow(dev_priv);
 	return status;
 }
 
-- 
1.8.1.2

Re: [PATCH 6/6] drm/i915: implement HSW display sequences for package C8+

[Chris Wilson]

On Wed, Jun 05, 2013 at 02:21:56PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This patch implements "Display Sequences for Package C8", from the
> "Display Mode Set Sequence" section from the Haswell documentation.
> 
> Notice that even if we allow PC8+, there's no guarantee that we will
> actually enter PC8+, since the other devices also need to allow it.
> Also notice that we need i915.disable_power_well=1 in order to test
> this feature: we don't allow PC8+ if the power well is still enabled.
> 
> v2: - Rebase
>     - Implement many review comments form Daniel Vetter
>     - Call intel_prepare_ddi and i915_gem_init_swizzling when
>       returning from C8+ so we can actually see the screen contents

Really don't like the colour you've chosen in places, and there seem to
be a lot of placeholder code.

Highlights:
 - intel_aux_display_shutdown_forbid / allow ->
   intel_aux_display_get / put

   Not convinced by "aux" there either. Perhaps,
   intel_display_wakelock_get() instead. Or we could go with
   intel_display_forcewake_get(). Hmm, forcewake is better as we already
   have that concept in our code (and wakelock may end up being confused
   with the system-wide wakelocks.)

 - move your c8 state into a struct:
   struct i915_package_c8 {
           struct mutex mutex;
	   union {
		   struct hsw_package_c8_registers hsw_regfile;
	   };
	   bool enabled; /* was allowing_package_c8 */
	   int forcewake; /* was c8_forbid_refcnt */
   };

 - The interrupt register handling is messy. We already track the values
   of the ones that we change, the others should be static. Looks
   mostly correct (barring a race with an interrupt!).

 - Hooking into modeset_global_resources seems strange and never
   explained.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 6/6] drm/i915: implement HSW display sequences for package C8+

[Daniel Vetter]

On Thu, Jun 06, 2013 at 07:49:49AM +0100, Chris Wilson wrote:
> On Wed, Jun 05, 2013 at 02:21:56PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > This patch implements "Display Sequences for Package C8", from the
> > "Display Mode Set Sequence" section from the Haswell documentation.
> > 
> > Notice that even if we allow PC8+, there's no guarantee that we will
> > actually enter PC8+, since the other devices also need to allow it.
> > Also notice that we need i915.disable_power_well=1 in order to test
> > this feature: we don't allow PC8+ if the power well is still enabled.
> > 
> > v2: - Rebase
> >     - Implement many review comments form Daniel Vetter
> >     - Call intel_prepare_ddi and i915_gem_init_swizzling when
> >       returning from C8+ so we can actually see the screen contents
> 
> Really don't like the colour you've chosen in places, and there seem to
> be a lot of placeholder code.
> 
> Highlights:
>  - intel_aux_display_shutdown_forbid / allow ->
>    intel_aux_display_get / put
> 
>    Not convinced by "aux" there either. Perhaps,
>    intel_display_wakelock_get() instead. Or we could go with
>    intel_display_forcewake_get(). Hmm, forcewake is better as we already
>    have that concept in our code (and wakelock may end up being confused
>    with the system-wide wakelocks.)

aux_display is my bikeshed ;-) The idea was that we have a russian doll
stack of display power knobs:
- the power well to shut of everything but edp on pipe A
- the power well for all display pipes (not yet on haswell)
- power control for aux stuff like gmbus/dp aux
- the real pci device

In reality it's probably more a tree since rc6 forcewake also hangs in
there somewhere. Maybe we could also track a few more things explicitly,
e.g. the refclock plls could probably be refcounted by their users (e.g.
ddi plls).

In the end I guess most of these need to be refcounted, and child power
domains need to also grab a reference on their parents on the 0->1
transition. One idea I'm pondering is whether we should make all those
power domains explicit by mapping them to platform devices we create.
Upsides:
- Someone clearly thought about funy races and locking issues when writing
  the pm runtime stuff. Using it would avoid us reinventing that wheel.
- struct device is the main object for pm in linux, so if we can make our
  aux device (like i2c) childs of the correct power domain (what I've
  called "aux display") instead of the pci device we might gain a bit in
  integration.
- Full runtime PM at the pci level seems to be on the table anyway.

>  - move your c8 state into a struct:
>    struct i915_package_c8 {
>            struct mutex mutex;
> 	   union {
> 		   struct hsw_package_c8_registers hsw_regfile;
> 	   };
> 	   bool enabled; /* was allowing_package_c8 */
> 	   int forcewake; /* was c8_forbid_refcnt */
>    };
> 
>  - The interrupt register handling is messy. We already track the values
>    of the ones that we change, the others should be static. Looks
>    mostly correct (barring a race with an interrupt!).

I've inked in today for starting my irq review (thanks to vecs), I think
we should do that first and then look again at the irq prep patches. But I
agree that in theory we should only need to clear out the unused
interrupts once in preinstall. For paranoia we could add a few WARNs in
the uninstall hooks.

>  - Hooking into modeset_global_resources seems strange and never
>    explained.

Modeset is a bit ugly for power domain refcounting (it's the same mess for
the power well as for pc8+ here). The issue I think is that we don't have
a resource acquire/release stage where platform code could hook in.
crtc_disable would be at the right spot, but is also called with the dpms
off stuff (so not suitable for everything). Off isn't called
unconditionally when shutting down a pipe for real (if we reuse the pipe
the cleanup is done in ->mode_set). And crtc_enable is too late for some
cases since global_modeset_resources and crtc mode_set already need to
touch the hw.

Maybe we could use the for_each_crtc loop we already have and grab a
refcount for each crtc. If we switch the power well code to refcounting
that would nicely unify things, too.

Another issue I'm not yet clear on is dpms handling. Atm we ignore all
that stuff for it (since neither modeset_global_resources nor mode_set
hooks are called, and the hw seems to completely forget register contents,
so this must be done). Probably the simplest solution is to call these
hooks in the dpms on path, too. But atm that plan runs afoul of the
resource accounting we still have in there (pll sharing and refcounting
mostly).

Tbh I don't have a clear overall idea for this yet.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/6] drm/i915: add ibx_irq_preinstall

[Daniel Vetter]

On Wed, Jun 05, 2013 at 02:21:51PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So we can remove some duplicate code. All the PCHs are very similar
> and right now the code is the same. I plan to add more code, so we
> would have more duplicated code.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch. For the other prep patches I'd
like to review our interrupt code first a bit, like describe in my reply
to the vecs enabling patches.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 44 ++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 63996aa..c482e8a 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2472,6 +2472,25 @@ void i915_hangcheck_elapsed(unsigned long data)
>  					   DRM_I915_HANGCHECK_JIFFIES));
>  }
>  
> +static void ibx_irq_preinstall(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (HAS_PCH_NOP(dev))
> +		return;
> +
> +	/* south display irq */
> +	I915_WRITE(SDEIMR, 0xffffffff);
> +	/*
> +	 * SDEIER is also touched by the interrupt handler to work around missed
> +	 * PCH interrupts. Hence we can't update it after the interrupt handler
> +	 * is enabled - instead we unconditionally enable all PCH interrupt
> +	 * sources here, but then only unmask them as needed with SDEIMR.
> +	 */
> +	I915_WRITE(SDEIER, 0xffffffff);
> +	POSTING_READ(SDEIER);
> +}
> +
>  /* drm_dma.h hooks
>  */
>  static void ironlake_irq_preinstall(struct drm_device *dev)
> @@ -2493,16 +2512,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
>  	I915_WRITE(GTIER, 0x0);
>  	POSTING_READ(GTIER);
>  
> -	/* south display irq */
> -	I915_WRITE(SDEIMR, 0xffffffff);
> -	/*
> -	 * SDEIER is also touched by the interrupt handler to work around missed
> -	 * PCH interrupts. Hence we can't update it after the interrupt handler
> -	 * is enabled - instead we unconditionally enable all PCH interrupt
> -	 * sources here, but then only unmask them as needed with SDEIMR.
> -	 */
> -	I915_WRITE(SDEIER, 0xffffffff);
> -	POSTING_READ(SDEIER);
> +	ibx_irq_preinstall(dev);
>  }
>  
>  static void ivybridge_irq_preinstall(struct drm_device *dev)
> @@ -2529,19 +2539,7 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
>  	I915_WRITE(GEN6_PMIER, 0x0);
>  	POSTING_READ(GEN6_PMIER);
>  
> -	if (HAS_PCH_NOP(dev))
> -		return;
> -
> -	/* south display irq */
> -	I915_WRITE(SDEIMR, 0xffffffff);
> -	/*
> -	 * SDEIER is also touched by the interrupt handler to work around missed
> -	 * PCH interrupts. Hence we can't update it after the interrupt handler
> -	 * is enabled - instead we unconditionally enable all PCH interrupt
> -	 * sources here, but then only unmask them as needed with SDEIMR.
> -	 */
> -	I915_WRITE(SDEIER, 0xffffffff);
> -	POSTING_READ(SDEIER);
> +	ibx_irq_preinstall(dev);
>  }
>  
>  static void valleyview_irq_preinstall(struct drm_device *dev)
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 0/6] Enable PC8+ states

[Daniel Vetter]

On Wed, Jun 05, 2013 at 02:21:50PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Hi
> 
> So this series is a second version of the patch I sent on April 16th. Daniel
> asked to write some patches to properly initialize the interrupts we're touching
> on the PC8+ patches, so the first 5 patches should do this.
> 
> The 6th patch is the big one enabling PC8+ and even though I still didn't
> address all of Daniel's concerns from his first review, the patch already works:
> we get into PC10 and when we get back to PC7 things are still working.

Ok, so real review now, not just function name bikeshedding ;-)

* I'd really like to see a testcase, but not per se to exercise pc8+ but
  to check that our prepare/exit code and especially the interrupt
  disabling/enabling doesn't break anything. So the overall structure of
  the test should probably be something like

  1. Put the console into raw mode (so that fbcon doesn't interfere),
  Imre's recent infrastructure patches should make this really simple.

  2. Disable all outputs with setcrtc(fb=NULL).

  3. Wait a bit until we're sure that our code has prepared for pc8+, i.e.
  all the interrupts are shut off, lcpll is disabled ...

  4. Run a subtest.

  5. Repeat.

  For subtests we need quite a pile of them I think:
  - Test getconnectors ioctl to make sure both our ->detec and ->get_modes
    callbacks have the proper aux_display runtime pm get/put calls. On a
    quick read through your patch the get/put (currently still call
    forbid/allow) in the ->get_modes hooks seem to be missing.
  - Direct i2c access from userspace. This happens both from random tools
    probing the i2c bus, but also for real uses-cases, like the ddccontrol
    tool or e.g. the Chromebook Pixie which uses gmbus as the hw i2c
    engine for the touchpad.
  - Submit a patch (this might just work when waking the gt automatically
    wakes up everything) _and_ do a blocking wait for it (this will break
    if interrupts are dead).

  It's probably good to have some correctness checks for these, e.g.
  compare the edid property while outputs are still on (so pc8+ is
  impossible) with what happens when they're off. Execbuf could be tested
  with a simple fill blt. Imo it's also important that we have a wait
  before each new subtest, to make sure we really teste everything. E.g. a
  subtest only checks EDID for one output, then waits again. Otherwise it
  could be that the working refcounting in e.g. HDMI could shadow broken
  runtime pm refcounting for dp aux if it's done right afterwards.

* spin_lock_irqsave isn't that irq-save ;-) It only blocks interrupt on
  the local cpu. See Rusty's unreliable guide to kernel locking in
  Documentation/DocBook/kernel-locking. If you don't understand what
  spin_lock_irqsave is useful for, how our current locking works or why
  the one in your patch is broken I think we should discuss this
  afterwards. Reading my irq locking review patches and comparing it to
  the locking guid could also be interesting.

  I haven't yet done the full review, but probably the safest route is to
  fully disable our interrupt handler with disable_irq while we do the
  pc8+ prepare sequence. On the exit sequence we probably don't need it.
  In any case we can reenable interrupts again with enable_irq. Maybe
  there's a possible race there (I need to review the core irq code
  first), but now I think having a tiny race there where we could miss hpd
  events is less evil than adding complex locking just for pc8+ entry.
  Even more so with the next point, which aims to unify pc8+ entry/exit
  with our existing suspend/resume code. And s/r runs in a non-reentrant
  enviroment, so if we can keep those basic assumptions that's imo
  worthwile.

* Unifying irq setup/teardown code. I'm not really happy with the
  setup/teardown code, and I think we should take a good look at trying to
  unify it with the existing driver load/unload/suspend/resume code.
  Massive register save/restore code tends to get out of sync with other
  code changes way too easily.

  Currently our irq enable sequence is:
  1. call ->irq_preinstall hook
  2. enable irq handler
  3. call ->irq_postinstall hook
  and somewhen later
  4. enable hotplug interrupt handling with our own ->hpd_irq_setup hook.

  The disabling sequence is:
  1. disable irq handler
  2. call ->irq_uninstall hook

  Now for pc8+ we want to disable/enable almost everything _but_ the hpd
  interrupts. I think with some code wrestling we should be able to do
  that with the existing functions:
  - Split the irq_unistall into two parts, irq_unistall which disables all
    interrupt sources _but_ hotplug, and a hpd_irq_teardown which disables
    hotplug, too. For that we'd need an i915_irq_unistall functions which
    first calls drm_irq_uninstall and the new ->hpd_irq_teardown hook.
    That would also allow us to unify the hotplug reenable timer teardown
    a bit.
  - Rework ->irq_postinstall and ->irq_uninstall to not frob hpd
    settings. Since we control hpd interrupts all through SDEIMR it should
    be fairly simple to disable everything but hotplug interrupts, even
    more so with the new helpers I've added in my irq locking review
    series.
  - Important is that the new ->irq_uninstall hook won't disable the
    master interrupt sources, that'd would all be moved into the
    ->hpd_irq_teardown code.

  With the pc8+ prepare sequence would look like
  1. disable irq handler
  2. call ->irq_unistall hook directly
  -> hpd interrupts are still working
  3. re-enable irq handler

  And the exit sequence would be
  1. disable irq handler
  2. call ->irq_postinstall
  3. re-enable irq handler

* As you might have guessed I see the biggest risk in us screwing up the
  interrupt handling and especially someone sneaking in while we're in
  pc8+ mode and changing the irq registers. Safe for the hotplug bits
  which might need masking due to a hotplug storm I expect all such
  changes to be bugs.

  The upshot of the above proposal now is that we can make sure that for
  all the pc8+ relevant interrupt bits calling ->irq_uninstall leaves
  behind the same state as ->irq_preinstall. So we could sprinkle massive
  amounts of WARNs into ->irq_postinstall to check that all the interupt
  registers are still in the correct state and no one changed them.

* Finally (and this part is really just an idea I'll throw out here) our
  driver setup/teardown sequences are getting pretty complicated, and
  different parts (load, s/r, pc8+, ...) will use them ever so slightly. I
  think adding an

  atomic_t dev_priv->driver_stage_ready

  bit mask would be useful. Every time a driver stage is
  loaded/setup/resumed it sets the bit, if we unload/suspend/teardown it
  gets cleared. And every driver setup stage can check at the beginning
  whether all the required pieces are present. This should help us in
  documenting the sometimes tricky ordering constraints in general.

  For this case specifically I'm thinking of 2 driver stages:
  - general_irq_ready
  - hpd_irq_ready
  Since atomic_read is as cheap as any unordered load we could sprinkle a
  few of these checks over the code, e.g. in the gmbus or dp aux irq-based
  wait code. Similar e.g. for gem in the __wait_seqno function. This
  should help a lot in debugging bugs due to missing refcounting.

For priorities I think the best approach is to start with the testcase.
I think currently you're code is missing quite a few runtime pm get/put
calls, so I'm pretty sure it'll blow up. And if it doesn't my
understanding of how pc8+ works is seriously wrong.

And once we have a solid testcase it should be much easier to move the
code piece around a bit so that we have a sane pc8+ enter/exit sequence.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch