[PATCH 1/5] drm/i915: check for the PCH when setting pch_transcoder

[PATCH 1/5] drm/i915: check for the PCH when setting pch_transcoder

[Paulo Zanoni]

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

Don't check the CPU, it doesn't have any PCH transcoder.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 093a163..8626abd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1763,7 +1763,7 @@ static void intel_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe,
 	int reg;
 	u32 val;
 
-	if (IS_HASWELL(dev_priv->dev))
+	if (HAS_PCH_LPT(dev_priv->dev))
 		pch_transcoder = TRANSCODER_A;
 	else
 		pch_transcoder = pipe;
-- 
1.7.11.7

[PATCH 2/5] drm/i915: remove leftover display.update_wm assignment

[Paulo Zanoni]

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

This was moved to intel_init_pm.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8626abd..0517fc2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8422,8 +8422,7 @@ static void intel_init_display(struct drm_device *dev)
 		} else if (IS_HASWELL(dev)) {
 			dev_priv->display.fdi_link_train = hsw_fdi_link_train;
 			dev_priv->display.write_eld = haswell_write_eld;
-		} else
-			dev_priv->display.update_wm = NULL;
+		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
 	}
-- 
1.7.11.7

[PATCH 3/5] drm/i915: DDI naming standards cleanup

[Paulo Zanoni]

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

A big part of the Haswell display code is called intel_ddi_something
while a small part of it is called haswell_something. Being consistent
with the naming standards is a nice thing, so IMHO we should only use
one of the naming standards for everything.

So instead of converting everything to haswell_something I converted
to ddi_something, which seems more appropriate IMHO. Now our code
looks a little bit more consistent.

The next step is to move everything to intel_ddi.c.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c      |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c     |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_dp.c      | 20 +++++++++---------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c    | 10 ++++-----
 drivers/gpu/drm/i915/intel_pm.c      |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c  |  4 ++--
 8 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9363066..3c1c3cb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1588,7 +1588,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 
 	mutex_init(&dev_priv->rps.hw_lock);
 
-	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
+	if (IS_IVYBRIDGE(dev) || HAS_DDI(dev))
 		dev_priv->num_pipe = 3;
 	else if (IS_MOBILE(dev) || !IS_GEN2(dev))
 		dev_priv->num_pipe = 2;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f02b3fe..d8d83f9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -150,7 +150,7 @@ static const long hsw_ddi_buf_ctl_values[] = {
  * DDI A (which is used for eDP)
  */
 
-void hsw_fdi_link_train(struct drm_crtc *crtc)
+void intel_ddi_fdi_link_train(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_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0517fc2..7d88d7e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2152,7 +2152,7 @@ static int ironlake_update_plane(struct drm_crtc *crtc,
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	I915_MODIFY_DISPBASE(DSPSURF(plane),
 			     obj->gtt_offset + intel_crtc->dspaddr_offset);
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
 	} else {
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
@@ -2948,7 +2948,7 @@ static bool ironlake_crtc_driving_pch(struct drm_crtc *crtc)
 	return true;
 }
 
-static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
+static bool ddi_crtc_driving_pch(struct drm_crtc *crtc)
 {
 	return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
 }
@@ -3375,7 +3375,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 }
 
-static void haswell_crtc_enable(struct drm_crtc *crtc)
+static void ddi_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3393,7 +3393,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc->active = true;
 	intel_update_watermarks(dev);
 
-	is_pch_port = haswell_crtc_driving_pch(crtc);
+	is_pch_port = ddi_crtc_driving_pch(crtc);
 
 	if (is_pch_port)
 		dev_priv->display.fdi_link_train(crtc);
@@ -3532,7 +3532,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	mutex_unlock(&dev->struct_mutex);
 }
 
-static void haswell_crtc_disable(struct drm_crtc *crtc)
+static void ddi_crtc_disable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3546,7 +3546,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	is_pch_port = haswell_crtc_driving_pch(crtc);
+	is_pch_port = ddi_crtc_driving_pch(crtc);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->disable(encoder);
@@ -3593,7 +3593,7 @@ static void ironlake_crtc_off(struct drm_crtc *crtc)
 	intel_put_pch_pll(intel_crtc);
 }
 
-static void haswell_crtc_off(struct drm_crtc *crtc)
+static void ddi_crtc_off(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
@@ -4953,7 +4953,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
 	POSTING_READ(PIPECONF(pipe));
 }
 
