Skip to content

Commit defec17

Browse files
computersforpeacechanwoochoi
authored andcommitted
soc: rockchip: power-domain: Manage resource conflicts with firmware
On RK3399 platforms, power domains are managed mostly by the kernel (drivers/soc/rockchip/pm_domains.c), but there are a few exceptions where ARM Trusted Firmware has to be involved: (1) system suspend/resume (2) DRAM DVFS (a.k.a., "ddrfreq") Exception (1) does not cause much conflict, since the kernel has quiesced itself by the time we make the relevant PSCI call. Exception (2) can cause conflict, because of two actions: (a) ARM Trusted Firmware needs to read/modify/write the PMU_BUS_IDLE_REQ register to idle the memory controller domain; the kernel driver also has to touch this register for other domains. (b) ARM Trusted Firmware needs to manage the clocks associated with these domains. To elaborate on (b): idling a power domain has always required ungating an array of clocks; see this old explanation from Rockchip: https://lore.kernel.org/linux-arm-kernel/54503C19.9060607@rock-chips.com/ Historically, ARM Trusted Firmware has avoided this issue by using a special PMU_CRU_GATEDIS_CON0 register -- this register ungates all the necessary clocks -- when idling the memory controller. Unfortunately, we've found that this register is not 100% sufficient; it does not turn the relevant PLLs on [0]. So it's possible to trigger issues with something like the following: 1. enable a power domain (e.g., RK3399_PD_VDU) -- kernel will temporarily enable relevant clocks/PLLs, then turn them back off 2. a PLL (e.g., PLL_NPLL) is part of the clock tree for RK3399_PD_VDU's clocks but otherwise unused; NPLL is disabled 3. perform a ddrfreq transition (rk3399_dmcfreq_target() -> ... drivers/clk/rockchip/clk-ddr.c / ROCKCHIP_SIP_DRAM_FREQ) 4. ARM Trusted Firmware unagates VDU clocks (via PMU_CRU_GATEDIS_CON0) 5. ARM Trusted firmware idles the memory controller domain 6. Step 5 waits on the VDU domain/clocks, but NPLL is still off i.e., we hang the system. So for (b), we need to at a minimum manage the relevant PLLs on behalf of firmware. It's easier to simply manage the whole clock tree, in a similar way we do in rockchip_pd_power(). For (a), we need to provide mutual exclusion betwen rockchip_pd_power() and firmware. To resolve that, we simply grab the PMU mutex and release it when ddrfreq is done. The Chromium OS kernel has been carrying versions of part of this hack for a while, based on some new custom notifiers [1]. I've rewritten as a simple function call between the drivers, which is OK because: * the PMU driver isn't enabled, and we don't have this problem at all (the firmware should have left us in an OK state, and there are no runtime conflicts); or * the PMU driver is present, and is a single instance. And the power-domain driver cannot be removed, so there's no lifetime management to worry about. For completeness, there's a 'dmc_pmu_mutex' to guard (likely theoretical?) probe()-time races. It's OK for the memory controller driver to start running before the PMU, because the PMU will avoid any critical actions during the block() sequence. [0] The RK3399 TRM for PMU_CRU_GATEDIS_CON0 only talks about ungating clocks. Based on experimentation, we've found that it does not power up the necessary PLLs. [1] CHROMIUM: soc: rockchip: power-domain: Add notifier to dmc driver https://chromium-review.googlesource.com/q/I242dbd706d352f74ff706f5cbf42ebb92f9bcc60 Notably, the Chromium solution only handled conflict (a), not (b). In practice, item (b) wasn't a problem in many cases because we never managed to fully power off PLLs. Now that the (upstream) video decoder driver performs runtime clock management, we often power off NPLL. Signed-off-by: Brian Norris <briannorris@chromium.org> Tested-by: Peter Geis <pgwipeout@gmail.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
1 parent 5d521a3 commit defec17

2 files changed

Lines changed: 143 additions & 0 deletions

File tree

drivers/soc/rockchip/pm_domains.c

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <linux/io.h>
99
#include <linux/iopoll.h>
1010
#include <linux/err.h>
11+
#include <linux/mutex.h>
1112
#include <linux/pm_clock.h>
1213
#include <linux/pm_domain.h>
1314
#include <linux/of_address.h>
@@ -16,6 +17,7 @@
1617
#include <linux/clk.h>
1718
#include <linux/regmap.h>
1819
#include <linux/mfd/syscon.h>
20+
#include <soc/rockchip/pm_domains.h>
1921
#include <dt-bindings/power/px30-power.h>
2022
#include <dt-bindings/power/rk3036-power.h>
2123
#include <dt-bindings/power/rk3066-power.h>
@@ -139,6 +141,109 @@ struct rockchip_pmu {
139141
#define DOMAIN_RK3568(name, pwr, req, wakeup) \
140142
DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
141143

144+
/*
145+
* Dynamic Memory Controller may need to coordinate with us -- see
146+
* rockchip_pmu_block().
147+
*
148+
* dmc_pmu_mutex protects registration-time races, so DMC driver doesn't try to
149+
* block() while we're initializing the PMU.
150+
*/
151+
static DEFINE_MUTEX(dmc_pmu_mutex);
152+
static struct rockchip_pmu *dmc_pmu;
153+
154+
/*
155+
* Block PMU transitions and make sure they don't interfere with ARM Trusted
156+
* Firmware operations. There are two conflicts, noted in the comments below.
157+
*
158+
* Caller must unblock PMU transitions via rockchip_pmu_unblock().
159+
*/
160+
int rockchip_pmu_block(void)
161+
{
162+
struct rockchip_pmu *pmu;
163+
struct generic_pm_domain *genpd;
164+
struct rockchip_pm_domain *pd;
165+
int i, ret;
166+
167+
mutex_lock(&dmc_pmu_mutex);
168+
169+
/* No PMU (yet)? Then we just block rockchip_pmu_probe(). */
170+
if (!dmc_pmu)
171+
return 0;
172+
pmu = dmc_pmu;
173+
174+
/*
175+
* mutex blocks all idle transitions: we can't touch the
176+
* PMU_BUS_IDLE_REQ (our ".idle_offset") register while ARM Trusted
177+
* Firmware might be using it.
178+
*/
179+
mutex_lock(&pmu->mutex);
180+
181+
/*
182+
* Power domain clocks: Per Rockchip, we *must* keep certain clocks
183+
* enabled for the duration of power-domain transitions. Most
184+
* transitions are handled by this driver, but some cases (in
185+
* particular, DRAM DVFS / memory-controller idle) must be handled by
186+
* firmware. Firmware can handle most clock management via a special
187+
* "ungate" register (PMU_CRU_GATEDIS_CON0), but unfortunately, this
188+
* doesn't handle PLLs. We can assist this transition by doing the
189+
* clock management on behalf of firmware.
190+
*/
191+
for (i = 0; i < pmu->genpd_data.num_domains; i++) {
192+
genpd = pmu->genpd_data.domains[i];
193+
if (genpd) {
194+
pd = to_rockchip_pd(genpd);
195+
ret = clk_bulk_enable(pd->num_clks, pd->clks);
196+
if (ret < 0) {
197+
dev_err(pmu->dev,
198+
"failed to enable clks for domain '%s': %d\n",
199+
genpd->name, ret);
200+
goto err;
201+
}
202+
}
203+
}
204+
205+
return 0;
206+
207+
err:
208+
for (i = i - 1; i >= 0; i--) {
209+
genpd = pmu->genpd_data.domains[i];
210+
if (genpd) {
211+
pd = to_rockchip_pd(genpd);
212+
clk_bulk_disable(pd->num_clks, pd->clks);
213+
}
214+
}
215+
mutex_unlock(&pmu->mutex);
216+
mutex_unlock(&dmc_pmu_mutex);
217+
218+
return ret;
219+
}
220+
EXPORT_SYMBOL_GPL(rockchip_pmu_block);
221+
222+
/* Unblock PMU transitions. */
223+
void rockchip_pmu_unblock(void)
224+
{
225+
struct rockchip_pmu *pmu;
226+
struct generic_pm_domain *genpd;
227+
struct rockchip_pm_domain *pd;
228+
int i;
229+
230+
if (dmc_pmu) {
231+
pmu = dmc_pmu;
232+
for (i = 0; i < pmu->genpd_data.num_domains; i++) {
233+
genpd = pmu->genpd_data.domains[i];
234+
if (genpd) {
235+
pd = to_rockchip_pd(genpd);
236+
clk_bulk_disable(pd->num_clks, pd->clks);
237+
}
238+
}
239+
240+
mutex_unlock(&pmu->mutex);
241+
}
242+
243+
mutex_unlock(&dmc_pmu_mutex);
244+
}
245+
EXPORT_SYMBOL_GPL(rockchip_pmu_unblock);
246+
142247
static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
143248
{
144249
struct rockchip_pmu *pmu = pd->pmu;
@@ -690,6 +795,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
690795

691796
error = -ENODEV;
692797

798+
/*
799+
* Prevent any rockchip_pmu_block() from racing with the remainder of
800+
* setup (clocks, register initialization).
801+
*/
802+
mutex_lock(&dmc_pmu_mutex);
803+
693804
for_each_available_child_of_node(np, node) {
694805
error = rockchip_pm_add_one_domain(pmu, node);
695806
if (error) {
@@ -719,10 +830,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev)
719830
goto err_out;
720831
}
721832

833+
/* We only expect one PMU. */
834+
if (!WARN_ON_ONCE(dmc_pmu))
835+
dmc_pmu = pmu;
836+
837+
mutex_unlock(&dmc_pmu_mutex);
838+
722839
return 0;
723840

724841
err_out:
725842
rockchip_pm_domain_cleanup(pmu);
843+
mutex_unlock(&dmc_pmu_mutex);
726844
return error;
727845
}
728846

include/soc/rockchip/pm_domains.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/* SPDX-License-Identifier: GPL-2.0 */
2+
/*
3+
* Copyright 2022, The Chromium OS Authors. All rights reserved.
4+
*/
5+
6+
#ifndef __SOC_ROCKCHIP_PM_DOMAINS_H__
7+
#define __SOC_ROCKCHIP_PM_DOMAINS_H__
8+
9+
#ifdef CONFIG_ROCKCHIP_PM_DOMAINS
10+
11+
int rockchip_pmu_block(void);
12+
void rockchip_pmu_unblock(void);
13+
14+
#else /* CONFIG_ROCKCHIP_PM_DOMAINS */
15+
16+
static inline int rockchip_pmu_block(void)
17+
{
18+
return 0;
19+
}
20+
21+
static inline void rockchip_pmu_unblock(void) { }
22+
23+
#endif /* CONFIG_ROCKCHIP_PM_DOMAINS */
24+
25+
#endif /* __SOC_ROCKCHIP_PM_DOMAINS_H__ */

0 commit comments

Comments
 (0)