[PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Zhenyu Wang]

We should enable FDI normal training on Sandybridge/CPT system
as well. Also restore back some original register posting read
that got removed. LVDS/VGA/HDMI seems back to life but DP still
fails.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   80 +++++++++++++++++++++++-----------
 2 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5948e05..228c571 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1322,6 +1322,7 @@ static inline void i915_write(struct drm_i915_private *dev_priv, u32 reg,
 
 #define INTEL_PCH_TYPE(dev) (((struct drm_i915_private *)(dev)->dev_private)->pch_type)
 #define HAS_PCH_CPT(dev) (INTEL_PCH_TYPE(dev) == PCH_CPT)
+#define HAS_PCH_IBX(dev) (INTEL_PCH_TYPE(dev) == PCH_IBX)
 
 #define PRIMARY_RINGBUFFER_SIZE         (128*1024)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a413db6..b6f977b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1685,6 +1685,37 @@ static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock)
 	udelay(500);
 }
 
+static void intel_fdi_normal_train(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	u32 reg, temp;
+
+	/* enable normal train */
+	reg = FDI_TX_CTL(pipe);
+	temp = I915_READ(reg);
+	temp &= ~FDI_LINK_TRAIN_NONE;
+	temp |= FDI_LINK_TRAIN_NONE | FDI_TX_ENHANCE_FRAME_ENABLE;
+	I915_WRITE(reg, temp);
+
+	reg = FDI_RX_CTL(pipe);
+	temp = I915_READ(reg);
+	if (HAS_PCH_CPT(dev)) {
+		temp &= ~FDI_LINK_TRAIN_PATTERN_MASK_CPT;
+		temp |= FDI_LINK_TRAIN_NORMAL_CPT;
+	} else {
+		temp &= ~FDI_LINK_TRAIN_NONE;
+		temp |= FDI_LINK_TRAIN_NONE;
+	}
+	I915_WRITE(reg, temp | FDI_RX_ENHANCE_FRAME_ENABLE);
+
+	/* wait one idle pattern time */
+	POSTING_READ(reg);
+	udelay(1000);
+}
+
 /* The FDI link training functions for ILK/Ibexpeak. */
 static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 {
@@ -1771,27 +1802,6 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 
 	DRM_DEBUG_KMS("FDI train done\n");
 
-	/* enable normal train */
-	reg = FDI_TX_CTL(pipe);
-	temp = I915_READ(reg);
-	temp &= ~FDI_LINK_TRAIN_NONE;
-	temp |= FDI_LINK_TRAIN_NONE | FDI_TX_ENHANCE_FRAME_ENABLE;
-	I915_WRITE(reg, temp);
-
-	reg = FDI_RX_CTL(pipe);
-	temp = I915_READ(reg);
-	if (HAS_PCH_CPT(dev)) {
-		temp &= ~FDI_LINK_TRAIN_PATTERN_MASK_CPT;
-		temp |= FDI_LINK_TRAIN_NORMAL_CPT;
-	} else {
-		temp &= ~FDI_LINK_TRAIN_NONE;
-		temp |= FDI_LINK_TRAIN_NONE;
-	}
-	I915_WRITE(reg, temp | FDI_RX_ENHANCE_FRAME_ENABLE);
-
-	/* wait one idle pattern time */
-	POSTING_READ(reg);
-	udelay(1000);
 }
 
 static const int const snb_b_fdi_train_param [] = {
@@ -1980,7 +1990,7 @@ static void intel_clear_scanline_wait(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 tmp;
 
-	if (IS_GEN2(dev))
+	if (IS_GEN2(dev) || IS_GEN6(dev))
 		/* Can't break the hang on i8xx */
 		return;
 
@@ -2022,8 +2032,10 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
 		temp = I915_READ(PCH_LVDS);
-		if ((temp & LVDS_PORT_EN) == 0)
+		if ((temp & LVDS_PORT_EN) == 0) {
 			I915_WRITE(PCH_LVDS, temp | LVDS_PORT_EN);
+			POSTING_READ(PCH_LVDS);
+		}
 	}
 
 	ironlake_fdi_enable(crtc);
@@ -2083,6 +2095,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 		else if (pipe == 1 && (temp & TRANSB_DPLL_ENABLE) == 0)
 			temp |= (TRANSB_DPLL_ENABLE | TRANSB_DPLLB_SEL);
 		I915_WRITE(PCH_DPLL_SEL, temp);
+		POSTING_READ(PCH_DPLL_SEL);
 	}
 
 	/* set transcoder timing */
