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

pm: Make device pm optional per state - V2 #70623

Merged
merged 10 commits into from
Jun 4, 2024

Conversation

ceolin
Copy link
Member

@ceolin ceolin commented Mar 22, 2024

This is a proposal to make device power management (triggered when SoC is idle) completely optional.

  • Targets can select which power states should NOT trigger device power management
  • If there is no power state with this property enabled, the pm subsystem won't trigger device power management
  • Removed the need for 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

@ceolin
Copy link
Member Author

ceolin commented Mar 25, 2024

@JordanYates @erwango @nashif can you take a look on this please ?

@ceolin
Copy link
Member Author

ceolin commented Mar 28, 2024

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.

@JordanYates
Copy link
Collaborator

@JordanYates @erwango @nashif can you take a look on this please ?

I have no experience working with system PM, I don't feel I can provide any useful feedback.

@ceolin
Copy link
Member Author

ceolin commented Mar 29, 2024

@JordanYates @erwango @nashif can you take a look on this please ?

I have no experience working with system PM, I don't feel I can provide any useful feedback.

Sure thing. Thanks for replying it :)

erwango
erwango previously requested changes Apr 5, 2024
Copy link
Member

@erwango erwango left a 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).

subsys/pm/Kconfig Outdated Show resolved Hide resolved
subsys/pm/Kconfig Outdated Show resolved Hide resolved
subsys/pm/Kconfig Outdated Show resolved Hide resolved
subsys/pm/Kconfig Outdated Show resolved Hide resolved
subsys/pm/device_system_managed.c Outdated Show resolved Hide resolved
dts/bindings/power/zephyr,power-state.yaml Outdated Show resolved Hide resolved
include/zephyr/pm/state.h Outdated Show resolved Hide resolved
@erwango
Copy link
Member

erwango commented May 27, 2024

@ceolin Please rebase

Flavio Ceolin added 3 commits May 29, 2024 18:07
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]>
@ceolin
Copy link
Member Author

ceolin commented May 30, 2024

@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.
Copy link
Member

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.

Copy link
Member Author

@ceolin ceolin May 31, 2024

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 :/

doc/services/pm/device.rst Outdated Show resolved Hide resolved
doc/services/pm/device.rst Outdated Show resolved Hide resolved
Flavio Ceolin added 7 commits May 31, 2024 11:08
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]>
@ceolin
Copy link
Member Author

ceolin commented May 31, 2024

@erwango thanks a lot for reviewing it. Have updated it with the suggestions. Would you mind look it again ?

@nashif nashif merged commit 4ffefed into zephyrproject-rtos:main Jun 4, 2024
23 checks passed
#else

bool pm_resume_devices(void) { return true; }
void pm_suspend_devices(void) {}
Copy link
Collaborator

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Power Management area: RISCV RISCV Architecture (32-bit & 64-bit) area: Samples Samples platform: ITE ITE platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants