Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce new power management #6160

Merged
merged 26 commits into from
Jan 12, 2017

Conversation

kaspar030
Copy link
Contributor

@kaspar030 kaspar030 commented Nov 25, 2016

I finally got around to code some ideas we've been discussing in the past for improving our power management.

Reasons:

  • current LPM us completely unused, only idling is enabled, more sleep modes are hard commented out even if implemented.
  • current LPM is actually completely unusable as there's only one global "wakelock" for every power mode

This is intentionally kept simple in order to make it implementable for a wide range of platforms. IMHO It will never be possible to find an API that supports all the possible power saving mechanisms on all platforms (enabling/disabling/changing clock sources for everything, sleepwalking), but it is possible to save a lot of power compared what we currently have, and I also believe it is possible to come close enough to minimal power usage to be usable.

From doxygen:

This simple power management interface is based on the following assumptions:

- CPUs define up to 4 power modes (from zero, the lowest power mode, to
  PM_NUM_MODES-1, the highest)
- there is an implicit extra idle mode (which has the number PM_NUM_MODES)
- individual power modes can be blocked/unblocked, e.g., by peripherals
- if a mode is blocked, so are implicitly all lower modes
- the idle thread automatically selects and sets the lowest unblocked mode

The following functions need to be implemented for each platform:

pm_init()
pm_set()

I've selected SAM R21 as test board to try an actual implementation.

WARNING the test application currently sets STANDBY mode at some point, from which openocd cannot flash the board anymore. I have to use edbg to clean the flash before retrying.

@kaspar030 kaspar030 added Area: pm Area: (Low) power management Type: new feature The issue requests / The PR implemements a new feature for RIOT Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR labels Nov 25, 2016
@kaspar030
Copy link
Contributor Author

Open questions:

  • @immesys I'm very interested in your changes you've mentioned in cpu/samd21: implement low power modes #2309 to get the power consumption down!

  • how do we integrate turning off a board? Seems overkill to reserve wakelock memory even if that is usually just another "low power mode"

  • while the PM module from this PR is currently completely optional, should we just remove the old lpm infrastructure at the same time? It is not used anywhere for more than idling the CPU, anyways.

  • are four power modes (plus CPU idle, plus maybe OFF) enough?

@kaspar030
Copy link
Contributor Author

kaspar030 commented Nov 25, 2016

The test application just tries xtimer_spin() and xtimer_sleep() for 5 seconds each, unblocking power modes every 20 secs. As soon as mode 0 is unblocked, after the spinning completes, the node basically shuts down, as expected.

My measurement equipment is rather crude (cheap multimeter with uCurrent gold, samr21 gets power with the current measurement header piped through the ucurrent).

Power consumption currently is as follows:

Mode uA notes
spinning 19 LED on
idle (3) 14.8
2 13.4
1 12.2
0 ~10.5 fluctuates between 9.5 and 11.5, system stuck

@miri64
Copy link
Member

miri64 commented Nov 25, 2016

Not sure this is a level to low, but aometimes an action after wake-up is required (especially for network or MAC protocols). See e.g. 6Lo-ND's wakeup behavior. Are there plans to integrate the pending event management into this?

@kaspar030
Copy link
Contributor Author

Not sure this is a level to low, but aometimes an action after wake-up is required (especially for network or MAC protocols). See e.g. 6Lo-ND's wakeup behavior. Are there plans to integrate the pending event management into this?

Hmm, that would require every ISR to call a hook. What are the exact requirements? Just "whenever a node wakes up from a low power mode, do something"? That seems very unspecified when thinking about the possible implementations of sleeping.

@miri64
Copy link
Member

miri64 commented Nov 25, 2016

Well my idea was to just send out some IPC message (or have some generic callback like in xtimer). From the general to the specific: The exact requirements for 6Lo-ND's wakeup behavior is: iff the next wakeup happens after the registration timeout: re-new registration now by sending an NS with an ARO out to the default router.

@miri64
Copy link
Member

miri64 commented Nov 25, 2016

(registration lifetime is in order of minutes, so this happens rather rarely per node)

@lebrush
Copy link
Member

lebrush commented Nov 25, 2016

A grain of sand. I attended to a presentation by TI during electronica and they presented their concept for LP Embedded OS which I found very interesting. The rough summary follows:

  • Consider a MCU specific LP-Manager
  • On the implementation of every peripheral they register the required elements for their functionality to the LP-Manager (i.e. I need DMA, Timer, ADC and be able to wake-up on interrupt).
  • When the use of the peripheral is done, they communicate it to the LP-Manager, so the constraint is no longer considered.
  • During the idle-thread the LP-Manager is polled and the lowest possible LP is selected.
  • On "wake-up" the timers are adjusted (if required) to compensate the delay.

