| Message ID | 20190503122949.12266-17-tomi.valkeinen@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/bridge: tc358767: DP support | expand |
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 --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
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(-)