rp2: Fix power consumption when sleeping with a timeout. #15398
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a regression introduced in 3af006e (part of #13329), where WFE never blocked in
mp_wfe_or_timeout()
function and would busy-wait instead. This increases power consumption measurably. Originally found when testing #15345Root cause is that
mp_wfe_or_timeout()
calls soft timer functions that (after the regression) callrecursive_mutex_enter()
andrecursive_mutex_exit()
. Mutex exit callslock_internal_spin_unlock_with_notify()
and the default pico-sdk implementation of this macro issues a SEV which negates the WFE that follows it, meaning the CPU never suspends.See https://forums.raspberrypi.com/viewtopic.php?p=2233908 for more details.
The fix in this comment adds a custom "nowait" variant mutex that doesn't do WFE/SEV, and uses this one for PendSV. This will use more power when there's contention for the PendSV mutex as the other core will spin, but this shouldn't happen very often.
Additional commit fixes a bug that was masked by this bug, specifically that core1 didn't have any way to resume from WFE (because the soft timer interrupt only triggers on core0).
Testing
Tested on PICO board while measuring USB power consumption. Prior to this fix,
time.sleep(1)
uses 3-4mA more power than sitting at an idle REPL. After this fix, the power consumption is approximately the same (actually approx 0.3mA higher while sleeping, I assume due to the extra clock or maybe because of USB traffic).Also ran the full rp2 test suite multiple times. The thread_sleep2 test successfully found the core1 bug mentioned above.
Also connected a PICO-W to a Wi-Fi network.
Trade-offs and Alternatives
This is the third fix I wrote for this issue(!)
mp_wfe_or_timeout()
with a dedicated hardware timer alarm. This works fine for single core, but in dual core there's potential for both cores to enter this function at the same time meaning this approach is no longer simple.This approach adds a bit more code size, but it means the low power mutex variant is still used everywhere except for PendSV - and there shouldn't be a lot of lock contention over PendSV.