IssueTracker

The rcg clk parking logic in the qualcomm clk driver is broken. The original parking logic was written in commit 7ef6f11887bd ("clk: qcom: Configure the RCGs to a safe source as needed")[1] and then rewritten in commit 703db1f5da1e ("clk: qcom: rcg2: Cache CFG register updates for parked RCGs")[2]. The original implementation's flaw was that it left the RCG (root clock generator) dirty with the frequency that the clk framework had set on the clk instead of the safe frequency (XO speed). This caused two problems. Confusion when debugging because the register contents didn't match what the clk was actually programmed for and a real problem once the GDSC itself started hitting the RCG's "update" bit when powering on. Once the GDSC started trying to enable the clk by clearing the dirty state it caused clks to get stuck because the clk registers would be dirty with a configuration that selected a parent that's off (e.g. the display PLL).

The rewritten patch fixed this by caching the configuration register contents in software instead of in hardware. Moving the intended register setting out of the register fixed the problem because the GDSC enabling would only cause XO to be selected, and thus no stuck clk could occur. There's one problem in the patch though. The cached configuration register needs to be populated when the clk is registered, otherwise the contents are all zero, meaning source from XO. If an RCG is sourcing something besides XO (which we are on Trogdor) it will be switched over to XO when the clk is enabled the first time.

This design is still broken though because of another issue. The GDSC may need to enable more than one RCG to do "housekeeping" while turning on. In the case of display clks, the display GDSC enables more than one clk. If all the clks haven't been parked and some parent has been disabled then the GDSC is going to lock up and not actually turn on. This will lead to clks that otherwise have been parked not turning on properly. This is very confusing.

I diagnosed this problem in this thread. Here's what happens:

1. mdp and rot rcgs are sourcing from disp_cc_pll0 at driver probe, disp_cc_pll0 is enabled, mdp rcg/branch and rot rcg/branch are enabled
2. mdp rcg is configured to restore on XO (because of the wrong cached register)
3. rot rcg is configured to restore on XO (because of the wrong cached register)
4. mdp rcg is switched to XO on clk_enable()
5. mdp rcg 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 rcg is switched to XO on clk_enable()
11. mdp branch is stuck off

[    0.511452] disp_cc_mdss_mdp_clk status stuck at 'off'
[    0.511475] WARNING: CPU: 1 PID: 81 at drivers/clk/qcom/clk-branch.c:87 clk_branch_toggle+0x114/0x168
[    0.511498] Modules linked in:
[    0.511508] CPU: 1 PID: 81 Comm: kworker/u16:4 Not tainted 6.7.0-g0dd3ee311255 #1 f5757d475795053fd2ad52247a070cd50dd046f2
[    0.511519] Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
[    0.511526] Workqueue: events_unbound deferred_probe_work_func
[    0.511540] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    0.511549] pc : clk_branch_toggle+0x114/0x168
[    0.511558] lr : clk_branch_toggle+0x110/0x168
[    0.511567] sp : ffffffc08084b670
[    0.511573] pmr_save: 00000060
[    0.511578] x29: ffffffc08084b680 x28: ffffff808006de00 x27: 0000000000000001
[    0.511591] x26: ffffff8080dbd4f4 x25: 0000000000000000 x24: 0000000000000000
[    0.511604] x23: 0000000000000000 x22: ffffffd838461198 x21: ffffffd838007997
[    0.511616] x20: ffffffd837541d5c x19: 0000000000000001 x18: 0000000000000004
[    0.511628] x17: 0000000000000000 x16: 0000000000000010 x15: ffffffd837070fac
[    0.511640] x14: 0000000000000003 x13: 0000000000000004 x12: 0000000000000001
[    0.511651] x11: c0000000ffffdfff x10: ffffffd838347aa0 x9 : 08dadf92e516c000
[    0.511663] x8 : 08dadf92e516c000 x7 : 0000000000000000 x6 : 0000000000000027
[    0.511675] x5 : ffffffd8385a61f2 x4 : 0000000000000000 x3 : ffffffc08084b398
[    0.511687] x2 : ffffffc08084b3a0 x1 : 00000000ffffdfff x0 : 00000000fffffff0
[    0.511699] Call trace:
[    0.511705]  clk_branch_toggle+0x114/0x168
[    0.511715]  clk_branch2_enable+0x24/0x30
[    0.511725]  clk_core_enable+0x5c/0x1c8
[    0.511735]  clk_enable+0x38/0x58
[    0.511744]  clk_bulk_enable+0x40/0xb0
[    0.511753]  mdss_runtime_resume+0x68/0x258
[    0.511764]  pm_generic_runtime_resume+0x30/0x44
[    0.511775]  __genpd_runtime_resume+0x30/0x80
[    0.511786]  genpd_runtime_resume+0x124/0x214
[    0.511795]  __rpm_callback+0x7c/0x15c
[    0.511805]  rpm_callback+0x30/0x88
[    0.511813]  rpm_resume+0x390/0x4d8
[    0.511823]  rpm_resume+0x43c/0x4d8
[    0.511831]  __pm_runtime_resume+0x54/0x98
[    0.511839]  __device_attach+0xe0/0x170
[    0.511847]  device_initial_probe+0x1c/0x28
[    0.511854]  bus_probe_device+0x48/0xa4
[    0.511864]  device_add+0x52c/0x6fc
[    0.511872]  mipi_dsi_device_register_full+0x104/0x1a8
[    0.511882]  devm_mipi_dsi_device_register_full+0x28/0x78
[    0.511891]  ti_sn_bridge_probe+0x1dc/0x2bc
[    0.511900]  auxiliary_bus_probe+0x4c/0x94
[    0.511909]  really_probe+0xf8/0x270
[    0.511917]  __driver_probe_device+0xa8/0x130
[    0.511925]  driver_probe_device+0x44/0x104
[    0.511934]  __device_attach_driver+0xa4/0xcc
[    0.511941]  bus_for_each_drv+0x94/0xe8
[    0.511951]  __device_attach+0xf8/0x170
[    0.511959]  device_initial_probe+0x1c/0x28
[    0.511967]  bus_probe_device+0x48/0xa4
[    0.511976]  deferred_probe_work_func+0x9c/0xd8
[    0.511984]  process_scheduled_works+0x1ac/0x4dc
[    0.511995]  worker_thread+0x110/0x320
[    0.512004]  kthread+0x100/0x120
[    0.512013]  ret_from_fork+0x10/0x20
[    0.512023] ---[ end trace 0000000000000000 ]---

More specifically, the way we disable clks during late init and how we skip reading or writing the rcg enable bit causes clks to get stuck when the GDSC is turned on after disabling of unused clks. The RCG has a register bit that turns on the RCG. The branches which are children of that RCG automatically turn on the RCG when they are enabled or disabled themselves. There's a feedback mechanism in hardware that enables the RCG when a child branch is enabled. Given this design, the linux clk driver never writes the RCG enable bit unless it needs to ensure the RCG is on regardless of child branch state. The parking logic writes the rcg enable bit while parking the clk to ensure the parking actually works. Otherwise the parking itself may get stuck because both sources need to be clocking, along with the rcg itself, to ensure a proper switch.

When we disable unused clks, the rot branch is disabled but the rot rcg is not because it doesn't implement struct clk_ops::is_enabled(). Enabling the GDSC after late init wedges all the clks for the display subsystem (mdp and rot) because the GDSC turns on the clks temporarily and gets stuck when the rot rcg is sourcing from the display pll that is disabled.

Another patch series has attempted to implement struct clk_ops::is_enabled() for parked RCGs. The downside of this approach is that it relies on the parent being XO to know that the clk is disabled and therefore XO has to be removed as a possible parent. This makes linking up parents confusing if the RCG is parented to XO during registration. The patch moves the parent to the first frequency in the frequency table during registration to avoid confusing the is_enabled() clk op.

The root cause of the problem is that the enable state of the clk tree is not being carried over properly from the bootloader. The disable unused clk code disables the child of the rcg, but it doesn't know about the parent being enabled and thus the rot rcg is never parked. To make matters worse, if the disable unused logic never runs we'll still have a problem because the display driver turns off the display pll (disp_cc_pll0), causing the rot clk to be stuck and any future attempts to enable the GDSC will wedge the mdp clk until the display pll is enabled (because rot has always been sourcing from the display pll).

A proper fix would probably be to teach the GDSC logic to park RCGs when the GDSC is powered down. This may run into a problem though where the display PLL is disabled before the GDSC is, and then the RCG can't be parked anymore. Another solution is to park all RCGs that are parkable at clk registration time and also clear the RCG enable bit (root enable bit). If we did this we may ruin continuous splash screen, but it would allow the GDSC to be enabled/disabled regardless of clk framework state.