Attachment 117909 Details for Bug 91451 – [PATCH] drm/dp: Adjust i2c-over-aux retry count based on message size and i2c bus speed

[patch] [PATCH] drm/dp: Adjust i2c-over-aux retry count based on message size and i2c bus speed

0001-drm-dp-Adjust-i2c-over-aux-retry-count-based-on-mess.patch (text/plain), 4.71 KB, created by Ville Syrjala on 2015-08-25 14:59:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ville Syrjala

Created: 2015-08-25 14:59:34 UTC

Size: 4.71 KB

patch

obsolete

>From 2909da79a4c059647f7ebbfcb670360e6cbcaee4 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
>Date: Tue, 25 Aug 2015 17:13:55 +0300
>Subject: [PATCH] drm/dp: Adjust i2c-over-aux retry count based on message size
> and i2c bus speed
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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@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@onelan.com>
>Cc: moosotc@gmail.coma
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91451
>Signed-off-by: Ville SyrjÃ¤lÃ¤ <ville.syrjala@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);
>-- 
>2.4.6
>

Actions: View | Splinter Review

Attachments on bug 91451: 117341 | 117909

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.