>-----Original Message----- >From: ext Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] >Sent: 16 November, 2009 21:59 >To: Kristo Tero (Nokia-D/Tampere) >Cc: linux-omap@xxxxxxxxxxxxxxx >Subject: Re: [PATCH 6/6] OMAP3: CPUidle: Added peripheral >pwrdm checks into bm check > >Tero Kristo <tero.kristo@xxxxxxxxx> writes: > >> From: Tero Kristo <tero.kristo@xxxxxxxxx> >> >> 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@xxxxxxxxx> > >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 >-- 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
