public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Paulo Zanoni <przanoni@gmail.com>
To: intel-gfx@lists.freedesktop.org
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Subject: [PATCH 07/09] drm/i915: implement Haswell DP link train
Date: Fri, 29 Jun 2012 16:03:39 -0300	[thread overview]
Message-ID: <1340996621-14345-7-git-send-email-przanoni@gmail.com> (raw)
In-Reply-To: <1340996621-14345-1-git-send-email-przanoni@gmail.com>

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

Previously, the DP register was used for everything. On Haswell, it
was split into DDI_BUF_CTL (which is the new intel_dp->DP register)
and DP_TP_CTL.

The logic behind this patch is heavily based on a patch written by
Shobhit Kumar, but the way it was written is very different.

Credits-to: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |    3 +
 drivers/gpu/drm/i915/intel_ddi.c |   25 ++++++++
 drivers/gpu/drm/i915/intel_dp.c  |  131 ++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 4 files changed, 146 insertions(+), 14 deletions(-)

Ok, so here's another explanation. This patch is based on the following patch by
Shobhit:
[PATCH 04/21] drm/i915: Haswell specific code for the DP Link Training

Most of the logic from that patch is implemented here. The main difference is
the way this patch was coded. Patch 01/09 from this patch series moved some code
to intel_dp_set_link_train, so now we're adding even more code to it. The lack
of the intel_dp->TP variable introduced even more changes to the coding style.
Another difference is that in this new patch series, intel_dp->output_reg is
DDI_BUF_CTL, while in the previous patch series intel_dp->output_reg pointed to
the PCH DP register which does not exist on Haswell anymore, so the previous
code had to avoid writing to output_reg.

Patch 04/21 also had an udelay(600) every time set_link_train_src was called.
The way my patch series is written moves this udelay just to when it is
necessary (when we're enabling the port).

My patch also introduces intel_ddi_dp_prepare_link_retrain. When we need to
retrain DP we really need to disable all the port registers, otherwise we'll
keep getting link train failures even though "everything seems correct". But in
order to disable the port registers we need to disable the pipe registers
(otherwise we'll hang the machine when writing to PIPECONF). This is exactly the
same problem described in patch 05/09 and we hope Daniel will present a magic
solution that will save us all from breaking the abstractions and helpers.

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ce3fc2c..f28f2b2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4315,6 +4315,8 @@
 #define  DP_TP_CTL_LINK_TRAIN_MASK		(7<<8)
 #define  DP_TP_CTL_LINK_TRAIN_PAT1		(0<<8)
 #define  DP_TP_CTL_LINK_TRAIN_PAT2		(1<<8)
+#define  DP_TP_CTL_LINK_TRAIN_PAT3		(4<<8)
+#define  DP_TP_CTL_LINK_TRAIN_IDLE		(2<<8)
 #define  DP_TP_CTL_LINK_TRAIN_NORMAL	(3<<8)
 #define  DP_TP_CTL_SCRAMBLE_DISABLE	(1<<7)
 
@@ -4324,6 +4326,7 @@
 #define DP_TP_STATUS(port) _PORT(port, \
 					DP_TP_STATUS_A, \
 					DP_TP_STATUS_B)
+#define  DP_TP_STATUS_IDLE_DONE		(1<<25)
 #define  DP_TP_STATUS_AUTOTRAIN_DONE	(1<<12)
 
 /* DDI Buffer Control */
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c98a3a7..082b923 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1029,3 +1029,28 @@ void intel_ddi_dp_link_down(struct intel_dp *intel_dp)
 	intel_ddi_disable_dp_pipe(encoder);
 	intel_ddi_disable_dp_port(intel_dp);
 }
+
+/*
+ * Just disabling/reenabling DP_TP_CTL or DP_TP_CTL and DDI_BUF_CTL is not
+ * enough. We need to fully disable/reenable PORT_CLK_SEL too, otherwise the
+ * link training will always silently fail. But then there's the other problem:
+ * if we disable the port registers we also need to disable the pipe registers,
+ * otherwise we may hang the machine while writing the pipe registers.
+ *
+ * The good thing is that even though the spec says that we should only enable
+ * the pipe after the link train, it seems we can reenable it here without
+ * problems. If we find this is a problem, we should split this function in two.
+ */
+void intel_ddi_dp_prepare_link_retrain(struct drm_encoder *encoder)
+{
+	struct drm_crtc *crtc = encoder->crtc;
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+
+	ironlake_crtc_disable(crtc);
+	intel_ddi_disable_dp_pipe(encoder);
+	intel_ddi_disable_dp_port(intel_dp);
+
+	intel_ddi_enable_dp_port(intel_dp);
+	intel_ddi_enable_dp_pipe(encoder);
+	ironlake_crtc_enable(crtc);
+}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8c66c0c..287bef9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1439,7 +1439,20 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
 {
 	struct drm_device *dev = intel_dp->base.base.dev;
 
-	if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
+	if (IS_HASWELL(dev)) {
+		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
+		case DP_TRAIN_VOLTAGE_SWING_400:
+			return DP_TRAIN_PRE_EMPHASIS_9_5;
+		case DP_TRAIN_VOLTAGE_SWING_600:
+			return DP_TRAIN_PRE_EMPHASIS_6;
+		case DP_TRAIN_VOLTAGE_SWING_800:
+			return DP_TRAIN_PRE_EMPHASIS_3_5;
+		case DP_TRAIN_VOLTAGE_SWING_1200:
+		default:
+			return DP_TRAIN_PRE_EMPHASIS_0;
+		}
+
+	} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
 		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
 		case DP_TRAIN_VOLTAGE_SWING_400:
 			return DP_TRAIN_PRE_EMPHASIS_6;
