From ddcccd543f5dbd841fe305452651b0f8c1d74f0f Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 26 May 2020 15:36:33 -0500 Subject: [PATCH 1/8] ASoC: SOF: Intel: byt: Add PM callbacks Add the PM callbacks for BYT/CHT platforms. Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Link: https://lore.kernel.org/r/20200526203640.25980-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/byt.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c index 3747f2c2c28b..457da8fcc6a0 100644 --- a/sound/soc/sof/intel/byt.c +++ b/sound/soc/sof/intel/byt.c @@ -428,6 +428,34 @@ static void byt_set_mach_params(const struct snd_soc_acpi_mach *mach, mach_params->platform = dev_name(dev); } +static void byt_reset_dsp_disable_int(struct snd_sof_dev *sdev) +{ + /* Disable Interrupt from both sides */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x3); + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x3); + + /* Put DSP into reset, set reset vector */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_CSR, + SHIM_BYT_CSR_RST | SHIM_BYT_CSR_VECTOR_SEL, + SHIM_BYT_CSR_RST | SHIM_BYT_CSR_VECTOR_SEL); +} + +static int byt_suspend(struct snd_sof_dev *sdev, u32 target_state) +{ + byt_reset_dsp_disable_int(sdev); + + return 0; +} + +static int byt_resume(struct snd_sof_dev *sdev) +{ + /* Enable Interrupt from both sides */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x0); + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x0); + + return 0; +} + /* Baytrail DAIs */ static struct snd_soc_dai_driver byt_dai[] = { { @@ -832,6 +860,10 @@ const struct snd_sof_dsp_ops sof_byt_ops = { /*Firmware loading */ .load_firmware = snd_sof_load_firmware_memcpy, + /* PM */ + .suspend = byt_suspend, + .resume = byt_resume, + /* DAI drivers */ .drv = byt_dai, .num_drv = 3, /* we have only 3 SSPs on byt*/ @@ -906,6 +938,10 @@ const struct snd_sof_dsp_ops sof_cht_ops = { /*Firmware loading */ .load_firmware = snd_sof_load_firmware_memcpy, + /* PM */ + .suspend = byt_suspend, + .resume = byt_resume, + /* DAI drivers */ .drv = byt_dai, /* all 6 SSPs may be available for cherrytrail */ From fc907cc527e6568b7486309188e545161891e1f2 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 26 May 2020 15:36:34 -0500 Subject: [PATCH 2/8] ASoC: SOF: pm: handle resume on legacy Intel platforms Add new case when set_power_state() is not supported, e.g. for Intel Baytrail/Cherrytrail legacy platforms. Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Link: https://lore.kernel.org/r/20200526203640.25980-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/pm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c index 5e804a7728f5..92e5f9b15f3a 100644 --- a/sound/soc/sof/pm.c +++ b/sound/soc/sof/pm.c @@ -114,8 +114,12 @@ static int sof_resume(struct device *dev, bool runtime_resume) return ret; } - /* Nothing further to do if resuming from a low-power D0 substate */ - if (!runtime_resume && old_state == SOF_DSP_PM_D0) + /* + * Nothing further to be done for platforms that support the low power + * D0 substate. + */ + if (!runtime_resume && sof_ops(sdev)->set_power_state && + old_state == SOF_DSP_PM_D0) return 0; sdev->fw_state = SOF_FW_BOOT_PREPARE; From 512e76724ffd07c6a5eb7d93c79d160e85465228 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 26 May 2020 15:36:35 -0500 Subject: [PATCH 3/8] ASoC: Intel: bytcr_rt5640/51: remove .ignore_suspend Low-power playback was never enabled on Baytrail devices, remove what looks like copy/paste from other machine drivers which were never submitted upstream. Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200526203640.25980-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/intel/boards/bytcr_rt5640.c | 4 ---- sound/soc/intel/boards/bytcr_rt5651.c | 3 --- 2 files changed, 7 deletions(-) diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index fbfd53874b47..d7e42bd9b308 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -910,9 +910,6 @@ static int byt_rt5640_init(struct snd_soc_pcm_runtime *runtime) if (ret) return ret; - snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); - snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); - if (byt_rt5640_quirk & BYT_RT5640_MCLK_EN) { /* * The firmware might enable the clock at @@ -1065,7 +1062,6 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = { .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, .be_hw_params_fixup = byt_rt5640_codec_fixup, - .ignore_suspend = 1, .nonatomic = true, .dpcm_playback = 1, .dpcm_capture = 1, diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 214ef41e23e6..0468fc35445f 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -601,8 +601,6 @@ static int byt_rt5651_init(struct snd_soc_pcm_runtime *runtime) dev_err(card->dev, "unable to add card controls\n"); return ret; } - snd_soc_dapm_ignore_suspend(&card->dapm, "Headphone"); - snd_soc_dapm_ignore_suspend(&card->dapm, "Speaker"); if (byt_rt5651_quirk & BYT_RT5651_MCLK_EN) { /* @@ -775,7 +773,6 @@ static struct snd_soc_dai_link byt_rt5651_dais[] = { .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBS_CFS, .be_hw_params_fixup = byt_rt5651_codec_fixup, - .ignore_suspend = 1, .nonatomic = true, .dpcm_playback = 1, .dpcm_capture = 1, From 68224376bc2a0508f57bff67c8dcd2b5761dc939 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 26 May 2020 15:36:36 -0500 Subject: [PATCH 4/8] ASoC: Intel: byt/cht: add .pm_ops Add required .pm_ops to support suspend/resume on baytrail/cherrytrail machines. This .pm_ops is conditionally-added to avoid impacting the legacy driver where power management is handled differently. Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200526203640.25980-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/intel/boards/bytcht_cx2072x.c | 3 +++ sound/soc/intel/boards/bytcht_da7213.c | 3 +++ sound/soc/intel/boards/bytcht_es8316.c | 3 +++ sound/soc/intel/boards/bytcr_rt5640.c | 3 +++ sound/soc/intel/boards/bytcr_rt5651.c | 3 +++ sound/soc/intel/boards/cht_bsw_max98090_ti.c | 3 +++ sound/soc/intel/boards/cht_bsw_nau8824.c | 3 +++ sound/soc/intel/boards/cht_bsw_rt5645.c | 3 +++ sound/soc/intel/boards/cht_bsw_rt5672.c | 3 +++ 9 files changed, 27 insertions(+) diff --git a/sound/soc/intel/boards/bytcht_cx2072x.c b/sound/soc/intel/boards/bytcht_cx2072x.c index c7f81a93d7c8..fad937610494 100644 --- a/sound/soc/intel/boards/bytcht_cx2072x.c +++ b/sound/soc/intel/boards/bytcht_cx2072x.c @@ -261,6 +261,9 @@ static int snd_byt_cht_cx2072x_probe(struct platform_device *pdev) static struct platform_driver snd_byt_cht_cx2072x_driver = { .driver = { .name = "bytcht_cx2072x", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_byt_cht_cx2072x_probe, }; diff --git a/sound/soc/intel/boards/bytcht_da7213.c b/sound/soc/intel/boards/bytcht_da7213.c index 5e96e7d02733..f3791ff2bad1 100644 --- a/sound/soc/intel/boards/bytcht_da7213.c +++ b/sound/soc/intel/boards/bytcht_da7213.c @@ -272,6 +272,9 @@ static int bytcht_da7213_probe(struct platform_device *pdev) static struct platform_driver bytcht_da7213_driver = { .driver = { .name = "bytcht_da7213", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = bytcht_da7213_probe, }; diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c index ddcd070100ef..9e5fc9430628 100644 --- a/sound/soc/intel/boards/bytcht_es8316.c +++ b/sound/soc/intel/boards/bytcht_es8316.c @@ -605,6 +605,9 @@ static int snd_byt_cht_es8316_mc_remove(struct platform_device *pdev) static struct platform_driver snd_byt_cht_es8316_mc_driver = { .driver = { .name = "bytcht_es8316", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_byt_cht_es8316_mc_probe, .remove = snd_byt_cht_es8316_mc_remove, diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c index d7e42bd9b308..30f70bbdf89c 100644 --- a/sound/soc/intel/boards/bytcr_rt5640.c +++ b/sound/soc/intel/boards/bytcr_rt5640.c @@ -1319,6 +1319,9 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev) static struct platform_driver snd_byt_rt5640_mc_driver = { .driver = { .name = "bytcr_rt5640", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_byt_rt5640_mc_probe, }; diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 0468fc35445f..520e916e329c 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -1097,6 +1097,9 @@ static int snd_byt_rt5651_mc_probe(struct platform_device *pdev) static struct platform_driver snd_byt_rt5651_mc_driver = { .driver = { .name = "bytcr_rt5651", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_byt_rt5651_mc_probe, }; diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c index 135701738a44..767ac2ae03e2 100644 --- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c +++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c @@ -616,6 +616,9 @@ static int snd_cht_mc_remove(struct platform_device *pdev) static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-max98090", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_cht_mc_probe, .remove = snd_cht_mc_remove, diff --git a/sound/soc/intel/boards/cht_bsw_nau8824.c b/sound/soc/intel/boards/cht_bsw_nau8824.c index 67b46de2f088..2f7c94d335c1 100644 --- a/sound/soc/intel/boards/cht_bsw_nau8824.c +++ b/sound/soc/intel/boards/cht_bsw_nau8824.c @@ -282,6 +282,9 @@ static int snd_cht_mc_probe(struct platform_device *pdev) static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-nau8824", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_cht_mc_probe, }; diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c index e64eca56e426..22de138ffa33 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5645.c +++ b/sound/soc/intel/boards/cht_bsw_rt5645.c @@ -680,6 +680,9 @@ static int snd_cht_mc_probe(struct platform_device *pdev) static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-rt5645", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_cht_mc_probe, }; diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c index 097023a3ec14..7a43c70a1378 100644 --- a/sound/soc/intel/boards/cht_bsw_rt5672.c +++ b/sound/soc/intel/boards/cht_bsw_rt5672.c @@ -459,6 +459,9 @@ static int snd_cht_mc_probe(struct platform_device *pdev) static struct platform_driver snd_cht_mc_driver = { .driver = { .name = "cht-bsw-rt5672", +#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL) + .pm = &snd_soc_pm_ops, +#endif }, .probe = snd_cht_mc_probe, }; From d7a1ed268993f4bc758fa509b22fc730af1623f9 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 26 May 2020 15:36:37 -0500 Subject: [PATCH 5/8] ASoC: SOF: ipc: ignore DSP replies received when they are not expected We currently ignore the reply messages from the DSP when they are not expected but call it out as an error. Change the error message to a debug message. Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Link: https://lore.kernel.org/r/20200526203640.25980-6-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/ipc.c | 9 ++++----- sound/soc/sof/sof-priv.h | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index f7a0353596fb..36e2d4d43da4 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -335,21 +335,20 @@ int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header, EXPORT_SYMBOL(sof_ipc_tx_message_no_pm); /* handle reply message from DSP */ -int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) +void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id) { struct snd_sof_ipc_msg *msg = &sdev->ipc->msg; if (msg->ipc_complete) { - dev_err(sdev->dev, "error: no reply expected, received 0x%x", + dev_dbg(sdev->dev, + "no reply expected, received 0x%x, will be ignored", msg_id); - return -EINVAL; + return; } /* wake up and return the error if we have waiters on this message ? */ msg->ipc_complete = true; wake_up(&msg->waitq); - - return 0; } EXPORT_SYMBOL(snd_sof_ipc_reply); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 3ed39b887214..64f28e082049 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -480,7 +480,7 @@ int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset); */ struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev); void snd_sof_ipc_free(struct snd_sof_dev *sdev); -int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id); +void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id); void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev); int snd_sof_ipc_stream_pcm_params(struct snd_sof_dev *sdev, struct sof_ipc_pcm_params *params); From c691f0c6e267da4207392b1082d011323c3f8606 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Tue, 26 May 2020 15:36:38 -0500 Subject: [PATCH 6/8] ASoC: SOF: Intel: BYT: add .remove op Add .remove op that disables interrupts and reset the DSP for BYT and CHT platforms. Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Link: https://lore.kernel.org/r/20200526203640.25980-7-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/byt.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c index 457da8fcc6a0..8784c7319098 100644 --- a/sound/soc/sof/intel/byt.c +++ b/sound/soc/sof/intel/byt.c @@ -456,6 +456,13 @@ static int byt_resume(struct snd_sof_dev *sdev) return 0; } +static int byt_remove(struct snd_sof_dev *sdev) +{ + byt_reset_dsp_disable_int(sdev); + + return 0; +} + /* Baytrail DAIs */ static struct snd_soc_dai_driver byt_dai[] = { { @@ -811,6 +818,7 @@ static int byt_acpi_probe(struct snd_sof_dev *sdev) const struct snd_sof_dsp_ops sof_byt_ops = { /* device init */ .probe = byt_acpi_probe, + .remove = byt_remove, /* DSP core boot / reset */ .run = byt_run, @@ -889,6 +897,7 @@ EXPORT_SYMBOL_NS(byt_chip_info, SND_SOC_SOF_BAYTRAIL); const struct snd_sof_dsp_ops sof_cht_ops = { /* device init */ .probe = byt_acpi_probe, + .remove = byt_remove, /* DSP core boot / reset */ .run = byt_run, From 3d3d1fb9ce34bc045b9d140a5f2ec531eff6a0fe Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 26 May 2020 15:36:39 -0500 Subject: [PATCH 7/8] ASoC: SOF: Intel: BYT: mask BUSY or DONE interrupts in handler The DSP may send the same interrupt multiple times before it's handled in the interrupt thread. Rather than masking it in the thread, mask it in the handler directly. This patch also removes useless checks that cannot happen, and masks that are set don't need to be re-tested. Suggested-by: Keyon Jie Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Reviewed-by: Ranjani Sridharan BugLink: https://github.com/thesofproject/linux/issues/1492 Link: https://lore.kernel.org/r/20200526203640.25980-8-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/byt.c | 46 +++++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c index 8784c7319098..c6ac914ee56c 100644 --- a/sound/soc/sof/intel/byt.c +++ b/sound/soc/sof/intel/byt.c @@ -160,13 +160,31 @@ static void byt_dump(struct snd_sof_dev *sdev, u32 flags) static irqreturn_t byt_irq_handler(int irq, void *context) { struct snd_sof_dev *sdev = context; - u64 isr; + u64 ipcx, ipcd; int ret = IRQ_NONE; - /* Interrupt arrived, check src */ - isr = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_ISRX); - if (isr & (SHIM_ISRX_DONE | SHIM_ISRX_BUSY)) + ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX); + ipcd = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCD); + + if (ipcx & SHIM_BYT_IPCX_DONE) { + + /* reply message from DSP, Mask Done interrupt first */ + snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, + SHIM_IMRX, + SHIM_IMRX_DONE, + SHIM_IMRX_DONE); ret = IRQ_WAKE_THREAD; + } + + if (ipcd & SHIM_BYT_IPCD_BUSY) { + + /* new message from DSP, Mask Busy interrupt first */ + snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, + SHIM_IMRX, + SHIM_IMRX_BUSY, + SHIM_IMRX_BUSY); + ret = IRQ_WAKE_THREAD; + } return ret; } @@ -175,19 +193,12 @@ static irqreturn_t byt_irq_thread(int irq, void *context) { struct snd_sof_dev *sdev = context; u64 ipcx, ipcd; - u64 imrx; - imrx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IMRX); ipcx = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCX); + ipcd = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCD); /* reply message from DSP */ - if (ipcx & SHIM_BYT_IPCX_DONE && - !(imrx & SHIM_IMRX_DONE)) { - /* Mask Done interrupt before first */ - snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, - SHIM_IMRX, - SHIM_IMRX_DONE, - SHIM_IMRX_DONE); + if (ipcx & SHIM_BYT_IPCX_DONE) { spin_lock_irq(&sdev->ipc_lock); @@ -207,14 +218,7 @@ static irqreturn_t byt_irq_thread(int irq, void *context) } /* new message from DSP */ - ipcd = snd_sof_dsp_read64(sdev, BYT_DSP_BAR, SHIM_IPCD); - if (ipcd & SHIM_BYT_IPCD_BUSY && - !(imrx & SHIM_IMRX_BUSY)) { - /* Mask Busy interrupt before return */ - snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, - SHIM_IMRX, - SHIM_IMRX_BUSY, - SHIM_IMRX_BUSY); + if (ipcd & SHIM_BYT_IPCD_BUSY) { /* Handle messages from DSP Core */ if ((ipcd & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { From 3d2e5c480742b4a22534e72e2647b6c8c98a94a4 Mon Sep 17 00:00:00 2001 From: Keyon Jie Date: Tue, 26 May 2020 15:36:40 -0500 Subject: [PATCH 8/8] ASoC: SOF: Intel: BYT: harden IPC initialization and handling On probe and reset, we should not touch the SHIM_IMRD register since it is configured by firmware. The driver only configures SHIM_IMRX with the BUSY interrupt enabled by default and DONE interrupt disabled. When sending an IPC message, the DONE interrupt is enabled until the DSP response is provided. This sequence hardens the IPC communication and avoid interrupt-related issues when adding/removing modules or during system suspend-resume transitions. Signed-off-by: Keyon Jie Signed-off-by: Pierre-Louis Bossart Tested-by: Enric Balletbo i Serra Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200526203640.25980-9-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/byt.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c index c6ac914ee56c..e6ba8382b1de 100644 --- a/sound/soc/sof/intel/byt.c +++ b/sound/soc/sof/intel/byt.c @@ -236,6 +236,10 @@ static irqreturn_t byt_irq_thread(int irq, void *context) static int byt_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) { + /* unmask and prepare to receive Done interrupt */ + snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX, + SHIM_IMRX_DONE, 0); + /* send the message */ sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); @@ -301,7 +305,7 @@ static void byt_host_done(struct snd_sof_dev *sdev) SHIM_BYT_IPCD_DONE, SHIM_BYT_IPCD_DONE); - /* unmask busy interrupt */ + /* unmask and prepare to receive next new message */ snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX, SHIM_IMRX_BUSY, 0); } @@ -311,10 +315,6 @@ static void byt_dsp_done(struct snd_sof_dev *sdev) /* clear DONE bit - tell DSP we have completed */ snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IPCX, SHIM_BYT_IPCX_DONE, 0); - - /* unmask Done interrupt */ - snd_sof_dsp_update_bits64_unlocked(sdev, BYT_DSP_BAR, SHIM_IMRX, - SHIM_IMRX_DONE, 0); } /* @@ -453,9 +453,10 @@ static int byt_suspend(struct snd_sof_dev *sdev, u32 target_state) static int byt_resume(struct snd_sof_dev *sdev) { - /* Enable Interrupt from both sides */ - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x0); - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x0); + /* enable BUSY and disable DONE Interrupt by default */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, + SHIM_IMRX_BUSY | SHIM_IMRX_DONE, + SHIM_IMRX_DONE); return 0; } @@ -606,9 +607,10 @@ static int tangier_pci_probe(struct snd_sof_dev *sdev) return ret; } - /* enable Interrupt from both sides */ - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x0); - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x0); + /* enable BUSY and disable DONE Interrupt by default */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, + SHIM_IMRX_BUSY | SHIM_IMRX_DONE, + SHIM_IMRX_DONE); /* set default mailbox offset for FW ready message */ sdev->dsp_box.offset = MBOX_OFFSET; @@ -808,9 +810,10 @@ static int byt_acpi_probe(struct snd_sof_dev *sdev) return ret; } - /* enable Interrupt from both sides */ - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, 0x3, 0x0); - snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRD, 0x3, 0x0); + /* enable BUSY and disable DONE Interrupt by default */ + snd_sof_dsp_update_bits64(sdev, BYT_DSP_BAR, SHIM_IMRX, + SHIM_IMRX_BUSY | SHIM_IMRX_DONE, + SHIM_IMRX_DONE); /* set default mailbox offset for FW ready message */ sdev->dsp_box.offset = MBOX_OFFSET;