[PATCH 0/2] ASoC: SOF: Intel: hda: Use cold/purge boot after firmware crash
Hi,
In case of a firmware crash we force the DSP to be powered down and rebooted. To make sure that the next boot is going to be clean, force the boot process to skip the IMR booting and re-download the firmware.
Regards, Peter --- Peter Ujfalusi (2): ASoC: SOF: Intel: hda: Introduce skip_imr_boot flag ASoC: SOF: Intel: hda: Skip IMR boot after a firmware crash or boot failure
sound/soc/sof/intel/hda-dsp.c | 12 ++++++++++++ sound/soc/sof/intel/hda-loader.c | 10 ++++++---- sound/soc/sof/intel/hda.h | 1 + 3 files changed, 19 insertions(+), 4 deletions(-)
Use a dedicated flag instead of directly checking the sdev->system_suspend_target to decide if we need to skip IMR boot due to too deep sleep state where the memory used for IMR booting will not retain its content.
The skip_imr_boot flag will be set true during suspend if the target state is deeper than S3 and reset back to false on successful boot to re-enable IMR booting in shallower sleep states.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 7 +++++++ sound/soc/sof/intel/hda-loader.c | 10 ++++++---- sound/soc/sof/intel/hda.h | 1 + 3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 2afaee91b982..eddfd77ad90f 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -617,6 +617,13 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend) #endif int ret, j;
+ /* + * The memory used for IMR boot loses its content in deeper than S3 state + * We must not try IMR boot on next power up (as it will fail). + */ + if (sdev->system_suspend_target > SOF_SUSPEND_S3) + hda->skip_imr_boot = true; + hda_sdw_int_enable(sdev, false);
/* disable IPC interrupts */ diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 819b3b08c655..eb22eb3f6fee 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -395,8 +395,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) struct snd_dma_buffer dmab; int ret, ret1, i;
- if (sdev->system_suspend_target < SOF_SUSPEND_S4 && - hda->imrboot_supported && !sdev->first_boot) { + if (hda->imrboot_supported && !sdev->first_boot && !hda->skip_imr_boot) { dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n"); hda->boot_iteration = 0; ret = hda_dsp_boot_imr(sdev); @@ -480,11 +479,14 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) */ hda->boot_iteration = HDA_FW_BOOT_ATTEMPTS; ret = hda_cl_copy_fw(sdev, hext_stream); - if (!ret) + if (!ret) { dev_dbg(sdev->dev, "Firmware download successful, booting...\n"); - else + hda->skip_imr_boot = false; + } else { snd_sof_dsp_dbg_dump(sdev, "Firmware download failed", SOF_DBG_DUMP_PCI | SOF_DBG_DUMP_MBOX); + hda->skip_imr_boot = true; + }
cleanup: /* diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index dc713c20ba1d..2b4d23af6054 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -419,6 +419,7 @@ enum sof_hda_D0_substate { /* represents DSP HDA controller frontend - i.e. host facing control */ struct sof_intel_hda_dev { bool imrboot_supported; + bool skip_imr_boot;
int boot_iteration;
To make sure that we start from a clean state next time when the DSP is powered up after a firmware crash or boot failure we must skip the IMR booting attempt.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index eddfd77ad90f..671c3e02d7df 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -620,8 +620,13 @@ static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend) /* * The memory used for IMR boot loses its content in deeper than S3 state * We must not try IMR boot on next power up (as it will fail). + * + * In case of firmware crash or boot failure set the skip_imr_boot to true + * as well in order to try to re-load the firmware to do a 'cold' boot. */ - if (sdev->system_suspend_target > SOF_SUSPEND_S3) + if (sdev->system_suspend_target > SOF_SUSPEND_S3 || + sdev->fw_state == SOF_FW_CRASHED || + sdev->fw_state == SOF_FW_BOOT_FAILED) hda->skip_imr_boot = true;
hda_sdw_int_enable(sdev, false);
On Tue, 12 Jul 2022 15:09:34 +0300, Peter Ujfalusi wrote:
In case of a firmware crash we force the DSP to be powered down and rebooted. To make sure that the next boot is going to be clean, force the boot process to skip the IMR booting and re-download the firmware.
Regards, Peter
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: SOF: Intel: hda: Introduce skip_imr_boot flag commit: 57724db17a946476f11c1b1be9750bc0cf877adc
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Peter Ujfalusi