[PATCH 0/6] Idle status patches revisited

[PATCH 0/6] Idle status patches revisited

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Following set implements checks for idle states of powerdomains, and
changes target cpuidle state according to those. This also includes as
a cleanup removal of some hacks from omap_sram_idle(), and also introduces
INACTIVE / ON state support for powerdomains which simplifies the code.

Applies on top of PM branch.

[PATCH 1/6] OMAP: Powerdomains: Add support for INACTIVE state on pwrdm level

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Currently only ON, RET and OFF are supported, and ON is arguably broken as it
allows the powerdomain to enter INACTIVE state unless idle is prevented.
Now, pwrdm code prevents idle if ON is selected, and also adds support for
INACTIVE. This simplifies the control needed inside sleep code.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/powerdomain.c             |   32 +++++++++++++++++++++----
 arch/arm/mach-omap2/powerdomains34xx.h        |   26 ++++++++++----------
 arch/arm/plat-omap/include/plat/powerdomain.h |    6 ++++-
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index b6990e3..1237717 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -112,8 +112,8 @@ static struct powerdomain *_pwrdm_deps_lookup(struct powerdomain *pwrdm,
 static int _pwrdm_state_switch(struct powerdomain *pwrdm, int flag)
 {
 
-	int prev;
-	int state;
+	u8 prev;
+	u8 state;
 
 	if (pwrdm == NULL)
 		return -EINVAL;
@@ -220,7 +220,7 @@ int pwrdm_register(struct powerdomain *pwrdm)
 
 	pr_debug("powerdomain: registered %s\n", pwrdm->name);
 	ret = 0;
-
+	pwrdm->next_state = -1;
 pr_unlock:
 	write_unlock_irqrestore(&pwrdm_rwlock, flags);
 
@@ -701,19 +701,38 @@ int pwrdm_get_mem_bank_count(struct powerdomain *pwrdm)
  */
 int pwrdm_set_next_pwrst(struct powerdomain *pwrdm, u8 pwrst)
 {
+	u8 prg_pwrst;
+
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->next_state == pwrst)
+		return 0;
+
 	if (!(pwrdm->pwrsts & (1 << pwrst)))
 		return -EINVAL;
 
 	pr_debug("powerdomain: setting next powerstate for %s to %0x\n",
 		 pwrdm->name, pwrst);
 
+	/* INACTIVE is reserved, so we program pwrdm as ON */
+	if (pwrst == PWRDM_POWER_INACTIVE)
+		prg_pwrst = PWRDM_POWER_ON;
+	else
+		prg_pwrst = pwrst;
+
 	prm_rmw_mod_reg_bits(OMAP_POWERSTATE_MASK,
-			     (pwrst << OMAP_POWERSTATE_SHIFT),
+			     (prg_pwrst << OMAP_POWERSTATE_SHIFT),
 			     pwrdm->prcm_offs, PM_PWSTCTRL);
 
+	/* If next state is ON, prevent idle */
+	if (pwrst == PWRDM_POWER_ON)
+		omap2_clkdm_deny_idle(pwrdm->pwrdm_clkdms[0]);
+	else
+		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
+
+	pwrdm->next_state = pwrst;
+
 	return 0;
 }
 
@@ -730,8 +749,11 @@ int pwrdm_read_next_pwrst(struct powerdomain *pwrdm)
 	if (!pwrdm)
 		return -EINVAL;
 
+	if (pwrdm->next_state > -1)
+		return pwrdm->next_state;
+
 	return prm_read_mod_bits_shift(pwrdm->prcm_offs, PM_PWSTCTRL,
-					OMAP_POWERSTATE_MASK);
+				       OMAP_POWERSTATE_MASK);
 }
 
 /**
diff --git a/arch/arm/mach-omap2/powerdomains34xx.h b/arch/arm/mach-omap2/powerdomains34xx.h
index fd09b08..9eb2dc5 100644
--- a/arch/arm/mach-omap2/powerdomains34xx.h
+++ b/arch/arm/mach-omap2/powerdomains34xx.h
@@ -165,7 +165,7 @@ static struct powerdomain iva2_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_IVA2_SHIFT,
 	.wkdep_srcs	  = iva2_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 4,
 	.pwrsts_mem_ret	  = {
@@ -188,7 +188,7 @@ static struct powerdomain mpu_34xx_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.dep_bit	  = OMAP3430_EN_MPU_SHIFT,
 	.wkdep_srcs	  = mpu_34xx_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -206,7 +206,7 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430ES1 |
 					   CHIP_IS_OMAP3430ES2 |
 					   CHIP_IS_OMAP3430ES3_0),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
 	.banks		  = 2,
 	.pwrsts_mem_ret	  = {
@@ -214,8 +214,8 @@ static struct powerdomain core_34xx_pre_es3_1_pwrdm = {
 		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
 	},
 	.pwrsts_mem_on	  = {
-		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
-		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
+		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
+		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
 	},
 };
 
@@ -224,7 +224,7 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
 	.name		  = "core_pwrdm",
 	.prcm_offs	  = CORE_MOD,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES3_1),
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.dep_bit	  = OMAP3430_EN_CORE_SHIFT,
 	.flags		  = PWRDM_HAS_HDWR_SAR, /* for USBTLL only */
 	.banks		  = 2,
@@ -233,8 +233,8 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
 		[1] = PWRSTS_OFF_RET,	 /* MEM2RETSTATE */
 	},
 	.pwrsts_mem_on	  = {
-		[0] = PWRSTS_OFF_RET_ON, /* MEM1ONSTATE */
-		[1] = PWRSTS_OFF_RET_ON, /* MEM2ONSTATE */
+		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
+		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
 	},
 };
 
@@ -246,7 +246,7 @@ static struct powerdomain dss_pwrdm = {
 	.dep_bit	  = OMAP3430_PM_WKDEP_MPU_EN_DSS_SHIFT,
 	.wkdep_srcs	  = cam_dss_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -286,7 +286,7 @@ static struct powerdomain cam_pwrdm = {
 	.prcm_offs	  = OMAP3430_CAM_MOD,
 	.wkdep_srcs	  = cam_dss_wkdeps,
 	.sleepdep_srcs	  = cam_gfx_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -304,7 +304,7 @@ static struct powerdomain per_pwrdm = {
 	.dep_bit	  = OMAP3430_EN_PER_SHIFT,
 	.wkdep_srcs	  = per_usbhost_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRSTS_OFF_RET,
 	.banks		  = 1,
 	.pwrsts_mem_ret	  = {
@@ -326,7 +326,7 @@ static struct powerdomain neon_pwrdm = {
 	.prcm_offs	  = OMAP3430_NEON_MOD,
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 	.wkdep_srcs	  = neon_wkdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 };
 
@@ -336,7 +336,7 @@ static struct powerdomain usbhost_pwrdm = {
 	.omap_chip	  = OMAP_CHIP_INIT(CHIP_GE_OMAP3430ES2),
 	.wkdep_srcs	  = per_usbhost_wkdeps,
 	.sleepdep_srcs	  = dss_per_usbhost_sleepdeps,
-	.pwrsts		  = PWRSTS_OFF_RET_ON,
+	.pwrsts		  = PWRSTS_OFF_RET_INA_ON,
 	.pwrsts_logic_ret = PWRDM_POWER_RET,
 	/*
 	 * REVISIT: Enabling usb host save and restore mechanism seems to
diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
index 3d45ee1..55350d0 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -37,6 +37,9 @@
 
 #define PWRSTS_OFF_RET_ON	(PWRSTS_OFF_RET | (1 << PWRDM_POWER_ON))
 
+#define PWRSTS_OFF_RET_INA_ON	(PWRSTS_OFF_RET_ON | \
+				 (1 << PWRDM_POWER_INACTIVE))
+
 
 /* Powerdomain flags */
 #define PWRDM_HAS_HDWR_SAR	(1 << 0) /* hardware save-and-restore support */
@@ -117,7 +120,8 @@ struct powerdomain {
 
 	struct list_head node;
 
-	int state;
+	u8 state;
+	s8 next_state;
 	unsigned state_counter[4];
 
 #ifdef CONFIG_PM_DEBUG
-- 
1.5.4.3

[PATCH 2/6] OMAP3: PM: Added support for INACTIVE and ON states for powerdomains

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Previously omap_sram_idle() did not know about the difference between ON and
INACTIVE states, which complicated the state handling in these cases.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9d0a9b4..7b52f30 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -383,6 +383,7 @@ void omap_sram_idle(void)
 	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
 	switch (mpu_next_state) {
 	case PWRDM_POWER_ON:
+	case PWRDM_POWER_INACTIVE:
 	case PWRDM_POWER_RET:
 		/* No need to save context */
 		save_state = 0;
@@ -448,9 +449,11 @@ void omap_sram_idle(void)
 						OMAP3430_GR_MOD,
 						OMAP3_PRM_VOLTCTRL_OFFSET);
 		}
-		/* Enable IO-PAD and IO-CHAIN wakeups */
-		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
-		omap3_enable_io_chain();
+		if (core_next_state <= PWRDM_POWER_RET) {
+			/* Enable IO-PAD and IO-CHAIN wakeups */
+			prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
+			omap3_enable_io_chain();
+		}
 	}
 	omap3_intc_prepare_idle();
 
@@ -542,15 +545,13 @@ void omap_sram_idle(void)
 	}
 
 	/* Disable IO-PAD and IO-CHAIN wakeup */
-	if (core_next_state < PWRDM_POWER_ON) {
+	if (core_next_state <= PWRDM_POWER_ON) {
 		prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
 		omap3_disable_io_chain();
 	}
 
 
 	pwrdm_post_transition();
-
-	omap2_clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
 int omap3_can_sleep(void)
@@ -598,7 +599,6 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
 	}
 
 	if (sleep_switch) {
-		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
 		pwrdm_wait_transition(pwrdm);
 		pwrdm_state_switch(pwrdm);
 	}
-- 
1.5.4.3

[PATCH 3/6] OMAP3: CPUidle: Fixed support for ON / INACTIVE states

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

New powerdomain code support for INACTIVE state removes the need to control
clockdomains directly from cpuidle. Also, cpuidle state definitions can now
directly support ON / INACTIVE simplifying the implementation.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   32 ++++----------------------------
 1 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index fdfa1d5..e46345f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -91,20 +91,6 @@ static int omap3_idle_bm_check(void)
 	return 0;
 }
 
-static int _cpuidle_allow_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
-{
-	omap2_clkdm_allow_idle(clkdm);
-	return 0;
-}
-
-static int _cpuidle_deny_idle(struct powerdomain *pwrdm,
-				struct clockdomain *clkdm)
-{
-	omap2_clkdm_deny_idle(clkdm);
-	return 0;
-}
-
 /**
  * omap3_enter_idle - Programs OMAP3 to enter the specified state
  * @dev: cpuidle device
@@ -141,19 +127,9 @@ static int omap3_enter_idle(struct cpuidle_device *dev,
 	if (omap_irq_pending() || need_resched())
 		goto return_sleep_time;
 
-	if (cx->type == OMAP3_STATE_C1) {
-		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
-		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
-	}
-
 	/* Execute ARM wfi */
 	omap_sram_idle();
 
-	if (cx->type == OMAP3_STATE_C1) {
-		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
-		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
-	}
-
 return_sleep_time:
 	getnstimeofday(&ts_postidle);
 	ts_idle = timespec_sub(ts_postidle, ts_preidle);
@@ -242,8 +218,8 @@ void omap_init_power_states(void)
 			cpuidle_params_table[OMAP3_STATE_C2].wake_latency;
 	omap3_power_states[OMAP3_STATE_C2].threshold =
 			cpuidle_params_table[OMAP3_STATE_C2].threshold;
-	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON;
-	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_INACTIVE;
+	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID;
 
 	/* C3 . MPU CSWR + Core inactive */
@@ -256,7 +232,7 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C3].threshold =
 			cpuidle_params_table[OMAP3_STATE_C3].threshold;
 	omap3_power_states[OMAP3_STATE_C3].mpu_state = PWRDM_POWER_RET;
-	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C3].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C3].flags = CPUIDLE_FLAG_TIME_VALID |
 				CPUIDLE_FLAG_CHECK_BM;
 
@@ -270,7 +246,7 @@ void omap_init_power_states(void)
 	omap3_power_states[OMAP3_STATE_C4].threshold =
 			cpuidle_params_table[OMAP3_STATE_C4].threshold;
 	omap3_power_states[OMAP3_STATE_C4].mpu_state = PWRDM_POWER_OFF;
-	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_ON;
+	omap3_power_states[OMAP3_STATE_C4].core_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C4].flags = CPUIDLE_FLAG_TIME_VALID |
 				CPUIDLE_FLAG_CHECK_BM;
 
-- 
1.5.4.3

[PATCH 4/6] OMAP3: PM: Removed PER + CORE state hacking from omap_sram_idle

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

This should be moved inside cpuidle to simplify design.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/pm34xx.c |   18 ++----------------
 1 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 7b52f30..c704756 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -81,7 +81,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
 
 static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
 static struct powerdomain *core_pwrdm, *per_pwrdm;
-static struct powerdomain *cam_pwrdm;
 
 static struct prm_setup_times_vc prm_setup_times_default = {
 	.clksetup = 0xff,
@@ -370,7 +369,6 @@ void omap_sram_idle(void)
 	int core_next_state = PWRDM_POWER_ON;
 	int core_prev_state, per_prev_state;
 	u32 sdrc_pwr = 0;
-	int per_state_modified = 0;
 
 	if (!_omap_sram_idle)
 		return;
@@ -409,19 +407,10 @@ void omap_sram_idle(void)
 	if (per_next_state < PWRDM_POWER_ON) {
 		omap_uart_prepare_idle(2);
 		omap2_gpio_prepare_for_idle(per_next_state);
-		if (per_next_state == PWRDM_POWER_OFF) {
-			if (core_next_state == PWRDM_POWER_ON) {
-				per_next_state = PWRDM_POWER_RET;
-				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
-				per_state_modified = 1;
-			} else
-				omap3_per_save_context();
-		}
+		if (per_next_state == PWRDM_POWER_OFF)
+			omap3_per_save_context();
 	}
 
-	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
-		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
-
 	/*
 	 * Disable smartreflex before entering WFI.
 	 * Only needed if we are going to enter retention or off.
@@ -540,8 +529,6 @@ void omap_sram_idle(void)
 			omap3_gpio_restore_pad_context(1);
 		omap2_gpio_resume_after_idle();
 		omap_uart_resume_idle(2);
-		if (per_state_modified)
-			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
 	}
 
 	/* Disable IO-PAD and IO-CHAIN wakeup */
@@ -1170,7 +1157,6 @@ static int __init omap3_pm_init(void)
 	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
 	per_pwrdm = pwrdm_lookup("per_pwrdm");
 	core_pwrdm = pwrdm_lookup("core_pwrdm");
-	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
 
 	omap_push_sram_idle();
 #ifdef CONFIG_SUSPEND
-- 
1.5.4.3

[PATCH 5/6] OMAP: Powerdomains: Add support for checking if pwrdm can idle

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

pwrdm_can_idle(pwrdm) will check if the specified powerdomain can enter
idle. This is done by checking the current fclk enable bits.

This call can be used e.g. inside cpuidle to decide which power states
core and mpu should enter during idle, as there are certain dependencies
between wakeup capabilities and reset logic.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/powerdomain.c             |   22 ++++++++++++++++++++++
 arch/arm/mach-omap2/powerdomains34xx.h        |   14 ++++++++++++++
 arch/arm/plat-omap/include/plat/powerdomain.h |    9 +++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
index 1237717..bf2b97a 100644
--- a/arch/arm/mach-omap2/powerdomain.c
+++ b/arch/arm/mach-omap2/powerdomain.c
@@ -1217,6 +1217,28 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
 	return 0;
 }
 
+/**
+ * pwrdm_can_idle - check if the powerdomain can enter idle
+ * @pwrdm: struct powerdomain * the powerdomain to check status of
+ *
+ * Does a functional clock check for the powerdomain and returns 1 if the
+ * powerdomain can enter idle, 0 if not.
+ */
+int pwrdm_can_idle(struct powerdomain *pwrdm)
+{
+	int i;
+	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
+
+	if (!pwrdm)
+		return -EINVAL;
+
+	for (i = 0; i < pwrdm->fclk_reg_amt; i++)
+		if (cm_read_mod_reg(pwrdm->prcm_offs, fclk_regs[i]) &
+				(0xffffffff ^ pwrdm->fclk_masks[i]))
+			return 0;
+	return 1;
+}
+
 int pwrdm_state_switch(struct powerdomain *pwrdm)
 {
 	return _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
diff --git a/arch/arm/mach-omap2/powerdomains34xx.h b/arch/arm/mach-omap2/powerdomains34xx.h
index 9eb2dc5..c8cd297 100644
--- a/arch/arm/mach-omap2/powerdomains34xx.h
+++ b/arch/arm/mach-omap2/powerdomains34xx.h
@@ -180,6 +180,7 @@ static struct powerdomain iva2_pwrdm = {
 		[2] = PWRSTS_OFF_ON,
 		[3] = PWRDM_POWER_ON,
 	},
+	.fclk_reg_amt	  = 1,
 };
 
 static struct powerdomain mpu_34xx_pwrdm = {
@@ -236,6 +237,11 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
 		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
 		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
 	},
+	.fclk_reg_amt	  = 2,
+	.fclk_masks	  = {
+		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
+		[1] = 0,
+	},
 };
 
 /* Another case of bit name collisions between several registers: EN_DSS */
@@ -255,6 +261,7 @@ static struct powerdomain dss_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
 	},
+	.fclk_reg_amt	  = 1,
 };
 
 /*
@@ -278,6 +285,7 @@ static struct powerdomain sgx_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
 	},
+	.fclk_reg_amt	  = 1,
 };
 
 static struct powerdomain cam_pwrdm = {
@@ -295,6 +303,7 @@ static struct powerdomain cam_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
 	},
+	.fclk_reg_amt	  = 1,
 };
 
 static struct powerdomain per_pwrdm = {
@@ -313,6 +322,10 @@ static struct powerdomain per_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
 	},
+	.fclk_reg_amt	  = 1,
+	.fclk_masks	  = {
+		[0] = OMAP3430_EN_UART3,
+	},
 };
 
 static struct powerdomain emu_pwrdm = {
@@ -352,6 +365,7 @@ static struct powerdomain usbhost_pwrdm = {
 	.pwrsts_mem_on	  = {
 		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
 	},
+	.fclk_reg_amt	  = 1,
 };
 
 static struct powerdomain dpll1_pwrdm = {
diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
index 55350d0..b004d88 100644
--- a/arch/arm/plat-omap/include/plat/powerdomain.h
+++ b/arch/arm/plat-omap/include/plat/powerdomain.h
@@ -57,6 +57,12 @@
  */
 #define PWRDM_MAX_CLKDMS	4
 
+/*
+ * Maximum number of FCLK register masks that can be associated with a
+ * powerdomain. CORE powerdomain on OMAP3 is the worst case
+ */
+#define PWRDM_MAX_FCLK		2
+
 /* XXX A completely arbitrary number. What is reasonable here? */
 #define PWRDM_TRANSITION_BAILOUT 100000
 
@@ -124,6 +130,8 @@ struct powerdomain {
 	s8 next_state;
 	unsigned state_counter[4];
 
+	u8 fclk_reg_amt;
+	u32 fclk_masks[PWRDM_MAX_FCLK];
 #ifdef CONFIG_PM_DEBUG
 	s64 timer;
 	s64 state_timer[4];
@@ -177,6 +185,7 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
 bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
 
 int pwrdm_wait_transition(struct powerdomain *pwrdm);
+int pwrdm_can_idle(struct powerdomain *pwrdm);
 
 int pwrdm_state_switch(struct powerdomain *pwrdm);
 int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
-- 
1.5.4.3

[PATCH 6/6] OMAP3: CPUidle: Added peripheral pwrdm checks into bm check

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Following checks are made (and their reasoning):

- If CAM domain is active, prevent idle completely
  * CAM pwrdm does not have HW wakeup capability
- If PER is likely to remain on, prevent PER off
  * Saves on unnecessary context save/restore
- If CORE domain is active, prevent PER off-mode
  * PER off in this case would prevent wakeups from PER completely
- Only allow CORE off, if all peripheral domains are off
  * CORE off will cause a chipwide reset

Also, enabled CHECK_BM flag for C2, as this is needed for the camera case.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |  105 ++++++++++++++++++++++++++++++++++---
 1 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index e46345f..4654e87 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -58,7 +58,8 @@ struct omap3_processor_cx {
 
 struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
 struct omap3_processor_cx current_cx_state;
-struct powerdomain *mpu_pd, *core_pd;
+struct powerdomain *mpu_pd, *core_pd, *per_pd, *iva2_pd;
+struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
 
 /*
  * The latencies/thresholds for various C states have
@@ -91,6 +92,13 @@ static int omap3_idle_bm_check(void)
 	return 0;
 }
 
+static int pwrdm_get_idle_state(struct powerdomain *pwrdm)
+{
+	if (pwrdm_can_idle(pwrdm))
+		return pwrdm_read_next_pwrst(pwrdm);
+	return PWRDM_POWER_ON;
+}
+
 /**
  * omap3_enter_idle - Programs OMAP3 to enter the specified state
  * @dev: cpuidle device
@@ -153,14 +161,90 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 			       struct cpuidle_state *state)
 {
 	struct cpuidle_state *new_state = state;
-
-	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
-		BUG_ON(!dev->safe_state);
-		new_state = dev->safe_state;
+	u32 per_state = 0, saved_per_state = 0, cam_state, usb_state;
+	u32 iva2_state, sgx_state, dss_state, new_core_state;
+	struct omap3_processor_cx *cx;
+	int ret;
+
+	if (state->flags & CPUIDLE_FLAG_CHECK_BM) {
+		if (omap3_idle_bm_check()) {
+			BUG_ON(!dev->safe_state);
+			new_state = dev->safe_state;
+			goto select_state;
+		}
+		cx = cpuidle_get_statedata(state);
+		new_core_state = cx->core_state;
+
+		/* Check if CORE is active, if yes, fallback to inactive */
+		if (!pwrdm_can_idle(core_pd))
+			new_core_state = PWRDM_POWER_INACTIVE;
+
+		/*
+		 * Prevent idle completely if CAM is active.
+		 * CAM does not have wakeup capability in OMAP3.
+		 */
+		cam_state = pwrdm_get_idle_state(cam_pd);
+		if (cam_state == PWRDM_POWER_ON) {
+			new_state = dev->safe_state;
+			goto select_state;
+		}
+
+		/*
+		 * Check if PER can idle or not. If we are not likely
+		 * to idle, deny PER off. This prevents unnecessary
+		 * context save/restore.
+		 */
+		saved_per_state = pwrdm_read_next_pwrst(per_pd);
+		if (pwrdm_can_idle(per_pd)) {
+			per_state = saved_per_state;
+			/*
+			 * Prevent PER off if CORE is active as this
+			 * would disable PER wakeups completely
+			 */
+			if (per_state == PWRDM_POWER_OFF &&
+			    new_core_state > PWRDM_POWER_RET)
+				per_state = PWRDM_POWER_RET;
+
+		} else if (saved_per_state == PWRDM_POWER_OFF)
+			per_state = PWRDM_POWER_RET;
+
+		/*
+		 * If we are attempting CORE off, check if any other
+		 * powerdomains are at retention or higher. CORE off causes
+		 * chipwide reset which would reset these domains also.
+		 */
+		if (new_core_state == PWRDM_POWER_OFF) {
+			dss_state = pwrdm_get_idle_state(dss_pd);
+			iva2_state = pwrdm_get_idle_state(iva2_pd);
+			sgx_state = pwrdm_get_idle_state(sgx_pd);
+			usb_state = pwrdm_get_idle_state(usb_pd);
+
+			if (cam_state > PWRDM_POWER_OFF ||
+			    dss_state > PWRDM_POWER_OFF ||
+			    iva2_state > PWRDM_POWER_OFF ||
+			    per_state > PWRDM_POWER_OFF ||
+			    sgx_state > PWRDM_POWER_OFF ||
+			    usb_state > PWRDM_POWER_OFF)
+				new_core_state = PWRDM_POWER_RET;
+		}
+		/* Fallback to new target core state */
+		while (cx->core_state > new_core_state) {
+			state--;
+			cx = cpuidle_get_statedata(state);
+		}
+		new_state = state;
+		/* Are we changing PER target state? */
+		if (per_state != saved_per_state)
+			pwrdm_set_next_pwrst(per_pd, per_state);
 	}
 
+select_state:
 	dev->last_state = new_state;
-	return omap3_enter_idle(dev, new_state);
+	ret = omap3_enter_idle(dev, new_state);
+	/* Restore potentially tampered PER state */
+	if (per_state != saved_per_state)
+		pwrdm_set_next_pwrst(per_pd, saved_per_state);
+	return ret;
 }
 
 DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
@@ -220,7 +304,8 @@ void omap_init_power_states(void)
 			cpuidle_params_table[OMAP3_STATE_C2].threshold;
 	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_INACTIVE;
 	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_INACTIVE;
-	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID;
+	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
+				CPUIDLE_FLAG_CHECK_BM;
 
 	/* C3 . MPU CSWR + Core inactive */
 	omap3_power_states[OMAP3_STATE_C3].valid = 1;
@@ -313,6 +398,12 @@ int __init omap3_idle_init(void)
 
 	mpu_pd = pwrdm_lookup("mpu_pwrdm");
 	core_pd = pwrdm_lookup("core_pwrdm");
+	per_pd = pwrdm_lookup("per_pwrdm");
+	iva2_pd = pwrdm_lookup("iva2_pwrdm");
+	sgx_pd = pwrdm_lookup("sgx_pwrdm");
+	usb_pd = pwrdm_lookup("usbhost_pwrdm");
+	cam_pd = pwrdm_lookup("cam_pwrdm");
+	dss_pd = pwrdm_lookup("dss_pwrdm");
 
 	omap_init_power_states();
 	cpuidle_register_driver(&omap3_idle_driver);
-- 
1.5.4.3

Re: [PATCH 6/6] OMAP3: CPUidle: Added peripheral pwrdm checks into bm check

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Following checks are made (and their reasoning):
>
> - If CAM domain is active, prevent idle completely
>   * CAM pwrdm does not have HW wakeup capability
> - If PER is likely to remain on, prevent PER off
>   * Saves on unnecessary context save/restore
> - If CORE domain is active, prevent PER off-mode
>   * PER off in this case would prevent wakeups from PER completely
> - Only allow CORE off, if all peripheral domains are off
>   * CORE off will cause a chipwide reset
>
> Also, enabled CHECK_BM flag for C2, as this is needed for the camera case.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

Some questions and a couple minor style comments below...

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |  105 ++++++++++++++++++++++++++++++++++---
>  1 files changed, 98 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index e46345f..4654e87 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -58,7 +58,8 @@ struct omap3_processor_cx {
>  
>  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
>  struct omap3_processor_cx current_cx_state;
> -struct powerdomain *mpu_pd, *core_pd;
> +struct powerdomain *mpu_pd, *core_pd, *per_pd, *iva2_pd;
> +struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
>  
>  /*
>   * The latencies/thresholds for various C states have
> @@ -91,6 +92,13 @@ static int omap3_idle_bm_check(void)
>  	return 0;
>  }

> +static int pwrdm_get_idle_state(struct powerdomain *pwrdm)

could use a function comment

> +{
> +	if (pwrdm_can_idle(pwrdm))
> +		return pwrdm_read_next_pwrst(pwrdm);
> +	return PWRDM_POWER_ON;
> +}
> +

Possible candidate for powerdomain API?

>  /**
>   * omap3_enter_idle - Programs OMAP3 to enter the specified state
>   * @dev: cpuidle device
> @@ -153,14 +161,90 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  			       struct cpuidle_state *state)
>  {
>  	struct cpuidle_state *new_state = state;
> -
> -	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && omap3_idle_bm_check()) {
> -		BUG_ON(!dev->safe_state);
> -		new_state = dev->safe_state;
> +	u32 per_state = 0, saved_per_state = 0, cam_state, usb_state;
> +	u32 iva2_state, sgx_state, dss_state, new_core_state;
> +	struct omap3_processor_cx *cx;
> +	int ret;
> +
> +	if (state->flags & CPUIDLE_FLAG_CHECK_BM) {
> +		if (omap3_idle_bm_check()) {
> +			BUG_ON(!dev->safe_state);
> +			new_state = dev->safe_state;
> +			goto select_state;
> +		}
> +		cx = cpuidle_get_statedata(state);
> +		new_core_state = cx->core_state;
> +
> +		/* Check if CORE is active, if yes, fallback to inactive */
> +		if (!pwrdm_can_idle(core_pd))
> +			new_core_state = PWRDM_POWER_INACTIVE;
> +
> +		/*
> +		 * Prevent idle completely if CAM is active.
> +		 * CAM does not have wakeup capability in OMAP3.
> +		 */
> +		cam_state = pwrdm_get_idle_state(cam_pd);
> +		if (cam_state == PWRDM_POWER_ON) {
> +			new_state = dev->safe_state;
> +			goto select_state;
> +		}
> +
> +		/*
> +		 * Check if PER can idle or not. If we are not likely
> +		 * to idle, deny PER off. This prevents unnecessary
> +		 * context save/restore.
> +		 */
> +		saved_per_state = pwrdm_read_next_pwrst(per_pd);
> +		if (pwrdm_can_idle(per_pd)) {
> +			per_state = saved_per_state;
> +			/*
> +			 * Prevent PER off if CORE is active as this
> +			 * would disable PER wakeups completely
> +			 */
> +			if (per_state == PWRDM_POWER_OFF &&
> +			    new_core_state > PWRDM_POWER_RET)
> +				per_state = PWRDM_POWER_RET;
> +
> +		} else if (saved_per_state == PWRDM_POWER_OFF)
> +			per_state = PWRDM_POWER_RET;
> +
> +		/*
> +		 * If we are attempting CORE off, check if any other
> +		 * powerdomains are at retention or higher. CORE off causes
> +		 * chipwide reset which would reset these domains also.
> +		 */
> +		if (new_core_state == PWRDM_POWER_OFF) {
> +			dss_state = pwrdm_get_idle_state(dss_pd);
> +			iva2_state = pwrdm_get_idle_state(iva2_pd);
> +			sgx_state = pwrdm_get_idle_state(sgx_pd);
> +			usb_state = pwrdm_get_idle_state(usb_pd);
> +
> +			if (cam_state > PWRDM_POWER_OFF ||
> +			    dss_state > PWRDM_POWER_OFF ||
> +			    iva2_state > PWRDM_POWER_OFF ||
> +			    per_state > PWRDM_POWER_OFF ||
> +			    sgx_state > PWRDM_POWER_OFF ||
> +			    usb_state > PWRDM_POWER_OFF)
> +				new_core_state = PWRDM_POWER_RET;
> +		}

add a blank line here

> +		/* Fallback to new target core state */
> +		while (cx->core_state > new_core_state) {
> +			state--;
> +			cx = cpuidle_get_statedata(state);
> +		}
> +		new_state = state;

here

> +		/* Are we changing PER target state? */
> +		if (per_state != saved_per_state)
> +			pwrdm_set_next_pwrst(per_pd, per_state);
>  	}
>  
> +select_state:
>  	dev->last_state = new_state;
> -	return omap3_enter_idle(dev, new_state);
> +	ret = omap3_enter_idle(dev, new_state);

here

> +	/* Restore potentially tampered PER state */
> +	if (per_state != saved_per_state)
> +		pwrdm_set_next_pwrst(per_pd, saved_per_state);

and here

+	return ret;
>  }
>  
>  DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
> @@ -220,7 +304,8 @@ void omap_init_power_states(void)
>  			cpuidle_params_table[OMAP3_STATE_C2].threshold;
>  	omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_INACTIVE;
>  	omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_INACTIVE;
> -	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID;
> +	omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID |
> +				CPUIDLE_FLAG_CHECK_BM;
>  
>  	/* C3 . MPU CSWR + Core inactive */
>  	omap3_power_states[OMAP3_STATE_C3].valid = 1;
> @@ -313,6 +398,12 @@ int __init omap3_idle_init(void)
>  
>  	mpu_pd = pwrdm_lookup("mpu_pwrdm");
>  	core_pd = pwrdm_lookup("core_pwrdm");
> +	per_pd = pwrdm_lookup("per_pwrdm");
> +	iva2_pd = pwrdm_lookup("iva2_pwrdm");
> +	sgx_pd = pwrdm_lookup("sgx_pwrdm");
> +	usb_pd = pwrdm_lookup("usbhost_pwrdm");
> +	cam_pd = pwrdm_lookup("cam_pwrdm");
> +	dss_pd = pwrdm_lookup("dss_pwrdm");
>  
>  	omap_init_power_states();
>  	cpuidle_register_driver(&omap3_idle_driver);
> -- 
> 1.5.4.3

RE: [PATCH 6/6] OMAP3: CPUidle: Added peripheral pwrdm checks into bm check

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 16 November, 2009 21:59
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 6/6] OMAP3: CPUidle: Added peripheral 
>pwrdm checks into bm check
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Following checks are made (and their reasoning):
>>
>> - If CAM domain is active, prevent idle completely
>>   * CAM pwrdm does not have HW wakeup capability
>> - If PER is likely to remain on, prevent PER off
>>   * Saves on unnecessary context save/restore
>> - If CORE domain is active, prevent PER off-mode
>>   * PER off in this case would prevent wakeups from PER completely
>> - Only allow CORE off, if all peripheral domains are off
>>   * CORE off will cause a chipwide reset
>>
>> Also, enabled CHECK_BM flag for C2, as this is needed for 
>the camera case.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>Some questions and a couple minor style comments below...

Will do the style changes, answers below.

>
>> ---
>>  arch/arm/mach-omap2/cpuidle34xx.c |  105 
>++++++++++++++++++++++++++++++++++---
>>  1 files changed, 98 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c 
>b/arch/arm/mach-omap2/cpuidle34xx.c
>> index e46345f..4654e87 100644
>> --- a/arch/arm/mach-omap2/cpuidle34xx.c
>> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
>> @@ -58,7 +58,8 @@ struct omap3_processor_cx {
>>  
>>  struct omap3_processor_cx omap3_power_states[OMAP3_MAX_STATES];
>>  struct omap3_processor_cx current_cx_state;
>> -struct powerdomain *mpu_pd, *core_pd;
>> +struct powerdomain *mpu_pd, *core_pd, *per_pd, *iva2_pd;
>> +struct powerdomain *sgx_pd, *usb_pd, *cam_pd, *dss_pd;
>>  
>>  /*
>>   * The latencies/thresholds for various C states have
>> @@ -91,6 +92,13 @@ static int omap3_idle_bm_check(void)
>>  	return 0;
>>  }
>
>> +static int pwrdm_get_idle_state(struct powerdomain *pwrdm)
>
>could use a function comment

Ok.

>
>> +{
>> +	if (pwrdm_can_idle(pwrdm))
>> +		return pwrdm_read_next_pwrst(pwrdm);
>> +	return PWRDM_POWER_ON;
>> +}
>> +
>
>Possible candidate for powerdomain API?

Candidate yes, if we would need this somewhere else. I did not want to make an API change that is not needed anywhere else at the moment. Maybe Paul has some comments on this?

>
>>  /**
>>   * omap3_enter_idle - Programs OMAP3 to enter the specified state
>>   * @dev: cpuidle device
>> @@ -153,14 +161,90 @@ static int omap3_enter_idle_bm(struct 
>cpuidle_device *dev,
>>  			       struct cpuidle_state *state)
>>  {
>>  	struct cpuidle_state *new_state = state;
>> -
>> -	if ((state->flags & CPUIDLE_FLAG_CHECK_BM) && 
>omap3_idle_bm_check()) {
>> -		BUG_ON(!dev->safe_state);
>> -		new_state = dev->safe_state;
>> +	u32 per_state = 0, saved_per_state = 0, cam_state, usb_state;
>> +	u32 iva2_state, sgx_state, dss_state, new_core_state;
>> +	struct omap3_processor_cx *cx;
>> +	int ret;
>> +
>> +	if (state->flags & CPUIDLE_FLAG_CHECK_BM) {
>> +		if (omap3_idle_bm_check()) {
>> +			BUG_ON(!dev->safe_state);
>> +			new_state = dev->safe_state;
>> +			goto select_state;
>> +		}
>> +		cx = cpuidle_get_statedata(state);
>> +		new_core_state = cx->core_state;
>> +
>> +		/* Check if CORE is active, if yes, fallback to 
>inactive */
>> +		if (!pwrdm_can_idle(core_pd))
>> +			new_core_state = PWRDM_POWER_INACTIVE;
>> +
>> +		/*
>> +		 * Prevent idle completely if CAM is active.
>> +		 * CAM does not have wakeup capability in OMAP3.
>> +		 */
>> +		cam_state = pwrdm_get_idle_state(cam_pd);
>> +		if (cam_state == PWRDM_POWER_ON) {
>> +			new_state = dev->safe_state;
>> +			goto select_state;
>> +		}
>> +
>> +		/*
>> +		 * Check if PER can idle or not. If we are not likely
>> +		 * to idle, deny PER off. This prevents unnecessary
>> +		 * context save/restore.
>> +		 */
>> +		saved_per_state = pwrdm_read_next_pwrst(per_pd);
>> +		if (pwrdm_can_idle(per_pd)) {
>> +			per_state = saved_per_state;
>> +			/*
>> +			 * Prevent PER off if CORE is active as this
>> +			 * would disable PER wakeups completely
>> +			 */
>> +			if (per_state == PWRDM_POWER_OFF &&
>> +			    new_core_state > PWRDM_POWER_RET)
>> +				per_state = PWRDM_POWER_RET;
>> +
>> +		} else if (saved_per_state == PWRDM_POWER_OFF)
>> +			per_state = PWRDM_POWER_RET;
>> +
>> +		/*
>> +		 * If we are attempting CORE off, check if any other
>> +		 * powerdomains are at retention or higher. 
>CORE off causes
>> +		 * chipwide reset which would reset these domains also.
>> +		 */
>> +		if (new_core_state == PWRDM_POWER_OFF) {
>> +			dss_state = pwrdm_get_idle_state(dss_pd);
>> +			iva2_state = pwrdm_get_idle_state(iva2_pd);
>> +			sgx_state = pwrdm_get_idle_state(sgx_pd);
>> +			usb_state = pwrdm_get_idle_state(usb_pd);
>> +
>> +			if (cam_state > PWRDM_POWER_OFF ||
>> +			    dss_state > PWRDM_POWER_OFF ||
>> +			    iva2_state > PWRDM_POWER_OFF ||
>> +			    per_state > PWRDM_POWER_OFF ||
>> +			    sgx_state > PWRDM_POWER_OFF ||
>> +			    usb_state > PWRDM_POWER_OFF)
>> +				new_core_state = PWRDM_POWER_RET;
>> +		}
>
>add a blank line here
>
>> +		/* Fallback to new target core state */
>> +		while (cx->core_state > new_core_state) {
>> +			state--;
>> +			cx = cpuidle_get_statedata(state);
>> +		}
>> +		new_state = state;
>
>here
>
>> +		/* Are we changing PER target state? */
>> +		if (per_state != saved_per_state)
>> +			pwrdm_set_next_pwrst(per_pd, per_state);
>>  	}
>>  
>> +select_state:
>>  	dev->last_state = new_state;
>> -	return omap3_enter_idle(dev, new_state);
>> +	ret = omap3_enter_idle(dev, new_state);
>
>here
>
>> +	/* Restore potentially tampered PER state */
>> +	if (per_state != saved_per_state)
>> +		pwrdm_set_next_pwrst(per_pd, saved_per_state);
>
>and here
>
>+	return ret;
>>  }
>>  
>>  DEFINE_PER_CPU(struct cpuidle_device, omap3_idle_dev);
>> @@ -220,7 +304,8 @@ void omap_init_power_states(void)
>>  			cpuidle_params_table[OMAP3_STATE_C2].threshold;
>>  	omap3_power_states[OMAP3_STATE_C2].mpu_state = 
>PWRDM_POWER_INACTIVE;
>>  	omap3_power_states[OMAP3_STATE_C2].core_state = 
>PWRDM_POWER_INACTIVE;
>> -	omap3_power_states[OMAP3_STATE_C2].flags = 
>CPUIDLE_FLAG_TIME_VALID;
>> +	omap3_power_states[OMAP3_STATE_C2].flags = 
>CPUIDLE_FLAG_TIME_VALID |
>> +				CPUIDLE_FLAG_CHECK_BM;
>>  
>>  	/* C3 . MPU CSWR + Core inactive */
>>  	omap3_power_states[OMAP3_STATE_C3].valid = 1;
>> @@ -313,6 +398,12 @@ int __init omap3_idle_init(void)
>>  
>>  	mpu_pd = pwrdm_lookup("mpu_pwrdm");
>>  	core_pd = pwrdm_lookup("core_pwrdm");
>> +	per_pd = pwrdm_lookup("per_pwrdm");
>> +	iva2_pd = pwrdm_lookup("iva2_pwrdm");
>> +	sgx_pd = pwrdm_lookup("sgx_pwrdm");
>> +	usb_pd = pwrdm_lookup("usbhost_pwrdm");
>> +	cam_pd = pwrdm_lookup("cam_pwrdm");
>> +	dss_pd = pwrdm_lookup("dss_pwrdm");
>>  
>>  	omap_init_power_states();
>>  	cpuidle_register_driver(&omap3_idle_driver);
>> -- 
>> 1.5.4.3
>

Re: [PATCH 5/6] OMAP: Powerdomains: Add support for checking if pwrdm can idle

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> pwrdm_can_idle(pwrdm) will check if the specified powerdomain can enter
> idle. This is done by checking the current fclk enable bits.
>
> This call can be used e.g. inside cpuidle to decide which power states
> core and mpu should enter during idle, as there are certain dependencies
> between wakeup capabilities and reset logic.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

In your initial implementatio, you were checking all (most) of the
fclks in a given powerdomain.  In this version, you're currently only
checking masks in CORE (UART1 and 2) and PER (UART3.)

I'll assume it's just to propose the idea and we can add more fclks
later.  

That being said, I'm a little reluctant to add another list of FCLK
masks. This seems like something that clock/clockdomain code should be
handling.

In terms of cleanness, it seems that pwrdm_can_idle() should call some
sort of clkdm_can_idle() call for all the clockdomains associated with
it (pwrdm->pwrdm_clkdms[].)

The clockdomain code already tracks its usecount, so clkdm_can_idle()
might be as simple as checking clkdm->usecount.  Just a thought
without digging into the clkdm code.

Kevin

> ---
>  arch/arm/mach-omap2/powerdomain.c             |   22 ++++++++++++++++++++++
>  arch/arm/mach-omap2/powerdomains34xx.h        |   14 ++++++++++++++
>  arch/arm/plat-omap/include/plat/powerdomain.h |    9 +++++++++
>  3 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/powerdomain.c b/arch/arm/mach-omap2/powerdomain.c
> index 1237717..bf2b97a 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -1217,6 +1217,28 @@ int pwrdm_wait_transition(struct powerdomain *pwrdm)
>  	return 0;
>  }
>  
> +/**
> + * pwrdm_can_idle - check if the powerdomain can enter idle
> + * @pwrdm: struct powerdomain * the powerdomain to check status of
> + *
> + * Does a functional clock check for the powerdomain and returns 1 if the
> + * powerdomain can enter idle, 0 if not.
> + */
> +int pwrdm_can_idle(struct powerdomain *pwrdm)
> +{
> +	int i;
> +	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
> +
> +	if (!pwrdm)
> +		return -EINVAL;
> +
> +	for (i = 0; i < pwrdm->fclk_reg_amt; i++)
> +		if (cm_read_mod_reg(pwrdm->prcm_offs, fclk_regs[i]) &
> +				(0xffffffff ^ pwrdm->fclk_masks[i]))
> +			return 0;
> +	return 1;
> +}
> +
>  int pwrdm_state_switch(struct powerdomain *pwrdm)
>  {
>  	return _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
> diff --git a/arch/arm/mach-omap2/powerdomains34xx.h b/arch/arm/mach-omap2/powerdomains34xx.h
> index 9eb2dc5..c8cd297 100644
> --- a/arch/arm/mach-omap2/powerdomains34xx.h
> +++ b/arch/arm/mach-omap2/powerdomains34xx.h
> @@ -180,6 +180,7 @@ static struct powerdomain iva2_pwrdm = {
>  		[2] = PWRSTS_OFF_ON,
>  		[3] = PWRDM_POWER_ON,
>  	},
> +	.fclk_reg_amt	  = 1,
>  };
>  
>  static struct powerdomain mpu_34xx_pwrdm = {
> @@ -236,6 +237,11 @@ static struct powerdomain core_34xx_es3_1_pwrdm = {
>  		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
>  		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>  	},
> +	.fclk_reg_amt	  = 2,
> +	.fclk_masks	  = {
> +		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
> +		[1] = 0,
> +	},
>  };
>  
>  /* Another case of bit name collisions between several registers: EN_DSS */
> @@ -255,6 +261,7 @@ static struct powerdomain dss_pwrdm = {
>  	.pwrsts_mem_on	  = {
>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>  	},
> +	.fclk_reg_amt	  = 1,
>  };
>  
>  /*
> @@ -278,6 +285,7 @@ static struct powerdomain sgx_pwrdm = {
>  	.pwrsts_mem_on	  = {
>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>  	},
> +	.fclk_reg_amt	  = 1,
>  };
>  
>  static struct powerdomain cam_pwrdm = {
> @@ -295,6 +303,7 @@ static struct powerdomain cam_pwrdm = {
>  	.pwrsts_mem_on	  = {
>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>  	},
> +	.fclk_reg_amt	  = 1,
>  };
>  
>  static struct powerdomain per_pwrdm = {
> @@ -313,6 +322,10 @@ static struct powerdomain per_pwrdm = {
>  	.pwrsts_mem_on	  = {
>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>  	},
> +	.fclk_reg_amt	  = 1,
> +	.fclk_masks	  = {
> +		[0] = OMAP3430_EN_UART3,
> +	},
>  };
>  
>  static struct powerdomain emu_pwrdm = {
> @@ -352,6 +365,7 @@ static struct powerdomain usbhost_pwrdm = {
>  	.pwrsts_mem_on	  = {
>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>  	},
> +	.fclk_reg_amt	  = 1,
>  };
>  
>  static struct powerdomain dpll1_pwrdm = {
> diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h b/arch/arm/plat-omap/include/plat/powerdomain.h
> index 55350d0..b004d88 100644
> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
> @@ -57,6 +57,12 @@
>   */
>  #define PWRDM_MAX_CLKDMS	4
>  
> +/*
> + * Maximum number of FCLK register masks that can be associated with a
> + * powerdomain. CORE powerdomain on OMAP3 is the worst case
> + */
> +#define PWRDM_MAX_FCLK		2
> +
>  /* XXX A completely arbitrary number. What is reasonable here? */
>  #define PWRDM_TRANSITION_BAILOUT 100000
>  
> @@ -124,6 +130,8 @@ struct powerdomain {
>  	s8 next_state;
>  	unsigned state_counter[4];
>  
> +	u8 fclk_reg_amt;
> +	u32 fclk_masks[PWRDM_MAX_FCLK];
>  #ifdef CONFIG_PM_DEBUG
>  	s64 timer;
>  	s64 state_timer[4];
> @@ -177,6 +185,7 @@ int pwrdm_disable_hdwr_sar(struct powerdomain *pwrdm);
>  bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
>  
>  int pwrdm_wait_transition(struct powerdomain *pwrdm);
> +int pwrdm_can_idle(struct powerdomain *pwrdm);
>  
>  int pwrdm_state_switch(struct powerdomain *pwrdm);
>  int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
> -- 
> 1.5.4.3

RE: [PATCH 5/6] OMAP: Powerdomains: Add support for checking if pwrdm can idle

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 16 November, 2009 22:13
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 5/6] OMAP: Powerdomains: Add support for 
>checking if pwrdm can idle
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> pwrdm_can_idle(pwrdm) will check if the specified 
>powerdomain can enter
>> idle. This is done by checking the current fclk enable bits.
>>
>> This call can be used e.g. inside cpuidle to decide which 
>power states
>> core and mpu should enter during idle, as there are certain 
>dependencies
>> between wakeup capabilities and reset logic.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>In your initial implementatio, you were checking all (most) of the
>fclks in a given powerdomain.  In this version, you're currently only
>checking masks in CORE (UART1 and 2) and PER (UART3.)

The masks are negative masks, we are checking whether all the rest of the clocks are inactive or not. UART clocks are currently masked away because we are controlling those inside omap_sram_idle, thus they are always on when we do the check, and the system assumes we can enter idle after they are disabled.

>
>I'll assume it's just to propose the idea and we can add more fclks
>later.  

There might be some sort of need to add or change the masks on some boards. Some of the UARTs might be controlled by some other drivers, in which case their FCLK should actually be checked against, and not masked, because they would no longer be controlled inside omap_sram_idle().

>
>That being said, I'm a little reluctant to add another list of FCLK
>masks. This seems like something that clock/clockdomain code should be
>handling.
>
>In terms of cleanness, it seems that pwrdm_can_idle() should call some
>sort of clkdm_can_idle() call for all the clockdomains associated with
>it (pwrdm->pwrdm_clkdms[].)
>
>The clockdomain code already tracks its usecount, so clkdm_can_idle()
>might be as simple as checking clkdm->usecount.  Just a thought
>without digging into the clkdm code.

I did think and experiment with this usecount option a bit. The problem with usecounts is that you will have some interesting usecount numbers due to always_on clocks, uart clocks, and interface clocks. For example per_clkdm usecount is currently 9 on my rx51 board when we are entering omap_sram_idle and there is nothing but UART3 active. This number is most likely something else if we have a different board which has e.g. different number of interface clocks active. Interface clocks do not matter in the idle equation because we have autoidle enabled for all of those when we are entering idle and they will be turned off by hw. It might be possible to somehow keep a separate usecount for FCLKs and all clocks inside clock framework, but this sounds awfully complex, as you can simply read thi
 s from HW.

However, the part where this idle check would be moved inside clockdomain code is simple, I could just move the FCLK check bit there, and move the fclk_reg and mask definitions from pwrdm to clkdm.

How does the option of moving the FCLK checks inside clkdm code sound like, but keeping the implementation similar otherwise?


>
>Kevin
>
>> ---
>>  arch/arm/mach-omap2/powerdomain.c             |   22 
>++++++++++++++++++++++
>>  arch/arm/mach-omap2/powerdomains34xx.h        |   14 ++++++++++++++
>>  arch/arm/plat-omap/include/plat/powerdomain.h |    9 +++++++++
>>  3 files changed, 45 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/powerdomain.c 
>b/arch/arm/mach-omap2/powerdomain.c
>> index 1237717..bf2b97a 100644
>> --- a/arch/arm/mach-omap2/powerdomain.c
>> +++ b/arch/arm/mach-omap2/powerdomain.c
>> @@ -1217,6 +1217,28 @@ int pwrdm_wait_transition(struct 
>powerdomain *pwrdm)
>>  	return 0;
>>  }
>>  
>> +/**
>> + * pwrdm_can_idle - check if the powerdomain can enter idle
>> + * @pwrdm: struct powerdomain * the powerdomain to check status of
>> + *
>> + * Does a functional clock check for the powerdomain and 
>returns 1 if the
>> + * powerdomain can enter idle, 0 if not.
>> + */
>> +int pwrdm_can_idle(struct powerdomain *pwrdm)
>> +{
>> +	int i;
>> +	const int fclk_regs[] = { CM_FCLKEN, OMAP3430ES2_CM_FCLKEN3 };
>> +
>> +	if (!pwrdm)
>> +		return -EINVAL;
>> +
>> +	for (i = 0; i < pwrdm->fclk_reg_amt; i++)
>> +		if (cm_read_mod_reg(pwrdm->prcm_offs, fclk_regs[i]) &
>> +				(0xffffffff ^ pwrdm->fclk_masks[i]))
>> +			return 0;
>> +	return 1;
>> +}
>> +
>>  int pwrdm_state_switch(struct powerdomain *pwrdm)
>>  {
>>  	return _pwrdm_state_switch(pwrdm, PWRDM_STATE_NOW);
>> diff --git a/arch/arm/mach-omap2/powerdomains34xx.h 
>b/arch/arm/mach-omap2/powerdomains34xx.h
>> index 9eb2dc5..c8cd297 100644
>> --- a/arch/arm/mach-omap2/powerdomains34xx.h
>> +++ b/arch/arm/mach-omap2/powerdomains34xx.h
>> @@ -180,6 +180,7 @@ static struct powerdomain iva2_pwrdm = {
>>  		[2] = PWRSTS_OFF_ON,
>>  		[3] = PWRDM_POWER_ON,
>>  	},
>> +	.fclk_reg_amt	  = 1,
>>  };
>>  
>>  static struct powerdomain mpu_34xx_pwrdm = {
>> @@ -236,6 +237,11 @@ static struct powerdomain 
>core_34xx_es3_1_pwrdm = {
>>  		[0] = PWRSTS_OFF_RET_INA_ON, /* MEM1ONSTATE */
>>  		[1] = PWRSTS_OFF_RET_INA_ON, /* MEM2ONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 2,
>> +	.fclk_masks	  = {
>> +		[0] = OMAP3430_EN_UART2 | OMAP3430_EN_UART1,
>> +		[1] = 0,
>> +	},
>>  };
>>  
>>  /* Another case of bit name collisions between several 
>registers: EN_DSS */
>> @@ -255,6 +261,7 @@ static struct powerdomain dss_pwrdm = {
>>  	.pwrsts_mem_on	  = {
>>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 1,
>>  };
>>  
>>  /*
>> @@ -278,6 +285,7 @@ static struct powerdomain sgx_pwrdm = {
>>  	.pwrsts_mem_on	  = {
>>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 1,
>>  };
>>  
>>  static struct powerdomain cam_pwrdm = {
>> @@ -295,6 +303,7 @@ static struct powerdomain cam_pwrdm = {
>>  	.pwrsts_mem_on	  = {
>>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 1,
>>  };
>>  
>>  static struct powerdomain per_pwrdm = {
>> @@ -313,6 +322,10 @@ static struct powerdomain per_pwrdm = {
>>  	.pwrsts_mem_on	  = {
>>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 1,
>> +	.fclk_masks	  = {
>> +		[0] = OMAP3430_EN_UART3,
>> +	},
>>  };
>>  
>>  static struct powerdomain emu_pwrdm = {
>> @@ -352,6 +365,7 @@ static struct powerdomain usbhost_pwrdm = {
>>  	.pwrsts_mem_on	  = {
>>  		[0] = PWRDM_POWER_ON,  /* MEMONSTATE */
>>  	},
>> +	.fclk_reg_amt	  = 1,
>>  };
>>  
>>  static struct powerdomain dpll1_pwrdm = {
>> diff --git a/arch/arm/plat-omap/include/plat/powerdomain.h 
>b/arch/arm/plat-omap/include/plat/powerdomain.h
>> index 55350d0..b004d88 100644
>> --- a/arch/arm/plat-omap/include/plat/powerdomain.h
>> +++ b/arch/arm/plat-omap/include/plat/powerdomain.h
>> @@ -57,6 +57,12 @@
>>   */
>>  #define PWRDM_MAX_CLKDMS	4
>>  
>> +/*
>> + * Maximum number of FCLK register masks that can be 
>associated with a
>> + * powerdomain. CORE powerdomain on OMAP3 is the worst case
>> + */
>> +#define PWRDM_MAX_FCLK		2
>> +
>>  /* XXX A completely arbitrary number. What is reasonable here? */
>>  #define PWRDM_TRANSITION_BAILOUT 100000
>>  
>> @@ -124,6 +130,8 @@ struct powerdomain {
>>  	s8 next_state;
>>  	unsigned state_counter[4];
>>  
>> +	u8 fclk_reg_amt;
>> +	u32 fclk_masks[PWRDM_MAX_FCLK];
>>  #ifdef CONFIG_PM_DEBUG
>>  	s64 timer;
>>  	s64 state_timer[4];
>> @@ -177,6 +185,7 @@ int pwrdm_disable_hdwr_sar(struct 
>powerdomain *pwrdm);
>>  bool pwrdm_has_hdwr_sar(struct powerdomain *pwrdm);
>>  
>>  int pwrdm_wait_transition(struct powerdomain *pwrdm);
>> +int pwrdm_can_idle(struct powerdomain *pwrdm);
>>  
>>  int pwrdm_state_switch(struct powerdomain *pwrdm);
>>  int pwrdm_clkdm_state_switch(struct clockdomain *clkdm);
>> -- 
>> 1.5.4.3
>

Re: [PATCH 4/6] OMAP3: PM: Removed PER + CORE state hacking from omap_sram_idle

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>

A more descriptive changelog would be nice here.

> This should be moved inside cpuidle to simplify design.

And this should probably say "... and will be moved into CPUidle in
subsequent patch."

Kevin

> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   18 ++----------------
>  1 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 7b52f30..c704756 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -81,7 +81,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
>  
>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>  static struct powerdomain *core_pwrdm, *per_pwrdm;
> -static struct powerdomain *cam_pwrdm;
>  
>  static struct prm_setup_times_vc prm_setup_times_default = {
>  	.clksetup = 0xff,
> @@ -370,7 +369,6 @@ void omap_sram_idle(void)
>  	int core_next_state = PWRDM_POWER_ON;
>  	int core_prev_state, per_prev_state;
>  	u32 sdrc_pwr = 0;
> -	int per_state_modified = 0;
>  
>  	if (!_omap_sram_idle)
>  		return;
> @@ -409,19 +407,10 @@ void omap_sram_idle(void)
>  	if (per_next_state < PWRDM_POWER_ON) {
>  		omap_uart_prepare_idle(2);
>  		omap2_gpio_prepare_for_idle(per_next_state);
> -		if (per_next_state == PWRDM_POWER_OFF) {
> -			if (core_next_state == PWRDM_POWER_ON) {
> -				per_next_state = PWRDM_POWER_RET;
> -				pwrdm_set_next_pwrst(per_pwrdm, per_next_state);
> -				per_state_modified = 1;
> -			} else
> -				omap3_per_save_context();
> -		}
> +		if (per_next_state == PWRDM_POWER_OFF)
> +			omap3_per_save_context();
>  	}
>  
> -	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
> -		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> -
>  	/*
>  	 * Disable smartreflex before entering WFI.
>  	 * Only needed if we are going to enter retention or off.
> @@ -540,8 +529,6 @@ void omap_sram_idle(void)
>  			omap3_gpio_restore_pad_context(1);
>  		omap2_gpio_resume_after_idle();
>  		omap_uart_resume_idle(2);
> -		if (per_state_modified)
> -			pwrdm_set_next_pwrst(per_pwrdm, PWRDM_POWER_OFF);
>  	}
>  
>  	/* Disable IO-PAD and IO-CHAIN wakeup */
> @@ -1170,7 +1157,6 @@ static int __init omap3_pm_init(void)
>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>  
>  	omap_push_sram_idle();
>  #ifdef CONFIG_SUSPEND
> -- 
> 1.5.4.3

RE: [PATCH 4/6] OMAP3: PM: Removed PER + CORE state hacking from omap_sram_idle

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 16 November, 2009 21:45
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 4/6] OMAP3: PM: Removed PER + CORE state 
>hacking from omap_sram_idle
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>
>A more descriptive changelog would be nice here.
>
>> This should be moved inside cpuidle to simplify design.
>
>And this should probably say "... and will be moved into CPUidle in
>subsequent patch."

I'll update this.

>
>Kevin
>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   18 ++----------------
>>  1 files changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index 7b52f30..c704756 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -81,7 +81,6 @@ static int (*_omap_save_secure_sram)(u32 *addr);
>>  
>>  static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>>  static struct powerdomain *core_pwrdm, *per_pwrdm;
>> -static struct powerdomain *cam_pwrdm;
>>  
>>  static struct prm_setup_times_vc prm_setup_times_default = {
>>  	.clksetup = 0xff,
>> @@ -370,7 +369,6 @@ void omap_sram_idle(void)
>>  	int core_next_state = PWRDM_POWER_ON;
>>  	int core_prev_state, per_prev_state;
>>  	u32 sdrc_pwr = 0;
>> -	int per_state_modified = 0;
>>  
>>  	if (!_omap_sram_idle)
>>  		return;
>> @@ -409,19 +407,10 @@ void omap_sram_idle(void)
>>  	if (per_next_state < PWRDM_POWER_ON) {
>>  		omap_uart_prepare_idle(2);
>>  		omap2_gpio_prepare_for_idle(per_next_state);
>> -		if (per_next_state == PWRDM_POWER_OFF) {
>> -			if (core_next_state == PWRDM_POWER_ON) {
>> -				per_next_state = PWRDM_POWER_RET;
>> -				pwrdm_set_next_pwrst(per_pwrdm, 
>per_next_state);
>> -				per_state_modified = 1;
>> -			} else
>> -				omap3_per_save_context();
>> -		}
>> +		if (per_next_state == PWRDM_POWER_OFF)
>> +			omap3_per_save_context();
>>  	}
>>  
>> -	if (pwrdm_read_pwrst(cam_pwrdm) == PWRDM_POWER_ON)
>> -		omap2_clkdm_deny_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>> -
>>  	/*
>>  	 * Disable smartreflex before entering WFI.
>>  	 * Only needed if we are going to enter retention or off.
>> @@ -540,8 +529,6 @@ void omap_sram_idle(void)
>>  			omap3_gpio_restore_pad_context(1);
>>  		omap2_gpio_resume_after_idle();
>>  		omap_uart_resume_idle(2);
>> -		if (per_state_modified)
>> -			pwrdm_set_next_pwrst(per_pwrdm, 
>PWRDM_POWER_OFF);
>>  	}
>>  
>>  	/* Disable IO-PAD and IO-CHAIN wakeup */
>> @@ -1170,7 +1157,6 @@ static int __init omap3_pm_init(void)
>>  	neon_pwrdm = pwrdm_lookup("neon_pwrdm");
>>  	per_pwrdm = pwrdm_lookup("per_pwrdm");
>>  	core_pwrdm = pwrdm_lookup("core_pwrdm");
>> -	cam_pwrdm = pwrdm_lookup("cam_pwrdm");
>>  
>>  	omap_push_sram_idle();
>>  #ifdef CONFIG_SUSPEND
>> -- 
>> 1.5.4.3
>

Re: [PATCH 2/6] OMAP3: PM: Added support for INACTIVE and ON states for powerdomains

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Previously omap_sram_idle() did not know about the difference between ON and
> INACTIVE states, which complicated the state handling in these cases.

This changelog needs an update.

This patch changes makes the IO-chain arming conditional and removes
the clockdomain idle calls.

While I understand the clkdm idle call removal, It's not clear from
the changelog how these are related to the new handling of INACTIVE
states.

> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/pm34xx.c |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index 9d0a9b4..7b52f30 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -383,6 +383,7 @@ void omap_sram_idle(void)
>  	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
>  	switch (mpu_next_state) {
>  	case PWRDM_POWER_ON:
> +	case PWRDM_POWER_INACTIVE:
>  	case PWRDM_POWER_RET:
>  		/* No need to save context */
>  		save_state = 0;
> @@ -448,9 +449,11 @@ void omap_sram_idle(void)
>  						OMAP3430_GR_MOD,
>  						OMAP3_PRM_VOLTCTRL_OFFSET);
>  		}
> -		/* Enable IO-PAD and IO-CHAIN wakeups */
> -		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
> -		omap3_enable_io_chain();
> +		if (core_next_state <= PWRDM_POWER_RET) {
> +			/* Enable IO-PAD and IO-CHAIN wakeups */
> +			prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
> +			omap3_enable_io_chain();
> +		}
>  	}
>  	omap3_intc_prepare_idle();
>  
> @@ -542,15 +545,13 @@ void omap_sram_idle(void)
>  	}
>  
>  	/* Disable IO-PAD and IO-CHAIN wakeup */
> -	if (core_next_state < PWRDM_POWER_ON) {
> +	if (core_next_state <= PWRDM_POWER_ON) {

Hmm, did you mean <= RET here?   

Otherwise, you can just remove the conditional.

>  		prm_clear_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>  		omap3_disable_io_chain();
>  	}
>  
>  
>  	pwrdm_post_transition();
> -
> -	omap2_clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>  }
>  
>  int omap3_can_sleep(void)
> @@ -598,7 +599,6 @@ int set_pwrdm_state(struct powerdomain *pwrdm, u32 state)
>  	}
>  
>  	if (sleep_switch) {
> -		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>  		pwrdm_wait_transition(pwrdm);
>  		pwrdm_state_switch(pwrdm);
>  	}
> -- 
> 1.5.4.3

Kevin

RE: [PATCH 2/6] OMAP3: PM: Added support for INACTIVE and ON states for powerdomains

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 16 November, 2009 21:42
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH 2/6] OMAP3: PM: Added support for INACTIVE 
>and ON states for powerdomains
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Previously omap_sram_idle() did not know about the 
>difference between ON and
>> INACTIVE states, which complicated the state handling in these cases.
>
>This changelog needs an update.
>
>This patch changes makes the IO-chain arming conditional and removes
>the clockdomain idle calls.

IO-chain arming is already conditional, the condition needs to be changed a bit due to changed powerdomain states.

- previously: less than PWRDM_POWER_ON = RET and OFF
- now: less than or equal to PWRDM_POWER_RET = RET and OFF

What is changed actually is that if CORE or PER is going INACTIVE, we will disable UART clocks which will allow CORE and UART to go inactive. Previously this did not happen.

>While I understand the clkdm idle call removal, It's not clear from
>the changelog how these are related to the new handling of INACTIVE
>states.

Yes, I can try to fix the changelog a bit regarding this.

>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/pm34xx.c |   14 +++++++-------
>>  1 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c 
>b/arch/arm/mach-omap2/pm34xx.c
>> index 9d0a9b4..7b52f30 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -383,6 +383,7 @@ void omap_sram_idle(void)
>>  	mpu_next_state = pwrdm_read_next_pwrst(mpu_pwrdm);
>>  	switch (mpu_next_state) {
>>  	case PWRDM_POWER_ON:
>> +	case PWRDM_POWER_INACTIVE:
>>  	case PWRDM_POWER_RET:
>>  		/* No need to save context */
>>  		save_state = 0;
>> @@ -448,9 +449,11 @@ void omap_sram_idle(void)
>>  						OMAP3430_GR_MOD,
>>  						
>OMAP3_PRM_VOLTCTRL_OFFSET);
>>  		}
>> -		/* Enable IO-PAD and IO-CHAIN wakeups */
>> -		prm_set_mod_reg_bits(OMAP3430_EN_IO, WKUP_MOD, PM_WKEN);
>> -		omap3_enable_io_chain();
>> +		if (core_next_state <= PWRDM_POWER_RET) {
>> +			/* Enable IO-PAD and IO-CHAIN wakeups */
>> +			prm_set_mod_reg_bits(OMAP3430_EN_IO, 
>WKUP_MOD, PM_WKEN);
>> +			omap3_enable_io_chain();
>> +		}
>>  	}
>>  	omap3_intc_prepare_idle();
>>  
>> @@ -542,15 +545,13 @@ void omap_sram_idle(void)
>>  	}
>>  
>>  	/* Disable IO-PAD and IO-CHAIN wakeup */
>> -	if (core_next_state < PWRDM_POWER_ON) {
>> +	if (core_next_state <= PWRDM_POWER_ON) {
>
>Hmm, did you mean <= RET here?   

True, this is a bug. Should be RET.

>Otherwise, you can just remove the conditional.
>
>>  		prm_clear_mod_reg_bits(OMAP3430_EN_IO, 
>WKUP_MOD, PM_WKEN);
>>  		omap3_disable_io_chain();
>>  	}
>>  
>>  
>>  	pwrdm_post_transition();
>> -
>> -	omap2_clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
>>  }
>>  
>>  int omap3_can_sleep(void)
>> @@ -598,7 +599,6 @@ int set_pwrdm_state(struct powerdomain 
>*pwrdm, u32 state)
>>  	}
>>  
>>  	if (sleep_switch) {
>> -		omap2_clkdm_allow_idle(pwrdm->pwrdm_clkdms[0]);
>>  		pwrdm_wait_transition(pwrdm);
>>  		pwrdm_state_switch(pwrdm);
>>  	}
>> -- 
>> 1.5.4.3
>
>Kevin
>

Re: [PATCH 0/6] Idle status patches revisited

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Following set implements checks for idle states of powerdomains, and
> changes target cpuidle state according to those. This also includes as
> a cleanup removal of some hacks from omap_sram_idle(), and also introduces
> INACTIVE / ON state support for powerdomains which simplifies the code.
>
> Applies on top of PM branch.

Hi Tero,

I *really* like this cleanup and re-organization you've done.  I have
a few minor comments on some of the patches, but overall it looks
really good.

I'll wait for Paul's comments/signoff on PATCH 1/6, and after some of
my comments are addressed, I'll gladly pull an updated series into the
PM branch.

Kevin