All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train
@ 2012-06-29 19:03 Paulo Zanoni
  2012-06-29 19:03 ` [PATCH 02/09] drm/i915: try to train DP even harder Paulo Zanoni
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

IMHO, that code really belongs to intel_dp_set_link_train. The caller
functions are also big, so now they'll be a little bit easier to read.

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

We'll add even more code to intel_dp_set_link_train on Haswell.

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 76a7080..e315f73 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1672,6 +1672,41 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	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;
+			break;
+		case DP_TRAINING_PATTERN_1:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_1_CPT;
+			break;
+		case DP_TRAINING_PATTERN_2:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
+			break;
+		case DP_TRAINING_PATTERN_3:
+			DRM_ERROR("DP training pattern 3 not supported\n");
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
+			break;
+		}
+
+	} else {
+		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
+		case DP_TRAINING_PATTERN_DISABLE:
+			dp_reg_value |= DP_LINK_TRAIN_OFF;
+			break;
+		case DP_TRAINING_PATTERN_1:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_1;
+			break;
+		case DP_TRAINING_PATTERN_2:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
+			break;
+		case DP_TRAINING_PATTERN_3:
+			DRM_ERROR("DP training pattern 3 not supported\n");
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
+			break;
+		}
+	}
+
 	I915_WRITE(intel_dp->output_reg, dp_reg_value);
 	POSTING_READ(intel_dp->output_reg);
 
@@ -1679,12 +1714,15 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 				    DP_TRAINING_PATTERN_SET,
 				    dp_train_pat);
 
-	ret = intel_dp_aux_native_write(intel_dp,
-					DP_TRAINING_LANE0_SET,
-					intel_dp->train_set,
-					intel_dp->lane_count);
-	if (ret != intel_dp->lane_count)
-		return false;
+	if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) !=
+	    DP_TRAINING_PATTERN_DISABLE) {
+		ret = intel_dp_aux_native_write(intel_dp,
+						DP_TRAINING_LANE0_SET,
+						intel_dp->train_set,
+						intel_dp->lane_count);
+		if (ret != intel_dp->lane_count)
+			return false;
+	}
 
 	return true;
 }
@@ -1700,7 +1738,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	uint8_t voltage;
 	bool clock_recovery = false;
 	int voltage_tries, loop_tries;
-	u32 reg;
 	uint32_t DP = intel_dp->DP;
 
 	/*
@@ -1748,12 +1785,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
 		}
 
-		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-			reg = DP | DP_LINK_TRAIN_PAT_1_CPT;
-		else
-			reg = DP | DP_LINK_TRAIN_PAT_1;
-
-		if (!intel_dp_set_link_train(intel_dp, reg,
+		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_1 |
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
@@ -1808,10 +1840,8 @@ static void
 intel_dp_complete_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;
 	bool channel_eq = false;
 	int tries, cr_tries;
-	u32 reg;
 	uint32_t DP = intel_dp->DP;
 
 	/* channel equalization */
@@ -1840,13 +1870,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
 		}
 
-		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-			reg = DP | DP_LINK_TRAIN_PAT_2_CPT;
-		else
-			reg = DP | DP_LINK_TRAIN_PAT_2;
-
 		/* channel eq pattern */
-		if (!intel_dp_set_link_train(intel_dp, reg,
+		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_2 |
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
@@ -1881,15 +1906,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		++tries;
 	}
 
-	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-		reg = DP | DP_LINK_TRAIN_OFF_CPT;
-	else
-		reg = DP | DP_LINK_TRAIN_OFF;
-
-	I915_WRITE(intel_dp->output_reg, reg);
-	POSTING_READ(intel_dp->output_reg);
-	intel_dp_aux_native_write_1(intel_dp,
-				    DP_TRAINING_PATTERN_SET, DP_TRAINING_PATTERN_DISABLE);
+	intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE);
 }
 
 static void
-- 
1.7.10.2

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

* [PATCH 02/09] drm/i915: try to train DP even harder
  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 ` 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
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

While debugging Haswell link train failures I observed that we never
try the maximum voltage configuration more than once consecutively. We
start the training, the monitor keeps telling us to increase the
voltage, then when we reach the maximum we just go back to the start
(because of the "memset" above "voltage_tries = 0"). When we reach
this point, we keep alternating between the maximum and the minimum
voltages until we give up.

The DP spec suggests that we should try the same voltage 5 times
before giving up. This patch makes us try the maximum voltage at
least 5 times before going back to the minimum voltages.

This patch does not fix any particular bug I'm aware of.

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(-)

Since there's no real bug that I know this patch fixes, I won't care too much if
this gets rejected.

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e315f73..e0531b2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1807,7 +1807,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 		for (i = 0; i < intel_dp->lane_count; i++)
 			if ((intel_dp->train_set[i] & DP_TRAIN_MAX_SWING_REACHED) == 0)
 				break;
