Re: [RFC PATCH v2 2/3] clk: qcom: implement RCG2 'parked' clock support

[Stephen Boyd <sboyd@kernel.org>]

Quoting Dmitry Baryshkov (2023-11-06 18:00:04)
> On Tue, 7 Nov 2023 at 03:36, Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Stephen Boyd (2023-11-03 18:24:47)
> >
> > I looked at this more today. It seems that I need to both read the
> > config register at init and also move over the rcg to the safe source at
> > init (i.e. park the clk at init). That's doable with a call to
> > clk_rcg2_shared_disable() for the clk_ops::init callback. Otherwise I
> > get a stuck clk warning.
> >
> > Either
> >
> >  disp_cc_mdss_mdp_clk status stuck at 'off'
> >
> > or
> >
> >  disp_cc_mdss_rot_clk status stuck at 'on'
> >
> > When I don't park the rcg, the disp_cc_mdss_rot_clk gets stuck during
> > disabling of unused clks. I think I understand that problem. What
> > happens is disp_cc_mdss_mdp_clk_src and disp_cc_mdss_rot_clk_src are
> > both sourcing from disp_cc_pll0 at boot. Fixing the parent mapping makes
> > it so that enabling and then disabling disp_cc_mdss_ahb_clk causes
> > disp_cc_pll0 to be turned off when disp_cc_mdss_rot_clk_src is sourced
> > from it (and the branch disp_cc_mdss_rot_clk is enabled). If we park
> > both the rcgs at clk registration time we avoid this problem because the
> > PLL is disabled but nothing is actually a child clk. The act of reading
> > the config register and stashing that in the 'parked_cfg' only helps
> > avoid duplicate calls to change the rate, and doesn't help when we try
> > to repoint the clk at XO when the parent PLL is off.
> >
> > The part I still don't understand is why reading the config register at
> > init and stashing that in 'parked_cfg' fixes the disp_cc_mdss_mdp_clk
> > stuck at off problem. I see that the branch clk is turned off and on
> > many times during boot and there aren't any warnings regardless of
> > stashing the config register. That means we should be moving the RCG to
> > XO source, between forcibly enabling and disabling the RCG, which
> > presumably would complain about being unable to update the config
> > register, but it doesn't. Only after late init does the clk fail to
> > enable, and the source is still XO at that time. Something else must be
> > happening that wedges the branch to the point that it can't be
> > recovered. But simply reporting the proper parent is enough for
> > disp_cc_mdss_mdp_clk.
> 
> I suppose that the issue is caused by mdss_gdsc or mmcx also being
> shut down at the late_init. And if I remember correctly, properly
> parking disp_cc_mdss_mdp_clk to the XO solves this issue. This is
> where is_enabled comes to play. Adding it changes the
> clk_disable_unused behaviour.

The thing is that disp_cc_mdss_mdp_clk_src has been parked to XO by the
time late_init runs. The branch clk (disp_cc_mdss_mdp_clk) has been
enabled and disabled repeatedly during boot as well, and all those times
nothing has signaled a failure. That means the RCG has supposedly
switched away from the disp_cc_pll0 to XO (parked) and the branch isn't
stuck on or off. So how does turning the mdss_gdsc or mmcx off during
late_init cause the branch to get stuck off if the parent of the RCG is
XO? Is something changing the parent back to the display PLL?