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.valkei...@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.ha...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
