From 0786d68853700687a7221240945e0bff51ef75db Mon Sep 17 00:00:00 2001 From: Caesar Wang Date: Thu, 25 Aug 2016 06:29:46 +0800 Subject: [PATCH 1/4] rockchip: on rk3399 init the PMU counts at boot; set 24M/32k properly In a previous change we mistakenly thought that PMU_24M_EN_CFG directly controlled whether the PMU counts ran off the 32k vs. 24M clock. Apparently that's not true. Real logic is now documented in code. Also in the previous change we mistaknely though that PMU_24M_EN_CFG was normally supposed to be 1 and we should "restore" it at resume time. This is a terrible idea and made the system totally unreliable after resume. Apparently PMU_24M_EN_CFG should always be 0 with all the current code and settings. Let's fix the above two problems. While we're changing all of this, let's also: 1. Init at boot time. Many of these counts are used when the system is running normally. We want the behavior at boot to match the behavior after suspend/resume. 2. Init CPU counts to be 1 us. Although old code was trying to set this to 1 ms (1000x slower) at suspend/resume time, we've been testing the kernel with 1 us for a long time now. That's because the kernel (at boot time) set these values to 24. Let's keep at 24 until we know that's wrong. 3. Init GPU counts to be 1 us. Old code wasn't touching the GPU, but as documented in comments it makes sense to init here. Do it. 4. Document the crap out of this code, since the SoC's behavior is confusing and poorly documented in the TRM. 5. Increase some stabilization times to 30 ms (from 3 ms). It's unclear that a full 30 ms is needed, but let's be safe for now. This also inits the counts for the GPU. (Thanks to Doug's patch that come from https://crosreview.com/372381) Change-Id: Id1bc159a5a99916aeab043895e5c4585c4adab22 --- plat/rockchip/rk3399/drivers/pmu/pmu.c | 127 +++++++++++++++++-------- 1 file changed, 85 insertions(+), 42 deletions(-) diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu.c b/plat/rockchip/rk3399/drivers/pmu/pmu.c index 01f84e92e..9fc796cc4 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3399/drivers/pmu/pmu.c @@ -734,6 +734,89 @@ static int hlvl_pwr_domain_resume(uint32_t lvl, plat_local_state_t lvl_state) return 0; } +/** + * init_pmu_counts - Init timing counts in the PMU register area + * + * At various points when we power up or down parts of the system we need + * a delay to wait for power / clocks to become stable. The PMU has counters + * to help software do the delay properly. Basically, it works like this: + * - Software sets up counter values + * - When software turns on something in the PMU, the counter kicks off + * - The hardware sets a bit automatically when the counter has finished and + * software knows that the initialization is done. + * + * It's software's job to setup these counters. The hardware power on default + * for these settings is conservative, setting everything to 0x5dc0 + * (750 ms in 32 kHz counts or 1 ms in 24 MHz counts). + * + * Note that some of these counters are only really used at suspend/resume + * time (for instance, that's the only time we turn off/on the oscillator) and + * others are used during normal runtime (like turning on/off a CPU or GPU) but + * it doesn't hurt to init everything at boot. + * + * Also note that these counters can run off the 32 kHz clock or the 24 MHz + * clock. While the 24 MHz clock can give us more precision, it's not always + * available (like when we turn the oscillator off at sleep time). Current + * understanding is that counts work like this: + * IF (pmu_use_lf == 0) || (power_mode_en == 0) + * use the 24M OSC for counts + * ELSE + * use the 32K OSC for counts + * + * Notes: + * - There is a separate bit for the PMU called PMU_24M_EN_CFG. At the moment + * we always keep that 0. This apparently choose between using the PLL as + * the source for the PMU vs. the 24M clock. If we ever set it to 1 we + * should consider how it affects these counts (if at all). + * - The power_mode_en is documented to auto-clear automatically when we leave + * "power mode". That's why most clocks are on 24M. Only timings used when + * in "power mode" are 32k. + * - In some cases the kernel may override these counts. + * + * The PMU_STABLE_CNT / PMU_OSC_CNT / PMU_PLLLOCK_CNT are important CNTs + * in power mode, we need to ensure that they are available. + */ +static void init_pmu_counts(void) +{ + /* COUNTS FOR INSIDE POWER MODE */ + + /* + * From limited testing, need PMU stable >= 2ms, but go overkill + * and choose 30 ms to match testing on past SoCs. Also let + * OSC have 30 ms for stabilization. + */ + mmio_write_32(PMU_BASE + PMU_STABLE_CNT, CYCL_32K_CNT_MS(30)); + mmio_write_32(PMU_BASE + PMU_OSC_CNT, CYCL_32K_CNT_MS(30)); + + /* Unclear what these should be; try 3 ms */ + mmio_write_32(PMU_BASE + PMU_WAKEUP_RST_CLR_CNT, CYCL_32K_CNT_MS(3)); + + /* Unclear what this should be, but set the default explicitly */ + mmio_write_32(PMU_BASE + PMU_TIMEOUT_CNT, 0x5dc0); + + /* COUNTS FOR OUTSIDE POWER MODE */ + + /* Put something sorta conservative here until we know better */ + mmio_write_32(PMU_BASE + PMU_PLLLOCK_CNT, CYCL_24M_CNT_MS(3)); + mmio_write_32(PMU_BASE + PMU_DDRIO_PWRON_CNT, CYCL_24M_CNT_MS(1)); + mmio_write_32(PMU_BASE + PMU_CENTER_PWRDN_CNT, CYCL_24M_CNT_MS(1)); + mmio_write_32(PMU_BASE + PMU_CENTER_PWRUP_CNT, CYCL_24M_CNT_MS(1)); + + /* + * Set CPU/GPU to 1 us. + * + * NOTE: Even though ATF doesn't configure the GPU we'll still setup + * counts here. After all ATF controls all these other bits and also + * chooses which clock these counters use. + */ + mmio_write_32(PMU_BASE + PMU_SCU_L_PWRDN_CNT, CYCL_24M_CNT_US(1)); + mmio_write_32(PMU_BASE + PMU_SCU_L_PWRUP_CNT, CYCL_24M_CNT_US(1)); + mmio_write_32(PMU_BASE + PMU_SCU_B_PWRDN_CNT, CYCL_24M_CNT_US(1)); + mmio_write_32(PMU_BASE + PMU_SCU_B_PWRUP_CNT, CYCL_24M_CNT_US(1)); + mmio_write_32(PMU_BASE + PMU_GPU_PWRDN_CNT, CYCL_24M_CNT_US(1)); + mmio_write_32(PMU_BASE + PMU_GPU_PWRUP_CNT, CYCL_24M_CNT_US(1)); +} + static void sys_slp_config(void) { uint32_t slp_mode_cfg = 0; @@ -777,52 +860,12 @@ static void sys_slp_config(void) mmio_setbits_32(PMU_BASE + PMU_WKUP_CFG4, BIT(PMU_GPIO_WKUP_EN)); mmio_write_32(PMU_BASE + PMU_PWRMODE_CON, slp_mode_cfg); - /* - * About to switch PMU counters to 32K; switch all timings to 32K - * for simplicity even if we don't plan on using them. - */ - mmio_write_32(PMU_BASE + PMU_SCU_L_PWRDN_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_L_PWRUP_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_B_PWRDN_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_B_PWRUP_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_CENTER_PWRDN_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_CENTER_PWRUP_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_WAKEUP_RST_CLR_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_OSC_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_DDRIO_PWRON_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_PLLLOCK_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_PLLRST_CNT, CYCL_32K_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_STABLE_CNT, CYCL_32K_CNT_MS(3)); - - mmio_clrbits_32(PMU_BASE + PMU_SFT_CON, BIT(PMU_24M_EN_CFG)); mmio_write_32(PMU_BASE + PMU_PLL_CON, PLL_PD_HW); mmio_write_32(PMUGRF_BASE + PMUGRF_SOC_CON0, EXTERNAL_32K); mmio_write_32(PMUGRF_BASE, IOMUX_CLK_32K); /* 32k iomux */ } -static void sys_slp_unconfig(void) -{ - /* - * About to switch PMU counters to 24M; switch all timings to 24M - * for simplicity even if we don't plan on using them. - */ - mmio_write_32(PMU_BASE + PMU_SCU_L_PWRDN_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_L_PWRUP_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_B_PWRDN_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_SCU_B_PWRUP_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_CENTER_PWRDN_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_CENTER_PWRUP_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_WAKEUP_RST_CLR_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_OSC_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_DDRIO_PWRON_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_PLLLOCK_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_PLLRST_CNT, CYCL_24M_CNT_MS(3)); - mmio_write_32(PMU_BASE + PMU_STABLE_CNT, CYCL_24M_CNT_MS(3)); - - mmio_setbits_32(PMU_BASE + PMU_SFT_CON, BIT(PMU_24M_EN_CFG)); -} - static void set_hw_idle(uint32_t hw_idle) { mmio_setbits_32(PMU_BASE + PMU_BUS_CLR, hw_idle); @@ -899,8 +942,6 @@ static int sys_pwr_domain_resume(void) enable_dvfs_plls(); plls_resume_finish(); - sys_slp_unconfig(); - mmio_write_32(SGRF_BASE + SGRF_SOC_CON0_1(1), (cpu_warm_boot_addr >> CPU_BOOT_ADDR_ALIGN) | CPU_BOOT_ADDR_WMASK); @@ -1037,6 +1078,8 @@ void plat_rockchip_pmu_init(void) CPU_BOOT_ADDR_WMASK); mmio_write_32(PMU_BASE + PMU_NOC_AUTO_ENA, NOC_AUTO_ENABLE); + init_pmu_counts(); + nonboot_cpus_off(); INFO("%s(%d): pd status %x\n", __func__, __LINE__, From 9d5aee2b1fdcb0d7df70b0a13622b09780fbc9dc Mon Sep 17 00:00:00 2001 From: Caesar Wang Date: Thu, 25 Aug 2016 06:31:32 +0800 Subject: [PATCH 2/4] rockchip: on rk3399 enable Schmitt trigger on 32 kHz clock If we don't enable the Schmitt trigger on the 32 kHz clock then systems won't always resume from suspend properly. Presumably anything else in the system that relies on the 32 kHz clock also will have problems without the Schmitt trigger enabled. Enable it always since having the 32 kHz clock on GPIO0_A0 isn't exactly an optional feature, so all boards using rk3399 will need this. Change-Id: Idc18c6cd1adc5be5f60efd9cb805d83d5cd40129 --- plat/rockchip/rk3399/drivers/pmu/pmu.c | 6 ++++++ plat/rockchip/rk3399/drivers/pmu/pmu.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu.c b/plat/rockchip/rk3399/drivers/pmu/pmu.c index 9fc796cc4..00df0858d 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3399/drivers/pmu/pmu.c @@ -1078,6 +1078,12 @@ void plat_rockchip_pmu_init(void) CPU_BOOT_ADDR_WMASK); mmio_write_32(PMU_BASE + PMU_NOC_AUTO_ENA, NOC_AUTO_ENABLE); + /* + * Enable Schmitt trigger for better 32 kHz input signal, which is + * important for suspend/resume reliability among other things. + */ + mmio_write_32(PMUGRF_BASE + PMUGRF_GPIO0A_SMT, GPIO0A0_SMT_ENABLE); + init_pmu_counts(); nonboot_cpus_off(); diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu.h b/plat/rockchip/rk3399/drivers/pmu/pmu.h index c821efc0f..65fe7dbe0 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu.h +++ b/plat/rockchip/rk3399/drivers/pmu/pmu.h @@ -819,6 +819,7 @@ enum pmu_core_pwr_st { #define AP_PWROFF 0x0a +#define GPIO0A0_SMT_ENABLE BITS_WITH_WMASK(1, 3, 0) #define GPIO1A6_IOMUX BITS_WITH_WMASK(0, 3, 12) #define TSADC_INT_PIN 38 @@ -876,6 +877,7 @@ enum pmu_core_pwr_st { #define GRF_SOC_CON4 0x0e210 #define GRF_GPIO4C_IOMUX 0x0e028 +#define PMUGRF_GPIO0A_SMT 0x0120 #define PMUGRF_SOC_CON0 0x0180 #define CCI_FORCE_WAKEUP WMSK_BIT(8) From b3464232513bc9cb3572a0b42a7cf6dcde8ec255 Mon Sep 17 00:00:00 2001 From: Caesar Wang Date: Tue, 23 Aug 2016 12:52:59 -0700 Subject: [PATCH 3/4] rockchip: remove the unused code for rk3399 Change-Id: I986d64df9dc62354d50ccea0468b90f090a44160 Signed-off-by: Caesar Wang --- plat/rockchip/rk3399/drivers/pmu/pmu.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu.c b/plat/rockchip/rk3399/drivers/pmu/pmu.c index 00df0858d..827f48308 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3399/drivers/pmu/pmu.c @@ -855,8 +855,6 @@ static void sys_slp_config(void) BIT(PMU_OSC_DIS) | BIT(PMU_PMU_USE_LF); - mmio_setbits_32(PMU_BASE + PMU_WKUP_CFG4, BIT(PMU_CLUSTER_L_WKUP_EN)); - mmio_setbits_32(PMU_BASE + PMU_WKUP_CFG4, BIT(PMU_CLUSTER_B_WKUP_EN)); mmio_setbits_32(PMU_BASE + PMU_WKUP_CFG4, BIT(PMU_GPIO_WKUP_EN)); mmio_write_32(PMU_BASE + PMU_PWRMODE_CON, slp_mode_cfg); From bdb2763d64e41a2a18f7ec77b51a2e69ebb512e1 Mon Sep 17 00:00:00 2001 From: Caesar Wang Date: Wed, 17 Aug 2016 17:22:10 -0700 Subject: [PATCH 4/4] rockchip: handle some interrupt before enter power mode for rk3399 For the PMU design, we don't expect to get the interrupts before enter the power mode. Since that will cause the confusion for the state machine in the power mode. Change-Id: Id8dee79ae617a66271b5caf92caf35f520f45099 --- plat/rockchip/common/include/plat_private.h | 1 + plat/rockchip/common/plat_pm.c | 13 +++++ plat/rockchip/rk3399/drivers/pmu/pmu.c | 58 ++++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/plat/rockchip/common/include/plat_private.h b/plat/rockchip/common/include/plat_private.h index 71998998d..b9b634e7b 100644 --- a/plat/rockchip/common/include/plat_private.h +++ b/plat/rockchip/common/include/plat_private.h @@ -56,6 +56,7 @@ struct rockchip_pm_ops_cb { int (*sys_pwr_dm_resume)(void); void (*sys_gbl_soft_reset)(void) __dead2; void (*system_off)(void) __dead2; + void (*sys_pwr_down_wfi)(const psci_power_state_t *state_info) __dead2; }; /****************************************************************************** diff --git a/plat/rockchip/common/plat_pm.c b/plat/rockchip/common/plat_pm.c index b6291bbf0..7372fcff9 100644 --- a/plat/rockchip/common/plat_pm.c +++ b/plat/rockchip/common/plat_pm.c @@ -311,6 +311,18 @@ static void __dead2 rockchip_system_poweroff(void) rockchip_ops->system_off(); } +static void +__dead2 rockchip_pwr_domain_pwr_down_wfi(const psci_power_state_t *target_state) +{ + if ((RK_CORE_PWR_STATE(target_state) == PLAT_MAX_OFF_STATE) && + (rockchip_ops)) { + if (RK_SYSTEM_PWR_STATE(target_state) == PLAT_MAX_OFF_STATE && + rockchip_ops->sys_pwr_down_wfi) + rockchip_ops->sys_pwr_down_wfi(target_state); + } + psci_power_down_wfi(); +} + /******************************************************************************* * Export the platform handlers via plat_rockchip_psci_pm_ops. The rockchip * standard @@ -323,6 +335,7 @@ const plat_psci_ops_t plat_rockchip_psci_pm_ops = { .pwr_domain_suspend = rockchip_pwr_domain_suspend, .pwr_domain_on_finish = rockchip_pwr_domain_on_finish, .pwr_domain_suspend_finish = rockchip_pwr_domain_suspend_finish, + .pwr_domain_pwr_down_wfi = rockchip_pwr_domain_pwr_down_wfi, .system_reset = rockchip_system_reset, .system_off = rockchip_system_poweroff, .validate_power_state = rockchip_validate_power_state, diff --git a/plat/rockchip/rk3399/drivers/pmu/pmu.c b/plat/rockchip/rk3399/drivers/pmu/pmu.c index 827f48308..d2d1acde0 100644 --- a/plat/rockchip/rk3399/drivers/pmu/pmu.c +++ b/plat/rockchip/rk3399/drivers/pmu/pmu.c @@ -47,6 +47,7 @@ #include #include #include +#include DEFINE_BAKERY_LOCK(rockchip_pd_lock); @@ -756,8 +757,9 @@ static int hlvl_pwr_domain_resume(uint32_t lvl, plat_local_state_t lvl_state) * * Also note that these counters can run off the 32 kHz clock or the 24 MHz * clock. While the 24 MHz clock can give us more precision, it's not always - * available (like when we turn the oscillator off at sleep time). Current - * understanding is that counts work like this: + * available (like when we turn the oscillator off at sleep time). The + * pmu_use_lf (lf: low freq) is available in power mode. Current understanding + * is that counts work like this: * IF (pmu_use_lf == 0) || (power_mode_en == 0) * use the 24M OSC for counts * ELSE @@ -920,6 +922,10 @@ static int sys_pwr_domain_suspend(void) } mmio_setbits_32(PMU_BASE + PMU_PWRDN_CON, BIT(PMU_SCU_B_PWRDWN_EN)); + /* + * Disabling PLLs/PWM/DVFS is approaching WFI which is + * the last steps in suspend. + */ plls_suspend_prepare(); disable_dvfs_plls(); disable_pwms(); @@ -940,6 +946,17 @@ static int sys_pwr_domain_resume(void) enable_dvfs_plls(); plls_resume_finish(); + /* + * The wakeup status is not cleared by itself, we need to clear it + * manually. Otherwise we will alway query some interrupt next time. + * + * NOTE: If the kernel needs to query this, we might want to stash it + * somewhere. + */ + mmio_write_32(PMU_BASE + PMU_WAKEUP_STATUS, 0xffffffff); + + mmio_write_32(PMU_BASE + PMU_WKUP_CFG4, 0x00); + mmio_write_32(SGRF_BASE + SGRF_SOC_CON0_1(1), (cpu_warm_boot_addr >> CPU_BOOT_ADDR_ALIGN) | CPU_BOOT_ADDR_WMASK); @@ -1032,6 +1049,42 @@ void __dead2 soc_system_off(void) while (1) ; } +static void __dead2 sys_pwr_down_wfi(const psci_power_state_t *target_state) +{ + uint32_t wakeup_status; + + /* + * Check wakeup status and abort suspend early if we see a wakeup + * event. + * + * NOTE: technically I we're supposed to just execute a wfi here and + * we'll either execute a normal suspend/resume or the wfi will be + * treated as a no-op if a wake event was present and caused an abort + * of the suspend/resume. For some reason that's not happening and if + * we execute the wfi while a wake event is pending then the whole + * system wedges. + * + * Until the above is solved this extra check prevents system wedges in + * most cases but there is still a small race condition between checking + * PMU_WAKEUP_STATUS and executing wfi. If a wake event happens in + * there then we will die. + */ + wakeup_status = mmio_read_32(PMU_BASE + PMU_WAKEUP_STATUS); + if (wakeup_status) { + WARN("early wake, will not enter power mode.\n"); + + mmio_write_32(PMU_BASE + PMU_PWRMODE_CON, 0); + + disable_mmu_icache_el3(); + bl31_warm_entrypoint(); + + while (1) + ; + } else { + /* Enter WFI */ + psci_power_down_wfi(); + } +} static struct rockchip_pm_ops_cb pm_ops = { .cores_pwr_dm_on = cores_pwr_domain_on, @@ -1047,6 +1100,7 @@ static struct rockchip_pm_ops_cb pm_ops = { .sys_pwr_dm_resume = sys_pwr_domain_resume, .sys_gbl_soft_reset = soc_soft_reset, .system_off = soc_system_off, + .sys_pwr_down_wfi = sys_pwr_down_wfi, }; void plat_rockchip_pmu_init(void)