-
Notifications
You must be signed in to change notification settings - Fork 132
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
Query FW config to reload library #4680
Query FW config to reload library #4680
Conversation
4f3b4f1
to
896daba
Compare
896daba
to
7a998dd
Compare
4964196
to
f0e5f81
Compare
/* IMR booting will restore the libraries as well, skip the loading */ | ||
if (reload && hda->booted_from_imr) | ||
/* if IMR booting is enabled and fw context is saved for D3 state, skip the loading */ | ||
if (reload && hda->booted_from_imr && ipc4_data->fw_context_save) |
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.
it's a bit odd to have different structures to stored 'booted_from_imr' and 'fw_context_save'. Can we put all the information in the same place?
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.
@plbossart booted_from_imr is a intel feature which is set in sof/intel layer. fw_context_save is a common feature which is set in sof core layer. So I don't prefer to combine them in hda structure.
include/sound/sof/ipc4/header.h
Outdated
SOF_IPC4_BUS_HARDWARE_ID, | ||
SOF_IPC4_DEBUG_WINDOW_SIZE, | ||
SOF_IPC4_KPB_RT_SINK_COUNT, | ||
SOF_IPC4_DMI_FORCE_L1_EXIT, |
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 we need to expose all of the new definitions? Should we have a comment that says which ones are actually used by the driver? I don't know e.g. what the "BUS_HARDWARE_ID" might refer to.
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.
These config id are used by ref fw. I will change them to SOF_IPC4_RESERVED_xxx
Update fw_config_params in driver. Signed-off-by: Rander Wang <[email protected]>
Driver queries fw_context_save feature when fw is ready and can skip library reload with this feature since library is saved in persistent memory. The default value of fw_context_save is true unless fw reports false. Signed-off-by: Rander Wang <[email protected]>
If fw_context_save is defined by fw, driver can skip library reload on d3 exit or reload library. Signed-off-by: Rander Wang <[email protected]>
f0e5f81
to
7902f64
Compare
SOFCI TEST |
@plbossart @ujfalusi the fw side PR thesofproject/sof#8428 was merged, please help to review kernel side. Thanks! we are preparing release for customer. |
@@ -139,6 +139,8 @@ int sof_lnl_ops_init(struct snd_sof_dev *sdev) | |||
|
|||
ipc4_data->mtrace_type = SOF_IPC4_MTRACE_INTEL_CAVS_2; | |||
|
|||
ipc4_data->fw_context_save = true; |
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.
why do we need the default value to be true?
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.
It is for ref fw which supports it by default. Thanks @ujfalusi mentioned it for several times :)
We have 3 approvals and the test result is positive. @RanderWang has answered @plbossart's questions and the PR is urgent. I will go ahead to merge it to let CI run daily test with this PR. We can submit a follow up PR if there is any change needed. |
We got a final conclusion that driver should query FW config to reload library
fw pr: thesofproject/sof#8428