ville.syrjala@xxxxxxxxxxxxxxx writes: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxx> > Date: Tue Feb 10 18:38:08 2015 +0000 > > drm/dp: Use large transactions for I2C over AUX > > Cc: Simon Farnsworth <simon.farnsworth@xxxxxxxxxx> > Cc: moosotc@xxxxxxxxx > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91451 > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxx -- mailto:moosotc@xxxxxxxxx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
