-
Notifications
You must be signed in to change notification settings - Fork 7.3k
pm: Optimize power state selection with early exit when no state is available #88149
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
Conversation
44c07a4
to
0ad194c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :)
When all states are locked or latency requirement cannot be met by any power state it is important to be able to quickly exit suspend procedure because that usually means that application requires high performance. Add function for detecting if any power state is available. Additionally, add function pm_policy_state_is_available for checking if given state is available which means that it is not locked and fulfills current latency requirement. Signed-off-by: Krzysztof Chruściński <[email protected]>
Update power state selection. Previously, it was iterating over states starting from the last one so the most common short sleep periods were taking the longest time to select. Order is now swapped so that short sleeps will get power state as quick as possible. Signed-off-by: Krzysztof Chruściński <[email protected]>
0ad194c
to
084b967
Compare
@bjarki-andreasen can you reapprove? I had done minor fixes. |
@ceolin @JordanYates can you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
subsys/pm/policy/policy_state_lock.c:76
- [nitpick] The error message 'Unbalanced state lock get/put' could include additional context (such as the affected state and substate) to aid debugging when an imbalance occurs.
__ASSERT(lock_cnt[i] > 0, "Unbalanced state lock get/put");
subsys/pm/policy/policy_default.c:38
- [nitpick] The early exit using break may result in returning a NULL state if no valid candidate has been selected before the break is hit; confirm that this behavior is intended for cases when the first state fails the min residency check.
if (ticks < min_residency_ticks) {
bool pm_policy_state_is_available(enum pm_state state, uint8_t substate_id); | ||
|
||
/** | ||
* @brief Check if any power state can be used. | ||
* | ||
* Function allows to quickly check if any power state is available and exit | ||
* suspend operation early. | ||
* | ||
* @retval true if any power state is active. | ||
* @retval false if all power states are unavailable. | ||
*/ | ||
bool pm_policy_state_any_active(void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add these to release notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes. I think the improvements for shallow power states and when they are locked is better than the little performance degradation for deeper power states. Really good imho !
if (!pm_policy_state_any_active()) { | ||
/* Return early if all states are unavailable. */ | ||
return false; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if breaks the functionality provided by the pm_state_force
function.
https://docs.zephyrproject.org/latest/doxygen/html/group__subsys__pm__sys.html#ga075be307983f4efdcc93252a31a4258a
/**
* @brief Force usage of given power state.
*
* This function overrides decision made by PM policy forcing
* usage of given power state upon next entry of the idle thread.
*
* @note This function can only run in thread context
*
* @param cpu CPU index.
* @param info Power state which should be used in the ongoing
* suspend operation.
*/
bool pm_state_force(uint8_t cpu, const struct pm_state_info *info);
My interpretation is that this function should result in overriding restrictions such as pm policy lock or max latency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened #89037 to fix that.
Issues found in power management:
Lack of early exit when there are no power states available
When low latency requirement is set (e.g. 0us) then
pm_suspend
does not perform faster as it still iterates over all power modes only to find that there is no power mode that fulfills the requirement. When all power modes are lockedpm_suspend
behavior is the same as when latency requirement is set. In both cases user sets those to get higher performance so ideally,pm_suspend
should be limited.Added
pm_policy_state_any_active
which is using bitmasks to determine if any state is available. It is used early in thepm_suspend
and allows early exit. Addedpm_policy_state_is_available
which checks if state is locked or latency requirement is not met.Default algorithm for picking power state iterates always from the last power state
Algorithm for picking power state iterates always from the last power state (the longest sleep) and because of that
pm_suspend
is the longest for the shortest (most common) sleep periods.Order is reverted.
Performance results
Timings were measured on nrf54h20 with 2 power states. Code includes other proposed improvements: #88144 and #88146. It measures the time between entering
pm_suspend
and selecting the power state or early exit. Results show significant improvement for case where no state is available and 33% improvement for shortest sleep.Results show that high performance case is significantly improved.
Short sleep times is slightly improved.