@@ -2094,6 +2107,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	I915_WRITE(TRANS_VBLANK(pipe), I915_READ(VBLANK(pipe)));
 	I915_WRITE(TRANS_VSYNC(pipe),  I915_READ(VSYNC(pipe)));
 
+	intel_fdi_normal_train(crtc);
+
 	/* For PCH DP, enable TRANS_DP_CTL */
 	if (HAS_PCH_CPT(dev) &&
 	    intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
@@ -2204,9 +2219,10 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	udelay(100);
 
 	/* Ironlake workaround, disable clock pointer after downing FDI */
-	I915_WRITE(FDI_RX_CHICKEN(pipe),
-		   I915_READ(FDI_RX_CHICKEN(pipe) &
-			     ~FDI_RX_PHASE_SYNC_POINTER_ENABLE));
+	if (HAS_PCH_IBX(dev))
+		I915_WRITE(FDI_RX_CHICKEN(pipe),
+			   I915_READ(FDI_RX_CHICKEN(pipe) &
+				     ~FDI_RX_PHASE_SYNC_POINTER_ENABLE));
 
 	/* still set train pattern 1 */
 	reg = FDI_TX_CTL(pipe);
@@ -2251,12 +2267,21 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 			DRM_ERROR("failed to disable transcoder\n");
 	}
 
+	/* keep bpc mask in transcoder */
+	temp = I915_READ(reg);
+	temp &= ~PIPE_BPC_MASK;
+	temp |= (I915_READ(PIPECONF(pipe)) & PIPE_BPC_MASK);
+	I915_WRITE(reg, temp);
+	POSTING_READ(reg);
+	udelay(100);
+
 	if (HAS_PCH_CPT(dev)) {
 		/* disable TRANS_DP_CTL */
 		reg = TRANS_DP_CTL(pipe);
 		temp = I915_READ(reg);
 		temp &= ~(TRANS_DP_OUTPUT_ENABLE | TRANS_DP_PORT_SEL_MASK);
 		I915_WRITE(reg, temp);
+		POSTING_READ(reg);
 
 		/* disable DPLL_SEL */
 		temp = I915_READ(PCH_DPLL_SEL);
@@ -2265,17 +2290,20 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 		else
 			temp &= ~(TRANSB_DPLL_ENABLE | TRANSB_DPLLB_SEL);
 		I915_WRITE(PCH_DPLL_SEL, temp);
+		POSTING_READ(PCH_DPLL_SEL);
 	}
 
 	/* disable PCH DPLL */
 	reg = PCH_DPLL(pipe);
 	temp = I915_READ(reg);
 	I915_WRITE(reg, temp & ~DPLL_VCO_ENABLE);
+	POSTING_READ(reg);
 
 	/* Switch from PCDclk to Rawclk */
 	reg = FDI_RX_CTL(pipe);
 	temp = I915_READ(reg);
 	I915_WRITE(reg, temp & ~FDI_PCDCLK);
+	POSTING_READ(reg);
 
 	/* Disable CPU FDI TX PLL */
 	reg = FDI_TX_CTL(pipe);
-- 
1.7.1

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Chris Wilson]

On Thu, 28 Oct 2010 16:38:08 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> We should enable FDI normal training on Sandybridge/CPT system
> as well. Also restore back some original register posting read
> that got removed. LVDS/VGA/HDMI seems back to life but DP still
> fails.

The POSTING_READs you added are no-ops since they are all followed by a
read. The transconf should have the bpc in the register at this point or
else we should have bugged much earlier. The break-exec-wait condition is
still documented for gen6, augmented with an additional break-sempahore-wait
and obviously per-ring, but this is unrelated to the changelog.

I've shrunk the patch to just the FDI portion, pushed to staging for
review.  Let's try to keep patches as minimal as possible and addressing
a single issue. (I'm as guilty of violating that as anyone.) Mostly so
that we have a clear history of why/how the code works (and I don't break
it later).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 958 bytes --]

