From 53ec753137f2a407c949cef3158e89ece7688637 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 25 Aug 2020 16:50:34 -0700 Subject: [PATCH 1/7] ASoC: SOF: Intel: hda: report error only for the last ROM init iteration The FW boot sequence includes multiple attempts for ROM init. When it does take more than one attempt, we should not log the errors encountered during the failed attempts and only log them during the final iteration. Reviewed-by: Guennadi Liakhovetski Reviewed-by: Kai Vehmanen Reviewed-by: Pierre-Louis Bossart Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-2-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-loader.c | 44 +++++++++++++++++++------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 441d05cda604..9cb219b87b3e 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -79,7 +79,7 @@ static int cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format, * reset/stall and then turn it off */ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, - u32 fwsize, int stream_tag) + u32 fwsize, int stream_tag, int iteration) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; const struct sof_intel_dsp_desc *chip = hda->desc; @@ -90,7 +90,8 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, /* step 1: power up corex */ ret = hda_dsp_core_power_up(sdev, chip->cores_mask); if (ret < 0) { - dev_err(sdev->dev, "error: dsp core 0/1 power up failed\n"); + if (iteration == HDA_FW_BOOT_ATTEMPTS) + dev_err(sdev->dev, "error: dsp core 0/1 power up failed\n"); goto err; } @@ -112,7 +113,9 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, /* step 3: unset core 0 reset state & unstall/run core 0 */ ret = hda_dsp_core_run(sdev, HDA_DSP_CORE_MASK(0)); if (ret < 0) { - dev_err(sdev->dev, "error: dsp core start failed %d\n", ret); + if (iteration == HDA_FW_BOOT_ATTEMPTS) + dev_err(sdev->dev, + "error: dsp core start failed %d\n", ret); ret = -EIO; goto err; } @@ -126,8 +129,10 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, HDA_DSP_INIT_TIMEOUT_US); if (ret < 0) { - dev_err(sdev->dev, "error: %s: timeout for HIPCIE done\n", - __func__); + if (iteration == HDA_FW_BOOT_ATTEMPTS) + dev_err(sdev->dev, + "error: %s: timeout for HIPCIE done\n", + __func__); goto err; } @@ -141,7 +146,9 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, ret = hda_dsp_core_power_down(sdev, chip->cores_mask & ~(HDA_DSP_CORE_MASK(0))); if (ret < 0) { - dev_err(sdev->dev, "error: dsp core x power down failed\n"); + if (iteration == HDA_FW_BOOT_ATTEMPTS) + dev_err(sdev->dev, + "error: dsp core x power down failed\n"); goto err; } @@ -159,9 +166,10 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, const void *fwdata, if (!ret) return 0; - dev_err(sdev->dev, - "error: %s: timeout HDA_DSP_SRAM_REG_ROM_STATUS read\n", - __func__); + if (iteration == HDA_FW_BOOT_ATTEMPTS) + dev_err(sdev->dev, + "error: %s: timeout HDA_DSP_SRAM_REG_ROM_STATUS read\n", + __func__); err: hda_dsp_dump(sdev, SOF_DBG_REGS | SOF_DBG_PCI | SOF_DBG_MBOX); @@ -329,25 +337,25 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) /* try ROM init a few times before giving up */ for (i = 0; i < HDA_FW_BOOT_ATTEMPTS; i++) { + dev_dbg(sdev->dev, + "Attempting iteration %d of Core En/ROM load...\n", i); + ret = cl_dsp_init(sdev, stripped_firmware.data, - stripped_firmware.size, tag); + stripped_firmware.size, tag, i + 1); /* don't retry anymore if successful */ if (!ret) break; - - dev_dbg(sdev->dev, "iteration %d of Core En/ROM load failed: %d\n", - i, ret); - dev_dbg(sdev->dev, "Error code=0x%x: FW status=0x%x\n", - snd_sof_dsp_read(sdev, HDA_DSP_BAR, - HDA_DSP_SRAM_REG_ROM_ERROR), - snd_sof_dsp_read(sdev, HDA_DSP_BAR, - HDA_DSP_SRAM_REG_ROM_STATUS)); } if (i == HDA_FW_BOOT_ATTEMPTS) { dev_err(sdev->dev, "error: dsp init failed after %d attempts with err: %d\n", i, ret); + dev_err(sdev->dev, "ROM error=0x%x: FW status=0x%x\n", + snd_sof_dsp_read(sdev, HDA_DSP_BAR, + HDA_DSP_SRAM_REG_ROM_ERROR), + snd_sof_dsp_read(sdev, HDA_DSP_BAR, + HDA_DSP_SRAM_REG_ROM_STATUS)); goto cleanup; } From 6c63c954e1c52f1262f986f36d95f557c6f8fa94 Mon Sep 17 00:00:00 2001 From: Rander Wang Date: Tue, 25 Aug 2020 16:50:35 -0700 Subject: [PATCH 2/7] ASoC: SOF: fix a runtime pm issue in SOF when HDMI codec doesn't work When hda_codec_probe() doesn't initialize audio component, we disable the codec and keep going. However,the resources are not released. The child_count of SOF device is increased in snd_hdac_ext_bus_device_init but is not decrease in error case, so SOF can't get suspended. snd_hdac_ext_bus_device_exit will be invoked in HDA framework if it gets a error. Now copy this behavior to release resources and decrease SOF device child_count to release SOF device. Signed-off-by: Rander Wang Reviewed-by: Pierre-Louis Bossart Reviewed-by: Bard Liao Reviewed-by: Guennadi Liakhovetski Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-3-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-codec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index 55811b99e47a..f6ba3b593e1f 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -151,7 +151,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, if (!hdev->bus->audio_component) { dev_dbg(sdev->dev, "iDisp hw present but no driver\n"); - return -ENOENT; + goto error; } hda_priv->need_display_power = true; } @@ -174,7 +174,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address, * other return codes without modification */ if (ret == 0) - ret = -ENOENT; + goto error; } return ret; From 3dca35e35b42b3405ddad7ee95c02a2d8cf28592 Mon Sep 17 00:00:00 2001 From: Jaska Uimonen Date: Tue, 25 Aug 2020 16:50:36 -0700 Subject: [PATCH 3/7] ASoC: SOF: intel: hda: support also devices with 1 and 3 dmics Currently the dmic check code supports only devices with 2 or 4 dmics. With other dmic counts the function will return 0. Lately we've seen devices with only 1 dmic thus enable also configurations with 1, and possibly 3, dmics. Add also topology postfix -1ch and -3ch for new dmic configuration. Signed-off-by: Jaska Uimonen Reviewed-by: Pierre-Louis Bossart Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-4-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index b8157c1f37f3..0e8285b34a7d 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -544,7 +544,7 @@ static int check_nhlt_dmic(struct snd_sof_dev *sdev) if (nhlt) { dmic_num = intel_nhlt_get_dmic_geo(sdev->dev, nhlt); intel_nhlt_free(nhlt); - if (dmic_num == 2 || dmic_num == 4) + if (dmic_num >= 1 && dmic_num <= 4) return dmic_num; } @@ -992,9 +992,15 @@ static int hda_generic_machine_select(struct snd_sof_dev *sdev) dmic_num = hda_dmic_num; switch (dmic_num) { + case 1: + dmic_str = "-1ch"; + break; case 2: dmic_str = "-2ch"; break; + case 3: + dmic_str = "-3ch"; + break; case 4: dmic_str = "-4ch"; break; From 878694dcbe51794c7a68195b92a3707ed4ff5826 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 25 Aug 2020 16:50:37 -0700 Subject: [PATCH 4/7] ASoC: SOF: topology: fix the ipc_size calculation for process component The topology private struct is used for token parsing and its size should not be included to the ipc_size, fix it here though it didn't cause any real issue as the Firmware won't use this wrong-added data. Signed-off-by: Keyon Jie Reviewed-by: Guennadi Liakhovetski Reviewed-by: Jaska Uimonen Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-5-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/topology.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 707fbac3e64f..95e63d138326 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2114,9 +2114,7 @@ static int sof_process_load(struct snd_soc_component *scomp, int index, goto out; } - ipc_size = sizeof(struct sof_ipc_comp_process) + - le32_to_cpu(private->size) + - ipc_data_size; + ipc_size = sizeof(struct sof_ipc_comp_process) + ipc_data_size; /* we are exceeding max ipc size, config needs to be sent separately */ if (ipc_size > SOF_IPC_MSG_MAX_SIZE) { From 29c8e4398f02adacd429c7847dacc8aea5a0c2f1 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 25 Aug 2020 16:50:38 -0700 Subject: [PATCH 5/7] ASoC: SOF: Intel: hda: add extended rom status dump to error log Dump the extended ROM status information to the error logs to aid with remote support. The analysis of these logs requires access to non-public technical information. Reviewed-by: Kai Vehmanen Reviewed-by: Jaska Uimonen Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-6-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 0e8285b34a7d..de8e85920402 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -37,6 +37,7 @@ #include "shim.h" #define EXCEPT_MAX_HDR_SIZE 0x400 +#define HDA_EXT_ROM_STATUS_SIZE 8 #if IS_ENABLED(CONFIG_SND_SOC_SOF_INTEL_SOUNDWIRE) @@ -414,6 +415,22 @@ void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags) } } +/* dump the first 8 dwords representing the extended ROM status */ +static void hda_dsp_dump_ext_rom_status(struct snd_sof_dev *sdev) +{ + char msg[128]; + int len = 0; + u32 value; + int i; + + for (i = 0; i < HDA_EXT_ROM_STATUS_SIZE; i++) { + value = snd_sof_dsp_read(sdev, HDA_DSP_BAR, HDA_DSP_SRAM_REG_ROM_STATUS + i * 0x4); + len += snprintf(msg + len, sizeof(msg) - len, " 0x%x", value); + } + + dev_err(sdev->dev, "error: extended rom status:%s", msg); +} + void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags) { struct sof_ipc_dsp_oops_xtensa xoops; @@ -437,6 +454,7 @@ void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags) } else { dev_err(sdev->dev, "error: status = 0x%8.8x panic = 0x%8.8x\n", status, panic); + hda_dsp_dump_ext_rom_status(sdev); hda_dsp_get_status(sdev); } } From 17b3f99a360d76fed46e7ec1e12d6670bd7ca884 Mon Sep 17 00:00:00 2001 From: Iulian Olaru Date: Tue, 25 Aug 2020 16:50:39 -0700 Subject: [PATCH 6/7] ASoC: SOF: imx: Replace sdev->private with sdev->pdata->hw_pdata The correct way to save private data is to use sdev->pdata->hw_pdata. Removed superfluous type-casts. Signed-off-by: Iulian Olaru Reviewed-by: Daniel Baluta Reviewed-by: Pierre-Louis Bossart Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-7-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/imx/imx8.c | 10 +++++----- sound/soc/sof/imx/imx8m.c | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index bc0628c7b88c..325bf59e27c1 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -126,7 +126,7 @@ static struct imx_dsp_ops dsp_ops = { static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { - struct imx8_priv *priv = (struct imx8_priv *)sdev->private; + struct imx8_priv *priv = sdev->pdata->hw_pdata; sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); @@ -140,7 +140,7 @@ static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) */ static int imx8x_run(struct snd_sof_dev *sdev) { - struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private; + struct imx8_priv *dsp_priv = sdev->pdata->hw_pdata; int ret; ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, @@ -180,7 +180,7 @@ static int imx8x_run(struct snd_sof_dev *sdev) static int imx8_run(struct snd_sof_dev *sdev) { - struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private; + struct imx8_priv *dsp_priv = sdev->pdata->hw_pdata; int ret; ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, @@ -213,7 +213,7 @@ static int imx8_probe(struct snd_sof_dev *sdev) if (!priv) return -ENOMEM; - sdev->private = priv; + sdev->pdata->hw_pdata = priv; priv->dev = sdev->dev; priv->sdev = sdev; @@ -339,7 +339,7 @@ static int imx8_probe(struct snd_sof_dev *sdev) static int imx8_remove(struct snd_sof_dev *sdev) { - struct imx8_priv *priv = (struct imx8_priv *)sdev->private; + struct imx8_priv *priv = sdev->pdata->hw_pdata; int i; platform_device_unregister(priv->ipc_dev); diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c index 86320941fcee..c4f1ca939068 100644 --- a/sound/soc/sof/imx/imx8m.c +++ b/sound/soc/sof/imx/imx8m.c @@ -99,7 +99,7 @@ static struct imx_dsp_ops imx8m_dsp_ops = { static int imx8m_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { - struct imx8m_priv *priv = (struct imx8m_priv *)sdev->private; + struct imx8m_priv *priv = sdev->pdata->hw_pdata; sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); @@ -133,7 +133,7 @@ static int imx8m_probe(struct snd_sof_dev *sdev) if (!priv) return -ENOMEM; - sdev->private = priv; + sdev->pdata->hw_pdata = priv; priv->dev = sdev->dev; priv->sdev = sdev; @@ -209,7 +209,7 @@ static int imx8m_probe(struct snd_sof_dev *sdev) static int imx8m_remove(struct snd_sof_dev *sdev) { - struct imx8m_priv *priv = (struct imx8m_priv *)sdev->private; + struct imx8m_priv *priv = sdev->pdata->hw_pdata; platform_device_unregister(priv->ipc_dev); From 5a1fa00ad74bd5e77feab84a6d64c06f267df415 Mon Sep 17 00:00:00 2001 From: Iulian Olaru Date: Tue, 25 Aug 2020 16:50:40 -0700 Subject: [PATCH 7/7] ASoC: SOF: sof-of-dev: Add .arch_ops field Add .arch_ops field in the sof_imx8x_ops structure. The inclusion of this field will allow the usage of functions from sof/core.c in order to print debug information such as the registers and a stack dump in case of a firmware ops. The SND_SOC_SOF_XTENSA is added in the imx/Kconfig file so the compilation is successful. Signed-off-by: Iulian Olaru Reviewed-by: Pierre-Louis Bossart Reviewed-by: Daniel Baluta Signed-off-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200825235040.1586478-8-ranjani.sridharan@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/imx/Kconfig | 2 ++ sound/soc/sof/imx/imx8.c | 7 +++++++ sound/soc/sof/imx/imx8m.c | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index 8230285baa43..23bfd79d09c3 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -30,6 +30,7 @@ config SND_SOC_SOF_IMX8_SUPPORT config SND_SOC_SOF_IMX8 tristate + select SND_SOC_SOF_XTENSA help This option is not user-selectable but automagically handled by 'select' statements at a higher level @@ -44,6 +45,7 @@ config SND_SOC_SOF_IMX8M_SUPPORT config SND_SOC_SOF_IMX8M tristate + select SND_SOC_SOF_XTENSA help This option is not user-selectable but automagically handled by 'select' statements at a higher level diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c index 325bf59e27c1..3b9ffc760cb5 100644 --- a/sound/soc/sof/imx/imx8.c +++ b/sound/soc/sof/imx/imx8.c @@ -424,6 +424,9 @@ struct snd_sof_dsp_ops sof_imx8_ops = { /* firmware loading */ .load_firmware = snd_sof_load_firmware_memcpy, + /* Firmware ops */ + .arch_ops = &sof_xtensa_arch_ops, + /* DAI drivers */ .drv = imx8_dai, .num_drv = ARRAY_SIZE(imx8_dai), @@ -464,6 +467,9 @@ struct snd_sof_dsp_ops sof_imx8x_ops = { /* firmware loading */ .load_firmware = snd_sof_load_firmware_memcpy, + /* Firmware ops */ + .arch_ops = &sof_xtensa_arch_ops, + /* DAI drivers */ .drv = imx8_dai, .num_drv = ARRAY_SIZE(imx8_dai), @@ -477,4 +483,5 @@ struct snd_sof_dsp_ops sof_imx8x_ops = { }; EXPORT_SYMBOL(sof_imx8x_ops); +MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA); MODULE_LICENSE("Dual BSD/GPL"); diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c index c4f1ca939068..ca23ac99a63d 100644 --- a/sound/soc/sof/imx/imx8m.c +++ b/sound/soc/sof/imx/imx8m.c @@ -277,6 +277,9 @@ struct snd_sof_dsp_ops sof_imx8m_ops = { /* firmware loading */ .load_firmware = snd_sof_load_firmware_memcpy, + /* Firmware ops */ + .arch_ops = &sof_xtensa_arch_ops, + /* DAI drivers */ .drv = imx8m_dai, .num_drv = ARRAY_SIZE(imx8m_dai), @@ -289,4 +292,5 @@ struct snd_sof_dsp_ops sof_imx8m_ops = { }; EXPORT_SYMBOL(sof_imx8m_ops); +MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA); MODULE_LICENSE("Dual BSD/GPL");