[PATCH 0/4] ASoC/SoundWire: clean up link DMA during stop for IPC4

[PATCH 0/4] ASoC/SoundWire: clean up link DMA during stop for IPC4

[Bard Liao]

Clean up the link DMA for playback during stop for IPC4 is required to
reset the DMA read/write pointers when the stream is prepared and
restarted after a call to snd_pcm_drain()/snd_pcm_drop(). 

The change is mainly on ASoC. We may go via ASoC tree with Vinod's
Acked-by tag

Ranjani Sridharan (4):
  ASoC: SOF: ipc4-topology: Do not set ALH node_id for aggregated DAIs
  ASoC: SOF: Intel: hda: Handle prepare without close for non-HDA DAI's
  soundwire: intel_ace2x: Send PDI stream number during prepare
  ASoC: SOF: Intel: hda: Always clean up link DMA during stop

 drivers/soundwire/intel_ace2x.c   | 19 +++++-----------
 sound/soc/sof/intel/hda-dai-ops.c | 23 +++++++++----------
 sound/soc/sof/intel/hda-dai.c     | 37 +++++++++++++++++++++++++++----
 sound/soc/sof/ipc4-topology.c     | 15 +++++++++++--
 4 files changed, 62 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH 1/4] ASoC: SOF: ipc4-topology: Do not set ALH node_id for aggregated DAIs

[Bard Liao]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

For aggregated DAIs, the node ID is set to the group_id during the DAI
widget's ipc_prepare op. With the current logic, setting the dai_index
for node_id in the dai_config is redundant as it will be overwritten
with the group_id anyway. Removing it will also prevent any accidental
clearing/resetting of the group_id for aggregated DAIs due to the
dai_config calls could that happen before the allocated group_id is
freed.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/ipc4-topology.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index 56427d6e3679..1fcf4692a21f 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -3250,9 +3250,20 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
 		 * group_id during copier's ipc_prepare op.
 		 */
 		if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS) {
+			struct sof_ipc4_alh_configuration_blob *blob;
+
+			blob = (struct sof_ipc4_alh_configuration_blob *)ipc4_copier->copier_config;
 			ipc4_copier->dai_index = data->dai_node_id;
-			copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK;
-			copier_data->gtw_cfg.node_id |= SOF_IPC4_NODE_INDEX(data->dai_node_id);
+
+			/*
+			 * no need to set the node_id for aggregated DAI's. These will be assigned
+			 * a group_id during widget ipc_prepare
+			 */
+			if (blob->alh_cfg.device_count == 1) {
+				copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK;
+				copier_data->gtw_cfg.node_id |=
+					SOF_IPC4_NODE_INDEX(data->dai_node_id);
+			}
 		}
 
 		break;
-- 
2.43.0

[PATCH 2/4] ASoC: SOF: Intel: hda: Handle prepare without close for non-HDA DAI's

[Bard Liao]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

When a PCM is restarted after a snd_pcm_drain/snd_pcm_drop(), the prepare
callback will be invoked and the hw_params will be set again. For the
HDA DAI's, the hw_params function handles this case already but not for
the non-HDA DAI's. So, add the check for link_prepared to verify if the
hw_params should be done again or not. Additionally, for SDW DAI's reset
the PCMSyCM registers as would be done in the case of a start after a
hw_free.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 36 +++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 1c823f9eea57..8cccf38967e7 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -370,6 +370,13 @@ static int non_hda_dai_hw_params_data(struct snd_pcm_substream *substream,
 		return -EINVAL;
 	}
 
