-
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
drivers: intel_adsp: IPC driver uses D3 power state #56168
drivers: intel_adsp: IPC driver uses D3 power state #56168
Conversation
ce205b3
to
80c040c
Compare
82e5150
to
afa98c5
Compare
mem_win_init(DEVICE_DT_GET(MEM_WINDOW_NODE(0))); | ||
mem_win_init(DEVICE_DT_GET(MEM_WINDOW_NODE(1))); | ||
mem_win_init(DEVICE_DT_GET(MEM_WINDOW_NODE(2))); | ||
mem_win_init(DEVICE_DT_GET(MEM_WINDOW_NODE(3))); |
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.
0-1-2-3 From where come that numbers?
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.
From Devicetree instance API.
I've checked all .dtsi
files from dts\xtensa\intel\ directory
.
All boards have 4 memory_window instances.
soc/xtensa/intel_adsp/common/ipc.c
Outdated
{ | ||
if (k_is_in_isr()) { | ||
return -ESRCH; | ||
} |
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.
Do you really need this ??
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.
Well, you started a subject on TOCTOU race and did not answer my question... Please read my comments above.
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.
Nope, it does not solve it. If you have two threads that call pm_device_action_run()
is possible that both threads pass the check pm_device_state_is_locked()
and both ends up calling ipc_pm_action
and in that case you don't know exactly what will happen. That is a problem that device_runtime solves, pm_device_state_lock() was not thought to be used this way (though there is nothing disallowing it). It was simply not thought.
My suggestion is to remove it for now (since is not really protecting) and ensure that you are calling it from a safe context. I will probably change pm_device_lock()
to return a boolean and fails when the device state is already locked and you can use here it and that will probably be a solution.
Does it work for you ?
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.
Look inline comments please.
4a7818d
to
0e98166
Compare
0e98166
to
1b4d5aa
Compare
@aborisovich please fix the return error code information that I commented and if you agree just drop that check if it is from ISR (though I won't block this commit if you decided to keep that). Once you fix that error code mismatch it is good to me. |
1b4d5aa
to
3f61533
Compare
Push update - addressed @ceolin issues found:
|
@aborisovich SOF CI looks good thesofproject/sof#7325. How did our internal full scope tests go? |
Generic header for system clock allows to define a sys_clock_idle_exit function for the clock implementation. Implemented the function in the intel_adsp_timer to reinitialize device driver after the idle exit state. Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
Exiting idle state requires to reinitialize all memory window instances to flush the cached memory. Added function that calls initialization of devices during runtime. Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
When option CONFIG_PM_DEVICE is enabled, IPC driver is capable of entering D3 power state described as PM_DEVICE_ACTION_SUSPEND (and leaving that state - powering back to PM_DEVICE_ACTION_ACTIVE). New power control callbacks 'ipc_power_control_api' are introduced for use during power state transisions. They allow Zephyr application specific code to be executed. Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
When CONFIG_PM_DEVICE is enabled IPC Device may be during power transition during a call to intel_adsp_ipc_send_message function. Changed signatures of intel_adsp_ipc_send_message and its sync version to return int and negative error codes on error. On attempt to send IPC message during Device power transition -ESHUTDOWN error code is returned, on busy state -EBUSY. Updated all function references. Signed-off-by: Andrey Borisovich <andrey.borisovich@intel.com>
3f61533
to
2bf3358
Compare
Rebased to newest Zephyr main.
We have improvement of ~1.5% passrate. |
Last run after Zephyr main rebase gave us ~3% passrate improvement. |
This PR enables suspend/resume of the Intel ADSP IPC driver using Zephyr Power Manager Device API.
Options are enabled when CONFIG_PM_DEVICE=y.
New power control API performs hardware reinitialization after wake from D3 power state (PM_DEVICE_ACTION_SUSPEND):
Consists of 4 commits (2 main commits and two dependent):
Blocked by bug:
Signed-off-by: Andrey Borisovich andrey.borisovich@intel.com