diff mbox series

[PATCHv3,16/23] drm/bridge: tc358767: clean-up link training

Message ID 20190503122949.12266-17-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show
Series drm/bridge: tc358767: DP support | expand

Commit Message

Tomi Valkeinen May 3, 2019, 12:29 p.m. UTC
The current link training code does unnecessary retry-loops, and does
extra writes to the registers. It is easier to follow the flow and
ensure it's similar to Toshiba's documentation if we deal with LT inside
tc_main_link_enable() function.

This patch adds tc_wait_link_training() which handles waiting for the LT
phase to finish, and does the necessary LT register setups in
tc_main_link_enable, without extra loops.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/bridge/tc358767.c | 136 +++++++++++++-----------------
 1 file changed, 60 insertions(+), 76 deletions(-)

Comments

Andrzej Hajda May 20, 2019, 11:29 a.m. UTC | #1
On 03.05.2019 14:29, Tomi Valkeinen wrote:
> The current link training code does unnecessary retry-loops, and does
> extra writes to the registers. It is easier to follow the flow and
> ensure it's similar to Toshiba's documentation if we deal with LT inside
> tc_main_link_enable() function.
>
> This patch adds tc_wait_link_training() which handles waiting for the LT
> phase to finish, and does the necessary LT register setups in
> tc_main_link_enable, without extra loops.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/bridge/tc358767.c | 136 +++++++++++++-----------------
>  1 file changed, 60 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 2cec06520fcf..86175e8d01b3 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -740,84 +740,24 @@ static int tc_set_video_mode(struct tc_data *tc,
>  	return ret;
>  }
>  
> -static int tc_link_training(struct tc_data *tc, int pattern)
> +static int tc_wait_link_training(struct tc_data *tc)
>  {
> -	const char * const *errors;
> -	u32 srcctrl = tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
> -		      DP0_SRCCTRL_AUTOCORRECT;
> -	int timeout;
> -	int retry;
> +	u32 timeout = 1000;
>  	u32 value;
>  	int ret;
>  
> -	if (pattern == DP_TRAINING_PATTERN_1) {
> -		srcctrl |= DP0_SRCCTRL_TP1;
> -		errors = training_pattern1_errors;
> -	} else {
> -		srcctrl |= DP0_SRCCTRL_TP2;
> -		errors = training_pattern2_errors;
> -	}
> -
> -	/* Set DPCD 0x102 for Training Part 1 or 2 */
> -	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE | pattern);
> -
> -	tc_write(DP0_LTLOOPCTRL,
> -		 (0x0f << 28) |	/* Defer Iteration Count */
> -		 (0x0f << 24) |	/* Loop Iteration Count */
> -		 (0x0d << 0));	/* Loop Timer Delay */
> -
> -	retry = 5;
>  	do {
> -		/* Set DP0 Training Pattern */
> -		tc_write(DP0_SRCCTRL, srcctrl);
> -
> -		/* Enable DP0 to start Link Training */
> -		tc_write(DP0CTL, DP_EN);
> -
> -		/* wait */
> -		timeout = 1000;
> -		do {
> -			tc_read(DP0_LTSTAT, &value);
> -			udelay(1);
> -		} while ((!(value & LT_LOOPDONE)) && (--timeout));
> -		if (timeout == 0) {
> -			dev_err(tc->dev, "Link training timeout!\n");
> -		} else {
> -			int pattern = (value >> 11) & 0x3;
> -			int error = (value >> 8) & 0x7;
> -
> -			dev_dbg(tc->dev,
> -				"Link training phase %d done after %d uS: %s\n",
> -				pattern, 1000 - timeout, errors[error]);
> -			if (pattern == DP_TRAINING_PATTERN_1 && error == 0)
> -				break;
> -			if (pattern == DP_TRAINING_PATTERN_2) {
> -				value &= LT_CHANNEL1_EQ_BITS |
> -					 LT_INTERLANE_ALIGN_DONE |
> -					 LT_CHANNEL0_EQ_BITS;
> -				/* in case of two lanes */
> -				if ((tc->link.base.num_lanes == 2) &&
> -				    (value == (LT_CHANNEL1_EQ_BITS |
> -					       LT_INTERLANE_ALIGN_DONE |
> -					       LT_CHANNEL0_EQ_BITS)))
> -					break;
> -				/* in case of one line */
> -				if ((tc->link.base.num_lanes == 1) &&
> -				    (value == (LT_INTERLANE_ALIGN_DONE |
> -					       LT_CHANNEL0_EQ_BITS)))
> -					break;
> -			}


As I understand all above checks can be dropped.

> -		}
> -		/* restart */
> -		tc_write(DP0CTL, 0);
> -		usleep_range(10, 20);
> -	} while (--retry);
> -	if (retry == 0) {
> -		dev_err(tc->dev, "Failed to finish training phase %d\n",
> -			pattern);
> +		udelay(1);
> +		tc_read(DP0_LTSTAT, &value);
> +	} while ((!(value & LT_LOOPDONE)) && (--timeout));
> +
> +	if (timeout == 0) {
> +		dev_err(tc->dev, "Link training timeout waiting for LT_LOOPDONE!\n");
> +		return -ETIMEDOUT;
>  	}
>  
> -	return 0;
> +	return (value >> 8) & 0x7;
> +
>  err:
>  	return ret;
>  }
> @@ -927,7 +867,7 @@ static int tc_main_link_enable(struct tc_data *tc)
>  
>  		if (tmp[0] != tc->assr) {
>  			dev_dbg(dev, "Failed to switch display ASSR to %d, falling back to unscrambled mode\n",
> -				 tc->assr);
> +				tc->assr);
>  			/* trying with disabled scrambler */
>  			tc->link.scrambler_dis = true;
>  		}
> @@ -953,13 +893,57 @@ static int tc_main_link_enable(struct tc_data *tc)
>  	if (ret < 0)
>  		goto err_dpcd_write;
>  
> -	ret = tc_link_training(tc, DP_TRAINING_PATTERN_1);
> -	if (ret)
> +	/* Clock-Recovery */
> +
> +	/* Set DPCD 0x102 for Training Pattern 1 */
> +	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE |
> +		 DP_TRAINING_PATTERN_1);
> +
> +	tc_write(DP0_LTLOOPCTRL,
> +		 (15 << 28) |	/* Defer Iteration Count */
> +		 (15 << 24) |	/* Loop Iteration Count */
> +		 (0xd << 0));	/* Loop Timer Delay */
> +
> +	tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
> +		 DP0_SRCCTRL_AUTOCORRECT | DP0_SRCCTRL_TP1);
> +
> +	/* Enable DP0 to start Link Training */
> +	tc_write(DP0CTL,
> +		 ((tc->link.base.capabilities & DP_LINK_CAP_ENHANCED_FRAMING) ? EF_EN : 0) |
> +		 DP_EN);
> +
> +	/* wait */
> +	ret = tc_wait_link_training(tc);
> +	if (ret < 0)
>  		goto err;
>  
> -	ret = tc_link_training(tc, DP_TRAINING_PATTERN_2);
> -	if (ret)
> +	if (ret) {
> +		dev_err(tc->dev, "Link training phase 1 failed: %s\n",
> +			training_pattern1_errors[ret]);
> +		ret = -ENODEV;
>  		goto err;
> +	}
> +
> +	/* Channel Equalization */
> +
> +	/* Set DPCD 0x102 for Training Pattern 2 */
> +	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE |
> +		 DP_TRAINING_PATTERN_2);
> +
> +	tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
> +		 DP0_SRCCTRL_AUTOCORRECT | DP0_SRCCTRL_TP2);
> +
> +	/* wait */
> +	ret = tc_wait_link_training(tc);
> +	if (ret < 0)
> +		goto err;
> +
> +	if (ret) {
> +		dev_err(tc->dev, "Link training phase 2 failed: %s\n",
> +			training_pattern2_errors[ret]);
> +		ret = -ENODEV;
> +		goto err;
> +	}