+	sdev = widget_to_sdev(w);
+	hext_stream = ops->get_hext_stream(sdev, cpu_dai, substream);
+
+	/* nothing more to do if the link is already prepared */
+	if (hext_stream && hext_stream->link_prepared)
+		return 0;
+
 	/* use HDaudio stream handling */
 	ret = hda_dai_hw_params_data(substream, params, cpu_dai, data, flags);
 	if (ret < 0) {
@@ -377,7 +384,6 @@ static int non_hda_dai_hw_params_data(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	sdev = widget_to_sdev(w);
 	if (sdev->dspless_mode_selected)
 		return 0;
 
@@ -482,6 +488,31 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream,
 	int ret;
 	int i;
 
+	ops = hda_dai_get_ops(substream, cpu_dai);
+	if (!ops) {
+		dev_err(cpu_dai->dev, "DAI widget ops not set\n");
+		return -EINVAL;
+	}
+
+	sdev = widget_to_sdev(w);
+	hext_stream = ops->get_hext_stream(sdev, cpu_dai, substream);
+
+	/* nothing more to do if the link is already prepared */
+	if (hext_stream && hext_stream->link_prepared)
+		return 0;
+
+	/*
+	 * reset the PCMSyCM registers to handle a prepare callback when the PCM is restarted
+	 * due to xruns or after a call to snd_pcm_drain/drop()
+	 */
+	ret = hdac_bus_eml_sdw_map_stream_ch(sof_to_bus(sdev), link_id, cpu_dai->id,
+					     0, 0, substream->stream);
+	if (ret < 0) {
+		dev_err(cpu_dai->dev, "%s:  hdac_bus_eml_sdw_map_stream_ch failed %d\n",
+			__func__, ret);
+		return ret;
+	}
+
 	data.dai_index = (link_id << 8) | cpu_dai->id;
 	data.dai_node_id = intel_alh_id;
 	ret = non_hda_dai_hw_params_data(substream, params, cpu_dai, &data, flags);
@@ -490,10 +521,7 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream,
 		return ret;
 	}
 
-	ops = hda_dai_get_ops(substream, cpu_dai);
-	sdev = widget_to_sdev(w);
 	hext_stream = ops->get_hext_stream(sdev, cpu_dai, substream);
-
 	if (!hext_stream)
 		return -ENODEV;
 
-- 
2.43.0

[PATCH 3/4] soundwire: intel_ace2x: Send PDI stream number during prepare

[Bard Liao]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

In the case of a prepare callback after an xrun or when the PCM is
restarted after a call to snd_pcm_drain/snd_pcm_drop, avoid
reprogramming the SHIM registers but send the PDI stream number so that
the link DMA data can be set. This is needed for the case that the DMA
data is cleared when the PCM is stopped and restarted without being
closed.

Link: https://github.com/thesofproject/sof/issues/9502
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_ace2x.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/soundwire/intel_ace2x.c b/drivers/soundwire/intel_ace2x.c
index 3084f0ac7159..a005b63582e9 100644
--- a/drivers/soundwire/intel_ace2x.c
+++ b/drivers/soundwire/intel_ace2x.c
@@ -383,11 +383,12 @@ static int intel_hw_params(struct snd_pcm_substream *substream,
 static int intel_prepare(struct snd_pcm_substream *substream,
 			 struct snd_soc_dai *dai)
 {
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct sdw_cdns *cdns = snd_soc_dai_get_drvdata(dai);
 	struct sdw_intel *sdw = cdns_to_intel(cdns);
 	struct sdw_cdns_dai_runtime *dai_runtime;
+	struct snd_pcm_hw_params *hw_params;
 	int ch, dir;
-	int ret = 0;
 
 	dai_runtime = cdns->dai_runtime_array[dai->id];
 	if (!dai_runtime) {
@@ -396,12 +397,8 @@ static int intel_prepare(struct snd_pcm_substream *substream,
 		return -EIO;
 	}
 
+	hw_params = &rtd->dpcm[substream->stream].hw_params;
 	if (dai_runtime->suspended) {
-		struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-		struct snd_pcm_hw_params *hw_params;
-
-		hw_params = &rtd->dpcm[substream->stream].hw_params;
-
 		dai_runtime->suspended = false;
 
 		/*
@@ -422,15 +419,11 @@ static int intel_prepare(struct snd_pcm_substream *substream,
 		/* the SHIM will be configured in the callback functions */
 
 		sdw_cdns_config_stream(cdns, ch, dir, dai_runtime->pdi);
-
-		/* Inform DSP about PDI stream number */
-		ret = intel_params_stream(sdw, substream, dai,
-					  hw_params,
-					  sdw->instance,
-					  dai_runtime->pdi->intel_alh_id);
 	}
 
-	return ret;
+	/* Inform DSP about PDI stream number */
+	return intel_params_stream(sdw, substream, dai, hw_params, sdw->instance,
+				   dai_runtime->pdi->intel_alh_id);
 }
 
 static int
-- 
2.43.0

Re: [PATCH 3/4] soundwire: intel_ace2x: Send PDI stream number during prepare

[Vinod Koul]

On 16-10-24, 11:29, Bard Liao wrote:
> From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> 
> In the case of a prepare callback after an xrun or when the PCM is
> restarted after a call to snd_pcm_drain/snd_pcm_drop, avoid
> reprogramming the SHIM registers but send the PDI stream number so that
> the link DMA data can be set. This is needed for the case that the DMA
> data is cleared when the PCM is stopped and restarted without being
> closed.

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

[PATCH 4/4] ASoC: SOF: Intel: hda: Always clean up link DMA during stop

[Bard Liao]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This is required to reset the DMA read/write pointers when the stream is
prepared and restarted after a call to snd_pcm_drain()/snd_pcm_drop().
Also, now that the stream is reset during stop, do not save LLP registers
in the case of STOP/suspend to avoid erroneous delay reporting.

Link: https://github.com/thesofproject/sof/issues/9502
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai-ops.c | 23 ++++++++++-------------
 sound/soc/sof/intel/hda-dai.c     |  1 +
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c
index 484c76147885..92681ca7f24d 100644
--- a/sound/soc/sof/intel/hda-dai-ops.c
+++ b/sound/soc/sof/intel/hda-dai-ops.c
@@ -346,20 +346,21 @@ static int hda_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai,
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		snd_hdac_ext_stream_start(hext_stream);
 		break;
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		snd_hdac_ext_stream_clear(hext_stream);
-
 		/*
-		 * Save the LLP registers in case the stream is
-		 * restarting due PAUSE_RELEASE, or START without a pcm
-		 * close/open since in this case the LLP register is not reset
-		 * to 0 and the delay calculation will return with invalid
-		 * results.
+		 * Save the LLP registers since in case of PAUSE the LLP
+		 * register are not reset to 0, the delay calculation will use
+		 * the saved offsets for compensating the delay calculation.
 		 */
 		hext_stream->pplcllpl = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPL);
 		hext_stream->pplcllpu = readl(hext_stream->pplc_addr + AZX_REG_PPLCLLPU);
+		snd_hdac_ext_stream_clear(hext_stream);
+		break;
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		hext_stream->pplcllpl = 0;
+		hext_stream->pplcllpu = 0;
+		snd_hdac_ext_stream_clear(hext_stream);
 		break;
 	default:
 		dev_err(sdev->dev, "unknown trigger command %d\n", cmd);
@@ -512,7 +513,6 @@ static const struct hda_dai_widget_dma_ops sdw_ipc4_chain_dma_ops = {
 static int hda_ipc3_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai,
 				 struct snd_pcm_substream *substream, int cmd)
 {
-	struct hdac_ext_stream *hext_stream = hda_get_hext_stream(sdev, cpu_dai, substream);
 	struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream);
 
 	switch (cmd) {
@@ -527,9 +527,6 @@ static int hda_ipc3_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c
 		if (ret < 0)
 			return ret;
 
-		if (cmd == SNDRV_PCM_TRIGGER_STOP)
-			return hda_link_dma_cleanup(substream, hext_stream, cpu_dai);
-
 		break;
 	}
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 8cccf38967e7..ac505c7ad342 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -302,6 +302,7 @@ static int __maybe_unused hda_dai_trigger(struct snd_pcm_substream *substream, i
 	}
 
 	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 		ret = hda_link_dma_cleanup(substream, hext_stream, dai);
 		if (ret < 0) {
-- 
2.43.0

Re: [PATCH 0/4] ASoC/SoundWire: clean up link DMA during stop for IPC4

[Liao, Bard]

On 10/16/2024 11:29 AM, Bard Liao wrote:
> Clean up the link DMA for playback during stop for IPC4 is required to
> reset the DMA read/write pointers when the stream is prepared and
> restarted after a call to snd_pcm_drain()/snd_pcm_drop(). 
> 
> The change is mainly on ASoC. We may go via ASoC tree with Vinod's
> Acked-by tag
> 

Mark,

This is a bug fix. Link: https://github.com/thesofproject/sof/issues/9502

Can you pick it for kernel 6.12? Also

All: Cc: stable@vger.kernel.org # 6.10.x 6.11.x

Thanks,
Bard

> Ranjani Sridharan (4):
>   ASoC: SOF: ipc4-topology: Do not set ALH node_id for aggregated DAIs
>   ASoC: SOF: Intel: hda: Handle prepare without close for non-HDA DAI's
>   soundwire: intel_ace2x: Send PDI stream number during prepare
>   ASoC: SOF: Intel: hda: Always clean up link DMA during stop
> 
>  drivers/soundwire/intel_ace2x.c   | 19 +++++-----------
>  sound/soc/sof/intel/hda-dai-ops.c | 23 +++++++++----------
>  sound/soc/sof/intel/hda-dai.c     | 37 +++++++++++++++++++++++++++----
>  sound/soc/sof/ipc4-topology.c     | 15 +++++++++++--
>  4 files changed, 62 insertions(+), 32 deletions(-)
> 

Re: [PATCH 0/4] ASoC/SoundWire: clean up link DMA during stop for IPC4

[Mark Brown]

On Wed, 16 Oct 2024 11:29:06 +0800, Bard Liao wrote:
> Clean up the link DMA for playback during stop for IPC4 is required to
> reset the DMA read/write pointers when the stream is prepared and
> restarted after a call to snd_pcm_drain()/snd_pcm_drop().
> 
> The change is mainly on ASoC. We may go via ASoC tree with Vinod's
> Acked-by tag
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: SOF: ipc4-topology: Do not set ALH node_id for aggregated DAIs
      commit: 9822b4c90d77e3c6555fb21c459c4a61c6a8619f
[2/4] ASoC: SOF: Intel: hda: Handle prepare without close for non-HDA DAI's
      commit: 6e38a7e098d32d128b00b42a536151de9ea1340b
[3/4] soundwire: intel_ace2x: Send PDI stream number during prepare
      commit: c78f1e15e46ac82607eed593b22992fd08644d96
[4/4] ASoC: SOF: Intel: hda: Always clean up link DMA during stop
      commit: ab5593793e9088abcddce30ba8e376e31b7285fd

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