-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
intel_adsp: ace: clock gating in idle #64468
intel_adsp: ace: clock gating in idle #64468
Conversation
Why don't you have shallow sleep states, then in |
a066641
to
9a3ed36
Compare
If I add this as a sub-state for PM_STATE_RUNTIME_IDLE for example, PM_STATE_ACTIVE would become a state that we would probably end up in less than 5% of the time. I also think it wouldn't be very efficient. We gain the most when we switch the clock and wait for the next interrupt as long as possible. |
Update ww45: I will rewrite it using the changes made in #64760. After that ACE will have its own idle implementation, so I dont need to add anything to the kernel idle. |
b6517f8
to
8d300df
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.
Please take a look in individual comments.
@ceolin is the use for substates for PM_STATE_ACTIVE a blocker for you? |
I think that is the wrong way to do this, that is not the PM_STATE_ACTIVE semantic. In the end what we need is a way to set a CPU property to when it gets idle decides how to set its clock. Right ? |
b4e47b9
to
f27bbfa
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.
The first part looks ok to enabling gating based on Kconfig. The adsp-specific interface to get/put a lock, that I'm less sure about and wonder whether this could be done in a separate PR (unless this lock capability is a must-have and we cannot enable anything without it). Comments inline.
f27bbfa
to
6dda1f4
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.
nitpicks
This patch enables DSP clock gating for ACE platforms. By default, clock gating is blocked by the firmware in the hardware configuration. If CONFIG_ADSP_IDLE_CLOCK_GATING is enabled, this prevent is not active and clock can be gated when core is in idle state. WIth this option disabled clock gating will only be enabled in hardware during power gating. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
6dda1f4
to
0c584de
Compare
I abandoned the desire to add an API that allows you to change clock gating settings in the runtime. Currently, the option to permanently enable this in the hardware is sufficient. |
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 @tmleman , looks good! We can build on top of this to add runtime control later if/when needed (and have more time to discuss what is the proper interface to expose this).
This patch enables DSP clock gating for ACE platforms. By default, clock gating is blocked by the firmware in the hardware configuration. If CONFIG_ADSP_IDLE_CLOCK_GATING is enabled, this prevent is not active and clock can be gated when core is in idle state. WIth this option disabled clock gating will only be enabled in hardware during power gating.