Imagine a SPI is configured in master mode. When initialized we don't care, but when starting a communication, it will tell the LP-Manager, not to turn off the SPI clock and i.e. to keep the DMA and it's interrupts enabled. The LP-Manager will go to the lowest power mode possible within this constrains while the transfer is ongoing. When the SPI is done, the LP-Manager knows the constrains are no longer applicable and can send the MCU to an even lower LPM.

I'm not sure it's understandable. Let me know if I can improve the description or give more details :-)

@kaspar030
Copy link
Contributor Author

kaspar030 commented Nov 25, 2016 via email

@kaspar030
Copy link
Contributor Author

Well my idea was to just send out some IPC message (or have some generic callback like in xtimer).

I think this is unrelated to actual power management. We could, before going to sleep, disable thread_yield() somehow, then fire the wakeup hooks (send messages, set thread flags, unlock mutexes, whatever). That would put the threads in pending mode, but not trigger the actual context switch. On wakeup the threads would get scheduled right after the ISR's are handled.

@lebrush
Copy link
Member

lebrush commented Nov 25, 2016

@kaspar030 good point, it makes it way easier to implement :-)

I understand that the plan is to have several LPM which are MCU dependent and not anymore RIOT generic, right? Two comments with the current approach:

  • each peripheral needs to know what peripherals are on/off in each power mode
  • if I only need an interrupt in the GPIO port A (i.e. stm32) this would keep the clock running in the other ports as well. In case of the stm32l1, disabling the clock and setting the pins floating of each port has a huge impact in the power consumption.

In any case I think is a really good approach and I think is the right way to go from the current standing point. Good job! :-) Maybe once this first step is implemented, we can evaluate how much work is (and if it is worth) to go for a individual peripheral selection (TI approach), which at the moment is far too ambitious.

@kaspar030
Copy link
Contributor Author

kaspar030 commented Nov 25, 2016 via email

@immesys
Copy link
Contributor

immesys commented Nov 25, 2016

I have extensive experience with getting the power consumption low on the SAMR21 and the SAM4L. Both with TinyOS and with RIOT. My previous platform had an idle current of 2.3uA on SAM4L and my present one has 2.6uA on SAMR21 (where idle is defined as in xtimer_usleep for example). As it turns out the experience is a little different between them. In the SAMR21/SAMD21 Atmel put a lot of effort into things like ONDEMAND bits for clocks. To go into low power mode you mostly just go into the lowest power mode every time and the peripherals themselves will take care of keeping the correct clocks active. For the SAM4L it was a royal pain to make sure peripherals had the correct clocks active and you entered the right depth of sleep. When you woke up, every ISR that might be the wakeup source had to call into a hook that would perform the wake-up-dance of re-enabling the correct clocks.

I will caution very strongly against the approach of "poll a bunch of things to determine the lowest sleep state in the idle thread". TinyOS did that because on msp430 (and other earlier platforms) the amount of state that required polling was small. On modern platforms, the number of peripherals and the complexity of the state they may be in (different clocks depending on how the peripherals are configured) makes it far more difficult to poll the state and you really don't want to spend more than a handful of cycles working out how to go to sleep because it wastes a lot of energy.

I have more than 100 motes deployed running RIOT. They have an average current of about 22uA sampling and sending every 10-30s. They ought to last about 5-10 years on the batteries they have. You can see the changes I had to make to RIOT to get that working here:

hamilton-mote/RIOT-OS@b333b9b...hamilton-mote:hamilton-lp-patches

The TLDR is:

  • use OSCULP32K as it runs always during sleep anyway (EDIT: it feeds the FLL at 48Mhz while awake)
  • use the RTT to back xtimer, not a high-power-timer
  • change the GPIO EIC clock to OSCULP32K not GCLK0 as that doesn't run during sleep so you won't wake up
  • Change the app to put the radio to sleep after sending (we really need decent MACs in RIOT...)

EDIT: I needed to merge PR 5608 to get low power too (so xtimer runs from osculp32)

@immesys
Copy link
Contributor

immesys commented Nov 25, 2016

@kaspar030 I can send you a SAMR21 board that is better suited for power measurement if you want, or revive my remote-measurement jig. The SAMR21XPRO has a lot of other stuff of in that ruins measurements of just the MCU.

@haukepetersen
Copy link
Contributor

Nice, I very much like the new/reworked concept!

how do we integrate turning off a board?

How about we introduce a explicit function for this? While at it, how about also including the reboot function:

void pm_reboot(void);
void pm_halt(void);

while the PM module from this PR is currently completely optional, should we just remove the old lpm infrastructure at the same time? It is not used anywhere for more than idling the CPU, anyways.

Yes, I am 100% for dropping the old lpm stuff right away -> otherwise it will take years again until stuff converges.

are four power modes (plus CPU idle, plus maybe OFF) enough?

Could well be, but I would tend to go into a slightly different direction: from what we have learned and know, the low power stuff is highly dependent on the CPU. So I think it is best to make the CPU define the actual structure of the pm_blocker_t struct. Also I tend to think that it makes sense, have less (or no?) code except the interface in core and let the CPUs care about everything. So how about such an interface:

void pm_block(unsigned mode);
void pm_unblock(unsigned mode);
void pm_set_lowest(void);
void pm_halt(void);
void pm_reboot(void);

The actual definition and structure of the blocker union could be completely internal to the actual CPU specific implementation, allowing for any number of power states. What do you think about that?

@LudwigKnuepfer
Copy link
Member

I think I like the concept in general.

However I think there needs to be some generic concept that enables things like xtimer (I can't think of anything else right now, but there might be others) to communicate the time until the next wakeup. The reason is that there generally is some overhead associated with switching power modes and clock frequencies, so the power manager needs to know if it is "reasonable" to switch at all.

@immesys
Copy link
Contributor

immesys commented Dec 5, 2016

@LudwigKnuepfer My 2c, 99% of the time it is fine to have a policy of "immediately try to go to the lowest power mode allowed when you hit the idle thread". I wouldn't worry about the overhead of going to sleep when you are just about to wake up. It is a rare occurrence and you are likely to spend more energy on the cycles checking for that condition than you are going to save.

@LudwigKnuepfer
Copy link
Member

There is not only energy consumption but also timeliness involved.

@haukepetersen
Copy link
Contributor

@kaspar030: had any time to update the PR?

@LudwigKnuepfer: timeliness is indeed something that needs to be clearly handled. For the (current version) of the 'xtimer' it is quite simple: the xtimer uses low-level drivers, which block the system from going to deep-sleep anyway, so not timing problems here :-). But as this PR changes the paradigm from 'pm tells x y z to shut down because we want to sleep` to 'x y z tell the PM how deep it can go to sleep', the modules depending on some (CPU specific) delays (or not to have them), one should be able to configure the system in a way, so that all timing demands can be met. But this behavior needs to be actually verified in practice...

@haukepetersen
Copy link
Contributor

any news on 'periphying' this?

*
* @param[in] mode power mode to block
*/
static inline void pm_block(unsigned mode)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just played with this -> IMHO it makes sense not to inline the (un)block functions to save some code size, and as they are usually not called in a time critical path?!

@kaspar030 kaspar030 force-pushed the introduce_new_power_management branch 2 times, most recently from 7d4257f to 6178094 Compare January 9, 2017 22:38
@kaspar030 kaspar030 force-pushed the introduce_new_power_management branch from c865f41 to 51a86f7 Compare January 12, 2017 15:26
@kaspar030
Copy link
Contributor Author

@kaspar030: something with the FEATURE_PROVIDED var seems to go wrong, Murdock tried to build applications for boards that don't have specific features...

Shitto, we need to redo that logic. ;)

Anyhow, I squashed in a change that fixes it locally. I had moved the features processing up before the include of Makefile.buildtest, that seems to break it.

@lebrush
Copy link
Member

lebrush commented Jan 12, 2017

Might it be related to the issue in Murdock with #5573? (it's compiling for platforms which have no i2c even if it's required)

@kaspar030
Copy link
Contributor Author

All green! @haukepetersen would you do me the honour of pressing the button? At least half of the concept has been devised by you!

@haukepetersen
Copy link
Contributor

Oh yeah -> let's power down now :-)

@haukepetersen haukepetersen merged commit 6270283 into RIOT-OS:master Jan 12, 2017

void __attribute__((weak)) pm_set_lowest(void) {}

void __attribute__((weak)) pm_off(void)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this already got merged, but doc states pm_off() must be provided by every platform. So why is this here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be taken care of in a follow up PR...

This was referenced Jan 13, 2017
@OlegHahm
Copy link
Member

I don't have any concrete objections against the content of this PR, but I think that this kind of crucial feature should have required more than one ACK.

@immesys
Copy link
Contributor

immesys commented Feb 22, 2017

@kaspar030 sorry for being dense, but where should I implement the pm_set_lowest for my platform? Is this something that goes in boards/X/? My platform is SAMR21 based so would I put it in the cpu/samd21/periph/pm.c? Is there documentation somewhere I am missing?

EDIT: actually I'm more confused than I thought I was. Is there a brief overview of how to integrate into the layered pm subsystem?
EDIT2: sorry your system was actually more elegant than it first appeared. I get it now and it's exactly what I would have wanted. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pm Area: (Low) power management CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants