diff mbox

[2/4] drm/dp/i2c: send bare addresses to properly reset i2c connections (v2)

Message ID 1396633926-10403-3-git-send-email-alexander.deucher@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Deucher April 4, 2014, 5:52 p.m. UTC
We need bare address packets at the start and end of
each i2c over aux transaction to properly reset the connection
between transactions.  This mirrors what the existing dp i2c
over aux algo currently does.

This fixes EDID fetches on certain monitors especially with
dp bridges.

v2: update as per Ville's comments
    - Set buffer to NULL for zero sized packets
    - abort the entre transaction if one of the messages fails

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 54 +++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 23 deletions(-)

Comments

Ville Syrjälä April 4, 2014, 6:46 p.m. UTC | #1
On Fri, Apr 04, 2014 at 01:52:04PM -0400, Alex Deucher wrote:
> We need bare address packets at the start and end of
> each i2c over aux transaction to properly reset the connection
> between transactions.  This mirrors what the existing dp i2c
> over aux algo currently does.
> 
> This fixes EDID fetches on certain monitors especially with
> dp bridges.
> 
> v2: update as per Ville's comments
>     - Set buffer to NULL for zero sized packets
>     - abort the entre transaction if one of the messages fails
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 54 +++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 74724aa..125f84d 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -664,12 +664,25 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>  			   int num)
>  {
>  	struct drm_dp_aux *aux = adapter->algo_data;
> -	unsigned int i, j;
> +	unsigned int m, b;
> +	struct drm_dp_aux_msg msg;
> +	int err = 0;
>  
> -	for (i = 0; i < num; i++) {
> -		struct drm_dp_aux_msg msg;
> -		int err;
> +	memset(&msg, 0, sizeof(msg));
>  
> +	for (m = 0; m < num; m++) {
> +		msg.address = msgs[m].addr;
> +		msg.request = (msgs[m].flags & I2C_M_RD) ?
> +			DP_AUX_I2C_READ :
> +			DP_AUX_I2C_WRITE;
> +		msg.request |= DP_AUX_I2C_MOT;
> +		msg.buffer = NULL;
> +		msg.size = 0;
> +		err = drm_dp_i2c_do_msg(aux, &msg);
> +		if (err < 0) {
> +			printk("error %d in bare address write\n", err);

I guess this printk was some leftover debug thing? Either should be
dropped or converted to some more appropriate DRM_ thing I suppose.

But otherwise the patch looks good:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +			break;
> +		}
>  		/*
>  		 * Many hardware implementations support FIFOs larger than a
>  		 * single byte, but it has been empirically determined that
> @@ -677,31 +690,26 @@ static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>  		 * decreased performance. Therefore each message is simply
>  		 * transferred byte-by-byte.
>  		 */
> -		for (j = 0; j < msgs[i].len; j++) {
> -			memset(&msg, 0, sizeof(msg));
> -			msg.address = msgs[i].addr;
> -
> -			msg.request = (msgs[i].flags & I2C_M_RD) ?
> -					DP_AUX_I2C_READ :
> -					DP_AUX_I2C_WRITE;
> -
> -			/*
> -			 * All messages except the last one are middle-of-
> -			 * transfer messages.
> -			 */
> -			if ((i < num - 1) || (j < msgs[i].len - 1))
> -				msg.request |= DP_AUX_I2C_MOT;
> -
> -			msg.buffer = msgs[i].buf + j;
> +		for (b = 0; b < msgs[m].len; b++) {
> +			msg.buffer = msgs[m].buf + b;
>  			msg.size = 1;
>  
>  			err = drm_dp_i2c_do_msg(aux, &msg);
>  			if (err < 0)
> -				return err;
> +				break;
>  		}
> +		if (err < 0)
> +			break;
>  	}
> -
> -	return num;
> +	if (err >= 0)
> +		err = num;
> +	/* send a bare address packet to close out the connection */
> +	msg.request &= ~DP_AUX_I2C_MOT;
> +	msg.buffer = NULL;
> +	msg.size = 0;
> +	(void)drm_dp_i2c_do_msg(aux, &msg);
> +
> +	return err;
>  }
>  
>  static const struct i2c_algorithm drm_dp_i2c_algo = {
> -- 
> 1.8.3.1
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 74724aa..125f84d 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -664,12 +664,25 @@  static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
 			   int num)
 {
 	struct drm_dp_aux *aux = adapter->algo_data;
-	unsigned int i, j;
+	unsigned int m, b;
+	struct drm_dp_aux_msg msg;
+	int err = 0;
 
-	for (i = 0; i < num; i++) {
-		struct drm_dp_aux_msg msg;
-		int err;
+	memset(&msg, 0, sizeof(msg));
 
+	for (m = 0; m < num; m++) {
+		msg.address = msgs[m].addr;
+		msg.request = (msgs[m].flags & I2C_M_RD) ?
+			DP_AUX_I2C_READ :
+			DP_AUX_I2C_WRITE;
+		msg.request |= DP_AUX_I2C_MOT;
+		msg.buffer = NULL;
+		msg.size = 0;
+		err = drm_dp_i2c_do_msg(aux, &msg);
+		if (err < 0) {
+			printk("error %d in bare address write\n", err);
+			break;
+		}
 		/*
 		 * Many hardware implementations support FIFOs larger than a
 		 * single byte, but it has been empirically determined that
@@ -677,31 +690,26 @@  static int drm_dp_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
 		 * decreased performance. Therefore each message is simply
 		 * transferred byte-by-byte.
 		 */
-		for (j = 0; j < msgs[i].len; j++) {
-			memset(&msg, 0, sizeof(msg));
-			msg.address = msgs[i].addr;
-
-			msg.request = (msgs[i].flags & I2C_M_RD) ?
-					DP_AUX_I2C_READ :
-					DP_AUX_I2C_WRITE;
-
-			/*
-			 * All messages except the last one are middle-of-
-			 * transfer messages.
-			 */
-			if ((i < num - 1) || (j < msgs[i].len - 1))
-				msg.request |= DP_AUX_I2C_MOT;
-
-			msg.buffer = msgs[i].buf + j;
+		for (b = 0; b < msgs[m].len; b++) {
+			msg.buffer = msgs[m].buf + b;
 			msg.size = 1;
 
 			err = drm_dp_i2c_do_msg(aux, &msg);
 			if (err < 0)
-				return err;
+				break;
 		}
+		if (err < 0)
+			break;
 	}
-
-	return num;
+	if (err >= 0)
+		err = num;
+	/* send a bare address packet to close out the connection */
+	msg.request &= ~DP_AUX_I2C_MOT;
+	msg.buffer = NULL;
+	msg.size = 0;
+	(void)drm_dp_i2c_do_msg(aux, &msg);
+
+	return err;
 }
 
 static const struct i2c_algorithm drm_dp_i2c_algo = {