[PATCH 0/2] ASoC: SOF: ipc4: Fixup dailink format based on copier
Hi,
If the copier supports a single format on the DAI side we should fixup the BE dailink to use this single format.
Regards, Peter --- Bard Liao (2): ASoC: SOF: ipc4-topology: export sof_ipc4_copier_is_single_format ASoC: SOF: ipc4-pcm: fixup dailink based on copier format
sound/soc/sof/ipc4-pcm.c | 55 ++++++++++++++++++++++++++++++----- sound/soc/sof/ipc4-topology.c | 8 ++--- sound/soc/sof/ipc4-topology.h | 3 ++ 3 files changed, 54 insertions(+), 12 deletions(-)
From: Bard Liao yung-chuan.liao@linux.intel.com
We will use the sof_ipc4_copier_is_single_format() function to check if a ipc4 copier has single format available in ipc4-pcm.c in the next patch.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/ipc4-topology.c | 8 ++++---- sound/soc/sof/ipc4-topology.h | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index f2a30cd31378..21fae27eb67c 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1369,9 +1369,9 @@ static int snd_sof_get_nhlt_endpoint_data(struct snd_sof_dev *sdev, struct snd_s } #endif
-static bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev, - struct sof_ipc4_pin_format *pin_fmts, - u32 pin_fmts_size) +bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev, + struct sof_ipc4_pin_format *pin_fmts, + u32 pin_fmts_size) { struct sof_ipc4_audio_format *fmt; u32 valid_bits; @@ -1380,7 +1380,7 @@ static bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev, fmt = &pin_fmts[0].audio_fmt; valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg);
- /* check if all output formats in topology are the same */ + /* check if all formats in topology are the same */ for (i = 1; i < pin_fmts_size; i++) { u32 _valid_bits;
diff --git a/sound/soc/sof/ipc4-topology.h b/sound/soc/sof/ipc4-topology.h index d75f17f4749c..d94f0ab4aee3 100644 --- a/sound/soc/sof/ipc4-topology.h +++ b/sound/soc/sof/ipc4-topology.h @@ -442,4 +442,7 @@ struct sof_ipc4_process { u32 init_config; };
+bool sof_ipc4_copier_is_single_format(struct snd_sof_dev *sdev, + struct sof_ipc4_pin_format *pin_fmts, + u32 pin_fmts_size); #endif
From: Bard Liao yung-chuan.liao@linux.intel.com
When a copier exposes a single format, we can fixup the BE dailink with that format. This is helpful when some codec have format restrictions and e.g. don't support a 32-bit format. In that case, the copier output formats mirror that restriction in the topology file.
An alternate solution was suggested earlier using a dedicated topology token. When specified, the token would be used to fix-up the dailink. The main reason why this solution was chosen is that there is a risk of a disconnect between token definition and copier format. With a single piece of information as suggested in this patch, there are fewer risks of a bad configuration.
Signed-off-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/ipc4-pcm.c | 55 ++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index db19cd03ecad..775c864313fa 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -517,11 +517,14 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, { struct snd_soc_component *component = snd_soc_rtdcom_lookup(rtd, SOF_AUDIO_PCM_DRV_NAME); struct snd_sof_dai *dai = snd_sof_find_dai(component, rtd->dai_link->name); + struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(component); struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); + struct sof_ipc4_audio_format *ipc4_fmt; struct sof_ipc4_copier *ipc4_copier; - bool use_chain_dma = false; - int dir; + bool single_fmt = false; + u32 valid_bits = 0; + int dir, ret;
if (!dai) { dev_err(component->dev, "%s: No DAI found with name %s\n", __func__, @@ -540,21 +543,57 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_soc_dapm_widget *w = snd_soc_dai_get_widget(cpu_dai, dir);
if (w) { + struct sof_ipc4_available_audio_format *available_fmt = + &ipc4_copier->available_fmt; struct snd_sof_widget *swidget = w->dobj.private; struct snd_sof_widget *pipe_widget = swidget->spipe->pipe_widget; struct sof_ipc4_pipeline *pipeline = pipe_widget->private;
+ /* Chain DMA does not use copiers, so no fixup needed */ if (pipeline->use_chain_dma) - use_chain_dma = true; + return 0; + + if (dir == SNDRV_PCM_STREAM_PLAYBACK) { + if (sof_ipc4_copier_is_single_format(sdev, + available_fmt->output_pin_fmts, + available_fmt->num_output_formats)) { + ipc4_fmt = &available_fmt->output_pin_fmts->audio_fmt; + single_fmt = true; + } + } else { + if (sof_ipc4_copier_is_single_format(sdev, + available_fmt->input_pin_fmts, + available_fmt->num_input_formats)) { + ipc4_fmt = &available_fmt->input_pin_fmts->audio_fmt; + single_fmt = true; + } + } } }
- /* Chain DMA does not use copiers, so no fixup needed */ - if (!use_chain_dma) { - int ret = sof_ipc4_pcm_dai_link_fixup_rate(sdev, params, ipc4_copier); + ret = sof_ipc4_pcm_dai_link_fixup_rate(sdev, params, ipc4_copier); + if (ret) + return ret;
- if (ret) - return ret; + if (single_fmt) { + snd_mask_none(fmt); + valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(ipc4_fmt->fmt_cfg); + dev_dbg(component->dev, "Set %s to %d bit format\n", dai->name, valid_bits); + } + + /* Set format if it is specified */ + switch (valid_bits) { + case 16: + snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE); + break; + case 24: + snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE); + break; + case 32: + snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE); + break; + default: + break; }
switch (ipc4_copier->dai_type) {
On Fri, 15 Sep 2023 12:35:05 +0300, Peter Ujfalusi wrote:
If the copier supports a single format on the DAI side we should fixup the BE dailink to use this single format.
Regards, Peter
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/2] ASoC: SOF: ipc4-topology: export sof_ipc4_copier_is_single_format commit: 94fc6da924072399e4f475c7d7124a7272197e6a [2/2] ASoC: SOF: ipc4-pcm: fixup dailink based on copier format commit: 26dfc43461102957e33454e766d592df330ef7a0
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
-
Peter Ujfalusi