Re: [PATCH] clk: rockchip: rk3588: Don't change PLL rates when setting dclk_vop2_src

[Heiko Stuebner <heiko@sntech.de>]

Am Donnerstag, 16. Oktober 2025, 00:57:15 Mitteleuropäische Sommerzeit schrieb Sebastian Reichel:
> Hi,
> 
> On Wed, Oct 15, 2025 at 03:27:12PM +0200, Heiko Stübner wrote:
> > Am Mittwoch, 15. Oktober 2025, 14:58:46 Mitteleuropäische Sommerzeit schrieb Quentin Schulz:
> > > Hi Heiko,
> > > 
> > > On 10/8/25 3:31 PM, Heiko Stuebner wrote:
> > > > dclk_vop2_src currently has CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT
> > > > flags set, which is vastly different than dclk_vop0_src or dclk_vop1_src,
> > > > which have none of those.
> > > > 
> > > > With these flags in dclk_vop2_src, actually setting the clock then results
> > > > in a lot of other peripherals breaking, because setting the rate results
> > > > in the PLL source getting changed:
> > > > 
> > > > [   14.898718] clk_core_set_rate_nolock: setting rate for dclk_vop2 to 152840000
> > > > [   15.155017] clk_change_rate: setting rate for pll_gpll to 1680000000
> > > > [ clk adjusting every gpll user ]
> > > > 
> > > > This includes possibly the other vops, i2s, spdif and even the uarts.
> > > > Among other possible things, this breaks the uart console on a board
> > > > I use. Sometimes it recovers later on, but there will be a big block
> > > 
> > > I can reproduce on the same board as yours and this fixes the issue 
> > > indeed (note I can only reproduce for now when display the modetest 
> > > pattern, otherwise after boot the console seems fine to me).
> > 
> > I boot into a Debian rootfs with fbcon on my system, and the serial
> > console produces garbled output when the vop adjusts the clock
> > 
> > Sometimes it recovers after a bit, but other times it doesn't
> > 
> > > Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
> > > Tested-by: Quentin Schulz <quentin.schulz@cherry.de> # RK3588 Tiger w/DP carrierboard
> 
> I'm pretty sure I've seen this while playing with USB-C DP AltMode
> on Rock 5B. So far I had no time to investigate further.
> 
> What I'm missing in the commit message is the impact on VOP. Also
> it might be a good idea to have Andy in Cc, so I've added him.

Hmm, it brings VP2 in line with the other two VPs, only VP2 had this
special setting - even right from the start, so it could very well
have been left there accidentially during submission.

So in the end VP2 will have to deal with this, because when the VP
causes a rate change in the GPLL, this changes so many clocks of
other possibly running devices. Not only the uart, but also emmc
and many more. And all those devices do not like if their clock gets
changed under them I think.


Heiko