Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/display: Don't assume dual mode adaptors support i2c sub-addressing

Current dual mode adaptor ("DP++") detection code assumes that all
adaptors support i2c sub-addressing for read operations from the
DP-HDMI adaptor ID buffer. It has been observed that multiple
adaptors do not in fact support this, and always return data starting
at register 0. On affected adaptors, the code fails to read the proper
registers that would identify the device as a type 2 adaptor, and
handles those as type 1, limiting the TMDS clock to 165MHz, even if
the according register would announce a higher TMDS clock.
Fix this by always reading the ID buffer starting from offset 0, and
discarding any bytes before the actual offset of interest.

We tried finding authoritative documentation on whether or not this is
allowed behaviour, but since all the official VESA docs are paywalled,
the best we could come up with was the spec sheet for Texas Instruments'
SNx5DP149 chip family.[1] It explicitly mentions that sub-addressing is
supported for register writes, but *not* for reads (See NOTE in
section 8.5.3). Unless TI openly decided to violate the VESA spec, one
could take that as a hint that sub-addressing is in fact not mandated
by VESA.
The other two adaptors affected used the PS8409(A) and the LT8611,
according to the data returned from their ID buffers.

[1] https://www.ti.com/lit/ds/symlink/sn75dp149.pdf

Cc: stable@vger.kernel.org
Signed-off-by: Simon Rettberg <simon.rettberg@rz.uni-freiburg.de>
Reviewed-by: Rafael Gieschke <rafael.gieschke@rz.uni-freiburg.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221006113314.41101987@computer
Acked-by: Jani Nikula <jani.nikula@intel.com>

authored by

Simon Rettberg and committed by
Ville Syrjälä
5954acba 0a3e0fb8

+29 -22
+29 -22
drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
··· 63 63 ssize_t drm_dp_dual_mode_read(struct i2c_adapter *adapter, 64 64 u8 offset, void *buffer, size_t size) 65 65 { 66 + u8 zero = 0; 67 + char *tmpbuf = NULL; 68 + /* 69 + * As sub-addressing is not supported by all adaptors, 70 + * always explicitly read from the start and discard 71 + * any bytes that come before the requested offset. 72 + * This way, no matter whether the adaptor supports it 73 + * or not, we'll end up reading the proper data. 74 + */ 66 75 struct i2c_msg msgs[] = { 67 76 { 68 77 .addr = DP_DUAL_MODE_SLAVE_ADDRESS, 69 78 .flags = 0, 70 79 .len = 1, 71 - .buf = &offset, 80 + .buf = &zero, 72 81 }, 73 82 { 74 83 .addr = DP_DUAL_MODE_SLAVE_ADDRESS, 75 84 .flags = I2C_M_RD, 76 - .len = size, 85 + .len = size + offset, 77 86 .buf = buffer, 78 87 }, 79 88 }; 80 89 int ret; 81 90 91 + if (offset) { 92 + tmpbuf = kmalloc(size + offset, GFP_KERNEL); 93 + if (!tmpbuf) 94 + return -ENOMEM; 95 + 96 + msgs[1].buf = tmpbuf; 97 + } 98 + 82 99 ret = i2c_transfer(adapter, msgs, ARRAY_SIZE(msgs)); 100 + if (tmpbuf) 101 + memcpy(buffer, tmpbuf + offset, size); 102 + 103 + kfree(tmpbuf); 104 + 83 105 if (ret < 0) 84 106 return ret; 85 107 if (ret != ARRAY_SIZE(msgs)) ··· 230 208 if (ret) 231 209 return DRM_DP_DUAL_MODE_UNKNOWN; 232 210 233 - /* 234 - * Sigh. Some (maybe all?) type 1 adaptors are broken and ack 235 - * the offset but ignore it, and instead they just always return 236 - * data from the start of the HDMI ID buffer. So for a broken 237 - * type 1 HDMI adaptor a single byte read will always give us 238 - * 0x44, and for a type 1 DVI adaptor it should give 0x00 239 - * (assuming it implements any registers). Fortunately neither 240 - * of those values will match the type 2 signature of the 241 - * DP_DUAL_MODE_ADAPTOR_ID register so we can proceed with 242 - * the type 2 adaptor detection safely even in the presence 243 - * of broken type 1 adaptors. 244 - */ 245 211 ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_ADAPTOR_ID, 246 212 &adaptor_id, sizeof(adaptor_id)); 247 213 drm_dbg_kms(dev, "DP dual mode adaptor ID: %02x (err %zd)\n", adaptor_id, ret); ··· 243 233 return DRM_DP_DUAL_MODE_TYPE2_DVI; 244 234 } 245 235 /* 246 - * If neither a proper type 1 ID nor a broken type 1 adaptor 247 - * as described above, assume type 1, but let the user know 248 - * that we may have misdetected the type. 236 + * If not a proper type 1 ID, still assume type 1, but let 237 + * the user know that we may have misdetected the type. 249 238 */ 250 - if (!is_type1_adaptor(adaptor_id) && adaptor_id != hdmi_id[0]) 239 + if (!is_type1_adaptor(adaptor_id)) 251 240 drm_err(dev, "Unexpected DP dual mode adaptor ID %02x\n", adaptor_id); 252 241 253 242 } ··· 352 343 * @enable: enable (as opposed to disable) the TMDS output buffers 353 344 * 354 345 * Set the state of the TMDS output buffers in the adaptor. For 355 - * type2 this is set via the DP_DUAL_MODE_TMDS_OEN register. As 356 - * some type 1 adaptors have problems with registers (see comments 357 - * in drm_dp_dual_mode_detect()) we avoid touching the register, 358 - * making this function a no-op on type 1 adaptors. 346 + * type2 this is set via the DP_DUAL_MODE_TMDS_OEN register. 347 + * Type1 adaptors do not support any register writes. 359 348 * 360 349 * Returns: 361 350 * 0 on success, negative error code on failure