From 399108212168e32831b4dec1ee68583ab6a2dd6c Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 15 Feb 2024 13:25:06 -0600 Subject: [PATCH 1/8] soundwire: intel: add intel_free_stream() back Add the intel_free_stream() callback to deal with the change in IPC that requires additional steps to be done to clear the gateway node_id. Signed-off-by: Ranjani Sridharan --- drivers/soundwire/intel.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c index e15666962fe49d..01e1a0f3ec394e 100644 --- a/drivers/soundwire/intel.c +++ b/drivers/soundwire/intel.c @@ -700,6 +700,24 @@ static int intel_params_stream(struct sdw_intel *sdw, * DAI routines */ +static int intel_free_stream(struct sdw_intel *sdw, + struct snd_pcm_substream *substream, + struct snd_soc_dai *dai, + int link_id) +{ + struct sdw_intel_link_res *res = sdw->link_res; + struct sdw_intel_stream_free_data free_data; + + free_data.substream = substream; + free_data.dai = dai; + free_data.link_id = link_id; + + if (res->ops && res->ops->free_stream && res->dev) + return res->ops->free_stream(res->dev, &free_data); + + return 0; +} + static int intel_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -831,6 +849,7 @@ static int intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { 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; int ret; @@ -851,6 +870,12 @@ intel_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) return ret; } + ret = intel_free_stream(sdw, substream, dai, sdw->instance); + if (ret < 0) { + dev_err(dai->dev, "intel_free_stream: failed %d\n", ret); + return ret; + } + dai_runtime->pdi = NULL; return 0; From eecf4ab1e91d0a78b39a624c7d29641bf1aa9a57 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Thu, 15 Feb 2024 13:19:28 -0600 Subject: [PATCH 2/8] ASoC: SOF: Intel: hda: move helper to static inline To allow using widget_to_sdev() in other files, move it as static inline in shared header file. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/intel/hda-dai.c | 8 -------- sound/soc/sof/intel/hda.h | 8 ++++++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 6705007ee51878..840cad5344f28f 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -29,14 +29,6 @@ static bool hda_use_tplg_nhlt; module_param_named(sof_use_tplg_nhlt, hda_use_tplg_nhlt, bool, 0444); MODULE_PARM_DESC(sof_use_tplg_nhlt, "SOF topology nhlt override"); -static struct snd_sof_dev *widget_to_sdev(struct snd_soc_dapm_widget *w) -{ - struct snd_sof_widget *swidget = w->dobj.private; - struct snd_soc_component *component = swidget->scomp; - - return snd_soc_component_get_drvdata(component); -} - int hda_dai_config(struct snd_soc_dapm_widget *w, unsigned int flags, struct snd_sof_dai_config_data *data) { diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 817ad2d5977213..95432cd36c1293 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -1025,4 +1025,12 @@ int hda_dai_config(struct snd_soc_dapm_widget *w, unsigned int flags, int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_stream *hext_stream, struct snd_soc_dai *cpu_dai); +static inline struct snd_sof_dev *widget_to_sdev(struct snd_soc_dapm_widget *w) +{ + struct snd_sof_widget *swidget = w->dobj.private; + struct snd_soc_component *component = swidget->scomp; + + return snd_soc_component_get_drvdata(component); +} + #endif From ae16bb08661fe41d4e3026fb6c600c60d25fed76 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 15 Feb 2024 10:23:24 -0800 Subject: [PATCH 3/8] ASoC: SOF: Intel: hda: Clear Soundwire node ID during BE DAI hw_free When an xrun happens, the BE DAI hw_params doesn't get invoked before the stream restarts with a prepare. In this case, clearing the node ID when the DAI widget is freed and unprepared will result in an error when it is re-initialized. In order to avoid this, move the code to clear the node ID to the BE DAI hw_free op to keep it balanced with the BE DAI hw_params. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda.c | 24 ++++++++++++++++++++++++ sound/soc/sof/ipc4-topology.c | 4 ---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 66126fb31d52db..facf4a7114681d 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -31,6 +31,7 @@ #include "../sof-audio.h" #include "../sof-pci-dev.h" #include "../ops.h" +#include "../ipc4-topology.h" #include "hda.h" #include @@ -68,8 +69,31 @@ static int sdw_params_stream(struct device *dev, return hda_dai_config(w, SOF_DAI_CONFIG_FLAGS_HW_PARAMS, &data); } +static int sdw_params_free(struct device *dev, struct sdw_intel_stream_free_data *free_data) +{ + struct snd_soc_dai *d = free_data->dai; + struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(d, free_data->substream->stream); + struct snd_sof_dev *sdev = widget_to_sdev(w); + + if (sdev->pdata->ipc_type == SOF_IPC_TYPE_4) { + struct snd_sof_widget *swidget = w->dobj.private; + struct snd_sof_dai *dai = swidget->private; + struct sof_ipc4_copier_data *copier_data; + struct sof_ipc4_copier *ipc4_copier; + + ipc4_copier = dai->private; + copier_data = &ipc4_copier->data; + + /* clear the node ID */ + copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK; + } + + return 0; +} + struct sdw_intel_ops sdw_callback = { .params_stream = sdw_params_stream, + .free_stream = sdw_params_free, }; static int sdw_ace2x_params_stream(struct device *dev, diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index f05b000f1d9dc8..a288dd04bdcfc0 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1277,7 +1277,6 @@ static void sof_ipc4_unprepare_copier_module(struct snd_sof_widget *swidget) } if (ipc4_copier->dai_type == SOF_DAI_INTEL_ALH) { - struct sof_ipc4_copier_data *copier_data = &ipc4_copier->data; struct sof_ipc4_alh_configuration_blob *blob; unsigned int group_id; @@ -1287,9 +1286,6 @@ static void sof_ipc4_unprepare_copier_module(struct snd_sof_widget *swidget) ALH_MULTI_GTW_BASE; ida_free(&alh_group_ida, group_id); } - - /* clear the node ID */ - copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK; } } From e33ba6c20eae05322bd0b433448f98a189a306e2 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 13 Feb 2024 14:04:16 -0600 Subject: [PATCH 4/8] ASoC: SOF: sof-audio: revisit sof_pcm_stream_free() error handling and logs For some reason the existing code stops on the first error, which potentially leaves the DMA and widgets in a weird state. Change to free-up all resources even in case of errors. Also add a more consistent error handling and logs, with the first error code returned to the caller. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/sof-audio.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index e693dcb475e4d6..b5ca2861edbd5d 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -834,17 +834,21 @@ int sof_pcm_stream_free(struct snd_sof_dev *sdev, struct snd_pcm_substream *subs { const struct sof_ipc_pcm_ops *pcm_ops = sof_ipc_get_ops(sdev, pcm); int ret; + int err = 0; if (spcm->prepared[substream->stream]) { /* stop DMA first if needed */ if (pcm_ops && pcm_ops->platform_stop_during_hw_free) snd_sof_pcm_platform_trigger(sdev, substream, SNDRV_PCM_TRIGGER_STOP); - /* Send PCM_FREE IPC to reset pipeline */ + /* free PCM in the DSP */ if (pcm_ops && pcm_ops->hw_free) { ret = pcm_ops->hw_free(sdev->component, substream); - if (ret < 0) - return ret; + if (ret < 0) { + dev_err(sdev->dev, "%s: pcm_ops hw_free failed %d\n", + __func__, ret); + err = ret; + } } spcm->prepared[substream->stream] = false; @@ -852,17 +856,25 @@ int sof_pcm_stream_free(struct snd_sof_dev *sdev, struct snd_pcm_substream *subs /* reset the DMA */ ret = snd_sof_pcm_platform_hw_free(sdev, substream); - if (ret < 0) - return ret; + if (ret < 0) { + dev_err(sdev->dev, "%s: platform hw free failed %d\n", + __func__, ret); + if (!err) + err = ret; + } /* free widget list */ if (free_widget_list) { ret = sof_widget_list_free(sdev, spcm, dir); - if (ret < 0) - dev_err(sdev->dev, "failed to free widgets during suspend\n"); + if (ret < 0) { + dev_err(sdev->dev, "%s: sof_widget_list_free failed %d\n", + __func__, ret); + if (!err) + err = ret; + } } - return ret; + return err; } /* From 2a8d7edd151441faf764064992c240ae239a955c Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 13 Feb 2024 14:10:33 -0600 Subject: [PATCH 5/8] ASoC: SOF: pcm: simplify sof_pcm_hw_free() with helper The same sequence is used twice, use common helper. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/pcm.c | 32 +++----------------------------- 1 file changed, 3 insertions(+), 29 deletions(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 2f86aef4bc9c08..397fc872ee7406 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -196,9 +196,8 @@ static int sof_pcm_hw_free(struct snd_soc_component *component, { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); - const struct sof_ipc_pcm_ops *pcm_ops = sof_ipc_get_ops(sdev, pcm); struct snd_sof_pcm *spcm; - int ret, err = 0; + int ret; /* nothing to do for BE */ if (rtd->dai_link->no_pcm) @@ -211,36 +210,11 @@ static int sof_pcm_hw_free(struct snd_soc_component *component, dev_dbg(component->dev, "pcm: free stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream); - if (spcm->prepared[substream->stream]) { - /* stop DMA first if needed */ - if (pcm_ops && pcm_ops->platform_stop_during_hw_free) - snd_sof_pcm_platform_trigger(sdev, substream, SNDRV_PCM_TRIGGER_STOP); - - /* free PCM in the DSP */ - if (pcm_ops && pcm_ops->hw_free) { - ret = pcm_ops->hw_free(component, substream); - if (ret < 0) - err = ret; - } - - spcm->prepared[substream->stream] = false; - } - - /* reset DMA */ - ret = snd_sof_pcm_platform_hw_free(sdev, substream); - if (ret < 0) { - dev_err(component->dev, "error: platform hw free failed\n"); - err = ret; - } - - /* free the DAPM widget list */ - ret = sof_widget_list_free(sdev, spcm, substream->stream); - if (ret < 0) - err = ret; + ret = sof_pcm_stream_free(sdev, substream, spcm, substream->stream, true); cancel_work_sync(&spcm->stream[substream->stream].period_elapsed_work); - return err; + return ret; } static int sof_pcm_prepare(struct snd_soc_component *component, From 32afce30af648eae8cb38498ffcdf7f1d17543a4 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 13 Feb 2024 14:29:34 -0600 Subject: [PATCH 6/8] ASoC: SOF: pcm: add pending_stop state variable Add a state variable to keep track of delayed stops, in case pcm_ops->platform_stop_during_hw_free is set. This patch should be iso-functionality, possibly removing no-op cases. The main purpose of this new state variable is to prepare a follow-up patch to reset all PCM and DMAs in case of stop/prepare xrun sequences. Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/pcm.c | 11 +++++++++++ sound/soc/sof/sof-audio.c | 1 + sound/soc/sof/sof-audio.h | 1 + 3 files changed, 13 insertions(+) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 397fc872ee7406..7d2ea0f85a0690 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -276,6 +276,8 @@ static int sof_pcm_trigger(struct snd_soc_component *component, dev_dbg(component->dev, "pcm: trigger stream %d dir %d cmd %d\n", spcm->pcm.pcm_id, substream->stream, cmd); + spcm->pending_stop[substream->stream] = false; + switch (cmd) { case SNDRV_PCM_TRIGGER_PAUSE_PUSH: ipc_first = true; @@ -344,6 +346,15 @@ static int sof_pcm_trigger(struct snd_soc_component *component, /* invoke platform trigger to stop DMA even if pcm_ops isn't set or if it failed */ if (!pcm_ops || !pcm_ops->platform_stop_during_hw_free) snd_sof_pcm_platform_trigger(sdev, substream, cmd); + + /* + * set the pending_stop flag to indicate that pipeline stop has been delayed. + * This will be used later to stop the pipelines during prepare when recovering + * from xruns. + */ + if (pcm_ops && pcm_ops->platform_stop_during_hw_free && + cmd == SNDRV_PCM_TRIGGER_STOP) + spcm->pending_stop[substream->stream] = true; break; default: break; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index b5ca2861edbd5d..32fef64ef10ddb 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -852,6 +852,7 @@ int sof_pcm_stream_free(struct snd_sof_dev *sdev, struct snd_pcm_substream *subs } spcm->prepared[substream->stream] = false; + spcm->pending_stop[substream->stream] = false; } /* reset the DMA */ diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 64158ee6c3a839..72af874499f9ab 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -344,6 +344,7 @@ struct snd_sof_pcm { struct list_head list; /* list in sdev pcm list */ struct snd_pcm_hw_params params[2]; bool prepared[2]; /* PCM_PARAMS set successfully */ + bool pending_stop[2]; /* only used if (!pcm_ops->platform_stop_during_hw_free) */ }; struct snd_sof_led_control { From c328a1e8fc1843bec32bedd901a0a6f88d0d8ee5 Mon Sep 17 00:00:00 2001 From: Pierre-Louis Bossart Date: Tue, 13 Feb 2024 15:01:22 -0600 Subject: [PATCH 7/8] ASoC: SOF: pcm: reset all PCM sources in case of xruns With the delayed stops, the xrun handling is problematic: the applications expects everything to be reset but the firmware and DMA are still in a PAUSED state. This patch makes sure the prepare while pending_stop is set is special-cased. Co-developed-by: Ranjani Sridharan Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart --- sound/soc/sof/pcm.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 7d2ea0f85a0690..f74e4b2c2afca0 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -221,6 +221,7 @@ static int sof_pcm_prepare(struct snd_soc_component *component, struct snd_pcm_substream *substream) { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); struct snd_sof_pcm *spcm; int ret; @@ -232,8 +233,18 @@ static int sof_pcm_prepare(struct snd_soc_component *component, if (!spcm) return -EINVAL; - if (spcm->prepared[substream->stream]) - return 0; + if (spcm->prepared[substream->stream]) { + if (!spcm->pending_stop[substream->stream]) + return 0; + + /* + * this case should be reached in case of xruns where we absolutely + * want to free-up and reset all PCM/DMA resources + */ + ret = sof_pcm_stream_free(sdev, substream, spcm, substream->stream, true); + if (ret < 0) + return ret; + } dev_dbg(component->dev, "pcm: prepare stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream); From ba9f1ab8ef632af75ee8f0d2a7b9f9561282d291 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Thu, 15 Feb 2024 16:37:07 -0800 Subject: [PATCH 8/8] ASoC: SOF: ipc4-topology: Save the ALH DAI index during hw_params The node_id for multi-gateway ALH DAI's get overwritten with the group_id during the DAI copier's ipc_prepare op. So, save the ALH dai_index during the BE DAI hw_params in the dai_index field of struct ipc4_copier and use that to set the device ID in the configuration blob. This will avoid errors during copier init after an xrun. Note that the dai_index is typically set in topology for DMIC/SSP, but it's not used for ALH. Reclaiming this dai_index field to store the node_id does not generate a conflict with topology-defined values. Signed-off-by: Ranjani Sridharan --- sound/soc/sof/intel/hda.c | 1 + sound/soc/sof/ipc4-topology.c | 12 ++++++++++-- sound/soc/sof/ipc4-topology.h | 1 + 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index facf4a7114681d..25dba9eb383cfa 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -82,6 +82,7 @@ static int sdw_params_free(struct device *dev, struct sdw_intel_stream_free_data struct sof_ipc4_copier *ipc4_copier; ipc4_copier = dai->private; + ipc4_copier->dai_index = 0; copier_data = &ipc4_copier->data; /* clear the node ID */ diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index a288dd04bdcfc0..bf0f49e1d7c884 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1713,6 +1713,8 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, */ i = 0; list_for_each_entry(w, &sdev->widget_list, list) { + u32 node_type; + if (w->widget->sname && strcmp(w->widget->sname, swidget->widget->sname)) continue; @@ -1720,7 +1722,10 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, dai = w->private; alh_copier = (struct sof_ipc4_copier *)dai->private; alh_data = &alh_copier->data; - blob->alh_cfg.mapping[i].device = alh_data->gtw_cfg.node_id; + node_type = SOF_IPC4_GET_NODE_TYPE(alh_data->gtw_cfg.node_id); + blob->alh_cfg.mapping[i].device = SOF_IPC4_NODE_TYPE(node_type); + blob->alh_cfg.mapping[i].device |= + SOF_IPC4_NODE_INDEX(alh_copier->dai_index); /* * The mapping[i] device in ALH blob should be the same as the @@ -2839,12 +2844,15 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget * /* * Do not clear the node ID when this op is invoked with * SOF_DAI_CONFIG_FLAGS_HW_FREE. It is needed to free the group_ida during - * unprepare. + * unprepare. The node_id for multi-gateway DAI's will be overwritten with the + * group_id during copier's ipc_prepare op. */ if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS) { + 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); } + break; case SOF_DAI_INTEL_DMIC: case SOF_DAI_INTEL_SSP: diff --git a/sound/soc/sof/ipc4-topology.h b/sound/soc/sof/ipc4-topology.h index aa5122c3721d3a..6e33208a357f5d 100644 --- a/sound/soc/sof/ipc4-topology.h +++ b/sound/soc/sof/ipc4-topology.h @@ -45,6 +45,7 @@ #define SOF_IPC4_NODE_INDEX_MASK 0xFF #define SOF_IPC4_NODE_INDEX(x) ((x) & SOF_IPC4_NODE_INDEX_MASK) #define SOF_IPC4_NODE_TYPE(x) ((x) << 8) +#define SOF_IPC4_GET_NODE_TYPE(node_id) ((node_id) >> 8) /* Node ID for SSP type DAI copiers */ #define SOF_IPC4_NODE_INDEX_INTEL_SSP(x) (((x) & 0xf) << 4)