On Fri, 09 Jan 2026, Mario Limonciello <[email protected]> wrote:
> On 12/3/25 8:46 PM, Chia-Lin Kao (AceLan) wrote:
>> Some USB-C hubs and adapters have buggy firmware where multi-byte AUX
>> reads consistently timeout, while single-byte reads from the same address
>> work correctly.
>>
>> Known affected devices that exhibit this issue:
>> - Lenovo USB-C to VGA adapter (VIA VL817 chipset)
>> idVendor=17ef, idProduct=7217
>> - Dell DA310 USB-C mobile adapter hub
>> idVendor=413c, idProduct=c010
>>
>> Analysis of the failure pattern shows:
>> - Single-byte probes to 0xf0000 (LTTPR) succeed
>> - Single-byte probes to 0x00102 (TRAINING_AUX_RD_INTERVAL) succeed
>> - Multi-byte reads from 0x00000 (DPCD capabilities) timeout with -ETIMEDOUT
>> - Retrying does not help - the failure is consistent across all attempts
>>
>> The issue appears to be a firmware bug in the AUX transaction handling
>> that specifically affects multi-byte reads.
>>
>> Add a fallback mechanism in drm_dp_dpcd_read_data() that attempts
>> byte-by-byte reading when the normal multi-byte read fails. This
>> workaround only activates for adapters that fail the standard read path,
>> ensuring no impact on correctly functioning hardware.
>>
>> Tested with:
>> - Lenovo USB-C to VGA adapter (VIA VL817) - now works with fallback
>> - Dell DA310 USB-C hub - now works with fallback
>> - Dell/Analogix Slimport adapter - continues to work with normal path
>>
>> Signed-off-by: Chia-Lin Kao (AceLan) <[email protected]>
>
> Reviewed-by: Mario Limonciello (AMD) <[email protected]>
>
> As this fixes reads for some existing hardware on the market and is just
> in fallback path I feel this is low risk. I've applied this to
> drm-misc-fixes.
I've stumbled on this when I was looking at drm_dp_dpcd_read_data(). I
never received the original patch, for whatever reason, even though Lore
says I was Cc'd.
> a8f49a0043011 (HEAD -> drm-misc-fixes) drm/dp: Add byte-by-byte fallback
> for broken USB-C adapters
>
>> ---
>> v2. 1. Move the workaround from intel_dp_read_dprx_caps() to
>> drm_dp_dpcd_read_data(), so that it applies to all DPCD reads across
>> all DRM drivers benefit from this fix, not just i915.
>> 2. Move the definition of drm_dp_dpcd_readb() before
>> drm_dp_dpcd_read_data()
>> ---
>> include/drm/display/drm_dp_helper.h | 57 +++++++++++++++++++----------
>> 1 file changed, 37 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/drm/display/drm_dp_helper.h
>> b/include/drm/display/drm_dp_helper.h
>> index df2f24b950e4..14d2859f0bda 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -551,6 +551,22 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux,
>> unsigned int offset,
>> ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned int offset,
>> void *buffer, size_t size);
>>
>> +/**
>> + * drm_dp_dpcd_readb() - read a single byte from the DPCD
>> + * @aux: DisplayPort AUX channel
>> + * @offset: address of the register to read
>> + * @valuep: location where the value of the register will be stored
>> + *
>> + * Returns the number of bytes transferred (1) on success, or a negative
>> + * error code on failure. In most of the cases you should be using
>> + * drm_dp_dpcd_read_byte() instead.
>> + */
>> +static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux,
>> + unsigned int offset, u8 *valuep)
>> +{
>> + return drm_dp_dpcd_read(aux, offset, valuep, 1);
>> +}
>> +
>> /**
>> * drm_dp_dpcd_read_data() - read a series of bytes from the DPCD
>> * @aux: DisplayPort AUX channel (SST or MST)
>> @@ -570,12 +586,29 @@ static inline int drm_dp_dpcd_read_data(struct
>> drm_dp_aux *aux,
>> void *buffer, size_t size)
>> {
>> int ret;
>> + size_t i;
>> + u8 *buf = buffer;
>>
>> ret = drm_dp_dpcd_read(aux, offset, buffer, size);
>> - if (ret < 0)
>> - return ret;
>> - if (ret < size)
>> - return -EPROTO;
>> + if (ret >= 0) {
>> + if (ret < size)
>> + return -EPROTO;
>> + return 0;
>> + }
>> +
>> + /*
>> + * Workaround for USB-C hubs/adapters with buggy firmware that fail
>> + * multi-byte AUX reads but work with single-byte reads.
>> + * Known affected devices:
>> + * - Lenovo USB-C to VGA adapter (VIA VL817, idVendor=17ef,
>> idProduct=7217)
>> + * - Dell DA310 USB-C hub (idVendor=413c, idProduct=c010)
>> + * Attempt byte-by-byte reading as a fallback.
>> + */
>> + for (i = 0; i < size; i++) {
>> + ret = drm_dp_dpcd_readb(aux, offset + i, &buf[i]);
drm_dp_dpcd_read_byte() should be preferred over drm_dp_dpcd_readb()...
>> + if (ret < 0)
...because drm_dp_dpcd_readb() might return 0 on failures. You need to
use drm_dp_dpcd_readb() == 1 to check for success, which is why
drm_dp_dpcd_read_byte() and drm_dp_dpcd_read_data() were introduced in
the first place.
Moreover, this ugly workaround only impacts drm_dp_dpcd_read_data()
callers, but there are lots and lots of direct drm_dp_dpcd_read() calls
all over the place, which go unfixed.
It should be emphasized that DP AUX changes that affect absolutely all
drivers should go through more scrutiny, and require more acks.
This needs follow-up fixes.
BR,
Jani.
>> + return ret;
>> + }
>>
>> return 0;
>> }
>> @@ -609,22 +642,6 @@ static inline int drm_dp_dpcd_write_data(struct
>> drm_dp_aux *aux,
>> return 0;
>> }
>>
>> -/**
>> - * drm_dp_dpcd_readb() - read a single byte from the DPCD
>> - * @aux: DisplayPort AUX channel
>> - * @offset: address of the register to read
>> - * @valuep: location where the value of the register will be stored
>> - *
>> - * Returns the number of bytes transferred (1) on success, or a negative
>> - * error code on failure. In most of the cases you should be using
>> - * drm_dp_dpcd_read_byte() instead.
>> - */
>> -static inline ssize_t drm_dp_dpcd_readb(struct drm_dp_aux *aux,
>> - unsigned int offset, u8 *valuep)
>> -{
>> - return drm_dp_dpcd_read(aux, offset, valuep, 1);
>> -}
>> -
>> /**
>> * drm_dp_dpcd_writeb() - write a single byte to the DPCD
>> * @aux: DisplayPort AUX channel
>
--
Jani Nikula, Intel
