[PATCH v2 00/11] ASoC: Intel: Skylake: More driver updates

[PATCH v2 00/11] ASoC: Intel: Skylake: More driver updates

[jeeja.kp]

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

This patch series provides updates on the following:
 o Fix LLCH register read.
 o Add 16-bit constraint to FE bxt_rt298 machine.
 o Fix DMA position reporting for capture stream.
 o Use the sig_bits to define dai bps caps.
 o Update sig_bits based on converter capability.
 o Rearrangement of code to cleanup SKL SST library.
 o Add support for deferred DSP module bind.

Changes in v2:
	- Addressed Pierre's comments for "Fix DMA position reporting
	  for capture stream" patch.

B, Jayachandran (1):
  ALSA: hda: Fix LLCH register read

G Kranthi (1):
  ASoC: Intel: Skylake: Add 16-bit constraint to FE bxt_rt298 machine

Hardik T Shah (1):
  ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

Jeeja KP (5):
  ASoC: Intel: Skylake: Use the sig_bits to define dai bps capability
  ASoC: hdac_hdmi: Update sig_bits based on converter capability
  ASoC: Intel: Skylake: Rearrangement of code to cleanup SKL SST library
  ASoC: Intel: Skylake: Fix module state after unbind and delete
  ASoC: Intel: Skylake: Add support for deferred DSP module bind

Vinod Koul (3):
  ASoC: Intel: Skylake: Don't unload module when in use
  ASoC: Intel: Skylake: Remove redundant vmixer handler
  ASoC: Intel: Skylake: remove hard coded ACPI path

 sound/hda/hdac_controller.c             |   2 +-
 sound/soc/codecs/hdac_hdmi.c            |   4 +-
 sound/soc/intel/boards/bxt_rt298.c      |   3 +
 sound/soc/intel/skylake/skl-messages.c  |   2 +-
 sound/soc/intel/skylake/skl-nhlt.c      |   7 +-
 sound/soc/intel/skylake/skl-pcm.c       |  95 ++++++++++++++++---
 sound/soc/intel/skylake/skl-sst-dsp.h   |  24 +++--
 sound/soc/intel/skylake/skl-sst-ipc.h   |   8 ++
 sound/soc/intel/skylake/skl-sst-utils.c |  60 ++----------
 sound/soc/intel/skylake/skl-sst.c       |   5 +
 sound/soc/intel/skylake/skl-topology.c  | 159 +++++++++++++++++++++++++-------
 sound/soc/intel/skylake/skl-topology.h  |  17 ++--
 sound/soc/intel/skylake/skl.h           |   1 +
 13 files changed, 263 insertions(+), 124 deletions(-)

-- 
2.5.0

[PATCH v2 01/11] ALSA: hda: Fix LLCH register read

[jeeja.kp]

From: "B, Jayachandran" <jayachandran.b@intel.com>

LLCH is a 16 bit register. Use readw instead of readl API.

Signed-off-by: B, Jayachandran <jayachandran.b@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/hda/hdac_controller.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 0430658..6f1e99c 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -268,7 +268,7 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
 	unsigned int offset;
 	unsigned int counter = 0;
 
-	offset = snd_hdac_chip_readl(bus, LLCH);
+	offset = snd_hdac_chip_readw(bus, LLCH);
 
 	/* Lets walk the linked capabilities list */
 	do {
-- 
2.5.0

Re: [PATCH v2 01/11] ALSA: hda: Fix LLCH register read

[Takashi Iwai]

On Fri, 24 Mar 2017 18:40:24 +0100,
jeeja.kp@intel.com wrote:
> 
> From: "B, Jayachandran" <jayachandran.b@intel.com>
> 
> LLCH is a 16 bit register. Use readw instead of readl API.
> 
> Signed-off-by: B, Jayachandran <jayachandran.b@intel.com>
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>

Acked-by: Takashi Iwai <tiwai@suse.de>


Takashi

> ---
>  sound/hda/hdac_controller.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> index 0430658..6f1e99c 100644
> --- a/sound/hda/hdac_controller.c
> +++ b/sound/hda/hdac_controller.c
> @@ -268,7 +268,7 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
>  	unsigned int offset;
>  	unsigned int counter = 0;
>  
> -	offset = snd_hdac_chip_readl(bus, LLCH);
> +	offset = snd_hdac_chip_readw(bus, LLCH);
>  
>  	/* Lets walk the linked capabilities list */
>  	do {
> -- 
> 2.5.0
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

[PATCH v2 02/11] ASoC: Intel: Skylake: Use the sig_bits to define dai bps capability

[jeeja.kp]

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

For calculating the HDA DMA format, use the max_bps supported by the
DAI caps instead of fixing it to 32/24. For host DMA the Max bps support
is 32, but in case of link DMA, this depends on the codec capability.
So use the sig_bits to define the bps supported by dai.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c      | 25 ++++++++++++++++++++++---
 sound/soc/intel/skylake/skl-topology.c |  2 ++
 sound/soc/intel/skylake/skl-topology.h |  2 ++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 2f90bc4..3c61dba 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -155,7 +155,7 @@ int skl_pcm_host_dma_prepare(struct device *dev, struct skl_pipe_params *params)
 	snd_hdac_ext_stream_decouple(ebus, stream, true);
 
 	format_val = snd_hdac_calc_stream_format(params->s_freq,
-				params->ch, params->format, 32, 0);
+			params->ch, params->format, params->host_bps, 0);
 
 	dev_dbg(dev, "format_val=%d, rate=%d, ch=%d, format=%d\n",
 		format_val, params->s_freq, params->ch, params->format);
@@ -190,8 +190,8 @@ int skl_pcm_link_dma_prepare(struct device *dev, struct skl_pipe_params *params)
 
 	stream = stream_to_hdac_ext_stream(hstream);
 	snd_hdac_ext_stream_decouple(ebus, stream, true);
-	format_val = snd_hdac_calc_stream_format(params->s_freq,
-				params->ch, params->format, 24, 0);
+	format_val = snd_hdac_calc_stream_format(params->s_freq, params->ch,
+					params->format, params->link_bps, 0);
 
 	dev_dbg(dev, "format_val=%d, rate=%d, ch=%d, format=%d\n",
 		format_val, params->s_freq, params->ch, params->format);
@@ -309,6 +309,11 @@ static int skl_pcm_hw_params(struct snd_pcm_substream *substream,
 	p_params.host_dma_id = dma_id;
 	p_params.stream = substream->stream;
 	p_params.format = params_format(params);
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		p_params.host_bps = dai->driver->playback.sig_bits;
+	else
+		p_params.host_bps = dai->driver->capture.sig_bits;
+
 
 	m_cfg = skl_tplg_fe_get_cpr_module(dai, p_params.stream);
 	if (m_cfg)
@@ -547,6 +552,11 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
 	p_params.link_index = link->index;
 	p_params.format = params_format(params);
 
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		p_params.link_bps = codec_dai->driver->playback.sig_bits;
+	else
+		p_params.link_bps = codec_dai->driver->capture.sig_bits;
+
 	return skl_tplg_be_update_params(dai, &p_params);
 }
 
@@ -652,6 +662,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_8000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE |
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE,
+		.sig_bits = 32,
 	},
 	.capture = {
 		.stream_name = "System Capture",
@@ -659,6 +670,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.channels_max = HDA_STEREO,
 		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -670,6 +682,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.channels_max = HDA_QUAD,
 		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -681,6 +694,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.channels_max = HDA_STEREO,
 		.rates = SNDRV_PCM_RATE_48000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -692,6 +706,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.channels_max = HDA_STEREO,
 		.rates = SNDRV_PCM_RATE_48000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -703,6 +718,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 		.channels_max = HDA_QUAD,
 		.rates = SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_16000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -718,6 +734,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 			SNDRV_PCM_RATE_192000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
 			SNDRV_PCM_FMTBIT_S32_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -733,6 +750,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 			SNDRV_PCM_RATE_192000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
 			SNDRV_PCM_FMTBIT_S32_LE,
+		.sig_bits = 32,
 	},
 },
 {
@@ -748,6 +766,7 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 			SNDRV_PCM_RATE_192000,
 		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE |
 			SNDRV_PCM_FMTBIT_S32_LE,
+		.sig_bits = 32,
 	},
 },
 
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index c6bd4bb..43f9cb3 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1242,10 +1242,12 @@ static void skl_tplg_fill_dma_id(struct skl_module_cfg *mcfg,
 		case SKL_DEVICE_HDALINK:
 			pipe->p_params->link_dma_id = params->link_dma_id;
 			pipe->p_params->link_index = params->link_index;
+			pipe->p_params->link_bps = params->link_bps;
 			break;
 
 		case SKL_DEVICE_HDAHOST:
 			pipe->p_params->host_dma_id = params->host_dma_id;
+			pipe->p_params->host_bps = params->host_bps;
 			break;
 
 		default:
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index fefab0e..bf2c63b 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -257,6 +257,8 @@ struct skl_pipe_params {
 	snd_pcm_format_t format;
 	int link_index;
 	int stream;
+	unsigned int host_bps;
+	unsigned int link_bps;
 };
 
 struct skl_pipe {
-- 
2.5.0

[PATCH v2 03/11] ASoC: hdac_hdmi: Update sig_bits based on converter capability

[jeeja.kp]

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

When creating the codec dai, use sig_bits to update the max bps based
on the codec capability. So both the link DMA and codec format will be
calculated based on DAI sig_bits.

So update the sig_bits with converter capability and use the sig_bits
for HDA format calculation.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 78fca8a..5b76947 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -469,7 +469,7 @@ static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
 
 	format = snd_hdac_calc_stream_format(params_rate(hparams),
 			params_channels(hparams), params_format(hparams),
-			24, 0);
+			dai->driver->playback.sig_bits, 0);
 
 	pcm = hdac_hdmi_get_pcm_from_cvt(hdmi, dai_map->cvt);
 	if (!pcm)
@@ -1419,8 +1419,8 @@ static int hdac_hdmi_create_dais(struct hdac_device *hdac,
 		hdmi_dais[i].playback.rate_min = rate_min;
 		hdmi_dais[i].playback.channels_min = 2;
 		hdmi_dais[i].playback.channels_max = 2;
+		hdmi_dais[i].playback.sig_bits = bps;
 		hdmi_dais[i].ops = &hdmi_dai_ops;
-
 		i++;
 	}
 
-- 
2.5.0

[PATCH v2 04/11] ASoC: Intel: Skylake: Add 16-bit constraint to FE bxt_rt298 machine

[jeeja.kp]

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

Add constraint to FE to restrict sample format to 16-bit for bxt_rt298
machine

Signed-off-by: G Kranthi <gudishax.kranthikumar@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/boards/bxt_rt298.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/intel/boards/bxt_rt298.c b/sound/soc/intel/boards/bxt_rt298.c
index 176c080..1a68d04 100644
--- a/sound/soc/intel/boards/bxt_rt298.c
+++ b/sound/soc/intel/boards/bxt_rt298.c
@@ -274,12 +274,15 @@ static int bxt_fe_startup(struct snd_pcm_substream *substream)
 	 * on this platform for PCM device we support:
 	 *      48Khz
 	 *      stereo
+	 *	16-bit audio
 	 */
 
 	runtime->hw.channels_max = 2;
 	snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
 				&constraints_channels);
 
+	runtime->hw.formats = SNDRV_PCM_FMTBIT_S16_LE;
+	snd_pcm_hw_constraint_msbits(runtime, 0, 16, 16);
 	snd_pcm_hw_constraint_list(runtime, 0,
 				SNDRV_PCM_HW_PARAM_RATE, &constraints_rates);
 
-- 
2.5.0

[PATCH v2 05/11] ASoC: Intel: Skylake: Don't unload module when in use

[jeeja.kp]

From: Vinod Koul <vinod.koul@intel.com>

A module may have multiple instances in DSP, so unload only when usage
count is zero.

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

diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index 39d4aaa..5395297 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -417,6 +417,11 @@ static int skl_unload_module(struct sst_dsp *ctx, u16 mod_id)
 		dev_err(ctx->dev, "Module bad usage cnt!:%d\n", usage_cnt);
 		return -EIO;
 	}
+
+	/* if module is used by others return, no need to unload */
+	if (usage_cnt > 0)
+		return 0;
+
 	ret = skl_ipc_unload_modules(&skl->ipc,
 			SKL_NUM_MODULES, &mod_id);
 	if (ret < 0) {
-- 
2.5.0

[PATCH v2 06/11] ASoC: Intel: Skylake: Remove redundant vmixer handler

[jeeja.kp]

From: Vinod Koul <vinod.koul@intel.com>

Initially vmixer and mixer widget handlers were bit different, but over
time they became same so remove the duplicate code.

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-topology.c | 32 +-------------------------------
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 43f9cb3..35d9d7b 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1073,36 +1073,6 @@ static int skl_tplg_pga_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 }
 
 /*
- * In modelling, we assume there will be ONLY one mixer in a pipeline.  If
- * mixer is not required then it is treated as static mixer aka vmixer with
- * a hard path to source module
- * So we don't need to check if source is started or not as hard path puts
- * dependency on each other
- */
-static int skl_tplg_vmixer_event(struct snd_soc_dapm_widget *w,
-				struct snd_kcontrol *k, int event)
-{
-	struct snd_soc_dapm_context *dapm = w->dapm;
-	struct skl *skl = get_skl_ctx(dapm->dev);
-
-	switch (event) {
-	case SND_SOC_DAPM_PRE_PMU:
-		return skl_tplg_mixer_dapm_pre_pmu_event(w, skl);
-
-	case SND_SOC_DAPM_POST_PMU:
-		return skl_tplg_mixer_dapm_post_pmu_event(w, skl);
-
-	case SND_SOC_DAPM_PRE_PMD:
-		return skl_tplg_mixer_dapm_pre_pmd_event(w, skl);
-
-	case SND_SOC_DAPM_POST_PMD:
-		return skl_tplg_mixer_dapm_post_pmd_event(w, skl);
-	}
-
-	return 0;
-}
-
-/*
  * In modelling, we assume there will be ONLY one mixer in a pipeline. If a
  * second one is required that is created as another pipe entity.
  * The mixer is responsible for pipe management and represent a pipeline
@@ -1570,7 +1540,7 @@ int skl_tplg_be_update_params(struct snd_soc_dai *dai,
 
 static const struct snd_soc_tplg_widget_events skl_tplg_widget_ops[] = {
 	{SKL_MIXER_EVENT, skl_tplg_mixer_event},
-	{SKL_VMIXER_EVENT, skl_tplg_vmixer_event},
+	{SKL_VMIXER_EVENT, skl_tplg_mixer_event},
 	{SKL_PGA_EVENT, skl_tplg_pga_event},
 };
 
-- 
2.5.0

[PATCH v2 07/11] ASoC: Intel: Skylake: remove hard coded ACPI path

[jeeja.kp]

From: Vinod Koul <vinod.koul@intel.com>

We should not hard code the ACPI path to get acpi_handle. Instead use
ACPI_HANDLE macro to do the job.

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-nhlt.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-nhlt.c b/sound/soc/intel/skylake/skl-nhlt.c
index 7eb9c41..e3f0667 100644
--- a/sound/soc/intel/skylake/skl-nhlt.c
+++ b/sound/soc/intel/skylake/skl-nhlt.c
@@ -24,8 +24,6 @@
 static u8 OSC_UUID[16] = {0x6E, 0x88, 0x9F, 0xA6, 0xEB, 0x6C, 0x94, 0x45,
 				0xA4, 0x1F, 0x7B, 0x5D, 0xCE, 0x24, 0xC5, 0x53};
 
-#define DSDT_NHLT_PATH "\\_SB.PCI0.HDAS"
-
 struct nhlt_acpi_table *skl_nhlt_init(struct device *dev)
 {
 	acpi_handle handle;
@@ -33,8 +31,9 @@ struct nhlt_acpi_table *skl_nhlt_init(struct device *dev)
 	struct nhlt_resource_desc  *nhlt_ptr = NULL;
 	struct nhlt_acpi_table *nhlt_table = NULL;
 
-	if (ACPI_FAILURE(acpi_get_handle(NULL, DSDT_NHLT_PATH, &handle))) {
-		dev_err(dev, "Requested NHLT device not found\n");
+	handle = ACPI_HANDLE(dev);
+	if (!handle) {
+		dev_err(dev, "Didn't find ACPI_HANDLE\n");
 		return NULL;
 	}
 
-- 
2.5.0

[PATCH v2 08/11] ASoC: Intel: Skylake: Rearrangement of code to cleanup SKL SST library

[jeeja.kp]

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

Skylake driver topology header/driver structure is referenced and used
in SST library which creates circular dependency. Hence the
rearrangement.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c       | 35 +++++++++++++++----
 sound/soc/intel/skylake/skl-sst-dsp.h   | 24 +++++++++----
 sound/soc/intel/skylake/skl-sst-ipc.h   |  8 +++++
 sound/soc/intel/skylake/skl-sst-utils.c | 60 +++++----------------------------
 sound/soc/intel/skylake/skl-topology.c  | 11 ++++--
 sound/soc/intel/skylake/skl-topology.h  | 13 -------
 6 files changed, 70 insertions(+), 81 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 3c61dba..ef440d8 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1174,29 +1174,52 @@ static int skl_pcm_new(struct snd_soc_pcm_runtime *rtd)
 	return retval;
 }
 
+static int skl_get_module_info(struct skl *skl, struct skl_module_cfg *mconfig)
+{
+	struct skl_sst *ctx = skl->skl_sst;
+	struct uuid_module *module;
+	uuid_le *uuid_mod;
+
+	uuid_mod = (uuid_le *)mconfig->guid;
+
+	if (list_empty(&ctx->uuid_list)) {
+		dev_err(ctx->dev, "Module list is empty\n");
+		return -EIO;
+	}
+
+	list_for_each_entry(module, &ctx->uuid_list, list) {
+		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
+			mconfig->id.module_id = module->id;
+			mconfig->is_loadable = module->is_loadable;
+			return 0;
+		}
+	}
+
+	return -EIO;
+}
+
 static int skl_populate_modules(struct skl *skl)
 {
 	struct skl_pipeline *p;
 	struct skl_pipe_module *m;
 	struct snd_soc_dapm_widget *w;
 	struct skl_module_cfg *mconfig;
-	int ret;
+	int ret = 0;
 
 	list_for_each_entry(p, &skl->ppl_list, node) {
 		list_for_each_entry(m, &p->pipe->w_list, node) {
-
 			w = m->w;
 			mconfig = w->priv;
 
-			ret = snd_skl_get_module_info(skl->skl_sst, mconfig);
+			ret = skl_get_module_info(skl, mconfig);
 			if (ret < 0) {
 				dev_err(skl->skl_sst->dev,
-					"query module info failed:%d\n", ret);
-				goto err;
+					"query module info failed\n");
+				return ret;
 			}
 		}
 	}
-err:
+
 	return ret;
 }
 
diff --git a/sound/soc/intel/skylake/skl-sst-dsp.h b/sound/soc/intel/skylake/skl-sst-dsp.h
index 5d7a93a..7229a12 100644
--- a/sound/soc/intel/skylake/skl-sst-dsp.h
+++ b/sound/soc/intel/skylake/skl-sst-dsp.h
@@ -17,13 +17,14 @@
 #define __SKL_SST_DSP_H__
 
 #include <linux/interrupt.h>
+#include <linux/uuid.h>
 #include <sound/memalloc.h>
 #include "skl-sst-cldma.h"
-#include "skl-topology.h"
 
 struct sst_dsp;
 struct skl_sst;
 struct sst_dsp_device;
+struct skl_lib_info;
 
 /* Intel HD Audio General DSP Registers */
 #define SKL_ADSP_GEN_BASE		0x0
@@ -172,6 +173,19 @@ struct skl_dsp_loader_ops {
 				 int stream_tag);
 };
 
+#define MAX_INSTANCE_BUFF 2
+
+struct uuid_module {
+	uuid_le uuid;
+	int id;
+	int is_loadable;
+	int max_instance;
+	u64 pvt_id[MAX_INSTANCE_BUFF];
+	int *instance_id;
+
+	struct list_head list;
+};
+
 struct skl_load_module_info {
 	u16 mod_id;
 	const struct firmware *fw;
@@ -223,14 +237,10 @@ int bxt_sst_init_fw(struct device *dev, struct skl_sst *ctx);
 void skl_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx);
 void bxt_sst_dsp_cleanup(struct device *dev, struct skl_sst *ctx);
 
-int snd_skl_get_module_info(struct skl_sst *ctx,
-				struct skl_module_cfg *mconfig);
 int snd_skl_parse_uuids(struct sst_dsp *ctx, const struct firmware *fw,
 				unsigned int offset, int index);
-int skl_get_pvt_id(struct skl_sst *ctx,
-				struct skl_module_cfg *mconfig);
-int skl_put_pvt_id(struct skl_sst *ctx,
-				struct skl_module_cfg *mconfig);
+int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id);
+int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id);
 int skl_get_pvt_instance_id_map(struct skl_sst *ctx,
 				int module_id, int instance_id);
 void skl_freeup_uuid_list(struct skl_sst *ctx);
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.h b/sound/soc/intel/skylake/skl-sst-ipc.h
index fc07c39..4abf98c 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.h
+++ b/sound/soc/intel/skylake/skl-sst-ipc.h
@@ -69,6 +69,14 @@ struct skl_d0i3_data {
 	struct delayed_work work;
 };
 
+#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_sst {
 	struct device *dev;
 	struct sst_dsp *dsp;
diff --git a/sound/soc/intel/skylake/skl-sst-utils.c b/sound/soc/intel/skylake/skl-sst-utils.c
index ea162fb..6d5bff0 100644
--- a/sound/soc/intel/skylake/skl-sst-utils.c
+++ b/sound/soc/intel/skylake/skl-sst-utils.c
@@ -94,19 +94,6 @@ struct adsp_fw_hdr {
 	u32 load_offset;
 } __packed;
 
-#define MAX_INSTANCE_BUFF 2
-
-struct uuid_module {
-	uuid_le uuid;
-	int id;
-	int is_loadable;
-	int max_instance;
-	u64 pvt_id[MAX_INSTANCE_BUFF];
-	int *instance_id;
-
-	struct list_head list;
-};
-
 struct skl_ext_manifest_hdr {
 	u32 id;
 	u32 len;
@@ -115,32 +102,6 @@ struct skl_ext_manifest_hdr {
 	u32 entries;
 };
 
-int snd_skl_get_module_info(struct skl_sst *ctx,
-				struct skl_module_cfg *mconfig)
-{
-	struct uuid_module *module;
-	uuid_le *uuid_mod;
-
-	uuid_mod = (uuid_le *)mconfig->guid;
-
-	if (list_empty(&ctx->uuid_list)) {
-		dev_err(ctx->dev, "Module list is empty\n");
-		return -EINVAL;
-	}
-
-	list_for_each_entry(module, &ctx->uuid_list, list) {
-		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
-			mconfig->id.module_id = module->id;
-			mconfig->is_loadable = module->is_loadable;
-
-			return 0;
-		}
-	}
-
-	return -EINVAL;
-}
-EXPORT_SYMBOL_GPL(snd_skl_get_module_info);
-
 static int skl_get_pvtid_map(struct uuid_module *module, int instance_id)
 {
 	int pvt_id;
@@ -222,21 +183,18 @@ static inline int skl_pvtid_128(struct uuid_module *module)
  * This generates a 128 bit private unique id for a module TYPE so that
  * module instance is unique
  */
-int skl_get_pvt_id(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
+int skl_get_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int instance_id)
 {
 	struct uuid_module *module;
-	uuid_le *uuid_mod;
 	int pvt_id;
 
-	uuid_mod = (uuid_le *)mconfig->guid;
-
 	list_for_each_entry(module, &ctx->uuid_list, list) {
 		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
 
 			pvt_id = skl_pvtid_128(module);
 			if (pvt_id >= 0) {
-				module->instance_id[pvt_id] =
-						mconfig->id.instance_id;
+				module->instance_id[pvt_id] = instance_id;
+
 				return pvt_id;
 			}
 		}
@@ -254,23 +212,21 @@ EXPORT_SYMBOL_GPL(skl_get_pvt_id);
  *
  * This frees a 128 bit private unique id previously generated
  */
-int skl_put_pvt_id(struct skl_sst *ctx, struct skl_module_cfg *mconfig)
+int skl_put_pvt_id(struct skl_sst *ctx, uuid_le *uuid_mod, int *pvt_id)
 {
 	int i;
-	uuid_le *uuid_mod;
 	struct uuid_module *module;
 
-	uuid_mod = (uuid_le *)mconfig->guid;
 	list_for_each_entry(module, &ctx->uuid_list, list) {
 		if (uuid_le_cmp(*uuid_mod, module->uuid) == 0) {
 
-			if (mconfig->id.pvt_id != 0)
-				i = (mconfig->id.pvt_id) / 64;
+			if (*pvt_id != 0)
+				i = (*pvt_id) / 64;
 			else
 				i = 0;
 
-			module->pvt_id[i] &= ~(1 << (mconfig->id.pvt_id));
-			mconfig->id.pvt_id = -1;
+			module->pvt_id[i] &= ~(1 << (*pvt_id));
+			*pvt_id = -1;
 			return 0;
 		}
 	}
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 35d9d7b..8bd5ded 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -539,6 +539,7 @@ skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 	int ret = 0;
 
 	list_for_each_entry(w_module, &pipe->w_list, node) {
+		uuid_le *uuid_mod;
 		w = w_module->w;
 		mconfig = w->priv;
 
@@ -576,13 +577,15 @@ skl_tplg_init_pipe_modules(struct skl *skl, struct skl_pipe *pipe)
 		 * FE/BE params
 		 */
 		skl_tplg_update_module_params(w, ctx);
-		mconfig->id.pvt_id = skl_get_pvt_id(ctx, mconfig);
+		uuid_mod = (uuid_le *)mconfig->guid;
+		mconfig->id.pvt_id = skl_get_pvt_id(ctx, uuid_mod,
+						mconfig->id.instance_id);
 		if (mconfig->id.pvt_id < 0)
 			return ret;
 		skl_tplg_set_module_init_data(w);
 		ret = skl_init_module(ctx, mconfig);
 		if (ret < 0) {
-			skl_put_pvt_id(ctx, mconfig);
+			skl_put_pvt_id(ctx, uuid_mod, &mconfig->id.pvt_id);
 			return ret;
 		}
 		skl_tplg_alloc_pipe_mcps(skl, mconfig);
@@ -602,7 +605,9 @@ static int skl_tplg_unload_pipe_modules(struct skl_sst *ctx,
 	struct skl_module_cfg *mconfig = NULL;
 
 	list_for_each_entry(w_module, &pipe->w_list, node) {
+		uuid_le *uuid_mod;
 		mconfig  = w_module->w->priv;
+		uuid_mod = (uuid_le *)mconfig->guid;
 
 		if (mconfig->is_loadable && ctx->dsp->fw_ops.unload_mod &&
 			mconfig->m_state > SKL_MODULE_UNINIT) {
@@ -611,7 +616,7 @@ static int skl_tplg_unload_pipe_modules(struct skl_sst *ctx,
 			if (ret < 0)
 				return -EIO;
 		}
-		skl_put_pvt_id(ctx, mconfig);
+		skl_put_pvt_id(ctx, uuid_mod, &mconfig->id.pvt_id);
 	}
 
 	/* no modules to unload in this path, so return */
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index bf2c63b..8536d03 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -336,19 +336,6 @@ 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);
-- 
2.5.0

[PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[jeeja.kp]

From: Hardik T Shah <hardik.t.shah@intel.com>

As per hardware recommendation, for every capture stream completion
following operations need to be done in order to reflect the actual
data that is received in position buffer.

1. Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion.
2. Read any of the register to flush the DMA position value. This is
dummy read operation.

Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index ef440d8..1823197 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -21,6 +21,7 @@
 
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
+#include <linux/delay.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "skl.h"
@@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer
 	 * HAD space reflects the actual data that is transferred.
 	 * Use the position buffer for capture, as DPIB write gets
 	 * completed earlier than the actual data written to the DDR.
+	 *
+	 * For capture stream following workaround is required to fix the
+	 * incorrect position reporting.
+	 *
+	 * 1. Wait for 20us before reading the DMA position in buffer once
+	 * the interrupt is generated for stream completion as update happens
+	 * on the HDA frame boundary i.e. 20.833uSec.
+	 * 2. Read DPIB register to flush the DMA position value. This dummy
+	 * read is required to flush DMA position value.
+	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
+	 * or greater than period boundary.
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE +
 				(AZX_REG_VS_SDXDPIB_XINTERVAL *
 				hdac_stream(hstream)->index));
-	else
+	} else {
+		udelay(20);
+		readl(ebus->bus.remap_addr +
+				AZX_REG_VS_SDXDPIB_XBASE +
+				(AZX_REG_VS_SDXDPIB_XINTERVAL *
+				 hdac_stream(hstream)->index));
 		pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
+	}
 
 	if (pos >= hdac_stream(hstream)->bufsize)
 		pos = 0;
-- 
2.5.0

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Takashi Iwai]

On Fri, 24 Mar 2017 18:40:32 +0100,
jeeja.kp@intel.com wrote:
> 
> From: Hardik T Shah <hardik.t.shah@intel.com>
> 
> As per hardware recommendation, for every capture stream completion
> following operations need to be done in order to reflect the actual
> data that is received in position buffer.
> 
> 1. Wait for 20us before reading the DMA position in buffer once the
> interrupt is generated for stream completion.
> 2. Read any of the register to flush the DMA position value. This is
> dummy read operation.

Are these workarounds needed for the legacy driver?
If yes, which chips require it?


thanks,

Takashi

> 
> Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> ---
>  sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index ef440d8..1823197 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -21,6 +21,7 @@
>  
>  #include <linux/pci.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/delay.h>
>  #include <sound/pcm_params.h>
>  #include <sound/soc.h>
>  #include "skl.h"
> @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer
>  	 * HAD space reflects the actual data that is transferred.
>  	 * Use the position buffer for capture, as DPIB write gets
>  	 * completed earlier than the actual data written to the DDR.
> +	 *
> +	 * For capture stream following workaround is required to fix the
> +	 * incorrect position reporting.
> +	 *
> +	 * 1. Wait for 20us before reading the DMA position in buffer once
> +	 * the interrupt is generated for stream completion as update happens
> +	 * on the HDA frame boundary i.e. 20.833uSec.
> +	 * 2. Read DPIB register to flush the DMA position value. This dummy
> +	 * read is required to flush DMA position value.
> +	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
> +	 * or greater than period boundary.
>  	 */
> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +
> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>  		pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE +
>  				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>  				hdac_stream(hstream)->index));
> -	else
> +	} else {
> +		udelay(20);
> +		readl(ebus->bus.remap_addr +
> +				AZX_REG_VS_SDXDPIB_XBASE +
> +				(AZX_REG_VS_SDXDPIB_XINTERVAL *
> +				 hdac_stream(hstream)->index));
>  		pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
> +	}
>  
>  	if (pos >= hdac_stream(hstream)->bufsize)
>  		pos = 0;
> -- 
> 2.5.0
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Ughreja, Rakesh A]

>-----Original Message-----
>From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-
>project.org] On Behalf Of Takashi Iwai
>Sent: Friday, March 24, 2017 11:38 AM
>To: Kp, Jeeja <jeeja.kp@intel.com>
>Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>;
>Patches Audio <patches.audio@intel.com>; Shah, Hardik T
><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R
><liam.r.girdwood@intel.com>
>Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
>position reporting for capture stream
>
>On Fri, 24 Mar 2017 18:40:32 +0100,
>jeeja.kp@intel.com wrote:
>>
>> From: Hardik T Shah <hardik.t.shah@intel.com>
>>
>> As per hardware recommendation, for every capture stream completion
>> following operations need to be done in order to reflect the actual
>> data that is received in position buffer.
>>
>> 1. Wait for 20us before reading the DMA position in buffer once the
>> interrupt is generated for stream completion.
>> 2. Read any of the register to flush the DMA position value. This is
>> dummy read operation.
>
>Are these workarounds needed for the legacy driver?
>If yes, which chips require it?
>

Yes, these are needed in legacy driver as well.
>From SKL and BXT onwards, it is needed.

>
>thanks,
>
>Takashi
>
>>
>> Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
>> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
>> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
>> ---
>>  sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/intel/skylake/skl-pcm.c
>b/sound/soc/intel/skylake/skl-pcm.c
>> index ef440d8..1823197 100644
>> --- a/sound/soc/intel/skylake/skl-pcm.c
>> +++ b/sound/soc/intel/skylake/skl-pcm.c
>> @@ -21,6 +21,7 @@
>>
>>  #include <linux/pci.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/delay.h>
>>  #include <sound/pcm_params.h>
>>  #include <sound/soc.h>
>>  #include "skl.h"
>> @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
>skl_platform_pcm_pointer
>>  	 * HAD space reflects the actual data that is transferred.
>>  	 * Use the position buffer for capture, as DPIB write gets
>>  	 * completed earlier than the actual data written to the DDR.
>> +	 *
>> +	 * For capture stream following workaround is required to fix the
>> +	 * incorrect position reporting.
>> +	 *
>> +	 * 1. Wait for 20us before reading the DMA position in buffer once
>> +	 * the interrupt is generated for stream completion as update happens
>> +	 * on the HDA frame boundary i.e. 20.833uSec.
>> +	 * 2. Read DPIB register to flush the DMA position value. This dummy
>> +	 * read is required to flush DMA position value.
>> +	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
>> +	 * or greater than period boundary.
>>  	 */
>> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> +
>> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>>  		pos = readl(ebus->bus.remap_addr +
>AZX_REG_VS_SDXDPIB_XBASE +
>>  				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>>  				hdac_stream(hstream)->index));
>> -	else
>> +	} else {
>> +		udelay(20);
>> +		readl(ebus->bus.remap_addr +
>> +				AZX_REG_VS_SDXDPIB_XBASE +
>> +				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>> +				 hdac_stream(hstream)->index));
>>  		pos =
>snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
>> +	}
>>
>>  	if (pos >= hdac_stream(hstream)->bufsize)
>>  		pos = 0;
>> --
>> 2.5.0
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>_______________________________________________
>Alsa-devel mailing list
>Alsa-devel@alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Takashi Iwai]

On Fri, 24 Mar 2017 19:43:52 +0100,
Ughreja, Rakesh A wrote:
> 
> 
> 
> >-----Original Message-----
> >From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-
> >project.org] On Behalf Of Takashi Iwai
> >Sent: Friday, March 24, 2017 11:38 AM
> >To: Kp, Jeeja <jeeja.kp@intel.com>
> >Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>;
> >Patches Audio <patches.audio@intel.com>; Shah, Hardik T
> ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R
> ><liam.r.girdwood@intel.com>
> >Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
> >position reporting for capture stream
> >
> >On Fri, 24 Mar 2017 18:40:32 +0100,
> >jeeja.kp@intel.com wrote:
> >>
> >> From: Hardik T Shah <hardik.t.shah@intel.com>
> >>
> >> As per hardware recommendation, for every capture stream completion
> >> following operations need to be done in order to reflect the actual
> >> data that is received in position buffer.
> >>
> >> 1. Wait for 20us before reading the DMA position in buffer once the
> >> interrupt is generated for stream completion.
> >> 2. Read any of the register to flush the DMA position value. This is
> >> dummy read operation.
> >
> >Are these workarounds needed for the legacy driver?
> >If yes, which chips require it?
> >
> 
> Yes, these are needed in legacy driver as well.
> From SKL and BXT onwards, it is needed.

OK, thanks for confirmation.

Now, from what I read in the above, is the workaround required *only*
after the interrupt is generated?  20us delay isn't so cheap, and we
tend to inquire PCM positions often.  If the workaround is needed only
after the PCM period elapse, we can set some flag in the irq handler
and apply the workaround only when necessary.


Takashi

> 
> >
> >thanks,
> >
> >Takashi
> >
> >>
> >> Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
> >> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
> >> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
> >> ---
> >>  sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
> >>  1 file changed, 21 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/sound/soc/intel/skylake/skl-pcm.c
> >b/sound/soc/intel/skylake/skl-pcm.c
> >> index ef440d8..1823197 100644
> >> --- a/sound/soc/intel/skylake/skl-pcm.c
> >> +++ b/sound/soc/intel/skylake/skl-pcm.c
> >> @@ -21,6 +21,7 @@
> >>
> >>  #include <linux/pci.h>
> >>  #include <linux/pm_runtime.h>
> >> +#include <linux/delay.h>
> >>  #include <sound/pcm_params.h>
> >>  #include <sound/soc.h>
> >>  #include "skl.h"
> >> @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
> >skl_platform_pcm_pointer
> >>  	 * HAD space reflects the actual data that is transferred.
> >>  	 * Use the position buffer for capture, as DPIB write gets
> >>  	 * completed earlier than the actual data written to the DDR.
> >> +	 *
> >> +	 * For capture stream following workaround is required to fix the
> >> +	 * incorrect position reporting.
> >> +	 *
> >> +	 * 1. Wait for 20us before reading the DMA position in buffer once
> >> +	 * the interrupt is generated for stream completion as update happens
> >> +	 * on the HDA frame boundary i.e. 20.833uSec.
> >> +	 * 2. Read DPIB register to flush the DMA position value. This dummy
> >> +	 * read is required to flush DMA position value.
> >> +	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
> >> +	 * or greater than period boundary.
> >>  	 */
> >> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> >> +
> >> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> >>  		pos = readl(ebus->bus.remap_addr +
> >AZX_REG_VS_SDXDPIB_XBASE +
> >>  				(AZX_REG_VS_SDXDPIB_XINTERVAL *
> >>  				hdac_stream(hstream)->index));
> >> -	else
> >> +	} else {
> >> +		udelay(20);
> >> +		readl(ebus->bus.remap_addr +
> >> +				AZX_REG_VS_SDXDPIB_XBASE +
> >> +				(AZX_REG_VS_SDXDPIB_XINTERVAL *
> >> +				 hdac_stream(hstream)->index));
> >>  		pos =
> >snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
> >> +	}
> >>
> >>  	if (pos >= hdac_stream(hstream)->bufsize)
> >>  		pos = 0;
> >> --
> >> 2.5.0
> >>
> >> _______________________________________________
> >> Alsa-devel mailing list
> >> Alsa-devel@alsa-project.org
> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >>
> >_______________________________________________
> >Alsa-devel mailing list
> >Alsa-devel@alsa-project.org
> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Ughreja, Rakesh A]

>-----Original Message-----
>From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa-
>project.org] On Behalf Of Takashi Iwai
>Sent: Friday, March 24, 2017 11:50 AM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>
>Cc: alsa-devel@alsa-project.org; R, Dharageswari <dharageswari.r@intel.com>;
>Patches Audio <patches.audio@intel.com>; Shah, Hardik T
><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R
><liam.r.girdwood@intel.com>; Kp, Jeeja <jeeja.kp@intel.com>
>Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
>position reporting for capture stream
>
>On Fri, 24 Mar 2017 19:43:52 +0100,
>Ughreja, Rakesh A wrote:
>>
>>
>>
>> >-----Original Message-----
>> >From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-
>bounces@alsa-
>> >project.org] On Behalf Of Takashi Iwai
>> >Sent: Friday, March 24, 2017 11:38 AM
>> >To: Kp, Jeeja <jeeja.kp@intel.com>
>> >Cc: alsa-devel@alsa-project.org; R, Dharageswari
><dharageswari.r@intel.com>;
>> >Patches Audio <patches.audio@intel.com>; Shah, Hardik T
>> ><hardik.t.shah@intel.com>; broonie@kernel.org; Girdwood, Liam R
>> ><liam.r.girdwood@intel.com>
>> >Subject: Re: [alsa-devel] [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA
>> >position reporting for capture stream
>> >
>> >On Fri, 24 Mar 2017 18:40:32 +0100,
>> >jeeja.kp@intel.com wrote:
>> >>
>> >> From: Hardik T Shah <hardik.t.shah@intel.com>
>> >>
>> >> As per hardware recommendation, for every capture stream completion
>> >> following operations need to be done in order to reflect the actual
>> >> data that is received in position buffer.
>> >>
>> >> 1. Wait for 20us before reading the DMA position in buffer once the
>> >> interrupt is generated for stream completion.
>> >> 2. Read any of the register to flush the DMA position value. This is
>> >> dummy read operation.
>> >
>> >Are these workarounds needed for the legacy driver?
>> >If yes, which chips require it?
>> >
>>
>> Yes, these are needed in legacy driver as well.
>> From SKL and BXT onwards, it is needed.
>
>OK, thanks for confirmation.
>
>Now, from what I read in the above, is the workaround required *only*
>after the interrupt is generated?  20us delay isn't so cheap, and we
>tend to inquire PCM positions often.  If the workaround is needed only
>after the PCM period elapse, we can set some flag in the irq handler
>and apply the workaround only when necessary.
>

Yes, Takashi the workaround is required only in the period elapsed
interrupt. In some cases the DMA Position updates are delayed and so
when the period elapsed interrupt occurs the wait_for_avail thinks that
one period worth of data is not available and so returns only on the 
next period elapsed interrupt. This creates problem for 2 periods
playback/capture streams.

So even in the period elapsed interrupt the wait is required only if the
position is less than the period boundary.

>
>Takashi
>
>>
>> >
>> >thanks,
>> >
>> >Takashi
>> >
>> >>
>> >> Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
>> >> Signed-off-by: Hardik T Shah <hardik.t.shah@intel.com>
>> >> Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
>> >> ---
>> >>  sound/soc/intel/skylake/skl-pcm.c | 23 +++++++++++++++++++++--
>> >>  1 file changed, 21 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/sound/soc/intel/skylake/skl-pcm.c
>> >b/sound/soc/intel/skylake/skl-pcm.c
>> >> index ef440d8..1823197 100644
>> >> --- a/sound/soc/intel/skylake/skl-pcm.c
>> >> +++ b/sound/soc/intel/skylake/skl-pcm.c
>> >> @@ -21,6 +21,7 @@
>> >>
>> >>  #include <linux/pci.h>
>> >>  #include <linux/pm_runtime.h>
>> >> +#include <linux/delay.h>
>> >>  #include <sound/pcm_params.h>
>> >>  #include <sound/soc.h>
>> >>  #include "skl.h"
>> >> @@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t
>> >skl_platform_pcm_pointer
>> >>  	 * HAD space reflects the actual data that is transferred.
>> >>  	 * Use the position buffer for capture, as DPIB write gets
>> >>  	 * completed earlier than the actual data written to the DDR.
>> >> +	 *
>> >> +	 * For capture stream following workaround is required to fix the
>> >> +	 * incorrect position reporting.
>> >> +	 *
>> >> +	 * 1. Wait for 20us before reading the DMA position in buffer once
>> >> +	 * the interrupt is generated for stream completion as update happens
>> >> +	 * on the HDA frame boundary i.e. 20.833uSec.
>> >> +	 * 2. Read DPIB register to flush the DMA position value. This dummy
>> >> +	 * read is required to flush DMA position value.
>> >> +	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
>> >> +	 * or greater than period boundary.
>> >>  	 */
>> >> -	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> >> +
>> >> +	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>> >>  		pos = readl(ebus->bus.remap_addr +
>> >AZX_REG_VS_SDXDPIB_XBASE +
>> >>  				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>> >>  				hdac_stream(hstream)->index));
>> >> -	else
>> >> +	} else {
>> >> +		udelay(20);
>> >> +		readl(ebus->bus.remap_addr +
>> >> +				AZX_REG_VS_SDXDPIB_XBASE +
>> >> +				(AZX_REG_VS_SDXDPIB_XINTERVAL *
>> >> +				 hdac_stream(hstream)->index));
>> >>  		pos =
>> >snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
>> >> +	}
>> >>
>> >>  	if (pos >= hdac_stream(hstream)->bufsize)
>> >>  		pos = 0;
>> >> --
>> >> 2.5.0
>> >>
>> >> _______________________________________________
>> >> Alsa-devel mailing list
>> >> Alsa-devel@alsa-project.org
>> >> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>> >>
>> >_______________________________________________
>> >Alsa-devel mailing list
>> >Alsa-devel@alsa-project.org
>> >http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>>
>_______________________________________________
>Alsa-devel mailing list
>Alsa-devel@alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Vinod Koul]

On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
> >> >Are these workarounds needed for the legacy driver?
> >> >If yes, which chips require it?
> >> >
> >>
> >> Yes, these are needed in legacy driver as well.
> >> From SKL and BXT onwards, it is needed.
> >
> >OK, thanks for confirmation.
> >
> >Now, from what I read in the above, is the workaround required *only*
> >after the interrupt is generated?  20us delay isn't so cheap, and we
> >tend to inquire PCM positions often.  If the workaround is needed only
> >after the PCM period elapse, we can set some flag in the irq handler
> >and apply the workaround only when necessary.
> >
> 
> Yes, Takashi the workaround is required only in the period elapsed
> interrupt. In some cases the DMA Position updates are delayed and so
> when the period elapsed interrupt occurs the wait_for_avail thinks that
> one period worth of data is not available and so returns only on the 
> next period elapsed interrupt. This creates problem for 2 periods
> playback/capture streams.
> 
> So even in the period elapsed interrupt the wait is required only if the
> position is less than the period boundary.

Hi Takashi,

So we need this for 2 periods when the device is in irq mode. Not for other
cases. ie SKL_PLUS..

But have you seen any user reports on this till now.

-- 
~Vinod

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Takashi Iwai]

On Mon, 27 Mar 2017 12:29:54 +0200,
Vinod Koul wrote:
> 
> On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
> > >> >Are these workarounds needed for the legacy driver?
> > >> >If yes, which chips require it?
> > >> >
> > >>
> > >> Yes, these are needed in legacy driver as well.
> > >> From SKL and BXT onwards, it is needed.
> > >
> > >OK, thanks for confirmation.
> > >
> > >Now, from what I read in the above, is the workaround required *only*
> > >after the interrupt is generated?  20us delay isn't so cheap, and we
> > >tend to inquire PCM positions often.  If the workaround is needed only
> > >after the PCM period elapse, we can set some flag in the irq handler
> > >and apply the workaround only when necessary.
> > >
> > 
> > Yes, Takashi the workaround is required only in the period elapsed
> > interrupt. In some cases the DMA Position updates are delayed and so
> > when the period elapsed interrupt occurs the wait_for_avail thinks that
> > one period worth of data is not available and so returns only on the 
> > next period elapsed interrupt. This creates problem for 2 periods
> > playback/capture streams.
> > 
> > So even in the period elapsed interrupt the wait is required only if the
> > position is less than the period boundary.
> 
> Hi Takashi,
> 
> So we need this for 2 periods when the device is in irq mode. Not for other
> cases. ie SKL_PLUS..

Yeah, thanks.  I'd cook a couple of patches to do that for the legacy
driver.  But I still wonder whether the wait is always needed.

Judging from the description, does the discrepancy of posbuf read
happen *only* when the DMA position goes across the BD boundary?
Or does it happen at any time?

When you trace, you can see that the apps frequently inquires the
position.  So, an unconditional wait should be really avoided.

> But have you seen any user reports on this till now.

I've seen some bug reports mentioning about crackling audio capture on
SKL (I forgot URLs).  It might be triggered by that.


thanks,

Takashi

Re: [PATCH v2 09/11] ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

[Vinod Koul]

On Mon, Mar 27, 2017 at 03:12:02PM +0200, Takashi Iwai wrote:
> On Mon, 27 Mar 2017 12:29:54 +0200,
> Vinod Koul wrote:
> > 
> > On Sat, Mar 25, 2017 at 02:21:13AM +0530, Ughreja, Rakesh A wrote:
> > > >> >Are these workarounds needed for the legacy driver?
> > > >> >If yes, which chips require it?
> > > >> >
> > > >>
> > > >> Yes, these are needed in legacy driver as well.
> > > >> From SKL and BXT onwards, it is needed.
> > > >
> > > >OK, thanks for confirmation.
> > > >
> > > >Now, from what I read in the above, is the workaround required *only*
> > > >after the interrupt is generated?  20us delay isn't so cheap, and we
> > > >tend to inquire PCM positions often.  If the workaround is needed only
> > > >after the PCM period elapse, we can set some flag in the irq handler
> > > >and apply the workaround only when necessary.
> > > >
> > > 
> > > Yes, Takashi the workaround is required only in the period elapsed
> > > interrupt. In some cases the DMA Position updates are delayed and so
> > > when the period elapsed interrupt occurs the wait_for_avail thinks that
> > > one period worth of data is not available and so returns only on the 
> > > next period elapsed interrupt. This creates problem for 2 periods
> > > playback/capture streams.
> > > 
> > > So even in the period elapsed interrupt the wait is required only if the
> > > position is less than the period boundary.
> > 
> > Hi Takashi,
> > 
> > So we need this for 2 periods when the device is in irq mode. Not for other
> > cases. ie SKL_PLUS..
> 
> Yeah, thanks.  I'd cook a couple of patches to do that for the legacy
> driver.  But I still wonder whether the wait is always needed.
> 
> Judging from the description, does the discrepancy of posbuf read
> happen *only* when the DMA position goes across the BD boundary?
> Or does it happen at any time?

I think it can happen anytime, but the impact is not felt unless we have a 2
period case. The update is in-flight, so read returns a value lesser than
period boundary. We will sleep till next period ie next wake, which results
in overrun. For more than 2 periods it doesn't impact much as the overrun
case should not happen

> When you trace, you can see that the apps frequently inquires the
> position.  So, an unconditional wait should be really avoided.

If they enquire after a while then we should be okay, but if they are
written nicely with good power behaviour then we may have issue, as writes
are typically done after period boundaries.

> 
> > But have you seen any user reports on this till now.
> 
> I've seen some bug reports mentioning about crackling audio capture on
> SKL (I forgot URLs).  It might be triggered by that.

Quiet possible..

-- 
~Vinod

Applied "ASoC: Intel: Skylake: Fix DMA position reporting for capture stream" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: Fix DMA position reporting for capture stream

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 fdd85a054b850db43c6abe39c1da28b581be5e93 Mon Sep 17 00:00:00 2001
From: Hardik T Shah <hardik.t.shah@intel.com>
Date: Fri, 24 Mar 2017 23:10:32 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Fix DMA position reporting for capture
 stream

As per hardware recommendation, for every capture stream completion
following operations need to be done in order to reflect the actual
data that is received in position buffer.

1. Wait for 20us before reading the DMA position in buffer once the
interrupt is generated for stream completion.
2. Read any of the register to flush the DMA position value. This is
dummy read operation.

Signed-off-by: Dharageswari R <dharageswari.r@intel.com>
Signed-off-by: Hardik T Shah <hardik.t.shah@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-pcm.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index ef440d8629e8..1823197c15c8 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -21,6 +21,7 @@
 
 #include <linux/pci.h>
 #include <linux/pm_runtime.h>
+#include <linux/delay.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include "skl.h"
@@ -1063,13 +1064,31 @@ static snd_pcm_uframes_t skl_platform_pcm_pointer
 	 * HAD space reflects the actual data that is transferred.
 	 * Use the position buffer for capture, as DPIB write gets
 	 * completed earlier than the actual data written to the DDR.
+	 *
+	 * For capture stream following workaround is required to fix the
+	 * incorrect position reporting.
+	 *
+	 * 1. Wait for 20us before reading the DMA position in buffer once
+	 * the interrupt is generated for stream completion as update happens
+	 * on the HDA frame boundary i.e. 20.833uSec.
+	 * 2. Read DPIB register to flush the DMA position value. This dummy
+	 * read is required to flush DMA position value.
+	 * 3. Read the DMA Position-in-Buffer. This value now will be equal to
+	 * or greater than period boundary.
 	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		pos = readl(ebus->bus.remap_addr + AZX_REG_VS_SDXDPIB_XBASE +
 				(AZX_REG_VS_SDXDPIB_XINTERVAL *
 				hdac_stream(hstream)->index));
-	else
+	} else {
+		udelay(20);
+		readl(ebus->bus.remap_addr +
+				AZX_REG_VS_SDXDPIB_XBASE +
+				(AZX_REG_VS_SDXDPIB_XINTERVAL *
+				 hdac_stream(hstream)->index));
 		pos = snd_hdac_stream_get_pos_posbuf(hdac_stream(hstream));
+	}
 
 	if (pos >= hdac_stream(hstream)->bufsize)
 		pos = 0;
-- 
2.11.0

[PATCH v2 10/11] ASoC: Intel: Skylake: Fix module state after unbind and delete

[jeeja.kp]

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

When DSP module is unbound, the module state needs to be in INIT_DONE
state instead of UNINT. Also the state needs to be set to UNINIT after
module is deleted from DSP pipeline.

So, set the module state to INIT_DONE after unbind and then UNINIT after
module is deleted.

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

diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c
index ba1ec97..09730dd 100644
--- a/sound/soc/intel/skylake/skl-messages.c
+++ b/sound/soc/intel/skylake/skl-messages.c
@@ -862,7 +862,7 @@ static void skl_clear_module_state(struct skl_module_pin *mpin, int max,
 	}
 
 	if (!found)
-		mcfg->m_state = SKL_MODULE_UNINIT;
+		mcfg->m_state = SKL_MODULE_INIT_DONE;
 	return;
 }
 
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index 8bd5ded..e960d9f 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -1037,6 +1037,11 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 
 	skl_delete_pipe(ctx, mconfig->pipe);
 
+	list_for_each_entry(w_module, &s_pipe->w_list, node) {
+		src_module = w_module->w->priv;
+		src_module->m_state = SKL_MODULE_UNINIT;
+	}
+
 	return skl_tplg_unload_pipe_modules(ctx, s_pipe);
 }
 
-- 
2.5.0

[PATCH v2 11/11] ASoC: Intel: Skylake: Add support for deferred DSP module bind

[jeeja.kp]

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

Module at the end of DSP pipeline that needs to be connected to a module
in another pipeline are represented as a PGA(leaf node) and in PGA event
handler these modules are bound/unbounded. Modules other than PGA leaf
can be connected directly or via switch to a module in another pipeline.
Example: reference path.

To support the deferred DSP module bind, following changes are done:
o When the path is enabled, the destination module that needs to be
bound may not be initialized. If the module is not initialized, add
these modules in a deferred bind list.
o When the destination module is initialized, check for these modules
in deferred bind list. If found, bind them.
o When the destination module is deleted, Unbind the modules.
o When the source module is deleted, remove the entry from the deferred
bind list.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c      |  12 ++++
 sound/soc/intel/skylake/skl-topology.c | 109 ++++++++++++++++++++++++++++++++-
 sound/soc/intel/skylake/skl-topology.h |   6 ++
 sound/soc/intel/skylake/skl.h          |   1 +
 4 files changed, 127 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 1823197..600faad 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1300,6 +1300,7 @@ int skl_platform_register(struct device *dev)
 	struct skl *skl = ebus_to_skl(ebus);
 
 	INIT_LIST_HEAD(&skl->ppl_list);
+	INIT_LIST_HEAD(&skl->bind_list);
 
 	ret = snd_soc_register_platform(dev, &skl_platform_drv);
 	if (ret) {
@@ -1320,6 +1321,17 @@ int skl_platform_register(struct device *dev)
 
 int skl_platform_unregister(struct device *dev)
 {
+	struct hdac_ext_bus *ebus = dev_get_drvdata(dev);
+	struct skl *skl = ebus_to_skl(ebus);
+	struct skl_module_deferred_bind *modules;
+
+	if (!list_empty(&skl->bind_list)) {
+		list_for_each_entry(modules, &skl->bind_list, node) {
+			list_del(&modules->node);
+			kfree(modules);
+		}
+	}
+
 	snd_soc_unregister_component(dev);
 	snd_soc_unregister_platform(dev);
 	return 0;
diff --git a/sound/soc/intel/skylake/skl-topology.c b/sound/soc/intel/skylake/skl-topology.c
index e960d9f..7f28517 100644
--- a/sound/soc/intel/skylake/skl-topology.c
+++ b/sound/soc/intel/skylake/skl-topology.c
@@ -638,8 +638,9 @@ static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
 	struct skl_module_cfg *mconfig = w->priv;
 	struct skl_pipe_module *w_module;
 	struct skl_pipe *s_pipe = mconfig->pipe;
-	struct skl_module_cfg *src_module = NULL, *dst_module;
+	struct skl_module_cfg *src_module = NULL, *dst_module, *module;
 	struct skl_sst *ctx = skl->skl_sst;
+	struct skl_module_deferred_bind *modules;
 
 	/* check resource available */
 	if (!skl_is_pipe_mcps_avail(skl, mconfig))
@@ -680,6 +681,22 @@ static int skl_tplg_mixer_dapm_pre_pmu_event(struct snd_soc_dapm_widget *w,
 		src_module = dst_module;
 	}
 
+	/*
+	 * When the destination module is initialized, check for these modules
+	 * in deferred bind list. If found, bind them.
+	 */
+	list_for_each_entry(w_module, &s_pipe->w_list, node) {
+		if (list_empty(&skl->bind_list))
+			break;
+
+		list_for_each_entry(modules, &skl->bind_list, node) {
+			module = w_module->w->priv;
+			if (modules->dst == module)
+				skl_bind_modules(ctx, modules->src,
+							modules->dst);
+		}
+	}
+
 	return 0;
 }
 
@@ -776,6 +793,44 @@ static int skl_tplg_set_module_bind_params(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+
+static int skl_tplg_module_add_deferred_bind(struct skl *skl,
+	struct skl_module_cfg *src, struct skl_module_cfg *dst)
+{
+	struct skl_module_deferred_bind *m_list, *modules;
+	int i;
+
+	/* only supported for module with static pin connection */
+	for (i = 0; i < dst->max_in_queue; i++) {
+		struct skl_module_pin *pin = &dst->m_in_pin[i];
+
+		if (pin->is_dynamic)
+			continue;
+
+		if ((pin->id.module_id  == src->id.module_id) &&
+			(pin->id.instance_id  == src->id.instance_id)) {
+
+			if (!list_empty(&skl->bind_list)) {
+				list_for_each_entry(modules, &skl->bind_list, node) {
+					if (modules->src == src && modules->dst == dst)
+						return 0;
+				}
+			}
+
+			m_list = kzalloc(sizeof(*m_list), GFP_KERNEL);
+			if (!m_list)
+				return -ENOMEM;
+
+			m_list->src = src;
+			m_list->dst = dst;
+
+			list_add(&m_list->node, &skl->bind_list);
+		}
+	}
+
+	return 0;
+}
+
 static int skl_tplg_bind_sinks(struct snd_soc_dapm_widget *w,
 				struct skl *skl,
 				struct snd_soc_dapm_widget *src_w,
@@ -810,6 +865,28 @@ static int skl_tplg_bind_sinks(struct snd_soc_dapm_widget *w,
 			sink = p->sink;
 			sink_mconfig = sink->priv;
 
+			/*
+			 * Modules other than PGA leaf can be connected
+			 * directly or via switch to a module in another
+			 * pipeline. EX: reference path
+			 * when the path is enabled, the dst module that needs
+			 * to be bound may not be initialized. if the module is
+			 * not initialized, add these modules in the deferred
+			 * bind list and when the dst module is initialised,
+			 * bind this module to the dst_module in deferred list.
+			 */
+			if (((src_mconfig->m_state == SKL_MODULE_INIT_DONE)
+				&& (sink_mconfig->m_state == SKL_MODULE_UNINIT))) {
+
+				ret = skl_tplg_module_add_deferred_bind(skl,
+						src_mconfig, sink_mconfig);
+
+				if (ret < 0)
+					return ret;
+
+			}
+
+
 			if (src_mconfig->m_state == SKL_MODULE_UNINIT ||
 				sink_mconfig->m_state == SKL_MODULE_UNINIT)
 				continue;
@@ -1014,6 +1091,7 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 	struct skl_module_cfg *src_module = NULL, *dst_module;
 	struct skl_sst *ctx = skl->skl_sst;
 	struct skl_pipe *s_pipe = mconfig->pipe;
+	struct skl_module_deferred_bind *modules;
 
 	if (s_pipe->state == SKL_PIPE_INVALID)
 		return -EINVAL;
@@ -1022,6 +1100,35 @@ static int skl_tplg_mixer_dapm_post_pmd_event(struct snd_soc_dapm_widget *w,
 	skl_tplg_free_pipe_mem(skl, mconfig);
 
 	list_for_each_entry(w_module, &s_pipe->w_list, node) {
+		if (list_empty(&skl->bind_list))
+			break;
+
+		src_module = w_module->w->priv;
+
+		list_for_each_entry(modules, &skl->bind_list, node) {
+			/*
+			 * When the destination module is deleted, Unbind the
+			 * modules from deferred bind list.
+			 */
+			if (modules->dst == src_module) {
+				skl_unbind_modules(ctx, modules->src,
+						modules->dst);
+			}
+
+			/*
+			 * When the source module is deleted, remove this entry
+			 * from the deferred bind list.
+			 */
+			if (modules->src == src_module) {
+				list_del(&modules->node);
+				modules->src = NULL;
+				modules->dst = NULL;
+				kfree(modules);
+			}
+		}
+	}
+
+	list_for_each_entry(w_module, &s_pipe->w_list, node) {
 		dst_module = w_module->w->priv;
 
 		if (mconfig->m_state >= SKL_MODULE_INIT_DONE)
diff --git a/sound/soc/intel/skylake/skl-topology.h b/sound/soc/intel/skylake/skl-topology.h
index 8536d03..cc64d6b 100644
--- a/sound/soc/intel/skylake/skl-topology.h
+++ b/sound/soc/intel/skylake/skl-topology.h
@@ -336,6 +336,12 @@ struct skl_pipeline {
 	struct list_head node;
 };
 
+struct skl_module_deferred_bind {
+	struct skl_module_cfg *src;
+	struct skl_module_cfg *dst;
+	struct list_head node;
+};
+
 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.h b/sound/soc/intel/skylake/skl.h
index bbef77d2b..5b3fa4b 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -77,6 +77,7 @@ struct skl {
 
 	struct skl_dsp_resource resource;
 	struct list_head ppl_list;
+	struct list_head bind_list;
 
 	const char *fw_name;
 	char tplg_name[64];
-- 
2.5.0

Re: [PATCH v2 00/11] ASoC: Intel: Skylake: More driver updates

[Vinod Koul]

On Fri, Mar 24, 2017 at 11:10:23PM +0530, jeeja.kp@intel.com wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> This patch series provides updates on the following:
>  o Fix LLCH register read.
>  o Add 16-bit constraint to FE bxt_rt298 machine.
>  o Fix DMA position reporting for capture stream.
>  o Use the sig_bits to define dai bps caps.
>  o Update sig_bits based on converter capability.
>  o Rearrangement of code to cleanup SKL SST library.
>  o Add support for deferred DSP module bind.
> 
> Changes in v2:
> 	- Addressed Pierre's comments for "Fix DMA position reporting
> 	  for capture stream" patch.
> 
> B, Jayachandran (1):
>   ALSA: hda: Fix LLCH register read
> 
> G Kranthi (1):
>   ASoC: Intel: Skylake: Add 16-bit constraint to FE bxt_rt298 machine
> 
> Hardik T Shah (1):
>   ASoC: Intel: Skylake: Fix DMA position reporting for capture stream
> 
> Jeeja KP (5):
>   ASoC: Intel: Skylake: Use the sig_bits to define dai bps capability
>   ASoC: hdac_hdmi: Update sig_bits based on converter capability
>   ASoC: Intel: Skylake: Rearrangement of code to cleanup SKL SST library
>   ASoC: Intel: Skylake: Fix module state after unbind and delete
>   ASoC: Intel: Skylake: Add support for deferred DSP module bind

All these:

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

-- 
~Vinod