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 <davidf@rd.bbc.co.uk>
[ickle: fix up some surrounding checkpatch warnings]
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index df648cb4c29641cec581307b1c40a46d812fd58b..864417cffe9a7c3a45ea52716aae46da97e1b7c1 100644 (file)
--- 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