-		if (i == intel_dp->lane_count) {
+		if (i == intel_dp->lane_count && voltage_tries == 5) {
 			++loop_tries;
 			if (loop_tries == 5) {
 				DRM_DEBUG_KMS("too many full retries, give up\n");
-- 
1.7.10.2

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

* [PATCH 03/09] drm/i915: Move DP structs to shared location
  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-06-29 19:03 ` 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
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: Shobhit Kumar <shobhit.kumar@intel.com>

Move the DP structure to shared location so that it can be used from
within the ddi module.

Changes from Paulo:
- Move less code to intel_drv.h
- Remove #include statement
- Replace a tab with a space in train_set

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   32 --------------------------------
 drivers/gpu/drm/i915/intel_drv.h |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e0531b2..be5c47e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -36,42 +36,10 @@
 #include "intel_drv.h"
 #include "i915_drm.h"
 #include "i915_drv.h"
-#include "drm_dp_helper.h"
 
-#define DP_RECEIVER_CAP_SIZE	0xf
 #define DP_LINK_STATUS_SIZE	6
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
-#define DP_LINK_CONFIGURATION_SIZE	9
-
-struct intel_dp {
-	struct intel_encoder base;
-	uint32_t output_reg;
-	uint32_t DP;
-	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
-	bool has_audio;
-	enum hdmi_force_audio force_audio;
-	uint32_t color_range;
-	int dpms_mode;
-	uint8_t link_bw;
-	uint8_t lane_count;
-	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
-	struct i2c_adapter adapter;
-	struct i2c_algo_dp_aux_data algo;
-	bool is_pch_edp;
-	uint8_t	train_set[4];
-	int panel_power_up_delay;
-	int panel_power_down_delay;
-	int panel_power_cycle_delay;
-	int backlight_on_delay;
-	int backlight_off_delay;
-	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
-	struct delayed_work panel_vdd_work;
-	bool want_panel_vdd;
-	struct edid *edid; /* cached EDID for eDP */
-	int edid_mode_count;
-};
-
 /**
  * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
  * @intel_dp: DP struct
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d9a62d7..48a0fcb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -31,6 +31,7 @@
 #include "drm_crtc.h"
 #include "drm_crtc_helper.h"
 #include "drm_fb_helper.h"
+#include "drm_dp_helper.h"
 
 #define _wait_for(COND, MS, W) ({ \
 	unsigned long timeout__ = jiffies + msecs_to_jiffies(MS);	\
@@ -305,6 +306,37 @@ struct intel_hdmi {
 			       struct drm_display_mode *adjusted_mode);
 };
 
+#define DP_RECEIVER_CAP_SIZE		0xf
+#define DP_LINK_CONFIGURATION_SIZE	9
+
+struct intel_dp {
+	struct intel_encoder base;
+	uint32_t output_reg;
+	uint32_t DP;
+	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
+	bool has_audio;
+	enum hdmi_force_audio force_audio;
+	uint32_t color_range;
+	int dpms_mode;
+	uint8_t link_bw;
+	uint8_t lane_count;
+	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+	struct i2c_adapter adapter;
+	struct i2c_algo_dp_aux_data algo;
+	bool is_pch_edp;
+	uint8_t train_set[4];
+	int panel_power_up_delay;
+	int panel_power_down_delay;
+	int panel_power_cycle_delay;
+	int backlight_on_delay;
+	int backlight_off_delay;
+	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
+	struct delayed_work panel_vdd_work;
+	bool want_panel_vdd;
+	struct edid *edid; /* cached EDID for eDP */
+	int edid_mode_count;
+};
+
 static inline struct drm_crtc *
 intel_get_crtc_for_pipe(struct drm_device *dev, int pipe)
 {
-- 
1.7.10.2

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

* [PATCH 04/09] drm/i915: Add "port" field to struct intel_dp
  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-06-29 19:03 ` [PATCH 03/09] drm/i915: Move DP structs to shared location Paulo Zanoni
@ 2012-06-29 19:03 ` 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
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: Shobhit Kumar <shobhit.kumar@intel.com>

Useful for Haswell, it has a lot of DP registers that have one
instance per port.

Changes from Paulo:
- Completely change the commit message
- Rename "ddi_port" to "port"

Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   10 +++++++---
 drivers/gpu/drm/i915/intel_drv.h |    1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

I believe "port" is better than "ddi_port" since the "port" variable can be
used in the previous gens too...

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index be5c47e..a5aee26 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2477,12 +2477,16 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
 	connector->polled = DRM_CONNECTOR_POLL_HPD;
 
-	if (output_reg == DP_B || output_reg == PCH_DP_B)
+	if (output_reg == DP_B || output_reg == PCH_DP_B) {
 		intel_encoder->clone_mask = (1 << INTEL_DP_B_CLONE_BIT);
-	else if (output_reg == DP_C || output_reg == PCH_DP_C)
+		intel_dp->port = PORT_B;
+	} else if (output_reg == DP_C || output_reg == PCH_DP_C) {
 		intel_encoder->clone_mask = (1 << INTEL_DP_C_CLONE_BIT);
-	else if (output_reg == DP_D || output_reg == PCH_DP_D)
+		intel_dp->port = PORT_C;
+	} else if (output_reg == DP_D || output_reg == PCH_DP_D) {
 		intel_encoder->clone_mask = (1 << INTEL_DP_D_CLONE_BIT);
+		intel_dp->port = PORT_D;
+	}
 
 	if (is_edp(intel_dp)) {
 		intel_encoder->clone_mask = (1 << INTEL_EDP_CLONE_BIT);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 48a0fcb..1145403 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -316,6 +316,7 @@ struct intel_dp {
 	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
+	int port;
 	uint32_t color_range;
 	int dpms_mode;
 	uint8_t link_bw;
-- 
1.7.10.2

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

* [PATCH 05/09] drm/i915: add basic Haswell DP enablement
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (2 preceding siblings ...)
  2012-06-29 19:03 ` [PATCH 04/09] drm/i915: Add "port" field to struct intel_dp Paulo Zanoni
@ 2012-06-29 19:03 ` Paulo Zanoni
  2012-06-29 19:03 ` [PATCH 06/09] drm/i915: fix Haswell M/N registers Paulo Zanoni
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

Add functions to make it follow the mode set sequence. Nothing is
using this code yet. We still lack proper link training and port
detection.

Part of this patch is based on a previous patch made by Shobhit Kumar.

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      |   22 +++
 drivers/gpu/drm/i915/intel_ddi.c     |  267 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |    4 +-
 drivers/gpu/drm/i915/intel_dp.c      |   26 +++-
 drivers/gpu/drm/i915/intel_drv.h     |    8 +
 5 files changed, 323 insertions(+), 4 deletions(-)

Ok, so here comes a big explanation.

This patch is inspired by the following patch from Shobhit:
[PATCH 03/21] drm/i915: Add DP Helper functions for Haswell
But if you notice, there's a big difference.

One of the main differences is that in Haswell mode set sequence we really must
set the port registers before the pipe registers. If we disable the port
registers and then try to change the pipe registers, we can even get a full
machine hang. The previous patch by Shobhit was not following the mode set
sequence that should be done in our hardware (even though it was working for the
common case). This patch tries to write the pipe and port registers strictly in
the order they are meant to be written.

Writing the registers in the correct order leads to another problem: setting the
port registers before the pipe register is the reverse order of what the current
DRM abstractions do. What I did was to direclty call ironlake_crtc_{en,dis}able
from inside the encoder code, which can be considered as "breaking the
abstraction". I talked to Daniel today about this, and it seems he has a better
solution for this in his head, so let's wait and see his suggestions...

There is one MSA register per pipe, not a sigle one. Patch 03/21 called it
HSW_MSA_CTL. I call it PIPE_MSA_MISC(pipe).

This patch sets the sync polarity.

This patch does not assume a 450MHz clock always.

This patch does not assume 8BPC always.

I removed the intel_dp->TP variable. I do recognize it was created in an
analogous way than intel_dp->DP, but I think these variables just make the code
harder to read (should I check/change/use the value written in the register or
the value written in the variable? when should I update the register and when
should I update the variable?). My goal is to try to remove intel_dp->DP in the
future.

There's no SSC. I'm not sure that "dpcd[DP_MAX_DOWNSPREAD] & 0x1 == 0x1" means
we should be using SSC, especially since DisplayPort v1.1 says that this
statement should always be true.

There's no IS_HASWELL() on a function that only runs on Haswell.

There's DPMS now (and it's done by intel_dp_dpms, not intel_ddi_dpms).

I'm also starting to think that we maybe should move DP code from intel_ddi to
intel_dp. This will allow a few more functions to be static and may even remove
the need for the patch that moves "struct intel_dp" declaration from intel_dp.c
to intel_drv.h.

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8202d48..ce3fc2c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4284,6 +4284,7 @@
 /* Those bits are ignored by pipe EDP since it can only connect to DDI A */
 #define  PIPE_DDI_PORT_MASK			(7<<28)
 #define  PIPE_DDI_SELECT_PORT(x)		((x)<<28)
+#define  PIPE_DDI_PORT_NONE			(0<<28)
 #define  PIPE_DDI_MODE_SELECT_HDMI		(0<<24)
 #define  PIPE_DDI_MODE_SELECT_DVI		(1<<24)
 #define  PIPE_DDI_MODE_SELECT_DP_SST	(2<<24)
@@ -4293,6 +4294,8 @@
 #define  PIPE_DDI_BPC_10				(1<<20)
 #define  PIPE_DDI_BPC_6					(2<<20)
 #define  PIPE_DDI_BPC_12				(3<<20)
+#define  PIPE_DDI_PVSYNC			(1<<17)
+#define  PIPE_DDI_PHSYNC			(1<<16)
 #define  PIPE_DDI_BFI_ENABLE			(1<<4)
 #define  PIPE_DDI_PORT_WIDTH_X1			(0<<1)
 #define  PIPE_DDI_PORT_WIDTH_X2			(1<<1)
@@ -4313,6 +4316,7 @@
 #define  DP_TP_CTL_LINK_TRAIN_PAT1		(0<<8)
 #define  DP_TP_CTL_LINK_TRAIN_PAT2		(1<<8)
 #define  DP_TP_CTL_LINK_TRAIN_NORMAL	(3<<8)
+#define  DP_TP_CTL_SCRAMBLE_DISABLE	(1<<7)
 
 /* DisplayPort Transport Status */
 #define DP_TP_STATUS_A			0x64044
@@ -4385,6 +4389,9 @@
 #define  PIXCLK_GATE_UNGATE		1<<0
 #define  PIXCLK_GATE_GATE		0<<0
 
+#define CDCLK_FREQ		0x46200
+#define  CDCLK_FREQ_MASK	0x3FF
+
 /* SPLL */
 #define SPLL_CTL				0x46020
 #define  SPLL_PLL_ENABLE		(1<<31)
@@ -4417,6 +4424,7 @@
 #define  PORT_CLK_SEL_SPLL			(3<<29)
 #define  PORT_CLK_SEL_WRPLL1		(4<<29)
 #define  PORT_CLK_SEL_WRPLL2		(5<<29)
+#define  PORT_CLK_SEL_NONE		(7<<29)
 
 /* Pipe clock selection */
 #define PIPE_CLK_SEL_A			0x46140
@@ -4428,12 +4436,26 @@
 #define  PIPE_CLK_SEL_DISABLED	(0x0<<29)
 #define  PIPE_CLK_SEL_PORT(x)	((x+1)<<29)
 
+#define _PIPEA_MSA_MISC	0x60410
+#define _PIPEB_MSA_MISC	0x61410
+#define PIPE_MSA_MISC(pipe) _PIPE(pipe, _PIPEA_MSA_MISC, _PIPEB_MSA_MISC)
+#define  PIPE_MSA_SYNC_CLK	(1<<0)
+#define  PIPE_MSA_6_BPC		(0<<5)
+#define  PIPE_MSA_8_BPC		(1<<5)
+#define  PIPE_MSA_10_BPC	(2<<5)
+#define  PIPE_MSA_12_BPC	(3<<5)
+#define  PIPE_MSA_16_BPC	(3<<5)
+
 /* LCPLL Control */
 #define LCPLL_CTL				0x130040
 #define  LCPLL_PLL_DISABLE		(1<<31)
 #define  LCPLL_PLL_LOCK			(1<<30)
+#define  LCPLL_NON_SSC_REF		(0)
+#define  LCPLL_CD_FREQ_450MHZ		(0)
+#define  LCPLL_CD_FREQ_540MHZ		(1<<26)
 #define  LCPLL_CD_CLOCK_DISABLE	(1<<25)
 #define  LCPLL_CD2X_CLOCK_DISABLE	(1<<23)
+#define  LCPLL_CD_SOURCE_LCPLL		(0)
 
 /* Pipe WM_LINETIME - watermark line time */
 #define PIPE_WM_LINETIME_A		0x45270
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ac62330..c98a3a7 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -739,6 +739,148 @@ void intel_ddi_mode_set_hdmi(struct drm_encoder *encoder,
 	intel_hdmi->set_infoframes(encoder, adjusted_mode);
 }
 
+static void assert_ddi_dp_disabled(struct drm_encoder *encoder)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int port = intel_dp->port;
+	uint32_t val;
+
+	val = I915_READ(PORT_CLK_SEL(port));
+	WARN(val != PORT_CLK_SEL_NONE, "Port clock select not 'None'\n");
+	val = I915_READ(DDI_BUF_CTL(port));
+	WARN(val & DDI_BUF_CTL_ENABLE, "DP DDI buffer enabled\n");
+	val = I915_READ(DP_TP_CTL(port));
+	WARN(val & DP_TP_CTL_ENABLE, "DP TP control enabled\n");
+}
+
+
+static void intel_ddi_enable_dp_port(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int port = intel_dp->port;
+	uint32_t tp_val, tmp_val;
+
+	switch (intel_dp->link_bw) {
+	case DP_LINK_BW_1_62:
+		tmp_val = PORT_CLK_SEL_LCPLL_810;
+		break;
+	case DP_LINK_BW_2_7:
+		tmp_val = PORT_CLK_SEL_LCPLL_1350;
+		break;
+	case DP_LINK_BW_5_4:
+		tmp_val = PORT_CLK_SEL_LCPLL_2700;
+		break;
+	default:
+		tmp_val = PORT_CLK_SEL_LCPLL_1350;
+		WARN(1, "Link bandwidth %d unsupported\n", intel_dp->link_bw);
+	}
+	I915_WRITE(PORT_CLK_SEL(port), tmp_val);
+	POSTING_READ(PORT_CLK_SEL(port));
+
+	tp_val = DP_TP_CTL_ENABLE | DP_TP_CTL_MODE_SST |
+		 DP_TP_CTL_LINK_TRAIN_PAT1 | DP_TP_CTL_SCRAMBLE_DISABLE;
+	if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN)
+		tp_val |= DP_TP_CTL_ENHANCED_FRAME_ENABLE;
+	I915_WRITE(DP_TP_CTL(port), tp_val);
+	POSTING_READ(DP_TP_CTL(port));
+
+	intel_dp->DP |= DDI_BUF_CTL_ENABLE;
+	I915_WRITE(DDI_BUF_CTL(port), intel_dp->DP);
+	POSTING_READ(DDI_BUF_CTL(port));
+
+	udelay(600);
+}
+
+static void intel_ddi_disable_dp_port(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int port = intel_dp->port;
+	uint32_t val;
+
+	intel_dp->DP &= ~DDI_BUF_CTL_ENABLE;
+	I915_WRITE(DDI_BUF_CTL(port), intel_dp->DP);
+	POSTING_READ(DDI_BUF_CTL(port));
+
+	val = I915_READ(DP_TP_CTL(port));
+	val &= ~(DP_TP_CTL_ENABLE | DP_TP_CTL_LINK_TRAIN_MASK);
+	val |= DP_TP_CTL_LINK_TRAIN_PAT1;
+	I915_WRITE(DP_TP_CTL(port), val);
+	POSTING_READ(DP_TP_CTL(port));
+
+	udelay(8);
+
+	I915_WRITE(PORT_CLK_SEL(port), PORT_CLK_SEL_NONE);
+	POSTING_READ(PORT_CLK_SEL(port));
+}
+
+void intel_ddi_mode_set_dp(struct drm_encoder *encoder,
+			   struct drm_display_mode *mode,
+			   struct drm_display_mode *adjusted_mode)
+{
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int port = intel_dp->port;
+	uint32_t lcpll_val, tmp_val;
+
+	DRM_DEBUG_KMS("Preparing DP DDI mode on port %c\n", port_name(port));
+
+	assert_ddi_dp_disabled(encoder);
+
+	intel_dp->DP = DDI_BUF_CTL_ENABLE | DDI_BUF_EMP_400MV_0DB_HSW;
+	switch (intel_dp->lane_count) {
+	case 1:
+		intel_dp->DP |= DDI_PORT_WIDTH_X1;
+		break;
+	case 2:
+		intel_dp->DP |= DDI_PORT_WIDTH_X2;
+		break;
+	case 4:
+		intel_dp->DP |= DDI_PORT_WIDTH_X4;
+		break;
+	default:
+		intel_dp->DP |= DDI_PORT_WIDTH_X4;
+		WARN(1, "Unexpected DP lane_count %d\n", intel_dp->lane_count);
+		break;
+	}
+
+	if (intel_dp->has_audio)
+		DRM_DEBUG_DRIVER("HSW DP audio not supported yet.\n");
+
+	memset(intel_dp->link_configuration, 0, DP_LINK_CONFIGURATION_SIZE);
+	intel_dp->link_configuration[0] = intel_dp->link_bw;
+	intel_dp->link_configuration[1] = intel_dp->lane_count;
+	intel_dp->link_configuration[8] = DP_SET_ANSI_8B10B;
+	/*
+	 * Check for DPCD version > 1.1 and enhanced framing support
+	 */
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
+	    (intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_ENHANCED_FRAME_CAP))
+		intel_dp->link_configuration[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
+
+	lcpll_val = LCPLL_NON_SSC_REF | LCPLL_CD_SOURCE_LCPLL;
+
+	tmp_val = I915_READ(CDCLK_FREQ) & CDCLK_FREQ_MASK;
+	if (tmp_val == 449) {
+		lcpll_val |= LCPLL_CD_FREQ_450MHZ;
+	} else if (tmp_val == 539) {
+		lcpll_val |= LCPLL_CD_FREQ_540MHZ;
+	} else {
+		DRM_ERROR("CDCLK_FREQ is neither 450MHz nor 540MHz\n");
+		lcpll_val |= LCPLL_CD_FREQ_450MHZ;
+	}
+
+	I915_WRITE(LCPLL_CTL, lcpll_val);
+	POSTING_READ(LCPLL_CTL);
+	udelay(20);
+
+	intel_ddi_enable_dp_port(intel_dp);
+}
+
 void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
 {
 	struct drm_device *dev = encoder->dev;
@@ -762,3 +904,128 @@ void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
 	I915_WRITE(DDI_BUF_CTL(port),
 			temp);
 }
