Re: [PATCHv2 15/22] drm/bridge: tc358767: clean-up link training

[Andrzej Hajda <a.hajda@samsung.com>]

On 26.03.2019 11:31, 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>

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


Btw, there is other patchset which touches the same things, have you
decided which one should go first? yours or Andrey's?


 --
Regards
Andrzej


> ---
>  drivers/gpu/drm/bridge/tc358767.c | 129 +++++++++++++-----------------
>  1 file changed, 57 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
> index 220408db82f7..1c61f6205e43 100644
> --- a/drivers/gpu/drm/bridge/tc358767.c
> +++ b/drivers/gpu/drm/bridge/tc358767.c
> @@ -740,83 +740,25 @@ 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, u32 *error)
>  {
> -	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");
> +		ret = -ETIMEDOUT;
> +		goto err;
>  	}
>  
> +	*error = (value >> 8) & 0x7;
> +
>  	return 0;
>  err:
>  	return ret;
> @@ -832,6 +774,7 @@ static int tc_main_link_enable(struct tc_data *tc)
>  	u32 value;
>  	int ret;
>  	u8 tmp[8];
> +	u32 error;
>  
>  	/* display mode should be set at this point */
>  	if (!tc->mode)
> @@ -950,14 +893,56 @@ 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);
> +	/* LINK TRAINING PATTERN 1 */
> +
> +	/* 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, &error);
>  	if (ret)
>  		goto err;
>  
> -	ret = tc_link_training(tc, DP_TRAINING_PATTERN_2);
> +	if (error) {
> +		dev_err(tc->dev, "Link training phase 1 failed: %s\n",
> +			training_pattern1_errors[error]);
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
> +	/* LINK TRAINING PATTERN 2 */
> +
> +	/* 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, &error);
>  	if (ret)
>  		goto err;
>  
> +	if (error) {
> +		dev_err(tc->dev, "Link training phase 2 failed: %s\n",
> +			training_pattern2_errors[error]);
> +		ret = -ENODEV;
> +		goto err;
> +	}
> +
>  	/* Clear Training Pattern, set AutoCorrect Mode = 1 */
>  	tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_AUTOCORRECT);
>  


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel