On Fri, May 3, 2024 at 5:12 PM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote:
>
> Move the wait loop into its own function, so it doesn't need to be
> replicated in multiple locations. Also move the PLL lock checks between
> setting the link bandwidth, which may cause the PLL to unlock, and the
> MACRO_RST, which needs the PLL to be locked.
>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> ---
> .../drm/bridge/analogix/analogix_dp_core.c | 34 +++++++------------
> .../drm/bridge/analogix/analogix_dp_core.h | 7 +---
> .../gpu/drm/bridge/analogix/analogix_dp_reg.c | 12 +++----
> 3 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 736b2ed745e1..7bbc3d8a85df 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -231,7 +231,7 @@ static int analogix_dp_training_pattern_dis(struct analogix_dp_device *dp)
> static int analogix_dp_link_start(struct analogix_dp_device *dp)
> {
> u8 buf[4];
> - int lane, lane_count, pll_tries, retval;
> + int lane, lane_count, retval;
>
> lane_count = dp->link_train.lane_count;
>
> @@ -243,6 +243,11 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
>
> /* Set link rate and count as you want to establish*/
> analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> + retval = analogix_dp_wait_pll_locked(dp);
> + if (retval) {
> + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", retval);
> + return retval;
> + }
> /*
> * MACRO_RST must be applied after the PLL_LOCK to avoid
> * the DP inter pair skew issue for at least 10 us
> @@ -270,18 +275,6 @@ static int analogix_dp_link_start(struct analogix_dp_device *dp)
> DP_TRAIN_PRE_EMPH_LEVEL_0;
> analogix_dp_set_lane_link_training(dp);
>
> - /* Wait for PLL lock */
> - pll_tries = 0;
> - while (analogix_dp_get_pll_lock_status(dp) == PLL_UNLOCKED) {
> - if (pll_tries == DP_TIMEOUT_LOOP_COUNT) {
> - dev_err(dp->dev, "Wait for PLL lock timed out\n");
> - return -ETIMEDOUT;
> - }
> -
> - pll_tries++;
> - usleep_range(90, 120);
> - }
> -
> /* Set training pattern 1 */
> analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
>
> @@ -634,9 +627,14 @@ static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
> {
> int ret;
> u8 link_align, link_status[2];
> - enum pll_status status;
>
> analogix_dp_set_link_bandwidth(dp, dp->link_train.link_rate);
> + ret = analogix_dp_wait_pll_locked(dp);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);
> + return ret;
> + }
> +
> /*
> * MACRO_RST must be applied after the PLL_LOCK to avoid
> * the DP inter pair skew issue for at least 10 us
> @@ -645,14 +643,6 @@ static int analogix_dp_fast_link_train(struct analogix_dp_device *dp)
> analogix_dp_set_lane_count(dp, dp->link_train.lane_count);
> analogix_dp_set_lane_link_training(dp);
>
> - ret = readx_poll_timeout(analogix_dp_get_pll_lock_status, dp, status,
> - status != PLL_UNLOCKED, 120,
> - 120 * DP_TIMEOUT_LOOP_COUNT);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Wait for pll lock failed %d\n", ret);
> - return ret;
> - }
> -
> /* source Set training pattern 1 */
> analogix_dp_set_training_pattern(dp, TRAINING_PTN1);
> /* From DP spec, pattern must be on-screen for a minimum 500us */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index 382b2f068ab9..774d11574b09 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -95,11 +95,6 @@ enum dynamic_range {
> CEA
> };
>
> -enum pll_status {
> - PLL_UNLOCKED,
> - PLL_LOCKED
> -};
> -
> enum clock_recovery_m_value_type {
> CALCULATED_M,
> REGISTER_M
> @@ -191,7 +186,7 @@ void analogix_dp_swreset(struct analogix_dp_device *dp);
> void analogix_dp_config_interrupt(struct analogix_dp_device *dp);
> void analogix_dp_mute_hpd_interrupt(struct analogix_dp_device *dp);
> void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp);
> -enum pll_status analogix_dp_get_pll_lock_status(struct analogix_dp_device *dp);
> +int analogix_dp_wait_pll_locked(struct analogix_dp_device *dp);
> void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable);
> void analogix_dp_set_analog_power_down(struct analogix_dp_device *dp,
> enum analog_power_block block,
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index e9c643a8b6fc..143a78b1d156 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -217,15 +217,13 @@ void analogix_dp_unmute_hpd_interrupt(struct analogix_dp_device *dp)
> writel(reg, dp->reg_base + ANALOGIX_DP_INT_STA_MASK);
> }
>
> -enum pll_status analogix_dp_get_pll_lock_status(struct analogix_dp_device *dp)
> +int analogix_dp_wait_pll_locked(struct analogix_dp_device *dp)
> {
> - u32 reg;
> + u32 val;
>
> - reg = readl(dp->reg_base + ANALOGIX_DP_DEBUG_CTL);
> - if (reg & PLL_LOCK)
> - return PLL_LOCKED;
> - else
> - return PLL_UNLOCKED;
> + return readl_poll_timeout(dp->reg_base + ANALOGIX_DP_DEBUG_CTL, val,
> + val & PLL_LOCK, 120,
> + 120 * DP_TIMEOUT_LOOP_COUNT);
> }
>
> void analogix_dp_set_pll_power_down(struct analogix_dp_device *dp, bool enable)
> --
> 2.39.2
>
Reviewed-by: Robert Foss <rfoss@xxxxxxxxxx>