+
+void intel_ddi_enable_dp_pipe(struct drm_encoder *encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	struct drm_device *dev = encoder->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = encoder->crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int port = intel_dp->port;
+	int pipe = intel_crtc->pipe;
+	uint32_t func_val, msa_val;
+
+	I915_WRITE(PIPE_CLK_SEL(intel_crtc->pipe),
+		   PIPE_CLK_SEL_PORT(intel_dp->port));
+
+	func_val = PIPE_DDI_FUNC_ENABLE | PIPE_DDI_SELECT_PORT(port) |
+		   PIPE_DDI_MODE_SELECT_DP_SST;
+	msa_val = PIPE_MSA_SYNC_CLK;
+
+	switch (intel_crtc->bpp) {
+	case 18:
+		func_val |= PIPE_DDI_BPC_6;
+		msa_val |= PIPE_MSA_6_BPC;
+		break;
+	case 24:
+		func_val |= PIPE_DDI_BPC_8;
+		msa_val |= PIPE_MSA_8_BPC;
+		break;
+	case 30:
+		func_val |= PIPE_DDI_BPC_10;
+		msa_val |= PIPE_MSA_10_BPC;
+		break;
+	case 36:
+		func_val |= PIPE_DDI_BPC_12;
+		msa_val |= PIPE_MSA_12_BPC;
+		break;
+	default:
+		WARN(1, "BPP %d unsupported by pipe DDI function\n",
+		     intel_crtc->bpp);
+	}
+
+	if (crtc->mode.flags & DRM_MODE_FLAG_PVSYNC)
+		func_val |= PIPE_DDI_PVSYNC;
+	if (crtc->mode.flags & DRM_MODE_FLAG_PHSYNC)
+		func_val |= PIPE_DDI_PHSYNC;
+
+	switch (intel_dp->lane_count) {
+	case 1:
+		func_val |= PIPE_DDI_PORT_WIDTH_X1;
+		break;
+	case 2:
+		func_val |= PIPE_DDI_PORT_WIDTH_X2;
+		break;
+	case 4:
+		func_val |= PIPE_DDI_PORT_WIDTH_X4;
+		break;
+	default:
+		WARN(1, "Unsupported lane count %d\n", intel_dp->lane_count);
+	}
+
+	I915_WRITE(PIPE_MSA_MISC(pipe), msa_val);
+	I915_WRITE(DDI_FUNC_CTL(pipe), func_val);
+}
+
+static void intel_ddi_disable_dp_pipe(struct drm_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = encoder->dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	int pipe = intel_crtc->pipe;
+	uint32_t val;
+
+	val = I915_READ(DDI_FUNC_CTL(pipe));
+	val &= ~(PIPE_DDI_FUNC_ENABLE | PIPE_DDI_PORT_MASK);
+	val |= PIPE_DDI_PORT_NONE;
+	I915_WRITE(DDI_FUNC_CTL(pipe), val);
+
+	I915_WRITE(PIPE_CLK_SEL(pipe), PIPE_CLK_SEL_DISABLED);
+}
+
+
+/* If we disable PORT_CLK_SEL without disabling the pipe first, when we write to
+ * PIPECONF we'll freeze the machine. Make sure this won't happen. */
+static void intel_ddi_dp_disable_attached_pipes(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp->base.base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc;
+	int port = intel_dp->port;
+	int pipe;
+	uint32_t val;
+
+	for (pipe = 0; pipe < I915_MAX_PIPES; pipe++) {
+		val = I915_READ(DDI_FUNC_CTL(pipe));
+
+		if ((val & PIPE_DDI_PORT_MASK) == PIPE_DDI_SELECT_PORT(port)) {
+			DRM_DEBUG_KMS("Disabling pipe %c\n", pipe_name(pipe));
+			crtc = dev_priv->pipe_to_crtc_mapping[pipe];
+			ironlake_crtc_disable(crtc);
+
+			val = I915_READ(DDI_FUNC_CTL(pipe));
+			val &= ~(PIPE_DDI_FUNC_ENABLE | PIPE_DDI_PORT_MASK);
+			val |= PIPE_DDI_PORT_NONE;
+			I915_WRITE(DDI_FUNC_CTL(pipe), val);
+
+			I915_WRITE(PIPE_CLK_SEL(pipe), PIPE_CLK_SEL_DISABLED);
+		}
+	}
+}
+
+void intel_ddi_dp_link_down(struct intel_dp *intel_dp)
+{
+	struct drm_encoder *encoder = &intel_dp->base.base;
+
+	/* If there's no CRTC it means we may be initializing the driver and we
+	 * just don't know which pipes might be tied to this DP port, so we just
+	 * disable everything to prevent hanging the machine. */
+	if (!encoder->crtc) {
+		intel_ddi_dp_disable_attached_pipes(intel_dp);
+		return;
+	}
+
+	ironlake_crtc_disable(encoder->crtc);
+	intel_ddi_disable_dp_pipe(encoder);
+	intel_ddi_disable_dp_port(intel_dp);
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d6a3813..1475b18 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3229,7 +3229,7 @@ void intel_cpt_verify_modeset(struct drm_device *dev, int pipe)
 	}
 }
 
