diff mbox series

[12/14] drm/bridge: analogix_dp: simplify and correct PLL lock checks

Message ID 20240503151129.3901815-13-l.stach@pengutronix.de (mailing list archive)
State New
Headers show
Series improve Analogix DP AUX channel handling | expand

Commit Message

Lucas Stach May 3, 2024, 3:11 p.m. UTC
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@pengutronix.de>
---
 .../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(-)

Comments

Robert Foss June 10, 2024, 11:55 a.m. UTC | #1
On Fri, May 3, 2024 at 5:12 PM Lucas Stach <l.stach@pengutronix.de> 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@pengutronix.de>
> ---
>  .../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@kernel.org>
diff mbox series

Patch

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)