ASoC: SOF: sof-audio: Fix broken early bclk feature for SSP

Commit Message

Peter Ujfalusi March 7, 2023, 11:46 a.m. UTC

From: Ranjani Sridharan <[e-mail]>

With the removal of widget setup during BE hw_params, the DAI config IPC
is never sent with the SOF_DAI_CONFIG_FLAGS_HW_PARAMS. This means that
the early bit clock feature required for certain codecs will be broken.

Fix this by saving the config flags sent during BE DAI hw_params and
reusing it when the DAI_CONFIG IPC is sent after the DAI widget is set
up. Also, free the DAI config before the widget is freed.

The DAI_CONFIG IPC sent during the sof_widget_free() does not have the
DAI index information. So, save the dai_index in the config during
hw_params and reuse it during hw_free.

For IPC4, do not clear the node ID during hw_free. It will be needed for
freeing the group_ida during unprepare.

Signed-off-by: Ranjani Sridharan <[e-mail]>
Reviewed-by: Pierre-Louis Bossart <[e-mail]>
Reviewed-by: Rander Wang <[e-mail]>
Reviewed-by: Bard Liao <[e-mail]>
Reviewed-by: Péter Ujfalusi <[e-mail]>
Signed-off-by: Peter Ujfalusi <[e-mail]>
---
 sound/soc/sof/ipc3-topology.c | 32 ++++++++++++++++++++++++++++++--
 sound/soc/sof/ipc4-topology.c | 15 +++++++++++++--
 sound/soc/sof/sof-audio.c     | 28 +++++++++++++++++++++++++---
 3 files changed, 68 insertions(+), 7 deletions(-)

Comments

Mark Brown March 8, 2023, 1:52 p.m. UTC | #1

On Tue, 07 Mar 2023 13:46:39 +0200, Peter Ujfalusi wrote:
> With the removal of widget setup during BE hw_params, the DAI config IPC
> is never sent with the SOF_DAI_CONFIG_FLAGS_HW_PARAMS. This means that
> the early bit clock feature required for certain codecs will be broken.
> 
> Fix this by saving the config flags sent during BE DAI hw_params and
> reusing it when the DAI_CONFIG IPC is sent after the DAI widget is set
> up. Also, free the DAI config before the widget is freed.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: SOF: sof-audio: Fix broken early bclk feature for SSP
      commit: b66bfc3a9810caed5d55dd8907110bdc8028b06b

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

Patch

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index dceb78bfe17c..b1f425b39db9 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2081,7 +2081,9 @@  static int sof_ipc3_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
 		break;
 	case SOF_DAI_INTEL_ALH:
 		if (data) {
-			config->dai_index = data->dai_index;
+			/* save the dai_index during hw_params and reuse it for hw_free */
+			if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS)
+				config->dai_index = data->dai_index;
 			config->alh.stream_id = data->dai_data;
 		}
 		break;
@@ -2089,7 +2091,30 @@  static int sof_ipc3_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
 		break;
 	}
 
-	config->flags = flags;
+	/*
+	 * The dai_config op is invoked several times and the flags argument varies as below:
+	 * BE DAI hw_params: When the op is invoked during the BE DAI hw_params, flags contains
+	 * SOF_DAI_CONFIG_FLAGS_HW_PARAMS along with quirks
+	 * FE DAI hw_params: When invoked during FE DAI hw_params after the DAI widget has
+	 * just been set up in the DSP, flags is set to SOF_DAI_CONFIG_FLAGS_HW_PARAMS with no
+	 * quirks
+	 * BE DAI trigger: When invoked during the BE DAI trigger, flags is set to
+	 * SOF_DAI_CONFIG_FLAGS_PAUSE and contains no quirks
+	 * BE DAI hw_free: When invoked during the BE DAI hw_free, flags is set to
+	 * SOF_DAI_CONFIG_FLAGS_HW_FREE and contains no quirks
+	 * FE DAI hw_free: When invoked during the FE DAI hw_free, flags is set to
+	 * SOF_DAI_CONFIG_FLAGS_HW_FREE and contains no quirks
+	 *
+	 * The DAI_CONFIG IPC is sent to the DSP, only after the widget is set up during the FE
+	 * DAI hw_params. But since the BE DAI hw_params precedes the FE DAI hw_params, the quirks
+	 * need to be preserved when assigning the flags before sending the IPC.
+	 * For the case of PAUSE/HW_FREE, since there are no quirks, flags can be used as is.
+	 */
+
+	if (flags & SOF_DAI_CONFIG_FLAGS_HW_PARAMS)
+		config->flags |= flags;
+	else
+		config->flags = flags;
 
 	/* only send the IPC if the widget is set up in the DSP */
 	if (swidget->use_count > 0) {
@@ -2097,6 +2122,9 @@  static int sof_ipc3_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
 					 &reply, sizeof(reply));
 		if (ret < 0)
 			dev_err(sdev->dev, "Failed to set dai config for %s\n", dai->name);
+
+		/* clear the flags once the IPC has been sent even if it fails */
+		config->flags = SOF_DAI_CONFIG_FLAGS_NONE;
 	}
 
 	return ret;
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index 3e27c7a48ebd..37927e9a700d 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -980,6 +980,7 @@  static void sof_ipc4_unprepare_copier_module(struct snd_sof_widget *swidget)
 
 		ipc4_copier = dai->private;
 		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;
 
@@ -989,6 +990,9 @@  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;
 		}
 	}
 
@@ -1940,8 +1944,15 @@  static int sof_ipc4_dai_config(struct snd_sof_dev *sdev, struct snd_sof_widget *
 		pipeline->skip_during_fe_trigger = true;
 		fallthrough;
 	case SOF_DAI_INTEL_ALH:
-		copier_data->gtw_cfg.node_id &= ~SOF_IPC4_NODE_INDEX_MASK;
-		copier_data->gtw_cfg.node_id |= SOF_IPC4_NODE_INDEX(data->dai_data);
+		/*
+		 * 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.
+		 */
+		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_DMIC:
 	case SOF_DAI_INTEL_SSP:
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 760621bfc802..d7df29f2ada8 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -50,9 +50,27 @@  static int sof_widget_free_unlocked(struct snd_sof_dev *sdev,
 	/* reset route setup status for all routes that contain this widget */
 	sof_reset_route_setup_status(sdev, swidget);
 
+	/* free DAI config and continue to free widget even if it fails */
+	if (WIDGET_IS_DAI(swidget->id)) {
+		struct snd_sof_dai_config_data data;
+		unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_FREE;
+
+		data.dai_data = DMA_CHAN_INVALID;
+
+		if (tplg_ops && tplg_ops->dai_config) {
+			err = tplg_ops->dai_config(sdev, swidget, flags, &data);
+			if (err < 0)
+				dev_err(sdev->dev, "failed to free config for widget %s\n",
+					swidget->widget->name);
+		}
+	}
+
 	/* continue to disable core even if IPC fails */
-	if (tplg_ops && tplg_ops->widget_free)
-		err = tplg_ops->widget_free(sdev, swidget);
+	if (tplg_ops && tplg_ops->widget_free) {
+		ret = tplg_ops->widget_free(sdev, swidget);
+		if (ret < 0 && !err)
+			err = ret;
+	}
 
 	/*
 	 * disable widget core. continue to route setup status and complete flag
@@ -151,8 +169,12 @@  static int sof_widget_setup_unlocked(struct snd_sof_dev *sdev,
 
 	/* send config for DAI components */
 	if (WIDGET_IS_DAI(swidget->id)) {
-		unsigned int flags = SOF_DAI_CONFIG_FLAGS_NONE;
+		unsigned int flags = SOF_DAI_CONFIG_FLAGS_HW_PARAMS;
 
+		/*
+		 * The config flags saved during BE DAI hw_params will be used for IPC3. IPC4 does
+		 * not use the flags argument.
+		 */
 		if (tplg_ops && tplg_ops->dai_config) {
 			ret = tplg_ops->dai_config(sdev, swidget, flags, NULL);
 			if (ret < 0)