-static void ironlake_crtc_enable(struct drm_crtc *crtc)
+void ironlake_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3289,7 +3289,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_update_cursor(crtc, true);
 }
 
-static void ironlake_crtc_disable(struct drm_crtc *crtc)
+void ironlake_crtc_disable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a5aee26..2a8bf47 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -76,7 +76,7 @@ static bool is_cpu_edp(struct intel_dp *intel_dp)
 	return is_edp(intel_dp) && !is_pch_edp(intel_dp);
 }
 
-static struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
+struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder)
 {
 	return container_of(encoder, struct intel_dp, base.base);
 }
@@ -1282,6 +1282,9 @@ static void intel_dp_commit(struct drm_encoder *encoder)
 
 	intel_dp->dpms_mode = DRM_MODE_DPMS_ON;
 
+	if (IS_HASWELL(dev))
+		intel_ddi_enable_dp_pipe(encoder);
+
 	if (HAS_PCH_CPT(dev))
 		intel_cpt_verify_modeset(dev, intel_crtc->pipe);
 }
@@ -1884,6 +1887,11 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	uint32_t DP = intel_dp->DP;
 
+	if (IS_HASWELL(dev)) {
+		intel_ddi_dp_link_down(intel_dp);
+		return;
+	}
+
 	if ((I915_READ(intel_dp->output_reg) & DP_PORT_EN) == 0)
 		return;
 
@@ -2358,6 +2366,14 @@ static const struct drm_encoder_helper_funcs intel_dp_helper_funcs = {
 	.commit = intel_dp_commit,
 };
 
+static const struct drm_encoder_helper_funcs intel_dp_helper_funcs_hsw = {
+	.dpms = intel_dp_dpms,
+	.mode_fixup = intel_dp_mode_fixup,
+	.prepare = intel_dp_prepare,
+	.mode_set = intel_ddi_mode_set_dp,
+	.commit = intel_dp_commit,
+};
+
 static const struct drm_connector_funcs intel_dp_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
 	.detect = intel_dp_detect,
@@ -2501,7 +2517,13 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 
 	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
 			 DRM_MODE_ENCODER_TMDS);
