On Tue, May 20, 2025 at 01:13:26PM +0200, Krzysztof Kozlowski wrote: > Driver unconditionally saves current state on first init in > dsi_pll_10nm_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_10nm_pll_restore_state(). > > Restoring calls dsi_pll_10nm_vco_set_rate() with > pll_10nm->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. > > Suggested-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com> > Link: > https://lore.kernel.org/r/sz4kbwy5nwsebgf64ia7uq4ee7wbsa5uy3xmlqwcstsbntzcov@ew3dcyjdzmi2/ > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlow...@linaro.org>
Fixes? > > --- > > Not tested on hardware. > --- > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c > b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c > index 9812b4d69197..af2e30f3f842 100644 > --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c > +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c > @@ -704,6 +704,13 @@ static int dsi_pll_10nm_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_10nm_pll_restore_state(). > + */ > + if (!dsi_pll_10nm_vco_recalc_rate(&pll_10nm->clk_hw, VCO_REF_CLK_RATE)) > + pll_10nm->vco_current_rate = pll_10nm->phy->cfg->min_pll_rate; > + > return 0; > } > > -- > 2.45.2 > -- With best wishes Dmitry
