[alsa-devel] [PATCH 0/6] ASoC: Intel: Skylake: Driver updates on DSP firmware
From: Jeeja KP jeeja.kp@intel.com
This patch series provides update on DSP firmware download by optimizing o updating the dsp register poll method to use accurate timeout. o optimizing ROM init retries. o saving the firmware/library context at boot and use this ctx for downloading the firmware to DSP memory. o cleanup to store the library manifest data o release the firmware in cleanup routine.
Jeeja KP (6): ASoC: Intel: Common: Update dsp register poll implementation ASoC: Intel: bxtn: Use DSP poll API to poll FW status ASoC: Intel: Skylake: Clean up manifest info ASoC: Intel: Skylake: Release FW ctx in cleanup ASoC: Intel: bxtn: Fix to store the FW/Library context at boot ASoC: Intel: bxtn: optimize ROM init retries
sound/soc/intel/common/sst-dsp.c | 52 +++++----- sound/soc/intel/skylake/bxt-sst.c | 142 ++++++++++++++------------- sound/soc/intel/skylake/skl-sst-dsp.h | 4 +- sound/soc/intel/skylake/skl-sst-ipc.h | 5 +- sound/soc/intel/skylake/skl-sst.c | 3 + sound/soc/intel/skylake/skl-topology.c | 41 ++++---- sound/soc/intel/skylake/skl-topology.h | 13 +++ sound/soc/intel/skylake/skl-tplg-interface.h | 12 --- 8 files changed, 137 insertions(+), 135 deletions(-)
From: Jeeja KP jeeja.kp@intel.com
Poll implementation is not quite accurate, especially for smaller values of timeout or timeout values close to the actual timeout needed
Use jiffies to set the timeout value and time_before() to get the accurate time. So update the dsp register poll implementation to provide accurate timeout using jiffies.
Signed-off-by: Jayachandran B jayachandran.b@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/common/sst-dsp.c | 52 ++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index c00ede4..11c0805 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -252,44 +252,44 @@ void sst_dsp_shim_update_bits_forced(struct sst_dsp *sst, u32 offset, EXPORT_SYMBOL_GPL(sst_dsp_shim_update_bits_forced);
int sst_dsp_register_poll(struct sst_dsp *ctx, u32 offset, u32 mask, - u32 target, u32 timeout, char *operation) + u32 target, u32 time, char *operation) { - int time, ret; u32 reg; - bool done = false; + unsigned long timeout; + int k = 0, s = 500;
/* - * we will poll for couple of ms using mdelay, if not successful - * then go to longer sleep using usleep_range + * split the loop into sleeps of varying resolution. more accurately, + * the range of wakeups are: + * Phase 1(first 5ms): min sleep 0.5ms; max sleep 1ms. + * Phase 2:( 5ms to 10ms) : min sleep 0.5ms; max sleep 10ms + * (usleep_range (500, 1000) and usleep_range(5000, 10000) are + * both possible in this phase depending on whether k > 10 or not). + * Phase 3: (beyond 10 ms) min sleep 5ms; max sleep 10ms. */
- /* check if set state successful */ - for (time = 0; time < 5; time++) { - if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == target) { - done = true; - break; - } - mdelay(1); + timeout = jiffies + msecs_to_jiffies(time); + while (((sst_dsp_shim_read_unlocked(ctx, offset) & mask) != target) + && time_before(jiffies, timeout)) { + k++; + if (k > 10) + s = 5000; + + usleep_range(s, 2*s); }
- if (done == false) { - /* sleeping in 10ms steps so adjust timeout value */ - timeout /= 10; + reg = sst_dsp_shim_read_unlocked(ctx, offset);
- for (time = 0; time < timeout; time++) { - if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == target) - break; + if ((reg & mask) == target) { + dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s successful\n", + reg, operation);
- usleep_range(5000, 10000); - } + return 0; }
- reg = sst_dsp_shim_read_unlocked(ctx, offset); - dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s %s\n", reg, operation, - (time < timeout) ? "successful" : "timedout"); - ret = time < timeout ? 0 : -ETIME; - - return ret; + dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s timedout\n", + reg, operation); + return -ETIME; } EXPORT_SYMBOL_GPL(sst_dsp_register_poll);
The patch
ASoC: Intel: Common: Update dsp register poll implementation
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 9cc8f9fe0f9e84771f2872f8939d37414ef9d58d Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Mon, 2 Jan 2017 09:50:02 +0530 Subject: [PATCH] ASoC: Intel: Common: Update dsp register poll implementation
Poll implementation is not quite accurate, especially for smaller values of timeout or timeout values close to the actual timeout needed
Use jiffies to set the timeout value and time_before() to get the accurate time. So update the dsp register poll implementation to provide accurate timeout using jiffies.
Signed-off-by: Jayachandran B jayachandran.b@intel.com Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/common/sst-dsp.c | 52 ++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/sound/soc/intel/common/sst-dsp.c b/sound/soc/intel/common/sst-dsp.c index c00ede4ea4d7..11c0805393ff 100644 --- a/sound/soc/intel/common/sst-dsp.c +++ b/sound/soc/intel/common/sst-dsp.c @@ -252,44 +252,44 @@ void sst_dsp_shim_update_bits_forced(struct sst_dsp *sst, u32 offset, EXPORT_SYMBOL_GPL(sst_dsp_shim_update_bits_forced);
int sst_dsp_register_poll(struct sst_dsp *ctx, u32 offset, u32 mask, - u32 target, u32 timeout, char *operation) + u32 target, u32 time, char *operation) { - int time, ret; u32 reg; - bool done = false; + unsigned long timeout; + int k = 0, s = 500;
/* - * we will poll for couple of ms using mdelay, if not successful - * then go to longer sleep using usleep_range + * split the loop into sleeps of varying resolution. more accurately, + * the range of wakeups are: + * Phase 1(first 5ms): min sleep 0.5ms; max sleep 1ms. + * Phase 2:( 5ms to 10ms) : min sleep 0.5ms; max sleep 10ms + * (usleep_range (500, 1000) and usleep_range(5000, 10000) are + * both possible in this phase depending on whether k > 10 or not). + * Phase 3: (beyond 10 ms) min sleep 5ms; max sleep 10ms. */
- /* check if set state successful */ - for (time = 0; time < 5; time++) { - if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == target) { - done = true; - break; - } - mdelay(1); + timeout = jiffies + msecs_to_jiffies(time); + while (((sst_dsp_shim_read_unlocked(ctx, offset) & mask) != target) + && time_before(jiffies, timeout)) { + k++; + if (k > 10) + s = 5000; + + usleep_range(s, 2*s); }
- if (done == false) { - /* sleeping in 10ms steps so adjust timeout value */ - timeout /= 10; + reg = sst_dsp_shim_read_unlocked(ctx, offset);
- for (time = 0; time < timeout; time++) { - if ((sst_dsp_shim_read_unlocked(ctx, offset) & mask) == target) - break; + if ((reg & mask) == target) { + dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s successful\n", + reg, operation);
- usleep_range(5000, 10000); - } + return 0; }
- reg = sst_dsp_shim_read_unlocked(ctx, offset); - dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s %s\n", reg, operation, - (time < timeout) ? "successful" : "timedout"); - ret = time < timeout ? 0 : -ETIME; - - return ret; + dev_dbg(ctx->dev, "FW Poll Status: reg=%#x %s timedout\n", + reg, operation); + return -ETIME; } EXPORT_SYMBOL_GPL(sst_dsp_register_poll);
From: Jeeja KP jeeja.kp@intel.com
Use the optimized dsp_register_poll API to poll the DSP firmware status register rather than open coding it.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/bxt-sst.c | 39 ++++++++++----------------------------- 1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 1f9f33d..d959643 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -153,23 +153,13 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx, }
/* Step 4: Wait for DONE Bit */ - for (i = BXT_INIT_TIMEOUT; i > 0; --i) { - reg = sst_dsp_shim_read(ctx, SKL_ADSP_REG_HIPCIE); - - if (reg & SKL_ADSP_REG_HIPCIE_DONE) { - sst_dsp_shim_update_bits_forced(ctx, - SKL_ADSP_REG_HIPCIE, + ret = sst_dsp_register_poll(ctx, SKL_ADSP_REG_HIPCIE, SKL_ADSP_REG_HIPCIE_DONE, - SKL_ADSP_REG_HIPCIE_DONE); - break; - } - mdelay(1); - } - if (!i) { - dev_info(ctx->dev, "Waiting for HIPCIE done, reg: 0x%x\n", reg); - sst_dsp_shim_update_bits(ctx, SKL_ADSP_REG_HIPCIE, - SKL_ADSP_REG_HIPCIE_DONE, - SKL_ADSP_REG_HIPCIE_DONE); + SKL_ADSP_REG_HIPCIE_DONE, + BXT_INIT_TIMEOUT, "HIPCIE Done"); + if (ret < 0) { + dev_err(ctx->dev, "Timout for Purge Request%d\n", ret); + goto base_fw_load_failed; }
/* Step 5: power down core1 */ @@ -184,19 +174,10 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx, skl_ipc_op_int_enable(ctx);
/* Step 7: Wait for ROM init */ - for (i = BXT_INIT_TIMEOUT; i > 0; --i) { - if (SKL_FW_INIT == - (sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS) & - SKL_FW_STS_MASK)) { - - dev_info(ctx->dev, "ROM loaded, continue FW loading\n"); - break; - } - mdelay(1); - } - if (!i) { - dev_err(ctx->dev, "Timeout for ROM init, HIPCIE: 0x%x\n", reg); - ret = -EIO; + ret = sst_dsp_register_poll(ctx, BXT_ADSP_FW_STATUS, SKL_FW_STS_MASK, + SKL_FW_INIT, BXT_INIT_TIMEOUT, "ROM Load"); + if (ret < 0) { + dev_err(ctx->dev, "Timeout for ROM init, ret:%d\n", ret); goto base_fw_load_failed; }
The patch
ASoC: Intel: bxtn: Use DSP poll API to poll FW status
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 1448099dd3d55546057cdda0493a6493c007b9fd Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Mon, 2 Jan 2017 09:50:03 +0530 Subject: [PATCH] ASoC: Intel: bxtn: Use DSP poll API to poll FW status
Use the optimized dsp_register_poll API to poll the DSP firmware status register rather than open coding it.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/bxt-sst.c | 39 ++++++++++----------------------------- 1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index e4a382870132..15a063a403cc 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -151,23 +151,13 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx, }
/* Step 4: Wait for DONE Bit */ - for (i = BXT_INIT_TIMEOUT; i > 0; --i) { - reg = sst_dsp_shim_read(ctx, SKL_ADSP_REG_HIPCIE); - - if (reg & SKL_ADSP_REG_HIPCIE_DONE) { - sst_dsp_shim_update_bits_forced(ctx, - SKL_ADSP_REG_HIPCIE, + ret = sst_dsp_register_poll(ctx, SKL_ADSP_REG_HIPCIE, SKL_ADSP_REG_HIPCIE_DONE, - SKL_ADSP_REG_HIPCIE_DONE); - break; - } - mdelay(1); - } - if (!i) { - dev_info(ctx->dev, "Waiting for HIPCIE done, reg: 0x%x\n", reg); - sst_dsp_shim_update_bits(ctx, SKL_ADSP_REG_HIPCIE, - SKL_ADSP_REG_HIPCIE_DONE, - SKL_ADSP_REG_HIPCIE_DONE); + SKL_ADSP_REG_HIPCIE_DONE, + BXT_INIT_TIMEOUT, "HIPCIE Done"); + if (ret < 0) { + dev_err(ctx->dev, "Timout for Purge Request%d\n", ret); + goto base_fw_load_failed; }
/* Step 5: power down core1 */ @@ -182,19 +172,10 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx, skl_ipc_op_int_enable(ctx);
/* Step 7: Wait for ROM init */ - for (i = BXT_INIT_TIMEOUT; i > 0; --i) { - if (SKL_FW_INIT == - (sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS) & - SKL_FW_STS_MASK)) { - - dev_info(ctx->dev, "ROM loaded, continue FW loading\n"); - break; - } - mdelay(1); - } - if (!i) { - dev_err(ctx->dev, "Timeout for ROM init, HIPCIE: 0x%x\n", reg); - ret = -EIO; + ret = sst_dsp_register_poll(ctx, BXT_ADSP_FW_STATUS, SKL_FW_STS_MASK, + SKL_FW_INIT, BXT_INIT_TIMEOUT, "ROM Load"); + if (ret < 0) { + dev_err(ctx->dev, "Timeout for ROM init, ret:%d\n", ret); goto base_fw_load_failed; }
From: Jeeja KP jeeja.kp@intel.com
Instead of passing the topology manifest info directly to IPC library, define the manifest info in topology and use this in IPC Library. This will remove the dependency on topology interface definition with IPC library.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/bxt-sst.c | 25 ++++++++--------- sound/soc/intel/skylake/skl-sst-dsp.h | 4 +-- sound/soc/intel/skylake/skl-sst-ipc.h | 5 ++-- sound/soc/intel/skylake/skl-topology.c | 41 +++++++++++++--------------- sound/soc/intel/skylake/skl-topology.h | 13 +++++++++ sound/soc/intel/skylake/skl-tplg-interface.h | 12 -------- 6 files changed, 48 insertions(+), 52 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index d959643..15a063a 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -23,7 +23,6 @@ #include "../common/sst-dsp.h" #include "../common/sst-dsp-priv.h" #include "skl-sst-ipc.h" -#include "skl-tplg-interface.h"
#define BXT_BASEFW_TIMEOUT 3000 #define BXT_INIT_TIMEOUT 500 @@ -52,7 +51,7 @@ static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) }
static int -bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) +bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count) { struct snd_dma_buffer dmab; struct skl_sst *skl = ctx->thread_context; @@ -61,11 +60,11 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) int ret = 0, i, dma_id, stream_tag;
/* library indices start from 1 to N. 0 represents base FW */ - for (i = 1; i < minfo->lib_count; i++) { - ret = request_firmware(&fw, minfo->lib[i].name, ctx->dev); + for (i = 1; i < lib_count; i++) { + ret = request_firmware(&fw, linfo[i].name, ctx->dev); if (ret < 0) { dev_err(ctx->dev, "Request lib %s failed:%d\n", - minfo->lib[i].name, ret); + linfo[i].name, ret); return ret; }
@@ -96,7 +95,7 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) ret = skl_sst_ipc_load_library(&skl->ipc, dma_id, i); if (ret < 0) dev_err(ctx->dev, "IPC Load Lib for %s fail: %d\n", - minfo->lib[i].name, ret); + linfo[i].name, ret);
ctx->dsp_ops.trigger(ctx->dev, false, stream_tag); ctx->dsp_ops.cleanup(ctx->dev, &dmab, stream_tag); @@ -119,8 +118,7 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) static int sst_bxt_prepare_fw(struct sst_dsp *ctx, const void *fwdata, u32 fwsize) { - int stream_tag, ret, i; - u32 reg; + int stream_tag, ret;
stream_tag = ctx->dsp_ops.prepare(ctx->dev, 0x40, fwsize, &ctx->dmab); if (stream_tag <= 0) { @@ -413,7 +411,6 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) int ret; struct skl_ipc_dxstate_info dx; unsigned int core_mask = SKL_DSP_CORE_MASK(core_id); - struct skl_dfw_manifest *minfo = &skl->manifest;
if (skl->fw_loaded == false) { skl->boot_complete = false; @@ -423,8 +420,9 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) return ret; }
- if (minfo->lib_count > 1) { - ret = bxt_load_library(ctx, minfo); + if (skl->lib_count > 1) { + ret = bxt_load_library(ctx, skl->lib_info, + skl->lib_count); if (ret < 0) { dev_err(ctx->dev, "reload libs failed: %d\n", ret); return ret; @@ -621,8 +619,9 @@ int bxt_sst_init_fw(struct device *dev, struct skl_sst *ctx)
skl_dsp_init_core_state(sst);
- if (ctx->manifest.lib_count > 1) { - ret = sst->fw_ops.load_library(sst, &ctx->manifest); + if (ctx->lib_count > 1) { + ret = sst->fw_ops.load_library(sst, ctx->lib_info, + ctx->lib_count); if (ret < 0) { dev_err(dev, "Load Library failed : %x\n", ret); return ret; diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h index 7c272ba..849410d 100644 --- a/sound/soc/intel/skylake/skl-sst-dsp.h +++ b/sound/soc/intel/skylake/skl-sst-dsp.h @@ -19,7 +19,6 @@ #include <linux/interrupt.h> #include <sound/memalloc.h> #include "skl-sst-cldma.h" -#include "skl-tplg-interface.h" #include "skl-topology.h"
struct sst_dsp; @@ -145,7 +144,7 @@ struct skl_dsp_fw_ops { int (*load_fw)(struct sst_dsp *ctx); /* FW module parser/loader */ int (*load_library)(struct sst_dsp *ctx, - struct skl_dfw_manifest *minfo); + struct skl_lib_info *linfo, int count); int (*parse_fw)(struct sst_dsp *ctx); int (*set_state_D0)(struct sst_dsp *ctx, unsigned int core_id); int (*set_state_D3)(struct sst_dsp *ctx, unsigned int core_id); @@ -236,5 +235,4 @@ int skl_get_pvt_instance_id_map(struct skl_sst *ctx, void skl_freeup_uuid_list(struct skl_sst *ctx);
int skl_dsp_strip_extended_manifest(struct firmware *fw); - #endif /*__SKL_SST_DSP_H__*/ diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h index 0568f2e8f..65d7da4 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.h +++ b/sound/soc/intel/skylake/skl-sst-ipc.h @@ -98,8 +98,9 @@ struct skl_sst { /* multi-core */ struct skl_dsp_cores cores;
- /* tplg manifest */ - struct skl_dfw_manifest manifest; + /* library info */ + struct skl_lib_info lib_info[SKL_MAX_LIB]; + int lib_count;
/* Callback to update D0i3C register */ void (*update_d0i3c)(struct device *dev, bool enable); diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index edd0c60..547b29c 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -2305,20 +2305,21 @@ static int skl_tplg_control_load(struct snd_soc_component *cmpnt,
static int skl_tplg_fill_str_mfest_tkn(struct device *dev, struct snd_soc_tplg_vendor_string_elem *str_elem, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0; static int ref_count;
switch (str_elem->token) { case SKL_TKN_STR_LIB_NAME: - if (ref_count > minfo->lib_count - 1) { + if (ref_count > skl->skl_sst->lib_count - 1) { ref_count = 0; return -EINVAL; }
- strncpy(minfo->lib[ref_count].name, str_elem->string, - ARRAY_SIZE(minfo->lib[ref_count].name)); + strncpy(skl->skl_sst->lib_info[ref_count].name, + str_elem->string, + ARRAY_SIZE(skl->skl_sst->lib_info[ref_count].name)); ref_count++; tkn_count++; break; @@ -2333,14 +2334,14 @@ static int skl_tplg_fill_str_mfest_tkn(struct device *dev,
static int skl_tplg_get_str_tkn(struct device *dev, struct snd_soc_tplg_vendor_array *array, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0, ret; struct snd_soc_tplg_vendor_string_elem *str_elem;
str_elem = (struct snd_soc_tplg_vendor_string_elem *)array->value; while (tkn_count < array->num_elems) { - ret = skl_tplg_fill_str_mfest_tkn(dev, str_elem, minfo); + ret = skl_tplg_fill_str_mfest_tkn(dev, str_elem, skl); str_elem++;
if (ret < 0) @@ -2354,13 +2355,13 @@ static int skl_tplg_get_str_tkn(struct device *dev,
static int skl_tplg_get_int_tkn(struct device *dev, struct snd_soc_tplg_vendor_value_elem *tkn_elem, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0;
switch (tkn_elem->token) { case SKL_TKN_U32_LIB_COUNT: - minfo->lib_count = tkn_elem->value; + skl->skl_sst->lib_count = tkn_elem->value; tkn_count++; break;
@@ -2377,7 +2378,7 @@ static int skl_tplg_get_int_tkn(struct device *dev, * type. */ static int skl_tplg_get_manifest_tkn(struct device *dev, - char *pvt_data, struct skl_dfw_manifest *minfo, + char *pvt_data, struct skl *skl, int block_size) { int tkn_count = 0, ret; @@ -2393,7 +2394,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev, off += array->size; switch (array->type) { case SND_SOC_TPLG_TUPLE_TYPE_STRING: - ret = skl_tplg_get_str_tkn(dev, array, minfo); + ret = skl_tplg_get_str_tkn(dev, array, skl);
if (ret < 0) return ret; @@ -2415,7 +2416,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev,
while (tkn_count <= array->num_elems - 1) { ret = skl_tplg_get_int_tkn(dev, - tkn_elem, minfo); + tkn_elem, skl); if (ret < 0) return ret;
@@ -2436,7 +2437,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev, * preceded by descriptors for type and size of data block. */ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, - struct device *dev, struct skl_dfw_manifest *minfo) + struct device *dev, struct skl *skl) { struct snd_soc_tplg_vendor_array *array; int num_blocks, block_size = 0, block_type, off = 0; @@ -2479,7 +2480,7 @@ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, data = (manifest->priv.data + off);
if (block_type == SKL_TYPE_TUPLE) { - ret = skl_tplg_get_manifest_tkn(dev, data, minfo, + ret = skl_tplg_get_manifest_tkn(dev, data, skl, block_size);
if (ret < 0) @@ -2497,27 +2498,23 @@ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, static int skl_manifest_load(struct snd_soc_component *cmpnt, struct snd_soc_tplg_manifest *manifest) { - struct skl_dfw_manifest *minfo; struct hdac_ext_bus *ebus = snd_soc_component_get_drvdata(cmpnt); struct hdac_bus *bus = ebus_to_hbus(ebus); struct skl *skl = ebus_to_skl(ebus); - int ret = 0;
/* proceed only if we have private data defined */ if (manifest->priv.size == 0) return 0;
- minfo = &skl->skl_sst->manifest; - - skl_tplg_get_manifest_data(manifest, bus->dev, minfo); + skl_tplg_get_manifest_data(manifest, bus->dev, skl);
- if (minfo->lib_count > HDA_MAX_LIB) { + if (skl->skl_sst->lib_count > SKL_MAX_LIB) { dev_err(bus->dev, "Exceeding max Library count. Got:%d\n", - minfo->lib_count); - ret = -EINVAL; + skl->skl_sst->lib_count); + return -EINVAL; }
- return ret; + return 0; }
static struct snd_soc_tplg_ops skl_tplg_ops = { diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index a0d3158..fefab0e 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -334,6 +334,19 @@ struct skl_pipeline { struct list_head node; };
+#define SKL_LIB_NAME_LENGTH 128 +#define SKL_MAX_LIB 16 + +struct skl_lib_info { + char name[SKL_LIB_NAME_LENGTH]; + const struct firmware *fw; +}; + +struct skl_manifest { + u32 lib_count; + struct skl_lib_info lib[SKL_MAX_LIB]; +}; + static inline struct skl *get_skl_ctx(struct device *dev) { struct hdac_ext_bus *ebus = dev_get_drvdata(dev); diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h index 2f6281e..7a2febf 100644 --- a/sound/soc/intel/skylake/skl-tplg-interface.h +++ b/sound/soc/intel/skylake/skl-tplg-interface.h @@ -157,18 +157,6 @@ struct skl_dfw_algo_data { char params[0]; } __packed;
-#define LIB_NAME_LENGTH 128 -#define HDA_MAX_LIB 16 - -struct lib_info { - char name[LIB_NAME_LENGTH]; -} __packed; - -struct skl_dfw_manifest { - u32 lib_count; - struct lib_info lib[HDA_MAX_LIB]; -} __packed; - enum skl_tkn_dir { SKL_DIR_IN, SKL_DIR_OUT
The patch
ASoC: Intel: Skylake: Clean up manifest info
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From eee0e16f8c3cf31fbbae4a88e51d25abebbaf147 Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Mon, 2 Jan 2017 09:50:04 +0530 Subject: [PATCH] ASoC: Intel: Skylake: Clean up manifest info
Instead of passing the topology manifest info directly to IPC library, define the manifest info in topology and use this in IPC Library. This will remove the dependency on topology interface definition with IPC library.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/bxt-sst.c | 25 ++++++++--------- sound/soc/intel/skylake/skl-sst-dsp.h | 4 +-- sound/soc/intel/skylake/skl-sst-ipc.h | 5 ++-- sound/soc/intel/skylake/skl-topology.c | 41 +++++++++++++--------------- sound/soc/intel/skylake/skl-topology.h | 13 +++++++++ sound/soc/intel/skylake/skl-tplg-interface.h | 12 -------- 6 files changed, 48 insertions(+), 52 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 1f9f33d34000..e4a382870132 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -23,7 +23,6 @@ #include "../common/sst-dsp.h" #include "../common/sst-dsp-priv.h" #include "skl-sst-ipc.h" -#include "skl-tplg-interface.h"
#define BXT_BASEFW_TIMEOUT 3000 #define BXT_INIT_TIMEOUT 500 @@ -52,7 +51,7 @@ static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) }
static int -bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) +bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count) { struct snd_dma_buffer dmab; struct skl_sst *skl = ctx->thread_context; @@ -61,11 +60,11 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) int ret = 0, i, dma_id, stream_tag;
/* library indices start from 1 to N. 0 represents base FW */ - for (i = 1; i < minfo->lib_count; i++) { - ret = request_firmware(&fw, minfo->lib[i].name, ctx->dev); + for (i = 1; i < lib_count; i++) { + ret = request_firmware(&fw, linfo[i].name, ctx->dev); if (ret < 0) { dev_err(ctx->dev, "Request lib %s failed:%d\n", - minfo->lib[i].name, ret); + linfo[i].name, ret); return ret; }
@@ -96,7 +95,7 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) ret = skl_sst_ipc_load_library(&skl->ipc, dma_id, i); if (ret < 0) dev_err(ctx->dev, "IPC Load Lib for %s fail: %d\n", - minfo->lib[i].name, ret); + linfo[i].name, ret);
ctx->dsp_ops.trigger(ctx->dev, false, stream_tag); ctx->dsp_ops.cleanup(ctx->dev, &dmab, stream_tag); @@ -119,8 +118,7 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_dfw_manifest *minfo) static int sst_bxt_prepare_fw(struct sst_dsp *ctx, const void *fwdata, u32 fwsize) { - int stream_tag, ret, i; - u32 reg; + int stream_tag, ret;
stream_tag = ctx->dsp_ops.prepare(ctx->dev, 0x40, fwsize, &ctx->dmab); if (stream_tag <= 0) { @@ -432,7 +430,6 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) int ret; struct skl_ipc_dxstate_info dx; unsigned int core_mask = SKL_DSP_CORE_MASK(core_id); - struct skl_dfw_manifest *minfo = &skl->manifest;
if (skl->fw_loaded == false) { skl->boot_complete = false; @@ -442,8 +439,9 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id) return ret; }
- if (minfo->lib_count > 1) { - ret = bxt_load_library(ctx, minfo); + if (skl->lib_count > 1) { + ret = bxt_load_library(ctx, skl->lib_info, + skl->lib_count); if (ret < 0) { dev_err(ctx->dev, "reload libs failed: %d\n", ret); return ret; @@ -640,8 +638,9 @@ int bxt_sst_init_fw(struct device *dev, struct skl_sst *ctx)
skl_dsp_init_core_state(sst);
- if (ctx->manifest.lib_count > 1) { - ret = sst->fw_ops.load_library(sst, &ctx->manifest); + if (ctx->lib_count > 1) { + ret = sst->fw_ops.load_library(sst, ctx->lib_info, + ctx->lib_count); if (ret < 0) { dev_err(dev, "Load Library failed : %x\n", ret); return ret; diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h index 7c272ba0f4b5..849410d0823e 100644 --- a/sound/soc/intel/skylake/skl-sst-dsp.h +++ b/sound/soc/intel/skylake/skl-sst-dsp.h @@ -19,7 +19,6 @@ #include <linux/interrupt.h> #include <sound/memalloc.h> #include "skl-sst-cldma.h" -#include "skl-tplg-interface.h" #include "skl-topology.h"
struct sst_dsp; @@ -145,7 +144,7 @@ struct skl_dsp_fw_ops { int (*load_fw)(struct sst_dsp *ctx); /* FW module parser/loader */ int (*load_library)(struct sst_dsp *ctx, - struct skl_dfw_manifest *minfo); + struct skl_lib_info *linfo, int count); int (*parse_fw)(struct sst_dsp *ctx); int (*set_state_D0)(struct sst_dsp *ctx, unsigned int core_id); int (*set_state_D3)(struct sst_dsp *ctx, unsigned int core_id); @@ -236,5 +235,4 @@ int skl_get_pvt_instance_id_map(struct skl_sst *ctx, void skl_freeup_uuid_list(struct skl_sst *ctx);
int skl_dsp_strip_extended_manifest(struct firmware *fw); - #endif /*__SKL_SST_DSP_H__*/ diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h index cc40341233fa..9660ace379ab 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.h +++ b/sound/soc/intel/skylake/skl-sst-ipc.h @@ -97,8 +97,9 @@ struct skl_sst { /* multi-core */ struct skl_dsp_cores cores;
- /* tplg manifest */ - struct skl_dfw_manifest manifest; + /* library info */ + struct skl_lib_info lib_info[SKL_MAX_LIB]; + int lib_count;
/* Callback to update D0i3C register */ void (*update_d0i3c)(struct device *dev, bool enable); diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c index 422a9dee9270..e6e76237f46b 100644 --- a/sound/soc/intel/skylake/skl-topology.c +++ b/sound/soc/intel/skylake/skl-topology.c @@ -2300,20 +2300,21 @@ static int skl_tplg_control_load(struct snd_soc_component *cmpnt,
static int skl_tplg_fill_str_mfest_tkn(struct device *dev, struct snd_soc_tplg_vendor_string_elem *str_elem, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0; static int ref_count;
switch (str_elem->token) { case SKL_TKN_STR_LIB_NAME: - if (ref_count > minfo->lib_count - 1) { + if (ref_count > skl->skl_sst->lib_count - 1) { ref_count = 0; return -EINVAL; }
- strncpy(minfo->lib[ref_count].name, str_elem->string, - ARRAY_SIZE(minfo->lib[ref_count].name)); + strncpy(skl->skl_sst->lib_info[ref_count].name, + str_elem->string, + ARRAY_SIZE(skl->skl_sst->lib_info[ref_count].name)); ref_count++; tkn_count++; break; @@ -2328,14 +2329,14 @@ static int skl_tplg_fill_str_mfest_tkn(struct device *dev,
static int skl_tplg_get_str_tkn(struct device *dev, struct snd_soc_tplg_vendor_array *array, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0, ret; struct snd_soc_tplg_vendor_string_elem *str_elem;
str_elem = (struct snd_soc_tplg_vendor_string_elem *)array->value; while (tkn_count < array->num_elems) { - ret = skl_tplg_fill_str_mfest_tkn(dev, str_elem, minfo); + ret = skl_tplg_fill_str_mfest_tkn(dev, str_elem, skl); str_elem++;
if (ret < 0) @@ -2349,13 +2350,13 @@ static int skl_tplg_get_str_tkn(struct device *dev,
static int skl_tplg_get_int_tkn(struct device *dev, struct snd_soc_tplg_vendor_value_elem *tkn_elem, - struct skl_dfw_manifest *minfo) + struct skl *skl) { int tkn_count = 0;
switch (tkn_elem->token) { case SKL_TKN_U32_LIB_COUNT: - minfo->lib_count = tkn_elem->value; + skl->skl_sst->lib_count = tkn_elem->value; tkn_count++; break;
@@ -2372,7 +2373,7 @@ static int skl_tplg_get_int_tkn(struct device *dev, * type. */ static int skl_tplg_get_manifest_tkn(struct device *dev, - char *pvt_data, struct skl_dfw_manifest *minfo, + char *pvt_data, struct skl *skl, int block_size) { int tkn_count = 0, ret; @@ -2388,7 +2389,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev, off += array->size; switch (array->type) { case SND_SOC_TPLG_TUPLE_TYPE_STRING: - ret = skl_tplg_get_str_tkn(dev, array, minfo); + ret = skl_tplg_get_str_tkn(dev, array, skl);
if (ret < 0) return ret; @@ -2410,7 +2411,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev,
while (tkn_count <= array->num_elems - 1) { ret = skl_tplg_get_int_tkn(dev, - tkn_elem, minfo); + tkn_elem, skl); if (ret < 0) return ret;
@@ -2431,7 +2432,7 @@ static int skl_tplg_get_manifest_tkn(struct device *dev, * preceded by descriptors for type and size of data block. */ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, - struct device *dev, struct skl_dfw_manifest *minfo) + struct device *dev, struct skl *skl) { struct snd_soc_tplg_vendor_array *array; int num_blocks, block_size = 0, block_type, off = 0; @@ -2474,7 +2475,7 @@ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, data = (manifest->priv.data + off);
if (block_type == SKL_TYPE_TUPLE) { - ret = skl_tplg_get_manifest_tkn(dev, data, minfo, + ret = skl_tplg_get_manifest_tkn(dev, data, skl, block_size);
if (ret < 0) @@ -2492,27 +2493,23 @@ static int skl_tplg_get_manifest_data(struct snd_soc_tplg_manifest *manifest, static int skl_manifest_load(struct snd_soc_component *cmpnt, struct snd_soc_tplg_manifest *manifest) { - struct skl_dfw_manifest *minfo; struct hdac_ext_bus *ebus = snd_soc_component_get_drvdata(cmpnt); struct hdac_bus *bus = ebus_to_hbus(ebus); struct skl *skl = ebus_to_skl(ebus); - int ret = 0;
/* proceed only if we have private data defined */ if (manifest->priv.size == 0) return 0;
- minfo = &skl->skl_sst->manifest; - - skl_tplg_get_manifest_data(manifest, bus->dev, minfo); + skl_tplg_get_manifest_data(manifest, bus->dev, skl);
- if (minfo->lib_count > HDA_MAX_LIB) { + if (skl->skl_sst->lib_count > SKL_MAX_LIB) { dev_err(bus->dev, "Exceeding max Library count. Got:%d\n", - minfo->lib_count); - ret = -EINVAL; + skl->skl_sst->lib_count); + return -EINVAL; }
- return ret; + return 0; }
static struct snd_soc_tplg_ops skl_tplg_ops = { diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h index a0d3158196f0..fefab0e99a3b 100644 --- a/sound/soc/intel/skylake/skl-topology.h +++ b/sound/soc/intel/skylake/skl-topology.h @@ -334,6 +334,19 @@ struct skl_pipeline { struct list_head node; };
+#define SKL_LIB_NAME_LENGTH 128 +#define SKL_MAX_LIB 16 + +struct skl_lib_info { + char name[SKL_LIB_NAME_LENGTH]; + const struct firmware *fw; +}; + +struct skl_manifest { + u32 lib_count; + struct skl_lib_info lib[SKL_MAX_LIB]; +}; + static inline struct skl *get_skl_ctx(struct device *dev) { struct hdac_ext_bus *ebus = dev_get_drvdata(dev); diff --git a/sound/soc/intel/skylake/skl-tplg-interface.h b/sound/soc/intel/skylake/skl-tplg-interface.h index 2f6281e056d6..7a2febf99019 100644 --- a/sound/soc/intel/skylake/skl-tplg-interface.h +++ b/sound/soc/intel/skylake/skl-tplg-interface.h @@ -157,18 +157,6 @@ struct skl_dfw_algo_data { char params[0]; } __packed;
-#define LIB_NAME_LENGTH 128 -#define HDA_MAX_LIB 16 - -struct lib_info { - char name[LIB_NAME_LENGTH]; -} __packed; - -struct skl_dfw_manifest { - u32 lib_count; - struct lib_info lib[HDA_MAX_LIB]; -} __packed; - enum skl_tkn_dir { SKL_DIR_IN, SKL_DIR_OUT
From: Jeeja KP jeeja.kp@intel.com
Saved firmware ctx was not never released, so release Firmware ctx in cleanup routine.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/skl-sst.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c index 8fc3178..b30bd38 100644 --- a/sound/soc/intel/skylake/skl-sst.c +++ b/sound/soc/intel/skylake/skl-sst.c @@ -515,6 +515,9 @@ EXPORT_SYMBOL_GPL(skl_sst_init_fw);
void skl_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx) { + + if (ctx->dsp->fw) + release_firmware(ctx->dsp->fw); skl_clear_module_table(ctx->dsp); skl_freeup_uuid_list(ctx); skl_ipc_free(&ctx->ipc);
The patch
ASoC: Intel: Skylake: Release FW ctx in cleanup
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From bc65a326c579e93a5c2120a65ede72f11369ee5a Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Mon, 2 Jan 2017 09:50:05 +0530 Subject: [PATCH] ASoC: Intel: Skylake: Release FW ctx in cleanup
Saved firmware ctx was not never released, so release Firmware ctx in cleanup routine.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/skl-sst.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c index 8fc3178bc79c..b30bd384c8d3 100644 --- a/sound/soc/intel/skylake/skl-sst.c +++ b/sound/soc/intel/skylake/skl-sst.c @@ -515,6 +515,9 @@ EXPORT_SYMBOL_GPL(skl_sst_init_fw);
void skl_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx) { + + if (ctx->dsp->fw) + release_firmware(ctx->dsp->fw); skl_clear_module_table(ctx->dsp); skl_freeup_uuid_list(ctx); skl_ipc_free(&ctx->ipc);
From: Jeeja KP jeeja.kp@intel.com
Store the DSP firmware/library at boot, so that for S3 to S0 transition use the stored ctx for downloading the firmware to DSP memory.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/bxt-sst.c | 55 +++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 19 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 15a063a..7762d5a 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -50,33 +50,47 @@ static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) return sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE); }
+static void sst_bxt_release_library(struct skl_lib_info *linfo, int lib_count) +{ + int i; + + for (i = 1; i < lib_count; i++) { + if (linfo[i].fw) { + release_firmware(linfo[i].fw); + linfo[i].fw = NULL; + } + } +} + static int bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count) { struct snd_dma_buffer dmab; struct skl_sst *skl = ctx->thread_context; - const struct firmware *fw = NULL; struct firmware stripped_fw; int ret = 0, i, dma_id, stream_tag;
/* library indices start from 1 to N. 0 represents base FW */ for (i = 1; i < lib_count; i++) { - ret = request_firmware(&fw, linfo[i].name, ctx->dev); - if (ret < 0) { - dev_err(ctx->dev, "Request lib %s failed:%d\n", + if (linfo[i].fw == NULL) { + ret = request_firmware(&linfo[i].fw, linfo[i].name, + ctx->dev); + if (ret < 0) { + dev_err(ctx->dev, "Request lib %s failed:%d\n", linfo[i].name, ret); - return ret; + goto load_library_failed; + } }
if (skl->is_first_boot) { - ret = snd_skl_parse_uuids(ctx, fw, + ret = snd_skl_parse_uuids(ctx, linfo[i].fw, BXT_ADSP_FW_BIN_HDR_OFFSET, i); if (ret < 0) goto load_library_failed; }
- stripped_fw.data = fw->data; - stripped_fw.size = fw->size; + stripped_fw.data = linfo[i].fw->data; + stripped_fw.size = linfo[i].fw->size; skl_dsp_strip_extended_manifest(&stripped_fw);
stream_tag = ctx->dsp_ops.prepare(ctx->dev, 0x40, @@ -99,14 +113,12 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count)
ctx->dsp_ops.trigger(ctx->dev, false, stream_tag); ctx->dsp_ops.cleanup(ctx->dev, &dmab, stream_tag); - release_firmware(fw); - fw = NULL; }
return ret;
load_library_failed: - release_firmware(fw); + sst_bxt_release_library(linfo, lib_count); return ret; }
@@ -208,16 +220,14 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) struct skl_sst *skl = ctx->thread_context; int ret;
- ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); - if (ret < 0) { - dev_err(ctx->dev, "Request firmware failed %d\n", ret); - goto sst_load_base_firmware_failed; + if (ctx->fw == NULL) { + ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); + if (ret < 0) { + dev_err(ctx->dev, "Request firmware failed %d\n", ret); + return ret; + } }
- /* check for extended manifest */ - if (ctx->fw == NULL) - goto sst_load_base_firmware_failed; - /* prase uuids on first boot */ if (skl->is_first_boot) { ret = snd_skl_parse_uuids(ctx, ctx->fw, BXT_ADSP_FW_BIN_HDR_OFFSET, 0); @@ -265,8 +275,11 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) } }
+ return ret; + sst_load_base_firmware_failed: release_firmware(ctx->fw); + ctx->fw = NULL; return ret; }
@@ -635,6 +648,10 @@ EXPORT_SYMBOL_GPL(bxt_sst_init_fw);
void bxt_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx) { + + sst_bxt_release_library(ctx->lib_info, ctx->lib_count); + if (ctx->dsp->fw) + release_firmware(ctx->dsp->fw); skl_freeup_uuid_list(ctx); skl_ipc_free(&ctx->ipc); ctx->dsp->cl_dev.ops.cl_cleanup_controller(ctx->dsp);
On Mon, Jan 02, 2017 at 09:50:06AM +0530, jeeja.kp@intel.com wrote:
From: Jeeja KP jeeja.kp@intel.com
Store the DSP firmware/library at boot, so that for S3 to S0 transition use the stored ctx for downloading the firmware to DSP memory.
This doesn't apply against current code, please check and resend. It's also odd to see fixes like this at the end of the series after cleanups, I'd expect fixes to come earlier in the series as they should go to Linus wheras cleanups just go on the development branches.
On Fri, Jan 06, 2017 at 06:14:04PM +0000, Mark Brown wrote:
On Mon, Jan 02, 2017 at 09:50:06AM +0530, jeeja.kp@intel.com wrote:
From: Jeeja KP jeeja.kp@intel.com
Store the DSP firmware/library at boot, so that for S3 to S0 transition use the stored ctx for downloading the firmware to DSP memory.
This doesn't apply against current code, please check and resend. It's also odd to see fixes like this at the end of the series after cleanups, I'd expect fixes to come earlier in the series as they should go to Linus wheras cleanups just go on the development branches.
I have developed this fix on the firmware download optimization. This fix depends on this in the series "Driver updates on DSP firmware" for firmware download optimization. Sorry I missed to mention that in the cover letter, will take care of this details from next time. --
The patch
ASoC: Intel: bxtn: Store the FW/Library context at boot
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 31d648f051fe82c9d6c2176b1b5ee402b1a18f21 Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Fri, 17 Feb 2017 22:48:56 +0530 Subject: [PATCH] ASoC: Intel: bxtn: Store the FW/Library context at boot
Store the DSP firmware/library at boot, so that for S3 to S0 transition use the stored ctx for downloading the firmware to DSP memory.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/bxt-sst.c | 55 +++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 19 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 15a063a403cc..7762d5a18fce 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -50,33 +50,47 @@ static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) return sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE); }
+static void sst_bxt_release_library(struct skl_lib_info *linfo, int lib_count) +{ + int i; + + for (i = 1; i < lib_count; i++) { + if (linfo[i].fw) { + release_firmware(linfo[i].fw); + linfo[i].fw = NULL; + } + } +} + static int bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count) { struct snd_dma_buffer dmab; struct skl_sst *skl = ctx->thread_context; - const struct firmware *fw = NULL; struct firmware stripped_fw; int ret = 0, i, dma_id, stream_tag;
/* library indices start from 1 to N. 0 represents base FW */ for (i = 1; i < lib_count; i++) { - ret = request_firmware(&fw, linfo[i].name, ctx->dev); - if (ret < 0) { - dev_err(ctx->dev, "Request lib %s failed:%d\n", + if (linfo[i].fw == NULL) { + ret = request_firmware(&linfo[i].fw, linfo[i].name, + ctx->dev); + if (ret < 0) { + dev_err(ctx->dev, "Request lib %s failed:%d\n", linfo[i].name, ret); - return ret; + goto load_library_failed; + } }
if (skl->is_first_boot) { - ret = snd_skl_parse_uuids(ctx, fw, + ret = snd_skl_parse_uuids(ctx, linfo[i].fw, BXT_ADSP_FW_BIN_HDR_OFFSET, i); if (ret < 0) goto load_library_failed; }
- stripped_fw.data = fw->data; - stripped_fw.size = fw->size; + stripped_fw.data = linfo[i].fw->data; + stripped_fw.size = linfo[i].fw->size; skl_dsp_strip_extended_manifest(&stripped_fw);
stream_tag = ctx->dsp_ops.prepare(ctx->dev, 0x40, @@ -99,14 +113,12 @@ bxt_load_library(struct sst_dsp *ctx, struct skl_lib_info *linfo, int lib_count)
ctx->dsp_ops.trigger(ctx->dev, false, stream_tag); ctx->dsp_ops.cleanup(ctx->dev, &dmab, stream_tag); - release_firmware(fw); - fw = NULL; }
return ret;
load_library_failed: - release_firmware(fw); + sst_bxt_release_library(linfo, lib_count); return ret; }
@@ -208,16 +220,14 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) struct skl_sst *skl = ctx->thread_context; int ret;
- ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); - if (ret < 0) { - dev_err(ctx->dev, "Request firmware failed %d\n", ret); - goto sst_load_base_firmware_failed; + if (ctx->fw == NULL) { + ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); + if (ret < 0) { + dev_err(ctx->dev, "Request firmware failed %d\n", ret); + return ret; + } }
- /* check for extended manifest */ - if (ctx->fw == NULL) - goto sst_load_base_firmware_failed; - /* prase uuids on first boot */ if (skl->is_first_boot) { ret = snd_skl_parse_uuids(ctx, ctx->fw, BXT_ADSP_FW_BIN_HDR_OFFSET, 0); @@ -265,8 +275,11 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) } }
+ return ret; + sst_load_base_firmware_failed: release_firmware(ctx->fw); + ctx->fw = NULL; return ret; }
@@ -635,6 +648,10 @@ EXPORT_SYMBOL_GPL(bxt_sst_init_fw);
void bxt_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx) { + + sst_bxt_release_library(ctx->lib_info, ctx->lib_count); + if (ctx->dsp->fw) + release_firmware(ctx->dsp->fw); skl_freeup_uuid_list(ctx); skl_ipc_free(&ctx->ipc); ctx->dsp->cl_dev.ops.cl_cleanup_controller(ctx->dsp);
From: Jeeja KP jeeja.kp@intel.com
During S3->S0 transition, sometime ROM init fails because of authentication engine loads later than the OS. In this case driver waits for a longer period and then retries the FW download causing huge delay in resume time of audio device.
To avoid this, ROM INIT wait time is set to a optimal value and increased the retries for firmware download.
Signed-off-by: Jeeja KP jeeja.kp@intel.com --- sound/soc/intel/skylake/bxt-sst.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 7762d5a..d3be1be 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -25,7 +25,8 @@ #include "skl-sst-ipc.h"
#define BXT_BASEFW_TIMEOUT 3000 -#define BXT_INIT_TIMEOUT 500 +#define BXT_INIT_TIMEOUT 300 +#define BXT_ROM_INIT_TIMEOUT 70 #define BXT_IPC_PURGE_FW 0x01004000
#define BXT_ROM_INIT 0x5 @@ -45,6 +46,8 @@ /* Delay before scheduling D0i3 entry */ #define BXT_D0I3_DELAY 5000
+#define BXT_FW_ROM_INIT_RETRY 3 + static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) { return sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE); @@ -185,7 +188,7 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx,
/* Step 7: Wait for ROM init */ ret = sst_dsp_register_poll(ctx, BXT_ADSP_FW_STATUS, SKL_FW_STS_MASK, - SKL_FW_INIT, BXT_INIT_TIMEOUT, "ROM Load"); + SKL_FW_INIT, BXT_ROM_INIT_TIMEOUT, "ROM Load"); if (ret < 0) { dev_err(ctx->dev, "Timeout for ROM init, ret:%d\n", ret); goto base_fw_load_failed; @@ -218,7 +221,7 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) { struct firmware stripped_fw; struct skl_sst *skl = ctx->thread_context; - int ret; + int ret, i;
if (ctx->fw == NULL) { ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); @@ -239,18 +242,20 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) stripped_fw.size = ctx->fw->size; skl_dsp_strip_extended_manifest(&stripped_fw);
- ret = sst_bxt_prepare_fw(ctx, stripped_fw.data, stripped_fw.size); - /* Retry Enabling core and ROM load. Retry seemed to help */ - if (ret < 0) { + + for (i = 0; i < BXT_FW_ROM_INIT_RETRY; i++) { ret = sst_bxt_prepare_fw(ctx, stripped_fw.data, stripped_fw.size); - if (ret < 0) { - dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n", + if (ret == 0) + break; + } + + if (ret < 0) { + dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n", sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE), sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS));
- dev_err(ctx->dev, "Core En/ROM load fail:%d\n", ret); - goto sst_load_base_firmware_failed; - } + dev_err(ctx->dev, "Core En/ROM load fail:%d\n", ret); + goto sst_load_base_firmware_failed; }
ret = sst_transfer_fw_host_dma(ctx);
The patch
ASoC: Intel: bxtn: optimize ROM init retries
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
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
From 7d3f91dc1e4db18b644695c9442c62679a5dff6e Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Fri, 17 Feb 2017 22:48:57 +0530 Subject: [PATCH] ASoC: Intel: bxtn: optimize ROM init retries
During S3->S0 transition, sometime ROM init fails because of authentication engine loads later than the OS. In this case driver waits for a longer period and then retries the FW download causing huge delay in resume time of audio device.
To avoid this, ROM INIT wait time is set to a optimal value and increased the retries for firmware download.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Acked-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/intel/skylake/bxt-sst.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c index 7762d5a18fce..d3be1be5a372 100644 --- a/sound/soc/intel/skylake/bxt-sst.c +++ b/sound/soc/intel/skylake/bxt-sst.c @@ -25,7 +25,8 @@ #include "skl-sst-ipc.h"
#define BXT_BASEFW_TIMEOUT 3000 -#define BXT_INIT_TIMEOUT 500 +#define BXT_INIT_TIMEOUT 300 +#define BXT_ROM_INIT_TIMEOUT 70 #define BXT_IPC_PURGE_FW 0x01004000
#define BXT_ROM_INIT 0x5 @@ -45,6 +46,8 @@ /* Delay before scheduling D0i3 entry */ #define BXT_D0I3_DELAY 5000
+#define BXT_FW_ROM_INIT_RETRY 3 + static unsigned int bxt_get_errorcode(struct sst_dsp *ctx) { return sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE); @@ -185,7 +188,7 @@ static int sst_bxt_prepare_fw(struct sst_dsp *ctx,
/* Step 7: Wait for ROM init */ ret = sst_dsp_register_poll(ctx, BXT_ADSP_FW_STATUS, SKL_FW_STS_MASK, - SKL_FW_INIT, BXT_INIT_TIMEOUT, "ROM Load"); + SKL_FW_INIT, BXT_ROM_INIT_TIMEOUT, "ROM Load"); if (ret < 0) { dev_err(ctx->dev, "Timeout for ROM init, ret:%d\n", ret); goto base_fw_load_failed; @@ -218,7 +221,7 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) { struct firmware stripped_fw; struct skl_sst *skl = ctx->thread_context; - int ret; + int ret, i;
if (ctx->fw == NULL) { ret = request_firmware(&ctx->fw, ctx->fw_name, ctx->dev); @@ -239,18 +242,20 @@ static int bxt_load_base_firmware(struct sst_dsp *ctx) stripped_fw.size = ctx->fw->size; skl_dsp_strip_extended_manifest(&stripped_fw);
- ret = sst_bxt_prepare_fw(ctx, stripped_fw.data, stripped_fw.size); - /* Retry Enabling core and ROM load. Retry seemed to help */ - if (ret < 0) { + + for (i = 0; i < BXT_FW_ROM_INIT_RETRY; i++) { ret = sst_bxt_prepare_fw(ctx, stripped_fw.data, stripped_fw.size); - if (ret < 0) { - dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n", + if (ret == 0) + break; + } + + if (ret < 0) { + dev_err(ctx->dev, "Error code=0x%x: FW status=0x%x\n", sst_dsp_shim_read(ctx, BXT_ADSP_ERROR_CODE), sst_dsp_shim_read(ctx, BXT_ADSP_FW_STATUS));
- dev_err(ctx->dev, "Core En/ROM load fail:%d\n", ret); - goto sst_load_base_firmware_failed; - } + dev_err(ctx->dev, "Core En/ROM load fail:%d\n", ret); + goto sst_load_base_firmware_failed; }
ret = sst_transfer_fw_host_dma(ctx);
On Mon, Jan 02, 2017 at 09:50:01AM +0530, jeeja.kp@intel.com wrote:
From: Jeeja KP jeeja.kp@intel.com
This patch series provides update on DSP firmware download by optimizing o updating the dsp register poll method to use accurate timeout. o optimizing ROM init retries. o saving the firmware/library context at boot and use this ctx for downloading the firmware to DSP memory. o cleanup to store the library manifest data o release the firmware in cleanup routine.
8 files changed, 137 insertions(+), 135 deletions(-)
Thanks for the nice cleanup and fixes. As evident from diffstat, only two lines added.
Acked-by: Vinod Koul vinod.koul@intel.com
participants (4)
-
Jeeja KP
-
jeeja.kp@intel.com
-
Mark Brown
-
Vinod Koul