[PATCH 00/20] ASoC: SOF: Re-visit firmware state and panic tracking/handling
Hi,
this series will improve how we are tracking the firmware's state to be able to avoid communication with it when it is not going to answer due to a panic and we will attempt to force power cycle the DSP to recover at the next runtime suspend time.
The state handling brings in other improvements on the way the kernel reports errors and DSP panics to reduce the printed lines for normal users, but at the same time allowing developers (or for bug reports) to have more precise information available to track down the issue.
We can now place messages easily in the correct debug level and not bound to the static ERROR for some of the print chains, causing excess amount or partial information to be printed, confusing users and machines (CI).
I would have prefered to split this series up, but it was developed together to achieve a single goal to reduce the noise, but also provide the details we need to be able to rootcause issues.
Regards, Peter --- Peter Ujfalusi (20): ASoC: SOF: ops: Use dev_warn() if the panic offsets differ ASoC: SOF: Intel: hda-loader: Avoid re-defining the HDA_FW_BOOT_ATTEMPTS ASoC: SOF: core: Add simple wrapper to check flags in sof_core_debug ASoC: SOF: Use sof_debug_check_flag() instead of sof_core_debug directly ASoC: SOF: Add 'non_recoverable' parameter to snd_sof_dsp_panic() ASoC: SOF: Add a 'message' parameter to snd_sof_dsp_dbg_dump() ASoC: SOF: Introduce new firmware state: SOF_FW_CRASHED ASoC: SOF: Introduce new firmware state: SOF_FW_BOOT_READY_OK ASoC: SOF: Move the definition of enum snd_sof_fw_state to global header ASoC: SOF: Rename 'enum snd_sof_fw_state' to 'enum sof_fw_state' ASoC: SOF: ipc: Only allow sending of an IPC in SOF_FW_BOOT_COMPLETE state ASoC: SOF: Set SOF_FW_BOOT_FAILED in case we have failure during boot ASoC: SOF: pm: Force DSP off on suspend in BOOT_FAILED state also ASoc: SOF: core: Update the FW boot state transition diagram ASoC: SOF: ops: Always print DSP Panic message but use different message ASoC: SOF: dsp_arch_ops: add kernel log level parameter for oops and stack ASoC: SOF: Rename snd_sof_get_status() and add kernel log level parameter ASoC: SOF: Add clarifying comments for sof_core_debug and DSP dump flags ASoC: SOF: debug: Use DEBUG log level for optional prints ASoC: SOF: Intel: hda: Use DEBUG log level for optional prints
include/sound/sof.h | 22 ++++++ sound/soc/sof/core.c | 119 +++++++++++++++++++------------ sound/soc/sof/debug.c | 35 +++++---- sound/soc/sof/imx/imx-common.c | 4 +- sound/soc/sof/imx/imx8.c | 2 +- sound/soc/sof/imx/imx8m.c | 2 +- sound/soc/sof/intel/atom.c | 8 +-- sound/soc/sof/intel/bdw.c | 8 +-- sound/soc/sof/intel/cnl.c | 21 +++++- sound/soc/sof/intel/hda-ipc.c | 19 ++++- sound/soc/sof/intel/hda-loader.c | 24 ++++--- sound/soc/sof/intel/hda.c | 20 +++--- sound/soc/sof/intel/hda.h | 2 +- sound/soc/sof/ipc.c | 4 +- sound/soc/sof/loader.c | 16 ++--- sound/soc/sof/ops.c | 47 ++++++++---- sound/soc/sof/ops.h | 4 +- sound/soc/sof/pm.c | 10 +++ sound/soc/sof/sof-priv.h | 44 +++++------- sound/soc/sof/topology.c | 12 ++-- sound/soc/sof/xtensa/core.c | 44 +++++++----- 21 files changed, 299 insertions(+), 168 deletions(-)
Catch the cases when the stored sdev->dsp_oops_offset and the offset received via the panic message differs and print a warning, but keep using the dsp_oops_offset for the oops query.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/ops.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/ops.c b/sound/soc/sof/ops.c index 160b88a2d59f..1d6a95a00cf5 100644 --- a/sound/soc/sof/ops.c +++ b/sound/soc/sof/ops.c @@ -144,18 +144,23 @@ EXPORT_SYMBOL(snd_sof_dsp_update_bits_forced);
void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset) { - dev_err(sdev->dev, "error : DSP panic!\n"); - /* - * check if DSP is not ready and did not set the dsp_oops_offset. - * if the dsp_oops_offset is not set, set it from the panic message. - * Also add a check to memory window setting with panic message. + * if DSP is not ready and the dsp_oops_offset is not yet set, use the + * offset from the panic message. */ if (!sdev->dsp_oops_offset) sdev->dsp_oops_offset = offset; - else - dev_dbg(sdev->dev, "panic: dsp_oops_offset %zu offset %d\n", - sdev->dsp_oops_offset, offset); + + /* + * Print warning if the offset from the panic message differs from + * dsp_oops_offset + */ + if (sdev->dsp_oops_offset != offset) + dev_warn(sdev->dev, + "%s: dsp_oops_offset %zu differs from panic offset %u\n", + __func__, sdev->dsp_oops_offset, offset); + + dev_err(sdev->dev, "DSP panic!\n");
/* We want to see the DSP panic! */ sdev->dbg_dump_printed = false;
HDA_FW_BOOT_ATTEMPTS is defined in hda.h, do not define it again locally in hda-loader.c
At the same time correct the indentation for the define in hda.h
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 1 - sound/soc/sof/intel/hda.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index bfb0e374ebab..5f5f396f4fb8 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -23,7 +23,6 @@ #include "../ops.h" #include "hda.h"
-#define HDA_FW_BOOT_ATTEMPTS 3 #define HDA_CL_STREAM_FORMAT 0x40
static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format, diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 5b4d59647a1d..03a6bb7a165c 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -273,7 +273,7 @@ #define BXT_D0I3_DELAY 5000
#define FW_CL_STREAM_NUMBER 0x1 -#define HDA_FW_BOOT_ATTEMPTS 3 +#define HDA_FW_BOOT_ATTEMPTS 3
/* ADSPCS - Audio DSP Control & Status */
The sof_debug_check_flag() can be used to check a flag or a combination of them in sof_core_debug.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/core.c | 16 ++++++++++++++++ sound/soc/sof/sof-priv.h | 1 + 2 files changed, 17 insertions(+)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 40549cdd6d58..1224a7da053a 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -27,6 +27,22 @@ MODULE_PARM_DESC(sof_debug, "SOF core debug options (0x0 all off)"); #define TIMEOUT_DEFAULT_IPC_MS 500 #define TIMEOUT_DEFAULT_BOOT_MS 2000
+/** + * sof_debug_check_flag - check if a given flag(s) is set in sof_core_debug + * @mask: Flag or combination of flags to check + * + * Returns true if all bits set in mask is also set in sof_core_debug, otherwise + * false + */ +bool sof_debug_check_flag(int mask) +{ + if ((sof_core_debug & mask) == mask) + return true; + + return false; +} +EXPORT_SYMBOL(sof_debug_check_flag); + /* * FW Panic/fault handling. */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 114882e4370f..35c5b2d6930b 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -43,6 +43,7 @@
/* global debug state set by SOF_DBG_ flags */ extern int sof_core_debug; +bool sof_debug_check_flag(int mask);
/* max BARs mmaped devices can use */ #define SND_SOF_BARS 8
The sof_debug_check_flag() is available for checking flags set in sof_core_debug.
sof_core_debug can be marked static in core.c
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/core.c | 4 ++-- sound/soc/sof/debug.c | 6 +++--- sound/soc/sof/sof-priv.h | 1 - sound/soc/sof/topology.c | 12 ++++++------ 4 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 1224a7da053a..00f8ffee2866 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -19,7 +19,7 @@ #endif
/* see SOF_DBG_ flags */ -int sof_core_debug = IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_FIRMWARE_TRACE); +static int sof_core_debug = IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_FIRMWARE_TRACE); module_param_named(sof_debug, sof_core_debug, int, 0444); MODULE_PARM_DESC(sof_debug, "SOF core debug options (0x0 all off)");
@@ -218,7 +218,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) goto fw_run_err; }
- if (sof_core_debug & SOF_DBG_ENABLE_TRACE) { + if (sof_debug_check_flag(SOF_DBG_ENABLE_TRACE)) { sdev->dtrace_is_supported = true;
/* init DMA trace */ diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 2f8b5ac9b78a..9e4a128b5918 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -957,7 +957,7 @@ static void snd_sof_dbg_print_fw_state(struct snd_sof_dev *sdev)
void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags) { - bool print_all = !!(sof_core_debug & SOF_DBG_PRINT_ALL_DUMPS); + bool print_all = sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS);
if (flags & SOF_DBG_DUMP_OPTIONAL && !print_all) return; @@ -979,7 +979,7 @@ static void snd_sof_ipc_dump(struct snd_sof_dev *sdev) dev_err(sdev->dev, "------------[ IPC dump start ]------------\n"); sof_ops(sdev)->ipc_dump(sdev); dev_err(sdev->dev, "------------[ IPC dump end ]------------\n"); - if (!(sof_core_debug & SOF_DBG_PRINT_ALL_DUMPS)) + if (!sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS)) sdev->ipc_dump_printed = true; } } @@ -987,7 +987,7 @@ static void snd_sof_ipc_dump(struct snd_sof_dev *sdev) void snd_sof_handle_fw_exception(struct snd_sof_dev *sdev) { if (IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_RETAIN_DSP_CONTEXT) || - (sof_core_debug & SOF_DBG_RETAIN_CTX)) { + sof_debug_check_flag(SOF_DBG_RETAIN_CTX)) { /* should we prevent DSP entering D3 ? */ if (!sdev->ipc_dump_printed) dev_info(sdev->dev, diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 35c5b2d6930b..0f849cdbfbc8 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -42,7 +42,6 @@ #define SOF_DBG_DUMP_OPTIONAL BIT(4) /* only dump if SOF_DBG_PRINT_ALL_DUMPS is set */
/* global debug state set by SOF_DBG_ flags */ -extern int sof_core_debug; bool sof_debug_check_flag(int mask);
/* max BARs mmaped devices can use */ diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index ec59baf32699..e72dcae5e7ee 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1695,12 +1695,12 @@ static int sof_widget_load_pipeline(struct snd_soc_component *scomp, int index, goto err; }
- if (sof_core_debug & SOF_DBG_DISABLE_MULTICORE) + if (sof_debug_check_flag(SOF_DBG_DISABLE_MULTICORE)) pipeline->core = SOF_DSP_PRIMARY_CORE;
- if (sof_core_debug & SOF_DBG_DYNAMIC_PIPELINES_OVERRIDE) - swidget->dynamic_pipeline_widget = sof_core_debug & - SOF_DBG_DYNAMIC_PIPELINES_ENABLE; + if (sof_debug_check_flag(SOF_DBG_DYNAMIC_PIPELINES_OVERRIDE)) + swidget->dynamic_pipeline_widget = + sof_debug_check_flag(SOF_DBG_DYNAMIC_PIPELINES_ENABLE);
dev_dbg(scomp->dev, "pipeline %s: period %d pri %d mips %d core %d frames %d dynamic %d\n", swidget->widget->name, pipeline->period, pipeline->priority, @@ -2295,7 +2295,7 @@ static int sof_widget_ready(struct snd_soc_component *scomp, int index, return ret; }
- if (sof_core_debug & SOF_DBG_DISABLE_MULTICORE) + if (sof_debug_check_flag(SOF_DBG_DISABLE_MULTICORE)) comp.core = SOF_DSP_PRIMARY_CORE;
swidget->core = comp.core; @@ -3542,7 +3542,7 @@ static int sof_complete(struct snd_soc_component *scomp) }
/* verify topology components loading including dynamic pipelines */ - if (sof_core_debug & SOF_DBG_VERIFY_TPLG) { + if (sof_debug_check_flag(SOF_DBG_VERIFY_TPLG)) { ret = sof_set_up_pipelines(sdev, true); if (ret < 0) { dev_err(sdev->dev, "error: topology verification failed %d\n", ret);
Some platforms use retries during firmware boot to overcome DSP startup issues. In these cases we might receive a DSP panic message which should not be treated as fatal if it happens during boot.
Pass this information to snd_sof_dsp_panic() and omit the panic print if it is not fatal or the user does not want to see all dumps.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/imx/imx8.c | 2 +- sound/soc/sof/imx/imx8m.c | 2 +- sound/soc/sof/intel/atom.c | 4 ++-- sound/soc/sof/intel/bdw.c | 4 ++-- sound/soc/sof/intel/cnl.c | 21 ++++++++++++++++++--- sound/soc/sof/intel/hda-ipc.c | 19 +++++++++++++++++-- sound/soc/sof/intel/hda-loader.c | 8 ++++++-- sound/soc/sof/ops.c | 24 ++++++++++++++++++------ sound/soc/sof/ops.h | 2 +- 9 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index 099b4356122c..f6baecbb57fb 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -97,7 +97,7 @@ static void imx8_dsp_handle_request(struct imx_dsp_ipc *ipc)
/* Check to see if the message is a panic code (0x0dead***) */ if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) - snd_sof_dsp_panic(priv->sdev, p); + snd_sof_dsp_panic(priv->sdev, p, true); else snd_sof_ipc_msgs_rx(priv->sdev); } diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c index c026caea4c8b..788e77bcb603 100644 --- a/sound/soc/sof/imx/imx8m.c +++ b/sound/soc/sof/imx/imx8m.c @@ -90,7 +90,7 @@ static void imx8m_dsp_handle_request(struct imx_dsp_ipc *ipc)
/* Check to see if the message is a panic code (0x0dead***) */ if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) - snd_sof_dsp_panic(priv->sdev, p); + snd_sof_dsp_panic(priv->sdev, p, true); else snd_sof_ipc_msgs_rx(priv->sdev); } diff --git a/sound/soc/sof/intel/atom.c b/sound/soc/sof/intel/atom.c index 5aa064b28fca..bcb2eb2acf2e 100644 --- a/sound/soc/sof/intel/atom.c +++ b/sound/soc/sof/intel/atom.c @@ -165,8 +165,8 @@ irqreturn_t atom_irq_thread(int irq, void *context)
/* Handle messages from DSP Core */ if ((ipcd & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { - snd_sof_dsp_panic(sdev, PANIC_OFFSET(ipcd) + - MBOX_OFFSET); + snd_sof_dsp_panic(sdev, PANIC_OFFSET(ipcd) + MBOX_OFFSET, + true); } else { snd_sof_ipc_msgs_rx(sdev); } diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c index 1121711e9029..10c9a0b39371 100644 --- a/sound/soc/sof/intel/bdw.c +++ b/sound/soc/sof/intel/bdw.c @@ -344,8 +344,8 @@ static irqreturn_t bdw_irq_thread(int irq, void *context)
/* Handle messages from DSP Core */ if ((ipcd & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { - snd_sof_dsp_panic(sdev, BDW_PANIC_OFFSET(ipcx) + - MBOX_OFFSET); + snd_sof_dsp_panic(sdev, BDW_PANIC_OFFSET(ipcx) + MBOX_OFFSET, + true); } else { snd_sof_ipc_msgs_rx(sdev); } diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c index 3da158d08980..e615125d575e 100644 --- a/sound/soc/sof/intel/cnl.c +++ b/sound/soc/sof/intel/cnl.c @@ -82,9 +82,24 @@ irqreturn_t cnl_ipc_irq_thread(int irq, void *context) msg, msg_ext);
/* handle messages from DSP */ - if ((hipctdr & SOF_IPC_PANIC_MAGIC_MASK) == - SOF_IPC_PANIC_MAGIC) { - snd_sof_dsp_panic(sdev, HDA_DSP_PANIC_OFFSET(msg_ext)); + if ((hipctdr & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + bool non_recoverable = true; + + /* + * This is a PANIC message! + * + * If it is arriving during firmware boot and it is not + * the last boot attempt then change the non_recoverable + * to false as the DSP might be able to boot in the next + * iteration(s) + */ + if (sdev->fw_state == SOF_FW_BOOT_IN_PROGRESS && + hda->boot_iteration < HDA_FW_BOOT_ATTEMPTS) + non_recoverable = false; + + snd_sof_dsp_panic(sdev, HDA_DSP_PANIC_OFFSET(msg_ext), + non_recoverable); } else { snd_sof_ipc_msgs_rx(sdev); } diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c index 2019087a84ce..f0cf8019d72d 100644 --- a/sound/soc/sof/intel/hda-ipc.c +++ b/sound/soc/sof/intel/hda-ipc.c @@ -173,8 +173,23 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
/* handle messages from DSP */ if ((hipct & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { - /* this is a PANIC message !! */ - snd_sof_dsp_panic(sdev, HDA_DSP_PANIC_OFFSET(msg_ext)); + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + bool non_recoverable = true; + + /* + * This is a PANIC message! + * + * If it is arriving during firmware boot and it is not + * the last boot attempt then change the non_recoverable + * to false as the DSP might be able to boot in the next + * iteration(s) + */ + if (sdev->fw_state == SOF_FW_BOOT_IN_PROGRESS && + hda->boot_iteration < HDA_FW_BOOT_ATTEMPTS) + non_recoverable = false; + + snd_sof_dsp_panic(sdev, HDA_DSP_PANIC_OFFSET(msg_ext), + non_recoverable); } else { /* normal message - process normally */ snd_sof_ipc_msgs_rx(sdev); diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 5f5f396f4fb8..8ef16f1082e3 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -413,9 +413,13 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) hda_sdw_process_wakeen(sdev);
/* - * at this point DSP ROM has been initialized and - * should be ready for code loading and firmware boot + * Set the boot_iteration to the last attempt, indicating that the + * DSP ROM has been initialized and from this point there will be no + * retry done to boot. + * + * Continue with code loading and firmware boot */ + hda->boot_iteration = HDA_FW_BOOT_ATTEMPTS; ret = cl_copy_fw(sdev, stream); if (!ret) { dev_dbg(sdev->dev, "Firmware download successful, booting...\n"); diff --git a/sound/soc/sof/ops.c b/sound/soc/sof/ops.c index 1d6a95a00cf5..9abf7a8e55e0 100644 --- a/sound/soc/sof/ops.c +++ b/sound/soc/sof/ops.c @@ -142,7 +142,13 @@ void snd_sof_dsp_update_bits_forced(struct snd_sof_dev *sdev, u32 bar, } EXPORT_SYMBOL(snd_sof_dsp_update_bits_forced);
-void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset) +/** + * snd_sof_dsp_panic - handle a received DSP panic message + * @sdev: Pointer to the device's sdev + * @offset: offset of panic information + * @non_recoverable: the panic is fatal, no recovery will be done by the caller + */ +void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset, bool non_recoverable) { /* * if DSP is not ready and the dsp_oops_offset is not yet set, use the @@ -160,12 +166,18 @@ void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset) "%s: dsp_oops_offset %zu differs from panic offset %u\n", __func__, sdev->dsp_oops_offset, offset);
- dev_err(sdev->dev, "DSP panic!\n"); + /* + * Only print the panic information if we have non recoverable panic or + * if all dumps should be printed + */ + if (non_recoverable || sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS)) { + dev_err(sdev->dev, "DSP panic!\n");
- /* We want to see the DSP panic! */ - sdev->dbg_dump_printed = false; + /* We want to see the DSP panic! */ + sdev->dbg_dump_printed = false;
- snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); - snd_sof_trace_notify_for_error(sdev); + snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); + snd_sof_trace_notify_for_error(sdev); + } } EXPORT_SYMBOL(snd_sof_dsp_panic); diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index b0ffb2a93bcc..bca7d35536e4 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -643,5 +643,5 @@ int snd_sof_dsp_register_poll(struct snd_sof_dev *sdev, u32 bar, u32 offset, u32 mask, u32 target, u32 timeout_ms, u32 interval_us);
-void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset); +void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset, bool non_recoverable); #endif
When snd_sof_dsp_dbg_dump() is called we have an explanatory message to give some hint on the reason why we have the dump on the caller level.
Pass this message to snd_sof_dsp_dbg_dump() and handle the print according to the dump rules.
This way we can finally print information on the HDA boot iteration if all dumps are enabled.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com --- sound/soc/sof/debug.c | 9 +++++++-- sound/soc/sof/intel/hda-loader.c | 15 +++++++++------ sound/soc/sof/loader.c | 8 ++++---- sound/soc/sof/ops.c | 5 ++--- sound/soc/sof/ops.h | 2 +- 5 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 9e4a128b5918..cf7d95c33afe 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -955,7 +955,7 @@ static void snd_sof_dbg_print_fw_state(struct snd_sof_dev *sdev) dev_err(sdev->dev, "fw_state: UNKNOWN (%d)\n", sdev->fw_state); }
-void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags) +void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, const char *msg, u32 flags) { bool print_all = sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS);
@@ -964,11 +964,15 @@ void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags)
if (sof_ops(sdev)->dbg_dump && !sdev->dbg_dump_printed) { dev_err(sdev->dev, "------------[ DSP dump start ]------------\n"); + if (msg) + dev_err(sdev->dev, "%s\n", msg); snd_sof_dbg_print_fw_state(sdev); sof_ops(sdev)->dbg_dump(sdev, flags); dev_err(sdev->dev, "------------[ DSP dump end ]------------\n"); if (!print_all) sdev->dbg_dump_printed = true; + } else if (msg) { + dev_err(sdev->dev, "%s\n", msg); } } EXPORT_SYMBOL(snd_sof_dsp_dbg_dump); @@ -997,7 +1001,8 @@ void snd_sof_handle_fw_exception(struct snd_sof_dev *sdev)
/* dump vital information to the logs */ snd_sof_ipc_dump(sdev); - snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); + snd_sof_dsp_dbg_dump(sdev, "Firmware exception", + SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); snd_sof_trace_notify_for_error(sdev); } EXPORT_SYMBOL(snd_sof_handle_fw_exception); diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 8ef16f1082e3..33306d2023a7 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -88,6 +88,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) const struct sof_intel_dsp_desc *chip = hda->desc; unsigned int status; unsigned long mask; + char *dump_msg; u32 flags, j; int ret; int i; @@ -189,9 +190,12 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) flags &= ~SOF_DBG_DUMP_OPTIONAL;
- snd_sof_dsp_dbg_dump(sdev, flags); + 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); hda_dsp_core_reset_power_down(sdev, chip->host_managed_cores_mask);
+ kfree(dump_msg); return ret; }
@@ -421,12 +425,11 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) */ hda->boot_iteration = HDA_FW_BOOT_ATTEMPTS; ret = cl_copy_fw(sdev, stream); - if (!ret) { + if (!ret) dev_dbg(sdev->dev, "Firmware download successful, booting...\n"); - } else { - snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_PCI | SOF_DBG_DUMP_MBOX); - dev_err(sdev->dev, "error: load fw failed ret: %d\n", ret); - } + else + snd_sof_dsp_dbg_dump(sdev, "Firmware download failed", + SOF_DBG_DUMP_PCI | SOF_DBG_DUMP_MBOX);
cleanup: /* diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index c04646647637..8977a65b5704 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -820,8 +820,8 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev) /* boot the firmware on the DSP */ ret = snd_sof_dsp_run(sdev); if (ret < 0) { - dev_err(sdev->dev, "error: failed to start DSP\n"); - snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_MBOX | SOF_DBG_DUMP_PCI); + snd_sof_dsp_dbg_dump(sdev, "Failed to start DSP", + SOF_DBG_DUMP_MBOX | SOF_DBG_DUMP_PCI); return ret; }
@@ -835,8 +835,8 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev) sdev->fw_state > SOF_FW_BOOT_IN_PROGRESS, msecs_to_jiffies(sdev->boot_timeout)); if (ret == 0) { - dev_err(sdev->dev, "error: firmware boot failure\n"); - snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX | + snd_sof_dsp_dbg_dump(sdev, "Firmware boot failure due to timeout", + SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX | SOF_DBG_DUMP_TEXT | SOF_DBG_DUMP_PCI); sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); return -EIO; diff --git a/sound/soc/sof/ops.c b/sound/soc/sof/ops.c index 9abf7a8e55e0..edfd080a3e4f 100644 --- a/sound/soc/sof/ops.c +++ b/sound/soc/sof/ops.c @@ -171,12 +171,11 @@ void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset, bool non_recoverabl * if all dumps should be printed */ if (non_recoverable || sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS)) { - dev_err(sdev->dev, "DSP panic!\n"); - /* We want to see the DSP panic! */ sdev->dbg_dump_printed = false;
- snd_sof_dsp_dbg_dump(sdev, SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); + snd_sof_dsp_dbg_dump(sdev, "DSP panic!", + SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); snd_sof_trace_notify_for_error(sdev); } } diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h index bca7d35536e4..ffe7456e7713 100644 --- a/sound/soc/sof/ops.h +++ b/sound/soc/sof/ops.h @@ -274,7 +274,7 @@ snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev, }
/* debug */ -void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, u32 flags); +void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, const char *msg, u32 flags);
static inline int snd_sof_debugfs_add_region_item(struct snd_sof_dev *sdev, enum snd_sof_fw_blk_type blk_type, u32 offset, size_t size,
The SOF_FW_CRASHED state is meant to indicate the unfortunate case when the firmware has crashed after a successful boot.
IPC tx timeout is not treated as indication of a firmware crash as it tends to happen regularly while the firmware is operational.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Paul Olaru paul.olaru@oss.nxp.com --- sound/soc/sof/debug.c | 1 + sound/soc/sof/ipc.c | 2 +- sound/soc/sof/ops.c | 2 ++ sound/soc/sof/pm.c | 7 +++++++ sound/soc/sof/sof-priv.h | 1 + 5 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index cf7d95c33afe..5941316751dd 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -939,6 +939,7 @@ static const struct soc_fw_state_info { {SOF_FW_BOOT_FAILED, "SOF_FW_BOOT_FAILED"}, {SOF_FW_BOOT_READY_FAILED, "SOF_FW_BOOT_READY_FAILED"}, {SOF_FW_BOOT_COMPLETE, "SOF_FW_BOOT_COMPLETE"}, + {SOF_FW_CRASHED, "SOF_FW_CRASHED"}, };
static void snd_sof_dbg_print_fw_state(struct snd_sof_dev *sdev) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 12860da1d373..898f261e8603 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -302,7 +302,7 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header, struct snd_sof_ipc_msg *msg; int ret;
- if (ipc->disable_ipc_tx) + if (ipc->disable_ipc_tx || sdev->fw_state == SOF_FW_CRASHED) return -ENODEV;
/* diff --git a/sound/soc/sof/ops.c b/sound/soc/sof/ops.c index edfd080a3e4f..ed46f33ce72b 100644 --- a/sound/soc/sof/ops.c +++ b/sound/soc/sof/ops.c @@ -176,6 +176,8 @@ void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset, bool non_recoverabl
snd_sof_dsp_dbg_dump(sdev, "DSP panic!", SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); + if (non_recoverable) + sof_set_fw_state(sdev, SOF_FW_CRASHED); snd_sof_trace_notify_for_error(sdev); } } diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index ac8ae6e422a7..f22b5ee23478 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -312,6 +312,13 @@ int snd_sof_prepare(struct device *dev) /* will suspend to S3 by default */ sdev->system_suspend_target = SOF_SUSPEND_S3;
+ /* + * if the firmware is crashed then we try to aim for S3 to reboot the + * firmware + */ + if (sdev->fw_state == SOF_FW_CRASHED) + return 0; + if (!desc->use_acpi_target_states) return 0;
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 0f849cdbfbc8..9bb30b2a516f 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -382,6 +382,7 @@ enum snd_sof_fw_state { SOF_FW_BOOT_FAILED, SOF_FW_BOOT_READY_FAILED, /* firmware booted but fw_ready op failed */ SOF_FW_BOOT_COMPLETE, + SOF_FW_CRASHED, };
/*
The SOF_FW_BOOT_READY_OK fw_state indicates that the boot ready message has been received and there were no errors found.
The SOF_FW_BOOT_COMPLETE state will be reached after the snd_sof_dsp_post_fw_run() completes without error.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Paul Olaru paul.olaru@oss.nxp.com --- sound/soc/sof/debug.c | 1 + sound/soc/sof/ipc.c | 2 +- sound/soc/sof/loader.c | 7 ++++--- sound/soc/sof/sof-priv.h | 1 + 4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 5941316751dd..75aaf0da87a0 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -938,6 +938,7 @@ static const struct soc_fw_state_info { {SOF_FW_BOOT_IN_PROGRESS, "SOF_FW_BOOT_IN_PROGRESS"}, {SOF_FW_BOOT_FAILED, "SOF_FW_BOOT_FAILED"}, {SOF_FW_BOOT_READY_FAILED, "SOF_FW_BOOT_READY_FAILED"}, + {SOF_FW_BOOT_READY_OK, "SOF_FW_BOOT_READY_OK"}, {SOF_FW_BOOT_COMPLETE, "SOF_FW_BOOT_COMPLETE"}, {SOF_FW_CRASHED, "SOF_FW_CRASHED"}, }; diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 898f261e8603..bbd539071ac5 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -536,7 +536,7 @@ void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev) if (err < 0) sof_set_fw_state(sdev, SOF_FW_BOOT_READY_FAILED); else - sof_set_fw_state(sdev, SOF_FW_BOOT_COMPLETE); + sof_set_fw_state(sdev, SOF_FW_BOOT_READY_OK);
/* wake up firmware loader */ wake_up(&sdev->boot_wait); diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index 8977a65b5704..f81f24732799 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -842,9 +842,7 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev) return -EIO; }
- if (sdev->fw_state == SOF_FW_BOOT_COMPLETE) - dev_dbg(sdev->dev, "firmware boot complete\n"); - else + if (sdev->fw_state == SOF_FW_BOOT_READY_FAILED) return -EIO; /* FW boots but fw_ready op failed */
/* perform post fw run operations */ @@ -854,6 +852,9 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev) return ret; }
+ dev_dbg(sdev->dev, "firmware boot complete\n"); + sof_set_fw_state(sdev, SOF_FW_BOOT_COMPLETE); + return 0; } EXPORT_SYMBOL(snd_sof_run_firmware); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 9bb30b2a516f..c92103a028ff 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -381,6 +381,7 @@ enum snd_sof_fw_state { SOF_FW_BOOT_IN_PROGRESS, SOF_FW_BOOT_FAILED, SOF_FW_BOOT_READY_FAILED, /* firmware booted but fw_ready op failed */ + SOF_FW_BOOT_READY_OK, SOF_FW_BOOT_COMPLETE, SOF_FW_CRASHED, };
Move the enum snd_sof_fw_state to include/sound/sof.h to be accessible outside of the core SOF stack.
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Paul Olaru paul.olaru@oss.nxp.com --- include/sound/sof.h | 22 ++++++++++++++++++++++ sound/soc/sof/sof-priv.h | 11 ----------- 2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/include/sound/sof.h b/include/sound/sof.h index 23b374311d16..b9131c01eefd 100644 --- a/include/sound/sof.h +++ b/include/sound/sof.h @@ -17,6 +17,28 @@
struct snd_sof_dsp_ops;
+/** + * enum snd_sof_fw_state - DSP firmware state definitions + * @SOF_FW_BOOT_NOT_STARTED: firmware boot is not yet started + * @SOF_FW_BOOT_PREPARE: preparing for boot (firmware loading for exaqmple) + * @SOF_FW_BOOT_IN_PROGRESS: firmware boot is in progress + * @SOF_FW_BOOT_FAILED: firmware boot failed + * @SOF_FW_BOOT_READY_FAILED: firmware booted but fw_ready op failed + * @SOF_FW_BOOT_READY_OK: firmware booted and fw_ready op passed + * @SOF_FW_BOOT_COMPLETE: firmware is booted up and functional + * @SOF_FW_CRASHED: firmware crashed after successful boot + */ +enum snd_sof_fw_state { + SOF_FW_BOOT_NOT_STARTED = 0, + SOF_FW_BOOT_PREPARE, + SOF_FW_BOOT_IN_PROGRESS, + SOF_FW_BOOT_FAILED, + SOF_FW_BOOT_READY_FAILED, + SOF_FW_BOOT_READY_OK, + SOF_FW_BOOT_COMPLETE, + SOF_FW_CRASHED, +}; + /* * SOF Platform data. */ diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index c92103a028ff..a1ebc89b216a 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -375,17 +375,6 @@ struct snd_sof_ipc_msg { bool ipc_complete; };
-enum snd_sof_fw_state { - SOF_FW_BOOT_NOT_STARTED = 0, - SOF_FW_BOOT_PREPARE, - SOF_FW_BOOT_IN_PROGRESS, - SOF_FW_BOOT_FAILED, - SOF_FW_BOOT_READY_FAILED, /* firmware booted but fw_ready op failed */ - SOF_FW_BOOT_READY_OK, - SOF_FW_BOOT_COMPLETE, - SOF_FW_CRASHED, -}; - /* * SOF Device Level. */
Since there is nothing SND about the firmware state, rename the enum from `snd_sof_fw_state` to simply `sof_fw_state`
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 Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Paul Olaru paul.olaru@oss.nxp.com --- include/sound/sof.h | 4 ++-- sound/soc/sof/debug.c | 2 +- sound/soc/sof/sof-priv.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/sound/sof.h b/include/sound/sof.h index b9131c01eefd..813680ab9aad 100644 --- a/include/sound/sof.h +++ b/include/sound/sof.h @@ -18,7 +18,7 @@ struct snd_sof_dsp_ops;
/** - * enum snd_sof_fw_state - DSP firmware state definitions + * enum sof_fw_state - DSP firmware state definitions * @SOF_FW_BOOT_NOT_STARTED: firmware boot is not yet started * @SOF_FW_BOOT_PREPARE: preparing for boot (firmware loading for exaqmple) * @SOF_FW_BOOT_IN_PROGRESS: firmware boot is in progress @@ -28,7 +28,7 @@ struct snd_sof_dsp_ops; * @SOF_FW_BOOT_COMPLETE: firmware is booted up and functional * @SOF_FW_CRASHED: firmware crashed after successful boot */ -enum snd_sof_fw_state { +enum sof_fw_state { SOF_FW_BOOT_NOT_STARTED = 0, SOF_FW_BOOT_PREPARE, SOF_FW_BOOT_IN_PROGRESS, diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 75aaf0da87a0..d3640ff33134 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -930,7 +930,7 @@ void snd_sof_free_debug(struct snd_sof_dev *sdev) EXPORT_SYMBOL_GPL(snd_sof_free_debug);
static const struct soc_fw_state_info { - enum snd_sof_fw_state state; + enum sof_fw_state state; const char *name; } fw_state_dbg[] = { {SOF_FW_BOOT_NOT_STARTED, "SOF_FW_BOOT_NOT_STARTED"}, diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index a1ebc89b216a..44ae8d8d1333 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -399,7 +399,7 @@ struct snd_sof_dev {
/* DSP firmware boot */ wait_queue_head_t boot_wait; - enum snd_sof_fw_state fw_state; + enum sof_fw_state fw_state; bool first_boot;
/* work queue in case the probe is implemented in two steps */ @@ -591,7 +591,7 @@ extern const struct dsp_arch_ops sof_xtensa_arch_ops; * Firmware state tracking */ static inline void sof_set_fw_state(struct snd_sof_dev *sdev, - enum snd_sof_fw_state new_state) + enum sof_fw_state new_state) { if (sdev->fw_state == new_state) return;
If the state of the firmware is not BOOT_COMPLETE, it means that the firmware is not functioning, thus it is not capable of handling IPC messages. Do not try to send IPC if the state is not BOOT_COMPLETE
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index bbd539071ac5..5bcf906d90af 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -302,7 +302,7 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header, struct snd_sof_ipc_msg *msg; int ret;
- if (ipc->disable_ipc_tx || sdev->fw_state == SOF_FW_CRASHED) + if (ipc->disable_ipc_tx || sdev->fw_state != SOF_FW_BOOT_COMPLETE) return -ENODEV;
/*
Change the fw_state to SOF_FW_BOOT_FAILED if we encountered an error during booting the firmware.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/core.c | 2 ++ sound/soc/sof/loader.c | 1 - sound/soc/sof/pm.c | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 00f8ffee2866..aa7a721f34e4 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -202,6 +202,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) if (ret < 0) { dev_err(sdev->dev, "error: failed to load DSP firmware %d\n", ret); + sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); goto fw_load_err; }
@@ -215,6 +216,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) if (ret < 0) { dev_err(sdev->dev, "error: failed to boot DSP firmware %d\n", ret); + sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); goto fw_run_err; }
diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c index f81f24732799..697f03565a70 100644 --- a/sound/soc/sof/loader.c +++ b/sound/soc/sof/loader.c @@ -838,7 +838,6 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev) snd_sof_dsp_dbg_dump(sdev, "Firmware boot failure due to timeout", SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX | SOF_DBG_DUMP_TEXT | SOF_DBG_DUMP_PCI); - sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); return -EIO; }
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index f22b5ee23478..022b19669735 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -130,6 +130,7 @@ static int sof_resume(struct device *dev, bool runtime_resume) dev_err(sdev->dev, "error: failed to load DSP firmware after resume %d\n", ret); + sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); return ret; }
@@ -144,6 +145,7 @@ static int sof_resume(struct device *dev, bool runtime_resume) dev_err(sdev->dev, "error: failed to boot DSP firmware after resume %d\n", ret); + sof_set_fw_state(sdev, SOF_FW_BOOT_FAILED); return ret; }
Try to force the DSP to be turned off next time if the fw_state is either CRASHED or BOOT_FAILED when a suspend happens in order to attempt a clean boot to recover.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/pm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 022b19669735..197a88695fef 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -315,10 +315,11 @@ int snd_sof_prepare(struct device *dev) sdev->system_suspend_target = SOF_SUSPEND_S3;
/* - * if the firmware is crashed then we try to aim for S3 to reboot the - * firmware + * if the firmware is crashed or boot failed then we try to aim for S3 + * to reboot the firmware */ - if (sdev->fw_state == SOF_FW_CRASHED) + if (sdev->fw_state == SOF_FW_CRASHED || + sdev->fw_state == SOF_FW_BOOT_FAILED) return 0;
if (!desc->use_acpi_target_states)
Update the state flow diagram to reflect the current implementation.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/core.c | 52 ++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index aa7a721f34e4..bc3d7192bdda 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -114,32 +114,32 @@ EXPORT_SYMBOL(snd_sof_get_status); /* * FW Boot State Transition Diagram * - * +-----------------------------------------------------------------------+ - * | | - * ------------------ ------------------ | - * | | | | | - * | BOOT_FAILED | | READY_FAILED |-------------------------+ | - * | | | | | | - * ------------------ ------------------ | | - * ^ ^ | | - * | | | | - * (FW Boot Timeout) (FW_READY FAIL) | | - * | | | | - * | | | | - * ------------------ | ------------------ | | - * | | | | | | | - * | IN_PROGRESS |---------------+------------->| COMPLETE | | | - * | | (FW Boot OK) (FW_READY OK) | | | | - * ------------------ ------------------ | | - * ^ | | | - * | | | | - * (FW Loading OK) (System Suspend/Runtime Suspend) - * | | | | - * | | | | - * ------------------ ------------------ | | | - * | | | |<-----+ | | - * | PREPARE | | NOT_STARTED |<---------------------+ | - * | | | |<---------------------------+ + * +----------------------------------------------------------------------+ + * | | + * ------------------ ------------------ | + * | | | | | + * | BOOT_FAILED |<-------| READY_FAILED | | + * | |<--+ | | ------------------ | + * ------------------ | ------------------ | | | + * ^ | ^ | CRASHED |---+ | + * | | | | | | | + * (FW Boot Timeout) | (FW_READY FAIL) ------------------ | | + * | | | ^ | | + * | | | |(DSP Panic) | | + * ------------------ | | ------------------ | | + * | | | | | | | | + * | IN_PROGRESS |---------------+------------->| COMPLETE | | | + * | | (FW Boot OK) (FW_READY OK) | | | | + * ------------------ | ------------------ | | + * ^ | | | | + * | | | | | + * (FW Loading OK) | (System Suspend/Runtime Suspend) + * | | | | | + * | (FW Loading Fail) | | | + * ------------------ | ------------------ | | | + * | | | | |<-----+ | | + * | PREPARE |---+ | NOT_STARTED |<---------------------+ | + * | | | |<--------------------------+ * ------------------ ------------------ * | ^ | ^ * | | | |
Never suppress the DSP panic dump as it is always originates from an assert() or panic() call within the firmware.
Use different message for DSP panics when there will be recovery attempt going to be done compared to a definitive DSP panic.
Suggested-by: Chao Song chao.song@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/ops.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/ops.c b/sound/soc/sof/ops.c index ed46f33ce72b..235e2ef72178 100644 --- a/sound/soc/sof/ops.c +++ b/sound/soc/sof/ops.c @@ -167,18 +167,21 @@ void snd_sof_dsp_panic(struct snd_sof_dev *sdev, u32 offset, bool non_recoverabl __func__, sdev->dsp_oops_offset, offset);
/* - * Only print the panic information if we have non recoverable panic or - * if all dumps should be printed + * Set the fw_state to crashed only in case of non recoverable DSP panic + * event. + * Use different message within the snd_sof_dsp_dbg_dump() depending on + * the non_recoverable flag. */ - if (non_recoverable || sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS)) { - /* We want to see the DSP panic! */ - sdev->dbg_dump_printed = false; - + sdev->dbg_dump_printed = false; + if (non_recoverable) { snd_sof_dsp_dbg_dump(sdev, "DSP panic!", SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); - if (non_recoverable) - sof_set_fw_state(sdev, SOF_FW_CRASHED); + sof_set_fw_state(sdev, SOF_FW_CRASHED); snd_sof_trace_notify_for_error(sdev); + } else { + snd_sof_dsp_dbg_dump(sdev, + "DSP panic (recovery will be attempted)", + SOF_DBG_DUMP_REGS | SOF_DBG_DUMP_MBOX); } } EXPORT_SYMBOL(snd_sof_dsp_panic);
To allow custom log level to be used for the DSP oops and stack print, add a kernel log level parameter to the two ops.
Modify the xtensa oops and stack functions tom use this new log level parameter.
Pass KER_ERR from snd_sof_get_status() to make sure that there is no functional change with this new parameter.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/core.c | 4 ++-- sound/soc/sof/sof-priv.h | 15 +++++++------ sound/soc/sof/xtensa/core.c | 44 +++++++++++++++++++++---------------- 3 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index bc3d7192bdda..c3630ecc7d89 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -106,8 +106,8 @@ void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code, out: dev_err(sdev->dev, "panic at %s:%d\n", panic_info->filename, panic_info->linenum); - sof_oops(sdev, oops); - sof_stack(sdev, oops, stack, stack_words); + sof_oops(sdev, KERN_ERR, oops); + sof_stack(sdev, KERN_ERR, oops, stack, stack_words); } EXPORT_SYMBOL(snd_sof_get_status);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 44ae8d8d1333..598f858f0e1b 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -309,8 +309,8 @@ struct snd_sof_dsp_ops {
/* DSP architecture specific callbacks for oops and stack dumps */ struct dsp_arch_ops { - void (*dsp_oops)(struct snd_sof_dev *sdev, void *oops); - void (*dsp_stack)(struct snd_sof_dev *sdev, void *oops, + void (*dsp_oops)(struct snd_sof_dev *sdev, const char *level, void *oops); + void (*dsp_stack)(struct snd_sof_dev *sdev, const char *level, void *oops, u32 *stack, u32 stack_words); };
@@ -573,16 +573,17 @@ int snd_sof_debugfs_add_region_item_iomem(struct snd_sof_dev *sdev, /* * DSP Architectures. */ -static inline void sof_stack(struct snd_sof_dev *sdev, void *oops, u32 *stack, - u32 stack_words) +static inline void sof_stack(struct snd_sof_dev *sdev, const char *level, + void *oops, u32 *stack, u32 stack_words) { - sof_dsp_arch_ops(sdev)->dsp_stack(sdev, oops, stack, stack_words); + sof_dsp_arch_ops(sdev)->dsp_stack(sdev, level, oops, stack, + stack_words); }
-static inline void sof_oops(struct snd_sof_dev *sdev, void *oops) +static inline void sof_oops(struct snd_sof_dev *sdev, const char *level, void *oops) { if (sof_dsp_arch_ops(sdev)->dsp_oops) - sof_dsp_arch_ops(sdev)->dsp_oops(sdev, oops); + sof_dsp_arch_ops(sdev)->dsp_oops(sdev, level, oops); }
extern const struct dsp_arch_ops sof_xtensa_arch_ops; diff --git a/sound/soc/sof/xtensa/core.c b/sound/soc/sof/xtensa/core.c index bd09c3825caf..bebbe3a2865c 100644 --- a/sound/soc/sof/xtensa/core.c +++ b/sound/soc/sof/xtensa/core.c @@ -81,33 +81,39 @@ static const struct xtensa_exception_cause xtensa_exception_causes[] = { };
/* only need xtensa atm */ -static void xtensa_dsp_oops(struct snd_sof_dev *sdev, void *oops) +static void xtensa_dsp_oops(struct snd_sof_dev *sdev, const char *level, void *oops) { struct sof_ipc_dsp_oops_xtensa *xoops = oops; int i;
- dev_err(sdev->dev, "error: DSP Firmware Oops\n"); + dev_printk(level, sdev->dev, "error: DSP Firmware Oops\n"); for (i = 0; i < ARRAY_SIZE(xtensa_exception_causes); i++) { if (xtensa_exception_causes[i].id == xoops->exccause) { - dev_err(sdev->dev, "error: Exception Cause: %s, %s\n", - xtensa_exception_causes[i].msg, - xtensa_exception_causes[i].description); + dev_printk(level, sdev->dev, + "error: Exception Cause: %s, %s\n", + xtensa_exception_causes[i].msg, + xtensa_exception_causes[i].description); } } - dev_err(sdev->dev, "EXCCAUSE 0x%8.8x EXCVADDR 0x%8.8x PS 0x%8.8x SAR 0x%8.8x\n", - xoops->exccause, xoops->excvaddr, xoops->ps, xoops->sar); - dev_err(sdev->dev, "EPC1 0x%8.8x EPC2 0x%8.8x EPC3 0x%8.8x EPC4 0x%8.8x", - xoops->epc1, xoops->epc2, xoops->epc3, xoops->epc4); - dev_err(sdev->dev, "EPC5 0x%8.8x EPC6 0x%8.8x EPC7 0x%8.8x DEPC 0x%8.8x", - xoops->epc5, xoops->epc6, xoops->epc7, xoops->depc); - dev_err(sdev->dev, "EPS2 0x%8.8x EPS3 0x%8.8x EPS4 0x%8.8x EPS5 0x%8.8x", - xoops->eps2, xoops->eps3, xoops->eps4, xoops->eps5); - dev_err(sdev->dev, "EPS6 0x%8.8x EPS7 0x%8.8x INTENABL 0x%8.8x INTERRU 0x%8.8x", - xoops->eps6, xoops->eps7, xoops->intenable, xoops->interrupt); + dev_printk(level, sdev->dev, + "EXCCAUSE 0x%8.8x EXCVADDR 0x%8.8x PS 0x%8.8x SAR 0x%8.8x\n", + xoops->exccause, xoops->excvaddr, xoops->ps, xoops->sar); + dev_printk(level, sdev->dev, + "EPC1 0x%8.8x EPC2 0x%8.8x EPC3 0x%8.8x EPC4 0x%8.8x", + xoops->epc1, xoops->epc2, xoops->epc3, xoops->epc4); + dev_printk(level, sdev->dev, + "EPC5 0x%8.8x EPC6 0x%8.8x EPC7 0x%8.8x DEPC 0x%8.8x", + xoops->epc5, xoops->epc6, xoops->epc7, xoops->depc); + dev_printk(level, sdev->dev, + "EPS2 0x%8.8x EPS3 0x%8.8x EPS4 0x%8.8x EPS5 0x%8.8x", + xoops->eps2, xoops->eps3, xoops->eps4, xoops->eps5); + dev_printk(level, sdev->dev, + "EPS6 0x%8.8x EPS7 0x%8.8x INTENABL 0x%8.8x INTERRU 0x%8.8x", + xoops->eps6, xoops->eps7, xoops->intenable, xoops->interrupt); }
-static void xtensa_stack(struct snd_sof_dev *sdev, void *oops, u32 *stack, - u32 stack_words) +static void xtensa_stack(struct snd_sof_dev *sdev, const char *level, void *oops, + u32 *stack, u32 stack_words) { struct sof_ipc_dsp_oops_xtensa *xoops = oops; u32 stack_ptr = xoops->plat_hdr.stackptr; @@ -115,7 +121,7 @@ static void xtensa_stack(struct snd_sof_dev *sdev, void *oops, u32 *stack, unsigned char buf[4 * 8 + 3 + 1]; int i;
- dev_err(sdev->dev, "stack dump from 0x%8.8x\n", stack_ptr); + dev_printk(level, sdev->dev, "stack dump from 0x%8.8x\n", stack_ptr);
/* * example output: @@ -124,7 +130,7 @@ static void xtensa_stack(struct snd_sof_dev *sdev, void *oops, u32 *stack, for (i = 0; i < stack_words; i += 4) { hex_dump_to_buffer(stack + i, 16, 16, 4, buf, sizeof(buf), false); - dev_err(sdev->dev, "0x%08x: %s\n", stack_ptr + i * 4, buf); + dev_printk(level, sdev->dev, "0x%08x: %s\n", stack_ptr + i * 4, buf); } }
The snd_sof_get_status() is not the best name for a function which in fact is tasked to print out DSP oops and stack. Rename it to sof_print_oops_and_stack().
At the same time add a new parameter to specify the desired kernel log level to be used for the prints.
When updating the users of the function, pass KERN_ERR for now to make sure that there is no functional change happens.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/core.c | 45 +++++++++++++++++++++------------- sound/soc/sof/imx/imx-common.c | 4 +-- sound/soc/sof/intel/atom.c | 4 +-- sound/soc/sof/intel/bdw.c | 4 +-- sound/soc/sof/intel/hda.c | 4 +-- sound/soc/sof/sof-priv.h | 8 +++--- 6 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index c3630ecc7d89..8f32b5b12b3e 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -68,23 +68,33 @@ static const struct sof_panic_msg panic_msg[] = { {SOF_IPC_PANIC_ASSERT, "assertion failed"}, };
-/* +/** + * sof_print_oops_and_stack - Handle the printing of DSP oops and stack trace + * @sdev: Pointer to the device's sdev + * @level: prink log level to use for the printing + * @panic_code: the panic code + * @tracep_code: tracepoint code + * @oops: Pointer to DSP specific oops data + * @panic_info: Pointer to the received panic information message + * @stack: Pointer to the call stack data + * @stack_words: Number of words in the stack data + * * helper to be called from .dbg_dump callbacks. No error code is * provided, it's left as an exercise for the caller of .dbg_dump * (typically IPC or loader) */ -void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code, - u32 tracep_code, void *oops, - struct sof_ipc_panic_info *panic_info, - void *stack, size_t stack_words) +void sof_print_oops_and_stack(struct snd_sof_dev *sdev, const char *level, + u32 panic_code, u32 tracep_code, void *oops, + struct sof_ipc_panic_info *panic_info, + void *stack, size_t stack_words) { u32 code; int i;
/* is firmware dead ? */ if ((panic_code & SOF_IPC_PANIC_MAGIC_MASK) != SOF_IPC_PANIC_MAGIC) { - dev_err(sdev->dev, "unexpected fault %#010x trace %#010x\n", - panic_code, tracep_code); + dev_printk(level, sdev->dev, "unexpected fault %#010x trace %#010x\n", + panic_code, tracep_code); return; /* no fault ? */ }
@@ -92,24 +102,25 @@ void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code,
for (i = 0; i < ARRAY_SIZE(panic_msg); i++) { if (panic_msg[i].id == code) { - dev_err(sdev->dev, "reason: %s (%#x)\n", panic_msg[i].msg, - code & SOF_IPC_PANIC_CODE_MASK); - dev_err(sdev->dev, "trace point: %#010x\n", tracep_code); + dev_printk(level, sdev->dev, "reason: %s (%#x)\n", + panic_msg[i].msg, code & SOF_IPC_PANIC_CODE_MASK); + dev_printk(level, sdev->dev, "trace point: %#010x\n", tracep_code); goto out; } }
/* unknown error */ - dev_err(sdev->dev, "unknown panic code: %#x\n", code & SOF_IPC_PANIC_CODE_MASK); - dev_err(sdev->dev, "trace point: %#010x\n", tracep_code); + dev_printk(level, sdev->dev, "unknown panic code: %#x\n", + code & SOF_IPC_PANIC_CODE_MASK); + dev_printk(level, sdev->dev, "trace point: %#010x\n", tracep_code);
out: - dev_err(sdev->dev, "panic at %s:%d\n", panic_info->filename, - panic_info->linenum); - sof_oops(sdev, KERN_ERR, oops); - sof_stack(sdev, KERN_ERR, oops, stack, stack_words); + dev_printk(level, sdev->dev, "panic at %s:%d\n", panic_info->filename, + panic_info->linenum); + sof_oops(sdev, level, oops); + sof_stack(sdev, level, oops, stack, stack_words); } -EXPORT_SYMBOL(snd_sof_get_status); +EXPORT_SYMBOL(sof_print_oops_and_stack);
/* * FW Boot State Transition Diagram diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c index 9371e9062cb1..36e3d414a18f 100644 --- a/sound/soc/sof/imx/imx-common.c +++ b/sound/soc/sof/imx/imx-common.c @@ -69,8 +69,8 @@ void imx8_dump(struct snd_sof_dev *sdev, u32 flags) IMX8_STACK_DUMP_SIZE);
/* Print the information to the console */ - snd_sof_get_status(sdev, status, status, &xoops, &panic_info, stack, - IMX8_STACK_DUMP_SIZE); + sof_print_oops_and_stack(sdev, KERN_ERR, status, status, &xoops, + &panic_info, stack, IMX8_STACK_DUMP_SIZE); } EXPORT_SYMBOL(imx8_dump);
diff --git a/sound/soc/sof/intel/atom.c b/sound/soc/sof/intel/atom.c index bcb2eb2acf2e..ff5900b155dc 100644 --- a/sound/soc/sof/intel/atom.c +++ b/sound/soc/sof/intel/atom.c @@ -70,8 +70,8 @@ void atom_dump(struct snd_sof_dev *sdev, u32 flags) panic = snd_sof_dsp_read64(sdev, DSP_BAR, SHIM_IPCX); atom_get_registers(sdev, &xoops, &panic_info, stack, STACK_DUMP_SIZE); - snd_sof_get_status(sdev, status, panic, &xoops, &panic_info, stack, - STACK_DUMP_SIZE); + sof_print_oops_and_stack(sdev, KERN_ERR, status, panic, &xoops, + &panic_info, stack, STACK_DUMP_SIZE);
/* provide some context for firmware debug */ imrx = snd_sof_dsp_read64(sdev, DSP_BAR, SHIM_IMRX); diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c index 10c9a0b39371..d627b7498d5e 100644 --- a/sound/soc/sof/intel/bdw.c +++ b/sound/soc/sof/intel/bdw.c @@ -258,8 +258,8 @@ static void bdw_dump(struct snd_sof_dev *sdev, u32 flags) panic = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IPCX); bdw_get_registers(sdev, &xoops, &panic_info, stack, BDW_STACK_DUMP_SIZE); - snd_sof_get_status(sdev, status, panic, &xoops, &panic_info, stack, - BDW_STACK_DUMP_SIZE); + sof_print_oops_and_stack(sdev, KERN_ERR, status, panic, &xoops, + &panic_info, stack, BDW_STACK_DUMP_SIZE);
/* provide some context for firmware debug */ imrx = snd_sof_dsp_read(sdev, BDW_DSP_BAR, SHIM_IMRX); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 21100d2e6644..97027530ecef 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -554,8 +554,8 @@ void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags)
hda_dsp_get_registers(sdev, &xoops, &panic_info, stack, HDA_DSP_STACK_DUMP_SIZE); - snd_sof_get_status(sdev, status, panic, &xoops, &panic_info, - stack, HDA_DSP_STACK_DUMP_SIZE); + sof_print_oops_and_stack(sdev, KERN_ERR, status, panic, &xoops, + &panic_info, stack, HDA_DSP_STACK_DUMP_SIZE); } else { hda_dsp_dump_ext_rom_status(sdev, flags); } diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 598f858f0e1b..5fbd4f29321a 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -559,10 +559,10 @@ int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev, int snd_sof_trace_update_pos(struct snd_sof_dev *sdev, struct sof_ipc_dma_trace_posn *posn); void snd_sof_trace_notify_for_error(struct snd_sof_dev *sdev); -void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code, - u32 tracep_code, void *oops, - struct sof_ipc_panic_info *panic_info, - void *stack, size_t stack_words); +void sof_print_oops_and_stack(struct snd_sof_dev *sdev, const char *level, + u32 panic_code, u32 tracep_code, void *oops, + struct sof_ipc_panic_info *panic_info, + void *stack, size_t stack_words); int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev); void snd_sof_handle_fw_exception(struct snd_sof_dev *sdev); int snd_sof_dbg_memory_info_init(struct snd_sof_dev *sdev);
Update the comment for the global SOF level debug flags and add one for the flags used to control the DSP dump functionality.
Document the expected behavior when the SOF_DBG_DUMP_OPTIONAL is passed for the DSP dump: Only print the dump if SOF_DBG_PRINT_ALL_DUMPS is set Print must use KERN_DEBUG log level
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/sof-priv.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 5fbd4f29321a..087935192ce8 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -20,7 +20,7 @@ #include <uapi/sound/sof/fw.h> #include <sound/sof/ext_manifest.h>
-/* debug flags */ +/* Flag definitions used in sof_core_debug (sof_debug module parameter) */ #define SOF_DBG_ENABLE_TRACE BIT(0) #define SOF_DBG_RETAIN_CTX BIT(1) /* prevent DSP D3 on FW exception */ #define SOF_DBG_VERIFY_TPLG BIT(2) /* verify topology during load */ @@ -35,11 +35,13 @@ */ #define SOF_DBG_PRINT_ALL_DUMPS BIT(6) /* Print all ipc and dsp dumps */
+/* Flag definitions used for controlling the DSP dump behavior */ #define SOF_DBG_DUMP_REGS BIT(0) #define SOF_DBG_DUMP_MBOX BIT(1) #define SOF_DBG_DUMP_TEXT BIT(2) #define SOF_DBG_DUMP_PCI BIT(3) -#define SOF_DBG_DUMP_OPTIONAL BIT(4) /* only dump if SOF_DBG_PRINT_ALL_DUMPS is set */ +/* Output this dump (at the DEBUG level) only when SOF_DBG_PRINT_ALL_DUMPS is set */ +#define SOF_DBG_DUMP_OPTIONAL BIT(4)
/* global debug state set by SOF_DBG_ flags */ bool sof_debug_check_flag(int mask);
If the user requested to see all dumps (even the optional ones) then use KERN_DEBUG level for the optional dumps as they are only for debugging purposes.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/debug.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index d3640ff33134..6d6757075f7c 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -943,38 +943,42 @@ static const struct soc_fw_state_info { {SOF_FW_CRASHED, "SOF_FW_CRASHED"}, };
-static void snd_sof_dbg_print_fw_state(struct snd_sof_dev *sdev) +static void snd_sof_dbg_print_fw_state(struct snd_sof_dev *sdev, const char *level) { int i;
for (i = 0; i < ARRAY_SIZE(fw_state_dbg); i++) { if (sdev->fw_state == fw_state_dbg[i].state) { - dev_err(sdev->dev, "fw_state: %s (%d)\n", fw_state_dbg[i].name, i); + dev_printk(level, sdev->dev, "fw_state: %s (%d)\n", + fw_state_dbg[i].name, i); return; } }
- dev_err(sdev->dev, "fw_state: UNKNOWN (%d)\n", sdev->fw_state); + dev_printk(level, sdev->dev, "fw_state: UNKNOWN (%d)\n", sdev->fw_state); }
void snd_sof_dsp_dbg_dump(struct snd_sof_dev *sdev, const char *msg, u32 flags) { + char *level = flags & SOF_DBG_DUMP_OPTIONAL ? KERN_DEBUG : KERN_ERR; bool print_all = sof_debug_check_flag(SOF_DBG_PRINT_ALL_DUMPS);
if (flags & SOF_DBG_DUMP_OPTIONAL && !print_all) return;
if (sof_ops(sdev)->dbg_dump && !sdev->dbg_dump_printed) { - dev_err(sdev->dev, "------------[ DSP dump start ]------------\n"); + dev_printk(level, sdev->dev, + "------------[ DSP dump start ]------------\n"); if (msg) - dev_err(sdev->dev, "%s\n", msg); - snd_sof_dbg_print_fw_state(sdev); + dev_printk(level, sdev->dev, "%s\n", msg); + snd_sof_dbg_print_fw_state(sdev, level); sof_ops(sdev)->dbg_dump(sdev, flags); - dev_err(sdev->dev, "------------[ DSP dump end ]------------\n"); + dev_printk(level, sdev->dev, + "------------[ DSP dump end ]------------\n"); if (!print_all) sdev->dbg_dump_printed = true; } else if (msg) { - dev_err(sdev->dev, "%s\n", msg); + dev_printk(level, sdev->dev, "%s\n", msg); } } EXPORT_SYMBOL(snd_sof_dsp_dbg_dump);
If the user requested to see all dumps (even the optional ones) then use KERN_DEBUG level for the optional dumps as they are only for debugging purposes.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Chao Song chao.song@intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/intel/hda.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 97027530ecef..18abbd13d593 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -474,7 +474,7 @@ static const struct hda_dsp_msg_code hda_dsp_rom_msg[] = { {HDA_DSP_ROM_NULL_FW_ENTRY, "error: null FW entry point"}, };
-static void hda_dsp_get_status(struct snd_sof_dev *sdev) +static void hda_dsp_get_status(struct snd_sof_dev *sdev, const char *level) { u32 status; int i; @@ -484,8 +484,8 @@ static void hda_dsp_get_status(struct snd_sof_dev *sdev)
for (i = 0; i < ARRAY_SIZE(hda_dsp_rom_msg); i++) { if (status == hda_dsp_rom_msg[i].code) { - dev_err(sdev->dev, "%s - code %8.8x\n", - hda_dsp_rom_msg[i].msg, status); + dev_printk(level, sdev->dev, "%s - code %8.8x\n", + hda_dsp_rom_msg[i].msg, status); return; } } @@ -523,7 +523,8 @@ static void hda_dsp_get_registers(struct snd_sof_dev *sdev, }
/* dump the first 8 dwords representing the extended ROM status */ -static void hda_dsp_dump_ext_rom_status(struct snd_sof_dev *sdev, u32 flags) +static void hda_dsp_dump_ext_rom_status(struct snd_sof_dev *sdev, const char *level, + u32 flags) { char msg[128]; int len = 0; @@ -535,18 +536,19 @@ static void hda_dsp_dump_ext_rom_status(struct snd_sof_dev *sdev, u32 flags) len += snprintf(msg + len, sizeof(msg) - len, " 0x%x", value); }
- dev_err(sdev->dev, "extended rom status: %s", msg); + dev_printk(level, sdev->dev, "extended rom status: %s", msg);
}
void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags) { + char *level = flags & SOF_DBG_DUMP_OPTIONAL ? KERN_DEBUG : KERN_ERR; struct sof_ipc_dsp_oops_xtensa xoops; struct sof_ipc_panic_info panic_info; u32 stack[HDA_DSP_STACK_DUMP_SIZE];
/* print ROM/FW status */ - hda_dsp_get_status(sdev); + hda_dsp_get_status(sdev, level);
if (flags & SOF_DBG_DUMP_REGS) { u32 status = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_FW_STATUS); @@ -554,10 +556,10 @@ void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags)
hda_dsp_get_registers(sdev, &xoops, &panic_info, stack, HDA_DSP_STACK_DUMP_SIZE); - sof_print_oops_and_stack(sdev, KERN_ERR, status, panic, &xoops, + sof_print_oops_and_stack(sdev, level, status, panic, &xoops, &panic_info, stack, HDA_DSP_STACK_DUMP_SIZE); } else { - hda_dsp_dump_ext_rom_status(sdev, flags); + hda_dsp_dump_ext_rom_status(sdev, level, flags); } }
On Thu, 23 Dec 2021 13:36:08 +0200, Peter Ujfalusi wrote:
this series will improve how we are tracking the firmware's state to be able to avoid communication with it when it is not going to answer due to a panic and we will attempt to force power cycle the DSP to recover at the next runtime suspend time.
The state handling brings in other improvements on the way the kernel reports errors and DSP panics to reduce the printed lines for normal users, but at the same time allowing developers (or for bug reports) to have more precise information available to track down the issue.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/20] ASoC: SOF: ops: Use dev_warn() if the panic offsets differ commit: 72b8ed83f7eccf84c54b68a551beae400949cc29 [02/20] ASoC: SOF: Intel: hda-loader: Avoid re-defining the HDA_FW_BOOT_ATTEMPTS commit: b2539ef00e4427350b26896540ccabd98e88c7bb [03/20] ASoC: SOF: core: Add simple wrapper to check flags in sof_core_debug commit: f902b21adba98f28eaa1cf5e509d99eaa7b1b36e [04/20] ASoC: SOF: Use sof_debug_check_flag() instead of sof_core_debug directly commit: 12b401f4de787627f4a25784a0278bbbf93122b6 [05/20] ASoC: SOF: Add 'non_recoverable' parameter to snd_sof_dsp_panic() commit: b2b10aa79fe2fb3d3393d0e90ffb5c1802992412 [06/20] ASoC: SOF: Add a 'message' parameter to snd_sof_dsp_dbg_dump() commit: 2f148430b96e975e895163d763bfc9c5088100eb [07/20] ASoC: SOF: Introduce new firmware state: SOF_FW_CRASHED commit: 4e1f86482189ddbef73f7be8c6e62e8e3730e6b9 [08/20] ASoC: SOF: Introduce new firmware state: SOF_FW_BOOT_READY_OK commit: b2e9eb3adb9a498b997b18852773e75d7af3b60d [09/20] ASoC: SOF: Move the definition of enum snd_sof_fw_state to global header commit: fc179420fde3821c4d191e81b4f7b05c1dab87e2 [10/20] ASoC: SOF: Rename 'enum snd_sof_fw_state' to 'enum sof_fw_state' commit: d41607d37c1385da799f9a2ddb10c460e573687e [11/20] ASoC: SOF: ipc: Only allow sending of an IPC in SOF_FW_BOOT_COMPLETE state commit: 9421ff7665f66452f61ee40566c6f562d3847873 [12/20] ASoC: SOF: Set SOF_FW_BOOT_FAILED in case we have failure during boot commit: e2406275be2b6b15d985f33aec921e6555e4f87a [13/20] ASoC: SOF: pm: Force DSP off on suspend in BOOT_FAILED state also commit: b54b3a4e08bc0210768a1839af2ff888376cae4c [14/20] ASoc: SOF: core: Update the FW boot state transition diagram commit: 9f89a988d5c222f2fba495bbc861a476bdf1bd30 [15/20] ASoC: SOF: ops: Always print DSP Panic message but use different message commit: fdc573b1c26a8859996de6fbae2d436511b74e00 [16/20] ASoC: SOF: dsp_arch_ops: add kernel log level parameter for oops and stack commit: b9f0bfd16d8b390b35dbec67c3ed74e74a0ade24 [17/20] ASoC: SOF: Rename snd_sof_get_status() and add kernel log level parameter commit: 4995ffce2ce2164fa507a5dbaf1aa38bab679cca [18/20] ASoC: SOF: Add clarifying comments for sof_core_debug and DSP dump flags commit: beb6ade168177bf6c43abe78b3c9512b260b8068 [19/20] ASoC: SOF: debug: Use DEBUG log level for optional prints commit: 0152b8a2f0831b03bb7483159ef28167dcd33ab0 [20/20] ASoC: SOF: Intel: hda: Use DEBUG log level for optional prints commit: 34bfba9a63ece79c683591e757899e61fbcaa753
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