> -----Original Message----- > From: Kevin Hilman [mailto:khilman@xxxxxxxxxxxxxxxxxxx] > Sent: Tuesday, March 10, 2009 8:45 PM > To: Premi, Sanjeev > Cc: Högander Jouni; linux-omap@xxxxxxxxxxxxxxx > Subject: Re: Problems in cpuidle > > "Premi, Sanjeev" <premi@xxxxxx> writes: > > >> -----Original Message----- > >> From: Högander Jouni [mailto:jouni.hogander@xxxxxxxxx] > >> Sent: Monday, March 09, 2009 3:36 PM > >> To: Premi, Sanjeev > >> Cc: linux-omap@xxxxxxxxxxxxxxx > >> Subject: Re: Problems in cpuidle > >> > >> "ext Premi, Sanjeev" <premi@xxxxxx> writes: > >> > >> > While working with cpuidle, I have come across these problems. > >> > I am also working on the solutions, but would be good to > hear more > >> > thoughts. > >> > > >> > 1) The flag 'enable_dyn_sleep' is honoured only in > >> omap3_idle_bm_check() > >> > but in the C1 state, omap3_enter_idle() is invoked directly. > >> > So, the system can transition to deeper idle state(s) > >> > > >> > Same is the case with 'sleep_block'. > >> > > >> > Possible Solutions: > >> > a) Call omap3_can_sleep() in omap3_enter_idle(). > >> > This makes omap3_idle_bm_check() redundant; and > >> can be removed. > >> > > >> > b) Make single entry point for all idle states > >> > But would be an overkill for C1 state. > >> > > >> > c) Change omap3_can_sleep() to check for > omap_uart_can_sleep() > >> > and omap3_fclks_active() only. > >> > Move check for 'enable_dyn_sleep' and 'sleep_block' into > >> > omap3_enter_idle() > >> > > >> > I believe (c) would be the most optimal. > >> > >> Selecting (c) will break traditional pm_idle. Current plan > is to add > >> one more C state (C1) which would prevent mpu/core sleep > transitions. > >> Then remove fclk_active check completely. > > > > [sp] I meant doing the same for pm_idle as well. > > Also, the new cstate will not help with 'sleep' block. > > > > The proposed change look like: > > > > diff --git a/arch/arm/mach-omap2/cpuidle34xx.c > > b/arch/arm/mach-omap2/cpuidle34xx index 7fc3cb3..c25158c 100644 > > --- a/arch/arm/mach-omap2/cpuidle34xx.c > > +++ b/arch/arm/mach-omap2/cpuidle34xx.c > > @@ -82,6 +82,11 @@ static int omap3_enter_idle(struct > cpuidle_device *dev, > > /* Used to keep track of the total time in idle */ > > getnstimeofday(&ts_preidle); > > > > + if (!enable_dyn_sleep) > > + goto return_sleep_time; > > + if (atomic_read(&sleep_block) > 0) > > + goto return_sleep_time; > > + > > /* > > * Adjust the idle state (if required). > > * Also, ensure that usage statistics of correct > state are updated. > > diff --git a/arch/arm/mach-omap2/pm34xx.c > > b/arch/arm/mach-omap2/pm34xx.c index 0716d60..5c7819a 100644 > > --- a/arch/arm/mach-omap2/pm34xx.c > > +++ b/arch/arm/mach-omap2/pm34xx.c > > @@ -476,14 +476,10 @@ static int omap3_fclks_active(void) > > > > int omap3_can_sleep(void) > > { > > - if (!enable_dyn_sleep) > > - return 0; > > if (!omap_uart_can_sleep()) > > return 0; > > if (omap3_fclks_active()) > > return 0; > > - if (atomic_read(&sleep_block) > 0) > > - return 0; > > return 1; > > } > > > > @@ -534,6 +530,11 @@ err: > > > > static void omap3_pm_idle(void) > > { > > + if (!enable_dyn_sleep) > > + return; > > + if (atomic_read(&sleep_block) > 0) > > + return; > > + > > local_irq_disable(); > > local_fiq_disable(); > > > > Sanjeev, > > I think it's cleaner to just make all the states go through > the 'bm_check', so the standard PM idle and CPUidle use the > same paths. [sp] I was trying to avoid duplicate code. When all states go thru bm_check, omap3_enter_idle_bm() and omap3_enter_idle() can be collapsed into single function. Your thoughts? > Kevin > > -- 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