@@ -1593,6 +1606,40 @@ intel_gen7_edp_signal_levels(uint8_t train_set)
 	}
 }
 
+/* Gen7.5's (HSW) DP voltage swing and pre-emphasis control */
+static uint32_t
+intel_dp_signal_levels_hsw(uint8_t train_set)
+{
+	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
+					 DP_TRAIN_PRE_EMPHASIS_MASK);
+	switch (signal_levels) {
+	case DP_TRAIN_VOLTAGE_SWING_400 | DP_TRAIN_PRE_EMPHASIS_0:
+		return DDI_BUF_EMP_400MV_0DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_400 | DP_TRAIN_PRE_EMPHASIS_3_5:
+		return DDI_BUF_EMP_400MV_3_5DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_400 | DP_TRAIN_PRE_EMPHASIS_6:
+		return DDI_BUF_EMP_400MV_6DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_400 | DP_TRAIN_PRE_EMPHASIS_9_5:
+		return DDI_BUF_EMP_400MV_9_5DB_HSW;
+
+	case DP_TRAIN_VOLTAGE_SWING_600 | DP_TRAIN_PRE_EMPHASIS_0:
+		return DDI_BUF_EMP_600MV_0DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_600 | DP_TRAIN_PRE_EMPHASIS_3_5:
+		return DDI_BUF_EMP_600MV_3_5DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_600 | DP_TRAIN_PRE_EMPHASIS_6:
+		return DDI_BUF_EMP_600MV_6DB_HSW;
+
+	case DP_TRAIN_VOLTAGE_SWING_800 | DP_TRAIN_PRE_EMPHASIS_0:
+		return DDI_BUF_EMP_800MV_0DB_HSW;
+	case DP_TRAIN_VOLTAGE_SWING_800 | DP_TRAIN_PRE_EMPHASIS_3_5:
+		return DDI_BUF_EMP_800MV_3_5DB_HSW;
+	default:
+		DRM_DEBUG_KMS("Unsupported voltage swing/pre-emphasis level:"
+			      "0x%x\n", signal_levels);
+		return DDI_BUF_EMP_400MV_0DB_HSW;
+	}
+}
+
 static uint8_t
 intel_get_lane_status(uint8_t link_status[DP_LINK_STATUS_SIZE],
 		      int lane)
@@ -1649,8 +1696,49 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	struct drm_device *dev = intel_dp->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
+	uint32_t temp;
 
