Quoting Stephen Boyd (2023-11-07 19:18:45)
>
> I also tried skipping slamming a bunch of PLL config register writes
> into the PLL at probe by removing the clk_fabia_pll_configure() call but
> it doesn't fix it. Maybe I need to measure the clk at probe time to see
> if it is actually on XO or if it is stuck on the PLL but all the
> registers are saying it is XO.
>
I'm still chasing this, but I have another update. I tried moving mdp
and rot to XO from disp_cc_pll0 at init, and left 'parked_cfg' at
default value of 0. Then I disabled disp_cc_pll0 at the end of clk
driver probe. This fixes the problem. In this case, the perceived parent
of mdp and rot is XO because 'parked_cfg' is 0.
Then I tried the same sequence above, but disabled and then enabled the
disp_cc_pll0. This also worked. The disp_cc_pll0 was disabled during
late init because it was unused, but otherwise everything is fine. This
means that disabling and then enabling when nothing is sourcing the PLL
somehow "fixes" it.
Then I tried some wacky stuff. I moved rot to XO and left mdp on
disp_cc_pll0, and left 'parked_cfg' at default value of 0. Then I
disabled and enabled disp_cc_pll0 at driver probe. This also fixed the
problem. I would think disabling the PLL while mdp is sourcing from it
would cause the branch to be stuck, but apparently nothing goes wrong.
During boot, mdp is switched to XO when the clk is 'restored' for
clk_enable(), and then the branch is enabled and it reports the clk as
on.
Then I tried leaving rot on disp_cc_pll0, moving mdp to XO, and leaving
'parked_cfg' at default value of 0. Then I disabled and enabled
disp_cc_pll0 at driver probe. This didn't work. During boot and up to
the time of being stuck off, mdp is parked and unparked but it's always
sourcing from XO. I don't understand this part. mdp was moved to XO very
early, while rot was still sourcing from disp_cc_pll0 when we turned off
the PLL during late init. Presumably mdp shouldn't be stuck.
Then I tried leaving rot on disp_cc_pll0, moving mdp to XO, and leaving
'parked_cfg' at default value of 0. I skipped the PLL enable/disable
dance. This didn't work either. During late init, the rot branch clk
(disp_cc_mdss_rot_clk) is disabled, but the rot rcg is still configured
to source from disp_cc_pll0.
Then I tried leaving rot on disp_cc_pll0, moving mdp to XO, and leaving
'parked_cfg' at default value of 0. I only disable the PLL during probe.
This didn't work either! In fact, mdp is stuck after being turned on and
off once (but shouldn't it be sourcing from XO?).
Then I tried leaving rot on disp_cc_pll0, moving mdp to XO, and leaving
'parked_cfg' at default value of 0. I only enable the PLL during probe.
This didn't work either. mdp gets stuck after late init, but it is
supposed to be sourcing from XO.
I'm thinking what's happening is that disabling the PLL during late init
is hanging the branch, but only when an rcg is sourcing from the PLL.
Or maybe what's happening is the rot branch register value is wrong and
it's actually swapped with the mdp branch in the driver, or the rcg for
mdp and rot are swapped. In the cases above, it only breaks when the rot
rcg is sourcing from disp_cc_pll0, and disp_cc_pll0 is disabled.
Here's what's happening without any changes
1. mdp and rot are sourcing from disp_cc_pll0 at driver probe, disp_cc_pll0 is enabled
2. mdp is configured to restore on XO
3. rot is configured to restore on XO
4. mdp is switched to XO on clk_enable()
5. mdp is switched to XO on clk_disable()
6. disp_cc_pll0 is left untouched
7. rot branch clk is disabled during late init (disp_cc_mdss_rot_clk)
8. rot rcg is still enabled
9. disp_cc_pll0 is disabled during late init
10. mdp is switched to XO on clk_enable()
11. mdp branch is stuck off
I could see the problem happening if mdp branch and rot branch were
swapped. When we enable/disable mdp branch it will actually
enable/disable the rot rcg because feedback is going there. During boot
this is fine because disp_cc_pll0 is enabled. Leaving it enabled
throughout boot hides the problem because enabling mdp branch is
actually enabling rot branch. Once we get to late init, we disable rot
branch (actually mdp branch). The mdp rcg is already parked, so this
should be OK. The problem is the mdp branch (actually rot branch) has
been disabled, while the rot rcg is still sourcing disp_cc_pll0. We'll
disable the pll during late init, and this will wedge the clk off. When
we go to turn on the mdp branch (actually the rot branch) after late
init, we'll try to turn on the branch and the rot rcg will be parented
to the disp_cc_pll0 still (because we never moved it off).
This patch fixes the problem for me. Obviously, things are still bad
though because we don't report the proper parent to the framework. We
can do that pretty easily by parking at clk registration time though and
also by saving the config register.
diff --git a/drivers/clk/qcom/dispcc-sc7180.c b/drivers/clk/qcom/dispcc-sc7180.c
index 9536bfc72a43..26eea1e962d3 100644
--- a/drivers/clk/qcom/dispcc-sc7180.c
+++ b/drivers/clk/qcom/dispcc-sc7180.c
@@ -499,10 +499,10 @@ static struct clk_branch disp_cc_mdss_esc0_clk = {
};
static struct clk_branch disp_cc_mdss_mdp_clk = {
- .halt_reg = 0x200c,
+ .halt_reg = 0x2014,
.halt_check = BRANCH_HALT,
.clkr = {
- .enable_reg = 0x200c,
+ .enable_reg = 0x2014,
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "disp_cc_mdss_mdp_clk",
@@ -570,10 +570,10 @@ static struct clk_branch disp_cc_mdss_pclk0_clk = {
};
static struct clk_branch disp_cc_mdss_rot_clk = {
- .halt_reg = 0x2014,
+ .halt_reg = 0x200c,
.halt_check = BRANCH_HALT,
.clkr = {
- .enable_reg = 0x2014,
+ .enable_reg = 0x200c,
.enable_mask = BIT(0),
.hw.init = &(struct clk_init_data){
.name = "disp_cc_mdss_rot_clk",
TL;DR: Taniya or anyone at qcom, please double check that the
disp_cc_mdss_mdp_clk register is correct and not actually swapped with
disp_cc_mdss_rot_clk.
I tried swapping the rcg registers, and was met with a blue screen, so I
believe the rcg registers are correct. testclock confirmed as well. My
next experiment is to figure out a way to turn off the rot branch
and measure.
