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: review post sleep wake up application notification scheduling #32474

Closed
erwango opened this issue Feb 19, 2021 · 4 comments · Fixed by #32592
Closed

pm: review post sleep wake up application notification scheduling #32474

erwango opened this issue Feb 19, 2021 · 4 comments · Fixed by #32592
Assignees
Labels
area: Power Management Enhancement Changes/Updates/Additions to existing features

Comments

@erwango
Copy link
Member

erwango commented Feb 19, 2021

In sleep states exit treatment functions (pm_system_resume, pm_policy_mgr), we can see that application notification (pm_state_notify) is called before SoC level post wake up operations (pm_power_state_exit_post_ops).

This could lead to several issues as we don't know what treatment applications may do on such notification, but this should be noted that SoC is potentially not fully waken up (and potentially devices neither). So starting treamtents at this steps can lead to undefined behavior.

Maybe there were reasons to implement appplications notifications this way, but at least PM should provide the option to swap the operations order so that application is notified about sleep state exit only when SoC and devices are fully ready to treat application demands.

@erwango
Copy link
Member Author

erwango commented Feb 19, 2021

@ceolin, @pabigot, @nashif as discussed yesterday.

Regarding the pm_power_state_exit_post_ops vs devices resumption:
This is to be confirmed but it is likely that the first function called at wake up from sleep is pm_system_resume, which is calling pm_power_state_exit_post_ops (again after having notified the application first). Operation on devices will only occur after in resumption of pm_policy_mgr.
So I think that from this point of view (devices vs system) things seem in order.

Though, if it is confirmed that wake up from sleep always occurs via IRQ (pm_system_resume), maybe some clean up could be done in late stages of pm_policy_mgr. If this is not the case, I'll be curious to know how ..

@ceolin ceolin self-assigned this Feb 23, 2021
@ceolin ceolin linked a pull request Feb 23, 2021 that will close this issue
@ceolin
Copy link
Member

ceolin commented Feb 23, 2021

@erwango I think you are correct and notification should only be sent after SoC post ops, have submitted a pr changing it. Another thing that I was wondering about is, currently we send notification about entering in a power state before device pm and if a device fails we send another notification about exiting a power state without never really have changed. Thinking that we should send notification after device PM, just before ask SoC to go to a power state.

@ceolin
Copy link
Member

ceolin commented Mar 11, 2021

I end up removing the commit implementing it because I found a problem with that. Usually SoCs restore IRQ in the post operations callback and it can cause the idle thread be scheduled out and consequently not sending the notification about leaving a state in the right time. I currently don't have a better answer about how to fix this, if it is define that SoC should not restore IRQ and let it to the kernel or if we should use a work queue to send this notification.

@ceolin ceolin reopened this Mar 11, 2021
@galak galak added the Enhancement Changes/Updates/Additions to existing features label Mar 16, 2021
@ceolin
Copy link
Member

ceolin commented Apr 27, 2021

Fixed in 87c6311

@ceolin ceolin closed this as completed Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Power Management Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants