Merge pull request #691 from rockchip-linux/fixes-suspend/resume-bugs

Fixes suspend/resume bugs

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;
 };
 
 /******************************************************************************

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,

plat/rockchip/rk3399/drivers/pmu/pmu.c

@@ -47,6 +47,7 @@
 #include <pmu_com.h>
 #include <pwm.h>
 #include <soc.h>
+#include <bl31.h>
 
 DEFINE_BAKERY_LOCK(rockchip_pd_lock);
 
@@ -734,6 +735,90 @@ 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). 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
+ *      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;
@@ -772,57 +857,15 @@ 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);
 
-	/*
-	 * 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);
@@ -879,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();
@@ -899,7 +946,16 @@ static int sys_pwr_domain_resume(void)
 	enable_dvfs_plls();
 	plls_resume_finish();
 
-	sys_slp_unconfig();
+	/*
+	 * 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) |
@@ -993,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,
@@ -1008,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)
@@ -1037,6 +1130,14 @@ 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();
 
 	INFO("%s(%d): pd status %x\n", __func__, __LINE__,

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)