In general it seems to be correct, but I could miss something - too many
changes for me :)

Anyway:

Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>


But it would be good to have Tested-by tag, if possible?

With that I guess the patchset can be merged.


Laurent are you OK with it?


 --
Regards
Andrzej


>  
>  	/*
>  	 * Toshiba's documentation suggests to first clear DPCD 0x102, then
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 2cec06520fcf..86175e8d01b3 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -740,84 +740,24 @@  static int tc_set_video_mode(struct tc_data *tc,
 	return ret;
 }
 
-static int tc_link_training(struct tc_data *tc, int pattern)
+static int tc_wait_link_training(struct tc_data *tc)
 {
-	const char * const *errors;
-	u32 srcctrl = tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
-		      DP0_SRCCTRL_AUTOCORRECT;
-	int timeout;
-	int retry;
+	u32 timeout = 1000;
 	u32 value;
 	int ret;
 
-	if (pattern == DP_TRAINING_PATTERN_1) {
-		srcctrl |= DP0_SRCCTRL_TP1;
-		errors = training_pattern1_errors;
-	} else {
-		srcctrl |= DP0_SRCCTRL_TP2;
-		errors = training_pattern2_errors;
-	}
-
-	/* Set DPCD 0x102 for Training Part 1 or 2 */
-	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE | pattern);
-
-	tc_write(DP0_LTLOOPCTRL,
-		 (0x0f << 28) |	/* Defer Iteration Count */
-		 (0x0f << 24) |	/* Loop Iteration Count */
-		 (0x0d << 0));	/* Loop Timer Delay */
-
-	retry = 5;
 	do {
-		/* Set DP0 Training Pattern */
-		tc_write(DP0_SRCCTRL, srcctrl);
-
-		/* Enable DP0 to start Link Training */
-		tc_write(DP0CTL, DP_EN);
-
-		/* wait */
-		timeout = 1000;
-		do {
-			tc_read(DP0_LTSTAT, &value);
-			udelay(1);
-		} while ((!(value & LT_LOOPDONE)) && (--timeout));
-		if (timeout == 0) {
-			dev_err(tc->dev, "Link training timeout!\n");
-		} else {
-			int pattern = (value >> 11) & 0x3;
-			int error = (value >> 8) & 0x7;
-
-			dev_dbg(tc->dev,
-				"Link training phase %d done after %d uS: %s\n",
-				pattern, 1000 - timeout, errors[error]);
-			if (pattern == DP_TRAINING_PATTERN_1 && error == 0)
-				break;
-			if (pattern == DP_TRAINING_PATTERN_2) {
-				value &= LT_CHANNEL1_EQ_BITS |
-					 LT_INTERLANE_ALIGN_DONE |
-					 LT_CHANNEL0_EQ_BITS;
-				/* in case of two lanes */
-				if ((tc->link.base.num_lanes == 2) &&
-				    (value == (LT_CHANNEL1_EQ_BITS |
-					       LT_INTERLANE_ALIGN_DONE |
-					       LT_CHANNEL0_EQ_BITS)))
-					break;
-				/* in case of one line */
-				if ((tc->link.base.num_lanes == 1) &&
-				    (value == (LT_INTERLANE_ALIGN_DONE |
-					       LT_CHANNEL0_EQ_BITS)))
-					break;
-			}
-		}
-		/* restart */
-		tc_write(DP0CTL, 0);
-		usleep_range(10, 20);
-	} while (--retry);
-	if (retry == 0) {
-		dev_err(tc->dev, "Failed to finish training phase %d\n",
-			pattern);
+		udelay(1);
+		tc_read(DP0_LTSTAT, &value);
+	} while ((!(value & LT_LOOPDONE)) && (--timeout));
+
+	if (timeout == 0) {
+		dev_err(tc->dev, "Link training timeout waiting for LT_LOOPDONE!\n");
+		return -ETIMEDOUT;
 	}
 
-	return 0;
+	return (value >> 8) & 0x7;
+
 err:
 	return ret;
 }
@@ -927,7 +867,7 @@  static int tc_main_link_enable(struct tc_data *tc)
 
 		if (tmp[0] != tc->assr) {
 			dev_dbg(dev, "Failed to switch display ASSR to %d, falling back to unscrambled mode\n",
-				 tc->assr);
+				tc->assr);
 			/* trying with disabled scrambler */
 			tc->link.scrambler_dis = true;
 		}
@@ -953,13 +893,57 @@  static int tc_main_link_enable(struct tc_data *tc)
 	if (ret < 0)
 		goto err_dpcd_write;
 
-	ret = tc_link_training(tc, DP_TRAINING_PATTERN_1);
-	if (ret)
+	/* Clock-Recovery */
+
+	/* Set DPCD 0x102 for Training Pattern 1 */
+	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE |
+		 DP_TRAINING_PATTERN_1);
+
+	tc_write(DP0_LTLOOPCTRL,
+		 (15 << 28) |	/* Defer Iteration Count */
+		 (15 << 24) |	/* Loop Iteration Count */
+		 (0xd << 0));	/* Loop Timer Delay */
+
+	tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
+		 DP0_SRCCTRL_AUTOCORRECT | DP0_SRCCTRL_TP1);
+
+	/* Enable DP0 to start Link Training */
+	tc_write(DP0CTL,
+		 ((tc->link.base.capabilities & DP_LINK_CAP_ENHANCED_FRAMING) ? EF_EN : 0) |
+		 DP_EN);
+
+	/* wait */
+	ret = tc_wait_link_training(tc);
+	if (ret < 0)
 		goto err;
 
-	ret = tc_link_training(tc, DP_TRAINING_PATTERN_2);
-	if (ret)
+	if (ret) {
+		dev_err(tc->dev, "Link training phase 1 failed: %s\n",
+			training_pattern1_errors[ret]);
+		ret = -ENODEV;
 		goto err;
+	}
+
+	/* Channel Equalization */
+
+	/* Set DPCD 0x102 for Training Pattern 2 */
+	tc_write(DP0_SNKLTCTRL, DP_LINK_SCRAMBLING_DISABLE |
+		 DP_TRAINING_PATTERN_2);
+
+	tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS |
+		 DP0_SRCCTRL_AUTOCORRECT | DP0_SRCCTRL_TP2);
+
+	/* wait */
+	ret = tc_wait_link_training(tc);
+	if (ret < 0)
+		goto err;
+
+	if (ret) {
+		dev_err(tc->dev, "Link training phase 2 failed: %s\n",
+			training_pattern2_errors[ret]);
+		ret = -ENODEV;
+		goto err;
+	}
 
 	/*
 	 * Toshiba's documentation suggests to first clear DPCD 0x102, then