Re: [PATCHv3 16/23] drm/bridge: tc358767: clean-up link training
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
> ---
> 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, );
> - 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, );
> + } 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);
> +
[PATCHv3 16/23] drm/bridge: tc358767: clean-up link training
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
---
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, );
- 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, );
+ } 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
