| Message ID | 20190326103146.24795-16-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
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); >
On 15/04/2019 12:54, Andrzej Hajda wrote: > 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? Yes, we discussed that in "[PATCH v2 05/15] drm/bridge: tc358767: Drop custom tc_write()/tc_read() accessors" thread. Let's merge mine first, and Andrey rebases on top of that. Tomi
Hi Tomi, Thank you for the patch. On Tue, Mar 26, 2019 at 12:31:39PM +0200, 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 | 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; You can return -ETIMEDOUT directly. > } > > + *error = (value >> 8) & 0x7; How about returning the status through the return value instead ? The status will never be negative, so it won't clash with -ETIMEDOUT. > + > 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 */ While at it, could you define macros for these bits ? > + > + tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS | DP0_SRCCTRL_AUTOCORRECT | > + DP0_SRCCTRL_TP1); Let's break long lines (here and in other locations in this patch). > + > + /* 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 */ Do phase 1 and phase 2 correspond to clock recovery and channel equalization ? If so I would mention that instead of just training pattern 1 and 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); > + This channel equalization sequence of link training has a retry loop of 5 iterations. It that performed internally by the TC358767 ? > + /* 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); >
On 21/04/2019 01:13, Laurent Pinchart wrote: > Hi Tomi, > > Thank you for the patch. > > On Tue, Mar 26, 2019 at 12:31:39PM +0200, 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 | 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; > > You can return -ETIMEDOUT directly. Yep. >> } >> >> + *error = (value >> 8) & 0x7; > > How about returning the status through the return value instead ? The > status will never be negative, so it won't clash with -ETIMEDOUT. Hmm, yes, I think that makes sense here. >> + >> 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 */ > > While at it, could you define macros for these bits ? For the shifts? We don't have macros for almost any of the shifts. I'd rather leave such changes for later. This one is just a copy of the existing code. >> + >> + tc_write(DP0_SRCCTRL, tc_srcctrl(tc) | DP0_SRCCTRL_SCRMBLDIS | DP0_SRCCTRL_AUTOCORRECT | >> + DP0_SRCCTRL_TP1); > > Let's break long lines (here and in other locations in this patch). Ok. >> + >> + /* 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 */ > > Do phase 1 and phase 2 correspond to clock recovery and channel > equalization ? If so I would mention that instead of just training > pattern 1 and 2. Yes. All the docs talk about pattern 1 and 2, including DP 1.1a doc. But I agree, probably these comments should talk about clock recovery and channel eq. >> + >> + /* 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); >> + > > This channel equalization sequence of link training has a retry loop of > 5 iterations. It that performed internally by the TC358767 ? Yes, that is my understanding. It's the "Loop Iteration Count" in DP0_LTLOOPCTRL. Tomi
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);
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 | 129 +++++++++++++----------------- 1 file changed, 57 insertions(+), 72 deletions(-)