[PATCH 0/3] ASoC: SOF: ipc4-topology: Pipeline params improvements

[PATCH 0/3] ASoC: SOF: ipc4-topology: Pipeline params improvements

[Peter Ujfalusi]

Hi,

Improve handling of some corner cases that are not used by current topology
designs, but can be crafted within the rules of a topology file.

For example branching topologies, where a single input routed to multiple
output endpoints. The already configured part of the graph places constraint
on how the new branch can be configured.

Handling of process modules also updated to be able to 'guess' what
parameters can be changed by the module and allow flexible operation.

Regards,
Peter
--
Peter Ujfalusi (3):
  ASoC: SOF: ipc4-topology: Store the params change between input/output
    of a module
  ASoC: SOF: ipc4-topology: Correct the process module's output lookup
  ASoC: SOF: ipc4-topology: Update the pipeline_params of prepared
    modules

 sound/soc/sof/ipc4-topology.c | 201 ++++++++++++++++++++++++++++++----
 sound/soc/sof/ipc4-topology.h |   3 +
 sound/soc/sof/sof-audio.c     |   5 +-
 3 files changed, 183 insertions(+), 26 deletions(-)

-- 
2.55.0

[PATCH 1/3] ASoC: SOF: ipc4-topology: Store the params change between input/output of a module

[Peter Ujfalusi]

Based on the input and output formats we can evaluate what param might be
changed by the module instance.
If there is a difference between the input rate/channels/format and the
output  rate/channels/format it means that the module can change one or
multiple of the params.

Store this information during init for later use.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/ipc4-topology.c | 41 +++++++++++++++++++++++++++++++++++
 sound/soc/sof/ipc4-topology.h |  3 +++
 2 files changed, 44 insertions(+)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index 8ac7dde32f77..df002e69d2a7 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -406,6 +406,39 @@ sof_ipc4_get_input_pin_audio_fmt(struct snd_sof_widget *swidget, int pin_index)
 	return NULL;
 }
 
+static void
+sof_ipc4_evaluate_params_change(struct sof_ipc4_available_audio_format *available_fmt)
+{
+	struct sof_ipc4_audio_format *fmt;
+	u32 in_rate, in_channels, in_valid_bits;
+	u32 out_rate, out_channels, out_valid_bits;
+	u32 changed_params = 0;
+	int i, j;
+
+	for (i = 0; i < available_fmt->num_input_formats; i++) {
+		fmt = &available_fmt->input_pin_fmts[i].audio_fmt;
+		in_rate = fmt->sampling_frequency;
+		in_channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg);
+		in_valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg);
+
+		for (j = 0; j < available_fmt->num_output_formats; j++) {
+			fmt = &available_fmt->output_pin_fmts[j].audio_fmt;
+			out_rate = fmt->sampling_frequency;
+			out_channels = SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(fmt->fmt_cfg);
+			out_valid_bits = SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(fmt->fmt_cfg);
+
+			if (in_rate != out_rate)
+				changed_params |= BIT(SNDRV_PCM_HW_PARAM_RATE);
+			if (in_channels != out_channels)
+				changed_params |= BIT(SNDRV_PCM_HW_PARAM_CHANNELS);
+			if (in_valid_bits != out_valid_bits)
+				changed_params |= BIT(SNDRV_PCM_HW_PARAM_FORMAT);
+		}
+	}
+
+	available_fmt->changed_params = changed_params;
+}
+
 /**
  * sof_ipc4_get_audio_fmt - get available audio formats from swidget->tuples
  * @scomp: pointer to pointer to SOC component
@@ -497,6 +530,8 @@ static int sof_ipc4_get_audio_fmt(struct snd_soc_component *scomp,
 					  available_fmt->num_output_formats);
 	}
 
+	sof_ipc4_evaluate_params_change(available_fmt);
+
 	return 0;
 
 err_out:
@@ -661,6 +696,9 @@ static int sof_ipc4_widget_setup_pcm(struct snd_sof_widget *swidget)
 	if (ret)
 		goto free_copier;
 
+	/* Copier can only change format */
+	available_fmt->changed_params &= BIT(SNDRV_PCM_HW_PARAM_FORMAT);
+
 	/*
 	 * This callback is used by host copier and module-to-module copier,
 	 * and only host copier needs to set gtw_cfg.
@@ -789,6 +827,9 @@ static int sof_ipc4_widget_setup_comp_dai(struct snd_sof_widget *swidget)
 	if (ret)
 		goto free_copier;
 
+	/* Copier can only change format */
+	available_fmt->changed_params &= BIT(SNDRV_PCM_HW_PARAM_FORMAT);
+
 	ret = sof_update_ipc_object(scomp, &node_type,
 				    SOF_COPIER_TOKENS, swidget->tuples,
 				    swidget->num_tuples, sizeof(node_type), 1);