-	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp))) {
+	if (IS_HASWELL(dev)) {
+
+		/* Can't change without disabling the DDI function, so keep it
+		 * forever */
+		dp_train_pat |= DP_SCRAMBLING_DISABLE;
+
+		temp = I915_READ(DP_TP_CTL(intel_dp->port));
+
+		if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE)
+			temp |= DP_TP_CTL_SCRAMBLE_DISABLE;
+		else
+			temp &= ~DP_TP_CTL_SCRAMBLE_DISABLE;
+
+		temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
+		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
+		case DP_TRAINING_PATTERN_DISABLE:
+			temp |= DP_TP_CTL_LINK_TRAIN_IDLE;
+			I915_WRITE(DP_TP_CTL(intel_dp->port), temp);
+
+			if (wait_for((I915_READ(DP_TP_STATUS(intel_dp->port)) &
+				      DP_TP_STATUS_IDLE_DONE) == 0, 1))
+				DRM_ERROR("Timed out waiting for DP idle patterns\n");
+
+			temp &= ~DP_TP_CTL_LINK_TRAIN_MASK;
+			temp |= DP_TP_CTL_LINK_TRAIN_NORMAL;
+
+			break;
+		case DP_TRAINING_PATTERN_1:
+			temp |= DP_TP_CTL_LINK_TRAIN_PAT1;
+			break;
+		case DP_TRAINING_PATTERN_2:
+			temp |= DP_TP_CTL_LINK_TRAIN_PAT2;
+			break;
+		case DP_TRAINING_PATTERN_3:
+			temp |= DP_TP_CTL_LINK_TRAIN_PAT3;
+			break;
+		}
+		I915_WRITE(DP_TP_CTL(intel_dp->port), temp);
+
+	} else if (HAS_PCH_CPT(dev) &&
+		   (IS_GEN7(dev) || !is_cpu_edp(intel_dp))) {
 		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
 		case DP_TRAINING_PATTERN_DISABLE:
 			dp_reg_value |= DP_LINK_TRAIN_OFF_CPT;
@@ -1709,7 +1797,8 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 static void
 intel_dp_start_link_train(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_encoder *encoder = &intel_dp->base.base;
+	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc);
 	int i;
@@ -1718,12 +1807,15 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	int voltage_tries, loop_tries;
 	uint32_t DP = intel_dp->DP;
 
-	/*
-	 * On CPT we have to enable the port in training pattern 1, which
-	 * will happen below in intel_dp_set_link_train.  Otherwise, enable
-	 * the port and wait for it to become active.
-	 */
-	if (!HAS_PCH_CPT(dev)) {
+	if (IS_HASWELL(dev)) {
+		/* There are a lot of registers to disable and reenable. */
+		intel_ddi_dp_prepare_link_retrain(encoder);
+	} else if (!HAS_PCH_CPT(dev)) {
+		/*
+		 * On CPT we have to enable the port in training pattern 1,
+		 * which will happen below in intel_dp_set_link_train.
+		 * Otherwise, enable the port and wait for it to become active.
+		 */
 		I915_WRITE(intel_dp->output_reg, intel_dp->DP);
 		POSTING_READ(intel_dp->output_reg);
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
@@ -1738,8 +1830,9 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 
 	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
 		DP &= ~DP_LINK_TRAIN_MASK_CPT;
-	else
+	else if (!IS_HASWELL(dev))
 		DP &= ~DP_LINK_TRAIN_MASK;
+
 	memset(intel_dp->train_set, 0, 4);
 	voltage = 0xff;
 	voltage_tries = 0;
@@ -1750,8 +1843,11 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
 		uint32_t    signal_levels;
 
-
-		if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
+		if (IS_HASWELL(dev)) {
+			signal_levels = intel_dp_signal_levels_hsw(
+							intel_dp->train_set[0]);
+			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
+		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
 			signal_levels = intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
 			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB) | signal_levels;
 		} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
@@ -1759,9 +1855,10 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB) | signal_levels;
 		} else {
 			signal_levels = intel_dp_signal_levels(intel_dp->train_set[0]);
-			DRM_DEBUG_KMS("training pattern 1 signal levels %08x\n", signal_levels);
 			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
 		}
+		DRM_DEBUG_KMS("training pattern 1 signal levels %08x\n",
+			      signal_levels);
 
 		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_1 |
@@ -1837,7 +1934,10 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
+		if (IS_HASWELL(dev)) {
+			signal_levels = intel_dp_signal_levels_hsw(intel_dp->train_set[0]);
+			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
+		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp)) {
 			signal_levels = intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
 			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_IVB) | signal_levels;
 		} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
@@ -1884,6 +1984,9 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		++tries;
 	}
 
+	if (channel_eq)
+		DRM_DEBUG_KMS("Channel EQ done. DP Training successfull\n");
+
 	intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 623d16c..eaaed91 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -539,5 +539,6 @@ extern void intel_ddi_mode_set_dp(struct drm_encoder *encoder,
 				  struct drm_display_mode *adjusted_mode);
 extern void intel_ddi_enable_dp_pipe(struct drm_encoder *encoder);
 extern void intel_ddi_dp_link_down(struct intel_dp *intel_dp);
+extern void intel_ddi_dp_prepare_link_retrain(struct drm_encoder *encoder);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.7.10.2

  parent reply	other threads:[~2012-06-29 19:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
2012-06-29 19:03 ` [PATCH 02/09] drm/i915: try to train DP even harder Paulo Zanoni
2012-07-12 14:58   ` Daniel Vetter
2012-06-29 19:03 ` [PATCH 03/09] drm/i915: Move DP structs to shared location Paulo Zanoni
2012-07-12 15:13   ` Daniel Vetter
2012-06-29 19:03 ` [PATCH 04/09] drm/i915: Add "port" field to struct intel_dp Paulo Zanoni
2012-07-12 15:04   ` Daniel Vetter
2012-06-29 19:03 ` [PATCH 05/09] drm/i915: add basic Haswell DP enablement Paulo Zanoni
2012-06-29 19:03 ` [PATCH 06/09] drm/i915: fix Haswell M/N registers Paulo Zanoni
2012-06-29 19:03 ` Paulo Zanoni [this message]
2012-06-29 19:03 ` [PATCH 08/09] drm/i915: fix DP AUX register definitions on Haswell Paulo Zanoni
2012-06-29 19:03 ` [PATCH 09/09] drm/i915: init DP instead of HDMI on port B Paulo Zanoni
2012-07-12 14:52 ` [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Daniel Vetter
2012-07-17 19:55 ` [PATCH 1/2] " Paulo Zanoni
2012-07-17 20:53 ` [PATCH 2/2] drm/i915: add port field to struct intel_dp and use it Paulo Zanoni
2012-07-18 10:42   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1340996621-14345-7-git-send-email-przanoni@gmail.com \
    --to=przanoni@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=paulo.r.zanoni@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox