> -----Original Message-----
> From: "Högander" Jouni [mailto:[EMAIL PROTECTED]
> Sent: Wednesday, July 02, 2008 5:19 PM
> To: ext Rajendra Nayak
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 02/11] per/neon and core handling in idle
>
> "ext Rajendra Nayak" <[EMAIL PROTECTED]> writes:
>
> > This patches adds handling of PER/NEON and CORE domain in idle.
> >
> > Signed-off-by: Rajendra Nayak <[EMAIL PROTECTED]>
> >
> > ---
> > arch/arm/mach-omap2/cpuidle34xx.c | 98
> +++++++++++++++++++++++++++++++-------
> > arch/arm/mach-omap2/cpuidle34xx.h | 6 +-
> > arch/arm/mach-omap2/pm34xx.c | 58 ++++++++++++++--------
> > 3 files changed, 121 insertions(+), 41 deletions(-)
> >
> > Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
> > ===================================================================
> > --- linux-omap-2.6.orig/arch/arm/mach-omap2/cpuidle34xx.c
> 2008-07-01 18:48:09.962433167 +0530
> > +++ linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.c
> 2008-07-01 18:48:12.884339399 +0530
> > @@ -26,6 +26,8 @@
> > #include <asm/arch/pm.h>
> > #include <asm/arch/prcm.h>
> > #include <asm/arch/powerdomain.h>
> > +#include <asm/arch/clockdomain.h>
> > +#include <asm/arch/gpio.h>
> > #include "cpuidle34xx.h"
> >
> > #ifdef CONFIG_CPU_IDLE
> > @@ -35,6 +37,8 @@ struct omap3_processor_cx current_cx_sta
> >
> > static int omap3_idle_bm_check(void)
> > {
> > + if (!omap3_can_sleep())
> > + return 1;
> > return 0;
> > }
> >
> > @@ -46,34 +50,86 @@ static int omap3_enter_idle(struct cpuid
> > {
> > struct omap3_processor_cx *cx = cpuidle_get_statedata(state);
> > struct timespec ts_preidle, ts_postidle, ts_idle;
> > - struct powerdomain *mpu_pd;
> > + struct powerdomain *mpu_pd, *core_pd, *per_pd, *neon_pd;
> > + int per_pwrst, neon_pwrst;
> >
> > current_cx_state = *cx;
> >
> > - /* Used to keep track of the total time in idle */
> > - getnstimeofday(&ts_preidle);
> > -
> > -
> > if (cx->type == OMAP3_STATE_C0) {
> > /* Do nothing for C0, not even a wfi */
> > return 0;
> > }
> >
> > + /* Used to keep track of the total time in idle */
> > + getnstimeofday(&ts_preidle);
> > +
> > mpu_pd = pwrdm_lookup("mpu_pwrdm");
> > + core_pd = pwrdm_lookup("core_pwrdm");
> > + per_pd = pwrdm_lookup("per_pwrdm");
> > + neon_pd = pwrdm_lookup("neon_pwrdm");
> > +
> > + /* Reset previous power state registers */
> > + pwrdm_clear_all_prev_pwrst(mpu_pd);
> > + pwrdm_clear_all_prev_pwrst(neon_pd);
> > + pwrdm_clear_all_prev_pwrst(core_pd);
> > + pwrdm_clear_all_prev_pwrst(per_pd);
> > +
> > + if (omap_irq_pending())
> > + return 0;
> > +
> > + per_pwrst = pwrdm_read_pwrst(per_pd);
> > + neon_pwrst = pwrdm_read_pwrst(neon_pd);
> > +
> > /* Program MPU to target state */
> > - if (cx->mpu_state < PWRDM_POWER_ON)
> > + if (cx->mpu_state < PWRDM_POWER_ON) {
> > + if (neon_pwrst == PWRDM_POWER_ON)
> > + pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_RET);
> > pwrdm_set_next_pwrst(mpu_pd, cx->mpu_state);
> > + }
> > +
> > + /* Program CORE and PER to target state */
> > + if (cx->core_state < PWRDM_POWER_ON) {
> > + if (per_pwrst == PWRDM_POWER_ON) {
> > + omap2_gpio_prepare_for_retention();
> > + if (clocks_off_while_idle) {
> > + per_gpio_clk_disable();
> > + omap_serial_enable_clocks(0);
> > + }
> > + }
>
> Why this is here? Why per is not put to sleep state if core is not?
> Shouldn't you just disable gpio clocks in omap_sram_idle and if all
> clocks in per domain are cut off then per domain enters its sleep
> state. No matter what is the state of core domain.
>
> Why per_pwrst is checked here? You know it is always on currently. It
> won't never be anything else until those things in this if block is
> done.
>
> As the day comes when per_pwrst here is something else than
> PWRDM_POWER_ON then serial clocks in core domain are not disabled
> because disabling them are in that block.
PER domain has a hardwired sleep dep with CORE-L3, hence the above.
Also IO wakeup works only while CORE is in RET/OFF. While CORE is active
you need GPIO to wake you up. Hence cutting GPIO clocks only while
you attempt a CORE RET/OFF makes more sense, so that while GPIO is down
the IO pad can wake you up.
>
> > + pwrdm_set_next_pwrst(core_pd, cx->core_state);
> > + }
> >
> > /* Execute ARM wfi */
> > omap_sram_idle();
> >
> > - /* Program MPU to ON */
> > - if (cx->mpu_state < PWRDM_POWER_ON)
> > + /* Program MPU/NEON to ON */
> > + if (cx->mpu_state < PWRDM_POWER_ON) {
> > + if (neon_pwrst == PWRDM_POWER_ON)
> > + pwrdm_set_next_pwrst(neon_pd, PWRDM_POWER_ON);
> > pwrdm_set_next_pwrst(mpu_pd, PWRDM_POWER_ON);
> > + }
>
> No need to do these as they are written anyway in the next round. MPU
> and NEON are not going to change their state in between.
Yes, I can remove these.
>
> > +
> > + if (cx->core_state < PWRDM_POWER_ON) {
> > + if (per_pwrst == PWRDM_POWER_ON) {
> > + if (clocks_off_while_idle) {
> > + omap_serial_enable_clocks(1);
> > + per_gpio_clk_enable();
> > + }
> > + omap2_gpio_resume_after_retention();
>
> Similiar comments as before omap_sram_idle() for this block.
>
> > + }
> > + pwrdm_set_next_pwrst(core_pd, PWRDM_POWER_ON);
>
> This can be also left to value before omap_sram_idle(), because it is
> anyway written again in next round.
>
> > + }
> > +
> > + pr_debug("MPU prev st:%x,NEON prev st:%x\n",
> > + pwrdm_read_prev_pwrst(mpu_pd),
> > + pwrdm_read_prev_pwrst(neon_pd));
> > + pr_debug("PER prev st:%x,CORE prev st:%x\n",
> > + pwrdm_read_prev_pwrst(per_pd),
> > + pwrdm_read_prev_pwrst(core_pd));
> >
> > getnstimeofday(&ts_postidle);
> > ts_idle = timespec_sub(ts_postidle, ts_preidle);
> > - return timespec_to_ns(&ts_idle);
> > + return (u32)timespec_to_ns(&ts_idle)/1000;
> > }
> >
> > static int omap3_enter_idle_bm(struct cpuidle_device *dev,
> > @@ -130,7 +186,7 @@ void omap_init_power_states(void)
> > omap3_power_states[0].threshold = 0;
> > omap3_power_states[0].mpu_state = PWRDM_POWER_ON;
> > omap3_power_states[0].core_state = PWRDM_POWER_ON;
> > - omap3_power_states[0].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[0].flags = CPUIDLE_FLAG_SHALLOW;
> >
> > /* C1 . MPU WFI + Core active */
> > omap3_power_states[1].valid = 1;
> > @@ -140,7 +196,8 @@ void omap_init_power_states(void)
> > omap3_power_states[1].threshold = 30;
> > omap3_power_states[1].mpu_state = PWRDM_POWER_ON;
> > omap3_power_states[1].core_state = PWRDM_POWER_ON;
> > - omap3_power_states[1].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[1].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_SHALLOW;
> >
> > /* C2 . MPU CSWR + Core active */
> > omap3_power_states[2].valid = 1;
> > @@ -150,7 +207,8 @@ void omap_init_power_states(void)
> > omap3_power_states[2].threshold = 300;
> > omap3_power_states[2].mpu_state = PWRDM_POWER_RET;
> > omap3_power_states[2].core_state = PWRDM_POWER_ON;
> > - omap3_power_states[2].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[2].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_BALANCED;
> >
> > /* C3 . MPU OFF + Core active */
> > omap3_power_states[3].valid = 0;
> > @@ -159,18 +217,20 @@ void omap_init_power_states(void)
> > omap3_power_states[3].wakeup_latency = 1800;
> > omap3_power_states[3].threshold = 4000;
> > omap3_power_states[3].mpu_state = PWRDM_POWER_OFF;
> > - omap3_power_states[3].core_state = PWRDM_POWER_RET;
> > - omap3_power_states[3].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[3].core_state = PWRDM_POWER_ON;
> > + omap3_power_states[3].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
> >
> > /* C4 . MPU CSWR + Core CSWR*/
> > - omap3_power_states[4].valid = 0;
> > + omap3_power_states[4].valid = 1;
> > omap3_power_states[4].type = OMAP3_STATE_C4;
> > omap3_power_states[4].sleep_latency = 2500;
> > omap3_power_states[4].wakeup_latency = 7500;
> > omap3_power_states[4].threshold = 12000;
> > omap3_power_states[4].mpu_state = PWRDM_POWER_RET;
> > omap3_power_states[4].core_state = PWRDM_POWER_RET;
> > - omap3_power_states[4].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[4].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
> >
> > /* C5 . MPU OFF + Core CSWR */
> > omap3_power_states[5].valid = 0;
> > @@ -180,7 +240,8 @@ void omap_init_power_states(void)
> > omap3_power_states[5].threshold = 15000;
> > omap3_power_states[5].mpu_state = PWRDM_POWER_OFF;
> > omap3_power_states[5].core_state = PWRDM_POWER_RET;
> > - omap3_power_states[5].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[5].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_BALANCED | CPUIDLE_FLAG_CHECK_BM;
> >
> > /* C6 . MPU OFF + Core OFF */
> > omap3_power_states[6].valid = 0;
> > @@ -190,7 +251,8 @@ void omap_init_power_states(void)
> > omap3_power_states[6].threshold = 300000;
> > omap3_power_states[6].mpu_state = PWRDM_POWER_OFF;
> > omap3_power_states[6].core_state = PWRDM_POWER_OFF;
> > - omap3_power_states[6].flags = CPUIDLE_FLAG_TIME_VALID;
> > + omap3_power_states[6].flags = CPUIDLE_FLAG_TIME_VALID |
> > + CPUIDLE_FLAG_DEEP | CPUIDLE_FLAG_CHECK_BM;
> > }
> >
> > struct cpuidle_driver omap3_idle_driver = {
> > Index: linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.h
> > ===================================================================
> > --- linux-omap-2.6.orig/arch/arm/mach-omap2/cpuidle34xx.h
> 2008-07-01 18:48:09.962433167 +0530
> > +++ linux-omap-2.6/arch/arm/mach-omap2/cpuidle34xx.h
> 2008-07-01 18:48:12.885339367 +0530
> > @@ -33,11 +33,13 @@
> > /* Currently, we support only upto C2 */
> > #define MAX_SUPPORTED_STATES 3
> >
> > +extern unsigned short clocks_off_while_idle;
> > extern void omap_sram_idle(void);
> > -extern int omap3_irq_pending(void);
> > +extern int omap_irq_pending(void);
> > extern void per_gpio_clk_enable(void);
> > extern void per_gpio_clk_disable(void);
> > -
> > +extern void omap_serial_enable_clocks(int enable);
> > +extern int omap3_can_sleep();
> > struct omap3_processor_cx {
> > u8 valid;
> > u8 type;
> > Index: linux-omap-2.6/arch/arm/mach-omap2/pm34xx.c
> > ===================================================================
> > --- linux-omap-2.6.orig/arch/arm/mach-omap2/pm34xx.c
> 2008-07-01 18:48:09.962433167 +0530
> > +++ linux-omap-2.6/arch/arm/mach-omap2/pm34xx.c
> 2008-07-01 18:48:12.885339367 +0530
> > @@ -61,7 +61,7 @@ static struct clk *gpio_fcks[NUM_OF_PERG
> >
> > /* XXX This is for gpio fclk hack. Will be removed as gpio driver
> > * handles fcks correctly */
> > -static void per_gpio_clk_enable(void)
> > +void per_gpio_clk_enable(void)
> > {
> > int i;
> > for (i = 1; i < NUM_OF_PERGPIOS + 1; i++)
> > @@ -70,7 +70,7 @@ static void per_gpio_clk_enable(void)
> >
> > /* XXX This is for gpio fclk hack. Will be removed as gpio driver
> > * handles fcks correctly */
> > -static void per_gpio_clk_disable(void)
> > +void per_gpio_clk_disable(void)
> > {
> > int i;
> > for (i = 1; i < NUM_OF_PERGPIOS + 1; i++)
> > @@ -202,25 +202,7 @@ void omap_sram_idle(void)
> > return;
> > }
> >
> > - omap2_gpio_prepare_for_retention();
> > -
> > - if (clocks_off_while_idle) {
> > - omap_serial_enable_clocks(0);
> > - /* XXX This is for gpio fclk hack. Will be removed as
> > - * gpio driver * handles fcks correctly */
> > - per_gpio_clk_disable();
> > - }
> > -
> > _omap_sram_idle(NULL, save_state, clocks_off_while_idle);
> > -
> > - if (clocks_off_while_idle) {
> > - omap_serial_enable_clocks(1);
> > - /* XXX This is for gpio fclk hack. Will be removed as
> > - * gpio driver * handles fcks correctly */
> > - per_gpio_clk_enable();
> > - }
> > -
> > - omap2_gpio_resume_after_retention();
>
> Couldn't these be left here. I assume you want to do gpio and serial
> clock disabling only if per domain state is about to change. If this
> is what you want then you should check that all other clocks in per
> domain are disabled. If this is true then disable gpio and uart clocks
> here. Still this code could be left here if done that way.
All other fclks in PER are anyway checked for in omap3_fclks_active.
>
> > }
> >
> > static int omap3_fclks_active(void)
> > @@ -258,7 +240,7 @@ static int omap3_fclks_active(void)
> > return 0;
> > }
> >
> > -static int omap3_can_sleep(void)
> > +int omap3_can_sleep(void)
> > {
> > if (!enable_dyn_sleep)
> > return 0;
> > @@ -329,8 +311,25 @@ static void omap3_pm_idle(void)
> > if (omap_irq_pending())
> > goto out;
> >
> > + omap2_gpio_prepare_for_retention();
> > +
> > + if (clocks_off_while_idle) {
> > + omap_serial_enable_clocks(0);
> > + /* XXX This is for gpio fclk hack. Will be removed as
> > + * gpio driver * handles fcks correctly */
> > + per_gpio_clk_disable();
> > + }
>
> Same comment as above.
>
> > +
> > omap_sram_idle();
> >
> > + if (clocks_off_while_idle) {
> > + omap_serial_enable_clocks(1);
> > + /* XXX This is for gpio fclk hack. Will be removed as
> > + * gpio driver * handles fcks correctly */
> > + per_gpio_clk_enable();
> > + }
> > +
> > + omap2_gpio_resume_after_retention();
>
> and same goes here.
>
> > out:
> > local_fiq_enable();
> > local_irq_enable();
> > @@ -363,8 +362,25 @@ static int omap3_pm_suspend(void)
> > goto restore;
> > }
> >
> > + omap2_gpio_prepare_for_retention();
> > +
> > + if (clocks_off_while_idle) {
> > + omap_serial_enable_clocks(0);
> > + /* XXX This is for gpio fclk hack. Will be removed as
> > + * gpio driver * handles fcks correctly */
> > + per_gpio_clk_disable();
> > + }
> > +
>
> And here.
>
> > omap_sram_idle();
> >
> > + if (clocks_off_while_idle) {
> > + omap_serial_enable_clocks(1);
> > + /* XXX This is for gpio fclk hack. Will be removed as
> > + * gpio driver * handles fcks correctly */
> > + per_gpio_clk_enable();
> > + }
> > +
> > + omap2_gpio_resume_after_retention();
>
> And here.
>
> > restore:
> > /* Restore next_pwrsts */
> > list_for_each_entry(pwrst, &pwrst_list, node) {
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> linux-omap" in
> > the body of a message to [EMAIL PROTECTED]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >
>
> --
> Jouni Högander
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