diff --git a/sound/soc/sof/ipc4-topology.h b/sound/soc/sof/ipc4-topology.h
index 54bf0236a127..c9fe9f2e4083 100644
--- a/sound/soc/sof/ipc4-topology.h
+++ b/sound/soc/sof/ipc4-topology.h
@@ -197,12 +197,15 @@ struct sof_ipc4_pin_format {
  * @input_pin_fmts: Available input pin formats
  * @num_input_formats: Number of input pin formats
  * @num_output_formats: Number of output pin formats
+ * @changed_params: Mask of changed params by the module instance between it's
+ *		    input and output formts (rate, channels, depth)
  */
 struct sof_ipc4_available_audio_format {
 	struct sof_ipc4_pin_format *output_pin_fmts;
 	struct sof_ipc4_pin_format *input_pin_fmts;
 	u32 num_input_formats;
 	u32 num_output_formats;
+	u32 changed_params;
 };
 
 /**
-- 
2.55.0

[PATCH 2/3] ASoC: SOF: ipc4-topology: Correct the process module's output lookup

[Peter Ujfalusi]

The process module can change different parameters in the audio path and
this change has to be properly evaluated and applied.

In case of playback we are converting from multiple input formats to a
single format (or just passing through without change), the output format
lookup must be based on the input format.

In case of capture, we are converting from a single input format to a
format which is to be passed to the FE, we need to use the input parameters
and the FE parameters to be able to find the correct format:
for those parameters that are modified by the module instance we need to
use the FE parameter while for the rest we use the input parameters.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/ipc4-topology.c | 66 +++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index df002e69d2a7..6f9f5325a50b 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -2863,39 +2863,69 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget,
 	if (available_fmt->num_output_formats) {
 		struct sof_ipc4_audio_format *in_fmt;
 		struct sof_ipc4_pin_format *pin_fmt;
-		u32 out_ref_rate, out_ref_channels;
-		int out_ref_valid_bits, out_ref_type;
+		u32 ref_rate, ref_channels;
+		int ref_valid_bits, ref_type;
 
 		if (available_fmt->num_input_formats) {
+			/*
+			 * The process module can change parameters and their operation
+			 * depends on the direction:
+			 * Playback: typically they have single output format. This is
+			 *	     to 'force' the conversion from input to output.
+			 *	     Use the input format as reference since the single
+			 *	     format is going to be picked.
+			 * Capture: typically they have multiple output formats to
+			 *	    convert from dai (input) to FE (output) parameters.
+			 *          Use the input format as base and replace the param
+			 *	    which is changed by the module with the FE parameter
+			 *	    Reason: we can have module which changes the
+			 *	            parameters in path, we cannot use the full
+			 *		    FE param set for the module output lookup.
+			 */
 			in_fmt = &available_fmt->input_pin_fmts[input_fmt_index].audio_fmt;
 
-			out_ref_rate = in_fmt->sampling_frequency;
-			out_ref_channels =
+			ref_rate = in_fmt->sampling_frequency;
+			ref_channels =
 				SOF_IPC4_AUDIO_FORMAT_CFG_CHANNELS_COUNT(in_fmt->fmt_cfg);
-			out_ref_valid_bits =
+			ref_valid_bits =
 				SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(in_fmt->fmt_cfg);
-			out_ref_type = sof_ipc4_fmt_cfg_to_type(in_fmt->fmt_cfg);
+			ref_type = sof_ipc4_fmt_cfg_to_type(in_fmt->fmt_cfg);
 		} else {
 			/* for modules without input formats, use FE params as reference */
-			out_ref_rate = params_rate(fe_params);
-			out_ref_channels = params_channels(fe_params);
+			ref_rate = params_rate(fe_params);
+			ref_channels = params_channels(fe_params);
 			ret = sof_ipc4_get_sample_type(sdev, fe_params);
 			if (ret < 0)
 				return ret;
-			out_ref_type = (u32)ret;
+			ref_type = (u32)ret;
 
-			out_ref_valid_bits = sof_ipc4_get_valid_bits(sdev, fe_params);
-			if (out_ref_valid_bits < 0)
-				return out_ref_valid_bits;
+			ref_valid_bits = sof_ipc4_get_valid_bits(sdev, fe_params);
+			if (ref_valid_bits < 0)
+				return ref_valid_bits;
 		}
 
