> -----Original Message-----
> From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
> Sent: Tuesday, March 10, 2009 8:45 PM
> To: Premi, Sanjeev
> Cc: Högander Jouni; linux-omap@vger.kernel.org
> Subject: Re: Problems in cpuidle
>
> "Premi, Sanjeev" <pr...@ti.com> writes:
>
> >> -----Original Message-----
> >> From: Högander Jouni [mailto:jouni.hogan...@nokia.com]
> >> Sent: Monday, March 09, 2009 3:36 PM
> >> To: Premi, Sanjeev
> >> Cc: linux-omap@vger.kernel.org
> >> Subject: Re: Problems in cpuidle
> >>
> >> "ext Premi, Sanjeev" <pr...@ti.com> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
