> -----Original Message----- > From: "Högander" Jouni [mailto:jouni.hogander@xxxxxxxxx] > Sent: Wednesday, July 02, 2008 5:19 PM > To: ext Rajendra Nayak > Cc: linux-omap@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 02/11] per/neon and core handling in idle > > "ext Rajendra Nayak" <rnayak@xxxxxx> writes: > > > This patches adds handling of PER/NEON and CORE domain in idle. > > > > Signed-off-by: Rajendra Nayak <rnayak@xxxxxx> > > > > --- > > 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 majordomo@xxxxxxxxxxxxxxx > > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
