Re: [PATCH] Revert "clk: qcom: dispcc-sm8250: Enable parents for pixel clocks"

On Tue, Jul 21, 2026 at 01:39:23AM +0300, Vladimir Zapolskiy wrote:
> On 7/20/26 23:54, Dmitry Baryshkov wrote:
> > On Mon, Jul 20, 2026 at 09:21:03PM +0300, Vladimir Zapolskiy wrote:
> > > On 7/20/26 18:00, Dmitry Baryshkov wrote:
> > > > On Wed, Jul 01, 2026 at 03:11:47AM +0300, Vladimir Zapolskiy wrote:
> > > > > This reverts commit acf7a91d0b0e9e3ef374944021de62062125b7e4.
> > > > > 
> > > > > The commit causes quite severe problems in runtime due to the introduced
> > > > > cyclic clock dependency:
> > > > > 
> > > > > * dsi@ae9400 platform device and its probe depends on dispcc clocks,
> > > > > * the reverted commit links dispcc clocks enablement to mdss_dsiX_phy clocks,
> > > > > * at this stage DSI PHY PLL cannot be locked to complete driver initialization,
> > > > >     the parent clocks for DISPCC provided mdss_pclkX clocks can not be enabled.
> > > > > 
> > > > > On RB5 board with no connected display panel on boot I get this error report:
> > > > > 
> > > > >     DSI PLL(0) lock failed, status=0x00000000
> > > > >     PLL(0) lock failed
> > > > 
> > > > Why can't it be locked at this stage? Is it underpowered? Or is the code
> > > > trying to lock it on some random, unsupported value?
> > > 
> > > Since there is no display panel attached, I believe DSI PLL clocks should
> > > not be enabled at all, however the reverted commit requires it to happen.
> > 
> > If there is no display panel, you can keep DSI hosts and DSI PHYs
> > disabled. If they are enabled, they can be configured to some
> > intermediate values (even if for the sake of turning off the GDSC).
> > 
> > > 
> > > I don't have a panel for RB5 to test, if the PLL clocks can be locked or
> > > not, when there is an attached panel.
> > > 
> > > In general I believe the problem is plainly related to the established
> > > cyclic dependency as it's stated above, and in any case the reverted
> > > commit is a clear regression for RB5 board with no panel.
> > 
> > RB5 ships with the lt9611uxc bridge.
> > 
> > There is no cyclic dependency, AFAIR. It's DSI PHY PLL <- dispcc <- MDSS
> > pclk clocks.
> 
> What does '<-' above mean?
> 
> From sm8250.dtsi (using labels only):
> 
> * dispcc is supplied by mdss_dsi[01]_phy,
> * mdss_dsi[01]_phy are supplied by dispcc,
> * mdss_dsi[01]_phy are children of mdss,
> * mdss is supplied by dispcc.
> 
> After the applied commit, which I advocate to revert for now, the ball
> of clocks is no longer left untangled and linear.

The devices have always had a circular dependency. The commit in
question adds nothing to that.

> 
> If there is a clear regression caused by some commit, it would make
> sense to revert it first, then reapply it after an alternative fix
> is ready.

Unless the commit is correct and just uncovers an issue elsewhere.

> > > > >     dsi0_phy_pll_out_dsiclk already disabled
> > > > >     WARNING: drivers/clk/clk.c:1188 at clk_core_disable+0xa4/0xac, CPU#4: kworker/u32:1/61
> > > > >     ...
> > > > >     dsi0_phy_pll_out_dsiclk already unprepared
> > > > >     WARNING: drivers/clk/clk.c:1047 at clk_core_unprepare+0xe4/0x104, CPU#4: kworker/u32:1/61
> > > > >     ...
> > > > >     Tainted: [W]=WARN
> > > > > 
> > > > > The problem has been already reported and discussed before [1], the original
> > > > > commit was done for "preventing potential stability issues" reason, and this
> > > > > time fixing a potential issue introduced the actual one unfortunately.
> > > > > 
> > > > > [1] https://lore.kernel.org/linux-arm-msm/[email protected]/
> > > > > 
> > > > > Signed-off-by: Vladimir Zapolskiy <[email protected]>
> > > > > ---
> > > > >    drivers/clk/qcom/dispcc-sm8250.c | 4 ++--
> > > > >    1 file changed, 2 insertions(+), 2 deletions(-)

-- 
With best wishes
Dmitry