RE: [PATCHv2] PM : cpuidle - Update statistics for correct state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



 

> -----Original Message-----
> From: Högander Jouni [mailto:jouni.hogander@xxxxxxxxx] 
> Sent: Monday, March 09, 2009 4:07 PM
> To: Premi, Sanjeev
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCHv2] PM : cpuidle - Update statistics for 
> correct state
> 
> "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?

[sp] That should be possible. We could use the 'valid' field
     for the purpose.

> >
> >      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.

[sp] Okay. That was a loud thinking from my side :)
> 
> 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

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux