soc: intel: intel_adsp: Add sys_poweroff() #90821
Conversation
github-actions
bot
requested review from
andyross,
ceolin,
dcpleung,
jxstelter,
kv2019i,
lgirdwood,
lyakh,
marcinszkudlinski and
softwarecki
| } | ||
| #endif /* CONFIG_ADSP_POWER_DOWN_HPSRAM */ | ||
| /* do power down - this function won't return */ | ||
| power_down_cavs(true, uncache_to_cache(&hpsram_mask[0])); |
There was a problem hiding this comment.
nitpick, but it did confuse me for a minute: there's of course an eternal fight between &array[n] and array + n with the resolution often trying to understand the meaning of what the code is doing. In this case I don't think it's the address of the zeroth element that has to be passed to the function, but an address of the array instead. If it were the former, you wouldn't need the array and would just use a single element. So in this case I think power_down_cavs(true, uncache_to_cache(hpsram_mask)); actually makes more sense
There was a problem hiding this comment.
@lyakh this is the existing code that I've moved into a helper function. So for the sake of consistency I'd like to retain the original code.
|
@ranj063 need to resolve all those compliance issues and please take a look at sonarqube issues as well. |
github-actions
bot
requested review from
bjarki-andreasen,
JordanYates and
teburd
There was a problem hiding this comment.
The current implementation of the D0/D3 transition flow with CONFIG_ADSP_IMR_CONTEXT_SAVE enabled is causing exceptions when the system exits D3 state because it returns to sys_poweroff, which is marked as FUNC_NORETURN. This prevents the core from resuming its normal execution flow.
| /* do power down - this function won't return */ | ||
| power_down(true, IS_ENABLED(CONFIG_ADSP_POWER_DOWN_HPSRAM), true); | ||
| } else { | ||
| if (cpu == 0) |
There was a problem hiding this comment.
curly braces, this is not Linux :)
subsys/pm/pm.c
Outdated
| @@ -137,6 +137,8 @@ bool pm_state_force(uint8_t cpu, const struct pm_state_info *info) | |||
|
|
|||
| key = k_spin_lock(&pm_forced_state_lock); | |||
| z_cpus_pm_forced_state[cpu] = info; | |||
| if (info->state == PM_STATE_SOFT_OFF && cpu == 0) | |||
Add the sys_poweroff() implementation for Intel ADSPs. This will be invoked to when the host sends the IPC to transition the DSP to D3. Currently, the IPC response for D3 takes ~100ms (measured the ACE15 based MTL laptop) because we perform the actions for D3 in the idle context. Invoking sys_poweroff() from the IPC context will speed up the transition and reduce the transition time to ~1ms resulting in a 100-fold reduction compared to the current implementation. Signed-off-by: Ranjani Sridharan <[email protected]>
This will be used for performing a context save before powering off the system and the saved context will be restored upon a system resume. Signed-off-by: Ranjani Sridharan <[email protected]>
| * | ||
| * @kconfig{CONFIG_HIBERNATE} needs to be enabled to use this API. | ||
| */ | ||
| void sys_hibernate(void); |
There was a problem hiding this comment.
This needs to be discussed at an arch meeting :) We already have sys poweroff, and we do need more options regarding specific sleep modes, like the many various "off" modes out there, deep sleep, hibernate, system off, which all have various meanings to various SoCs.
Implement system hibernate for platforms that support IMR context save/restore. Signed-off-by: Ranjani Sridharan <[email protected]>
|
|
Dropping this PR for now as dropping full context save/restore achieves the desired goal. |
Add the sys_poweroff() implementation for Intel ADSPs. This will be invoked to when the host sends the IPC to transition the DSP to D3. Currently, the IPC response for D3 takes ~100ms (measured the ACE15 based MTL laptop) because we perform the actions for D3 in the idle context. Invoking sys_poweroff() from the IPC context will speed up the transition and reduce the transition time to ~1ms resulting in a 100-fold reduction compared to the current implementation.