dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: David Flynn <davidf@rd.bbc.co.uk>
To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: [PATCH] drm/i915/dp: Fix I2C/EDID handling with active DisplayPort to DVI converter
Date: Wed, 8 Dec 2010 16:10:21 +0000	[thread overview]
Message-ID: <20101208161021.GF20844@rd.bbc.co.uk> (raw)

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 <davidf@rd.bbc.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c |   27 +++++++++++++++++++++++----
 1 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index c8e0055..186cdc7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -482,6 +482,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
 	int msg_bytes;
 	int reply_bytes;
 	int ret;
+	unsigned retry;
 
 	/* Set up the command byte */
 	if (mode & MODE_I2C_READ)
@@ -513,7 +514,7 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
 		break;
 	}
 
-	for (;;) {
+	for (retry = 5; retry; retry--) {
 	  ret = intel_dp_aux_ch(intel_dp,
 				msg, msg_bytes,
 				reply, reply_bytes);
@@ -521,6 +522,21 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter, int mode,
 			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 nack\n");
+			return -EREMOTEIO;
+		case AUX_NATIVE_REPLY_DEFER:
+			udelay(100);
+			continue;
+		default:
+			DRM_ERROR("aux_ch invalid 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 +544,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
-- 
1.7.1

             reply	other threads:[~2010-12-08 16:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-08 16:10 David Flynn [this message]
2010-12-08 20:17 ` [PATCH] drm/i915/dp: Fix I2C/EDID handling with active DisplayPort to DVI converter Chris Wilson
2010-12-09 18:31   ` David Flynn

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20101208161021.GF20844@rd.bbc.co.uk \
    --to=davidf@rd.bbc.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

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

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