All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates
@ 2017-03-13 16:41 jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 01/10] ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe jeeja.kp
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

This patch series provides updates and fixes on the following:
 Fix parameter overwrite for KPB DSP Module.
 To handle module download with size > dma buffer size.
 Fix to handle dsp pipes correctly.
 Disable notification after dsp boot.
 Remove get dsp_ops in cleanup routine.
 Disable interrupt when DSP is in D3.
 Update DSP core state in D0.
 Reload the firmware in case of D3 failure.

Changes in v2:
	- fixed the typo's in commit message.
	- handle error in case when ipc fails in case of load module.

Dharageswari R (1):
  ASoC: Intel: Skylake: Fix parameter overwrite for KPB Module

G Kranthi (2):
  ASoC: Intel: Skylake: Disable notifications at boot after DSP FW init
  ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine

Guneshwor Singh (1):
  ASoC: Intel: Skylake: Fix not to stop src pipe in pre pmd event
    handler

Jeeja KP (6):
  ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe
  ASoC: Intel: bxtn: Disable interrupt when DSP is in D3
  ASoC: Intel: bxtn: Update DSP core state in D0
  ASoC: Intel: bxtn: Reload the firmware in case of D3 failure
  ASoC: Intel: Skylake: Remove BE prepare ops
  ASoC: Intel: Skylake: Fix module load when module size > DMA buffer
    size

 sound/soc/intel/skylake/bxt-sst.c       | 14 ++++++-
 sound/soc/intel/skylake/skl-messages.c  | 12 ++----
 sound/soc/intel/skylake/skl-pcm.c       | 19 +---------
 sound/soc/intel/skylake/skl-sst-cldma.c | 26 ++++++++-----
 sound/soc/intel/skylake/skl-sst-cldma.h |  2 +-
 sound/soc/intel/skylake/skl-sst-dsp.c   |  6 ++-
 sound/soc/intel/skylake/skl-sst-dsp.h   |  3 ++
 sound/soc/intel/skylake/skl-sst-ipc.c   | 66 ++++++++++++++++++++++-----------
 sound/soc/intel/skylake/skl-sst-ipc.h   |  7 ++++
 sound/soc/intel/skylake/skl-sst.c       | 53 ++++++++++++++++++++------
 sound/soc/intel/skylake/skl-topology.c  | 56 ++++++++++++++--------------
 11 files changed, 165 insertions(+), 99 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH V2 01/10] ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 02/10] ASoC: Intel: Skylake: Fix not to stop src pipe in pre pmd event handler jeeja.kp
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

DSP pipe needs to stopped before deleting the pipe. Currently check is
for pipe state > STARTED, which is incorrect. So changed to include
pipe state STARTED to stop the pipe if it started.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index e668704..ed57696 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -1098,7 +1098,7 @@ int skl_delete_pipe(struct skl_sst *ctx, struct skl_pipe *pipe)
 	dev_dbg(ctx->dev, "%s: pipe = %d\n", __func__, pipe->ppl_id);
 
 	/* If pipe is started, do stop the pipe in FW. */
-	if (pipe->state > SKL_PIPE_STARTED) {
+	if (pipe->state >= SKL_PIPE_STARTED) {
 		ret = skl_set_pipe_state(ctx, pipe, PPL_PAUSED);
 		if (ret < 0) {
 			dev_err(ctx->dev, "Failed to stop pipeline\n");
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 02/10] ASoC: Intel: Skylake: Fix not to stop src pipe in pre pmd event handler
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 01/10] ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 03/10] ASoC: Intel: bxtn: Disable interrupt when DSP is in D3 jeeja.kp
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, Guneshwor Singh, patches.audio, broonie, liam.r.girdwood,
	Jeeja KP

From: Guneshwor Singh <guneshwor.o.singh@intel.com>

If the widget is a mixin module, just unbind between source and sink
and don't stop the source pipe as there can be multiple sinks
connected.

Signed-off-by: Guneshwor Singh <guneshwor.o.singh@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index f38f8e6..f04a251 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -975,15 +975,6 @@ static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
 			src_mconfig = sink_mconfig->m_in_pin[i].tgt_mcfg;
 			if (!src_mconfig)
 				continue;
-			/*
-			 * If path_found == 1, that means pmd for source
-			 * pipe has not occurred, source is connected to
-			 * some other sink. so its responsibility of sink
-			 * to unbind itself from source.
-			 */
-			ret = skl_stop_pipe(ctx, src_mconfig->pipe);
-			if (ret < 0)
-				return ret;
 
 			ret = skl_unbind_modules(ctx,
 						src_mconfig, sink_mconfig);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 03/10] ASoC: Intel: bxtn: Disable interrupt when DSP is in D3
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 01/10] ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 02/10] ASoC: Intel: Skylake: Fix not to stop src pipe in pre pmd event handler jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 04/10] ASoC: Intel: bxtn: Update DSP core state in D0 jeeja.kp
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

When DSP is in D3, no interrupts are expected, so disable
interrupt while entering D3.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/bxt-sst.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c
index d3be1be..b34c965 100644
--- a/sound/soc/intel/skylake/bxt-sst.c
+++ b/sound/soc/intel/skylake/bxt-sst.c
@@ -537,6 +537,11 @@ static int bxt_set_dsp_D3(struct sst_dsp *ctx, unsigned int core_id)
 		"Failed to set DSP to D3:core id = %d;Continue reset\n",
 		core_id);
 
+	if (core_id == SKL_DSP_CORE0_ID) {
+		/* disable Interrupt */
+		skl_ipc_op_int_disable(ctx);
+		skl_ipc_int_disable(ctx);
+	}
 	ret = skl_dsp_disable_core(ctx, core_mask);
 	if (ret < 0) {
 		dev_err(ctx->dev, "Failed to disable core %d\n", ret);
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 04/10] ASoC: Intel: bxtn: Update DSP core state in D0
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (2 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 03/10] ASoC: Intel: bxtn: Disable interrupt when DSP is in D3 jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 05/10] ASoC: Intel: bxtn: Reload the firmware in case of D3 failure jeeja.kp
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

In system suspend, firmware needs to be re-downloaded as IMR is cleared.
When firmware is downloaded in D0, core state is not set to running
state causing instability with subsequent D0-D3 cycles.

So set the core state correctly during D0 and check the DSP core state
if not in reset to set the DSP to D3.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/bxt-sst.c     | 1 +
 sound/soc/intel/skylake/skl-sst-dsp.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c
index b34c965..2a2bb94 100644
--- a/sound/soc/intel/skylake/bxt-sst.c
+++ b/sound/soc/intel/skylake/bxt-sst.c
@@ -446,6 +446,7 @@ static int bxt_set_dsp_D0(struct sst_dsp *ctx, unsigned int core_id)
 				return ret;
 			}
 		}
+		skl->cores.state[core_id] = SKL_DSP_RUNNING;
 		return ret;
 	}
 
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.c b/sound/soc/intel/skylake/skl-sst-dsp.c
index c3deefa..0833272 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.c
+++ b/sound/soc/intel/skylake/skl-sst-dsp.c
@@ -355,12 +355,13 @@ int skl_dsp_get_core(struct sst_dsp *ctx, unsigned int core_id)
 		ret = ctx->fw_ops.set_state_D0(ctx, core_id);
 		if (ret < 0) {
 			dev_err(ctx->dev, "unable to get core%d\n", core_id);
-			return ret;
+			goto out;
 		}
 	}
 
 	skl->cores.usage_count[core_id]++;
 
+out:
 	dev_dbg(ctx->dev, "core id %d state %d usage_count %d\n",
 			core_id, skl->cores.state[core_id],
 			skl->cores.usage_count[core_id]);
@@ -379,7 +380,8 @@ int skl_dsp_put_core(struct sst_dsp *ctx, unsigned int core_id)
 		return -EINVAL;
 	}
 
-	if (--skl->cores.usage_count[core_id] == 0) {
+	if ((--skl->cores.usage_count[core_id] == 0) &&
+		(skl->cores.state[core_id] != SKL_DSP_RESET)) {
 		ret = ctx->fw_ops.set_state_D3(ctx, core_id);
 		if (ret < 0) {
 			dev_err(ctx->dev, "unable to put core %d: %d\n",
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 05/10] ASoC: Intel: bxtn: Reload the firmware in case of D3 failure
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (3 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 04/10] ASoC: Intel: bxtn: Update DSP core state in D0 jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 06/10] ASoC: Intel: Skylake: Remove BE prepare ops jeeja.kp
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

If D3 IPC fails or times out, firmware needs to be reloaded as driver
continues the reset.
So set the fw_load flag to false to reload the firmware in D0.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/bxt-sst.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/bxt-sst.c b/sound/soc/intel/skylake/bxt-sst.c
index 2a2bb94..600d958 100644
--- a/sound/soc/intel/skylake/bxt-sst.c
+++ b/sound/soc/intel/skylake/bxt-sst.c
@@ -533,10 +533,16 @@ static int bxt_set_dsp_D3(struct sst_dsp *ctx, unsigned int core_id)
 
 	ret = skl_ipc_set_dx(&skl->ipc, BXT_INSTANCE_ID,
 				BXT_BASE_FW_MODULE_ID, &dx);
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(ctx->dev,
 		"Failed to set DSP to D3:core id = %d;Continue reset\n",
 		core_id);
+		/*
+		 * In case of D3 failure, re-download the firmware, so set
+		 * fw_loaded to false.
+		 */
+		skl->fw_loaded = false;
+	}
 
 	if (core_id == SKL_DSP_CORE0_ID) {
 		/* disable Interrupt */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 06/10] ASoC: Intel: Skylake: Remove BE prepare ops
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (4 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 05/10] ASoC: Intel: bxtn: Reload the firmware in case of D3 failure jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 07/10] ASoC: Intel: Skylake: Disable notifications at boot after DSP FW init jeeja.kp
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

Remove BE prepare ops which enables MCLK by default. If MCLK is required
to be enabled for any specific platform, it needs to be enabled in the
corresponding machine driver.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 635cbb1..e3d206d 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -262,23 +262,6 @@ static int skl_pcm_open(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int skl_be_prepare(struct snd_pcm_substream *substream,
-		struct snd_soc_dai *dai)
-{
-	struct skl *skl = get_skl_ctx(dai->dev);
-	struct skl_sst *ctx = skl->skl_sst;
-	struct skl_module_cfg *mconfig;
-
-	if (dai->playback_widget->power || dai->capture_widget->power)
-		return 0;
-
-	mconfig = skl_tplg_be_get_cpr_module(dai, substream->stream);
-	if (mconfig == NULL)
-		return -EINVAL;
-
-	return skl_dsp_set_dma_control(ctx, mconfig);
-}
-
 static int skl_pcm_prepare(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
@@ -649,7 +632,6 @@ static struct snd_soc_dai_ops skl_dmic_dai_ops = {
 
 static struct snd_soc_dai_ops skl_be_ssp_dai_ops = {
 	.hw_params = skl_be_hw_params,
-	.prepare = skl_be_prepare,
 };
 
 static struct snd_soc_dai_ops skl_link_dai_ops = {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 07/10] ASoC: Intel: Skylake: Disable notifications at boot after DSP FW init
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (5 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 06/10] ASoC: Intel: Skylake: Remove BE prepare ops jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-13 16:41 ` [PATCH V2 08/10] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine jeeja.kp
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, G Kranthi, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: G Kranthi <gudishax.kranthikumar@intel.com>

DSP firmware sends notification every 1ms, which is disabled in runtime
suspend. But if a system has no runtime pm, we keep getting
notification, so disable after FW init as well.

Signed-off-by: G Kranthi <gudishax.kranthikumar@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 2 +-
 sound/soc/intel/skylake/skl-pcm.c      | 1 +
 sound/soc/intel/skylake/skl-sst-dsp.h  | 2 ++
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index ed57696..29523dd 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -58,7 +58,7 @@ static int skl_free_dma_buf(struct device *dev, struct snd_dma_buffer *dmab)
 #define NOTIFICATION_MASK 0xf
 
 /* disable notfication for underruns/overruns from firmware module */
-static void skl_dsp_enable_notification(struct skl_sst *ctx, bool enable)
+void skl_dsp_enable_notification(struct skl_sst *ctx, bool enable)
 {
 	struct notification_mask mask;
 	struct skl_ipc_large_config_msg	msg = {0};
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index e3d206d..2f90bc4 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1214,6 +1214,7 @@ static int skl_platform_soc_probe(struct snd_soc_platform *platform)
 		}
 		skl_populate_modules(skl);
 		skl->skl_sst->update_d0i3c = skl_update_d0i3c;
+		skl_dsp_enable_notification(skl->skl_sst, false);
 	}
 	pm_runtime_mark_last_busy(platform->dev);
 	pm_runtime_put_autosuspend(platform->dev);
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h
index 849410d..e8d1e14 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.h
+++ b/sound/soc/intel/skylake/skl-sst-dsp.h
@@ -235,4 +235,6 @@ 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);
+void skl_dsp_enable_notification(struct skl_sst *ctx, bool enable);
+
 #endif /*__SKL_SST_DSP_H__*/
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 08/10] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (6 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 07/10] ASoC: Intel: Skylake: Disable notifications at boot after DSP FW init jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-15 18:13   ` Applied "ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine" to the asoc tree Mark Brown
  2017-03-13 16:41 ` [PATCH V2 09/10] ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size jeeja.kp
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel
  Cc: tiwai, G Kranthi, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: G Kranthi <gudishax.kranthikumar@intel.com>

dsp ops is already set in init, so use this in cleanup routine
instead of again retrieving it. Also constify struct skl_dsp_ops.

Signed-off-by: G Kranthi <gudishax.kranthikumar@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-messages.c | 8 ++------
 sound/soc/intel/skylake/skl-sst-ipc.h  | 2 ++
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 29523dd..ba1ec97 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -274,6 +274,7 @@ int skl_init_dsp(struct skl *skl)
 	if (ret < 0)
 		return ret;
 
+	skl->skl_sst->dsp_ops = ops;
 	dev_dbg(bus->dev, "dsp registration status=%d\n", ret);
 
 	return ret;
@@ -284,16 +285,11 @@ int skl_free_dsp(struct skl *skl)
 	struct hdac_ext_bus *ebus = &skl->ebus;
 	struct hdac_bus *bus = ebus_to_hbus(ebus);
 	struct skl_sst *ctx = skl->skl_sst;
-	const struct skl_dsp_ops *ops;
 
 	/* disable  ppcap interrupt */
 	snd_hdac_ext_bus_ppcap_int_enable(&skl->ebus, false);
 
-	ops = skl_get_dsp_ops(skl->pci->device);
-	if (!ops)
-		return -EIO;
-
-	ops->cleanup(bus->dev, ctx);
+	ctx->dsp_ops->cleanup(bus->dev, ctx);
 
 	if (ctx->dsp->addr.lpe)
 		iounmap(ctx->dsp->addr.lpe);
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h
index 9660ace..7d21f05 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.h
+++ b/sound/soc/intel/skylake/skl-sst-ipc.h
@@ -105,6 +105,8 @@ struct skl_sst {
 	void (*update_d0i3c)(struct device *dev, bool enable);
 
 	struct skl_d0i3_data d0i3;
+
+	const struct skl_dsp_ops *dsp_ops;
 };
 
 struct skl_ipc_init_instance_msg {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 09/10] ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (7 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 08/10] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-15 18:13   ` Applied "ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size" to the asoc tree Mark Brown
  2017-03-13 16:41 ` [PATCH V2 10/10] ASoC: Intel: Skylake: Fix parameter overwrite for KPB Module jeeja.kp
  2017-03-14  5:00 ` [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates Vinod Koul
  10 siblings, 1 reply; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, patches.audio, broonie, liam.r.girdwood, Jeeja KP

From: Jeeja KP <jeeja.kp@intel.com>

When module size > DMA buffer size, driver copies first chunk and waits
for the BDL complete interrupt. BDL complete interrupt never occurs and
wait time expires as module load IPC is not send to start the DMA from
DSP.

To fix the above issue need to follow the below steps:
1. After copying the first chunk, send the module load IPC to start the
DMA.
2. Wait for the BDL interrupt. Once interrupt is received, copy the
next chunk.
3. Continue step 2 till all bytes are copied.
4. When all the bytes are copied (bytes_left = 0), wait for module load
IPC response
5. Handled module load IPC response messages, check the load module IPC
response and wake up the thread to complete module load.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-sst-cldma.c | 26 ++++++++-----
 sound/soc/intel/skylake/skl-sst-cldma.h |  2 +-
 sound/soc/intel/skylake/skl-sst-dsp.h   |  1 +
 sound/soc/intel/skylake/skl-sst-ipc.c   | 66 ++++++++++++++++++++++-----------
 sound/soc/intel/skylake/skl-sst-ipc.h   |  5 +++
 sound/soc/intel/skylake/skl-sst.c       | 53 ++++++++++++++++++++------
 6 files changed, 110 insertions(+), 43 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-sst-cldma.c b/sound/soc/intel/skylake/skl-sst-cldma.c
index c9f6d87..d2b1d60 100644
--- a/sound/soc/intel/skylake/skl-sst-cldma.c
+++ b/sound/soc/intel/skylake/skl-sst-cldma.c
@@ -164,7 +164,7 @@ static void skl_cldma_cleanup(struct sst_dsp  *ctx)
 	ctx->dsp_ops.free_dma_buf(ctx->dev, &ctx->cl_dev.dmab_bdl);
 }
 
-static int skl_cldma_wait_interruptible(struct sst_dsp *ctx)
+int skl_cldma_wait_interruptible(struct sst_dsp *ctx)
 {
 	int ret = 0;
 
@@ -243,9 +243,14 @@ static void skl_cldma_fill_buffer(struct sst_dsp *ctx, unsigned int size,
  * 2. Polling on fw register to identify if data left to transferred doesn't
  *    fill the ring buffer. Caller takes care of polling the required status
  *    register to identify the transfer status.
+ * 3. if wait flag is set, waits for DBL interrupt to copy the next chunk till
+ *    bytes_left is 0.
+ *    if wait flag is not set, doesn't wait for BDL interrupt. after ccopying
+ *    the first chunk return the no of bytes_left to be copied.
  */
 static int
-skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
+skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin,
+			u32 total_size, bool wait)
 {
 	int ret = 0;
 	bool start = true;
@@ -272,13 +277,14 @@ skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
 			size = ctx->cl_dev.bufsize;
 			skl_cldma_fill_buffer(ctx, size, curr_pos, true, start);
 
-			start = false;
-			ret = skl_cldma_wait_interruptible(ctx);
-			if (ret < 0) {
-				skl_cldma_stop(ctx);
-				return ret;
+			if (wait) {
+				start = false;
+				ret = skl_cldma_wait_interruptible(ctx);
+				if (ret < 0) {
+					skl_cldma_stop(ctx);
+					return ret;
+				}
 			}
-
 		} else {
 			skl_cldma_int_disable(ctx);
 
@@ -298,9 +304,11 @@ skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
 		}
 		bytes_left -= size;
 		curr_pos = curr_pos + size;
+		if (!wait)
+			return bytes_left;
 	}
 
-	return ret;
+	return bytes_left;
 }
 
 void skl_cldma_process_intr(struct sst_dsp *ctx)
diff --git a/sound/soc/intel/skylake/skl-sst-cldma.h b/sound/soc/intel/skylake/skl-sst-cldma.h
index 99e4c86..5b730a1 100644
--- a/sound/soc/intel/skylake/skl-sst-cldma.h
+++ b/sound/soc/intel/skylake/skl-sst-cldma.h
@@ -213,7 +213,7 @@ struct skl_cl_dev_ops {
 	void (*cl_trigger)(struct sst_dsp  *ctx, bool enable);
 	void (*cl_cleanup_controller)(struct sst_dsp  *ctx);
 	int (*cl_copy_to_dmabuf)(struct sst_dsp *ctx,
-			const void *bin, u32 size);
+			const void *bin, u32 size, bool wait);
 	void (*cl_stop_dma)(struct sst_dsp *ctx);
 };
 
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h
index e8d1e14..5d7a93a 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.h
+++ b/sound/soc/intel/skylake/skl-sst-dsp.h
@@ -186,6 +186,7 @@ struct skl_module_table {
 void skl_cldma_process_intr(struct sst_dsp *ctx);
 void skl_cldma_int_disable(struct sst_dsp *ctx);
 int skl_cldma_prepare(struct sst_dsp *ctx);
+int skl_cldma_wait_interruptible(struct sst_dsp *ctx);
 
 void skl_dsp_set_state_locked(struct sst_dsp *ctx, int state);
 struct sst_dsp *skl_dsp_ctx_init(struct device *dev,
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c
index e1391df..e90fe2c 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.c
+++ b/sound/soc/intel/skylake/skl-sst-ipc.c
@@ -34,6 +34,11 @@
 #define IPC_GLB_REPLY_STATUS_MASK	((0x1 << IPC_GLB_REPLY_STATUS_SHIFT) - 1)
 #define IPC_GLB_REPLY_STATUS(x)		((x) << IPC_GLB_REPLY_STATUS_SHIFT)
 
+#define IPC_GLB_REPLY_TYPE_SHIFT	29
+#define IPC_GLB_REPLY_TYPE_MASK		0x1F
+#define IPC_GLB_REPLY_TYPE(x)		(((x) >> IPC_GLB_REPLY_TYPE_SHIFT) \
+					& IPC_GLB_RPLY_TYPE_MASK)
+
 #define IPC_TIMEOUT_MSECS		3000
 
 #define IPC_EMPTY_LIST_SIZE		8
@@ -387,12 +392,27 @@ static int skl_ipc_process_notification(struct sst_generic_ipc *ipc,
 	return 0;
 }
 
+static int skl_ipc_set_reply_error_code(u32 reply)
+{
+	switch (reply) {
+	case IPC_GLB_REPLY_OUT_OF_MEMORY:
+		return -ENOMEM;
+
+	case IPC_GLB_REPLY_BUSY:
+		return -EBUSY;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 		struct skl_ipc_header header)
 {
 	struct ipc_message *msg;
 	u32 reply = header.primary & IPC_GLB_REPLY_STATUS_MASK;
 	u64 *ipc_header = (u64 *)(&header);
+	struct skl_sst *skl = container_of(ipc, struct skl_sst, ipc);
 
 	msg = skl_ipc_reply_get_msg(ipc, *ipc_header);
 	if (msg == NULL) {
@@ -401,33 +421,37 @@ static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 	}
 
 	/* first process the header */
-	switch (reply) {
-	case IPC_GLB_REPLY_SUCCESS:
+	if (reply == IPC_GLB_REPLY_SUCCESS) {
 		dev_dbg(ipc->dev, "ipc FW reply %x: success\n", header.primary);
 		/* copy the rx data from the mailbox */
 		sst_dsp_inbox_read(ipc->dsp, msg->rx_data, msg->rx_size);
-		break;
-
-	case IPC_GLB_REPLY_OUT_OF_MEMORY:
-		dev_err(ipc->dev, "ipc fw reply: %x: no memory\n", header.primary);
-		msg->errno = -ENOMEM;
-		break;
-
-	case IPC_GLB_REPLY_BUSY:
-		dev_err(ipc->dev, "ipc fw reply: %x: Busy\n", header.primary);
-		msg->errno = -EBUSY;
-		break;
+		switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) {
+		case IPC_GLB_LOAD_MULTIPLE_MODS:
+			skl->mod_load_complete = true;
+			skl->mod_load_status = true;
+			wake_up(&skl->mod_load_wait);
+			break;
 
-	default:
-		dev_err(ipc->dev, "Unknown ipc reply: 0x%x\n", reply);
-		msg->errno = -EINVAL;
-		break;
-	}
+		default:
+			break;
 
-	if (reply != IPC_GLB_REPLY_SUCCESS) {
+		}
+	} else {
+		msg->errno = skl_ipc_set_reply_error_code(reply);
 		dev_err(ipc->dev, "ipc FW reply: reply=%d\n", reply);
 		dev_err(ipc->dev, "FW Error Code: %u\n",
 			ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp));
+		switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) {
+		case IPC_GLB_LOAD_MULTIPLE_MODS:
+			skl->mod_load_complete = true;
+			skl->mod_load_status = false;
+			wake_up(&skl->mod_load_wait);
+			break;
+
+		default:
+			break;
+
+		}
 	}
 
 	list_del(&msg->list);
@@ -811,8 +835,8 @@ int skl_ipc_load_modules(struct sst_generic_ipc *ipc,
 	header.primary |= IPC_GLB_TYPE(IPC_GLB_LOAD_MULTIPLE_MODS);
 	header.primary |= IPC_LOAD_MODULE_CNT(module_cnt);
 
-	ret = sst_ipc_tx_message_wait(ipc, *ipc_header, data,
-				(sizeof(u16) * module_cnt), NULL, 0);
+	ret = sst_ipc_tx_message_nowait(ipc, *ipc_header, data,
+				(sizeof(u16) * module_cnt));
 	if (ret < 0)
 		dev_err(ipc->dev, "ipc: load modules failed :%d\n", ret);
 
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h
index 7d21f05..fc07c39 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.h
+++ b/sound/soc/intel/skylake/skl-sst-ipc.h
@@ -77,6 +77,11 @@ struct skl_sst {
 	wait_queue_head_t boot_wait;
 	bool boot_complete;
 
+	/* module load */
+	wait_queue_head_t mod_load_wait;
+	bool mod_load_complete;
+	bool mod_load_status;
+
 	/* IPC messaging */
 	struct sst_generic_ipc ipc;
 
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index b30bd38..39d4aaa 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -52,7 +52,8 @@ static int skl_transfer_firmware(struct sst_dsp *ctx,
 {
 	int ret = 0;
 
-	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, basefw, base_fw_size);
+	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, basefw, base_fw_size,
+								true);
 	if (ret < 0)
 		return ret;
 
@@ -323,22 +324,49 @@ static struct skl_module_table *skl_module_get_from_id(
 	return NULL;
 }
 
-static int skl_transfer_module(struct sst_dsp *ctx,
-			struct skl_load_module_info *module)
+static int skl_transfer_module(struct sst_dsp *ctx, const void *data,
+				u32 size, u16 mod_id)
 {
-	int ret;
+	int ret, bytes_left, curr_pos;
 	struct skl_sst *skl = ctx->thread_context;
+	skl->mod_load_complete = false;
+	init_waitqueue_head(&skl->mod_load_wait);
 
-	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, module->fw->data,
-							module->fw->size);
-	if (ret < 0)
-		return ret;
+	bytes_left = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, data, size, false);
+	if (bytes_left < 0)
+		return bytes_left;
 
-	ret = skl_ipc_load_modules(&skl->ipc, SKL_NUM_MODULES,
-						(void *)&module->mod_id);
-	if (ret < 0)
+	ret = skl_ipc_load_modules(&skl->ipc, SKL_NUM_MODULES, &mod_id);
+	if (ret < 0) {
 		dev_err(ctx->dev, "Failed to Load module: %d\n", ret);
+		goto out;
+	}
+
+	/*
+	 * if bytes_left > 0 then wait for BDL complete interrupt and
+	 * copy the next chunk till bytes_left is 0. if bytes_left is
+	 * is zero, then wait for load module IPC reply
+	 */
+	while (bytes_left > 0) {
+		curr_pos = size - bytes_left;
+
+		ret = skl_cldma_wait_interruptible(ctx);
+		if (ret < 0)
+			goto out;
+
+		bytes_left = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx,
+							data + curr_pos,
+							bytes_left, false);
+	}
+
+	ret = wait_event_timeout(skl->mod_load_wait, skl->mod_load_complete,
+				msecs_to_jiffies(SKL_IPC_BOOT_MSECS));
+	if (ret == 0 || !skl->mod_load_status) {
+		dev_err(ctx->dev, "Module Load failed\n");
+		ret = -EIO;
+	}
 
+out:
 	ctx->cl_dev.ops.cl_stop_dma(ctx);
 
 	return ret;
@@ -365,7 +393,8 @@ static int skl_load_module(struct sst_dsp *ctx, u16 mod_id, u8 *guid)
 	}
 
 	if (!module_entry->usage_cnt) {
-		ret = skl_transfer_module(ctx, module_entry->mod_info);
+		ret = skl_transfer_module(ctx, module_entry->mod_info->fw->data,
+				module_entry->mod_info->fw->size, mod_id);
 		if (ret < 0) {
 			dev_err(ctx->dev, "Failed to Load module\n");
 			return ret;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH V2 10/10] ASoC: Intel: Skylake: Fix parameter overwrite for KPB Module
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (8 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 09/10] ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size jeeja.kp
@ 2017-03-13 16:41 ` jeeja.kp
  2017-03-14  5:00 ` [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates Vinod Koul
  10 siblings, 0 replies; 14+ messages in thread
From: jeeja.kp @ 2017-03-13 16:41 UTC (permalink / raw)
  To: alsa-devel
  Cc: Dharageswari R, tiwai, Kranthikumar, GudishaX, patches.audio,
	broonie, liam.r.girdwood, Jeeja KP

From: Dharageswari R <dharageswari.r@intel.com>

KPB module default parameter were overwritten by the dynamic instance
id once use case is executed. This will cause module crash from
subsequent execution of use case as the updated parameters are used.

So instead of over writing the default parameter, make a copy and
update the module parameter and use this in IPC message.

Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
Signed-off-by: Kranthikumar, GudishaX <gudishax.kranthikumar@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 47 +++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index f04a251..33a4e0d 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -680,26 +680,29 @@ static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int skl_fill_sink_instance_id(struct skl_sst *ctx,
-				struct skl_algo_data *alg_data)
+static int skl_fill_sink_instance_id(struct skl_sst *ctx, u32 *params,
+				int size, struct skl_module_cfg *mcfg)
 {
-	struct skl_kpb_params *params = (struct skl_kpb_params *)alg_data->params;
-	struct skl_mod_inst_map *inst;
 	int i, pvt_id;
 
-	inst = params->map;
+	if (mcfg->m_type == SKL_MODULE_TYPE_KPB) {
+		struct skl_kpb_params *kpb_params =
+				(struct skl_kpb_params *)params;
+		struct skl_mod_inst_map *inst = kpb_params->map;
 
-	for (i = 0; i < params->num_modules; i++) {
-		pvt_id = skl_get_pvt_instance_id_map(ctx,
-					inst->mod_id, inst->inst_id);
-		if (pvt_id < 0)
-			return -EINVAL;
-		inst->inst_id = pvt_id;
-		inst++;
+		for (i = 0; i < kpb_params->num_modules; i++) {
+			pvt_id = skl_get_pvt_instance_id_map(ctx, inst->mod_id,
+								inst->inst_id);
+			if (pvt_id < 0)
+				return -EINVAL;
+
+			inst->inst_id = pvt_id;
+			inst++;
+		}
 	}
+
 	return 0;
 }
-
 /*
  * Some modules require params to be set after the module is bound to
  * all pins connected.
@@ -716,6 +719,7 @@ static int skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w,
 	struct soc_bytes_ext *sb;
 	struct skl_algo_data *bc;
 	struct skl_specific_cfg *sp_cfg;
+	u32 *params;
 
 	/*
 	 * check all out/in pins are in bind state.
@@ -748,11 +752,18 @@ static int skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w,
 			bc = (struct skl_algo_data *)sb->dobj.private;
 
 			if (bc->set_params == SKL_PARAM_BIND) {
-				if (mconfig->m_type == SKL_MODULE_TYPE_KPB)
-					skl_fill_sink_instance_id(ctx, bc);
-				ret = skl_set_module_params(ctx,
-						(u32 *)bc->params, bc->max,
-						bc->param_id, mconfig);
+				params = kzalloc(bc->max, GFP_KERNEL);
+				if (!params)
+					return -ENOMEM;
+
+				memcpy(params, bc->params, bc->max);
+				skl_fill_sink_instance_id(ctx, params, bc->max,
+								mconfig);
+
+				ret = skl_set_module_params(ctx, params,
+						bc->max, bc->param_id, mconfig);
+				kfree(params);
+
 				if (ret < 0)
 					return ret;
 			}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates
  2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
                   ` (9 preceding siblings ...)
  2017-03-13 16:41 ` [PATCH V2 10/10] ASoC: Intel: Skylake: Fix parameter overwrite for KPB Module jeeja.kp
@ 2017-03-14  5:00 ` Vinod Koul
  10 siblings, 0 replies; 14+ messages in thread
From: Vinod Koul @ 2017-03-14  5:00 UTC (permalink / raw)
  To: jeeja.kp; +Cc: tiwai, patches.audio, alsa-devel, broonie, liam.r.girdwood

On Mon, Mar 13, 2017 at 10:11:22PM +0530, jeeja.kp@intel.com wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> This patch series provides updates and fixes on the following:
>  Fix parameter overwrite for KPB DSP Module.
>  To handle module download with size > dma buffer size.
>  Fix to handle dsp pipes correctly.
>  Disable notification after dsp boot.
>  Remove get dsp_ops in cleanup routine.
>  Disable interrupt when DSP is in D3.
>  Update DSP core state in D0.
>  Reload the firmware in case of D3 failure.
> 
> Changes in v2:
> 	- fixed the typo's in commit message.
> 	- handle error in case when ipc fails in case of load module.

Thanks for updating these

Acked-by: Vinod Koul <vinod.koul@intel.com>

-- 
~Vinod

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Applied "ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size" to the asoc tree
  2017-03-13 16:41 ` [PATCH V2 09/10] ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size jeeja.kp
@ 2017-03-15 18:13   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2017-03-15 18:13 UTC (permalink / raw)
  To: Jeeja KP
  Cc: alsa-devel, Vinod Koul, patches.audio, tiwai, broonie, liam.r.girdwood

The patch

   ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size

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 b7d0254c51f3ce79a8931690e8a2f035208f6b55 Mon Sep 17 00:00:00 2001
From: Jeeja KP <jeeja.kp@intel.com>
Date: Mon, 13 Mar 2017 22:11:31 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Fix module load when module size > DMA
 buffer size

When module size > DMA buffer size, driver copies first chunk and waits
for the BDL complete interrupt. BDL complete interrupt never occurs and
wait time expires as module load IPC is not send to start the DMA from
DSP.

To fix the above issue need to follow the below steps:
1. After copying the first chunk, send the module load IPC to start the
DMA.
2. Wait for the BDL interrupt. Once interrupt is received, copy the
next chunk.
3. Continue step 2 till all bytes are copied.
4. When all the bytes are copied (bytes_left = 0), wait for module load
IPC response
5. Handled module load IPC response messages, check the load module IPC
response and wake up the thread to complete module load.

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-cldma.c | 26 ++++++++-----
 sound/soc/intel/skylake/skl-sst-cldma.h |  2 +-
 sound/soc/intel/skylake/skl-sst-dsp.h   |  1 +
 sound/soc/intel/skylake/skl-sst-ipc.c   | 66 ++++++++++++++++++++++-----------
 sound/soc/intel/skylake/skl-sst-ipc.h   |  5 +++
 sound/soc/intel/skylake/skl-sst.c       | 53 ++++++++++++++++++++------
 6 files changed, 110 insertions(+), 43 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-sst-cldma.c b/sound/soc/intel/skylake/skl-sst-cldma.c
index c9f6d87381db..d2b1d60fec02 100644
--- a/sound/soc/intel/skylake/skl-sst-cldma.c
+++ b/sound/soc/intel/skylake/skl-sst-cldma.c
@@ -164,7 +164,7 @@ static void skl_cldma_cleanup(struct sst_dsp  *ctx)
 	ctx->dsp_ops.free_dma_buf(ctx->dev, &ctx->cl_dev.dmab_bdl);
 }
 
-static int skl_cldma_wait_interruptible(struct sst_dsp *ctx)
+int skl_cldma_wait_interruptible(struct sst_dsp *ctx)
 {
 	int ret = 0;
 
@@ -243,9 +243,14 @@ static void skl_cldma_fill_buffer(struct sst_dsp *ctx, unsigned int size,
  * 2. Polling on fw register to identify if data left to transferred doesn't
  *    fill the ring buffer. Caller takes care of polling the required status
  *    register to identify the transfer status.
+ * 3. if wait flag is set, waits for DBL interrupt to copy the next chunk till
+ *    bytes_left is 0.
+ *    if wait flag is not set, doesn't wait for BDL interrupt. after ccopying
+ *    the first chunk return the no of bytes_left to be copied.
  */
 static int
-skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
+skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin,
+			u32 total_size, bool wait)
 {
 	int ret = 0;
 	bool start = true;
@@ -272,13 +277,14 @@ skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
 			size = ctx->cl_dev.bufsize;
 			skl_cldma_fill_buffer(ctx, size, curr_pos, true, start);
 
-			start = false;
-			ret = skl_cldma_wait_interruptible(ctx);
-			if (ret < 0) {
-				skl_cldma_stop(ctx);
-				return ret;
+			if (wait) {
+				start = false;
+				ret = skl_cldma_wait_interruptible(ctx);
+				if (ret < 0) {
+					skl_cldma_stop(ctx);
+					return ret;
+				}
 			}
-
 		} else {
 			skl_cldma_int_disable(ctx);
 
@@ -298,9 +304,11 @@ skl_cldma_copy_to_buf(struct sst_dsp *ctx, const void *bin, u32 total_size)
 		}
 		bytes_left -= size;
 		curr_pos = curr_pos + size;
+		if (!wait)
+			return bytes_left;
 	}
 
-	return ret;
+	return bytes_left;
 }
 
 void skl_cldma_process_intr(struct sst_dsp *ctx)
diff --git a/sound/soc/intel/skylake/skl-sst-cldma.h b/sound/soc/intel/skylake/skl-sst-cldma.h
index 99e4c86b6358..5b730a1a0ae4 100644
--- a/sound/soc/intel/skylake/skl-sst-cldma.h
+++ b/sound/soc/intel/skylake/skl-sst-cldma.h
@@ -213,7 +213,7 @@ struct skl_cl_dev_ops {
 	void (*cl_trigger)(struct sst_dsp  *ctx, bool enable);
 	void (*cl_cleanup_controller)(struct sst_dsp  *ctx);
 	int (*cl_copy_to_dmabuf)(struct sst_dsp *ctx,
-			const void *bin, u32 size);
+			const void *bin, u32 size, bool wait);
 	void (*cl_stop_dma)(struct sst_dsp *ctx);
 };
 
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h
index e8d1e149e0cd..5d7a93aa5bed 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.h
+++ b/sound/soc/intel/skylake/skl-sst-dsp.h
@@ -186,6 +186,7 @@ struct skl_module_table {
 void skl_cldma_process_intr(struct sst_dsp *ctx);
 void skl_cldma_int_disable(struct sst_dsp *ctx);
 int skl_cldma_prepare(struct sst_dsp *ctx);
+int skl_cldma_wait_interruptible(struct sst_dsp *ctx);
 
 void skl_dsp_set_state_locked(struct sst_dsp *ctx, int state);
 struct sst_dsp *skl_dsp_ctx_init(struct device *dev,
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c
index e1391dfbc9e9..e90fe2c0bf2c 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.c
+++ b/sound/soc/intel/skylake/skl-sst-ipc.c
@@ -34,6 +34,11 @@
 #define IPC_GLB_REPLY_STATUS_MASK	((0x1 << IPC_GLB_REPLY_STATUS_SHIFT) - 1)
 #define IPC_GLB_REPLY_STATUS(x)		((x) << IPC_GLB_REPLY_STATUS_SHIFT)
 
+#define IPC_GLB_REPLY_TYPE_SHIFT	29
+#define IPC_GLB_REPLY_TYPE_MASK		0x1F
+#define IPC_GLB_REPLY_TYPE(x)		(((x) >> IPC_GLB_REPLY_TYPE_SHIFT) \
+					& IPC_GLB_RPLY_TYPE_MASK)
+
 #define IPC_TIMEOUT_MSECS		3000
 
 #define IPC_EMPTY_LIST_SIZE		8
@@ -387,12 +392,27 @@ static int skl_ipc_process_notification(struct sst_generic_ipc *ipc,
 	return 0;
 }
 
+static int skl_ipc_set_reply_error_code(u32 reply)
+{
+	switch (reply) {
+	case IPC_GLB_REPLY_OUT_OF_MEMORY:
+		return -ENOMEM;
+
+	case IPC_GLB_REPLY_BUSY:
+		return -EBUSY;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 		struct skl_ipc_header header)
 {
 	struct ipc_message *msg;
 	u32 reply = header.primary & IPC_GLB_REPLY_STATUS_MASK;
 	u64 *ipc_header = (u64 *)(&header);
+	struct skl_sst *skl = container_of(ipc, struct skl_sst, ipc);
 
 	msg = skl_ipc_reply_get_msg(ipc, *ipc_header);
 	if (msg == NULL) {
@@ -401,33 +421,37 @@ static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 	}
 
 	/* first process the header */
-	switch (reply) {
-	case IPC_GLB_REPLY_SUCCESS:
+	if (reply == IPC_GLB_REPLY_SUCCESS) {
 		dev_dbg(ipc->dev, "ipc FW reply %x: success\n", header.primary);
 		/* copy the rx data from the mailbox */
 		sst_dsp_inbox_read(ipc->dsp, msg->rx_data, msg->rx_size);
-		break;
-
-	case IPC_GLB_REPLY_OUT_OF_MEMORY:
-		dev_err(ipc->dev, "ipc fw reply: %x: no memory\n", header.primary);
-		msg->errno = -ENOMEM;
-		break;
-
-	case IPC_GLB_REPLY_BUSY:
-		dev_err(ipc->dev, "ipc fw reply: %x: Busy\n", header.primary);
-		msg->errno = -EBUSY;
-		break;
+		switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) {
+		case IPC_GLB_LOAD_MULTIPLE_MODS:
+			skl->mod_load_complete = true;
+			skl->mod_load_status = true;
+			wake_up(&skl->mod_load_wait);
+			break;
 
-	default:
-		dev_err(ipc->dev, "Unknown ipc reply: 0x%x\n", reply);
-		msg->errno = -EINVAL;
-		break;
-	}
+		default:
+			break;
 
-	if (reply != IPC_GLB_REPLY_SUCCESS) {
+		}
+	} else {
+		msg->errno = skl_ipc_set_reply_error_code(reply);
 		dev_err(ipc->dev, "ipc FW reply: reply=%d\n", reply);
 		dev_err(ipc->dev, "FW Error Code: %u\n",
 			ipc->dsp->fw_ops.get_fw_errcode(ipc->dsp));
+		switch (IPC_GLB_NOTIFY_MSG_TYPE(header.primary)) {
+		case IPC_GLB_LOAD_MULTIPLE_MODS:
+			skl->mod_load_complete = true;
+			skl->mod_load_status = false;
+			wake_up(&skl->mod_load_wait);
+			break;
+
+		default:
+			break;
+
+		}
 	}
 
 	list_del(&msg->list);
@@ -811,8 +835,8 @@ int skl_ipc_load_modules(struct sst_generic_ipc *ipc,
 	header.primary |= IPC_GLB_TYPE(IPC_GLB_LOAD_MULTIPLE_MODS);
 	header.primary |= IPC_LOAD_MODULE_CNT(module_cnt);
 
-	ret = sst_ipc_tx_message_wait(ipc, *ipc_header, data,
-				(sizeof(u16) * module_cnt), NULL, 0);
+	ret = sst_ipc_tx_message_nowait(ipc, *ipc_header, data,
+				(sizeof(u16) * module_cnt));
 	if (ret < 0)
 		dev_err(ipc->dev, "ipc: load modules failed :%d\n", ret);
 
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h
index 7d21f055328d..fc07c397b060 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.h
+++ b/sound/soc/intel/skylake/skl-sst-ipc.h
@@ -77,6 +77,11 @@ struct skl_sst {
 	wait_queue_head_t boot_wait;
 	bool boot_complete;
 
+	/* module load */
+	wait_queue_head_t mod_load_wait;
+	bool mod_load_complete;
+	bool mod_load_status;
+
 	/* IPC messaging */
 	struct sst_generic_ipc ipc;
 
diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index b30bd384c8d3..39d4aaac73bf 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -52,7 +52,8 @@ static int skl_transfer_firmware(struct sst_dsp *ctx,
 {
 	int ret = 0;
 
-	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, basefw, base_fw_size);
+	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, basefw, base_fw_size,
+								true);
 	if (ret < 0)
 		return ret;
 
@@ -323,22 +324,49 @@ static struct skl_module_table *skl_module_get_from_id(
 	return NULL;
 }
 
-static int skl_transfer_module(struct sst_dsp *ctx,
-			struct skl_load_module_info *module)
+static int skl_transfer_module(struct sst_dsp *ctx, const void *data,
+				u32 size, u16 mod_id)
 {
-	int ret;
+	int ret, bytes_left, curr_pos;
 	struct skl_sst *skl = ctx->thread_context;
+	skl->mod_load_complete = false;
+	init_waitqueue_head(&skl->mod_load_wait);
 
-	ret = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, module->fw->data,
-							module->fw->size);
-	if (ret < 0)
-		return ret;
+	bytes_left = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx, data, size, false);
+	if (bytes_left < 0)
+		return bytes_left;
 
-	ret = skl_ipc_load_modules(&skl->ipc, SKL_NUM_MODULES,
-						(void *)&module->mod_id);
-	if (ret < 0)
+	ret = skl_ipc_load_modules(&skl->ipc, SKL_NUM_MODULES, &mod_id);
+	if (ret < 0) {
 		dev_err(ctx->dev, "Failed to Load module: %d\n", ret);
+		goto out;
+	}
+
+	/*
+	 * if bytes_left > 0 then wait for BDL complete interrupt and
+	 * copy the next chunk till bytes_left is 0. if bytes_left is
+	 * is zero, then wait for load module IPC reply
+	 */
+	while (bytes_left > 0) {
+		curr_pos = size - bytes_left;
+
+		ret = skl_cldma_wait_interruptible(ctx);
+		if (ret < 0)
+			goto out;
+
+		bytes_left = ctx->cl_dev.ops.cl_copy_to_dmabuf(ctx,
+							data + curr_pos,
+							bytes_left, false);
+	}
+
+	ret = wait_event_timeout(skl->mod_load_wait, skl->mod_load_complete,
+				msecs_to_jiffies(SKL_IPC_BOOT_MSECS));
+	if (ret == 0 || !skl->mod_load_status) {
+		dev_err(ctx->dev, "Module Load failed\n");
+		ret = -EIO;
+	}
 
+out:
 	ctx->cl_dev.ops.cl_stop_dma(ctx);
 
 	return ret;
@@ -365,7 +393,8 @@ static int skl_load_module(struct sst_dsp *ctx, u16 mod_id, u8 *guid)
 	}
 
 	if (!module_entry->usage_cnt) {
-		ret = skl_transfer_module(ctx, module_entry->mod_info);
+		ret = skl_transfer_module(ctx, module_entry->mod_info->fw->data,
+				module_entry->mod_info->fw->size, mod_id);
 		if (ret < 0) {
 			dev_err(ctx->dev, "Failed to Load module\n");
 			return ret;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Applied "ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine" to the asoc tree
  2017-03-13 16:41 ` [PATCH V2 08/10] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine jeeja.kp
@ 2017-03-15 18:13   ` Mark Brown
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2017-03-15 18:13 UTC (permalink / raw)
  To: G Kranthi
  Cc: alsa-devel, Vinod Koul, patches.audio, tiwai, broonie,
	liam.r.girdwood, Jeeja KP

The patch

   ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine

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 7bd86a30599de479bd863e18472207337485d339 Mon Sep 17 00:00:00 2001
From: G Kranthi <gudishax.kranthikumar@intel.com>
Date: Mon, 13 Mar 2017 22:11:30 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine

dsp ops is already set in init, so use this in cleanup routine
instead of again retrieving it. Also constify struct skl_dsp_ops.

Signed-off-by: G Kranthi <gudishax.kranthikumar@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/skylake/skl-messages.c | 8 ++------
 sound/soc/intel/skylake/skl-sst-ipc.h  | 2 ++
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index 29523ddcfab0..ba1ec973ded7 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -274,6 +274,7 @@ int skl_init_dsp(struct skl *skl)
 	if (ret < 0)
 		return ret;
 
+	skl->skl_sst->dsp_ops = ops;
 	dev_dbg(bus->dev, "dsp registration status=%d\n", ret);
 
 	return ret;
@@ -284,16 +285,11 @@ int skl_free_dsp(struct skl *skl)
 	struct hdac_ext_bus *ebus = &skl->ebus;
 	struct hdac_bus *bus = ebus_to_hbus(ebus);
 	struct skl_sst *ctx = skl->skl_sst;
-	const struct skl_dsp_ops *ops;
 
 	/* disable  ppcap interrupt */
 	snd_hdac_ext_bus_ppcap_int_enable(&skl->ebus, false);
 
-	ops = skl_get_dsp_ops(skl->pci->device);
-	if (!ops)
-		return -EIO;
-
-	ops->cleanup(bus->dev, ctx);
+	ctx->dsp_ops->cleanup(bus->dev, ctx);
 
 	if (ctx->dsp->addr.lpe)
 		iounmap(ctx->dsp->addr.lpe);
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h
index 9660ace379ab..7d21f055328d 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.h
+++ b/sound/soc/intel/skylake/skl-sst-ipc.h
@@ -105,6 +105,8 @@ struct skl_sst {
 	void (*update_d0i3c)(struct device *dev, bool enable);
 
 	struct skl_d0i3_data d0i3;
+
+	const struct skl_dsp_ops *dsp_ops;
 };
 
 struct skl_ipc_init_instance_msg {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-03-15 18:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 16:41 [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates jeeja.kp
2017-03-13 16:41 ` [PATCH V2 01/10] ASoC: Intel: Skylake: Fix to delete DSP pipe after stopping pipe jeeja.kp
2017-03-13 16:41 ` [PATCH V2 02/10] ASoC: Intel: Skylake: Fix not to stop src pipe in pre pmd event handler jeeja.kp
2017-03-13 16:41 ` [PATCH V2 03/10] ASoC: Intel: bxtn: Disable interrupt when DSP is in D3 jeeja.kp
2017-03-13 16:41 ` [PATCH V2 04/10] ASoC: Intel: bxtn: Update DSP core state in D0 jeeja.kp
2017-03-13 16:41 ` [PATCH V2 05/10] ASoC: Intel: bxtn: Reload the firmware in case of D3 failure jeeja.kp
2017-03-13 16:41 ` [PATCH V2 06/10] ASoC: Intel: Skylake: Remove BE prepare ops jeeja.kp
2017-03-13 16:41 ` [PATCH V2 07/10] ASoC: Intel: Skylake: Disable notifications at boot after DSP FW init jeeja.kp
2017-03-13 16:41 ` [PATCH V2 08/10] ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine jeeja.kp
2017-03-15 18:13   ` Applied "ASoC: Intel: Skylake: Remove get dsp_ops in cleanup routine" to the asoc tree Mark Brown
2017-03-13 16:41 ` [PATCH V2 09/10] ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size jeeja.kp
2017-03-15 18:13   ` Applied "ASoC: Intel: Skylake: Fix module load when module size > DMA buffer size" to the asoc tree Mark Brown
2017-03-13 16:41 ` [PATCH V2 10/10] ASoC: Intel: Skylake: Fix parameter overwrite for KPB Module jeeja.kp
2017-03-14  5:00 ` [PATCH V2 00/10] ASoC: Intel: Skylake: Driver updates Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.