ville.syrjala at linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> 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 <simon.farnsworth at onelan.co.uk>
> Date:   Tue Feb 10 18:38:08 2015 +0000
>
>     drm/dp: Use large transactions for I2C over AUX
>
> Cc: Simon Farnsworth <simon.farnsworth at onelan.com>
> Cc: moosotc at gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91451
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  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(&aux->hw_mutex);
>               ret = aux->transfer(aux, msg);
>               mutex_unlock(&aux->hw_mutex);

Tested-by: moosotc at gmail.com

-- 
mailto:moosotc at gmail.com

Reply via email to