Skip to content

ace: power policy management #6489

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.

Sign up for GitHub

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

Jump to bottom
Merged
kv2019i merged 5 commits into from
Nov 4, 2022
Merged

ace: power policy management #6489

kv2019i merged 5 commits into from
Nov 4, 2022

Conversation

@tmleman
Copy link
Contributor

@tmleman tmleman commented Oct 26, 2022

List of changes:

  • custom power policy: allows to override default power policy with application specific version,
  • default prevent for all lower power states,
  • add handlers to set/unset prevent on d0i3,
  • enabling zephyr power management function for MTL.

@tmleman tmleman mentioned this pull request Oct 26, 2022
Merged
@tmleman tmleman force-pushed the topic/upstream/zephyr/pm/pm_policy branch from 41f81ec to eb80502 Compare October 26, 2022 14:19
kv2019i
kv2019i reviewed Oct 27, 2022
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Looks good to go. One question @tmleman is what is the build error patch and why it's in this series? The build is fine in current main, and I don't see other changes in cpu.c in this PR. So what is failing?

lyakh
lyakh reviewed Oct 28, 2022
View reviewed changes
src/platform/intel/ace/lib/pm_runtime.c Outdated
Copy link
Collaborator

@lyakh lyakh Oct 28, 2022

Choose a reason for hiding this comment

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

I'm not familiar with Zephyr PM API, are we safe here from put() or get() being unbalanced? I.e. can this function be called multiple times with the same value of enable?

Copy link
Contributor Author

@tmleman tmleman Oct 31, 2022

Choose a reason for hiding this comment

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

Functions can be called multiple times but to balance the count of it is on the application side.

@tmleman tmleman force-pushed the topic/upstream/zephyr/pm/pm_policy branch from eb80502 to 0888514 Compare November 1, 2022 00:05
@tmleman tmleman requested review from kv2019i and lyakh November 1, 2022 00:20
lgirdwood
lgirdwood approved these changes Nov 1, 2022
View reviewed changes
Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@tmleman would be nice to have some more inline comments around the state logic i.e. explaining the policy and how we change state but that can be added later. Lets get this integrated and tested first.

abonislawski
abonislawski approved these changes Nov 2, 2022
View reviewed changes
@tmleman tmleman force-pushed the topic/upstream/zephyr/pm/pm_policy branch from 0888514 to 0d208e7 Compare November 2, 2022 09:01
@tmleman tmleman added MTL Applies to Meteor Lake platform MTL-UPSTREAM Upstream local changes for MTL platform labels Nov 2, 2022
@aborisovich aborisovich mentioned this pull request Nov 2, 2022
Merged
9 tasks
@lgirdwood
Copy link
Member

lgirdwood commented Nov 2, 2022

CI looks stuck. Retry.

@lgirdwood
Copy link
Member

lgirdwood commented Nov 2, 2022

SOFCI TEST

@tmleman tmleman force-pushed the topic/upstream/zephyr/pm/pm_policy branch from 0d208e7 to 87f7a4f Compare November 2, 2022 20:32
@lgirdwood
Copy link
Member

lgirdwood commented Nov 2, 2022

@tmleman like we have a build error on MTL in CI for GCC config.

@tmleman
ace: power: add custom power policy
2a8c798 
This patch introduce custom power management policy for ace platforms.
Default version depends mainly on system tick. Definition on the
application side will allow later for better customization.

When CONFIG_PM_POLICY_CUSTOM is set, kernel expect that application will
provide custom policy implementation.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
ace: power: handling d0i3 prevent
ddfe1b8 
This patch adds functions that allow to set/unset prevent on dynamic
power gating.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
zephyr: power: preventing lower power states
e97ceec 
D3 (PM_STATE_SOFT_OFF) state can be only entered after IPC request from
host. That why this prevent should be permanent.

D0i3 (PM_STATE_RUNTIME_IDLE) can be entered if host allows it and under
special conditions. State is prevented by default. Host will remove this
prevent with dedicated IPC message.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
ace: cpu: compilation error fix
1d90bf8 
Adding missing header: rtos/wait.h

The error appears in meteorlake after enabling power management.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
mtl: power: enabling zephyr power manager
aac1777 
Enabling zephyr power management functionalities for meteorlake board.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman tmleman force-pushed the topic/upstream/zephyr/pm/pm_policy branch from 87f7a4f to aac1777 Compare November 2, 2022 22:50
@lgirdwood
Copy link
Member

lgirdwood commented Nov 3, 2022

@tmleman still seeing an error

/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c: In function 'up_down_mixer_new':
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c:423:39: error: passing argument 2 of 'init_up_down_mixer' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  423 |         ret = init_up_down_mixer(dev, config, spec);
      |                                       ^~~~~~
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c:340:77: note: expected 'struct comp_ipc_config *' but argument is of type 'const struct comp_ipc_config *'
  340 | static int init_up_down_mixer(struct comp_dev *dev, struct comp_ipc_config *config, void *spec)
      |                                                     ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c:423:47: error: passing argument 3 of 'init_up_down_mixer' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
  423 |         ret = init_up_down_mixer(dev, config, spec);
      |                                               ^~~~
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c:340:91: note: expected 'void *' but argument is of type 'const void *'
  340 | static int init_up_down_mixer(struct comp_dev *dev, struct comp_ipc_config *config, void *spec)
      |                                                                                     ~~~~~~^~~~

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 3, 2022 •
edited
Loading

@lgirdwood wrote:

@tmleman still seeing an error

/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c: In function 'up_down_mixer_new':
/zep_workspace/sof/src/audio/up_down_mixer/up_down_mixer.c:423:39: error: passing argument 2 of

That's caused by #6517 that was merged yesterday. My bad, should have caught this pre-merge. Let's address this in a separate PR.

UPDATE: fixed partly in #6529 , see comments about the hifi3/XCC dependency that remains

@kv2019i
Copy link
Collaborator

kv2019i commented Nov 4, 2022

Known failures in CI. I did a local test with Zephyr SDK toolchain and all passes cleanly now. Proceeding with merge.

@kv2019i kv2019i merged commit 7628908 into thesofproject:main Nov 4, 2022
@tmleman tmleman deleted the topic/upstream/zephyr/pm/pm_policy branch May 23, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lgirdwood lgirdwood lgirdwood approved these changes

@abonislawski abonislawski abonislawski approved these changes

@kv2019i kv2019i kv2019i approved these changes

@marcinszkudlinski marcinszkudlinski marcinszkudlinski approved these changes

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@lyakh lyakh Awaiting requested review from lyakh lyakh is a code owner

+1 more reviewer

@aborisovich aborisovich aborisovich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

MTL Applies to Meteor Lake platform MTL-UPSTREAM Upstream local changes for MTL platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@tmleman @lgirdwood @kv2019i @lyakh @aborisovich @abonislawski @marcinszkudlinski