ville.syrjala at linux.intel.com writes:
> From: Ville Syrjälä
>
> Calculate the number of retries we should do for each i2c-over-aux
> message based on the time it takes to perform the i2c transfer vs. the
> aux transfer.
>
> This mostly matches what the DP spec recommends. The numbers didn't come
> out exactly the same as the tables in the spec, but I think there's
> something fishy about the AUX trasnfer size calculations in the spec.
> Also the way the i2c transfer length is calculated is somewhat open to
> interpretation.
>
> Note that currently we assume 10 kHz speed for the i2c bus. Some real
> world devices (eg. some Apple DP->VGA dongle) fails with less than 16
> retries, and that would correspond to something close to 20 kHz, so we
> assume 10 kHz to be on the safe side. Ideally we should query/set the
> i2c bus speed via DPCD but for now this should at leaast remove the
> regression from the 1->16 byte trasnfer size change. And of course if
> the sink completes the transfer quicker this shouldn't slow things down
> since we don't change the interval between retries.
>
> Fixes a regression with some DP dongles from:
> commit 1d002fa720738bcd0bddb9178e9ea0773288e1dd
> Author: Simon Farnsworth
> Date: Tue Feb 10 18:38:08 2015 +
>
> drm/dp: Use large transactions for I2C over AUX
>
> Cc: Simon Farnsworth
> Cc: moosotc at gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91451
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_dp_helper.c | 64
> +++--
> 1 file changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 80a02a4..2b6b7f9 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -422,6 +422,61 @@ static u32 drm_dp_i2c_functionality(struct i2c_adapter
> *adapter)
> I2C_FUNC_10BIT_ADDR;
> }
>
> +#define AUX_PRECHARGE_LEN 16 /* 10 to 16 */
> +#define AUX_SYNC_LEN (16 + 4) /* preamble + AUX_SYNC_END */
> +#define AUX_STOP_LEN 4
> +#define AUX_CMD_LEN 4
> +#define AUX_ADDRESS_LEN 20
> +#define AUX_REPLY_PAD_LEN 4
> +#define AUX_LENGTH_LEN 8
> +
> +#define AUX_RESPONSE_TIMEOUT 300
> +
> +static int drm_dp_aux_req_len(const struct drm_dp_aux_msg *msg)
> +{
> + int len = AUX_PRECHARGE_LEN + AUX_SYNC_LEN + AUX_STOP_LEN +
> + AUX_CMD_LEN + AUX_ADDRESS_LEN + AUX_LENGTH_LEN;
> +
> + if ((msg->request & DP_AUX_I2C_READ) == 0)
> + len += msg->size * 8;
> +
> + return len;
> +}
> +
> +static int drm_dp_aux_reply_len(const struct drm_dp_aux_msg *msg)
> +{
> + int len = AUX_PRECHARGE_LEN + AUX_SYNC_LEN + AUX_STOP_LEN +
> + AUX_CMD_LEN + AUX_REPLY_PAD_LEN;
> +
> + if (msg->request & DP_AUX_I2C_READ)
> + len += msg->size * 8;
> + else
> + len += 8; /* 0 or 1 data bytes for write reply */
> +
> + return len;
> +}
> +
> +#define I2C_START_LEN 1
> +#define I2C_STOP_LEN 1
> +#define I2C_ADDR_LEN 9 /* ADDRESS + R/W + ACK/NACK */
> +#define I2C_DATA_LEN 9 /* DATA + ACK/NACK */
> +
> +static int drm_dp_i2c_msg_len(const struct drm_dp_aux_msg *msg,
> + int i2c_speed_khz)
> +{
> + return (I2C_START_LEN + I2C_ADDR_LEN + msg->size * I2C_DATA_LEN +
> + I2C_STOP_LEN) * 1000 / i2c_speed_khz;
> +}
> +
> +static int drm_dp_i2c_retry_count(const struct drm_dp_aux_msg *msg,
> + int i2c_speed_khz)
> +{
> + int aux_len = drm_dp_aux_req_len(msg) + drm_dp_aux_reply_len(msg) +
> AUX_RESPONSE_TIMEOUT;
> + int i2c_len = drm_dp_i2c_msg_len(msg, i2c_speed_khz) -
> AUX_RESPONSE_TIMEOUT;
> +
> + return DIV_ROUND_UP(i2c_len, aux_len);
> +}
> +
> /*
> * Transfer a single I2C-over-AUX message and handle various error
> conditions,
> * retrying the transaction as appropriate. It is assumed that the
> @@ -434,13 +489,18 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux,
> struct drm_dp_aux_msg *msg)
> {
> unsigned int retry, defer_i2c;
> int ret;
> -
> /*
>* DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source device
>* is required to retry at least seven times upon receiving AUX_DEFER
>* before giving up the AUX transaction.
> + *
> + * We also try to account for the i2c bus speed.
> + * FIXME currently assumes 10 kHz as some real world devices seem
> + * to require it. We should query/set the speed via DPCD if supported.
>*/
> - for (retry = 0, defer_i2c = 0; retry < (7 + defer_i2c); retry++) {
> + int max_retries = max(7, drm_dp_i2c_retry_count(msg, 10));
> +
> + for (retry = 0, defer_i2c = 0; retry < (max_retries + defer_i2c);
> retry++) {
> mutex_lock(>hw_mutex);
> ret = aux->transfer(aux, msg);
> mutex_unlock(>hw_mutex);
Tested-by: moosotc at gmail.com
--
mailto:moosotc at gmail.com
