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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux