"ext Rajendra Nayak" <rnayak@xxxxxx> writes: > >> -----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. This is something I didn't know. Now it seems to be reasonable also to me to do these clock disables here:) Now it seems that we might still need to have some mean to add these deps statically. Wkdep between core and per might be needed at some point. Currently dep between per and mpu is enough, but if gpio clock disable is added to gpio code, then wkdep needs to be added. If statement is still even more unnecessary here, because it will never be anything else than per_pwrst == PWRDM_POWER_ON. > >> >> > + 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. Yes, you are right. As there is that hw sleep dep per domain wont enter ret without core. For all cases where core sleeps fclk check is done. > >> >> > } >> > >> > 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 >> >> > > > -- 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
