[alsa-devel] [PATCH v2 0/2] 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.
Changes in v2: - changed the title to remove fix as this is optimization. - removed already merged 4 patchs
Jeeja KP (2): ASoC: Intel: bxtn: Store the FW/Library context at boot ASoC: Intel: bxtn: optimize ROM init retries
sound/soc/intel/skylake/bxt-sst.c | 82 +++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 30 deletions(-)
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 Thu, 2017-01-12 at 09:54 +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.
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++) {
It probably makes sense to replace 1 with a macro here to indicate the first non base FW ID number. This can be done in other places too to make the code more readable for library management.
Liam
On Thu, Jan 12, 2017 at 08:11:21AM +0000, Liam Girdwood wrote:
On Thu, 2017-01-12 at 09:54 +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.
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++) {
It probably makes sense to replace 1 with a macro here to indicate the first non base FW ID number. This can be done in other places too to make the code more readable for library management.
sure, will add a macro for the non base FW ID and will address this as a separate patch as this is used in other places as well.
Liam
--
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);
participants (3)
-
Jeeja KP -
jeeja.kp＠intel.com
-
Liam Girdwood