ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop

Commit Message

Peter Ujfalusi Aug. 8, 2023, 11:06 a.m. UTC

From: Ranjani Sridharan <[e-mail]>

With IPC3, we reset hw_params during the stop trigger, so we should also
clean up the link DMA during the stop trigger.

Fixes: 1bf83fa6654c ("ASoC: SOF: Intel: hda-dai: Do not perform DMA cleanup during stop")
Closes: https://github.com/thesofproject/linux/issues/4455
Closes: https://github.com/thesofproject/linux/issues/4482
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217673
Signed-off-by: Ranjani Sridharan <[e-mail]>
Reviewed-by: Pierre-Louis Bossart <[e-mail]>
Reviewed-by: Rander Wang <[e-mail]>
Reviewed-by: Péter Ujfalusi <[e-mail]>
Reviewed-by: Guennadi Liakhovetski <[e-mail]>
Reviewed-by: Bard Liao <[e-mail]>
Signed-off-by: Peter Ujfalusi <[e-mail]>
---
 sound/soc/sof/intel/hda-dai-ops.c | 11 ++++++++++-
 sound/soc/sof/intel/hda-dai.c     |  5 ++---
 sound/soc/sof/intel/hda.h         |  2 ++
 3 files changed, 14 insertions(+), 4 deletions(-)

Comments

Mark Brown Aug. 8, 2023, 5:57 p.m. UTC | #1

On Tue, 08 Aug 2023 14:06:27 +0300, Peter Ujfalusi wrote:
> With IPC3, we reset hw_params during the stop trigger, so we should also
> clean up the link DMA during the stop trigger.
> 
> 

Applied to

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

Thanks!

[1/1] ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop
      commit: 90219f1bd273055f1dc1d7bdc0965755b992c045

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

Jaroslav Kysela Aug. 18, 2023, 3:08 p.m. UTC | #2

On 08. 08. 23 19:57, Mark Brown wrote:
> On Tue, 08 Aug 2023 14:06:27 +0300, Peter Ujfalusi wrote:
>> With IPC3, we reset hw_params during the stop trigger, so we should also
>> clean up the link DMA during the stop trigger.
>>
>>
> 
> Applied to
> 
>     https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
> 
> Thanks!
> 
> [1/1] ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop
>        commit: 90219f1bd273055f1dc1d7bdc0965755b992c045

Hi Mark,

	if it's not late, this patch should be also applied to the 6.5 tree. It seems 
that many new laptops are affected (sound may be really weird when an underrun 
occurs on those devices and users are not really happy).

						Jaroslav

Mark Brown Aug. 18, 2023, 3:18 p.m. UTC | #3

On Fri, Aug 18, 2023 at 05:08:35PM +0200, Jaroslav Kysela wrote:
> On 08. 08. 23 19:57, Mark Brown wrote:

> > [1/1] ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop
> >        commit: 90219f1bd273055f1dc1d7bdc0965755b992c045

> 	if it's not late, this patch should be also applied to the 6.5 tree. It
> seems that many new laptops are affected (sound may be really weird when an
> underrun occurs on those devices and users are not really happy).

It was applied for 6.5, the reports always say for-next due to
difficulties with serieses which mix fixes and new development.

Takashi Iwai Aug. 18, 2023, 3:22 p.m. UTC | #4

On Fri, 18 Aug 2023 17:18:54 +0200,
Mark Brown wrote:
> 
> On Fri, Aug 18, 2023 at 05:08:35PM +0200, Jaroslav Kysela wrote:
> > On 08. 08. 23 19:57, Mark Brown wrote:
> 
> > > [1/1] ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop
> > >        commit: 90219f1bd273055f1dc1d7bdc0965755b992c045
> 
> > 	if it's not late, this patch should be also applied to the 6.5 tree. It
> > seems that many new laptops are affected (sound may be really weird when an
> > underrun occurs on those devices and users are not really happy).
> 
> It was applied for 6.5, the reports always say for-next due to
> difficulties with serieses which mix fixes and new development.

Right, the commit is on the way to Linus now.


Takashi

Jaroslav Kysela Aug. 18, 2023, 3:23 p.m. UTC | #5

On 18. 08. 23 17:18, Mark Brown wrote:
> On Fri, Aug 18, 2023 at 05:08:35PM +0200, Jaroslav Kysela wrote:
>> On 08. 08. 23 19:57, Mark Brown wrote:
> 
>>> [1/1] ASoC: SOF: intel: hda: Clean up link DMA for IPC3 during stop
>>>         commit: 90219f1bd273055f1dc1d7bdc0965755b992c045
> 
>> 	if it's not late, this patch should be also applied to the 6.5 tree. It
>> seems that many new laptops are affected (sound may be really weird when an
>> underrun occurs on those devices and users are not really happy).
> 
> It was applied for 6.5, the reports always say for-next due to
> difficulties with serieses which mix fixes and new development.

Great. Thanks.

					Jaroslav

Patch

diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c
index f3513796c189..f33051eac1c0 100644
--- a/sound/soc/sof/intel/hda-dai-ops.c
+++ b/sound/soc/sof/intel/hda-dai-ops.c
@@ -372,6 +372,7 @@  static const struct hda_dai_widget_dma_ops hda_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) {
@@ -379,9 +380,17 @@  static int hda_ipc3_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c
 	case SNDRV_PCM_TRIGGER_STOP:
 	{
 		struct snd_sof_dai_config_data data = { 0 };
+		int ret;
 
 		data.dai_data = DMA_CHAN_INVALID;
-		return hda_dai_config(w, SOF_DAI_CONFIG_FLAGS_HW_FREE, &data);
+		ret = hda_dai_config(w, SOF_DAI_CONFIG_FLAGS_HW_FREE, &data);
+		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:
 		return hda_dai_config(w, SOF_DAI_CONFIG_FLAGS_PAUSE, NULL);
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 3297dea493aa..863865f3d77e 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -107,9 +107,8 @@  hda_dai_get_ops(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai
 	return sdai->platform_private;
 }
 
-static int hda_link_dma_cleanup(struct snd_pcm_substream *substream,
-				struct hdac_ext_stream *hext_stream,
-				struct snd_soc_dai *cpu_dai)
+int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_stream *hext_stream,
+			 struct snd_soc_dai *cpu_dai)
 {
 	const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai);
 	struct sof_intel_hda_stream *hda_stream;
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 3f7c6fb05e5d..5b9e4ebcc18b 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -963,5 +963,7 @@  const struct hda_dai_widget_dma_ops *
 hda_select_dai_widget_ops(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget);
 int hda_dai_config(struct snd_soc_dapm_widget *w, unsigned int flags,
 		   struct snd_sof_dai_config_data *data);
+int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_stream *hext_stream,
+			 struct snd_soc_dai *cpu_dai);
 
 #endif