-	drm_encoder_helper_add(&intel_encoder->base, &intel_dp_helper_funcs);
+
+	if (IS_HASWELL(dev))
+		drm_encoder_helper_add(&intel_encoder->base,
+				       &intel_dp_helper_funcs_hsw);
+	else
+		drm_encoder_helper_add(&intel_encoder->base,
+				       &intel_dp_helper_funcs);
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 	drm_sysfs_connector_add(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1145403..623d16c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -387,6 +387,7 @@ extern void intel_mark_busy(struct drm_device *dev,
 			    struct drm_i915_gem_object *obj);
 extern bool intel_lvds_init(struct drm_device *dev);
 extern void intel_dp_init(struct drm_device *dev, int dp_reg);
+extern struct intel_dp *enc_to_intel_dp(struct drm_encoder *encoder);
 void
 intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 		 struct drm_display_mode *adjusted_mode);
@@ -438,6 +439,8 @@ int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data,
 				struct drm_file *file_priv);
 extern void intel_wait_for_vblank(struct drm_device *dev, int pipe);
 extern void intel_wait_for_pipe_off(struct drm_device *dev, int pipe);
+extern void ironlake_crtc_enable(struct drm_crtc *crtc);
+extern void ironlake_crtc_disable(struct drm_crtc *crtc);
 
 struct intel_load_detect_pipe {
 	struct drm_framebuffer *release_fb;
@@ -531,5 +534,10 @@ extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
 extern void intel_ddi_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode);
+extern void intel_ddi_mode_set_dp(struct drm_encoder *encoder,
+				  struct drm_display_mode *mode,
+				  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);
 
 #endif /* __INTEL_DRV_H__ */
-- 
1.7.10.2

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

* [PATCH 06/09] drm/i915: fix Haswell M/N registers
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (3 preceding siblings ...)
  2012-06-29 19:03 ` [PATCH 05/09] drm/i915: add basic Haswell DP enablement Paulo Zanoni
@ 2012-06-29 19:03 ` Paulo Zanoni
  2012-06-29 19:03 ` [PATCH 07/09] drm/i915: implement Haswell DP link train Paulo Zanoni
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

We have to write the correct values inside intel_dp_set_m_n and then
we have to prevent ironlake_crtc_mode_set from overwriting the correct
values with wrong ones.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   10 ++++++----
 drivers/gpu/drm/i915/intel_dp.c      |    9 ++++++++-
 2 files changed, 14 insertions(+), 5 deletions(-)

When exactly the M/N values set inside crtc_mode_set should be used? It's
certainly not for DP...

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1475b18..d2542bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5018,10 +5018,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	I915_WRITE(PIPESRC(pipe),
 		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
 
-	I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
-	I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
-	I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
-	I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
+	if (!(IS_HASWELL(dev) && is_dp)) {
+		I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
+		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
+		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
+		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
+	}
 
 	if (is_cpu_edp)
 		ironlake_set_pll_edp(crtc, adjusted_mode->clock);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2a8bf47..8c66c0c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -795,7 +795,14 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	intel_dp_compute_m_n(intel_crtc->bpp, lane_count,
 			     mode->clock, adjusted_mode->clock, &m_n);
 
-	if (HAS_PCH_SPLIT(dev)) {
+	if (IS_HASWELL(dev)) {
+		I915_WRITE(PIPE_DATA_M1(pipe),
+			   ((m_n.tu - 1) << PIPE_GMCH_DATA_M_TU_SIZE_SHIFT) |
+			   m_n.gmch_m);
+		I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
+		I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
+		I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
+	} else if (HAS_PCH_SPLIT(dev)) {
 		I915_WRITE(TRANSDATA_M1(pipe),
 			   ((m_n.tu - 1) << PIPE_GMCH_DATA_M_TU_SIZE_SHIFT) |
 			   m_n.gmch_m);
-- 
1.7.10.2

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

* [PATCH 07/09] drm/i915: implement Haswell DP link train
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (4 preceding siblings ...)
  2012-06-29 19:03 ` [PATCH 06/09] drm/i915: fix Haswell M/N registers Paulo Zanoni
@ 2012-06-29 19:03 ` Paulo Zanoni
  2012-06-29 19:03 ` [PATCH 08/09] drm/i915: fix DP AUX register definitions on Haswell Paulo Zanoni
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

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

* [PATCH 08/09] drm/i915: fix DP AUX register definitions on Haswell
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (5 preceding siblings ...)
  2012-06-29 19:03 ` [PATCH 07/09] drm/i915: implement Haswell DP link train Paulo Zanoni
@ 2012-06-29 19:03 ` Paulo Zanoni
  2012-06-29 19:03 ` [PATCH 09/09] drm/i915: init DP instead of HDMI on port B Paulo Zanoni
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

The old rule that the AUX registers are just an offset (+4 and +10)
from output_reg is not true anymore, since output_reg in on the CPU
and some AUX regs are on the PCH.

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

The previous patch series didn't need this patch because they were calling
intel_dp_init with PCH_DP_B as the second argument. IMHO this is ugly because
PCH_DP_B does not exist on Haswell. This value will become intel_dp->output_reg
and then eventually some functions will even write to it.

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f28f2b2..475ada8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2493,6 +2493,14 @@
 #define DPD_AUX_CH_DATA4		0x64320
 #define DPD_AUX_CH_DATA5		0x64324
 
+#define PCH_DPB_AUX_CH_CTL		0xe4110
+#define PCH_DPC_AUX_CH_CTL		0xe4210
+#define PCH_DPD_AUX_CH_CTL		0xe4310
+
+#define PCH_DPB_AUX_CH_DATA		0xe4114
+#define PCH_DPC_AUX_CH_DATA		0xe4214
+#define PCH_DPD_AUX_CH_DATA		0xe4314
+
 #define   DP_AUX_CH_CTL_SEND_BUSY	    (1 << 31)
 #define   DP_AUX_CH_CTL_DONE		    (1 << 30)
 #define   DP_AUX_CH_CTL_INTERRUPT	    (1 << 29)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 287bef9..22dae6d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -356,6 +356,27 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	uint32_t aux_clock_divider;
 	int try, precharge;
 
+	if (IS_HASWELL(dev)) {
+		switch (intel_dp->port) {
+		case PORT_A:
+			ch_ctl = DPA_AUX_CH_CTL;
+			ch_data = DPA_AUX_CH_DATA1;
+			break;
+		case PORT_B:
+			ch_ctl = PCH_DPB_AUX_CH_CTL;
+			ch_data = PCH_DPB_AUX_CH_DATA;
+			break;
+		case PORT_C:
+			ch_ctl = PCH_DPC_AUX_CH_CTL;
+			ch_data = PCH_DPC_AUX_CH_DATA;
+			break;
+		case PORT_D:
+			ch_ctl = PCH_DPD_AUX_CH_CTL;
+			ch_data = PCH_DPD_AUX_CH_DATA;
+			break;
+		}
+	}
+
 	intel_dp_check_edp(intel_dp);
 	/* The clock divider is based off the hrawclk,
 	 * and would like to run at 2MHz. So, take the
-- 
1.7.10.2

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

* [PATCH 09/09] drm/i915: init DP instead of HDMI on port B
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (6 preceding siblings ...)
  2012-06-29 19:03 ` [PATCH 08/09] drm/i915: fix DP AUX register definitions on Haswell Paulo Zanoni
