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

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.

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.

>>>>     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(-)
>>>>
>>

-- 
Best wishes,
Vladimir