From: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
[ Upstream commit 5ddcb0cb9d10e6e70a68e0cb8f0b8e3a7eb8ccaf ]
Driver unconditionally saves current state on first init in dsi_pll_7nm_init(), but does not save the VCO rate, only some of the divider registers. The state is then restored during probe/enable via msm_dsi_phy_enable() -> msm_dsi_phy_pll_restore_state() -> dsi_7nm_pll_restore_state().
Restoring calls dsi_pll_7nm_vco_set_rate() with pll_7nm->vco_current_rate=0, which basically overwrites existing rate of VCO and messes with clock hierarchy, by setting frequency to 0 to clock tree. This makes anyway little sense - VCO rate was not saved, so should not be restored.
If PLL was not configured configure it to minimum rate to avoid glitches and configuring entire in clock hierarchy to 0 Hz.
Reviewed-by: Dmitry Baryshkov dmitry.baryshkov@oss.qualcomm.com Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Patchwork: https://patchwork.freedesktop.org/patch/657827/ Link: https://lore.kernel.org/r/20250610-b4-sm8750-display-v6-9-ee633e3ddbff@linar... Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@oss.qualcomm.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- Bug fixed: On first init the driver saves PLL state but not the VCO rate, so the subsequent restore path programs the VCO to 0 Hz, breaking the clock tree and potentially blanking display. This is evident because the init path unconditionally saves state without setting `vco_current_rate` (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:890), while the restore path uses `pll_7nm->vco_current_rate` to reprogram the VCO (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:677), and the VCO programming logic computes dividers from `pll->vco_current_rate` (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:129). - Root cause in code: - Init: `msm_dsi_phy_pll_save_state(phy)` is called but no VCO rate is captured (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:890). - Restore: `dsi_7nm_pll_restore_state()` writes cached mux/dividers and then calls `dsi_pll_7nm_vco_set_rate(…, pll_7nm->vco_current_rate, …)` (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:677), which assumes `vco_current_rate` is valid. - Divider calc uses `pll->vco_current_rate` directly (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:129), so a zero value yields dec/frac=0, propagating 0 Hz into the clock tree. - Restore is actually invoked during enable: `msm_dsi_phy_enable()` calls `msm_dsi_phy_pll_restore_state()` via the ops hook (drivers/gpu/drm/msm/dsi/phy/dsi_phy.c:774), so the bad `vco_current_rate` directly impacts runtime bring-up/handover. - The fix: Initialize `vco_current_rate` at init by reading the current hardware rate; if it can’t be determined, fall back to the minimum safe PLL rate to avoid 0 Hz: - Added in `dsi_pll_7nm_init()`: - `if (!dsi_pll_7nm_vco_recalc_rate(&pll_7nm->clk_hw, VCO_REF_CLK_RATE)) pll_7nm->vco_current_rate = pll_7nm->phy->cfg->min_pll_rate;` (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:893). - Why this is correct and low risk: - `dsi_pll_7nm_vco_recalc_rate()` reads current PLL dec/frac and updates/returns the actual VCO rate (drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:572), so subsequent restore reprograms the VCO to its real, pre-existing frequency, enabling clean handover from bootloader firmware. - If hardware isn’t configured (recalc returns 0), falling back to `min_pll_rate` avoids the destructive 0 Hz program while still keeping a safe, bounded frequency using SoC-provided limits (e.g., `min_pll_rate` in the 7nm cfgs at drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c:1289, 1309, 1332, 1350…). - This mirrors established practice in other MSM DSI PHY generations, e.g. the 10nm PHY already does the same recalc/fallback in its init path (drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c:709), and the 14nm PHY also guards against a 0 rate on startup (drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c:546). - The change is localized to a single function in one driver, does not alter interfaces, and only affects first-init/handover behavior. It reduces, rather than increases, the chance of glitches by avoiding a 0 Hz restore. - Backport criteria: - Important user-facing bug fix (prevents display/clock tree breakage on bring-up/handover). - Small and self-contained (one file, a few lines). - No new features or architectural changes; consistent with other PHY drivers’ behavior. - Low regression risk with clear, safe fallback behavior.
Given the above, this is a strong candidate for stable backport.
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c index 6b765f3fd529a..5c8a3394c3da0 100644 --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c @@ -843,6 +843,12 @@ static int dsi_pll_7nm_init(struct msm_dsi_phy *phy)
/* TODO: Remove this when we have proper display handover support */ msm_dsi_phy_pll_save_state(phy); + /* + * Store also proper vco_current_rate, because its value will be used in + * dsi_7nm_pll_restore_state(). + */ + if (!dsi_pll_7nm_vco_recalc_rate(&pll_7nm->clk_hw, VCO_REF_CLK_RATE)) + pll_7nm->vco_current_rate = pll_7nm->phy->cfg->min_pll_rate;
return 0; }