From 3cf2efb1a7c68d55d60dcb2ed9609e1a2fc25952 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 29 Nov 2010 10:09:55 +0000 Subject: Revert "drm/i915/dp: use VBT provided eDP params if available" This reverts commit 869184a675662bddcdf76c5b95665272facff2b8. This is required for the Sony Vaio Jesse was working on at the time, but breaks most other eDP machines - machines that were working in earlier kernels. Reported-and-tested-by: Dave Airlie Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=31188 Tested-by: Zhao Jian Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_dp.c | 146 ++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 89 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_dp.c') diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c8e005553310..300f64b4238b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -584,17 +584,6 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode, mode->clock = dev_priv->panel_fixed_mode->clock; } - /* Just use VBT values for eDP */ - if (is_edp(intel_dp)) { - intel_dp->lane_count = dev_priv->edp.lanes; - intel_dp->link_bw = dev_priv->edp.rate; - adjusted_mode->clock = intel_dp_link_clock(intel_dp->link_bw); - DRM_DEBUG_KMS("eDP link bw %02x lane count %d clock %d\n", - intel_dp->link_bw, intel_dp->lane_count, - adjusted_mode->clock); - return true; - } - for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) { for (clock = 0; clock <= max_clock; clock++) { int link_avail = intel_dp_max_data_rate(intel_dp_link_clock(bws[clock]), lane_count); @@ -613,6 +602,19 @@ intel_dp_mode_fixup(struct drm_encoder *encoder, struct drm_display_mode *mode, } } + if (is_edp(intel_dp)) { + /* okay we failed just pick the highest */ + intel_dp->lane_count = max_lane_count; + intel_dp->link_bw = bws[max_clock]; + adjusted_mode->clock = intel_dp_link_clock(intel_dp->link_bw); + DRM_DEBUG_KMS("Force picking display port link bw %02x lane " + "count %d clock %d\n", + intel_dp->link_bw, intel_dp->lane_count, + adjusted_mode->clock); + + return true; + } + return false; } @@ -1087,21 +1089,11 @@ intel_get_adjust_train(struct intel_dp *intel_dp) } static uint32_t -intel_dp_signal_levels(struct intel_dp *intel_dp) +intel_dp_signal_levels(uint8_t train_set, int lane_count) { - struct drm_device *dev = intel_dp->base.base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; - uint32_t signal_levels = 0; - u8 train_set = intel_dp->train_set[0]; - u32 vswing = train_set & DP_TRAIN_VOLTAGE_SWING_MASK; - u32 preemphasis = train_set & DP_TRAIN_PRE_EMPHASIS_MASK; + uint32_t signal_levels = 0; - if (is_edp(intel_dp)) { - vswing = dev_priv->edp.vswing; - preemphasis = dev_priv->edp.preemphasis; - } - - switch (vswing) { + switch (train_set & DP_TRAIN_VOLTAGE_SWING_MASK) { case DP_TRAIN_VOLTAGE_SWING_400: default: signal_levels |= DP_VOLTAGE_0_4; @@ -1116,7 +1108,7 @@ intel_dp_signal_levels(struct intel_dp *intel_dp) signal_levels |= DP_VOLTAGE_1_2; break; } - switch (preemphasis) { + switch (train_set & DP_TRAIN_PRE_EMPHASIS_MASK) { case DP_TRAIN_PRE_EMPHASIS_0: default: signal_levels |= DP_PRE_EMPHASIS_0; @@ -1202,18 +1194,6 @@ intel_channel_eq_ok(struct intel_dp *intel_dp) return true; } -static bool -intel_dp_aux_handshake_required(struct intel_dp *intel_dp) -{ - struct drm_device *dev = intel_dp->base.base.dev; - struct drm_i915_private *dev_priv = dev->dev_private; - - if (is_edp(intel_dp) && dev_priv->no_aux_handshake) - return false; - - return true; -} - static bool intel_dp_set_link_train(struct intel_dp *intel_dp, uint32_t dp_reg_value, @@ -1226,9 +1206,6 @@ intel_dp_set_link_train(struct intel_dp *intel_dp, I915_WRITE(intel_dp->output_reg, dp_reg_value); POSTING_READ(intel_dp->output_reg); - if (!intel_dp_aux_handshake_required(intel_dp)) - return true; - intel_dp_aux_native_write_1(intel_dp, DP_TRAINING_PATTERN_SET, dp_train_pat); @@ -1261,11 +1238,10 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) POSTING_READ(intel_dp->output_reg); intel_wait_for_vblank(dev, intel_crtc->pipe); - if (intel_dp_aux_handshake_required(intel_dp)) - /* Write the link configuration data */ - intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, - intel_dp->link_configuration, - DP_LINK_CONFIGURATION_SIZE); + /* Write the link configuration data */ + intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, + intel_dp->link_configuration, + DP_LINK_CONFIGURATION_SIZE); DP |= DP_PORT_EN; if (HAS_PCH_CPT(dev) && !is_edp(intel_dp)) @@ -1283,7 +1259,7 @@ intel_dp_start_link_train(struct intel_dp *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); + signal_levels = intel_dp_signal_levels(intel_dp->train_set[0], intel_dp->lane_count); DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels; } @@ -1297,37 +1273,33 @@ intel_dp_start_link_train(struct intel_dp *intel_dp) break; /* Set training pattern 1 */ - udelay(500); - if (intel_dp_aux_handshake_required(intel_dp)) { + udelay(100); + if (!intel_dp_get_link_status(intel_dp)) break; - } else { - if (!intel_dp_get_link_status(intel_dp)) - break; - if (intel_clock_recovery_ok(intel_dp->link_status, intel_dp->lane_count)) { - clock_recovery = true; - break; - } + if (intel_clock_recovery_ok(intel_dp->link_status, intel_dp->lane_count)) { + clock_recovery = true; + break; + } - /* Check to see if we've tried the max voltage */ - 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) + /* Check to see if we've tried the max voltage */ + 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) + break; - /* Check to see if we've tried the same voltage 5 times */ - if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) == voltage) { - ++tries; - if (tries == 5) - break; - } else - tries = 0; - voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK; + /* Check to see if we've tried the same voltage 5 times */ + if ((intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) == voltage) { + ++tries; + if (tries == 5) + break; + } else + tries = 0; + voltage = intel_dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK; - /* Compute new intel_dp->train_set as requested by target */ - intel_get_adjust_train(intel_dp); - } + /* Compute new intel_dp->train_set as requested by target */ + intel_get_adjust_train(intel_dp); } intel_dp->DP = DP; @@ -1354,7 +1326,7 @@ intel_dp_complete_link_train(struct intel_dp *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); + signal_levels = intel_dp_signal_levels(intel_dp->train_set[0], intel_dp->lane_count); DP = (DP & ~(DP_VOLTAGE_MASK|DP_PRE_EMPHASIS_MASK)) | signal_levels; } @@ -1368,28 +1340,24 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp) DP_TRAINING_PATTERN_2)) break; - udelay(500); - - if (!intel_dp_aux_handshake_required(intel_dp)) { + udelay(400); + if (!intel_dp_get_link_status(intel_dp)) break; - } else { - if (!intel_dp_get_link_status(intel_dp)) - break; - if (intel_channel_eq_ok(intel_dp)) { - channel_eq = true; - break; - } + if (intel_channel_eq_ok(intel_dp)) { + channel_eq = true; + break; + } - /* Try 5 times */ - if (tries > 5) - break; + /* Try 5 times */ + if (tries > 5) + break; - /* Compute new intel_dp->train_set as requested by target */ - intel_get_adjust_train(intel_dp); - ++tries; - } + /* Compute new intel_dp->train_set as requested by target */ + intel_get_adjust_train(intel_dp); + ++tries; } + if (HAS_PCH_CPT(dev) && !is_edp(intel_dp)) reg = DP | DP_LINK_TRAIN_OFF_CPT; else -- cgit v1.2.1 From 5bddd17fec58f253cddd0bc9eab2cd9eb1bbab4a Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 18 Nov 2010 09:32:59 +0800 Subject: drm/i915: Apply a workaround for transitioning from DP on pipe B to HDMI. This workaround only applies to Ironlake. Signed-off-by: Eric Anholt Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/intel_dp.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_dp.c') diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 300f64b4238b..1d8d068bc473 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1374,6 +1374,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc); uint32_t DP = intel_dp->DP; DRM_DEBUG_KMS("\n"); @@ -1398,6 +1399,26 @@ intel_dp_link_down(struct intel_dp *intel_dp) if (is_edp(intel_dp)) DP |= DP_LINK_TRAIN_OFF; + + if (!HAS_PCH_CPT(dev) && (DP & DP_PIPEB_SELECT)) { + /* Hardware workaround: leaving our transcoder select + * set to transcoder B while it's off will prevent the + * corresponding HDMI output on transcoder A. + * + * Combine this with another hardware workaround: + * transcoder select bit can only be cleared while the + * port is enabled. + */ + DP &= ~DP_PIPEB_SELECT; + I915_WRITE(intel_dp->output_reg, DP); + + /* Changes to enable or select take place the vblank + * after being written. + */ + intel_wait_for_vblank(intel_dp->base.base.dev, + intel_crtc->pipe); + } + I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN); POSTING_READ(intel_dp->output_reg); } -- cgit v1.2.1 From 160b1543cdae83e9f8914ac7afc3d2bd686140af Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 5 Dec 2010 14:54:05 +0000 Subject: drm/i915/dp: Trivial code tidy Locally scope the crtc to where it is used. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_dp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_dp.c') diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1d8d068bc473..2c9601f2f208 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1374,7 +1374,6 @@ intel_dp_link_down(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc); uint32_t DP = intel_dp->DP; DRM_DEBUG_KMS("\n"); @@ -1401,6 +1400,8 @@ intel_dp_link_down(struct intel_dp *intel_dp) DP |= DP_LINK_TRAIN_OFF; if (!HAS_PCH_CPT(dev) && (DP & DP_PIPEB_SELECT)) { + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc); + /* Hardware workaround: leaving our transcoder select * set to transcoder B while it's off will prevent the * corresponding HDMI output on transcoder A. @@ -1415,8 +1416,7 @@ intel_dp_link_down(struct intel_dp *intel_dp) /* Changes to enable or select take place the vblank * after being written. */ - intel_wait_for_vblank(intel_dp->base.base.dev, - intel_crtc->pipe); + intel_wait_for_vblank(dev, intel_crtc->pipe); } I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN); -- cgit v1.2.1 From 1b39d6f37622f1da70aa2cfd38bfff9a52c13e05 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 6 Dec 2010 11:20:45 +0000 Subject: drm/i915/dp: Only apply the workaround if the select is still active As we may try to power down the link at various times, it is not necessarily still coupled with an encoder and so we must be careful not to depend upon an operation that is only valid when the link is still attached to a pipe. Fixes regression in 5bddd17. Reported-and-tested-by: Daniel Vetter Signed-off-by: Chris Wilson Cc: stable@kernel.org [after applying 5bddd17] --- drivers/gpu/drm/i915/intel_dp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_dp.c') diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1d8d068bc473..df648cb4c296 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1374,9 +1374,11 @@ intel_dp_link_down(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc); uint32_t DP = intel_dp->DP; + if ((I915_READ(intel_dp->output_reg) & DP_PORT_EN) == 0) + return; + DRM_DEBUG_KMS("\n"); if (is_edp(intel_dp)) { @@ -1400,7 +1402,9 @@ intel_dp_link_down(struct intel_dp *intel_dp) if (is_edp(intel_dp)) DP |= DP_LINK_TRAIN_OFF; - if (!HAS_PCH_CPT(dev) && (DP & DP_PIPEB_SELECT)) { + if (!HAS_PCH_CPT(dev) && + I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) { + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.base.crtc); /* Hardware workaround: leaving our transcoder select * set to transcoder B while it's off will prevent the * corresponding HDMI output on transcoder A. -- cgit v1.2.1 From 8316f33766a82907c694267ff911e45e256f09f9 Mon Sep 17 00:00:00 2001 From: David Flynn Date: Wed, 8 Dec 2010 16:10:21 +0000 Subject: drm/i915/dp: Fix I2C/EDID handling with active DisplayPort to DVI converter The DisplayPort standard (1.1a) states that: The I2C-over-AUX Reply field is valid only when Native AUX CH Reply field is AUX_ACK (00). When Native AUX CH Reply field is not 00, then, I2C-over-AUX Reply field must be 00 and be ignored. This fixes broken EDID reading when using an active DisplayPort to duallink DVI converter. If the AUX CH replier chooses to defer the transaction, a short read occurs and erroneous data is returned as the i2c reply due to a lack of length checking and failure to check for AUX ACK. As a result, broken EDIDs can look like: 0 1 2 3 4 5 6 7 8 9 a b c d e f 0123456789abcdef 00: bc bc bc ff bc bc bc ff bc bc bc ac bc bc bc 45 ???.???.???????E 10: bc bc bc 10 bc bc bc 34 bc bc bc ee bc bc bc 4c ???????4???????L 20: bc bc bc 50 bc bc bc 00 bc bc bc 40 bc bc bc 00 ???P???.???@???. 30: bc bc bc 01 bc bc bc 01 bc bc bc a0 bc bc bc 40 ???????????????@ 40: bc bc bc 00 bc bc bc 00 bc bc bc 00 bc bc bc 55 ???.???.???.???U 50: bc bc bc 35 bc bc bc 31 bc bc bc 20 bc bc bc fc ???5???1??? ???? 60: bc bc bc 4c bc bc bc 34 bc bc bc 46 bc bc bc 00 ???L???4???F???. 70: bc bc bc 38 bc bc bc 11 bc bc bc 20 bc bc bc 20 ???8??????? ??? 80: bc bc bc ff bc bc bc ff bc bc bc ff bc bc bc ff ???.???.???.???. ... which can lead to: [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder [drm:drm_edid_block_valid] *ERROR* Raw EDID: <3>30 30 30 30 30 30 30 32 38 32 30 32 63 63 31 61 000000028202cc1a <3>28 00 02 8c 00 00 00 00 18 00 00 00 00 00 00 00 (............... <3>20 4c 61 73 74 20 62 65 61 63 6f 6e 3a 20 33 32 Last beacon: 32 <3>32 30 6d 73 20 61 67 6f 46 00 05 8c 00 00 00 00 20ms agoF....... <3>36 00 00 00 00 00 00 00 00 0c 57 69 2d 46 69 20 6.........Wi-Fi <3>52 6f 75 74 65 72 01 08 82 84 8b 96 24 30 48 6c Router......$0Hl <3>03 01 01 06 02 00 00 2a 01 00 2f 01 00 32 04 0c .......*../..2.. <3>12 18 60 dd 09 00 10 18 02 00 00 01 00 00 18 00 ..`............. Signed-off-by: David Flynn [ickle: fix up some surrounding checkpatch warnings] Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/intel_dp.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_dp.c') diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index df648cb4c296..864417cffe9a 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -479,6 +479,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, uint16_t address = algo_data->address; uint8_t msg[5]; uint8_t reply[2]; + unsigned retry; int msg_bytes; int reply_bytes; int ret; @@ -513,14 +514,33 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, break; } - for (;;) { - ret = intel_dp_aux_ch(intel_dp, - msg, msg_bytes, - reply, reply_bytes); + for (retry = 0; retry < 5; retry++) { + ret = intel_dp_aux_ch(intel_dp, + msg, msg_bytes, + reply, reply_bytes); if (ret < 0) { DRM_DEBUG_KMS("aux_ch failed %d\n", ret); return ret; } + + switch (reply[0] & AUX_NATIVE_REPLY_MASK) { + case AUX_NATIVE_REPLY_ACK: + /* I2C-over-AUX Reply field is only valid + * when paired with AUX ACK. + */ + break; + case AUX_NATIVE_REPLY_NACK: + DRM_DEBUG_KMS("aux_ch native nack\n"); + return -EREMOTEIO; + case AUX_NATIVE_REPLY_DEFER: + udelay(100); + continue; + default: + DRM_ERROR("aux_ch invalid native reply 0x%02x\n", + reply[0]); + return -EREMOTEIO; + } + switch (reply[0] & AUX_I2C_REPLY_MASK) { case AUX_I2C_REPLY_ACK: if (mode == MODE_I2C_READ) { @@ -528,17 +548,20 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode, } return reply_bytes - 1; case AUX_I2C_REPLY_NACK: - DRM_DEBUG_KMS("aux_ch nack\n"); + DRM_DEBUG_KMS("aux_i2c nack\n"); return -EREMOTEIO; case AUX_I2C_REPLY_DEFER: - DRM_DEBUG_KMS("aux_ch defer\n"); + DRM_DEBUG_KMS("aux_i2c defer\n"); udelay(100); break; default: - DRM_ERROR("aux_ch invalid reply 0x%02x\n", reply[0]); + DRM_ERROR("aux_i2c invalid reply 0x%02x\n", reply[0]); return -EREMOTEIO; } } + + DRM_ERROR("too many retries, giving up\n"); + return -EREMOTEIO; } static int -- cgit v1.2.1