[PATCH 0/3] OMAP3 PM: CPUIdle fixes

[PATCH 0/3] OMAP3 PM: CPUIdle fixes

[Vishwanath BS]

This patch series has some fixes/optimization for OMAP3 cpuidle code.
Tested on ZOOM3 for cpuidle and suspend/resume with OFF mode enabled.
Patches are rebased to latest kevin's pm branch 
(commit id: b6fb54bc4bfc396a9b982d76c1c954c974290a1a)

Vishwanath BS (3):
  OMAP3 PM: Deny clock gating only for safe state
  OMAP3 PM: Update only enabled C states
  OMAP36xx PM: Updated C state latencies for OMAP3630

 arch/arm/mach-omap2/board-3630sdp.c |   19 +++++++++++++
 arch/arm/mach-omap2/board-zoom.c    |   19 +++++++++++++
 arch/arm/mach-omap2/cpuidle34xx.c   |   49 +++++++++++++++++++++++-----------
 3 files changed, 71 insertions(+), 16 deletions(-)

[PATCH 1/3] OMAP3 PM: Deny clock gating only for safe state

[Vishwanath BS]

Currently clock gating for MPU and core are denied whenever C1 state is
selected. It should be denied only when safe state is selected.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index f8e35b3..1e4ec7f 100644
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -139,19 +139,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);
@@ -315,8 +305,18 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
 
 select_state:
 	dev->last_state = new_state;
+
+	if (new_state == dev->safe_state) {
+		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
+		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
+	}
 	ret = omap3_enter_idle(dev, new_state);
 
+	if (new_state == dev->safe_state) {
+		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
+		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
+	}
+
 	/* Restore original PER state if it was modified */
 	if (per_next_state != per_saved_state)
 		pwrdm_set_next_pwrst(per_pd, per_saved_state);
-- 
1.7.0.4

Re: [PATCH 1/3] OMAP3 PM: Deny clock gating only for safe state

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> Currently clock gating for MPU and core are denied whenever C1 state is
> selected. 

Yes, that is the definition of C1.

> It should be denied only when safe state is selected.

Why?

This changes the definition and behavior of C1 depending on how it is
entered.  Not a good idea IMO.

Kevin


> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   20 ++++++++++----------
>  1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index f8e35b3..1e4ec7f 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -139,19 +139,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);
> @@ -315,8 +305,18 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  
>  select_state:
>  	dev->last_state = new_state;
> +
> +	if (new_state == dev->safe_state) {
> +		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
> +		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
> +	}
>  	ret = omap3_enter_idle(dev, new_state);
>  
> +	if (new_state == dev->safe_state) {
> +		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
> +		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
> +	}
> +
>  	/* Restore original PER state if it was modified */
>  	if (per_next_state != per_saved_state)
>  		pwrdm_set_next_pwrst(per_pd, per_saved_state);

RE: [PATCH 1/3] OMAP3 PM: Deny clock gating only for safe state

[Vishwanath Sripathy]

Kevin,

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Wednesday, March 02, 2011 2:32 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org
> Subject: Re: [PATCH 1/3] OMAP3 PM: Deny clock gating only for safe
> state
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > Currently clock gating for MPU and core are denied whenever C1 state
> is
> > selected.
>
> Yes, that is the definition of C1.
>
> > It should be denied only when safe state is selected.
>
> Why?
Clock gating in C1 will reduce overall power consumption and it should not
impact any functionality as well.
>
> This changes the definition and behavior of C1 depending on how it is
> entered.  Not a good idea IMO.
I thought of adding a new C state, keeping C1 definition unchanged. Then
we will not get the real power benefit since in case of C1, clock gating
will be prevented. Let me know if you have some suggestion.

Vishwa

>
> Kevin
>
>
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > ---
> >  arch/arm/mach-omap2/cpuidle34xx.c |   20 ++++++++++----------
> >  1 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-
> omap2/cpuidle34xx.c
> > index f8e35b3..1e4ec7f 100644
> > --- a/arch/arm/mach-omap2/cpuidle34xx.c
> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> > @@ -139,19 +139,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);
> > @@ -315,8 +305,18 @@ static int omap3_enter_idle_bm(struct
> cpuidle_device *dev,
> >
> >  select_state:
> >  	dev->last_state = new_state;
> > +
> > +	if (new_state == dev->safe_state) {
> > +		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_deny_idle);
> > +		pwrdm_for_each_clkdm(core_pd, _cpuidle_deny_idle);
> > +	}
> >  	ret = omap3_enter_idle(dev, new_state);
> >
> > +	if (new_state == dev->safe_state) {
> > +		pwrdm_for_each_clkdm(mpu_pd, _cpuidle_allow_idle);
> > +		pwrdm_for_each_clkdm(core_pd, _cpuidle_allow_idle);
> > +	}
> > +
> >  	/* Restore original PER state if it was modified */
> >  	if (per_next_state != per_saved_state)
> >  		pwrdm_set_next_pwrst(per_pd, per_saved_state);

Re: [PATCH 1/3] OMAP3 PM: Deny clock gating only for safe state

[Kevin Hilman]

Vishwanath Sripathy <vishwanath.bs@ti.com> writes:

> Kevin,
>
>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman@ti.com]
>> Sent: Wednesday, March 02, 2011 2:32 AM
>> To: Vishwanath BS
>> Cc: linux-omap@vger.kernel.org; patches@linaro.org
>> Subject: Re: [PATCH 1/3] OMAP3 PM: Deny clock gating only for safe
>> state
>>
>> Vishwanath BS <vishwanath.bs@ti.com> writes:
>>
>> > Currently clock gating for MPU and core are denied whenever C1 state
>> is
>> > selected.
>>
>> Yes, that is the definition of C1.
>>
>> > It should be denied only when safe state is selected.
>>
>> Why?
> Clock gating in C1 will reduce overall power consumption and it should not
> impact any functionality as well.

Clock gating changes latencies.

>>
>> This changes the definition and behavior of C1 depending on how it is
>> entered.  Not a good idea IMO.
> I thought of adding a new C state, keeping C1 definition unchanged. Then
> we will not get the real power benefit since in case of C1, clock gating
> will be prevented. Let me know if you have some suggestion.

My problem with this patch is that you make C1 have different behavior
depending on whether it is selected directly by the governor or it is
entered due to the safe state.

The motiviation for this change is not at all described in the
changelog.

Kevin

[PATCH 2/3] OMAP3 PM: Update only enabled C states

[Vishwanath BS]

Currently function omap3_cpuidle_update_states updates valid flag of all the C 
states which includes the one which are disabled at init. So it's not really 
possible to selectively disable some of the C states using valid flag.
This is fixed now by adding another flag called enabled which will be 
initialized at omap_init_power_states. So update_states will operated only on 
enabled C states.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/cpuidle34xx.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
index 1e4ec7f..40c020a
--- a/arch/arm/mach-omap2/cpuidle34xx.c
+++ b/arch/arm/mach-omap2/cpuidle34xx.c
@@ -51,6 +51,7 @@
 
 struct omap3_processor_cx {
 	u8 valid;
+	u8 enabled;
 	u8 type;
 	u32 sleep_latency;
 	u32 wakeup_latency;
@@ -341,12 +342,13 @@ void omap3_cpuidle_update_states(u32 mpu_deepest_state, u32 core_deepest_state)
 
 	for (i = OMAP3_STATE_C1; i < OMAP3_MAX_STATES; i++) {
 		struct omap3_processor_cx *cx = &omap3_power_states[i];
-
-		if ((cx->mpu_state >= mpu_deepest_state) &&
-		    (cx->core_state >= core_deepest_state)) {
-			cx->valid = 1;
-		} else {
-			cx->valid = 0;
+		if (cx->enabled) {
+			if ((cx->mpu_state >= mpu_deepest_state) &&
+			    (cx->core_state >= core_deepest_state)) {
+				cx->valid = 1;
+			} else {
+				cx->valid = 0;
+			}
 		}
 	}
 }
@@ -387,6 +389,8 @@ void omap_init_power_states(void)
 	/* C1 . MPU WFI + Core active */
 	omap3_power_states[OMAP3_STATE_C1].valid =
 			cpuidle_params_table[OMAP3_STATE_C1].valid;
+	omap3_power_states[OMAP3_STATE_C1].enabled =
+			cpuidle_params_table[OMAP3_STATE_C1].valid;
 	omap3_power_states[OMAP3_STATE_C1].type = OMAP3_STATE_C1;
 	omap3_power_states[OMAP3_STATE_C1].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C1].sleep_latency;
@@ -401,6 +405,8 @@ void omap_init_power_states(void)
 	/* C2 . MPU WFI + Core inactive */
 	omap3_power_states[OMAP3_STATE_C2].valid =
 			cpuidle_params_table[OMAP3_STATE_C2].valid;
+	omap3_power_states[OMAP3_STATE_C2].enabled =
+			cpuidle_params_table[OMAP3_STATE_C2].valid;
 	omap3_power_states[OMAP3_STATE_C2].type = OMAP3_STATE_C2;
 	omap3_power_states[OMAP3_STATE_C2].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C2].sleep_latency;
@@ -416,6 +422,8 @@ void omap_init_power_states(void)
 	/* C3 . MPU CSWR + Core inactive */
 	omap3_power_states[OMAP3_STATE_C3].valid =
 			cpuidle_params_table[OMAP3_STATE_C3].valid;
+	omap3_power_states[OMAP3_STATE_C3].enabled =
+			cpuidle_params_table[OMAP3_STATE_C3].valid;
 	omap3_power_states[OMAP3_STATE_C3].type = OMAP3_STATE_C3;
 	omap3_power_states[OMAP3_STATE_C3].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C3].sleep_latency;
@@ -431,6 +439,8 @@ void omap_init_power_states(void)
 	/* C4 . MPU OFF + Core inactive */
 	omap3_power_states[OMAP3_STATE_C4].valid =
 			cpuidle_params_table[OMAP3_STATE_C4].valid;
+	omap3_power_states[OMAP3_STATE_C4].enabled =
+			cpuidle_params_table[OMAP3_STATE_C4].valid;
 	omap3_power_states[OMAP3_STATE_C4].type = OMAP3_STATE_C4;
 	omap3_power_states[OMAP3_STATE_C4].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C4].sleep_latency;
@@ -446,6 +456,8 @@ void omap_init_power_states(void)
 	/* C5 . MPU CSWR + Core CSWR*/
 	omap3_power_states[OMAP3_STATE_C5].valid =
 			cpuidle_params_table[OMAP3_STATE_C5].valid;
+	omap3_power_states[OMAP3_STATE_C5].enabled =
+			cpuidle_params_table[OMAP3_STATE_C5].valid;
 	omap3_power_states[OMAP3_STATE_C5].type = OMAP3_STATE_C5;
 	omap3_power_states[OMAP3_STATE_C5].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C5].sleep_latency;
@@ -461,6 +473,8 @@ void omap_init_power_states(void)
 	/* C6 . MPU OFF + Core CSWR */
 	omap3_power_states[OMAP3_STATE_C6].valid =
 			cpuidle_params_table[OMAP3_STATE_C6].valid;
+	omap3_power_states[OMAP3_STATE_C6].enabled =
+			cpuidle_params_table[OMAP3_STATE_C6].valid;
 	omap3_power_states[OMAP3_STATE_C6].type = OMAP3_STATE_C6;
 	omap3_power_states[OMAP3_STATE_C6].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C6].sleep_latency;
@@ -476,6 +490,8 @@ void omap_init_power_states(void)
 	/* C7 . MPU OFF + Core OFF */
 	omap3_power_states[OMAP3_STATE_C7].valid =
 			cpuidle_params_table[OMAP3_STATE_C7].valid;
+	omap3_power_states[OMAP3_STATE_C7].enabled =
+			cpuidle_params_table[OMAP3_STATE_C7].valid;
 	omap3_power_states[OMAP3_STATE_C7].type = OMAP3_STATE_C7;
 	omap3_power_states[OMAP3_STATE_C7].sleep_latency =
 			cpuidle_params_table[OMAP3_STATE_C7].sleep_latency;
@@ -495,6 +511,7 @@ void omap_init_power_states(void)
 	 */
 	if (IS_PM34XX_ERRATUM(PM_SDRC_WAKEUP_ERRATUM_i583)) {
 		omap3_power_states[OMAP3_STATE_C7].valid = 0;
+		omap3_power_states[OMAP3_STATE_C7].enabled = 0;
 		cpuidle_params_table[OMAP3_STATE_C7].valid = 0;
 		pr_warn("%s: core off state C7 disabled due to i583\n",
 				__func__);
-- 
1.7.0.4

[PATCH 3/3] OMAP36xx PM: Updated C state latencies for OMAP3630

[Vishwanath BS]

This patch has changes to update the C state latencies for OMAP3630 and
disables the useless C-States, keeping only the optimized ones with their
corresponding measured latencies. Only 4 C-states are kept instead of 7
C-States:
* C1 . MPU WFI clock gated + Core autogating
* C3 . MPU CSWR + Core inactive
* C5 . MPU CSWR + Core CSWR
* C7 . MPU OFF + Core OFF

Thanks to Nicole Chaloub<n-chalhoub@ti.com> and Vincent Bour <v-bour@ti.com>
for their investigation.

Tested on ZOOM3 board using latest pm branch.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/board-3630sdp.c |   19 +++++++++++++++++++
 arch/arm/mach-omap2/board-zoom.c    |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-3630sdp.c b/arch/arm/mach-omap2/board-3630sdp.c
index 6264564..8d05fc9
--- a/arch/arm/mach-omap2/board-3630sdp.c
+++ b/arch/arm/mach-omap2/board-3630sdp.c
@@ -25,6 +25,24 @@
 #include "board-flash.h"
 #include "mux.h"
 #include "sdram-hynix-h8mbx00u0mer-0em.h"
+#include "pm.h"
+
+static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
+	/* C1 */
+	{1, 74, 78, 152},
+	/* C2 */
+	{0, 165, 90, 255},
+	/* C3 */
+	{1, 163, 180, 345},
+	/* C4 */
+	{0, 2852, 605, 3457},
+	/* C5 */
+	{1, 800, 366, 2120},
+	/* C6 */
+	{0, 4080, 801, 4881},
+	/* C7 */
+	{1, 4300, 8794, 159000},
+};
 
 #if defined(CONFIG_SMC91X) || defined(CONFIG_SMC91X_MODULE)
 
@@ -212,6 +230,7 @@ static void __init omap_sdp_init(void)
 	board_flash_init(sdp_flash_partitions, chip_sel_sdp);
 	enable_board_wakeup_source();
 	usb_ehci_init(&ehci_pdata);
+	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
 }
 
 MACHINE_START(OMAP_3630SDP, "OMAP 3630SDP board")
diff --git a/arch/arm/mach-omap2/board-zoom.c b/arch/arm/mach-omap2/board-zoom.c
index e26754c..6bd364a
--- a/arch/arm/mach-omap2/board-zoom.c
+++ b/arch/arm/mach-omap2/board-zoom.c
@@ -30,6 +30,24 @@
 #include "mux.h"
 #include "sdram-micron-mt46h32m32lf-6.h"
 #include "sdram-hynix-h8mbx00u0mer-0em.h"
+#include "pm.h"
+
+static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
+	/* C1 */
+	{1, 74, 78, 152},
+	/* C2 */
+	{0, 165, 90, 255},
+	/* C3 */
+	{1, 163, 180, 345},
+	/* C4 */
+	{0, 2852, 605, 3457},
+	/* C5 */
+	{1, 800, 366, 2120},
+	/* C6 */
+	{0, 4080, 801, 4881},
+	/* C7 */
+	{1, 4300, 8794, 159000},
+};
 
 #define ZOOM3_EHCI_RESET_GPIO		64
 
@@ -126,6 +144,7 @@ static void __init omap_zoom_init(void)
 		usb_ehci_init(&ehci_pdata);
 	}
 
+	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
 	board_nand_init(zoom_nand_partitions,
 			ARRAY_SIZE(zoom_nand_partitions), ZOOM_NAND_CS);
 	zoom_debugboard_init();
-- 
1.7.0.4

Re: [PATCH 3/3] OMAP36xx PM: Updated C state latencies for OMAP3630

[Kevin Hilman]

Vishwanath BS <vishwanath.bs@ti.com> writes:

> This patch has changes to update the C state latencies for OMAP3630 and
> disables the useless C-States, keeping only the optimized ones with their
> corresponding measured latencies. 

I think "useless" vs. "optimized" here are not the right words, and
doesn't summarize the problem being solved.

If I understood Nicole and Vincent's summary it was that there is no
significant power savings between some of the states, so they are not
necessary.

Also, I believe Nicole & Vincent's research applies to both 34xx and
36xx.

> Only 4 C-states are kept instead of 7
> C-States:
> * C1 . MPU WFI clock gated + Core autogating
> * C3 . MPU CSWR + Core inactive
> * C5 . MPU CSWR + Core CSWR
> * C7 . MPU OFF + Core OFF
>
> Thanks to Nicole Chaloub<n-chalhoub@ti.com> and Vincent Bour <v-bour@ti.com>
> for their investigation.
>
> Tested on ZOOM3 board using latest pm branch.
>
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> ---
>  arch/arm/mach-omap2/board-3630sdp.c |   19 +++++++++++++++++++
>  arch/arm/mach-omap2/board-zoom.c    |   19 +++++++++++++++++++

Why are these added as board-specific changes?

If these are to be the default 3630 CPUidle states, add a default 36xx
cpuidle_params_table along side the 34xx one and have CPUidle pick the
right one at runtime.

Kevin

>  2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-3630sdp.c b/arch/arm/mach-omap2/board-3630sdp.c
> index 6264564..8d05fc9
> --- a/arch/arm/mach-omap2/board-3630sdp.c
> +++ b/arch/arm/mach-omap2/board-3630sdp.c
> @@ -25,6 +25,24 @@
>  #include "board-flash.h"
>  #include "mux.h"
>  #include "sdram-hynix-h8mbx00u0mer-0em.h"
> +#include "pm.h"
> +
> +static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
> +	/* C1 */
> +	{1, 74, 78, 152},
> +	/* C2 */
> +	{0, 165, 90, 255},
> +	/* C3 */
> +	{1, 163, 180, 345},
> +	/* C4 */
> +	{0, 2852, 605, 3457},
> +	/* C5 */
> +	{1, 800, 366, 2120},
> +	/* C6 */
> +	{0, 4080, 801, 4881},
> +	/* C7 */
> +	{1, 4300, 8794, 159000},
> +};
>  
>  #if defined(CONFIG_SMC91X) || defined(CONFIG_SMC91X_MODULE)
>  
> @@ -212,6 +230,7 @@ static void __init omap_sdp_init(void)
>  	board_flash_init(sdp_flash_partitions, chip_sel_sdp);
>  	enable_board_wakeup_source();
>  	usb_ehci_init(&ehci_pdata);
> +	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
>  }
>  
>  MACHINE_START(OMAP_3630SDP, "OMAP 3630SDP board")
> diff --git a/arch/arm/mach-omap2/board-zoom.c b/arch/arm/mach-omap2/board-zoom.c
> index e26754c..6bd364a
> --- a/arch/arm/mach-omap2/board-zoom.c
> +++ b/arch/arm/mach-omap2/board-zoom.c
> @@ -30,6 +30,24 @@
>  #include "mux.h"
>  #include "sdram-micron-mt46h32m32lf-6.h"
>  #include "sdram-hynix-h8mbx00u0mer-0em.h"
> +#include "pm.h"
> +
> +static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
> +	/* C1 */
> +	{1, 74, 78, 152},
> +	/* C2 */
> +	{0, 165, 90, 255},
> +	/* C3 */
> +	{1, 163, 180, 345},
> +	/* C4 */
> +	{0, 2852, 605, 3457},
> +	/* C5 */
> +	{1, 800, 366, 2120},
> +	/* C6 */
> +	{0, 4080, 801, 4881},
> +	/* C7 */
> +	{1, 4300, 8794, 159000},
> +};
>  
>  #define ZOOM3_EHCI_RESET_GPIO		64
>  
> @@ -126,6 +144,7 @@ static void __init omap_zoom_init(void)
>  		usb_ehci_init(&ehci_pdata);
>  	}
>  
> +	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
>  	board_nand_init(zoom_nand_partitions,
>  			ARRAY_SIZE(zoom_nand_partitions), ZOOM_NAND_CS);
>  	zoom_debugboard_init();

RE: [PATCH 3/3] OMAP36xx PM: Updated C state latencies for OMAP3630

[Vishwanath Sripathy]

> -----Original Message-----
> From: Kevin Hilman [mailto:khilman@ti.com]
> Sent: Wednesday, March 02, 2011 3:15 AM
> To: Vishwanath BS
> Cc: linux-omap@vger.kernel.org; patches@linaro.org
> Subject: Re: [PATCH 3/3] OMAP36xx PM: Updated C state latencies for
> OMAP3630
>
> Vishwanath BS <vishwanath.bs@ti.com> writes:
>
> > This patch has changes to update the C state latencies for OMAP3630
> and
> > disables the useless C-States, keeping only the optimized ones with
> their
> > corresponding measured latencies.
>
> I think "useless" vs. "optimized" here are not the right words, and
> doesn't summarize the problem being solved.
>
> If I understood Nicole and Vincent's summary it was that there is no
> significant power savings between some of the states, so they are not
> necessary.
Yes, also some of the C states were consuming more power than deeper C
state.

>
> Also, I believe Nicole & Vincent's research applies to both 34xx and
> 36xx.
I need to check. AFAIK, the experiments were done on OMAP3630 boards.
>
> > Only 4 C-states are kept instead of 7
> > C-States:
> > * C1 . MPU WFI clock gated + Core autogating
> > * C3 . MPU CSWR + Core inactive
> > * C5 . MPU CSWR + Core CSWR
> > * C7 . MPU OFF + Core OFF
> >
> > Thanks to Nicole Chaloub<n-chalhoub@ti.com> and Vincent Bour <v-
> bour@ti.com>
> > for their investigation.
> >
> > Tested on ZOOM3 board using latest pm branch.
> >
> > Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> > ---
> >  arch/arm/mach-omap2/board-3630sdp.c |   19
> +++++++++++++++++++
> >  arch/arm/mach-omap2/board-zoom.c    |   19
> +++++++++++++++++++
>
> Why are these added as board-specific changes?
>
> If these are to be the default 3630 CPUidle states, add a default 36xx
> cpuidle_params_table along side the 34xx one and have CPUidle pick the
> right one at runtime.
Yes, I can do that.

Vishwa
>
> Kevin
>
> >  2 files changed, 38 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-3630sdp.c b/arch/arm/mach-
> omap2/board-3630sdp.c
> > index 6264564..8d05fc9
> > --- a/arch/arm/mach-omap2/board-3630sdp.c
> > +++ b/arch/arm/mach-omap2/board-3630sdp.c
> > @@ -25,6 +25,24 @@
> >  #include "board-flash.h"
> >  #include "mux.h"
> >  #include "sdram-hynix-h8mbx00u0mer-0em.h"
> > +#include "pm.h"
> > +
> > +static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
> > +	/* C1 */
> > +	{1, 74, 78, 152},
> > +	/* C2 */
> > +	{0, 165, 90, 255},
> > +	/* C3 */
> > +	{1, 163, 180, 345},
> > +	/* C4 */
> > +	{0, 2852, 605, 3457},
> > +	/* C5 */
> > +	{1, 800, 366, 2120},
> > +	/* C6 */
> > +	{0, 4080, 801, 4881},
> > +	/* C7 */
> > +	{1, 4300, 8794, 159000},
> > +};
> >
> >  #if defined(CONFIG_SMC91X) || defined(CONFIG_SMC91X_MODULE)
> >
> > @@ -212,6 +230,7 @@ static void __init omap_sdp_init(void)
> >  	board_flash_init(sdp_flash_partitions, chip_sel_sdp);
> >  	enable_board_wakeup_source();
> >  	usb_ehci_init(&ehci_pdata);
> > +	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
> >  }
> >
> >  MACHINE_START(OMAP_3630SDP, "OMAP 3630SDP board")
> > diff --git a/arch/arm/mach-omap2/board-zoom.c b/arch/arm/mach-
> omap2/board-zoom.c
> > index e26754c..6bd364a
> > --- a/arch/arm/mach-omap2/board-zoom.c
> > +++ b/arch/arm/mach-omap2/board-zoom.c
> > @@ -30,6 +30,24 @@
> >  #include "mux.h"
> >  #include "sdram-micron-mt46h32m32lf-6.h"
> >  #include "sdram-hynix-h8mbx00u0mer-0em.h"
> > +#include "pm.h"
> > +
> > +static struct cpuidle_params omap36xx_cpuidle_params_table[] = {
> > +	/* C1 */
> > +	{1, 74, 78, 152},
> > +	/* C2 */
> > +	{0, 165, 90, 255},
> > +	/* C3 */
> > +	{1, 163, 180, 345},
> > +	/* C4 */
> > +	{0, 2852, 605, 3457},
> > +	/* C5 */
> > +	{1, 800, 366, 2120},
> > +	/* C6 */
> > +	{0, 4080, 801, 4881},
> > +	/* C7 */
> > +	{1, 4300, 8794, 159000},
> > +};
> >
> >  #define ZOOM3_EHCI_RESET_GPIO		64
> >
> > @@ -126,6 +144,7 @@ static void __init omap_zoom_init(void)
> >  		usb_ehci_init(&ehci_pdata);
> >  	}
> >
> > +	omap3_pm_init_cpuidle(omap36xx_cpuidle_params_table);
> >  	board_nand_init(zoom_nand_partitions,
> >  			ARRAY_SIZE(zoom_nand_partitions),
> ZOOM_NAND_CS);
> >  	zoom_debugboard_init();