@ 2012-06-29 19:03 ` Paulo Zanoni
  2012-07-12 14:52 ` [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Daniel Vetter
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-06-29 19:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

This is needed if you want to test DisplayPort on Haswell, but this is
not the correct solution.

Not-signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

---
 drivers/gpu/drm/i915/intel_ddi.c |    2 ++
 1 file changed, 2 insertions(+)

This patch is equivalent to this one:
[PATCH 06/21] drm/i915: Hook DP init in ddi module

Just as I don't expect 06/21 be committed, I don't expect this one to
be committed. IMHO these patches should be used just for testing. You
can't test DP on Haswell if you don't have this patch.

What we really need to do is to either create a DP and an HDMI
connector for each port (and then try to properly detect what is
connected to the port, and this is not as easy on Haswell as it was on
previous gens) or we create a new "Digital" connector and use it
instead of the DP and HDMI ones...

The main diference between this patch and 06/21 is that this patch
calls intel_dp_init with DDI_BUF_CTL_B as an argument instead of using
PCH_DP_B. The intel_dp->output_reg will contain the value we pass in
this argument. PCH_DP_B does not exist on Haswell, so code using
intel_dp->output_reg will be writing to an inexistent register. If we
pass DDI_BUF_CTL_B to it, intel_dp->output_reg will point to the
actual DP register, so code writing to output_reg will be correct.

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 082b923..a6b84e5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -249,6 +249,8 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 		break;
 	/* Assume that the  ports B, C and D are working in HDMI mode for now */
 	case PORT_B:
+		intel_dp_init(dev, DDI_BUF_CTL_B);
+		break;
 	case PORT_C:
 	case PORT_D:
 		intel_hdmi_init(dev, DDI_BUF_CTL(port));
-- 
1.7.10.2

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

* Re: [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (7 preceding siblings ...)
  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 ` 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
  10 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-07-12 14:52 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jun 29, 2012 at 04:03:33PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> IMHO, that code really belongs to intel_dp_set_link_train. The caller
> functions are also big, so now they'll be a little bit easier to read.

I've gotten rather confused with this patch until I've figure out what
you're actually doing. I think it'd be nice to explain what you mean by
"common code" exactly. What about adding:

- Change set_link_train to also do the right thing when disabling link
  training so that it can be used in the complete_link_train function.
- Don adjust the DP_CTL value in the caller, instead select the right
  bitmask for the desired training pattern according to dp_train_pat.

Yours, Daniel

> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   77 ++++++++++++++++++++++++---------------
>  1 file changed, 47 insertions(+), 30 deletions(-)
> 
> We'll add even more code to intel_dp_set_link_train on Haswell.
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 76a7080..e315f73 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1672,6 +1672,41 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
>  
> +	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;
> +			break;
> +		case DP_TRAINING_PATTERN_1:
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_1_CPT;
> +			break;
> +		case DP_TRAINING_PATTERN_2:
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
> +			break;
> +		case DP_TRAINING_PATTERN_3:
> +			DRM_ERROR("DP training pattern 3 not supported\n");
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
> +			break;
> +		}
> +
> +	} else {
> +		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
> +		case DP_TRAINING_PATTERN_DISABLE:
> +			dp_reg_value |= DP_LINK_TRAIN_OFF;
> +			break;
> +		case DP_TRAINING_PATTERN_1:
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_1;
> +			break;
> +		case DP_TRAINING_PATTERN_2:
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
> +			break;
> +		case DP_TRAINING_PATTERN_3:
> +			DRM_ERROR("DP training pattern 3 not supported\n");
> +			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
> +			break;
> +		}
> +	}
> +
>  	I915_WRITE(intel_dp->output_reg, dp_reg_value);
>  	POSTING_READ(intel_dp->output_reg);
>  
> @@ -1679,12 +1714,15 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
>  				    DP_TRAINING_PATTERN_SET,
>  				    dp_train_pat);
>  
> -	ret = intel_dp_aux_native_write(intel_dp,
> -					DP_TRAINING_LANE0_SET,
> -					intel_dp->train_set,
> -					intel_dp->lane_count);
> -	if (ret != intel_dp->lane_count)
> -		return false;
> +	if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) !=
> +	    DP_TRAINING_PATTERN_DISABLE) {
> +		ret = intel_dp_aux_native_write(intel_dp,
> +						DP_TRAINING_LANE0_SET,
> +						intel_dp->train_set,
> +						intel_dp->lane_count);
> +		if (ret != intel_dp->lane_count)
> +			return false;
> +	}
>  
>  	return true;
>  }
> @@ -1700,7 +1738,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  	uint8_t voltage;
>  	bool clock_recovery = false;
>  	int voltage_tries, loop_tries;
> -	u32 reg;
>  	uint32_t DP = intel_dp->DP;
>  
>  	/*
> @@ -1748,12 +1785,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
>  		}
>  
> -		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
> -			reg = DP | DP_LINK_TRAIN_PAT_1_CPT;
> -		else
> -			reg = DP | DP_LINK_TRAIN_PAT_1;
> -
> -		if (!intel_dp_set_link_train(intel_dp, reg,
> +		if (!intel_dp_set_link_train(intel_dp, DP,
>  					     DP_TRAINING_PATTERN_1 |
>  					     DP_LINK_SCRAMBLING_DISABLE))
>  			break;
> @@ -1808,10 +1840,8 @@ static void
>  intel_dp_complete_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;
>  	bool channel_eq = false;
>  	int tries, cr_tries;
> -	u32 reg;
>  	uint32_t DP = intel_dp->DP;
>  
>  	/* channel equalization */
> @@ -1840,13 +1870,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
>  		}
>  
> -		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
> -			reg = DP | DP_LINK_TRAIN_PAT_2_CPT;
> -		else
> -			reg = DP | DP_LINK_TRAIN_PAT_2;
> -
>  		/* channel eq pattern */
> -		if (!intel_dp_set_link_train(intel_dp, reg,
> +		if (!intel_dp_set_link_train(intel_dp, DP,
>  					     DP_TRAINING_PATTERN_2 |
>  					     DP_LINK_SCRAMBLING_DISABLE))
>  			break;
> @@ -1881,15 +1906,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  		++tries;
>  	}
>  
> -	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
> -		reg = DP | DP_LINK_TRAIN_OFF_CPT;
> -	else
> -		reg = DP | DP_LINK_TRAIN_OFF;
> -
> -	I915_WRITE(intel_dp->output_reg, reg);
> -	POSTING_READ(intel_dp->output_reg);
> -	intel_dp_aux_native_write_1(intel_dp,
> -				    DP_TRAINING_PATTERN_SET, DP_TRAINING_PATTERN_DISABLE);
> +	intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE);
>  }
>  
>  static void
> -- 
> 1.7.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* Re: [PATCH 02/09] drm/i915: try to train DP even harder
  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
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-07-12 14:58 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jun 29, 2012 at 04:03:34PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> While debugging Haswell link train failures I observed that we never
> try the maximum voltage configuration more than once consecutively. We
> start the training, the monitor keeps telling us to increase the
> voltage, then when we reach the maximum we just go back to the start
> (because of the "memset" above "voltage_tries = 0"). When we reach
> this point, we keep alternating between the maximum and the minimum
> voltages until we give up.
> 
> The DP spec suggests that we should try the same voltage 5 times
> before giving up. This patch makes us try the maximum voltage at
> least 5 times before going back to the minimum voltages.
> 
> This patch does not fix any particular bug I'm aware of.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Gosh, this is the most convoluted loop flattening code I've ever seen.
Patch queued for -next, thanks.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* Re: [PATCH 04/09] drm/i915: Add "port" field to struct intel_dp
  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
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-07-12 15:04 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jun 29, 2012 at 04:03:36PM -0300, Paulo Zanoni wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Useful for Haswell, it has a lot of DP registers that have one
> instance per port.
> 
> Changes from Paulo:
> - Completely change the commit message
> - Rename "ddi_port" to "port"
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |   10 +++++++---
>  drivers/gpu/drm/i915/intel_drv.h |    1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> I believe "port" is better than "ddi_port" since the "port" variable can be
> used in the previous gens too...
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index be5c47e..a5aee26 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2477,12 +2477,16 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  
>  	connector->polled = DRM_CONNECTOR_POLL_HPD;
>  
> -	if (output_reg == DP_B || output_reg == PCH_DP_B)
> +	if (output_reg == DP_B || output_reg == PCH_DP_B) {
>  		intel_encoder->clone_mask = (1 << INTEL_DP_B_CLONE_BIT);
> -	else if (output_reg == DP_C || output_reg == PCH_DP_C)
> +		intel_dp->port = PORT_B;
> +	} else if (output_reg == DP_C || output_reg == PCH_DP_C) {
>  		intel_encoder->clone_mask = (1 << INTEL_DP_C_CLONE_BIT);
> -	else if (output_reg == DP_D || output_reg == PCH_DP_D)
> +		intel_dp->port = PORT_C;
> +	} else if (output_reg == DP_D || output_reg == PCH_DP_D) {
>  		intel_encoder->clone_mask = (1 << INTEL_DP_D_CLONE_BIT);
> +		intel_dp->port = PORT_D;
> +	}

Imo we should pass in the port id into intel_dp_init, like I've done for
intel_hdmi.c. That way we0d avoid this rather ugly loop. Also, this patch
will conflict a bit with the cloning simplification from my modeset rework
branch (patch 19). Can I volunteer you to review patches 19+20 from that
series first, and then rebase this on top of these other patches?
-Daniel

>  
>  	if (is_edp(intel_dp)) {
>  		intel_encoder->clone_mask = (1 << INTEL_EDP_CLONE_BIT);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 48a0fcb..1145403 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -316,6 +316,7 @@ struct intel_dp {
>  	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
>  	bool has_audio;
>  	enum hdmi_force_audio force_audio;
> +	int port;
>  	uint32_t color_range;
>  	int dpms_mode;
>  	uint8_t link_bw;
> -- 
> 1.7.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* Re: [PATCH 03/09] drm/i915: Move DP structs to shared location
  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
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-07-12 15:13 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Fri, Jun 29, 2012 at 04:03:35PM -0300, Paulo Zanoni wrote:
> From: Shobhit Kumar <shobhit.kumar@intel.com>
> 
> Move the DP structure to shared location so that it can be used from
> within the ddi module.
> 
> Changes from Paulo:
> - Move less code to intel_drv.h
> - Remove #include statement
> - Replace a tab with a space in train_set
> 
> Signed-off-by: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* [PATCH 1/2] drm/i915: move common code to intel_dp_set_link_train
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (8 preceding siblings ...)
  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 ` Paulo Zanoni
  2012-07-17 20:53 ` [PATCH 2/2] drm/i915: add port field to struct intel_dp and use it Paulo Zanoni
  10 siblings, 0 replies; 16+ messages in thread
From: Paulo Zanoni @ 2012-07-17 19:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

We have some common code that we always run before calling
intel_dp_set_link_train. This common code sets the correct training
patterns to the DP variable. If we add more calls to
intel_dp_set_link_train, we'll also have to duplicate this common
code. So instead of repeating this code whenever we call
intel_dp_set_link_train, we move the code to inside the function: now
we check which training pattern we're going to set and then we set the
DP register according to it.

One of the side-effects of this change is that now we never forget to
mask the training pattern bits before changing them. It looks like
this was working before because we were first masking the bits, then
writing 00, 01 and then 11.

This patch also enables us to use the intel_dp_set_link_train function
when disabling link training: in this case we need to avoid writing
the DP_TRAINING_LANE*_SET AUX commands.

As a bonus, the big intel_dp_{start,complete}_link_train functions
will get smaller and a little bit easier to read.

Version 2 changes:
 - Rewrite commit message.
 - Also clear the training pattern bits before changing them.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5c40cce..1255c42 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1635,6 +1635,45 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
+	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp))) {
+		dp_reg_value &= ~DP_LINK_TRAIN_MASK_CPT;
+
+		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
+		case DP_TRAINING_PATTERN_DISABLE:
+			dp_reg_value |= DP_LINK_TRAIN_OFF_CPT;
+			break;
+		case DP_TRAINING_PATTERN_1:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_1_CPT;
+			break;
+		case DP_TRAINING_PATTERN_2:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
+			break;
+		case DP_TRAINING_PATTERN_3:
+			DRM_ERROR("DP training pattern 3 not supported\n");
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2_CPT;
+			break;
+		}
+
+	} else {
+		dp_reg_value &= ~DP_LINK_TRAIN_MASK;
+
+		switch (dp_train_pat & DP_TRAINING_PATTERN_MASK) {
+		case DP_TRAINING_PATTERN_DISABLE:
+			dp_reg_value |= DP_LINK_TRAIN_OFF;
+			break;
+		case DP_TRAINING_PATTERN_1:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_1;
+			break;
+		case DP_TRAINING_PATTERN_2:
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
+			break;
+		case DP_TRAINING_PATTERN_3:
+			DRM_ERROR("DP training pattern 3 not supported\n");
+			dp_reg_value |= DP_LINK_TRAIN_PAT_2;
+			break;
+		}
+	}
+
 	I915_WRITE(intel_dp->output_reg, dp_reg_value);
 	POSTING_READ(intel_dp->output_reg);
 
@@ -1642,12 +1681,15 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 				    DP_TRAINING_PATTERN_SET,
 				    dp_train_pat);
 
-	ret = intel_dp_aux_native_write(intel_dp,
-					DP_TRAINING_LANE0_SET,
-					intel_dp->train_set,
-					intel_dp->lane_count);
-	if (ret != intel_dp->lane_count)
-		return false;
+	if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) !=
+	    DP_TRAINING_PATTERN_DISABLE) {
+		ret = intel_dp_aux_native_write(intel_dp,
+						DP_TRAINING_LANE0_SET,
+						intel_dp->train_set,
+						intel_dp->lane_count);
+		if (ret != intel_dp->lane_count)
+			return false;
+	}
 
 	return true;
 }
@@ -1663,7 +1705,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	uint8_t voltage;
 	bool clock_recovery = false;
 	int voltage_tries, loop_tries;
-	u32 reg;
 	uint32_t DP = intel_dp->DP;
 
 	/*
@@ -1684,10 +1725,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 
 	DP |= DP_PORT_EN;
 
-	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-		DP &= ~DP_LINK_TRAIN_MASK_CPT;
-	else
-		DP &= ~DP_LINK_TRAIN_MASK;
 	memset(intel_dp->train_set, 0, 4);
 	voltage = 0xff;
 	voltage_tries = 0;
@@ -1711,12 +1748,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
 		}
 
-		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-			reg = DP | DP_LINK_TRAIN_PAT_1_CPT;
-		else
-			reg = DP | DP_LINK_TRAIN_PAT_1;
-
-		if (!intel_dp_set_link_train(intel_dp, reg,
+		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_1 |
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
@@ -1771,10 +1803,8 @@ static void
 intel_dp_complete_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;
 	bool channel_eq = false;
 	int tries, cr_tries;
-	u32 reg;
 	uint32_t DP = intel_dp->DP;
 
 	/* channel equalization */
@@ -1803,13 +1833,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
 		}
 
-		if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-			reg = DP | DP_LINK_TRAIN_PAT_2_CPT;
-		else
-			reg = DP | DP_LINK_TRAIN_PAT_2;
-
 		/* channel eq pattern */
-		if (!intel_dp_set_link_train(intel_dp, reg,
+		if (!intel_dp_set_link_train(intel_dp, DP,
 					     DP_TRAINING_PATTERN_2 |
 					     DP_LINK_SCRAMBLING_DISABLE))
 			break;
@@ -1844,15 +1869,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 		++tries;
 	}
 
-	if (HAS_PCH_CPT(dev) && (IS_GEN7(dev) || !is_cpu_edp(intel_dp)))
-		reg = DP | DP_LINK_TRAIN_OFF_CPT;
-	else
-		reg = DP | DP_LINK_TRAIN_OFF;
-
-	I915_WRITE(intel_dp->output_reg, reg);
-	POSTING_READ(intel_dp->output_reg);
-	intel_dp_aux_native_write_1(intel_dp,
-				    DP_TRAINING_PATTERN_SET, DP_TRAINING_PATTERN_DISABLE);
+	intel_dp_set_link_train(intel_dp, DP, DP_TRAINING_PATTERN_DISABLE);
 }
 
 static void
-- 
1.7.10.4

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

* [PATCH 2/2] drm/i915: add port field to struct intel_dp and use it
  2012-06-29 19:03 [PATCH 01/09] drm/i915: move common code to intel_dp_set_link_train Paulo Zanoni
                   ` (9 preceding siblings ...)
  2012-07-17 19:55 ` [PATCH 1/2] " Paulo Zanoni
@ 2012-07-17 20:53 ` Paulo Zanoni
  2012-07-18 10:42   ` Daniel Vetter
  10 siblings, 1 reply; 16+ messages in thread
From: Paulo Zanoni @ 2012-07-17 20:53 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

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

This will be needed for Haswell, but already has its uses here.

This patch started as a small patch written patch by Shobhit Kumar,
but it has changed so much that none of its original lines remain.

Credits-to: Shobhit Kumar <shobhit.kumar@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   20 ++++++++--------
 drivers/gpu/drm/i915/intel_dp.c      |   44 ++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |    4 +++-
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 80e05ce..7707a4b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6764,10 +6764,10 @@ static void intel_setup_outputs(struct drm_device *dev)
 		dpd_is_edp = intel_dpd_is_edp(dev);
 
 		if (has_edp_a(dev))
-			intel_dp_init(dev, DP_A);
+			intel_dp_init(dev, DP_A, PORT_A);
 
 		if (dpd_is_edp && (I915_READ(PCH_DP_D) & DP_DETECTED))
-			intel_dp_init(dev, PCH_DP_D);
+			intel_dp_init(dev, PCH_DP_D, PORT_D);
 	}
 
 	intel_crt_init(dev);
@@ -6800,7 +6800,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 			if (!found)
 				intel_hdmi_init(dev, HDMIB, PORT_B);
 			if (!found && (I915_READ(PCH_DP_B) & DP_DETECTED))
-				intel_dp_init(dev, PCH_DP_B);
+				intel_dp_init(dev, PCH_DP_B, PORT_B);
 		}
 
 		if (I915_READ(HDMIC) & PORT_DETECTED)
@@ -6810,10 +6810,10 @@ static void intel_setup_outputs(struct drm_device *dev)
 			intel_hdmi_init(dev, HDMID, PORT_D);
 
 		if (I915_READ(PCH_DP_C) & DP_DETECTED)
-			intel_dp_init(dev, PCH_DP_C);
+			intel_dp_init(dev, PCH_DP_C, PORT_C);
 
 		if (!dpd_is_edp && (I915_READ(PCH_DP_D) & DP_DETECTED))
-			intel_dp_init(dev, PCH_DP_D);
+			intel_dp_init(dev, PCH_DP_D, PORT_D);
 	} else if (IS_VALLEYVIEW(dev)) {
 		int found;
 
@@ -6823,7 +6823,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 			if (!found)
 				intel_hdmi_init(dev, SDVOB, PORT_B);
 			if (!found && (I915_READ(DP_B) & DP_DETECTED))
-				intel_dp_init(dev, DP_B);
+				intel_dp_init(dev, DP_B, PORT_B);
 		}
 
 		if (I915_READ(SDVOC) & PORT_DETECTED)
@@ -6831,7 +6831,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 
 		/* Shares lanes with HDMI on SDVOC */
 		if (I915_READ(DP_C) & DP_DETECTED)
-			intel_dp_init(dev, DP_C);
+			intel_dp_init(dev, DP_C, PORT_C);
 	} else if (SUPPORTS_DIGITAL_OUTPUTS(dev)) {
 		bool found = false;
 
@@ -6845,7 +6845,7 @@ static void intel_setup_outputs(struct drm_device *dev)
 
 			if (!found && SUPPORTS_INTEGRATED_DP(dev)) {
 				DRM_DEBUG_KMS("probing DP_B\n");
-				intel_dp_init(dev, DP_B);
+				intel_dp_init(dev, DP_B, PORT_B);
 			}
 		}
 
@@ -6864,14 +6864,14 @@ static void intel_setup_outputs(struct drm_device *dev)
 			}
 			if (SUPPORTS_INTEGRATED_DP(dev)) {
 				DRM_DEBUG_KMS("probing DP_C\n");
-				intel_dp_init(dev, DP_C);
+				intel_dp_init(dev, DP_C, PORT_C);
 			}
 		}
 
 		if (SUPPORTS_INTEGRATED_DP(dev) &&
 		    (I915_READ(DP_D) & DP_DETECTED)) {
 			DRM_DEBUG_KMS("probing DP_D\n");
-			intel_dp_init(dev, DP_D);
+			intel_dp_init(dev, DP_D, PORT_D);
 		}
 	} else if (IS_GEN2(dev))
 		intel_dvo_init(dev);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1255c42..ac9b8d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2425,7 +2425,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 }
 
 void
-intel_dp_init(struct drm_device *dev, int output_reg)
+intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_connector *connector;
@@ -2440,6 +2440,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 		return;
 
 	intel_dp->output_reg = output_reg;
+	intel_dp->port = port;
 	intel_dp->dpms_mode = -1;
 
 	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
@@ -2485,28 +2486,25 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 	drm_sysfs_connector_add(connector);
 
 	/* Set up the DDC bus. */
-	switch (output_reg) {
-		case DP_A:
-			name = "DPDDC-A";
-			break;
-		case DP_B:
-		case PCH_DP_B:
-			dev_priv->hotplug_supported_mask |=
-				DPB_HOTPLUG_INT_STATUS;
-			name = "DPDDC-B";
-			break;
-		case DP_C:
-		case PCH_DP_C:
-			dev_priv->hotplug_supported_mask |=
-				DPC_HOTPLUG_INT_STATUS;
-			name = "DPDDC-C";
-			break;
-		case DP_D:
-		case PCH_DP_D:
-			dev_priv->hotplug_supported_mask |=
-				DPD_HOTPLUG_INT_STATUS;
-			name = "DPDDC-D";
-			break;
+	switch (port) {
+	case PORT_A:
+		name = "DPDDC-A";
+		break;
+	case PORT_B:
+		dev_priv->hotplug_supported_mask |= DPB_HOTPLUG_INT_STATUS;
+		name = "DPDDC-B";
+		break;
+	case PORT_C:
+		dev_priv->hotplug_supported_mask |= DPC_HOTPLUG_INT_STATUS;
+		name = "DPDDC-C";
+		break;
+	case PORT_D:
+		dev_priv->hotplug_supported_mask |= DPD_HOTPLUG_INT_STATUS;
+		name = "DPDDC-D";
+		break;
+	default:
+		WARN(1, "Invalid port %c\n", port_name(port));
+		break;
 	}
 
 	intel_dp_i2c_init(intel_dp, intel_connector, name);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2000c7c..d6dfc44 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -307,6 +307,7 @@ struct intel_dp {
 	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
+	enum port port;
 	uint32_t color_range;
 	int dpms_mode;
 	uint8_t link_bw;
@@ -376,7 +377,8 @@ extern void intel_tv_init(struct drm_device *dev);
 extern void intel_mark_busy(struct drm_device *dev,
 			    struct drm_i915_gem_object *obj);
 extern bool intel_lvds_init(struct drm_device *dev);
-extern void intel_dp_init(struct drm_device *dev, int dp_reg);
+extern void intel_dp_init(struct drm_device *dev, int output_reg,
+			  enum port port);
 void
 intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
 		 struct drm_display_mode *adjusted_mode);
-- 
1.7.10.4

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

* Re: [PATCH 2/2] drm/i915: add port field to struct intel_dp and use it
  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
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2012-07-18 10:42 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni

On Tue, Jul 17, 2012 at 05:53:45PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> This will be needed for Haswell, but already has its uses here.
> 
> This patch started as a small patch written patch by Shobhit Kumar,
> but it has changed so much that none of its original lines remain.
> 
> Credits-to: Shobhit Kumar <shobhit.kumar@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Both patches queued for -next, thanks.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

end of thread, other threads:[~2012-07-18 10:42 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 ` [PATCH 07/09] drm/i915: implement Haswell DP link train Paulo Zanoni
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

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.