From a940ab9e218c07e450099cf8b18cf07af933a6b3 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 9 Dec 2024 11:11:22 +0200 Subject: [PATCH] ASoC: SOF: Intel: hda-dai: Reclaim the link DMA on STOP with IPC4 We need to reclaim the link DMA channel after clearing it with IPC4 as the pipelines are not cleared in firmware, the Link DMA channel is preserved. This issue is not easy to reproduce under normal conditions as usually after stop the stream is closed, or the same stream is restarted, but if another stream got in between the stop and start, like this: aplay -Dhw:0,3 -c2 -r48000 -fS32_LE /dev/zero -d 120 CTRL+z aplay -Dhw:0,0 -c2 -r48000 -fS32_LE /dev/zero -d 120 then the link DMA channels will be mixed up resulting firmware error or crash. Fixes: ab5593793e90 ("ASoC: SOF: Intel: hda: Always clean up link DMA during stop") Closes: https://github.com/thesofproject/sof/issues/9695 Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-dai.c | 63 ++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 2a6f821c9e14e9..5ffd78195b2509 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -140,36 +140,37 @@ int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_st return 0; } -static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, struct snd_soc_dai *cpu_dai) +static struct hdac_ext_stream * +hda_link_dma_setup(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, + struct snd_soc_dai *cpu_dai) { const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai); struct hdac_ext_stream *hext_stream; struct hdac_stream *hstream; struct hdac_ext_link *hlink; - struct snd_sof_dev *sdev; int stream_tag; - if (!ops) { - dev_err(cpu_dai->dev, "DAI widget ops not set\n"); - return -EINVAL; - } - - sdev = dai_to_sdev(substream, cpu_dai); + /* + * The callers already checked this, but static analyzers can get + * confused + */ + if (unlikely(!ops)) + return ERR_PTR(-EINVAL); hlink = ops->get_hlink(sdev, substream); if (!hlink) - return -EINVAL; + return ERR_PTR(-EINVAL); hext_stream = ops->get_hext_stream(sdev, cpu_dai, substream); if (!hext_stream) { if (ops->assign_hext_stream) - hext_stream = ops->assign_hext_stream(sdev, cpu_dai, substream); + hext_stream = ops->assign_hext_stream(sdev, cpu_dai, + substream); } if (!hext_stream) - return -EBUSY; + return ERR_PTR(-EBUSY); hstream = &hext_stream->hstream; stream_tag = hstream->stream_tag; @@ -184,6 +185,27 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, if (ops->reset_hext_stream) ops->reset_hext_stream(sdev, hext_stream); + return hext_stream; +} + +static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, struct snd_soc_dai *cpu_dai) +{ + const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, cpu_dai); + struct hdac_ext_stream *hext_stream; + struct snd_sof_dev *sdev; + + if (!ops) { + dev_err(cpu_dai->dev, "DAI widget ops not set\n"); + return -EINVAL; + } + + sdev = dai_to_sdev(substream, cpu_dai); + + hext_stream = hda_link_dma_setup(sdev, substream, cpu_dai); + if (IS_ERR(hext_stream)) + return PTR_ERR(hext_stream); + if (ops->calc_stream_format && ops->setup_hext_stream) { unsigned int format_val = ops->calc_stream_format(sdev, substream, params); @@ -309,6 +331,23 @@ static int __maybe_unused hda_dai_trigger(struct snd_pcm_substream *substream, i dev_err(sdev->dev, "%s: failed to clean up link DMA\n", __func__); return ret; } + + /* + * The firmware for IPC4 expects the linkDMA to be reset but not + * released on stop, the same channel must remain to be used if + * we have a start after the stop without a close. + * The firmware keeps it's side of the LinkDMA channel on stop, + * we need to do the same on the host side. + * + * If the stream is closed the linkDMA is going to be cleaned up + * in via hda_dai_hw_free() + */ + if (cmd == SNDRV_PCM_TRIGGER_STOP && + sdev->pdata->ipc_type == SOF_IPC_TYPE_4) { + hext_stream = hda_link_dma_setup(sdev, substream, dai); + if (IS_ERR(hext_stream)) + return PTR_ERR(hext_stream); + } break; default: break;