[PATCH 00/17] ASoC: SOF: Intel: improve SoundWire support for LunarLake
This patchset contains important updates for SoundWire support.
We initially implemented support for multiple amplifiers on different links using a single HDaudio DMA transfer. To align with the other OS, the 'aggregation' is now supported by the firmware. This change in directions has kernel impacts, since we now have multiple HDaudio DMAs to program and start, but since there are no platforms released so far there's no end-user impact.
In addition, the behavior in case of xruns is improved by clearing the PCM states and better handling of the hw_free case.
Note that the hw_free support will compile but will only be functional with the companion patch "soundwire: intel: add intel_free_stream() back" already applied in the SoundWire tree.
Bard Liao (5): Revert "ASoC: SOF: Intel: hda-dai-ops: reset device count for SoundWire DAIs" Revert "ASoC: SOF: Intel: hda-dai-ops: only allocate/release streams for first CPU DAI" ASoC: SOF: make dma_config_tlv be an array ASoC: SOF: Intel: hda-dai: set lowest N bits in ch_mask ASoC: SOF: Intel: hda-dai: set dma_stream_channel_map device
Pierre-Louis Bossart (10): ASoC: SOF: Intel: hda-dai: fix channel map configuration for aggregated dailink ASoC: SOF: Intel: hda-dai: add helpers to set dai config ASoC: SOF: Intel: set the DMA TLV device as dai_index ASoC: SOF: Intel: hda: extend signature of sdw_hda_dai_hw_params() ASoC: SOF: IPC4: extend dai_data with node_id ASoC: SOF: Intel: hda: move helper to static inline ASoC: SOF: sof-audio: revisit sof_pcm_stream_free() error handling and logs ASoC: SOF: pcm: simplify sof_pcm_hw_free() with helper ASoC: SOF: pcm: add pending_stop state variable ASoC: SOF: pcm: reset all PCM sources in case of xruns
Ranjani Sridharan (2): ASoC: SOF: Intel: hda: Clear Soundwire node ID during BE DAI hw_free ASoC: SOF: ipc4-topology: Save the ALH DAI index during hw_params
sound/soc/sof/intel/hda-dai-ops.c | 41 +--------- sound/soc/sof/intel/hda-dai.c | 124 +++++++++++++++++++++++------- sound/soc/sof/intel/hda.c | 29 ++++++- sound/soc/sof/intel/hda.h | 11 ++- sound/soc/sof/ipc4-topology.c | 62 ++++++++++----- sound/soc/sof/ipc4-topology.h | 3 +- sound/soc/sof/pcm.c | 58 +++++++------- sound/soc/sof/sof-audio.c | 29 +++++-- sound/soc/sof/sof-audio.h | 2 + 9 files changed, 232 insertions(+), 127 deletions(-)
The existing code derives the channel map used to program the HDaudio link DMA from the hw_params, but that is not quite right in the case of aggregation. The code in soc-pcm.c splits the hw_params depending on the codec_ch_map, and we need to reconstruct the channel-map to insert the data in the right places.
This issue is seen only on amplifier feedback capture where the data from the second amplifier was replaced by that of the first amplifier.
Note that the loop iterator of the macro for_each_rtd_cpu_dais() is reused in a following loop. This is different to all existing usages of that macro, hence the use of a boolean flag to avoid an access to an uninitialized variable.
Fixes: 2960ee5c4814 ("ASoC: SOF: Intel: hda-dai: add helpers for SoundWire callbacks") Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c1682bcdb5a6..6a39ca632f55 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -439,10 +439,17 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, int link_id) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); const struct hda_dai_widget_dma_ops *ops; + struct snd_soc_dai_link_ch_map *ch_maps; struct hdac_ext_stream *hext_stream; + struct snd_soc_dai *dai; struct snd_sof_dev *sdev; + bool cpu_dai_found = false; + int cpu_dai_id; + int ch_mask; int ret; + int j;
ret = non_hda_dai_hw_params(substream, params, cpu_dai); if (ret < 0) { @@ -457,9 +464,29 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, if (!hext_stream) return -ENODEV;
- /* in the case of SoundWire we need to program the PCMSyCM registers */ + /* + * in the case of SoundWire we need to program the PCMSyCM registers. In case + * of aggregated devices, we need to define the channel mask for each sublink + * by reconstructing the split done in soc-pcm.c + */ + for_each_rtd_cpu_dais(rtd, cpu_dai_id, dai) { + if (dai == cpu_dai) { + cpu_dai_found = true; + break; + } + } + + if (!cpu_dai_found) + return -ENODEV; + + ch_mask = 0; + for_each_link_ch_maps(rtd->dai_link, j, ch_maps) { + if (ch_maps->cpu == cpu_dai_id) + ch_mask |= ch_maps->ch_mask; + } + ret = hdac_bus_eml_sdw_map_stream_ch(sof_to_bus(sdev), link_id, cpu_dai->id, - GENMASK(params_channels(params) - 1, 0), + ch_mask, hdac_stream(hext_stream)->stream_tag, substream->stream); if (ret < 0) {
From: Bard Liao yung-chuan.liao@linux.intel.com
This reverts commit 699e146d9ebf42ee2a5d4e4e28f7a49c4aef0105. Don't reset device_count as we will use the multi-gateway firmware configuration.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai-ops.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c index b073720b4cf4..e17b83575e84 100644 --- a/sound/soc/sof/intel/hda-dai-ops.c +++ b/sound/soc/sof/intel/hda-dai-ops.c @@ -446,28 +446,6 @@ static int hda_ipc4_post_trigger(struct snd_sof_dev *sdev, struct snd_soc_dai *c return ret; }
-static struct hdac_ext_stream *sdw_hda_ipc4_get_hext_stream(struct snd_sof_dev *sdev, - struct snd_soc_dai *cpu_dai, - struct snd_pcm_substream *substream) -{ - struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); - struct snd_sof_widget *swidget = w->dobj.private; - struct snd_sof_dai *dai = swidget->private; - struct sof_ipc4_copier *ipc4_copier = dai->private; - struct sof_ipc4_alh_configuration_blob *blob; - - blob = (struct sof_ipc4_alh_configuration_blob *)ipc4_copier->copier_config; - - /* - * Starting with ACE_2_0, re-setting the device_count is mandatory to avoid using - * the multi-gateway firmware configuration. The DMA hardware can take care of - * multiple links without needing any firmware assistance - */ - blob->alh_cfg.device_count = 1; - - return hda_ipc4_get_hext_stream(sdev, cpu_dai, substream); -} - static const struct hda_dai_widget_dma_ops hda_ipc4_dma_ops = { .get_hext_stream = hda_ipc4_get_hext_stream, .assign_hext_stream = hda_assign_hext_stream, @@ -509,7 +487,7 @@ static const struct hda_dai_widget_dma_ops dmic_ipc4_dma_ops = { };
static const struct hda_dai_widget_dma_ops sdw_ipc4_dma_ops = { - .get_hext_stream = sdw_hda_ipc4_get_hext_stream, + .get_hext_stream = hda_ipc4_get_hext_stream, .assign_hext_stream = hda_assign_hext_stream, .release_hext_stream = hda_release_hext_stream, .setup_hext_stream = hda_setup_hext_stream,
From: Bard Liao yung-chuan.liao@linux.intel.com
This reverts commit f8ba62ac863c33fc0d8ac3f1270985c2b77f4377.
The SoundWire aggregated solution was to use one DMA on multiple links. But, the solution changed to use one DMA for each link. It means that we should assign HDaudio stream_tag for each cpu_dai.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai-ops.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai-ops.c b/sound/soc/sof/intel/hda-dai-ops.c index e17b83575e84..457144203999 100644 --- a/sound/soc/sof/intel/hda-dai-ops.c +++ b/sound/soc/sof/intel/hda-dai-ops.c @@ -146,17 +146,9 @@ static struct hdac_ext_stream *hda_assign_hext_stream(struct snd_sof_dev *sdev, struct snd_soc_dai *cpu_dai, struct snd_pcm_substream *substream) { - struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); - struct snd_soc_dai *dai; struct hdac_ext_stream *hext_stream;
- /* only allocate a stream_tag for the first DAI in the dailink */ - dai = snd_soc_rtd_to_cpu(rtd, 0); - if (dai == cpu_dai) - hext_stream = hda_link_stream_assign(sof_to_bus(sdev), substream); - else - hext_stream = snd_soc_dai_get_dma_data(dai, substream); - + hext_stream = hda_link_stream_assign(sof_to_bus(sdev), substream); if (!hext_stream) return NULL;
@@ -169,14 +161,9 @@ static void hda_release_hext_stream(struct snd_sof_dev *sdev, struct snd_soc_dai struct snd_pcm_substream *substream) { struct hdac_ext_stream *hext_stream = hda_get_hext_stream(sdev, cpu_dai, substream); - struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); - struct snd_soc_dai *dai;
- /* only release a stream_tag for the first DAI in the dailink */ - dai = snd_soc_rtd_to_cpu(rtd, 0); - if (dai == cpu_dai) - snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK); snd_soc_dai_set_dma_data(cpu_dai, substream, NULL); + snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK); }
static void hda_setup_hext_stream(struct snd_sof_dev *sdev, struct hdac_ext_stream *hext_stream,
From: Bard Liao yung-chuan.liao@linux.intel.com
Each stream needs a dma_config_tlv. We will handle multi dma_config_tlv in the follow up commits.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 2 +- sound/soc/sof/ipc4-topology.c | 25 ++++++++++++------------- sound/soc/sof/ipc4-topology.h | 2 +- 3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 6a39ca632f55..01c544b7e046 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -392,7 +392,7 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, /* configure TLV */ ipc4_copier = widget_to_copier(w);
- dma_config_tlv = &ipc4_copier->dma_config_tlv; + dma_config_tlv = &ipc4_copier->dma_config_tlv[0]; dma_config_tlv->type = SOF_IPC4_GTW_DMA_CONFIG_ID; /* dma_config_priv_size is zero */ dma_config_tlv->length = sizeof(dma_config_tlv->dma_config); diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index 427f186ddc11..1870488b3cf6 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1486,6 +1486,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, u32 deep_buffer_dma_ms = 0; int output_fmt_index; bool single_output_format; + int i;
dev_dbg(sdev->dev, "copier %s, type %d", swidget->widget->name, swidget->id);
@@ -1711,7 +1712,6 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, u32 ch_map; u32 step; u32 mask; - int i;
blob = (struct sof_ipc4_alh_configuration_blob *)ipc4_copier->copier_config;
@@ -1821,19 +1821,18 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, gtw_cfg_config_length = copier_data->gtw_cfg.config_length * 4; ipc_size = sizeof(*copier_data) + gtw_cfg_config_length;
- if (ipc4_copier->dma_config_tlv.type == SOF_IPC4_GTW_DMA_CONFIG_ID && - ipc4_copier->dma_config_tlv.length) { - dma_config_tlv_size = sizeof(ipc4_copier->dma_config_tlv) + - ipc4_copier->dma_config_tlv.dma_config.dma_priv_config_size; - - /* paranoia check on TLV size/length */ - if (dma_config_tlv_size != ipc4_copier->dma_config_tlv.length + - sizeof(uint32_t) * 2) { - dev_err(sdev->dev, "Invalid configuration, TLV size %d length %d\n", - dma_config_tlv_size, ipc4_copier->dma_config_tlv.length); - return -EINVAL; - } + dma_config_tlv_size = 0; + for (i = 0; i < SOF_IPC4_DMA_DEVICE_MAX_COUNT; i++) { + if (ipc4_copier->dma_config_tlv[i].type != SOF_IPC4_GTW_DMA_CONFIG_ID) + continue; + dma_config_tlv_size += ipc4_copier->dma_config_tlv[i].length; + dma_config_tlv_size += + ipc4_copier->dma_config_tlv[i].dma_config.dma_priv_config_size; + dma_config_tlv_size += (sizeof(ipc4_copier->dma_config_tlv[i]) - + sizeof(ipc4_copier->dma_config_tlv[i].dma_config)); + }
+ if (dma_config_tlv_size) { ipc_size += dma_config_tlv_size;
/* we also need to increase the size at the gtw level */ diff --git a/sound/soc/sof/ipc4-topology.h b/sound/soc/sof/ipc4-topology.h index dce174a190dd..aa5122c3721d 100644 --- a/sound/soc/sof/ipc4-topology.h +++ b/sound/soc/sof/ipc4-topology.h @@ -313,7 +313,7 @@ struct sof_ipc4_copier { struct sof_ipc4_gtw_attributes *gtw_attr; u32 dai_type; int dai_index; - struct sof_ipc4_dma_config_tlv dma_config_tlv; + struct sof_ipc4_dma_config_tlv dma_config_tlv[SOF_IPC4_DMA_DEVICE_MAX_COUNT]; };
/**
From: Bard Liao yung-chuan.liao@linux.intel.com
We always use the lowest N channels of stream. So, set ch_mask to GENMASK(params_channels(params) - 1, 0).
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 01c544b7e046..810d2997794f 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -441,7 +441,6 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); const struct hda_dai_widget_dma_ops *ops; - struct snd_soc_dai_link_ch_map *ch_maps; struct hdac_ext_stream *hext_stream; struct snd_soc_dai *dai; struct snd_sof_dev *sdev; @@ -449,7 +448,6 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, int cpu_dai_id; int ch_mask; int ret; - int j;
ret = non_hda_dai_hw_params(substream, params, cpu_dai); if (ret < 0) { @@ -479,11 +477,7 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, if (!cpu_dai_found) return -ENODEV;
- ch_mask = 0; - for_each_link_ch_maps(rtd->dai_link, j, ch_maps) { - if (ch_maps->cpu == cpu_dai_id) - ch_mask |= ch_maps->ch_mask; - } + ch_mask = GENMASK(params_channels(params) - 1, 0);
ret = hdac_bus_eml_sdw_map_stream_ch(sof_to_bus(sdev), link_id, cpu_dai->id, ch_mask,
From: Bard Liao yung-chuan.liao@linux.intel.com
sof_ipc4_dma_config_tlv{} is required for ACE2.x. The patch follow the convention to set the dma_stream_channel_map.mapping device as "link_id << 8 | pdi_id". And the mapping in sof_ipc4_alh_configuration_blob{} should be the same as dma_stream_channel_map.mapping in sof_ipc4_dma_config{}. The purposes of device id is to map DMA tlv.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 36 +++++++++++++++++++++++++++++++++-- sound/soc/sof/ipc4-topology.c | 13 +++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 810d2997794f..de71e1595a78 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -346,6 +346,7 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); + struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_dma_config_tlv *dma_config_tlv; const struct hda_dai_widget_dma_ops *ops; struct sof_ipc4_dma_config *dma_config; @@ -353,6 +354,8 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, struct hdac_ext_stream *hext_stream; struct hdac_stream *hstream; struct snd_sof_dev *sdev; + struct snd_soc_dai *dai; + int cpu_dai_id; int stream_id; int ret;
@@ -392,7 +395,12 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, /* configure TLV */ ipc4_copier = widget_to_copier(w);
- dma_config_tlv = &ipc4_copier->dma_config_tlv[0]; + for_each_rtd_cpu_dais(rtd, cpu_dai_id, dai) { + if (dai == cpu_dai) + break; + } + + dma_config_tlv = &ipc4_copier->dma_config_tlv[cpu_dai_id]; dma_config_tlv->type = SOF_IPC4_GTW_DMA_CONFIG_ID; /* dma_config_priv_size is zero */ dma_config_tlv->length = sizeof(dma_config_tlv->dma_config); @@ -403,7 +411,11 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, dma_config->pre_allocated_by_host = 1; dma_config->dma_channel_id = stream_id - 1; dma_config->stream_id = stream_id; - dma_config->dma_stream_channel_map.device_count = 0; /* mapping not used */ + /* + * Currently we use a DMA for each device in ALH blob. The device will + * be copied in sof_ipc4_prepare_copier_module. + */ + dma_config->dma_stream_channel_map.device_count = 1; dma_config->dma_priv_config_size = 0;
skip_tlv: @@ -440,7 +452,10 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); + struct sof_ipc4_dma_config_tlv *dma_config_tlv; const struct hda_dai_widget_dma_ops *ops; + struct sof_ipc4_dma_config *dma_config; + struct sof_ipc4_copier *ipc4_copier; struct hdac_ext_stream *hext_stream; struct snd_soc_dai *dai; struct snd_sof_dev *sdev; @@ -448,6 +463,7 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, int cpu_dai_id; int ch_mask; int ret; + int i;
ret = non_hda_dai_hw_params(substream, params, cpu_dai); if (ret < 0) { @@ -489,6 +505,22 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, return ret; }
+ ipc4_copier = widget_to_copier(w); + dma_config_tlv = &ipc4_copier->dma_config_tlv[cpu_dai_id]; + dma_config = &dma_config_tlv->dma_config; + dma_config->dma_stream_channel_map.mapping[0].device = link_id << 8 | cpu_dai->id; + dma_config->dma_stream_channel_map.mapping[0].channel_mask = ch_mask; + + /* + * copy the dma_config_tlv to all ipc4_copier in the same link. Because only one copier + * will be handled in sof_ipc4_prepare_copier_module. + */ + for_each_rtd_cpu_dais(rtd, i, dai) { + w = snd_soc_dai_get_widget(dai, substream->stream); + ipc4_copier = widget_to_copier(w); + memcpy(&ipc4_copier->dma_config_tlv[cpu_dai_id], dma_config_tlv, + sizeof(*dma_config_tlv)); + } return 0; }
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index 1870488b3cf6..28ab687a1d9c 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1704,6 +1704,7 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, */ if (ipc4_copier->dai_type == SOF_DAI_INTEL_ALH) { struct sof_ipc4_alh_configuration_blob *blob; + struct sof_ipc4_dma_config *dma_config; struct sof_ipc4_copier_data *alh_data; struct sof_ipc4_copier *alh_copier; struct snd_sof_widget *w; @@ -1743,6 +1744,18 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget, 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; + + /* + * The mapping[i] device in ALH blob should be the same as the + * dma_config_tlv[i] mapping device if a dma_config_tlv is present. + * The device id will be used for DMA tlv mapping purposes. + */ + if (ipc4_copier->dma_config_tlv[i].length) { + dma_config = &ipc4_copier->dma_config_tlv[i].dma_config; + blob->alh_cfg.mapping[i].device = + dma_config->dma_stream_channel_map.mapping[0].device; + } + /* * Set the same channel mask for playback as the audio data is * duplicated for all speakers. For capture, split the channels
We need to be able to set the dai config differently for SoundWire.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 51 ++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index de71e1595a78..c48ac931753c 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -221,15 +221,15 @@ static int __maybe_unused hda_dai_hw_free(struct snd_pcm_substream *substream, return hda_link_dma_cleanup(substream, hext_stream, cpu_dai); }
-static int __maybe_unused hda_dai_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *dai) +static int __maybe_unused hda_dai_hw_params_data(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai, + struct snd_sof_dai_config_data *data, + unsigned int flags) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(dai, substream->stream); const struct hda_dai_widget_dma_ops *ops = hda_dai_get_ops(substream, dai); struct hdac_ext_stream *hext_stream; - struct snd_sof_dai_config_data data = { 0 }; - unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_PARAMS; struct snd_sof_dev *sdev = widget_to_sdev(w); int ret;
@@ -249,9 +249,19 @@ static int __maybe_unused hda_dai_hw_params(struct snd_pcm_substream *substream, hext_stream = ops->get_hext_stream(sdev, dai, substream);
flags |= SOF_DAI_CONFIG_FLAGS_2_STEP_STOP << SOF_DAI_CONFIG_FLAGS_QUIRK_SHIFT; - data.dai_data = hdac_stream(hext_stream)->stream_tag - 1; + data->dai_data = hdac_stream(hext_stream)->stream_tag - 1;
- return hda_dai_config(w, flags, &data); + return hda_dai_config(w, flags, data); +} + +static int __maybe_unused hda_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct snd_sof_dai_config_data data = { 0 }; + unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_PARAMS; + + return hda_dai_hw_params_data(substream, params, dai, &data, flags); }
/* @@ -341,9 +351,11 @@ static struct sof_ipc4_copier *widget_to_copier(struct snd_soc_dapm_widget *w) return ipc4_copier; }
-static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params, - struct snd_soc_dai *cpu_dai) +static int non_hda_dai_hw_params_data(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *cpu_dai, + struct snd_sof_dai_config_data *data, + unsigned int flags) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); @@ -366,9 +378,9 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, }
/* use HDaudio stream handling */ - ret = hda_dai_hw_params(substream, params, cpu_dai); + ret = hda_dai_hw_params_data(substream, params, cpu_dai, data, flags); if (ret < 0) { - dev_err(cpu_dai->dev, "%s: hda_dai_hw_params failed: %d\n", __func__, ret); + dev_err(cpu_dai->dev, "%s: hda_dai_hw_params_data failed: %d\n", __func__, ret); return ret; }
@@ -422,6 +434,16 @@ static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, return 0; }
+static int non_hda_dai_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *cpu_dai) +{ + struct snd_sof_dai_config_data data = { 0 }; + unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_PARAMS; + + return non_hda_dai_hw_params_data(substream, params, cpu_dai, &data, flags); +} + static int non_hda_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai) { @@ -453,6 +475,8 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_dma_config_tlv *dma_config_tlv; + struct snd_sof_dai_config_data data = { 0 }; + unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_PARAMS; const struct hda_dai_widget_dma_ops *ops; struct sof_ipc4_dma_config *dma_config; struct sof_ipc4_copier *ipc4_copier; @@ -465,7 +489,8 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, int ret; int i;
- ret = non_hda_dai_hw_params(substream, params, cpu_dai); + data.dai_index = (link_id << 8) | cpu_dai->id; + ret = non_hda_dai_hw_params_data(substream, params, cpu_dai, &data, flags); if (ret < 0) { dev_err(cpu_dai->dev, "%s: non_hda_dai_hw_params failed %d\n", __func__, ret); return ret;
We've already defined the value for dai_index, let's use it instead of open-coding the same thing. No functionality change.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index c48ac931753c..86efcbe8f0d8 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -533,7 +533,7 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, ipc4_copier = widget_to_copier(w); dma_config_tlv = &ipc4_copier->dma_config_tlv[cpu_dai_id]; dma_config = &dma_config_tlv->dma_config; - dma_config->dma_stream_channel_map.mapping[0].device = link_id << 8 | cpu_dai->id; + dma_config->dma_stream_channel_map.mapping[0].device = data.dai_index; dma_config->dma_stream_channel_map.mapping[0].channel_mask = ch_mask;
/*
Add intel_alh_id to set the expected gateway node_id in a follow-up patch.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 3 ++- sound/soc/sof/intel/hda.c | 3 ++- sound/soc/sof/intel/hda.h | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 86efcbe8f0d8..5e3229c8fe13 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -470,7 +470,8 @@ static const struct snd_soc_dai_ops dmic_dai_ops = { int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *cpu_dai, - int link_id) + int link_id, + int intel_alh_id) { struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, substream->stream); struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index e26b8fd682e5..63f1cf3b915f 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -159,7 +159,8 @@ static int sdw_ace2x_params_stream(struct device *dev, return sdw_hda_dai_hw_params(params_data->substream, params_data->hw_params, params_data->dai, - params_data->link_id); + params_data->link_id, + params_data->alh_stream_id); }
static int sdw_ace2x_free_stream(struct device *dev, diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 81a1d4606d3c..eedd03020889 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -850,7 +850,8 @@ static inline bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev) int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *cpu_dai, - int link_id); + int link_id, + int intel_alh_id);
int sdw_hda_dai_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *cpu_dai,
The node_id value needs to be handled specifically for ALH.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 1 + sound/soc/sof/intel/hda.c | 1 + sound/soc/sof/ipc4-topology.c | 8 ++++++-- sound/soc/sof/sof-audio.h | 1 + 4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 5e3229c8fe13..86c2325e5949 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -491,6 +491,7 @@ int sdw_hda_dai_hw_params(struct snd_pcm_substream *substream, int i;
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); if (ret < 0) { dev_err(cpu_dai->dev, "%s: non_hda_dai_hw_params failed %d\n", __func__, ret); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 63f1cf3b915f..ae1a38f20bdb 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -145,6 +145,7 @@ static int sdw_params_stream(struct device *dev,
data.dai_index = (params_data->link_id << 8) | d->id; data.dai_data = params_data->alh_stream_id; + data.dai_node_id = data.dai_data;
return hda_dai_config(w, SOF_DAI_CONFIG_FLAGS_HW_PARAMS, &data); } diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index 28ab687a1d9c..6b9b16f3157e 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -2857,7 +2857,11 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget * case SOF_DAI_INTEL_HDA: gtw_attr = ipc4_copier->gtw_attr; gtw_attr->lp_buffer_alloc = pipeline->lp_mode; - fallthrough; + if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS) { + copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK; + copier_data->gtw_cfg.node_id |= SOF_IPC4_NODE_INDEX(data->dai_data); + } + break; case SOF_DAI_INTEL_ALH: /* * Do not clear the node ID when this op is invoked with @@ -2866,7 +2870,7 @@ static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget * */ if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS) { copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK; - copier_data->gtw_cfg.node_id |= SOF_IPC4_NODE_INDEX(data->dai_data); + copier_data->gtw_cfg.node_id |= SOF_IPC4_NODE_INDEX(data->dai_node_id); } break; case SOF_DAI_INTEL_DMIC: diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 86bbb531e142..7c08f915b35b 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -91,6 +91,7 @@ struct snd_sof_pcm; struct snd_sof_dai_config_data { int dai_index; int dai_data; /* contains DAI-specific information */ + int dai_node_id; /* contains DAI-specific information for Gateway configuration */ };
/**
To allow using widget_to_sdev() in other files, move it as static inline in shared header file.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 86c2325e5949..3f2fd84907d2 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 eedd03020889..c939a24d770e 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -1006,4 +1006,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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 ae1a38f20bdb..2c64c25d6f3b 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 "telemetry.h"
@@ -150,8 +151,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 6b9b16f3157e..e0e4f356275f 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1296,7 +1296,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;
@@ -1306,9 +1305,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; } }
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.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 e693dcb475e4..b5ca2861edbd 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; }
/*
The same sequence is used twice, use common helper.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 f03cee94bce6..ed09756f6af6 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,
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.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 ed09756f6af6..fbd7e045bcfb 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; @@ -345,6 +347,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 b5ca2861edbd..32fef64ef10d 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 7c08f915b35b..f4134257789e 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -349,6 +349,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 {
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.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Co-developed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 fbd7e045bcfb..4e11df6b4823 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: Ranjani Sridharan ranjani.sridharan@linux.intel.com
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.
Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- 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 2c64c25d6f3b..d38dc43c2f1c 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -164,6 +164,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 e0e4f356275f..c29c1de4e925 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1732,6 +1732,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; @@ -1739,7 +1741,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 @@ -2862,12 +2867,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 aa5122c3721d..6e33208a357f 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)
On Tue, 02 Apr 2024 10:18:11 -0500, Pierre-Louis Bossart wrote:
This patchset contains important updates for SoundWire support.
We initially implemented support for multiple amplifiers on different links using a single HDaudio DMA transfer. To align with the other OS, the 'aggregation' is now supported by the firmware. This change in directions has kernel impacts, since we now have multiple HDaudio DMAs to program and start, but since there are no platforms released so far there's no end-user impact.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[01/17] ASoC: SOF: Intel: hda-dai: fix channel map configuration for aggregated dailink commit: 831045513c8a2ef14c3cf39b33d1ccedf588c4a8 [02/17] Revert "ASoC: SOF: Intel: hda-dai-ops: reset device count for SoundWire DAIs" commit: 24b1f93df400e1ab1731e7bcb320e693a6a73792 [03/17] Revert "ASoC: SOF: Intel: hda-dai-ops: only allocate/release streams for first CPU DAI" commit: 1d0fb3d0c30749779cdd88be98761b17ebfe5590 [04/17] ASoC: SOF: make dma_config_tlv be an array commit: e9c6b118de1afc1d32a4eb3bc9f3d114d4fe0f1a [05/17] ASoC: SOF: Intel: hda-dai: set lowest N bits in ch_mask commit: 8fa10a243600ca8bd92fdc871100deb308fca5f1 [06/17] ASoC: SOF: Intel: hda-dai: set dma_stream_channel_map device commit: 17386cb1b48b0d85f69b21ae13d5408d67180e30 [07/17] ASoC: SOF: Intel: hda-dai: add helpers to set dai config commit: 58f32cb7011a8a15b18f35e4c0ee044aa98e365b [08/17] ASoC: SOF: Intel: set the DMA TLV device as dai_index commit: 219271481e8965e80ee425cdc2db85230a333a97 [09/17] ASoC: SOF: Intel: hda: extend signature of sdw_hda_dai_hw_params() commit: 8bc3b56cac748f6ef6a4b96c906007a546e7fb5a [10/17] ASoC: SOF: IPC4: extend dai_data with node_id commit: a936456d4bce27edc1a18dab270c657e9c07590c [11/17] ASoC: SOF: Intel: hda: move helper to static inline commit: 2ac9e09ba0e874deeba13c3259dc18f22b622311 [12/17] ASoC: SOF: Intel: hda: Clear Soundwire node ID during BE DAI hw_free commit: bfe9225455c032c9dd5637047760cf59562e599f [13/17] ASoC: SOF: sof-audio: revisit sof_pcm_stream_free() error handling and logs commit: a6f2b279d22894e81b23464620d03da6429d9ab5 [14/17] ASoC: SOF: pcm: simplify sof_pcm_hw_free() with helper commit: bb83ae04d9158276d17640f50c2a1e049100acb6 [15/17] ASoC: SOF: pcm: add pending_stop state variable commit: dbc78bce74f5f9057ba02bdc8d1549d24c573900 [16/17] ASoC: SOF: pcm: reset all PCM sources in case of xruns commit: ebd3b3014eebdd490f2c509d79e719fbcc680963 [17/17] ASoC: SOF: ipc4-topology: Save the ALH DAI index during hw_params commit: f0caa4fc244ca739ce6d12168aa588c412c81190
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart