Re: [PATCH] drm/msm/dsi: dsi_phy_14nm: Take ready-bit into account in poll_for_ready

[Dmitry Baryshkov]

On 06/09/2021 23:25, Marijn Suijten wrote:

The downstream driver models this PLL lock check as an if-elseif-else.
The only way to reach the else case where pll_locked=true [1] is by
succeeding both readl_poll_timeout_atomic calls (which return zero on
success) in the if _and_ elseif condition.  Hence both the "lock" and
"ready" bit need to be tested in the SM_READY_STATUS register before
considering the PLL locked and ready to go.

Tested on the Sony Xperia XA2 Ultra (nile-discovery, sdm630).

[1]: 
https://source.codeaurora.org/quic/la/kernel/msm-4.19/tree/drivers/clk/qcom/mdss/mdss-dsi-pll-14nm-util.c?h=LA.UM.9.2.1.r1-08000-sdm660.0#n302

Fixes: f079f6d999cb ("drm/msm/dsi: Add PHY/PLL for 8x96")
Signed-off-by: Marijn Suijten <marijn.suij...@somainline.org>

Reviewed-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>

---
  drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 30 +++++++++++-----------
  1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c 
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
index 8905f365c932..789b08c24d25 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c
@@ -110,14 +110,13 @@ static struct dsi_pll_14nm *pll_14nm_list[DSI_MAX];
  static bool pll_14nm_poll_for_ready(struct dsi_pll_14nm *pll_14nm,
                                    u32 nb_tries, u32 timeout_us)
  {
-       bool pll_locked = false;
+       bool pll_locked = false, pll_ready = false;
        void __iomem *base = pll_14nm->phy->pll_base;
        u32 tries, val;

tries = nb_tries;

        while (tries--) {
-               val = dsi_phy_read(base +
-                              REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
+               val = dsi_phy_read(base + 
REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
                pll_locked = !!(val & BIT(5));

if (pll_locked)

@@ -126,23 +125,24 @@ static bool pll_14nm_poll_for_ready(struct dsi_pll_14nm 
*pll_14nm,
                udelay(timeout_us);
        }

- if (!pll_locked) {

-               tries = nb_tries;
-               while (tries--) {
-                       val = dsi_phy_read(base +
-                               REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
-                       pll_locked = !!(val & BIT(0));
+       if (!pll_locked)
+               goto out;

- if (pll_locked)

-                               break;
+       tries = nb_tries;
+       while (tries--) {
+               val = dsi_phy_read(base + 
REG_DSI_14nm_PHY_PLL_RESET_SM_READY_STATUS);
+               pll_ready = !!(val & BIT(0));

- udelay(timeout_us);

-               }
+               if (pll_ready)
+                       break;
+
+               udelay(timeout_us);
        }

- DBG("DSI PLL is %slocked", pll_locked ? "" : "*not* ");

+out:
+       DBG("DSI PLL is %slocked, %sready", pll_locked ? "" : "*not* ", pll_ready ? "" : 
"*not* ");

- return pll_locked;

+       return pll_locked && pll_ready;
  }

static void dsi_pll_14nm_config_init(struct dsi_pll_config *pconf)

--
With best wishes
Dmitry