[PATCH 0/5] ASoC: SOF: Intel: don't download firmware at each resume
After the first firmware boot, the firmware is capable of saving/restoring its context to/from IMR (Isolated Memory Region, set aside by BIOS on startup). This capability improves the resume speed.
Due to an unexplained issue on Up2 boards, this capability is disabled on ApolloLake.
For backwards compatibility, the regular boot flow is used with older firmware. For added peace of mind, a kernel module parameter is provided to force the regular boot flow - this shouldn't be necessary since we've been testing these patches for 6+ months.
Keyon Jie (4): ASoC: SOF: add _D3_PERSISTENT flag to fw_ready message ASoC: SOF: Intel: hda-loader: add SSP helper ASoC: SOF: Intel: hda-loader: add IMR restore support ASoC: SOF: add flag to disable IMR restore to sof_debug
Pierre-Louis Bossart (1): ASoC: SOF: Intel: use inclusive language for SSP clocks
include/sound/sof/info.h | 1 + include/uapi/sound/sof/abi.h | 2 +- sound/soc/sof/intel/hda-loader.c | 68 +++++++++++++++++++++++++++----- sound/soc/sof/intel/hda.h | 6 +-- sound/soc/sof/sof-priv.h | 3 ++ 5 files changed, 66 insertions(+), 14 deletions(-)
From: Keyon Jie yang.jie@linux.intel.com
Add a bit definition to the fw_ready message, to denote if the FW supports the IMR (Isolated Memory Region) restoring feature.
If the bit is set, the driver can skip downloading the firmware again during system resume or runtime resume.
Bump the ABI version to 3.19 to make it aligned with FW side.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- include/sound/sof/info.h | 1 + include/uapi/sound/sof/abi.h | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/sound/sof/info.h b/include/sound/sof/info.h index 0b7101aef596..65e86e4e9fd8 100644 --- a/include/sound/sof/info.h +++ b/include/sound/sof/info.h @@ -25,6 +25,7 @@ #define SOF_IPC_INFO_LOCKS BIT(1) #define SOF_IPC_INFO_LOCKSV BIT(2) #define SOF_IPC_INFO_GDB BIT(3) +#define SOF_IPC_INFO_D3_PERSISTENT BIT(4)
/* extended data types that can be appended onto end of sof_ipc_fw_ready */ enum sof_ipc_ext_data { diff --git a/include/uapi/sound/sof/abi.h b/include/uapi/sound/sof/abi.h index fe2cfae94b45..f4232d289a22 100644 --- a/include/uapi/sound/sof/abi.h +++ b/include/uapi/sound/sof/abi.h @@ -26,7 +26,7 @@
/* SOF ABI version major, minor and patch numbers */ #define SOF_ABI_MAJOR 3 -#define SOF_ABI_MINOR 18 +#define SOF_ABI_MINOR 19 #define SOF_ABI_PATCH 0
/* SOF ABI version number. Format within 32bit word is MMmmmppp */
We introduced provider/consumer terms, and CBP_CFP acronyms for codec drivers, let's use them as well in SOF.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 6 +++--- sound/soc/sof/intel/hda.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 33306d2023a7..43de6f1d62a9 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -101,14 +101,14 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) goto err; }
- /* DSP is powered up, set all SSPs to slave mode */ + /* DSP is powered up, set all SSPs to clock consumer/codec provider mode */ for (i = 0; i < chip->ssp_count; i++) { snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR, chip->ssp_base_offset + i * SSP_DEV_MEM_SIZE + SSP_SSC1_OFFSET, - SSP_SET_SLAVE, - SSP_SET_SLAVE); + SSP_SET_CBP_CFP, + SSP_SET_CBP_CFP); }
/* step 2: purge FW request */ diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 03a6bb7a165c..7838a998ea95 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -383,9 +383,9 @@
/* SSP Registers */ #define SSP_SSC1_OFFSET 0x4 -#define SSP_SET_SCLK_SLAVE BIT(25) -#define SSP_SET_SFRM_SLAVE BIT(24) -#define SSP_SET_SLAVE (SSP_SET_SCLK_SLAVE | SSP_SET_SFRM_SLAVE) +#define SSP_SET_SCLK_CONSUMER BIT(25) +#define SSP_SET_SFRM_CONSUMER BIT(24) +#define SSP_SET_CBP_CFP (SSP_SET_SCLK_CONSUMER | SSP_SET_SFRM_CONSUMER)
#define HDA_IDISP_ADDR 2 #define HDA_IDISP_CODEC(x) ((x) & BIT(HDA_IDISP_ADDR))
From: Keyon Jie yang.jie@linux.intel.com
Move the SSP clock configuration to the hda_set_ssp_cbp_cfp() helper, to be used in follow-up patches
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 43de6f1d62a9..7f1b1d0f2422 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -25,6 +25,23 @@
#define HDA_CL_STREAM_FORMAT 0x40
+static void hda_ssp_set_cbp_cfp(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + const struct sof_intel_dsp_desc *chip = hda->desc; + int i; + + /* DSP is powered up, set all SSPs to clock consumer/codec provider mode */ + for (i = 0; i < chip->ssp_count; i++) { + snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR, + chip->ssp_base_offset + + i * SSP_DEV_MEM_SIZE + + SSP_SSC1_OFFSET, + SSP_SET_CBP_CFP, + SSP_SET_CBP_CFP); + } +} + static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsigned int format, unsigned int size, struct snd_dma_buffer *dmab, int direction) @@ -91,7 +108,6 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) char *dump_msg; u32 flags, j; int ret; - int i;
/* step 1: power up corex */ ret = hda_dsp_enable_core(sdev, chip->host_managed_cores_mask); @@ -101,15 +117,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) goto err; }
- /* DSP is powered up, set all SSPs to clock consumer/codec provider mode */ - for (i = 0; i < chip->ssp_count; i++) { - snd_sof_dsp_update_bits_unlocked(sdev, HDA_DSP_BAR, - chip->ssp_base_offset - + i * SSP_DEV_MEM_SIZE - + SSP_SSC1_OFFSET, - SSP_SET_CBP_CFP, - SSP_SET_CBP_CFP); - } + hda_ssp_set_cbp_cfp(sdev);
/* step 2: purge FW request */ snd_sof_dsp_write(sdev, HDA_DSP_BAR, chip->ipc_req,
From: Keyon Jie yang.jie@linux.intel.com
If the firmware declares the IMR restore feature, we only need to do a simple powering up to resume from D3, no firmware re-downloading needed - the context is saved/restored to/from IMR without needing driver support.
Add a hda_dsp_boot_imr() helper for this simple DSP reboot, and use it when it is available.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 7f1b1d0f2422..baf2ff146f50 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -353,6 +353,38 @@ int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev) return ret; }
+static int hda_dsp_boot_imr(struct snd_sof_dev *sdev) +{ + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + const struct sof_intel_dsp_desc *chip = hda->desc; + unsigned long mask; + u32 j; + int ret; + + /* power up & unstall/run the cores to run the firmware */ + ret = hda_dsp_enable_core(sdev, chip->init_core_mask); + if (ret < 0) { + dev_err(sdev->dev, "dsp core start failed %d\n", ret); + return -EIO; + } + + /* set enabled cores mask and increment ref count for cores in init_core_mask */ + sdev->enabled_cores_mask |= chip->init_core_mask; + mask = sdev->enabled_cores_mask; + for_each_set_bit(j, &mask, SOF_MAX_DSP_NUM_CORES) + sdev->dsp_core_ref_count[j]++; + + hda_ssp_set_cbp_cfp(sdev); + + /* enable IPC interrupts */ + hda_dsp_ipc_int_enable(sdev); + + /* process wakes */ + hda_sdw_process_wakeen(sdev); + + return ret; +} + int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; @@ -363,6 +395,12 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) struct firmware stripped_firmware; int ret, ret1, i;
+ if ((sdev->fw_ready.flags & SOF_IPC_INFO_D3_PERSISTENT) && + !sdev->first_boot) { + dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n"); + return hda_dsp_boot_imr(sdev); + } + chip_info = desc->chip_info;
if (plat_data->fw->size <= plat_data->fw_offset) {
From: Keyon Jie yang.jie@linux.intel.com
Add flag _IGNORE_D3_PERSISTENT to disable IMR restore feature to the sof_debug module parameter.
The IMR restore feature will be enabled for all Intel cAVS platforms by default, but setting the flag _IGNORE_D3_PERSISTENT can help to disable the feature for debug purpose, to rule out any possible regression introduced by the change of not re-downloading firmware to the DSP at resuming from suspended state.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Keyon Jie yang.jie@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-loader.c | 2 ++ sound/soc/sof/sof-priv.h | 3 +++ 2 files changed, 5 insertions(+)
diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index baf2ff146f50..6af3325b7e40 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -21,6 +21,7 @@ #include <sound/sof.h> #include "ext_manifest.h" #include "../ops.h" +#include "../sof-priv.h" #include "hda.h"
#define HDA_CL_STREAM_FORMAT 0x40 @@ -396,6 +397,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) int ret, ret1, i;
if ((sdev->fw_ready.flags & SOF_IPC_INFO_D3_PERSISTENT) && + !(sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT)) && !sdev->first_boot) { dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n"); return hda_dsp_boot_imr(sdev); diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h index 087935192ce8..29bb56b7267a 100644 --- a/sound/soc/sof/sof-priv.h +++ b/sound/soc/sof/sof-priv.h @@ -34,6 +34,9 @@ * on primary core */ #define SOF_DBG_PRINT_ALL_DUMPS BIT(6) /* Print all ipc and dsp dumps */ +#define SOF_DBG_IGNORE_D3_PERSISTENT BIT(7) /* ignore the DSP D3 persistent capability + * and always download firmware upon D3 exit + */
/* Flag definitions used for controlling the DSP dump behavior */ #define SOF_DBG_DUMP_REGS BIT(0)
On Thu, 20 Jan 2022 17:15:27 -0600, Pierre-Louis Bossart wrote:
After the first firmware boot, the firmware is capable of saving/restoring its context to/from IMR (Isolated Memory Region, set aside by BIOS on startup). This capability improves the resume speed.
Due to an unexplained issue on Up2 boards, this capability is disabled on ApolloLake.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] ASoC: SOF: add _D3_PERSISTENT flag to fw_ready message commit: 1dafede34dda19fc2878724145dc16c0b51dc174 [2/5] ASoC: SOF: Intel: use inclusive language for SSP clocks commit: bd586a0292e0724fac571a2e4b629134ab2c686c [3/5] ASoC: SOF: Intel: hda-loader: add SSP helper commit: a749d744561ccc8658cebe23fc284034a57e6ceb [4/5] ASoC: SOF: Intel: hda-loader: add IMR restore support commit: 5fb5f51185126059e1d7eb4e452e08c7b17c3301 [5/5] ASoC: SOF: add flag to disable IMR restore to sof_debug commit: d7a8fbd17bfef174e85d81d94507b8015732a58e
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 -
Pierre-Louis Bossart