"ext Premi, Sanjeev" <premi@xxxxxx> writes: >> -----Original Message----- >> From: Högander Jouni [mailto:jouni.hogander@xxxxxxxxx] >> Sent: Monday, March 09, 2009 3:38 PM >> To: Premi, Sanjeev >> Cc: linux-omap@xxxxxxxxxxxxxxx >> Subject: Re: [PATCHv2] PM : cpuidle - Update statistics for >> correct state >> >> ext Sanjeev Premi <premi@xxxxxx> writes: >> >> > When 'enable_off_mode' is 0, and (mpu_state < PWRDM_POWER_RET) the >> > local variables mpu_state and core_state are modified; but >> the usage >> > count for the original state selected by the governor are updated. >> > >> > This patch updates the 'last_state' in the cpuidle driver to ensure >> > that statistics for the correct state are updated. >> > >> > Signed-off-by: Sanjeev Premi <premi@xxxxxx> >> > --- >> > arch/arm/mach-omap2/cpuidle34xx.c | 29 >> +++++++++++++++++++---------- >> > 1 files changed, 19 insertions(+), 10 deletions(-) >> > >> > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c >> > b/arch/arm/mach-omap2/cpuidle34xx.c >> > index 62fbb2e..b138abd 100644 >> > --- a/arch/arm/mach-omap2/cpuidle34xx.c >> > +++ b/arch/arm/mach-omap2/cpuidle34xx.c >> > @@ -76,23 +76,32 @@ static int omap3_enter_idle(struct >> cpuidle_device >> > *dev, { >> > struct omap3_processor_cx *cx = cpuidle_get_statedata(state); >> > struct timespec ts_preidle, ts_postidle, ts_idle; >> > - u32 mpu_state = cx->mpu_state, core_state = cx->core_state; >> > - >> > - current_cx_state = *cx; >> > + u32 mpu_state, core_state; >> > >> > /* Used to keep track of the total time in idle */ >> > getnstimeofday(&ts_preidle); >> > >> > - local_irq_disable(); >> > - local_fiq_disable(); >> > - >> > + /* >> > + * Adjust the idle state (if required). >> > + * Also, ensure that usage statistics of correct state >> are updated. >> > + */ >> > if (!enable_off_mode) { >> > - if (mpu_state < PWRDM_POWER_RET) >> > - mpu_state = PWRDM_POWER_RET; >> > - if (core_state < PWRDM_POWER_RET) >> > - core_state = PWRDM_POWER_RET; >> > + if (cx->type > OMAP3_STATE_C4) { >> > + state = &(dev->states[OMAP3_STATE_C4 - 1]); >> > + dev->last_state = state ; >> > + >> > + cx = cpuidle_get_statedata(state); >> >> There is still C3 where OFF is used for MPU. This needs to be >> taken into account. > > [sp] Thanks. Good catch! > I wasn't happy doing the "OMAP3_STATEn - 1"; but could not find a better way. > It should be C2 as defined now. This means C4 is not used if off mode is not enabled? I think this is not wanted. Would it be possible to remove "OFF" C states when enable_off_mode is written to 0 and add them back when 1 written? > > On another note, would it make sense to swap the definitions for C3 and C4. > C3 : MPU CSWR + CORE CSWR > C4 : MPU OFF + CORE Actove No it doesn't. They are organized by latency. One grounding for current implementation is that enable_off_mode is more or less testing interface. In final solution it might be even removed. Adjusting states directly still shows guite accurate information on used C-states. > >> >> > + } >> > } >> > >> > + current_cx_state = *cx; >> > + >> > + mpu_state = cx->mpu_state; >> > + core_state = cx->core_state; >> > + >> > + local_irq_disable(); >> > + local_fiq_disable(); >> > + >> > pwrdm_set_next_pwrst(mpu_pd, mpu_state); >> > pwrdm_set_next_pwrst(core_pd, core_state); >> > >> > -- >> > 1.5.6 >> > >> > -- >> > 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