-static void haswell_set_pipeconf(struct drm_crtc *crtc,
+static void ddi_set_pipeconf(struct drm_crtc *crtc,
 				 struct drm_display_mode *adjusted_mode,
 				 bool dither)
 {
@@ -5449,7 +5449,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	return fdi_config_ok ? ret : -EINVAL;
 }
 
-static int haswell_crtc_mode_set(struct drm_crtc *crtc,
+static int ddi_crtc_mode_set(struct drm_crtc *crtc,
 				 struct drm_display_mode *mode,
 				 struct drm_display_mode *adjusted_mode,
 				 int x, int y,
@@ -5645,7 +5645,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 		if (is_cpu_edp)
 			ironlake_set_pll_edp(crtc, adjusted_mode->clock);
 
-	haswell_set_pipeconf(crtc, adjusted_mode, dither);
+	ddi_set_pipeconf(crtc, adjusted_mode, dither);
 
 	/* Set up the display plane register */
 	I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
@@ -5764,8 +5764,8 @@ static void g4x_write_eld(struct drm_connector *connector,
 	I915_WRITE(G4X_AUD_CNTL_ST, i);
 }
 
-static void haswell_write_eld(struct drm_connector *connector,
-				     struct drm_crtc *crtc)
+static void ddi_write_eld(struct drm_connector *connector,
+			  struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	uint8_t *eld = connector->eld;
@@ -6101,7 +6101,7 @@ static void intel_crtc_update_cursor(struct drm_crtc *crtc,
 	if (!visible && !intel_crtc->cursor_visible)
 		return;
 
-	if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
+	if (IS_IVYBRIDGE(dev) || HAS_DDI(dev)) {
 		I915_WRITE(CURPOS_IVB(pipe), pos);
 		ivb_update_cursor(crtc, base);
 	} else {
@@ -8361,10 +8361,10 @@ static void intel_init_display(struct drm_device *dev)
 
 	/* We always want a DPMS function */
 	if (HAS_DDI(dev)) {
-		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
-		dev_priv->display.crtc_enable = haswell_crtc_enable;
-		dev_priv->display.crtc_disable = haswell_crtc_disable;
-		dev_priv->display.off = haswell_crtc_off;
+		dev_priv->display.crtc_mode_set = ddi_crtc_mode_set;
+		dev_priv->display.crtc_enable = ddi_crtc_enable;
+		dev_priv->display.crtc_disable = ddi_crtc_disable;
+		dev_priv->display.off = ddi_crtc_off;
 		dev_priv->display.update_plane = ironlake_update_plane;
 	} else if (HAS_PCH_SPLIT(dev)) {
 		dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
@@ -8419,9 +8419,10 @@ static void intel_init_display(struct drm_device *dev)
 			dev_priv->display.write_eld = ironlake_write_eld;
 			dev_priv->display.modeset_global_resources =
 				ivb_modeset_global_resources;
-		} else if (IS_HASWELL(dev)) {
-			dev_priv->display.fdi_link_train = hsw_fdi_link_train;
-			dev_priv->display.write_eld = haswell_write_eld;
+		} else if (HAS_DDI(dev)) {
+			dev_priv->display.fdi_link_train =
+				intel_ddi_fdi_link_train;
+			dev_priv->display.write_eld = ddi_write_eld;
 		}
 	} else if (IS_G4X(dev)) {
 		dev_priv->display.write_eld = g4x_write_eld;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3f633ca..94a565d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -332,7 +332,7 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	uint32_t status;
 	bool done;
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (intel_dig_port->port) {
 		case PORT_A:
 			ch_ctl = DPA_AUX_CH_CTL;
@@ -387,7 +387,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (intel_dig_port->port) {
 		case PORT_A:
 			ch_ctl = DPA_AUX_CH_CTL;
@@ -859,7 +859,7 @@ 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 (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
 			   TU_SIZE(m_n.tu) | m_n.gmch_m);
 		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
@@ -1522,7 +1522,7 @@ intel_dp_pre_emphasis_max(struct intel_dp *intel_dp, uint8_t voltage_swing)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
 		case DP_TRAIN_VOLTAGE_SWING_400:
 			return DP_TRAIN_PRE_EMPHASIS_9_5;
@@ -1689,7 +1689,7 @@ 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)
+intel_dp_signal_levels_ddi(uint8_t train_set)
 {
 	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
 					 DP_TRAIN_PRE_EMPHASIS_MASK);
@@ -1733,7 +1733,7 @@ intel_dp_set_link_train(struct intel_dp *intel_dp,
 	int ret;
 	uint32_t temp;
 
-	if (IS_HASWELL(dev)) {
+	if (HAS_DDI(dev)) {
 		temp = I915_READ(DP_TP_CTL(port));
 
 		if (dp_train_pat & DP_LINK_SCRAMBLING_DISABLE)
@@ -1859,8 +1859,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
 		uint32_t    signal_levels;
 
-		if (IS_HASWELL(dev)) {
-			signal_levels = intel_dp_signal_levels_hsw(
+		if (HAS_DDI(dev)) {
+			signal_levels = intel_dp_signal_levels_ddi(
 							intel_dp->train_set[0]);
 			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
 		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
@@ -1950,8 +1950,8 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		if (IS_HASWELL(dev)) {
-			signal_levels = intel_dp_signal_levels_hsw(intel_dp->train_set[0]);
+		if (HAS_DDI(dev)) {
+			signal_levels = intel_dp_signal_levels_ddi(intel_dp->train_set[0]);
 			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
 		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
 			signal_levels = intel_gen7_edp_signal_levels(intel_dp->train_set[0]);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7ca7772..e6296ecd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -614,7 +614,7 @@ extern void intel_write_eld(struct drm_encoder *encoder,
 			    struct drm_display_mode *mode);
 extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
 extern void intel_prepare_ddi(struct drm_device *dev);
-extern void hsw_fdi_link_train(struct drm_crtc *crtc);
+extern void intel_ddi_fdi_link_train(struct drm_crtc *crtc);
 extern void intel_ddi_init(struct drm_device *dev, enum port port);
 
 /* For use by IVB LP watermark workaround in intel_sprite.c */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 53df0a8..5f171e9 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -286,7 +286,7 @@ static void vlv_write_infoframe(struct drm_encoder *encoder,
 	POSTING_READ(reg);
 }
 
-static void hsw_write_infoframe(struct drm_encoder *encoder,
+static void ddi_write_infoframe(struct drm_encoder *encoder,
 				struct dip_infoframe *frame)
 {
 	uint32_t *data = (uint32_t *)frame;
@@ -552,7 +552,7 @@ static void vlv_set_infoframes(struct drm_encoder *encoder,
 	intel_hdmi_set_spd_infoframe(encoder);
 }
 
-static void hsw_set_infoframes(struct drm_encoder *encoder,
+static void ddi_set_infoframes(struct drm_encoder *encoder,
 			       struct drm_display_mode *adjusted_mode)
 {
 	struct drm_i915_private *dev_priv = encoder->dev->dev_private;
@@ -1002,9 +1002,9 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
 	} else if (IS_VALLEYVIEW(dev)) {
 		intel_hdmi->write_infoframe = vlv_write_infoframe;
 		intel_hdmi->set_infoframes = vlv_set_infoframes;
-	} else if (IS_HASWELL(dev)) {
-		intel_hdmi->write_infoframe = hsw_write_infoframe;
-		intel_hdmi->set_infoframes = hsw_set_infoframes;
+	} else if (HAS_DDI(dev)) {
+		intel_hdmi->write_infoframe = ddi_write_infoframe;
+		intel_hdmi->set_infoframes = ddi_set_infoframes;
 	} else if (HAS_PCH_IBX(dev)) {
 		intel_hdmi->write_infoframe = ibx_write_infoframe;
 		intel_hdmi->set_infoframes = ibx_set_infoframes;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index abfff29..18eae23 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3932,7 +3932,7 @@ void intel_init_power_wells(struct drm_device *dev)
 	};
 	int i;
 
-	if (!IS_HASWELL(dev))
+	if (!HAS_DDI(dev))
 		return;
 
 	mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 827dcd4..f1caf88 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -129,7 +129,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 
 	/* HSW consolidates SPRTILEOFF and SPRLINOFF into a single SPROFFSET
 	 * register */
-	if (IS_HASWELL(dev))
+	if (HAS_DDI(dev))
 		I915_WRITE(SPROFFSET(pipe), (y << 16) | x);
 	else if (obj->tiling_mode != I915_TILING_NONE)
 		I915_WRITE(SPRTILEOFF(pipe), (y << 16) | x);
@@ -700,7 +700,7 @@ intel_plane_init(struct drm_device *dev, enum pipe pipe)
 		break;
 
 	case 7:
-		if (IS_HASWELL(dev) || IS_VALLEYVIEW(dev))
+		if (HAS_DDI(dev) || IS_VALLEYVIEW(dev))
 			intel_plane->can_scale = false;
 		else
 			intel_plane->can_scale = true;
-- 
1.7.11.7

[PATCH 4/5] drm/i915: add intel_dp_set_sinal_levels

[Paulo Zanoni]

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

So we can de-duplicate code that's inside intel_dp_start_link_train
and intel_dp_complete_link_train.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 94a565d..4b17980 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1591,7 +1591,7 @@ intel_get_adjust_train(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_ST
 }
 
 static uint32_t
-intel_dp_signal_levels(uint8_t train_set)
+intel_gen4_signal_levels(uint8_t train_set)
 {
 	uint32_t	signal_levels = 0;
 
@@ -1689,7 +1689,7 @@ 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_ddi(uint8_t train_set)
+intel_ddi_signal_levels(uint8_t train_set)
 {
 	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
 					 DP_TRAIN_PRE_EMPHASIS_MASK);
@@ -1721,6 +1721,34 @@ intel_dp_signal_levels_ddi(uint8_t train_set)
 	}
 }
 
+/* Properly updates "DP" with the correct signal levels. */
+static void
+intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	uint32_t signal_levels, mask;
+	uint8_t train_set = intel_dp->train_set[0];
+
+	if (HAS_DDI(dev)) {
+		signal_levels = intel_ddi_signal_levels(train_set);
+		mask = DDI_BUF_EMP_MASK;
+	} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
+		signal_levels = intel_gen7_edp_signal_levels(train_set);
+		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_IVB;
+	} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
+		signal_levels = intel_gen6_edp_signal_levels(train_set);
+		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_SNB;
+	} else {
+		signal_levels = intel_gen4_signal_levels(train_set);
+		mask = DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK;
+	}
+
+	DRM_DEBUG_KMS("Using signal levels %08x\n", signal_levels);
+
+	*DP = (*DP & ~mask) | signal_levels;
+}
+
 static bool
 intel_dp_set_link_train(struct intel_dp *intel_dp,
 			uint32_t dp_reg_value,
@@ -1857,24 +1885,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	for (;;) {
 		/* Use intel_dp->train_set[0] to set the voltage and pre emphasis values */
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
-		uint32_t    signal_levels;
-
-		if (HAS_DDI(dev)) {
-			signal_levels = intel_dp_signal_levels_ddi(
-							intel_dp->train_set[0]);
-			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
-		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
-			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)) {
-			signal_levels = intel_gen6_edp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB) | signal_levels;
-		} else {
-			signal_levels = intel_dp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
-		}
-		DRM_DEBUG_KMS("training pattern 1 signal levels %08x\n",
-			      signal_levels);
+
+		intel_dp_set_signal_levels(intel_dp, &DP);
 
 		/* Set training pattern 1 */
 		if (!intel_dp_set_link_train(intel_dp, DP,
@@ -1930,7 +1942,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 void
 intel_dp_complete_link_train(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
@@ -1941,7 +1952,6 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	channel_eq = false;
 	for (;;) {
 		/* Use intel_dp->train_set[0] to set the voltage and pre emphasis values */
-		uint32_t    signal_levels;
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
 
 		if (cr_tries > 5) {
@@ -1950,19 +1960,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		if (HAS_DDI(dev)) {
-			signal_levels = intel_dp_signal_levels_ddi(intel_dp->train_set[0]);
-			DP = (DP & ~DDI_BUF_EMP_MASK) | signal_levels;
-		} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
-			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)) {
-			signal_levels = intel_gen6_edp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB) | signal_levels;
-		} else {
-			signal_levels = intel_dp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
-		}
+		intel_dp_set_signal_levels(intel_dp, &DP);
 
 		/* channel eq pattern */
 		if (!intel_dp_set_link_train(intel_dp, DP,
-- 
1.7.11.7

[PATCH 5/5] drm/i915: add DP aux ctl regs to struct intel_dp

[Paulo Zanoni]

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

So we can just reuse the value whenever we need, reducing code
duplication.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 12 +++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 57 ++++++++--------------------------------
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 25 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f2a5ea6..04d7e66 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2541,6 +2541,9 @@
 #define DPD_AUX_CH_DATA4		0x64320
 #define DPD_AUX_CH_DATA5		0x64324
 
+#define DP_AUX_CH_CTL(port)	_PORT(port, DPA_AUX_CH_CTL, DPB_AUX_CH_CTL)
+#define DP_AUX_CH_DATA1(port)	_PORT(port, DPA_AUX_CH_DATA1, DPB_AUX_CH_DATA1)
+
 #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)
@@ -4066,6 +4069,15 @@
 #define PCH_DPD_AUX_CH_DATA4	0xe4320
 #define PCH_DPD_AUX_CH_DATA5	0xe4324
 
+/* Definitions to make the macro easier to understand: */
+#define __FAKE_PCH_DPA_AUX_CH_CTL	0xe4010
+#define __FAKE_PCH_DPA_AUX_CH_DATA1	0xe4014
+
+#define PCH_DP_AUX_CH_CTL(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_CTL, \
+				      PCH_DPB_AUX_CH_CTL)
+#define PCH_DP_AUX_CH_DATA1(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_DATA1, \
+					PCH_DPB_AUX_CH_DATA1)
+
 /* CPT */
 #define  PORT_TRANS_A_SEL_CPT	0
 #define  PORT_TRANS_B_SEL_CPT	(1<<29)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4b17980..7ae2a07 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -328,29 +328,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
 	uint32_t status;
 	bool done;
 
-	if (HAS_DDI(dev)) {
-		switch (intel_dig_port->port) {
-		case PORT_A:
-			ch_ctl = DPA_AUX_CH_CTL;
-			break;
-		case PORT_B:
-			ch_ctl = PCH_DPB_AUX_CH_CTL;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
 		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
@@ -369,12 +350,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		uint8_t *send, int send_bytes,
 		uint8_t *recv, int recv_size)
 {
-	uint32_t output_reg = intel_dp->output_reg;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = output_reg + 0x10;
-	uint32_t ch_data = ch_ctl + 4;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
+	uint32_t ch_data = intel_dp->aux_ch_data_reg;
 	int i, ret, recv_bytes;
 	uint32_t status;
 	uint32_t aux_clock_divider;
@@ -387,29 +367,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
-	if (HAS_DDI(dev)) {
-		switch (intel_dig_port->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_DATA1;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			ch_data = PCH_DPC_AUX_CH_DATA1;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			ch_data = PCH_DPD_AUX_CH_DATA1;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 	intel_dp_check_edp(intel_dp);
 	/* The clock divider is based off the hrawclk,
 	 * and would like to run at 2MHz. So, take the
@@ -2794,6 +2751,14 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 
+	if (port == PORT_A || INTEL_INFO(dev)->gen < 5) {
+		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
+		intel_dp->aux_ch_data_reg = DP_AUX_CH_DATA1(port);
+	} else {
+		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
+		intel_dp->aux_ch_data_reg = PCH_DP_AUX_CH_DATA1(port);
+	}
+
 	/* Set up the DDC bus. */
 	switch (port) {
 	case PORT_A:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e6296ecd..0ae734b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -352,6 +352,8 @@ struct intel_hdmi {
 
 struct intel_dp {
 	uint32_t output_reg;
+	uint32_t aux_ch_ctl_reg;
+	uint32_t aux_ch_data_reg;
 	uint32_t DP;
 	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
 	bool has_audio;
-- 
1.7.11.7

Re: [PATCH 3/5] drm/i915: DDI naming standards cleanup

[Chris Wilson]

On Thu,  6 Dec 2012 11:12:40 -0200, Paulo Zanoni <przanoni@gmail.com> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> A big part of the Haswell display code is called intel_ddi_something
> while a small part of it is called haswell_something. Being consistent
> with the naming standards is a nice thing, so IMHO we should only use
> one of the naming standards for everything.
> 
> So instead of converting everything to haswell_something I converted
> to ddi_something, which seems more appropriate IMHO. Now our code
> looks a little bit more consistent.
> 
> The next step is to move everything to intel_ddi.c.

Nak. Now you are really muddying the waters between generation
specific paths and one implementation of a display engine.

A few chunks are good; where you are indeed checking for the digital
ports, but most look silly.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/5] drm/i915: DDI naming standards cleanup

[Paulo Zanoni]

Hi

2012/12/6 Chris Wilson <chris@chris-wilson.co.uk>:
> On Thu,  6 Dec 2012 11:12:40 -0200, Paulo Zanoni <przanoni@gmail.com> wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> A big part of the Haswell display code is called intel_ddi_something
>> while a small part of it is called haswell_something. Being consistent
>> with the naming standards is a nice thing, so IMHO we should only use
>> one of the naming standards for everything.
>>
>> So instead of converting everything to haswell_something I converted
>> to ddi_something, which seems more appropriate IMHO. Now our code
>> looks a little bit more consistent.
>>
>> The next step is to move everything to intel_ddi.c.
>
> Nak. Now you are really muddying the waters between generation
> specific paths and one implementation of a display engine.
>
> A few chunks are good; where you are indeed checking for the digital
> ports, but most look silly.


Ok, so for now let's just discard this patch. I will resend patches 4
and 5 so they apply cleanly without requiring patch 3.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre



-- 
Paulo Zanoni

[PATCH 4/5] drm/i915: add intel_dp_set_signal_levels

[Paulo Zanoni]

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

So we can de-duplicate code that's inside intel_dp_start_link_train
and intel_dp_complete_link_train.

V2: Rebase since patch 3/5 was discarded.

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b2130bc..75073cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1587,7 +1587,7 @@ intel_get_adjust_train(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_ST
 }
 
 static uint32_t
-intel_dp_signal_levels(uint8_t train_set)
+intel_gen4_signal_levels(uint8_t train_set)
 {
 	uint32_t	signal_levels = 0;
 
@@ -1685,7 +1685,7 @@ 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)
+intel_hsw_signal_levels(uint8_t train_set)
 {
 	int signal_levels = train_set & (DP_TRAIN_VOLTAGE_SWING_MASK |
 					 DP_TRAIN_PRE_EMPHASIS_MASK);
@@ -1717,6 +1717,34 @@ intel_dp_signal_levels_hsw(uint8_t train_set)
 	}
 }
 
+/* Properly updates "DP" with the correct signal levels. */
+static void
+intel_dp_set_signal_levels(struct intel_dp *intel_dp, uint32_t *DP)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = intel_dig_port->base.base.dev;
+	uint32_t signal_levels, mask;
+	uint8_t train_set = intel_dp->train_set[0];
+
+	if (IS_HASWELL(dev)) {
+		signal_levels = intel_hsw_signal_levels(train_set);
+		mask = DDI_BUF_EMP_MASK;
+	} else if (IS_GEN7(dev) && is_cpu_edp(intel_dp) && !IS_VALLEYVIEW(dev)) {
+		signal_levels = intel_gen7_edp_signal_levels(train_set);
+		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_IVB;
+	} else if (IS_GEN6(dev) && is_cpu_edp(intel_dp)) {
+		signal_levels = intel_gen6_edp_signal_levels(train_set);
+		mask = EDP_LINK_TRAIN_VOL_EMP_MASK_SNB;
+	} else {
+		signal_levels = intel_gen4_signal_levels(train_set);
+		mask = DP_VOLTAGE_MASK | DP_PRE_EMPHASIS_MASK;
+	}
+
+	DRM_DEBUG_KMS("Using signal levels %08x\n", signal_levels);
+
+	*DP = (*DP & ~mask) | signal_levels;
+}
+
 static bool
 intel_dp_set_link_train(struct intel_dp *intel_dp,
 			uint32_t dp_reg_value,
@@ -1853,24 +1881,8 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 	for (;;) {
 		/* Use intel_dp->train_set[0] to set the voltage and pre emphasis values */
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
-		uint32_t    signal_levels;
-
-		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) && !IS_VALLEYVIEW(dev)) {
-			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)) {
-			signal_levels = intel_gen6_edp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB) | signal_levels;
-		} else {
-			signal_levels = intel_dp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
-		}
-		DRM_DEBUG_KMS("training pattern 1 signal levels %08x\n",
-			      signal_levels);
+
+		intel_dp_set_signal_levels(intel_dp, &DP);
 
 		/* Set training pattern 1 */
 		if (!intel_dp_set_link_train(intel_dp, DP,
@@ -1926,7 +1938,6 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 void
 intel_dp_complete_link_train(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
@@ -1936,8 +1947,6 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	cr_tries = 0;
 	channel_eq = false;
 	for (;;) {
-		/* Use intel_dp->train_set[0] to set the voltage and pre emphasis values */
-		uint32_t    signal_levels;
 		uint8_t	    link_status[DP_LINK_STATUS_SIZE];
 
 		if (cr_tries > 5) {
@@ -1946,19 +1955,7 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 			break;
 		}
 
-		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) && !IS_VALLEYVIEW(dev)) {
-			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)) {
-			signal_levels = intel_gen6_edp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~EDP_LINK_TRAIN_VOL_EMP_MASK_SNB) | signal_levels;
-		} else {
-			signal_levels = intel_dp_signal_levels(intel_dp->train_set[0]);
-			DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels;
-		}
+		intel_dp_set_signal_levels(intel_dp, &DP);
 
 		/* channel eq pattern */
 		if (!intel_dp_set_link_train(intel_dp, DP,
-- 
1.7.11.7

[PATCH 5/5] drm/i915: add DP aux ctl regs to struct intel_dp

[Paulo Zanoni]

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

So we can just reuse the value whenever we need, reducing code
duplication.

V2: Rebase since patch 3/5 was discarded.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  | 12 +++++++++
 drivers/gpu/drm/i915/intel_dp.c  | 56 +++++++---------------------------------
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f2a5ea6..4307cb2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2541,6 +2541,9 @@
 #define DPD_AUX_CH_DATA4		0x64320
 #define DPD_AUX_CH_DATA5		0x64324
 
+#define DP_AUX_CH_CTL(port)    _PORT(port, DPA_AUX_CH_CTL, DPB_AUX_CH_CTL)
+#define DP_AUX_CH_DATA1(port)  _PORT(port, DPA_AUX_CH_DATA1, DPB_AUX_CH_DATA1)
+
 #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)
@@ -4066,6 +4069,15 @@
 #define PCH_DPD_AUX_CH_DATA4	0xe4320
 #define PCH_DPD_AUX_CH_DATA5	0xe4324
 
+/* Definitions to make the macro easier to understand: */
+#define __FAKE_PCH_DPA_AUX_CH_CTL	0xe4010
+#define __FAKE_PCH_DPA_AUX_CH_DATA1	0xe4014
+
+#define PCH_DP_AUX_CH_CTL(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_CTL, \
+				      PCH_DPB_AUX_CH_CTL)
+#define PCH_DP_AUX_CH_DATA1(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_DATA1, \
+					PCH_DPB_AUX_CH_DATA1)
+
 /* CPT */
 #define  PORT_TRANS_A_SEL_CPT	0
 #define  PORT_TRANS_B_SEL_CPT	(1<<29)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 75073cc..2db90f8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -328,29 +328,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
 	uint32_t status;
 	bool done;
 
-	if (IS_HASWELL(dev)) {
-		switch (intel_dig_port->port) {
-		case PORT_A:
-			ch_ctl = DPA_AUX_CH_CTL;
-			break;
-		case PORT_B:
-			ch_ctl = PCH_DPB_AUX_CH_CTL;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
 	if (has_aux_irq)
 		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
@@ -369,12 +350,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 		uint8_t *send, int send_bytes,
 		uint8_t *recv, int recv_size)
 {
-	uint32_t output_reg = intel_dp->output_reg;
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	uint32_t ch_ctl = output_reg + 0x10;
-	uint32_t ch_data = ch_ctl + 4;
+	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
+	uint32_t ch_data = intel_dp->aux_ch_data_reg;
 	int i, ret, recv_bytes;
 	uint32_t status;
 	uint32_t aux_clock_divider;
@@ -387,29 +367,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	 */
 	pm_qos_update_request(&dev_priv->pm_qos, 0);
 
-	if (IS_HASWELL(dev)) {
-		switch (intel_dig_port->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_DATA1;
-			break;
-		case PORT_C:
-			ch_ctl = PCH_DPC_AUX_CH_CTL;
-			ch_data = PCH_DPC_AUX_CH_DATA1;
-			break;
-		case PORT_D:
-			ch_ctl = PCH_DPD_AUX_CH_CTL;
-			ch_data = PCH_DPD_AUX_CH_DATA1;
-			break;
-		default:
-			BUG();
-		}
-	}
-
 	intel_dp_check_edp(intel_dp);
 	/* The clock divider is based off the hrawclk,
 	 * and would like to run at 2MHz. So, take the
@@ -2786,6 +2743,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	else
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 
+	if (port == PORT_A || INTEL_INFO(dev)->gen < 5) {
+		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
+		intel_dp->aux_ch_data_reg = DP_AUX_CH_DATA1(port);
+	} else {
+		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
+		intel_dp->aux_ch_data_reg = PCH_DP_AUX_CH_DATA1(port);
+	}
 
 	/* Set up the DDC bus. */
 	switch (port) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7ca7772..aebb023 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -352,6 +352,8 @@ struct intel_hdmi {
 
 struct intel_dp {
 	uint32_t output_reg;
+	uint32_t aux_ch_ctl_reg;
+	uint32_t aux_ch_data_reg;
 	uint32_t DP;
 	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
 	bool has_audio;
-- 
1.7.11.7

Re: [PATCH 5/5] drm/i915: add DP aux ctl regs to struct intel_dp

[Daniel Vetter]

On Thu, Dec 06, 2012 at 04:53:22PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> So we can just reuse the value whenever we need, reducing code
> duplication.
> 
> V2: Rebase since patch 3/5 was discarded.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Since Haswel && !PORT_A is the only special case where the dp aux stuff
isn't at a fixed offset, I think we can condense this down a lot and rip
out all the FAKE stuff. Also, since dp aux data&control are everywhere at
fixed offsets relative to each another, I don't think we need them both.

Merged patches 1,2&4 from this series, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 12 +++++++++
>  drivers/gpu/drm/i915/intel_dp.c  | 56 +++++++---------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 24 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f2a5ea6..4307cb2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2541,6 +2541,9 @@
>  #define DPD_AUX_CH_DATA4		0x64320
>  #define DPD_AUX_CH_DATA5		0x64324
>  
> +#define DP_AUX_CH_CTL(port)    _PORT(port, DPA_AUX_CH_CTL, DPB_AUX_CH_CTL)
> +#define DP_AUX_CH_DATA1(port)  _PORT(port, DPA_AUX_CH_DATA1, DPB_AUX_CH_DATA1)
> +
>  #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)
> @@ -4066,6 +4069,15 @@
>  #define PCH_DPD_AUX_CH_DATA4	0xe4320
>  #define PCH_DPD_AUX_CH_DATA5	0xe4324
>  
> +/* Definitions to make the macro easier to understand: */
> +#define __FAKE_PCH_DPA_AUX_CH_CTL	0xe4010
> +#define __FAKE_PCH_DPA_AUX_CH_DATA1	0xe4014
> +
> +#define PCH_DP_AUX_CH_CTL(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_CTL, \
> +				      PCH_DPB_AUX_CH_CTL)
> +#define PCH_DP_AUX_CH_DATA1(port) _PORT(port, __FAKE_PCH_DPA_AUX_CH_DATA1, \
> +					PCH_DPB_AUX_CH_DATA1)
> +
>  /* CPT */
>  #define  PORT_TRANS_A_SEL_CPT	0
>  #define  PORT_TRANS_B_SEL_CPT	(1<<29)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 75073cc..2db90f8 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -328,29 +328,10 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t ch_ctl = intel_dp->output_reg + 0x10;
> +	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
>  	uint32_t status;
>  	bool done;
>  
> -	if (IS_HASWELL(dev)) {
> -		switch (intel_dig_port->port) {
> -		case PORT_A:
> -			ch_ctl = DPA_AUX_CH_CTL;
> -			break;
> -		case PORT_B:
> -			ch_ctl = PCH_DPB_AUX_CH_CTL;
> -			break;
> -		case PORT_C:
> -			ch_ctl = PCH_DPC_AUX_CH_CTL;
> -			break;
> -		case PORT_D:
> -			ch_ctl = PCH_DPD_AUX_CH_CTL;
> -			break;
> -		default:
> -			BUG();
> -		}
> -	}
> -
>  #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
>  	if (has_aux_irq)
>  		done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, 10);
> @@ -369,12 +350,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  		uint8_t *send, int send_bytes,
>  		uint8_t *recv, int recv_size)
>  {
> -	uint32_t output_reg = intel_dp->output_reg;
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	uint32_t ch_ctl = output_reg + 0x10;
> -	uint32_t ch_data = ch_ctl + 4;
> +	uint32_t ch_ctl = intel_dp->aux_ch_ctl_reg;
> +	uint32_t ch_data = intel_dp->aux_ch_data_reg;
>  	int i, ret, recv_bytes;
>  	uint32_t status;
>  	uint32_t aux_clock_divider;
> @@ -387,29 +367,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	 */
>  	pm_qos_update_request(&dev_priv->pm_qos, 0);
>  
> -	if (IS_HASWELL(dev)) {
> -		switch (intel_dig_port->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_DATA1;
> -			break;
> -		case PORT_C:
> -			ch_ctl = PCH_DPC_AUX_CH_CTL;
> -			ch_data = PCH_DPC_AUX_CH_DATA1;
> -			break;
> -		case PORT_D:
> -			ch_ctl = PCH_DPD_AUX_CH_CTL;
> -			ch_data = PCH_DPD_AUX_CH_DATA1;
> -			break;
> -		default:
> -			BUG();
> -		}
> -	}
> -
>  	intel_dp_check_edp(intel_dp);
>  	/* The clock divider is based off the hrawclk,
>  	 * and would like to run at 2MHz. So, take the
> @@ -2786,6 +2743,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	else
>  		intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
> +	if (port == PORT_A || INTEL_INFO(dev)->gen < 5) {
> +		intel_dp->aux_ch_ctl_reg = DP_AUX_CH_CTL(port);
> +		intel_dp->aux_ch_data_reg = DP_AUX_CH_DATA1(port);
> +	} else {
> +		intel_dp->aux_ch_ctl_reg = PCH_DP_AUX_CH_CTL(port);
> +		intel_dp->aux_ch_data_reg = PCH_DP_AUX_CH_DATA1(port);
> +	}
>  
>  	/* Set up the DDC bus. */
>  	switch (port) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 7ca7772..aebb023 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -352,6 +352,8 @@ struct intel_hdmi {
>  
>  struct intel_dp {
>  	uint32_t output_reg;
> +	uint32_t aux_ch_ctl_reg;
> +	uint32_t aux_ch_data_reg;
>  	uint32_t DP;
>  	uint8_t  link_configuration[DP_LINK_CONFIGURATION_SIZE];
>  	bool has_audio;
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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