On 2010.10.28 10:50:04 +0100, Chris Wilson wrote:
> 
> The POSTING_READs you added are no-ops since they are all followed by a
> read. The transconf should have the bpc in the register at this point or
> else we should have bugged much earlier. The break-exec-wait condition is
> still documented for gen6, augmented with an additional break-sempahore-wait
> and obviously per-ring, but this is unrelated to the changelog.
> 
> I've shrunk the patch to just the FDI portion, pushed to staging for
> review. 

Current drm-intel-staging still fails unfortunately..

> Let's try to keep patches as minimal as possible and addressing
> a single issue. (I'm as guilty of violating that as anyone.) Mostly so
> that we have a clear history of why/how the code works (and I don't break
> it later).

yeah, will do that, thanks for remind.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Chris Wilson]

On Fri, 29 Oct 2010 10:34:51 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> On 2010.10.28 10:50:04 +0100, Chris Wilson wrote:
> > I've shrunk the patch to just the FDI portion, pushed to staging for
> > review. 
> 
> Current drm-intel-staging still fails unfortunately..

I'm very interested to know which was the magic bit then. :)

Did we lose the transcoder bpc setting? Is an extra READ masking a missing
delay? Is the break-scanline-wait fouling gen6?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 982 bytes --]

On 2010.10.29 09:52:31 +0100, Chris Wilson wrote:
> On Fri, 29 Oct 2010 10:34:51 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > On 2010.10.28 10:50:04 +0100, Chris Wilson wrote:
> > > I've shrunk the patch to just the FDI portion, pushed to staging for
> > > review. 
> > 
> > Current drm-intel-staging still fails unfortunately..
> 
> I'm very interested to know which was the magic bit then. :)
> 
> Did we lose the transcoder bpc setting? Is an extra READ masking a missing
> delay? Is the break-scanline-wait fouling gen6?

I don't think transcoder bpc setting should matter, but sorry that I'm short
of time to track down which one extra read made the difference, my sandybridge
laptop normally refuse to boot on first time.. ;)

The code operation is same as what we have in .36 kernel, so could you restore 
behavior back first?

thanks.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Chris Wilson]

On Tue, 2 Nov 2010 14:24:22 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> I don't think transcoder bpc setting should matter, but sorry that I'm short
> of time to track down which one extra read made the difference, my sandybridge
> laptop normally refuse to boot on first time.. ;)
> 
> The code operation is same as what we have in .36 kernel, so could you restore 
> behavior back first?

The original patch was unacceptable since it did more than it claimed to
in its changelog. Just disabling the Ironlake workaround and restoring the
FDI normal train on crtc disable is insufficient. I remain dubious that
adding the POSTING_READs is sufficient without at least some explanation
and some testing.

I could break the original patch down into the 5 separate patches (of
which it sounds like we only need 3) and I would still be waiting on
someone to test them...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1358 bytes --]

On 2010.11.04 13:50:24 +0800, Zhenyu Wang wrote:
> On 2010.11.04 02:23:46 +0000, Chris Wilson wrote:
> > On Tue, 2 Nov 2010 14:24:22 +0800, Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > > I don't think transcoder bpc setting should matter, but sorry that I'm short
> > > of time to track down which one extra read made the difference, my sandybridge
> > > laptop normally refuse to boot on first time.. ;)
> > > 
> > > The code operation is same as what we have in .36 kernel, so could you restore 
> > > behavior back first?
> > 
> > The original patch was unacceptable since it did more than it claimed to
> > in its changelog. Just disabling the Ironlake workaround and restoring the
> > FDI normal train on crtc disable is insufficient. I remain dubious that
> > adding the POSTING_READs is sufficient without at least some explanation
> > and some testing.
> > 
> 
> Chris, I try to retest your changed version on drm-intel-staging.
> It looks things work fine now with that. ;)
> 
> Sorry that I'm not quite sure why my last test failed...please pick that
> one to -fixes, so QA team could pick it up for validation. 
> 
> Thanks.
> 

oops, this was rejected, now with correct subscribed mail address to the list..

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] drm/i915: Fix KMS regression on Sandybridge/CPT

[Chris Wilson]

On Thu, 4 Nov 2010 13:50:24 +0800, Zhenyu Wang <zhenyu.z.wang@intel.com> wrote:
> Chris, I try to retest your changed version on drm-intel-staging.
> It looks things work fine now with that. ;)
> 
> Sorry that I'm not quite sure why my last test failed...please pick that
> one to -fixes, so QA team could pick it up for validation. 

Applied to -fixes, thanks for following up.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre