[PATCH 0/7] ASoC: SOF: Intel: mtl/lnl: Improve firmware boot state handling

[PATCH 0/7] ASoC: SOF: Intel: mtl/lnl: Improve firmware boot state handling

[Peter Ujfalusi]

Hi,

this series improves the firmware/boot state handling which will allow
failed IMR boot recovery and human readable boot failure decoding.

Additionally a new debugfs file is added to force a purge/clean boot
of the DSP for developers.

Regards,
Peter
---
Peter Ujfalusi (7):
  ASoC: SOF: Intel: hda: Create debugfs file to force a clean DSP boot
  ASoC: SOF: Intel: mtl: Correct rom_status_reg
  ASoC: SOF: Intel: lnl: Correct rom_status_reg
  ASoC: SOF: Intel: mtl: Disable interrupts when firmware boot failed
  ASoC: SOF: Intel: mtl: Implement firmware boot state check
  ASoC: SOF: Intel: hda-dsp/mtl: Add support for ACE ROM state codes
  ASoC: SOF: Intel: mtl: Correct the mtl_dsp_dump output

 sound/soc/sof/intel/hda-loader.c |  7 ++-
 sound/soc/sof/intel/hda.c        | 73 +++++++++++++++++++++++++++++---
 sound/soc/sof/intel/hda.h        |  2 +
 sound/soc/sof/intel/lnl.c        | 10 ++++-
 sound/soc/sof/intel/lnl.h        | 15 +++++++
 sound/soc/sof/intel/mtl.c        | 63 +++++++++++++++++++--------
 sound/soc/sof/intel/mtl.h        | 48 ++++++++++++++++++++-
 7 files changed, 189 insertions(+), 29 deletions(-)
 create mode 100644 sound/soc/sof/intel/lnl.h

-- 
2.44.0

[PATCH 1/7] ASoC: SOF: Intel: hda: Create debugfs file to force a clean DSP boot

[Peter Ujfalusi]

When IMR boot is supported on a platform it is always going to be used to
boot the DSP unless some catastrophic event happens.
There is no way for a developer to force a clean DSP boot without removing
and re-inserting the modules.

Create a 'skip_imr_boot' debugfs file which can be used to force the
next DSP boot as clean (prune) boot.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 7 ++++++-
 sound/soc/sof/intel/hda.c        | 1 +
 sound/soc/sof/intel/lnl.c        | 7 ++++++-
 sound/soc/sof/intel/mtl.c        | 7 ++++++-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index b81f231abee3..d5b9209beb5a 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -15,6 +15,7 @@
  * Hardware interface for HDA DSP code loader
  */
 
+#include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
@@ -643,8 +644,12 @@ int hda_dsp_post_fw_run(struct snd_sof_dev *sdev)
 		/* Check if IMR boot is usable */
 		if (!sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT) &&
 		    (sdev->fw_ready.flags & SOF_IPC_INFO_D3_PERSISTENT ||
-		     sdev->pdata->ipc_type == SOF_IPC_TYPE_4))
+		     sdev->pdata->ipc_type == SOF_IPC_TYPE_4)) {
 			hdev->imrboot_supported = true;
+			debugfs_create_bool("skip_imr_boot",
+					    0644, sdev->debugfs_root,
+					    &hdev->skip_imr_boot);
+		}
 	}
 
 	hda_sdw_int_enable(sdev, true);
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index d38dc43c2f1c..33721e817ef4 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -19,6 +19,7 @@
 #include <sound/hda_register.h>
 
 #include <linux/acpi.h>
+#include <linux/debugfs.h>
 #include <linux/module.h>
 #include <linux/soundwire/sdw.h>
 #include <linux/soundwire/sdw_intel.h>
diff --git a/sound/soc/sof/intel/lnl.c b/sound/soc/sof/intel/lnl.c
index d1c73d407e68..5114411f1e36 100644
--- a/sound/soc/sof/intel/lnl.c
+++ b/sound/soc/sof/intel/lnl.c
@@ -6,6 +6,7 @@
  * Hardware interface for audio DSP on LunarLake.
  */
 
+#include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <sound/hda_register.h>
 #include <sound/sof/ipc4/header.h>
@@ -83,8 +84,12 @@ static int lnl_dsp_post_fw_run(struct snd_sof_dev *sdev)
 		struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 
 		/* Check if IMR boot is usable */
-		if (!sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT))
+		if (!sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT)) {
 			hda->imrboot_supported = true;
+			debugfs_create_bool("skip_imr_boot",
+					    0644, sdev->debugfs_root,
+					    &hda->skip_imr_boot);
+		}
 	}
 
 	return 0;
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index 060c34988e90..c640fbf6615a 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -9,6 +9,7 @@
  * Hardware interface for audio DSP on Meteorlake.
  */
 
+#include <linux/debugfs.h>
 #include <linux/firmware.h>
 #include <sound/sof/ipc4/header.h>
 #include <trace/events/sof_intel.h>
@@ -294,8 +295,12 @@ int mtl_dsp_post_fw_run(struct snd_sof_dev *sdev)
 		}
 
 		/* Check if IMR boot is usable */
-		if (!sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT))
+		if (!sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT)) {
 			hdev->imrboot_supported = true;
+			debugfs_create_bool("skip_imr_boot",
+					    0644, sdev->debugfs_root,
+					    &hdev->skip_imr_boot);
+		}
 	}
 
 	hda_sdw_int_enable(sdev, true);
-- 
2.44.0

[PATCH 2/7] ASoC: SOF: Intel: mtl: Correct rom_status_reg

[Peter Ujfalusi]

ACE1 architecture changed the place where the ROM updates the status code
from the shared SRAM window to HFFLGP1QW0 register for the status and
HFFLGP1QW0 + 4 for the error code.

The rom_status_reg is not used on MTL because it was wrongly assigned based
on older platform convention (SRAM window) and it was giving inconsistent
readings.

Fixes: 064520e8aeaa ("ASoC: SOF: Intel: Add support for MeteorLake (MTL)")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/mtl.c | 4 ++--
 sound/soc/sof/intel/mtl.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index c640fbf6615a..a27ce8debe91 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -732,7 +732,7 @@ const struct sof_intel_dsp_desc mtl_chip_info = {
 	.ipc_ack = MTL_DSP_REG_HFIPCXIDA,
 	.ipc_ack_mask = MTL_DSP_REG_HFIPCXIDA_DONE,
 	.ipc_ctl = MTL_DSP_REG_HFIPCXCTL,
-	.rom_status_reg = MTL_DSP_ROM_STS,
+	.rom_status_reg = MTL_DSP_REG_HFFLGPXQWY,
 	.rom_init_timeout	= 300,
 	.ssp_count = MTL_SSP_COUNT,
 	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
@@ -760,7 +760,7 @@ const struct sof_intel_dsp_desc arl_s_chip_info = {
 	.ipc_ack = MTL_DSP_REG_HFIPCXIDA,
 	.ipc_ack_mask = MTL_DSP_REG_HFIPCXIDA_DONE,
 	.ipc_ctl = MTL_DSP_REG_HFIPCXCTL,
-	.rom_status_reg = MTL_DSP_ROM_STS,
+	.rom_status_reg = MTL_DSP_REG_HFFLGPXQWY,
 	.rom_init_timeout	= 300,
 	.ssp_count = MTL_SSP_COUNT,
 	.ssp_base_offset = CNL_SSP_BASE_OFFSET,
diff --git a/sound/soc/sof/intel/mtl.h b/sound/soc/sof/intel/mtl.h
index ea8c1b83f712..3c56427a966b 100644
--- a/sound/soc/sof/intel/mtl.h
+++ b/sound/soc/sof/intel/mtl.h
@@ -70,8 +70,8 @@
 #define MTL_DSP_ROM_STS			MTL_SRAM_WINDOW_OFFSET(0) /* ROM status */
 #define MTL_DSP_ROM_ERROR		(MTL_SRAM_WINDOW_OFFSET(0) + 0x4) /* ROM error code */
 
-#define MTL_DSP_REG_HFFLGPXQWY		0x163200 /* ROM debug status */
-#define MTL_DSP_REG_HFFLGPXQWY_ERROR	0x163204 /* ROM debug error code */
+#define MTL_DSP_REG_HFFLGPXQWY		0x163200 /* DSP core0 status */
+#define MTL_DSP_REG_HFFLGPXQWY_ERROR	0x163204 /* DSP core0 error */
 #define MTL_DSP_REG_HfIMRIS1		0x162088
 #define MTL_DSP_REG_HfIMRIS1_IU_MASK	BIT(0)
 
-- 
2.44.0

[PATCH 3/7] ASoC: SOF: Intel: lnl: Correct rom_status_reg

[Peter Ujfalusi]

ACE2 architecture changed the place where the ROM updates the status code
from the shared SRAM window (and HFFLGP1QW0 in ACE1) to HFDSC register for
the status and HFDEC (HFDSC + 4) for the error code.

The rom_status_reg is not used on LNL because it was wrongly assigned based
on older platform convention (SRAM window) and it was giving inconsistent
readings.

Add new header file for lnl specific register definitions.

Fixes: 64a63d9914a5 ("ASoC: SOF: Intel: LNL: Add support for Lunarlake platform")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/lnl.c |  3 ++-
 sound/soc/sof/intel/lnl.h | 15 +++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)
 create mode 100644 sound/soc/sof/intel/lnl.h

diff --git a/sound/soc/sof/intel/lnl.c b/sound/soc/sof/intel/lnl.c
index 5114411f1e36..8e7193344341 100644
--- a/sound/soc/sof/intel/lnl.c
+++ b/sound/soc/sof/intel/lnl.c
@@ -17,6 +17,7 @@
 #include "hda-ipc.h"
 #include "../sof-audio.h"
 #include "mtl.h"
+#include "lnl.h"
 #include <sound/hda-mlink.h>
 
 /* LunarLake ops */
@@ -197,7 +198,7 @@ const struct sof_intel_dsp_desc lnl_chip_info = {
 	.ipc_ack = MTL_DSP_REG_HFIPCXIDA,
 	.ipc_ack_mask = MTL_DSP_REG_HFIPCXIDA_DONE,
 	.ipc_ctl = MTL_DSP_REG_HFIPCXCTL,
-	.rom_status_reg = MTL_DSP_ROM_STS,
+	.rom_status_reg = LNL_DSP_REG_HFDSC,
 	.rom_init_timeout = 300,
 	.ssp_count = MTL_SSP_COUNT,
 	.d0i3_offset = MTL_HDA_VS_D0I3C,
diff --git a/sound/soc/sof/intel/lnl.h b/sound/soc/sof/intel/lnl.h
new file mode 100644
index 000000000000..4f4734fe7e08
--- /dev/null
+++ b/sound/soc/sof/intel/lnl.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) */
+/*
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * Copyright(c) 2024 Intel Corporation. All rights reserved.
+ */
+
+#ifndef __SOF_INTEL_LNL_H
+#define __SOF_INTEL_LNL_H
+
+#define LNL_DSP_REG_HFDSC		0x160200 /* DSP core0 status */
+#define LNL_DSP_REG_HFDEC		0x160204 /* DSP core0 error */
+
+#endif /* __SOF_INTEL_LNL_H */
-- 
2.44.0

[PATCH 4/7] ASoC: SOF: Intel: mtl: Disable interrupts when firmware boot failed

[Peter Ujfalusi]

In case of error during the firmware boot we need to disable the interrupts
which were enabled as part of the boot sequence.

Fixes: 064520e8aeaa ("ASoC: SOF: Intel: Add support for MeteorLake (MTL)")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/mtl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index a27ce8debe91..3bc229957365 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -508,6 +508,7 @@ int mtl_dsp_cl_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_boot)
 	dump_msg = kasprintf(GFP_KERNEL, "Boot iteration failed: %d/%d",
 			     hda->boot_iteration, HDA_FW_BOOT_ATTEMPTS);
 	snd_sof_dsp_dbg_dump(sdev, dump_msg, flags);
+	mtl_enable_interrupts(sdev, false);
 	mtl_dsp_core_power_down(sdev, SOF_DSP_PRIMARY_CORE);
 
 	kfree(dump_msg);
-- 
2.44.0

[PATCH 5/7] ASoC: SOF: Intel: mtl: Implement firmware boot state check

[Peter Ujfalusi]

With the corrected rom_status_reg values we can now add a check for target
boot status for firmware booting.
With the check now we can identify failed firmware boots (IMR boots) and
we can use the fallback to purge boot the DSP.

Fixes: 064520e8aeaa ("ASoC: SOF: Intel: Add support for MeteorLake (MTL)")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/mtl.c | 37 ++++++++++++++++++++++++++++++++-----
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index 3bc229957365..5c9d14e5925d 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -444,7 +444,7 @@ int mtl_dsp_cl_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_boot)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	const struct sof_intel_dsp_desc *chip = hda->desc;
-	unsigned int status;
+	unsigned int status, target_status;
 	u32 ipc_hdr, flags;
 	char *dump_msg;
 	int ret;
@@ -490,13 +490,40 @@ int mtl_dsp_cl_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_boot)
 
 	mtl_enable_ipc_interrupts(sdev);
 
+	if (chip->rom_status_reg == MTL_DSP_ROM_STS) {
+		/*
+		 * Workaround: when the ROM status register is pointing to
+		 * the SRAM window (MTL_DSP_ROM_STS) the platform cannot catch
+		 * ROM_INIT_DONE because of a very short timing window.
+		 * Follow the recommendations and skip target state waiting.
+		 */
+		return 0;
+	}
+
 	/*
-	 * ACE workaround: don't wait for ROM INIT.
-	 * The platform cannot catch ROM_INIT_DONE because of a very short
-	 * timing window. Follow the recommendations and skip this part.
+	 * step 7:
+	 * - Cold/Full boot: wait for ROM init to proceed to download the firmware
+	 * - IMR boot: wait for ROM firmware entered (firmware booted up from IMR)
 	 */
+	if (imr_boot)
+		target_status = FSR_STATE_FW_ENTERED;
+	else
+		target_status = FSR_STATE_INIT_DONE;
 
-	return 0;
+	ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_BAR,
+					chip->rom_status_reg, status,
+					(FSR_TO_STATE_CODE(status) == target_status),
+					HDA_DSP_REG_POLL_INTERVAL_US,
+					chip->rom_init_timeout *
+					USEC_PER_MSEC);
+
+	if (!ret)
+		return 0;
+
+	if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS)
+		dev_err(sdev->dev,
+			"%s: timeout with rom_status_reg (%#x) read\n",
+			__func__, chip->rom_status_reg);
 
 err:
 	flags = SOF_DBG_DUMP_PCI | SOF_DBG_DUMP_MBOX | SOF_DBG_DUMP_OPTIONAL;
-- 
2.44.0

[PATCH 6/7] ASoC: SOF: Intel: hda-dsp/mtl: Add support for ACE ROM state codes

[Peter Ujfalusi]

The ROM state codes differ between CAVS and ACE architecture, there is a
slight overlap.
Add the ACE related state defines to mtl.h, introduce new table and
use it on case the function is called when running on ACE architecture.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/hda.c | 72 +++++++++++++++++++++++++++++++++++----
 sound/soc/sof/intel/hda.h |  2 ++
 sound/soc/sof/intel/mtl.h | 44 ++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 6 deletions(-)

diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 33721e817ef4..2fc10bec7331 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -35,6 +35,7 @@
 #include "../ipc4-topology.h"
 #include "hda.h"
 #include "telemetry.h"
+#include "mtl.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/sof_intel.h>
@@ -597,7 +598,7 @@ static const struct hda_dsp_msg_code hda_dsp_rom_fw_error_texts[] = {
 };
 
 #define FSR_ROM_STATE_ENTRY(state)	{FSR_STATE_ROM_##state, #state}
-static const struct hda_dsp_msg_code fsr_rom_state_names[] = {
+static const struct hda_dsp_msg_code cavs_fsr_rom_state_names[] = {
 	FSR_ROM_STATE_ENTRY(INIT),
 	FSR_ROM_STATE_ENTRY(INIT_DONE),
 	FSR_ROM_STATE_ENTRY(CSE_MANIFEST_LOADED),
@@ -620,6 +621,58 @@ static const struct hda_dsp_msg_code fsr_rom_state_names[] = {
 	FSR_ROM_STATE_ENTRY(CSE_IPC_DOWN),
 };
 
+static const struct hda_dsp_msg_code ace_fsr_rom_state_names[] = {
+	FSR_ROM_STATE_ENTRY(INIT),
+	FSR_ROM_STATE_ENTRY(INIT_DONE),
+	FSR_ROM_STATE_ENTRY(CSE_MANIFEST_LOADED),
+	FSR_ROM_STATE_ENTRY(FW_MANIFEST_LOADED),
+	FSR_ROM_STATE_ENTRY(FW_FW_LOADED),
+	FSR_ROM_STATE_ENTRY(FW_ENTERED),
+	FSR_ROM_STATE_ENTRY(VERIFY_FEATURE_MASK),
+	FSR_ROM_STATE_ENTRY(GET_LOAD_OFFSET),
+	FSR_ROM_STATE_ENTRY(RESET_VECTOR_DONE),
+	FSR_ROM_STATE_ENTRY(PURGE_BOOT),
+	FSR_ROM_STATE_ENTRY(RESTORE_BOOT),
+	FSR_ROM_STATE_ENTRY(FW_ENTRY_POINT),
+	FSR_ROM_STATE_ENTRY(VALIDATE_PUB_KEY),
+	FSR_ROM_STATE_ENTRY(POWER_DOWN_HPSRAM),
+	FSR_ROM_STATE_ENTRY(POWER_DOWN_ULPSRAM),
+	FSR_ROM_STATE_ENTRY(POWER_UP_ULPSRAM_STACK),
+	FSR_ROM_STATE_ENTRY(POWER_UP_HPSRAM_DMA),
+	FSR_ROM_STATE_ENTRY(BEFORE_EP_POINTER_READ),
+	FSR_ROM_STATE_ENTRY(VALIDATE_MANIFEST),
+	FSR_ROM_STATE_ENTRY(VALIDATE_FW_MODULE),
+	FSR_ROM_STATE_ENTRY(PROTECT_IMR_REGION),
+	FSR_ROM_STATE_ENTRY(PUSH_MODEL_ROUTINE),
+	FSR_ROM_STATE_ENTRY(PULL_MODEL_ROUTINE),
+	FSR_ROM_STATE_ENTRY(VALIDATE_PKG_DIR),
+	FSR_ROM_STATE_ENTRY(VALIDATE_CPD),
+	FSR_ROM_STATE_ENTRY(VALIDATE_CSS_MAN_HEADER),
+	FSR_ROM_STATE_ENTRY(VALIDATE_BLOB_SVN),
+	FSR_ROM_STATE_ENTRY(VERIFY_IFWI_PARTITION),
+	FSR_ROM_STATE_ENTRY(REMOVE_ACCESS_CONTROL),
+	FSR_ROM_STATE_ENTRY(AUTH_BYPASS),
+	FSR_ROM_STATE_ENTRY(AUTH_ENABLED),
+	FSR_ROM_STATE_ENTRY(INIT_DMA),
+	FSR_ROM_STATE_ENTRY(PURGE_FW_ENTRY),
+	FSR_ROM_STATE_ENTRY(PURGE_FW_END),
+	FSR_ROM_STATE_ENTRY(CLEAN_UP_BSS_DONE),
+	FSR_ROM_STATE_ENTRY(IMR_RESTORE_ENTRY),
+	FSR_ROM_STATE_ENTRY(IMR_RESTORE_END),
+	FSR_ROM_STATE_ENTRY(FW_MANIFEST_IN_DMA_BUFF),
+	FSR_ROM_STATE_ENTRY(LOAD_CSE_MAN_TO_IMR),
+	FSR_ROM_STATE_ENTRY(LOAD_FW_MAN_TO_IMR),
+	FSR_ROM_STATE_ENTRY(LOAD_FW_CODE_TO_IMR),
+	FSR_ROM_STATE_ENTRY(FW_LOADING_DONE),
+	FSR_ROM_STATE_ENTRY(FW_CODE_LOADED),
+	FSR_ROM_STATE_ENTRY(VERIFY_IMAGE_TYPE),
+	FSR_ROM_STATE_ENTRY(AUTH_API_INIT),
+	FSR_ROM_STATE_ENTRY(AUTH_API_PROC),
+	FSR_ROM_STATE_ENTRY(AUTH_API_FIRST_BUSY),
+	FSR_ROM_STATE_ENTRY(AUTH_API_FIRST_RESULT),
+	FSR_ROM_STATE_ENTRY(AUTH_API_CLEANUP),
+};
+
 #define FSR_BRINGUP_STATE_ENTRY(state)	{FSR_STATE_BRINGUP_##state, #state}
 static const struct hda_dsp_msg_code fsr_bringup_state_names[] = {
 	FSR_BRINGUP_STATE_ENTRY(INIT),
@@ -664,7 +717,7 @@ hda_dsp_get_state_text(u32 code, const struct hda_dsp_msg_code *msg_code,
 	return NULL;
 }
 
-static void hda_dsp_get_state(struct snd_sof_dev *sdev, const char *level)
+void hda_dsp_get_state(struct snd_sof_dev *sdev, const char *level)
 {
 	const struct sof_intel_dsp_desc *chip = get_chip_info(sdev->pdata);
 	const char *state_text, *error_text, *module_text;
@@ -680,12 +733,19 @@ static void hda_dsp_get_state(struct snd_sof_dev *sdev, const char *level)
 	else
 		module_text = fsr_module_names[module];
 
-	if (module == FSR_MOD_BRNGUP)
+	if (module == FSR_MOD_BRNGUP) {
 		state_text = hda_dsp_get_state_text(state, fsr_bringup_state_names,
 						    ARRAY_SIZE(fsr_bringup_state_names));
-	else
-		state_text = hda_dsp_get_state_text(state, fsr_rom_state_names,
-						    ARRAY_SIZE(fsr_rom_state_names));
+	} else {
+		if (chip->hw_ip_version < SOF_INTEL_ACE_1_0)
+			state_text = hda_dsp_get_state_text(state,
+							cavs_fsr_rom_state_names,
+							ARRAY_SIZE(cavs_fsr_rom_state_names));
+		else
+			state_text = hda_dsp_get_state_text(state,
+							ace_fsr_rom_state_names,
+							ARRAY_SIZE(ace_fsr_rom_state_names));
+	}
 
 	/* not for us, must be generic sof message */
 	if (!state_text) {
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index c939a24d770e..16140ae22c90 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -695,6 +695,8 @@ int hda_dsp_ipc_get_window_offset(struct snd_sof_dev *sdev, u32 id);
 irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context);
 int hda_dsp_ipc_cmd_done(struct snd_sof_dev *sdev, int dir);
 
+void hda_dsp_get_state(struct snd_sof_dev *sdev, const char *level);
+
 /*
  * DSP Code loader.
  */
diff --git a/sound/soc/sof/intel/mtl.h b/sound/soc/sof/intel/mtl.h
index 3c56427a966b..d2d709fb4f06 100644
--- a/sound/soc/sof/intel/mtl.h
+++ b/sound/soc/sof/intel/mtl.h
@@ -72,6 +72,50 @@
 
 #define MTL_DSP_REG_HFFLGPXQWY		0x163200 /* DSP core0 status */
 #define MTL_DSP_REG_HFFLGPXQWY_ERROR	0x163204 /* DSP core0 error */
+
+/* FSR status codes */
+#define FSR_STATE_ROM_RESET_VECTOR_DONE		0x8
+#define FSR_STATE_ROM_PURGE_BOOT		0x9
+#define FSR_STATE_ROM_RESTORE_BOOT		0xA
+#define FSR_STATE_ROM_FW_ENTRY_POINT		0xB
+#define FSR_STATE_ROM_VALIDATE_PUB_KEY		0xC
+#define FSR_STATE_ROM_POWER_DOWN_HPSRAM		0xD
+#define FSR_STATE_ROM_POWER_DOWN_ULPSRAM	0xE
+#define FSR_STATE_ROM_POWER_UP_ULPSRAM_STACK	0xF
+#define FSR_STATE_ROM_POWER_UP_HPSRAM_DMA	0x10
+#define FSR_STATE_ROM_BEFORE_EP_POINTER_READ	0x11
+#define FSR_STATE_ROM_VALIDATE_MANIFEST		0x12
+#define FSR_STATE_ROM_VALIDATE_FW_MODULE	0x13
+#define FSR_STATE_ROM_PROTECT_IMR_REGION	0x14
+#define FSR_STATE_ROM_PUSH_MODEL_ROUTINE	0x15
+#define FSR_STATE_ROM_PULL_MODEL_ROUTINE	0x16
+#define FSR_STATE_ROM_VALIDATE_PKG_DIR		0x17
+#define FSR_STATE_ROM_VALIDATE_CPD		0x18
+#define FSR_STATE_ROM_VALIDATE_CSS_MAN_HEADER	0x19
+#define FSR_STATE_ROM_VALIDATE_BLOB_SVN		0x1A
+#define FSR_STATE_ROM_VERIFY_IFWI_PARTITION	0x1B
+#define FSR_STATE_ROM_REMOVE_ACCESS_CONTROL	0x1C
+#define FSR_STATE_ROM_AUTH_BYPASS		0x1D
+#define FSR_STATE_ROM_AUTH_ENABLED		0x1E
+#define FSR_STATE_ROM_INIT_DMA			0x1F
+#define FSR_STATE_ROM_PURGE_FW_ENTRY		0x20
+#define FSR_STATE_ROM_PURGE_FW_END		0x21
+#define FSR_STATE_ROM_CLEAN_UP_BSS_DONE		0x22
+#define FSR_STATE_ROM_IMR_RESTORE_ENTRY		0x23
+#define FSR_STATE_ROM_IMR_RESTORE_END		0x24
+#define FSR_STATE_ROM_FW_MANIFEST_IN_DMA_BUFF	0x25
+#define FSR_STATE_ROM_LOAD_CSE_MAN_TO_IMR	0x26
+#define FSR_STATE_ROM_LOAD_FW_MAN_TO_IMR	0x27
+#define FSR_STATE_ROM_LOAD_FW_CODE_TO_IMR	0x28
+#define FSR_STATE_ROM_FW_LOADING_DONE		0x29
+#define FSR_STATE_ROM_FW_CODE_LOADED		0x2A
+#define FSR_STATE_ROM_VERIFY_IMAGE_TYPE		0x2B
+#define FSR_STATE_ROM_AUTH_API_INIT		0x2C
+#define FSR_STATE_ROM_AUTH_API_PROC		0x2D
+#define FSR_STATE_ROM_AUTH_API_FIRST_BUSY	0x2E
+#define FSR_STATE_ROM_AUTH_API_FIRST_RESULT	0x2F
+#define FSR_STATE_ROM_AUTH_API_CLEANUP		0x30
+
 #define MTL_DSP_REG_HfIMRIS1		0x162088
 #define MTL_DSP_REG_HfIMRIS1_IU_MASK	BIT(0)
 
-- 
2.44.0

[PATCH 7/7] ASoC: SOF: Intel: mtl: Correct the mtl_dsp_dump output

[Peter Ujfalusi]

The ROM/firmware state handling has changed between CAVS and ACE
architecture:
CAVS: ROM and firmware uses the SRAM window for the state and status/error
code reporting

ACE: ROM code is using two registers to report the state and error while
the firmware is using the SRAM window to report states and status/error
codes.

Use the generic hda_dsp_get_state() to decode ROM state and error codes and
print out the firmware state and status/error code only if the SRAM
window is accessible - the firmware is booted and the Status readout is
not 0xffffffff.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/intel/mtl.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index 5c9d14e5925d..dc203505536c 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -310,22 +310,16 @@ int mtl_dsp_post_fw_run(struct snd_sof_dev *sdev)
 void mtl_dsp_dump(struct snd_sof_dev *sdev, u32 flags)
 {
 	char *level = (flags & SOF_DBG_DUMP_OPTIONAL) ? KERN_DEBUG : KERN_ERR;
-	u32 romdbgsts;
-	u32 romdbgerr;
 	u32 fwsts;
 	u32 fwlec;
 
+	hda_dsp_get_state(sdev, level);
 	fwsts = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_DSP_ROM_STS);
 	fwlec = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_DSP_ROM_ERROR);
-	romdbgsts = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_DSP_REG_HFFLGPXQWY);
-	romdbgerr = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_DSP_REG_HFFLGPXQWY_ERROR);
-
-	dev_err(sdev->dev, "ROM status: %#x, ROM error: %#x\n", fwsts, fwlec);
-	dev_err(sdev->dev, "ROM debug status: %#x, ROM debug error: %#x\n", romdbgsts,
-		romdbgerr);
-	romdbgsts = snd_sof_dsp_read(sdev, HDA_DSP_BAR, MTL_DSP_REG_HFFLGPXQWY + 0x8 * 3);
-	dev_printk(level, sdev->dev, "ROM feature bit%s enabled\n",
-		   romdbgsts & BIT(24) ? "" : " not");
+
+	if (fwsts != 0xffffffff)
+		dev_err(sdev->dev, "Firmware state: %#x, status/error code: %#x\n",
+			fwsts, fwlec);
 
 	sof_ipc4_intel_dump_telemetry_state(sdev, flags);
 }
-- 
2.44.0

Re: [PATCH 0/7] ASoC: SOF: Intel: mtl/lnl: Improve firmware boot state handling

[Mark Brown]

On Wed, 03 Apr 2024 13:52:03 +0300, Peter Ujfalusi wrote:
> this series improves the firmware/boot state handling which will allow
> failed IMR boot recovery and human readable boot failure decoding.
> 
> Additionally a new debugfs file is added to force a purge/clean boot
> of the DSP for developers.
> 
> Regards,
> Peter
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/7] ASoC: SOF: Intel: hda: Create debugfs file to force a clean DSP boot
      commit: bbdf9af261adca039de29e7fc1faff367bf7e9a0
[2/7] ASoC: SOF: Intel: mtl: Correct rom_status_reg
      commit: 1f1b820dc3c65b6883da3130ba3b8624dcbf87db
[3/7] ASoC: SOF: Intel: lnl: Correct rom_status_reg
      commit: b852574c671a9983dd51c81582c8c5085f3dc382
[4/7] ASoC: SOF: Intel: mtl: Disable interrupts when firmware boot failed
      commit: 26187f44aabdf3df7609b7c78724a059c230a2ad
[5/7] ASoC: SOF: Intel: mtl: Implement firmware boot state check
      commit: 6b1c1c47e76f0161bda2b1ac2e86a219fe70244f
[6/7] ASoC: SOF: Intel: hda-dsp/mtl: Add support for ACE ROM state codes
      commit: 3dc2682870ea8f5a7749c069dfc4e0040e69cb5d
[7/7] ASoC: SOF: Intel: mtl: Correct the mtl_dsp_dump output
      commit: 40bdf121a3ed91281196068e50789888e4b1d2d2

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