[PATCH 0/2] ASoC: SOF: Intel: hda: Use cold/purge boot after firmware crash

[PATCH 0/2] ASoC: SOF: Intel: hda: Use cold/purge boot after firmware crash

[Peter Ujfalusi]

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(-)

-- 
2.37.0

[PATCH 1/2] ASoC: SOF: Intel: hda: Introduce skip_imr_boot flag

[Peter Ujfalusi]

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;
 
-- 
2.37.0

[PATCH 2/2] ASoC: SOF: Intel: hda: Skip IMR boot after a firmware crash or boot failure

[Peter Ujfalusi]

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);
-- 
2.37.0

Re: [PATCH 0/2] ASoC: SOF: Intel: hda: Use cold/purge boot after firmware crash

[Mark Brown]

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