-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
pm: Make device pm optional per state - V2 #70623
Conversation
@JordanYates @erwango @nashif can you take a look on this please ? |
folks, can I get some feedback here ? the original changes was already discussed in appropriated forum and there were no push-backs. This version is simpler and kept the current behavior but it adds more flexibility and organize device pm imho. If there is no major block I would like to have it sooner than later to avoid regressions close to the release. |
I have no experience working with system PM, I don't feel I can provide any useful feedback. |
Sure thing. Thanks for replying it :) |
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 rebase.
Some addiotinal comments:
You're mentioning #60369, but I don't see a direct link, can you elaborate on this point ?
It would be nice to have example of use case where it is interesting to make use of System Managed Device Power Management in some states and not on others.
(maybe both are linked, but I've not been able to connect the dots).
@ceolin Please rebase |
That is option has shown confusing on it is attempt to prevent system pm doing device power management. Lets address this problem properly. Signed-off-by: Flavio Ceolin <[email protected]>
Make it possible to disble device power management individually per power state. This allows targets tuning which states should (and which should not) trigger device power management. Signed-off-by: Flavio Ceolin <[email protected]>
Remove device pm path when there is no is no power state in DT with device pm enabled. This basically does the same thing that was done by PM_DEVICE_RUNTIME_EXCLUSIVE. Signed-off-by: Flavio Ceolin <[email protected]>
@erwango done. Can you take a look again please ? |
management subsystem. This allows device drivers to do any | ||
necessary power management operations like turning off | ||
device clocks and peripherals. Device drivers may also save | ||
and restore states in these hook functions. |
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.
Non blocking: We could use more detailed information on states
here.
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.
it is hard to be specific without the risk of saying something that can be incomplete or wrong for a particular device :/
PM_DEVICE is not attached to system managed device power management. It is a very common use case targets with device runtime power management that don't want system device power management enabled. We introduce a new symbol (PM_DEVICE_SYSTEM_MANAGED) to explicit control whether or not system device power management should be globally enabled. Signed-off-by: Flavio Ceolin <[email protected]>
Define the power state needed in the test in DT. Signed-off-by: Flavio Ceolin <[email protected]>
Define the power state needed in the test in DT. Signed-off-by: Flavio Ceolin <[email protected]>
Add new information about new property to enable/disable device power management per power state. Signed-off-by: Flavio Ceolin <[email protected]>
Add a new test that disables PM_DEVICE_SYSTEM_MANAGED, since this option is enabled by default. Signed-off-by: Flavio Ceolin <[email protected]>
Replace the usage of CONFIG_PM_DEVICE_RUNTIME_EXCLUSIVE with CONFIG_PM_DEVICE_SYSTEM_MANAGED. Signed-off-by: Flavio Ceolin <[email protected]>
Check that a power state that has system-managed device power management disabled does not trigger device power management when the system sleeps. Signed-off-by: Flavio Ceolin <[email protected]>
@erwango thanks a lot for reviewing it. Have updated it with the suggestions. Would you mind look it again ? |
#else | ||
|
||
bool pm_resume_devices(void) { return true; } | ||
void pm_suspend_devices(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.
@ceolin, is this a bug? Why are the function definitions different in the #if and #else cases?
This is a proposal to make device power management (triggered when SoC is idle) completely optional.
CONFIG_PM_DEVICE_RUNTIME_EXCLUSIVE
Notes:
This is an alternative version to #60939. This version is less intrusive and keep the current behavior. When device runtime pm is enabled, system managed device power management is disabled (can be enabled though).
In this version device pm is triggered by default and can have it disabled individually (instead of by default does not trigger device power management)
Additional context:
#60463
#60369
#60939