+		if (dir == SNDRV_PCM_STREAM_CAPTURE) {
+			if (available_fmt->changed_params & BIT(SNDRV_PCM_HW_PARAM_RATE))
+				ref_rate = params_rate(fe_params);
+			if (available_fmt->changed_params & BIT(SNDRV_PCM_HW_PARAM_CHANNELS))
+				ref_channels = params_channels(fe_params);
+			if (available_fmt->changed_params & BIT(SNDRV_PCM_HW_PARAM_FORMAT)) {
+				ref_valid_bits = sof_ipc4_get_valid_bits(sdev, fe_params);
+				if (ref_valid_bits < 0)
+					return ref_valid_bits;
+
+				ref_type = sof_ipc4_get_sample_type(sdev, fe_params);
+				if (ref_type < 0)
+					return ref_type;
+			}
+		}
 		output_fmt_index = sof_ipc4_init_output_audio_fmt(sdev, swidget,
 								  &process->base_config,
 								  available_fmt,
-								  out_ref_rate,
-								  out_ref_channels,
-								  out_ref_valid_bits,
-								  out_ref_type);
+								  ref_rate,
+								  ref_channels,
+								  ref_valid_bits,
+								  ref_type);
 		if (output_fmt_index < 0)
 			return output_fmt_index;
 
@@ -2909,9 +2939,7 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget,
 			/* modify the pipeline params with the output format */
 			ret = sof_ipc4_update_hw_params(sdev, pipeline_params,
 							&process->output_format,
-							BIT(SNDRV_PCM_HW_PARAM_FORMAT) |
-							BIT(SNDRV_PCM_HW_PARAM_CHANNELS) |
-							BIT(SNDRV_PCM_HW_PARAM_RATE));
+							available_fmt->changed_params);
 			if (ret)
 				return ret;
 		}
-- 
2.55.0

[PATCH 3/3] ASoC: SOF: ipc4-topology: Update the pipeline_params of prepared modules

[Peter Ujfalusi]

If the module in path has been already prepared on a branch type of
topology, where the branching happens downstream:
A1--> A2 ---> B1 --> B2 ... B-branch
          |-> C1 --> C2 ... C-branch

In this case if B-branch is started then A1/A2 is prepared, but when
C-branch starts we still need to refine the parameters up to C1 to arrive
with a correct params to configure C1.

This branching can happen with copiers process modules.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
---
 sound/soc/sof/ipc4-topology.c | 94 +++++++++++++++++++++++++++++++++--
 sound/soc/sof/sof-audio.c     |  5 +-
 2 files changed, 92 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index 6f9f5325a50b..9303e018b3fe 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -2106,10 +2106,57 @@ static void sof_ipc4_host_config(struct snd_sof_dev *sdev, struct snd_sof_widget
 }
 
 static int
-sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
-			       struct snd_pcm_hw_params *fe_params,
-			       struct snd_sof_platform_stream_params *platform_params,
-			       struct snd_pcm_hw_params *pipeline_params, int dir)
+sof_ipc4_copier_module_update_params(struct snd_sof_widget *swidget,
+				     struct snd_pcm_hw_params *pipeline_params)
+{
+	struct snd_soc_component *scomp = swidget->scomp;
+	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
+	struct sof_ipc4_copier_data *copier_data;
+	struct sof_ipc4_copier *ipc4_copier;
+
+	switch (swidget->id) {
+	case snd_soc_dapm_aif_in:
+	case snd_soc_dapm_aif_out:
+	case snd_soc_dapm_buffer:
+		ipc4_copier = swidget->private;
+		copier_data = &ipc4_copier->data;
+		break;
+	case snd_soc_dapm_dai_in:
+	case snd_soc_dapm_dai_out:
+	{
+		struct snd_sof_widget *pipe_widget = swidget->spipe->pipe_widget;
+		struct sof_ipc4_pipeline *pipeline = pipe_widget->private;
+		struct snd_sof_dai *dai;
+
+		if (pipeline->use_chain_dma)
+			return 0;
+
+		dai = swidget->private;
+
+		ipc4_copier = (struct sof_ipc4_copier *)dai->private;
+		copier_data = &ipc4_copier->data;
+
+		break;
+	}
+	default:
+		dev_err(sdev->dev, "unsupported type %d for copier %s",
+			swidget->id, swidget->widget->name);
+		return -EINVAL;
+	}
+
+	/* modify the input params for the next widget */
+	return sof_ipc4_update_hw_params(sdev, pipeline_params,
+					 &copier_data->out_format,
+					 BIT(SNDRV_PCM_HW_PARAM_FORMAT) |
+					 BIT(SNDRV_PCM_HW_PARAM_CHANNELS) |
+					 BIT(SNDRV_PCM_HW_PARAM_RATE));
+}
+
+static int
+_sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
+				struct snd_pcm_hw_params *fe_params,
+				struct snd_sof_platform_stream_params *platform_params,
+				struct snd_pcm_hw_params *pipeline_params, int dir)
 {
 	struct sof_ipc4_available_audio_format *available_fmt;
 	struct snd_soc_component *scomp = swidget->scomp;
@@ -2581,6 +2628,21 @@ sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
 	return 0;
 }
 
+static int
+sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
+			       struct snd_pcm_hw_params *fe_params,
+			       struct snd_sof_platform_stream_params *platform_params,
+			       struct snd_pcm_hw_params *pipeline_params, int dir)
+{
+	if (swidget->prepared)
+		return sof_ipc4_copier_module_update_params(swidget,
+							    pipeline_params);
+
+	return _sof_ipc4_prepare_copier_module(swidget, fe_params,
+					       platform_params, pipeline_params,
+					       dir);
+}
+
 static int sof_ipc4_prepare_gain_module(struct snd_sof_widget *swidget,
 					struct snd_pcm_hw_params *fe_params,
 					struct snd_sof_platform_stream_params *platform_params,
@@ -2594,6 +2656,10 @@ static int sof_ipc4_prepare_gain_module(struct snd_sof_widget *swidget,
 	u32 out_ref_rate, out_ref_channels, out_ref_valid_bits, out_ref_type;
 	int input_fmt_index, output_fmt_index;
 
+	/* This cannot happen */
+	if (unlikely(swidget->prepared))
+		return 0;
+
 	input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget,
 							&gain->data.base_config,
 							pipeline_params,
@@ -2639,6 +2705,10 @@ static int sof_ipc4_prepare_mixer_module(struct snd_sof_widget *swidget,
 	u32 out_ref_rate, out_ref_channels, out_ref_valid_bits, out_ref_type;
 	int input_fmt_index, output_fmt_index;
 
+	/* Already prepared, nothing to do */
+	if (swidget->prepared)
+		return 0;
+
 	input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget,
 							&mixer->base_config,
 							pipeline_params,
@@ -2685,6 +2755,10 @@ static int sof_ipc4_prepare_src_module(struct snd_sof_widget *swidget,
 	u32 out_ref_rate, out_ref_channels, out_ref_valid_bits, out_ref_type;
 	int output_fmt_index, input_fmt_index;
 
+	/* This cannot happen */
+	if (unlikely(swidget->prepared))
+		return 0;
+
 	input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget,
 							&src->data.base_config,
 							pipeline_params,
@@ -2851,6 +2925,18 @@ static int sof_ipc4_prepare_process_module(struct snd_sof_widget *swidget,
 	int ret;
 
 	if (available_fmt->num_input_formats) {
+		if (swidget->prepared) {
+			if (!available_fmt->num_output_formats)
+				return 0;
+
+			/* modify the pipeline params with the output format */
+			return sof_ipc4_update_hw_params(sdev, pipeline_params,
+							&process->output_format,
+							BIT(SNDRV_PCM_HW_PARAM_FORMAT) |
+							BIT(SNDRV_PCM_HW_PARAM_CHANNELS) |
+							BIT(SNDRV_PCM_HW_PARAM_RATE));
+		}
+
 		input_fmt_index = sof_ipc4_init_input_audio_fmt(sdev, swidget,
 								&process->base_config,
 								pipeline_params,
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index acf56607bc9c..a58617d2b824 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -502,9 +502,8 @@ sof_prepare_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget
 	    !sof_widget_in_same_direction(swidget, dir))
 		return 0;
 
-	/* skip widgets already prepared or aggregated DAI widgets*/
-	if (!widget_ops[widget->id].ipc_prepare || swidget->prepared ||
-	    is_aggregated_dai(swidget))
+	/* skip widgets aggregated DAI widgets */
+	if (!widget_ops[widget->id].ipc_prepare || is_aggregated_dai(swidget))
 		goto sink_prepare;
 
 	/* prepare the source widget */
-- 
2.55.0