On Fri, 13 Jan 2012 03:05:03 -0700 (MST) Paul Walmsley <paul@xxxxxxxxx> wrote:
> cc Tero, Govindraj
>
> On Thu, 12 Jan 2012, NeilBrown wrote:
>
> > On Wed, 11 Jan 2012 06:43:04 -0700 (MST) Paul Walmsley <paul@xxxxxxxxx> wrote:
> >
> > I spent some time exploring why cpuidle never drops below state 0 and found
> > out that the code explicitly disables other states when uart is active - for
> > a fairly broad definition of 'active'.
> >
> > I found the "sleep_timeout" setting and set them all to 1 second. This meant
> > that cpuidle started working, but I got a lot of garbage characters.
> >
>
> ...
>
> > Does this really mean CPU_IDLE cannot be used when you might expect chars
> > from a serial port - e.g. GPS or Bluetooth?
>
> Hmmm. Looking at mach-omap2/pm34xx.c and mach-omap2/cpuidle34xx.c, it
> indeed prevents even the MPU from entering a low-power state when the UART
> is active, as you write. That doesn't seem correct.
>
> Please try the following patch. It works fine for me on v3.2 with
> omap2plus_defconfig on a 35xx BeagleBoard. No dropped or garbled
> characters with console use. Not too surprising, since the UART has a
> receive FIFO to withstand interrupt servicing delays.
Much nicer!!
I now see CPUIDLE using state1 and state2 as well as the normal state0.
I also see idle current drain drop from about 160mA to 95mA
And there are no garbled characters when I type.
Having CPUIDLE makes the DSS2 problem worse: lots of
[ 21.085113] omapdss DISPC error: SYNC_LOST on channel lcd, restarting the output with video overlays disabled
messages whenever the CPU isn't busy.
The only way to get rid of them that I have found is to blank the display:
# echo 1 > /sys/devices/platform/omapfb/graphics/fb0/blank
Also, the HDQ access to the battery 'fuel gauge' is working fine, so
presumably that gets disturbed by one of the lower power states (3,4,5).
I guess it is 4,5,6 when CORE goes to RET or OFF. So we need some way for
HDQ to say "I'm busy" just like UART can. omap_hdq_can_sleep() ???
There must be a cleaner way...
More experiments:
- I enabled off_mode and started seeing state3 happening. UART and HDQ
still fine - as expected.
- I set the /sys/devices/system/cpu/cpu0/cpuidle/state?/usage values
all to '10'.
Now things start to break;
Console is mostly OK, but the first char after 10 seconds of idle is bad.
I type 'enter' and get 'C'. In general the first char typed is mapped to
something else in a consistent way:
0a -> c3
20 -> 90
55 -> d5
2a -> 95
It is a bit like we are missing a start bit, and a stop bit is shifting
down into the msb .. sometimes.
HDQ also breaks.
That surprised me a bit as the HDQ access is immediately after typing so
it should be in the 10-second timeout when the UART keeps CORE powered on.
Maybe we need runtime_pm to run omap_hdq_break() again to reinitialise
the HDQ port if CORE might have been turned off.
Also saw states all the way down to 6. Cannot tell if this helps current
drain as I need HDQ working for that.
So this is real progress, but there is still room for improvement. I might
try writing an omap_hdq_can_sleep() and use it like omap_uart_can_sleep().
And get runtime_pm working on HDQ so it turns off after a short delay, and
re-initialises if we have to turn it back on again.
Thanks for the patch, it's been:
Tested-by: NeilBrown <neilb@xxxxxxx>
Thanks,
NeilBrown
>
>
> - Paul
>
> From: Paul Walmsley <paul@xxxxxxxxx>
> Date: Fri, 13 Jan 2012 02:10:30 -0700
> Subject: [PATCH] ARM: OMAP3: PM: allow MPU to enter low-power states even
> when the UART is active
>
> For some reason, both the existing OMAP3 PM code and the OMAP3 CPUIdle
> driver prevent the MPU powerdomain from entering low-power modes when
> any UART isn't asleep. Possibly it is intended to minimize the ARM
> wakeup latency when UART activity arrives, but the UART has a FIFO
> that should handle this for most cases, with no dropped characters. I
> may be forgetting something important, though. And CORE/PER low-power
> states are a different matter entirely.
>
> Thanks to NeilBrown <neilb@xxxxxxx> for reporting the problem.
>
> Signed-off-by: Paul Walmsley <paul@xxxxxxxxx>
> Cc: NeilBrown <neilb@xxxxxxx>
> Cc: Joe Woodward <jw@xxxxxxxxxxxxxx>
> Cc: Tero Kristo <t-kristo@xxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxx>
> Cc: Govindraj.R <govindraj.raja@xxxxxx>
> ---
> arch/arm/mach-omap2/cpuidle34xx.c | 8 +++-----
> arch/arm/mach-omap2/pm.h | 1 -
> arch/arm/mach-omap2/pm34xx.c | 10 ----------
> 3 files changed, 3 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 942bb4f..4ef32d4 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -183,6 +183,9 @@ static int next_valid_state(struct cpuidle_device *dev,
> core_deepest_state = PWRDM_POWER_OFF;
> }
>
> + if (!omap_uart_can_sleep())
> + core_deepest_state = PWRDM_POWER_ON;
> +
> /* Check if current state is valid */
> if ((cx->valid) &&
> (cx->mpu_state >= mpu_deepest_state) &&
> @@ -244,11 +247,6 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
> struct omap3_idle_statedata *cx;
> int ret;
>
> - if (!omap3_can_sleep()) {
> - new_state_idx = drv->safe_state_index;
> - goto select_state;
> - }
> -
> /*
> * Prevent idle completely if CAM is active.
> * CAM does not have wakeup capability in OMAP3.
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 4e166ad..eac6fce 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -18,7 +18,6 @@
> extern void *omap3_secure_ram_storage;
> extern void omap3_pm_off_mode_enable(int);
> extern void omap_sram_idle(void);
> -extern int omap3_can_sleep(void);
> extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
> extern int omap3_idle_init(void);
>
> diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
> index efa6649..1c49824 100644
> --- a/arch/arm/mach-omap2/pm34xx.c
> +++ b/arch/arm/mach-omap2/pm34xx.c
> @@ -484,21 +484,11 @@ console_still_active:
> clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
> }
>
> -int omap3_can_sleep(void)
> -{
> - if (!omap_uart_can_sleep())
> - return 0;
> - return 1;
> -}
> -
> static void omap3_pm_idle(void)
> {
> local_irq_disable();
> local_fiq_disable();
>
> - if (!omap3_can_sleep())
> - goto out;
> -
> if (omap_irq_pending() || need_resched())
> goto out;
>
Attachment:
signature.asc
Description: PGP signature
