[PATCH 00/14] ASoC: SOF: Intel: improve HDaudio DAI support

[PATCH 00/14] ASoC: SOF: Intel: improve HDaudio DAI support

[Pierre-Louis Bossart]

The SOF CI and daily tests exposed a number of issues with corner
cases on platforms using the HDaudio DAI, such as UpExtreme boards or
usual HDaudio+DMIC laptops.

This patchset provides improvements for pause_push/pause_release,
suspend-resume, mixing use cases and combinations of all three.

The initial patches provide a cleanup, the last patches improve the
state machine and DMA handling.

Pierre-Louis Bossart (12):
  ASOC: SOF: Intel: hda-dai: consistent naming for HDA DAI and HDA link
    DMA
  ASoC: SOF: Intel: hda-dai: simplify hda_dai_widget_update() prototype
  ASoC: SOF: Intel: hda-dai: use snd_soc_dai_get_widget() helper
  ASoC: SOF: Intel: hda-dai: split link DMA and dai operations
  ASoC: SOF: Intel: hda-dai: regroup dai and link DMA operations
  ASoC: SOF: sof-audio: flag errors on pipeline teardown
  ASOC: SOF: Intel: hda-dai: add hda_dai_hw_free_ipc() helper
  ASoC: SOF: Intel: hda-dai: move code to deal with hda dai/dailink
    suspend
  ASoC: SOF: Intel: hda-dai: improve suspend case
  ASoC: SOF: Intel: hda-dai: reset dma_data and release stream
  ASoC: SOF: Intel: add helper for link DMA cleanups
  ASoC: SOF: Intel: hda-dai: protect hw_params against successive calls

Ranjani Sridharan (2):
  ASoC: SOF: remove incorrect clearing of prepared flag
  ASoC: SOF: Intel: Add IPC-specific dai ops for IPC3

 sound/soc/sof/intel/apl.c     |   3 +
 sound/soc/sof/intel/cnl.c     |   3 +
 sound/soc/sof/intel/hda-dai.c | 434 ++++++++++++++++++++++------------
 sound/soc/sof/intel/hda-dsp.c |  42 +---
 sound/soc/sof/intel/hda.h     |   3 +
 sound/soc/sof/intel/icl.c     |   3 +
 sound/soc/sof/intel/tgl.c     |   3 +
 sound/soc/sof/ipc3-topology.c |  12 +
 sound/soc/sof/pm.c            |   2 +-
 sound/soc/sof/sof-audio.c     |  36 ---
 sound/soc/sof/sof-audio.h     |   1 -
 11 files changed, 312 insertions(+), 230 deletions(-)

-- 
2.30.2

[PATCH 01/14] ASoC: SOF: remove incorrect clearing of prepared flag

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

When the system is suspended while a PCM is paused, it doesn't receive
the SUSPEND trigger. So, the SOF driver has to ensure that the PCM and
the widgets associated with the paused PCM are freed in the firmware
during suspend. This is handled in the
sof_tear_down_left_over_pipelines() call. But since the state of this
PCM is SUSPENDED, we end up clearing the prepared flag for the PCM
before freeing it. This results in IPC errors while freeing the widgets.
But because the widget use_counts are reset to 0 even though the IPC
fails, releasing the paused stream after resuming from suspend proceeds
normally.

Fix the IPC errors by removing the clearing of the prepared flag in
sof_set_hw_params_upon_resume(). In fact, we can remove the
sof_set_hw_params_upon_resume() and call
snd_sof_dsp_hw_params_upon_resume() directly. This will ensure that the
PCM is freed in the firmware before the IPC's for freeing the widgets
are sent.

BugLink: https://github.com/thesofproject/linux/issues/3543
Reviewed-by: Bard Liao <yung-chuan.liao@linux.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/pm.c        |  2 +-
 sound/soc/sof/sof-audio.c | 36 ------------------------------------
 sound/soc/sof/sof-audio.h |  1 -
 3 files changed, 1 insertion(+), 38 deletions(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index 44008dd075c21..fa3f5514c00fb 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -194,7 +194,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 
 	/* prepare for streams to be resumed properly upon resume */
 	if (!runtime_suspend) {
-		ret = sof_set_hw_params_upon_resume(sdev->dev);
+		ret = snd_sof_dsp_hw_params_upon_resume(sdev);
 		if (ret < 0) {
 			dev_err(sdev->dev,
 				"error: setting hw_params flag during suspend %d\n",
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index e2ec60887568b..7ecc84f9872bf 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -413,42 +413,6 @@ bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev)
 	return false;
 }
 
-int sof_set_hw_params_upon_resume(struct device *dev)
-{
-	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	struct snd_pcm_substream *substream;
-	struct snd_sof_pcm *spcm;
-	snd_pcm_state_t state;
-	int dir;
-
-	/*
-	 * SOF requires hw_params to be set-up internally upon resume.
-	 * So, set the flag to indicate this for those streams that
-	 * have been suspended.
-	 */
-	list_for_each_entry(spcm, &sdev->pcm_list, list) {
-		for_each_pcm_streams(dir) {
-			/*
-			 * do not reset hw_params upon resume for streams that
-			 * were kept running during suspend
-			 */
-			if (spcm->stream[dir].suspend_ignored)
-				continue;
-
-			substream = spcm->stream[dir].substream;
-			if (!substream || !substream->runtime)
-				continue;
-
-			state = substream->runtime->status->state;
-			if (state == SNDRV_PCM_STATE_SUSPENDED)
-				spcm->prepared[dir] = false;
-		}
-	}
-
-	/* set internal flag for BE */
-	return snd_sof_dsp_hw_params_upon_resume(sdev);
-}
-
 int sof_pcm_stream_free(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream,
 			struct snd_sof_pcm *spcm, int dir, bool free_widget_list)
 {
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index a0af7c421fd9b..f36c4f62bc994 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -434,7 +434,6 @@ static inline void snd_sof_compr_init_elapsed_work(struct work_struct *work) { }
 int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params);
 
 /* PM */
-int sof_set_hw_params_upon_resume(struct device *dev);
 bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev);
 bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev);
 
-- 
2.30.2

[PATCH 02/14] ASoC: SOF: Intel: Add IPC-specific dai ops for IPC3

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

The BE DAI driver ops involve operations that are IPC-specific. For ex:
for the HDA DAI, the trigger op involves sending the DAI_CONFIG IPC to
the DSP to stop the DMA for the stop/pause commands. This sequence is
different for IPC3 and IPC4. So, make the dai driver ops IPC-specific
and set the IPC3-specific ops during the ops_init() callback.

Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.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/apl.c     |  3 ++
 sound/soc/sof/intel/cnl.c     |  3 ++
 sound/soc/sof/intel/hda-dai.c | 53 +++++++++++++++++++++--------------
 sound/soc/sof/intel/hda.h     |  2 ++
 sound/soc/sof/intel/icl.c     |  3 ++
 sound/soc/sof/intel/tgl.c     |  3 ++
 6 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index cb499f3905cec..b7839fd04dfb7 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -43,6 +43,9 @@ int sof_apl_ops_init(struct snd_sof_dev *sdev)
 	/* ipc */
 	sof_apl_ops.send_msg	= hda_dsp_ipc_send_msg;
 
+	/* set DAI driver ops */
+	hda_set_dai_drv_ops(sdev, &sof_apl_ops);
+
 	/* debug */
 	sof_apl_ops.debug_map	= apl_dsp_debugfs;
 	sof_apl_ops.debug_map_count	= ARRAY_SIZE(apl_dsp_debugfs);
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index f5bac91c335ba..98c4e4f61e7c4 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -261,6 +261,9 @@ int sof_cnl_ops_init(struct snd_sof_dev *sdev)
 	/* ipc */
 	sof_cnl_ops.send_msg	= cnl_ipc_send_msg;
 
+	/* set DAI driver ops */
+	hda_set_dai_drv_ops(sdev, &sof_cnl_ops);
+
 	/* debug */
 	sof_cnl_ops.debug_map	= cnl_dsp_debugfs;
 	sof_cnl_ops.debug_map_count	= ARRAY_SIZE(cnl_dsp_debugfs);
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index f9cb9f1f0237b..8891077d8d8c3 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -276,8 +276,8 @@ static int hda_link_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
 	return ret;
 }
 
-static int hda_link_pcm_trigger(struct snd_pcm_substream *substream,
-				int cmd, struct snd_soc_dai *dai)
+static int ipc3_hda_link_pcm_trigger(struct snd_pcm_substream *substream,
+				     int cmd, struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *hext_stream =
 				snd_soc_dai_get_dma_data(dai, substream);
@@ -395,10 +395,10 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static const struct snd_soc_dai_ops hda_link_dai_ops = {
+static const struct snd_soc_dai_ops ipc3_hda_link_dai_ops = {
 	.hw_params = hda_link_hw_params,
 	.hw_free = hda_link_hw_free,
-	.trigger = hda_link_pcm_trigger,
+	.trigger = ipc3_hda_link_pcm_trigger,
 	.prepare = hda_link_pcm_prepare,
 };
 
@@ -478,8 +478,8 @@ static int ssp_dai_prepare(struct snd_pcm_substream *substream,
 	return ssp_dai_setup(substream, dai, true);
 }
 
-static int ssp_dai_trigger(struct snd_pcm_substream *substream,
-			   int cmd, struct snd_soc_dai *dai)
+static int ipc3_ssp_dai_trigger(struct snd_pcm_substream *substream,
+				int cmd, struct snd_soc_dai *dai)
 {
 	if (cmd != SNDRV_PCM_TRIGGER_SUSPEND)
 		return 0;
@@ -507,15 +507,39 @@ static void ssp_dai_shutdown(struct snd_pcm_substream *substream,
 	kfree(dma_data);
 }
 
-static const struct snd_soc_dai_ops ssp_dai_ops = {
+static const struct snd_soc_dai_ops ipc3_ssp_dai_ops = {
 	.startup = ssp_dai_startup,
 	.hw_params = ssp_dai_hw_params,
 	.prepare = ssp_dai_prepare,
-	.trigger = ssp_dai_trigger,
+	.trigger = ipc3_ssp_dai_trigger,
 	.hw_free = ssp_dai_hw_free,
 	.shutdown = ssp_dai_shutdown,
 };
 
+void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops)
+{
+	int i;
+
+	switch (sdev->pdata->ipc_type) {
+	case SOF_IPC:
+		for (i = 0; i < ops->num_drv; i++) {
+			if (strstr(ops->drv[i].name, "SSP")) {
+				ops->drv[i].ops = &ipc3_ssp_dai_ops;
+				continue;
+			}
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
+			if (strstr(ops->drv[i].name, "iDisp") ||
+			    strstr(ops->drv[i].name, "Analog") ||
+			    strstr(ops->drv[i].name, "Digital"))
+				ops->drv[i].ops = &ipc3_hda_link_dai_ops;
+#endif
+		}
+		break;
+	default:
+		break;
+	}
+}
+
 /*
  * common dai driver for skl+ platforms.
  * some products who use this DAI array only physically have a subset of
@@ -524,7 +548,6 @@ static const struct snd_soc_dai_ops ssp_dai_ops = {
 struct snd_soc_dai_driver skl_dai[] = {
 {
 	.name = "SSP0 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -536,7 +559,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "SSP1 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -548,7 +570,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "SSP2 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -560,7 +581,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "SSP3 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -572,7 +592,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "SSP4 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -584,7 +603,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "SSP5 Pin",
-	.ops = &ssp_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -611,7 +629,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
 {
 	.name = "iDisp1 Pin",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -619,7 +636,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "iDisp2 Pin",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -627,7 +643,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "iDisp3 Pin",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -635,7 +650,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "iDisp4 Pin",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 8,
@@ -643,7 +657,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "Analog CPU DAI",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 16,
@@ -655,7 +668,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "Digital CPU DAI",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 16,
@@ -667,7 +679,6 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 {
 	.name = "Alt Analog CPU DAI",
-	.ops = &hda_link_dai_ops,
 	.playback = {
 		.channels_min = 1,
 		.channels_max = 16,
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 6e05c77594809..f520d1cf70c90 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -761,4 +761,6 @@ int hda_ctrl_dai_widget_free(struct snd_soc_dapm_widget *w, unsigned int quirk_f
 
 extern int sof_hda_position_quirk;
 
+void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops);
+
 #endif
diff --git a/sound/soc/sof/intel/icl.c b/sound/soc/sof/intel/icl.c
index f845064c3589a..f19517dffd624 100644
--- a/sound/soc/sof/intel/icl.c
+++ b/sound/soc/sof/intel/icl.c
@@ -127,6 +127,9 @@ int sof_icl_ops_init(struct snd_sof_dev *sdev)
 	/* dsp core get/put */
 	sof_icl_ops.core_get = hda_dsp_core_get;
 
+	/* set DAI driver ops */
+	hda_set_dai_drv_ops(sdev, &sof_icl_ops);
+
 	return 0;
 };
 EXPORT_SYMBOL_NS(sof_icl_ops_init, SND_SOC_SOF_INTEL_HDA_COMMON);
diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c
index 816571305f247..ed76f736afb46 100644
--- a/sound/soc/sof/intel/tgl.c
+++ b/sound/soc/sof/intel/tgl.c
@@ -76,6 +76,9 @@ int sof_tgl_ops_init(struct snd_sof_dev *sdev)
 	/* ipc */
 	sof_tgl_ops.send_msg	= cnl_ipc_send_msg;
 
+	/* set DAI driver ops */
+	hda_set_dai_drv_ops(sdev, &sof_tgl_ops);
+
 	/* debug */
 	sof_tgl_ops.debug_map	= tgl_dsp_debugfs;
 	sof_tgl_ops.debug_map_count	= ARRAY_SIZE(tgl_dsp_debugfs);
-- 
2.30.2

[PATCH 03/14] ASOC: SOF: Intel: hda-dai: consistent naming for HDA DAI and HDA link DMA

[Pierre-Louis Bossart]

The Intel documentation refers to the concepts of 'HDAudio host
DMA' (system memory <--> DSP) and 'HDaudio link DMA' (DSP <-->
peripherals). We currently use the prefix 'hda_link' to describe DAI
operations, which can be confused for dailink operations.

Since the topology tokens refer unambiguously to the 'HDA' dai, let's
drop the link prefix for dai-related ops/callbacks. Conversely let's
use 'hda_link_dma' for routines related to the DMA management. In a
follow-up patch we will introduce the 'hda_dai_link' prefix for dailink
ops/callbacks.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 52 +++++++++++++++++------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 8891077d8d8c3..65f3ff5196244 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -50,8 +50,8 @@ static bool hda_check_fes(struct snd_soc_pcm_runtime *rtd,
 }
 
 static struct hdac_ext_stream *
-	hda_link_stream_assign(struct hdac_bus *bus,
-			       struct snd_pcm_substream *substream)
+hda_link_stream_assign(struct hdac_bus *bus,
+		       struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct sof_intel_hda_stream *hda_stream;
@@ -162,9 +162,9 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream,
 	return 0;
 }
 
-static int hda_link_dai_widget_update(struct sof_intel_hda_stream *hda_stream,
-				      struct snd_soc_dapm_widget *w,
-				      int channel, bool widget_setup)
+static int hda_dai_widget_update(struct sof_intel_hda_stream *hda_stream,
+				 struct snd_soc_dapm_widget *w,
+				 int channel, bool widget_setup)
 {
 	struct snd_sof_dai_config_data data;
 
@@ -177,9 +177,9 @@ static int hda_link_dai_widget_update(struct sof_intel_hda_stream *hda_stream,
 	return hda_ctrl_dai_widget_free(w, SOF_DAI_CONFIG_FLAGS_NONE, &data);
 }
 
-static int hda_link_hw_params(struct snd_pcm_substream *substream,
-			      struct snd_pcm_hw_params *params,
-			      struct snd_soc_dai *dai)
+static int hda_dai_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
 {
 	struct hdac_stream *hstream = substream->runtime->private_data;
 	struct hdac_bus *bus = hstream->bus;
@@ -213,7 +213,7 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,
 		w = dai->capture_widget;
 
 	/* set up the DAI widget and send the DAI_CONFIG with the new tag */
-	ret = hda_link_dai_widget_update(hda_stream, w, stream_tag - 1, true);
+	ret = hda_dai_widget_update(hda_stream, w, stream_tag - 1, true);
 	if (ret < 0)
 		return ret;
 
@@ -239,8 +239,8 @@ static int hda_link_hw_params(struct snd_pcm_substream *substream,
 	return hda_link_dma_params(hext_stream, &p_params);
 }
 
-static int hda_link_pcm_prepare(struct snd_pcm_substream *substream,
-				struct snd_soc_dai *dai)
+static int hda_dai_prepare(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *hext_stream =
 				snd_soc_dai_get_dma_data(dai, substream);
@@ -254,11 +254,11 @@ static int hda_link_pcm_prepare(struct snd_pcm_substream *substream,
 
 	dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream);
 
-	return hda_link_hw_params(substream, &rtd->dpcm[stream].hw_params,
+	return hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params,
 				  dai);
 }
 
-static int hda_link_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
+static int hda_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
 {
 	struct snd_sof_widget *swidget = w->dobj.private;
 	struct snd_soc_component *component = swidget->scomp;
@@ -276,8 +276,8 @@ static int hda_link_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
 	return ret;
 }
 
-static int ipc3_hda_link_pcm_trigger(struct snd_pcm_substream *substream,
-				     int cmd, struct snd_soc_dai *dai)
+static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
+				int cmd, struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *hext_stream =
 				snd_soc_dai_get_dma_data(dai, substream);
@@ -316,7 +316,7 @@ static int ipc3_hda_link_pcm_trigger(struct snd_pcm_substream *substream,
 		/*
 		 * free DAI widget during stop/suspend to keep widget use_count's balanced.
 		 */
-		ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
+		ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
 		if (ret < 0)
 			return ret;
 
@@ -330,7 +330,7 @@ static int ipc3_hda_link_pcm_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		snd_hdac_ext_link_stream_clear(hext_stream);
 
-		ret = hda_link_dai_config_pause_push_ipc(w);
+		ret = hda_dai_config_pause_push_ipc(w);
 		if (ret < 0)
 			return ret;
 		break;
@@ -340,8 +340,8 @@ static int ipc3_hda_link_pcm_trigger(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int hda_link_hw_free(struct snd_pcm_substream *substream,
-			    struct snd_soc_dai *dai)
+static int hda_dai_hw_free(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
 {
 	unsigned int stream_tag;
 	struct sof_intel_hda_stream *hda_stream;
@@ -372,7 +372,7 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream,
 		w = dai->capture_widget;
 
 	/* free the link DMA channel in the FW and the DAI widget */
-	ret = hda_link_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
+	ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
 	if (ret < 0)
 		return ret;
 
@@ -395,11 +395,11 @@ static int hda_link_hw_free(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static const struct snd_soc_dai_ops ipc3_hda_link_dai_ops = {
-	.hw_params = hda_link_hw_params,
-	.hw_free = hda_link_hw_free,
-	.trigger = ipc3_hda_link_pcm_trigger,
-	.prepare = hda_link_pcm_prepare,
+static const struct snd_soc_dai_ops ipc3_hda_dai_ops = {
+	.hw_params = hda_dai_hw_params,
+	.hw_free = hda_dai_hw_free,
+	.trigger = ipc3_hda_dai_trigger,
+	.prepare = hda_dai_prepare,
 };
 
 #endif
@@ -531,7 +531,7 @@ void hda_set_dai_drv_ops(struct snd_sof_dev *sdev, struct snd_sof_dsp_ops *ops)
 			if (strstr(ops->drv[i].name, "iDisp") ||
 			    strstr(ops->drv[i].name, "Analog") ||
 			    strstr(ops->drv[i].name, "Digital"))
-				ops->drv[i].ops = &ipc3_hda_link_dai_ops;
+				ops->drv[i].ops = &ipc3_hda_dai_ops;
 #endif
 		}
 		break;
-- 
2.30.2

[PATCH 04/14] ASoC: SOF: Intel: hda-dai: simplify hda_dai_widget_update() prototype

[Pierre-Louis Bossart]

the argument "struct sof_intel_hda_stream *hda_stream" is not used, remove.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 65f3ff5196244..3113f61ae7370 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -162,8 +162,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream,
 	return 0;
 }
 
-static int hda_dai_widget_update(struct sof_intel_hda_stream *hda_stream,
-				 struct snd_soc_dapm_widget *w,
+static int hda_dai_widget_update(struct snd_soc_dapm_widget *w,
 				 int channel, bool widget_setup)
 {
 	struct snd_sof_dai_config_data data;
@@ -186,7 +185,6 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 	struct hdac_ext_stream *hext_stream;
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	struct sof_intel_hda_stream *hda_stream;
 	struct hda_pipe_params p_params = {0};
 	struct snd_soc_dapm_widget *w;
 	struct hdac_ext_link *link;
@@ -205,15 +203,13 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 
 	stream_tag = hdac_stream(hext_stream)->stream_tag;
 
-	hda_stream = hstream_to_sof_hda_stream(hext_stream);
-
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		w = dai->playback_widget;
 	else
 		w = dai->capture_widget;
 
 	/* set up the DAI widget and send the DAI_CONFIG with the new tag */
-	ret = hda_dai_widget_update(hda_stream, w, stream_tag - 1, true);
+	ret = hda_dai_widget_update(w, stream_tag - 1, true);
 	if (ret < 0)
 		return ret;
 
@@ -281,7 +277,6 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 {
 	struct hdac_ext_stream *hext_stream =
 				snd_soc_dai_get_dma_data(dai, substream);
-	struct sof_intel_hda_stream *hda_stream;
 	struct snd_soc_pcm_runtime *rtd;
 	struct snd_soc_dapm_widget *w;
 	struct hdac_ext_link *link;
@@ -298,8 +293,6 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 	if (!link)
 		return -EINVAL;
 
-	hda_stream = hstream_to_sof_hda_stream(hext_stream);
-
 	dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd);
 
 	w = snd_soc_dai_get_widget(dai, substream->stream);
@@ -316,7 +309,7 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 		/*
 		 * free DAI widget during stop/suspend to keep widget use_count's balanced.
 		 */
-		ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
+		ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
 		if (ret < 0)
 			return ret;
 
@@ -372,7 +365,7 @@ static int hda_dai_hw_free(struct snd_pcm_substream *substream,
 		w = dai->capture_widget;
 
 	/* free the link DMA channel in the FW and the DAI widget */
-	ret = hda_dai_widget_update(hda_stream, w, DMA_CHAN_INVALID, false);
+	ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
 	if (ret < 0)
 		return ret;
 
-- 
2.30.2

[PATCH 05/14] ASoC: SOF: Intel: hda-dai: use snd_soc_dai_get_widget() helper

[Pierre-Louis Bossart]

Use helper instead of open-coding the same thing multiple times.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 3113f61ae7370..245009809894b 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -203,10 +203,7 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 
 	stream_tag = hdac_stream(hext_stream)->stream_tag;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		w = dai->playback_widget;
-	else
-		w = dai->capture_widget;
+	w = snd_soc_dai_get_widget(dai, substream->stream);
 
 	/* set up the DAI widget and send the DAI_CONFIG with the new tag */
 	ret = hda_dai_widget_update(w, stream_tag - 1, true);
@@ -359,10 +356,7 @@ static int hda_dai_hw_free(struct snd_pcm_substream *substream,
 
 	hda_stream = hstream_to_sof_hda_stream(hext_stream);
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		w = dai->playback_widget;
-	else
-		w = dai->capture_widget;
+	w = snd_soc_dai_get_widget(dai, substream->stream);
 
 	/* free the link DMA channel in the FW and the DAI widget */
 	ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
@@ -407,10 +401,7 @@ static int ssp_dai_setup_or_free(struct snd_pcm_substream *substream, struct snd
 {
 	struct snd_soc_dapm_widget *w;
 
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-		w = dai->playback_widget;
-	else
-		w = dai->capture_widget;
+	w = snd_soc_dai_get_widget(dai, substream->stream);
 
 	if (setup)
 		return hda_ctrl_dai_widget_setup(w, SOF_DAI_CONFIG_FLAGS_NONE, NULL);
-- 
2.30.2

[PATCH 06/14] ASoC: SOF: Intel: hda-dai: split link DMA and dai operations

[Pierre-Louis Bossart]

The link DMA state management is handled completely on the host side,
while the DAI operations require an IPC. Split the first part in
dedicated helpers.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 220 +++++++++++++++++++++-------------
 1 file changed, 138 insertions(+), 82 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 245009809894b..d5ca5b1fefe67 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -176,40 +176,28 @@ static int hda_dai_widget_update(struct snd_soc_dapm_widget *w,
 	return hda_ctrl_dai_widget_free(w, SOF_DAI_CONFIG_FLAGS_NONE, &data);
 }
 
-static int hda_dai_hw_params(struct snd_pcm_substream *substream,
-			     struct snd_pcm_hw_params *params,
-			     struct snd_soc_dai *dai)
+static int hda_link_dma_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *params)
 {
 	struct hdac_stream *hstream = substream->runtime->private_data;
-	struct hdac_bus *bus = hstream->bus;
 	struct hdac_ext_stream *hext_stream;
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
 	struct hda_pipe_params p_params = {0};
-	struct snd_soc_dapm_widget *w;
+	struct hdac_bus *bus = hstream->bus;
 	struct hdac_ext_link *link;
-	int stream_tag;
-	int ret;
 
 	/* get stored dma data if resuming from system suspend */
-	hext_stream = snd_soc_dai_get_dma_data(dai, substream);
+	hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
 	if (!hext_stream) {
 		hext_stream = hda_link_stream_assign(bus, substream);
 		if (!hext_stream)
 			return -EBUSY;
 
-		snd_soc_dai_set_dma_data(dai, substream, (void *)hext_stream);
+		snd_soc_dai_set_dma_data(cpu_dai, substream, (void *)hext_stream);
 	}
 
-	stream_tag = hdac_stream(hext_stream)->stream_tag;
-
-	w = snd_soc_dai_get_widget(dai, substream->stream);
-
-	/* set up the DAI widget and send the DAI_CONFIG with the new tag */
-	ret = hda_dai_widget_update(w, stream_tag - 1, true);
-	if (ret < 0)
-		return ret;
-
 	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
 	if (!link)
 		return -EINVAL;
@@ -232,23 +220,45 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 	return hda_link_dma_params(hext_stream, &p_params);
 }
 
-static int hda_dai_prepare(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static int hda_dai_hw_params_update(struct snd_pcm_substream *substream,
+				    struct snd_pcm_hw_params *params,
+				    struct snd_soc_dai *dai)
 {
-	struct hdac_ext_stream *hext_stream =
-				snd_soc_dai_get_dma_data(dai, substream);
-	struct snd_sof_dev *sdev =
-				snd_soc_component_get_drvdata(dai->component);
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	int stream = substream->stream;
+	struct hdac_ext_stream *hext_stream;
+	struct snd_soc_dapm_widget *w;
+	int stream_tag;
 
-	if (hext_stream->link_prepared)
-		return 0;
+	hext_stream = snd_soc_dai_get_dma_data(dai, substream);
+	if (!hext_stream)
+		return -EINVAL;
+
+	stream_tag = hdac_stream(hext_stream)->stream_tag;
 
-	dev_dbg(sdev->dev, "hda: prepare stream dir %d\n", substream->stream);
+	w = snd_soc_dai_get_widget(dai, substream->stream);
 
-	return hda_dai_hw_params(substream, &rtd->dpcm[stream].hw_params,
-				  dai);
+	/* set up the DAI widget and send the DAI_CONFIG with the new tag */
+	return hda_dai_widget_update(w, stream_tag - 1, true);
+}
+
+static int hda_dai_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{
+	int ret;
+
+	ret = hda_link_dma_hw_params(substream, params);
+	if (ret < 0)
+		return ret;
+
+	return hda_dai_hw_params_update(substream, params, dai);
+}
+
+static int hda_link_dma_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	int stream = substream->stream;
+
+	return hda_link_dma_hw_params(substream, &rtd->dpcm[stream].hw_params);
 }
 
 static int hda_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
@@ -269,31 +279,44 @@ static int hda_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
 	return ret;
 }
 
-static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
-				int cmd, struct snd_soc_dai *dai)
+static int ipc3_hda_dai_prepare(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
 {
 	struct hdac_ext_stream *hext_stream =
 				snd_soc_dai_get_dma_data(dai, substream);
-	struct snd_soc_pcm_runtime *rtd;
-	struct snd_soc_dapm_widget *w;
-	struct hdac_ext_link *link;
-	struct hdac_stream *hstream;
-	struct hdac_bus *bus;
-	int stream_tag;
+	struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(dai->component);
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	int stream = substream->stream;
 	int ret;
 
-	hstream = substream->runtime->private_data;
-	bus = hstream->bus;
-	rtd = asoc_substream_to_rtd(substream);
+	if (hext_stream->link_prepared)
+		return 0;
 
-	link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name);
-	if (!link)
-		return -EINVAL;
+	dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream);
 
-	dev_dbg(dai->dev, "In %s cmd=%d\n", __func__, cmd);
+	ret = hda_link_dma_prepare(substream);
+	if (ret < 0)
+		return ret;
 
-	w = snd_soc_dai_get_widget(dai, substream->stream);
+	return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai);
+}
+
+static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct hdac_stream *hstream = substream->runtime->private_data;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
+	struct hdac_ext_link *link;
+	struct hdac_bus *bus = hstream->bus;
+	int stream_tag;
+
+	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
+	if (!link)
+		return -EINVAL;
 
+	dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
@@ -303,13 +326,6 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_STOP:
 		snd_hdac_ext_link_stream_clear(hext_stream);
 
-		/*
-		 * free DAI widget during stop/suspend to keep widget use_count's balanced.
-		 */
-		ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
-		if (ret < 0)
-			return ret;
-
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 			stream_tag = hdac_stream(hext_stream)->stream_tag;
 			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
@@ -320,50 +336,69 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		snd_hdac_ext_link_stream_clear(hext_stream);
 
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
+				int cmd, struct snd_soc_dai *dai)
+{
+	struct snd_soc_dapm_widget *w;
+	int ret;
+
+	ret = hda_link_dma_trigger(substream, cmd);
+	if (ret < 0)
+		return ret;
+
+	w = snd_soc_dai_get_widget(dai, substream->stream);
+
+	dev_dbg(dai->dev, "%s: cmd=%d\n", __func__, cmd);
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		/*
+		 * free DAI widget during stop/suspend to keep widget use_count's balanced.
+		 */
+		ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
+		if (ret < 0)
+			return ret;
+
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		ret = hda_dai_config_pause_push_ipc(w);
 		if (ret < 0)
 			return ret;
 		break;
+
 	default:
-		return -EINVAL;
+		break;
 	}
 	return 0;
 }
 
-static int hda_dai_hw_free(struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static int hda_link_dma_hw_free(struct snd_pcm_substream *substream)
 {
-	unsigned int stream_tag;
+	struct hdac_stream *hstream = substream->runtime->private_data;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
 	struct sof_intel_hda_stream *hda_stream;
-	struct hdac_bus *bus;
-	struct hdac_ext_link *link;
-	struct hdac_stream *hstream;
-	struct snd_soc_pcm_runtime *rtd;
+	struct hdac_bus *bus = hstream->bus;
 	struct hdac_ext_stream *hext_stream;
-	struct snd_soc_dapm_widget *w;
-	int ret;
-
-	hstream = substream->runtime->private_data;
-	bus = hstream->bus;
-	rtd = asoc_substream_to_rtd(substream);
-	hext_stream = snd_soc_dai_get_dma_data(dai, substream);
+	struct hdac_ext_link *link;
+	int stream_tag;
 
+	hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
 	if (!hext_stream) {
-		dev_dbg(dai->dev,
+		dev_dbg(cpu_dai->dev,
 			"%s: hext_stream is not assigned\n", __func__);
 		return -EINVAL;
 	}
 
-	hda_stream = hstream_to_sof_hda_stream(hext_stream);
-
-	w = snd_soc_dai_get_widget(dai, substream->stream);
-
-	/* free the link DMA channel in the FW and the DAI widget */
-	ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
-	if (ret < 0)
-		return ret;
-
-	link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name);
+	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
 	if (!link)
 		return -EINVAL;
 
@@ -372,21 +407,42 @@ static int hda_dai_hw_free(struct snd_pcm_substream *substream,
 		snd_hdac_ext_link_clear_stream_id(link, stream_tag);
 	}
 
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
+	snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
 	snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
 	hext_stream->link_prepared = 0;
 
 	/* free the host DMA channel reserved by hostless streams */
+	hda_stream = hstream_to_sof_hda_stream(hext_stream);
 	hda_stream->host_reserved = 0;
 
 	return 0;
 }
 
+static int hda_dai_hw_free(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_dapm_widget *w;
+	int ret;
+
+	ret = hda_link_dma_hw_free(substream);
+	if (ret < 0)
+		return ret;
+
+	w = snd_soc_dai_get_widget(dai, substream->stream);
+
+	/* free the link DMA channel in the FW and the DAI widget */
+	ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static const struct snd_soc_dai_ops ipc3_hda_dai_ops = {
 	.hw_params = hda_dai_hw_params,
 	.hw_free = hda_dai_hw_free,
 	.trigger = ipc3_hda_dai_trigger,
-	.prepare = hda_dai_prepare,
+	.prepare = ipc3_hda_dai_prepare,
 };
 
 #endif
-- 
2.30.2

[PATCH 07/14] ASoC: SOF: Intel: hda-dai: regroup dai and link DMA operations

[Pierre-Louis Bossart]

Just code move with no functionality change, to clearly separate out
the 'dai' operation from the link DMA ones.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 203 +++++++++++++++++-----------------
 1 file changed, 102 insertions(+), 101 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index d5ca5b1fefe67..20eb4097ce753 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -162,20 +162,6 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream,
 	return 0;
 }
 
-static int hda_dai_widget_update(struct snd_soc_dapm_widget *w,
-				 int channel, bool widget_setup)
-{
-	struct snd_sof_dai_config_data data;
-
-	data.dai_data = channel;
-
-	/* set up/free DAI widget and send DAI_CONFIG IPC */
-	if (widget_setup)
-		return hda_ctrl_dai_widget_setup(w, SOF_DAI_CONFIG_FLAGS_2_STEP_STOP, &data);
-
-	return hda_ctrl_dai_widget_free(w, SOF_DAI_CONFIG_FLAGS_NONE, &data);
-}
-
 static int hda_link_dma_hw_params(struct snd_pcm_substream *substream,
 				  struct snd_pcm_hw_params *params)
 {
@@ -220,6 +206,108 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream,
 	return hda_link_dma_params(hext_stream, &p_params);
 }
 
+static int hda_link_dma_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	int stream = substream->stream;
+
+	return hda_link_dma_hw_params(substream, &rtd->dpcm[stream].hw_params);
+}
+
+static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct hdac_stream *hstream = substream->runtime->private_data;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
+	struct hdac_ext_link *link;
+	struct hdac_bus *bus = hstream->bus;
+	int stream_tag;
+
+	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
+	if (!link)
+		return -EINVAL;
+
+	dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd);
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		snd_hdac_ext_link_stream_start(hext_stream);
+		break;
+	case SNDRV_PCM_TRIGGER_SUSPEND:
+	case SNDRV_PCM_TRIGGER_STOP:
+		snd_hdac_ext_link_stream_clear(hext_stream);
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			stream_tag = hdac_stream(hext_stream)->stream_tag;
+			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+		}
+
+		hext_stream->link_prepared = 0;
+		break;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		snd_hdac_ext_link_stream_clear(hext_stream);
+
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int hda_link_dma_hw_free(struct snd_pcm_substream *substream)
+{
+	struct hdac_stream *hstream = substream->runtime->private_data;
+	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	struct sof_intel_hda_stream *hda_stream;
+	struct hdac_bus *bus = hstream->bus;
+	struct hdac_ext_stream *hext_stream;
+	struct hdac_ext_link *link;
+	int stream_tag;
+
+	hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
+	if (!hext_stream) {
+		dev_dbg(cpu_dai->dev, "%s: hext_stream is not assigned\n", __func__);
+		return -EINVAL;
+	}
+
+	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
+	if (!link)
+		return -EINVAL;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		stream_tag = hdac_stream(hext_stream)->stream_tag;
+		snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+	}
+
+	snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
+	snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
+	hext_stream->link_prepared = 0;
+
+	/* free the host DMA channel reserved by hostless streams */
+	hda_stream = hstream_to_sof_hda_stream(hext_stream);
+	hda_stream->host_reserved = 0;
+
+	return 0;
+}
+
+static int hda_dai_widget_update(struct snd_soc_dapm_widget *w,
+				 int channel, bool widget_setup)
+{
+	struct snd_sof_dai_config_data data;
+
+	data.dai_data = channel;
+
+	/* set up/free DAI widget and send DAI_CONFIG IPC */
+	if (widget_setup)
+		return hda_ctrl_dai_widget_setup(w, SOF_DAI_CONFIG_FLAGS_2_STEP_STOP, &data);
+
+	return hda_ctrl_dai_widget_free(w, SOF_DAI_CONFIG_FLAGS_NONE, &data);
+}
+
 static int hda_dai_hw_params_update(struct snd_pcm_substream *substream,
 				    struct snd_pcm_hw_params *params,
 				    struct snd_soc_dai *dai)
@@ -253,13 +341,6 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 	return hda_dai_hw_params_update(substream, params, dai);
 }
 
-static int hda_link_dma_prepare(struct snd_pcm_substream *substream)
-{
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	int stream = substream->stream;
-
-	return hda_link_dma_hw_params(substream, &rtd->dpcm[stream].hw_params);
-}
 
 static int hda_dai_config_pause_push_ipc(struct snd_soc_dapm_widget *w)
 {
@@ -301,47 +382,6 @@ static int ipc3_hda_dai_prepare(struct snd_pcm_substream *substream,
 	return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai);
 }
 
-static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	struct hdac_stream *hstream = substream->runtime->private_data;
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
-	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	struct hdac_ext_link *link;
-	struct hdac_bus *bus = hstream->bus;
-	int stream_tag;
-
-	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
-	if (!link)
-		return -EINVAL;
-
-	dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd);
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		snd_hdac_ext_link_stream_start(hext_stream);
-		break;
-	case SNDRV_PCM_TRIGGER_SUSPEND:
-	case SNDRV_PCM_TRIGGER_STOP:
-		snd_hdac_ext_link_stream_clear(hext_stream);
-
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			stream_tag = hdac_stream(hext_stream)->stream_tag;
-			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-		}
-
-		hext_stream->link_prepared = 0;
-		break;
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		snd_hdac_ext_link_stream_clear(hext_stream);
-
-		break;
-	default:
-		return -EINVAL;
-	}
-	return 0;
-}
 
 static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 				int cmd, struct snd_soc_dai *dai)
@@ -379,45 +419,6 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int hda_link_dma_hw_free(struct snd_pcm_substream *substream)
-{
-	struct hdac_stream *hstream = substream->runtime->private_data;
-	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
-	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	struct sof_intel_hda_stream *hda_stream;
-	struct hdac_bus *bus = hstream->bus;
-	struct hdac_ext_stream *hext_stream;
-	struct hdac_ext_link *link;
-	int stream_tag;
-
-	hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	if (!hext_stream) {
-		dev_dbg(cpu_dai->dev,
-			"%s: hext_stream is not assigned\n", __func__);
-		return -EINVAL;
-	}
-
-	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
-	if (!link)
-		return -EINVAL;
-
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		stream_tag = hdac_stream(hext_stream)->stream_tag;
-		snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-	}
-
-	snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
-	snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
-	hext_stream->link_prepared = 0;
-
-	/* free the host DMA channel reserved by hostless streams */
-	hda_stream = hstream_to_sof_hda_stream(hext_stream);
-	hda_stream->host_reserved = 0;
-
-	return 0;
-}
-
 static int hda_dai_hw_free(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
-- 
2.30.2

[PATCH 08/14] ASoC: SOF: sof-audio: flag errors on pipeline teardown

[Pierre-Louis Bossart]

Before suspending, walk through all the widgets to make sure all
refcounts are zero. If not, the resume will not work and random errors
will be reported. Adding this paranoia check will help identify leaks
and broken sequences.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/ipc3-topology.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index 572bcbfdb3566..3d00d371fbf31 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2247,6 +2247,18 @@ static int sof_ipc3_tear_down_all_pipelines(struct snd_sof_dev *sdev, bool verif
 	list_for_each_entry(sroute, &sdev->route_list, list)
 		sroute->setup = false;
 
+	/*
+	 * before suspending, make sure the refcounts are all zeroed out. There's no way
+	 * to recover at this point but this will help root cause bad sequences leading to
+	 * more issues on resume
+	 */
+	list_for_each_entry(swidget, &sdev->widget_list, list) {
+		if (swidget->use_count != 0) {
+			dev_err(sdev->dev, "%s: widget %s is still in use: count %d\n",
+				__func__, swidget->widget->name, swidget->use_count);
+		}
+	}
+
 	return 0;
 }
 
-- 
2.30.2

[PATCH 09/14] ASOC: SOF: Intel: hda-dai: add hda_dai_hw_free_ipc() helper

[Pierre-Louis Bossart]

We do the same thing from different places, let's use a helper.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 20eb4097ce753..0521cb755a8af 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -382,6 +382,16 @@ static int ipc3_hda_dai_prepare(struct snd_pcm_substream *substream,
 	return hda_dai_hw_params_update(substream, &rtd->dpcm[stream].hw_params, dai);
 }
 
+static int hda_dai_hw_free_ipc(int stream, /* direction */
+			       struct snd_soc_dai *dai)
+{
+	struct snd_soc_dapm_widget *w;
+
+	w = snd_soc_dai_get_widget(dai, stream);
+
+	/* free the link DMA channel in the FW and the DAI widget */
+	return hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
+}
 
 static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 				int cmd, struct snd_soc_dai *dai)
@@ -402,7 +412,7 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 		/*
 		 * free DAI widget during stop/suspend to keep widget use_count's balanced.
 		 */
-		ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
+		ret = hda_dai_hw_free_ipc(substream->stream, dai);
 		if (ret < 0)
 			return ret;
 
@@ -422,21 +432,13 @@ static int ipc3_hda_dai_trigger(struct snd_pcm_substream *substream,
 static int hda_dai_hw_free(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
-	struct snd_soc_dapm_widget *w;
 	int ret;
 
 	ret = hda_link_dma_hw_free(substream);
 	if (ret < 0)
 		return ret;
 
-	w = snd_soc_dai_get_widget(dai, substream->stream);
-
-	/* free the link DMA channel in the FW and the DAI widget */
-	ret = hda_dai_widget_update(w, DMA_CHAN_INVALID, false);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return hda_dai_hw_free_ipc(substream->stream, dai);
 }
 
 static const struct snd_soc_dai_ops ipc3_hda_dai_ops = {
-- 
2.30.2

[PATCH 10/14] ASoC: SOF: Intel: hda-dai: move code to deal with hda dai/dailink suspend

[Pierre-Louis Bossart]

The location of the code was not optimal and prevents us from using
helpers, let's move it to hda-dai.c.

No functionality change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 58 +++++++++++++++++++++++++++++++++++
 sound/soc/sof/intel/hda-dsp.c | 42 ++++---------------------
 sound/soc/sof/intel/hda.h     |  1 +
 3 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 0521cb755a8af..c1ff7145745bc 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -448,6 +448,45 @@ static const struct snd_soc_dai_ops ipc3_hda_dai_ops = {
 	.prepare = ipc3_hda_dai_prepare,
 };
 
+static int hda_dai_suspend(struct hdac_bus *bus)
+{
+	struct snd_soc_pcm_runtime *rtd;
+	struct hdac_ext_stream *hext_stream;
+	struct hdac_ext_link *link;
+	struct hdac_stream *s;
+	const char *name;
+	int stream_tag;
+
+	/* set internal flag for BE */
+	list_for_each_entry(s, &bus->stream_list, list) {
+		hext_stream = stream_to_hdac_ext_stream(s);
+
+		/*
+		 * clear stream. This should already be taken care for running
+		 * streams when the SUSPEND trigger is called. But paused
+		 * streams do not get suspended, so this needs to be done
+		 * explicitly during suspend.
+		 */
+		if (hext_stream->link_substream) {
+			rtd = asoc_substream_to_rtd(hext_stream->link_substream);
+			name = asoc_rtd_to_codec(rtd, 0)->component->name;
+			link = snd_hdac_ext_bus_get_link(bus, name);
+			if (!link)
+				return -EINVAL;
+
+			hext_stream->link_prepared = 0;
+
+			if (hdac_stream(hext_stream)->direction ==
+				SNDRV_PCM_STREAM_CAPTURE)
+				continue;
+
+			stream_tag = hdac_stream(hext_stream)->stream_tag;
+			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+		}
+	}
+
+	return 0;
+}
 #endif
 
 /* only one flag used so far to harden hw_params/hw_free/trigger/prepare */
@@ -733,3 +772,22 @@ struct snd_soc_dai_driver skl_dai[] = {
 },
 #endif
 };
+
+int hda_dsp_dais_suspend(struct snd_sof_dev *sdev)
+{
+	/*
+	 * In the corner case where a SUSPEND happens during a PAUSE, the ALSA core
+	 * does not throw the TRIGGER_SUSPEND. This leaves the DAIs in an unbalanced state.
+	 * Since the component suspend is called last, we can trap this corner case
+	 * and force the DAIs to release their resources.
+	 */
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
+	int ret;
+
+	ret = hda_dai_suspend(sof_to_bus(sdev));
+	if (ret < 0)
+		return ret;
+#endif
+
+	return 0;
+}
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index ad11df345be75..c068a3f2f6df3 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -894,44 +894,14 @@ int hda_dsp_shutdown(struct snd_sof_dev *sdev)
 
 int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
 {
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
-	struct hdac_bus *bus = sof_to_bus(sdev);
-	struct snd_soc_pcm_runtime *rtd;
-	struct hdac_ext_stream *hext_stream;
-	struct hdac_ext_link *link;
-	struct hdac_stream *s;
-	const char *name;
-	int stream_tag;
-
-	/* set internal flag for BE */
-	list_for_each_entry(s, &bus->stream_list, list) {
-		hext_stream = stream_to_hdac_ext_stream(s);
-
-		/*
-		 * clear stream. This should already be taken care for running
-		 * streams when the SUSPEND trigger is called. But paused
-		 * streams do not get suspended, so this needs to be done
-		 * explicitly during suspend.
-		 */
-		if (hext_stream->link_substream) {
-			rtd = asoc_substream_to_rtd(hext_stream->link_substream);
-			name = asoc_rtd_to_codec(rtd, 0)->component->name;
-			link = snd_hdac_ext_bus_get_link(bus, name);
-			if (!link)
-				return -EINVAL;
-
-			hext_stream->link_prepared = 0;
+	int ret;
 
-			if (hdac_stream(hext_stream)->direction ==
-				SNDRV_PCM_STREAM_CAPTURE)
-				continue;
+	/* make sure all DAI resources are freed */
+	ret = hda_dsp_dais_suspend(sdev);
+	if (ret < 0)
+		dev_warn(sdev->dev, "%s: failure in hda_dsp_dais_suspend\n", __func__);
 
-			stream_tag = hdac_stream(hext_stream)->stream_tag;
-			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-		}
-	}
-#endif
-	return 0;
+	return ret;
 }
 
 void hda_dsp_d0i3_work(struct work_struct *work)
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index f520d1cf70c90..e52cade756179 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -697,6 +697,7 @@ static inline bool hda_common_check_sdw_irq(struct snd_sof_dev *sdev)
 
 /* common dai driver */
 extern struct snd_soc_dai_driver skl_dai[];
+int hda_dsp_dais_suspend(struct snd_sof_dev *sdev);
 
 /*
  * Platform Specific HW abstraction Ops.
-- 
2.30.2

[PATCH 11/14] ASoC: SOF: Intel: hda-dai: improve suspend case

[Pierre-Louis Bossart]

Add comments and re-align with the TRIGGER_SUSPEND case with an
additional call to hda_dai_hw_free_ipc() to free-up resources.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index c1ff7145745bc..dbccd75defe84 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -456,6 +456,7 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 	struct hdac_stream *s;
 	const char *name;
 	int stream_tag;
+	int ret;
 
 	/* set internal flag for BE */
 	list_for_each_entry(s, &bus->stream_list, list) {
@@ -468,20 +469,32 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 		 * explicitly during suspend.
 		 */
 		if (hext_stream->link_substream) {
+			struct snd_soc_dai *cpu_dai;
+			struct snd_soc_dai *codec_dai;
+
 			rtd = asoc_substream_to_rtd(hext_stream->link_substream);
-			name = asoc_rtd_to_codec(rtd, 0)->component->name;
+			cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+			codec_dai = asoc_rtd_to_codec(rtd, 0);
+			name = codec_dai->component->name;
 			link = snd_hdac_ext_bus_get_link(bus, name);
 			if (!link)
 				return -EINVAL;
 
+			/*
+			 * we don't need to call snd_hdac_ext_link_stream_clear(he_stream)
+			 * since we can only reach this case in the pause_push state, and
+			 * the TRIGGER_PAUSE_PUSH already stops the DMA
+			 */
+			if (hdac_stream(hext_stream)->direction == SNDRV_PCM_STREAM_PLAYBACK) {
+				stream_tag = hdac_stream(hext_stream)->stream_tag;
+				snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+			}
 			hext_stream->link_prepared = 0;
 
-			if (hdac_stream(hext_stream)->direction ==
-				SNDRV_PCM_STREAM_CAPTURE)
-				continue;
-
-			stream_tag = hdac_stream(hext_stream)->stream_tag;
-			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+			/* for consistency with TRIGGER_SUSPEND we free DAI resources */
+			ret = hda_dai_hw_free_ipc(hdac_stream(hext_stream)->direction, cpu_dai);
+			if (ret < 0)
+				return ret;
 		}
 	}
 
-- 
2.30.2

[PATCH 12/14] ASoC: SOF: Intel: hda-dai: reset dma_data and release stream

[Pierre-Louis Bossart]

The sequences are missing a call to snd_soc_dai_set_dma_data() when
the stream is cleared, as well as a release of the stream, and tests
to avoid pointer dereferences.

This fixes an underflow issue in a corner case with two streams paused
before a suspend-resume cycle. After resume, the pause_release of the
last stream causes an underflow due to an invalid sequence.

This problem probably existed since the beginning and is only see with
prototypes of a 'deep-buffer' capability, which depends on additional
ASoC fixes, so there's is no Fixes: tag and no real requirement to
backport this patch.

BugLink: https://github.com/thesofproject/linux/issues/3151
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>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index dbccd75defe84..644f75081edd3 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -230,6 +230,9 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 		return -EINVAL;
 
 	dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd);
+	if (!hext_stream)
+		return 0;
+
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
@@ -243,8 +246,10 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 			stream_tag = hdac_stream(hext_stream)->stream_tag;
 			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
 		}
-
+		snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
+		snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
 		hext_stream->link_prepared = 0;
+
 		break;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		snd_hdac_ext_link_stream_clear(hext_stream);
@@ -370,7 +375,7 @@ static int ipc3_hda_dai_prepare(struct snd_pcm_substream *substream,
 	int stream = substream->stream;
 	int ret;
 
-	if (hext_stream->link_prepared)
+	if (hext_stream && hext_stream->link_prepared)
 		return 0;
 
 	dev_dbg(sdev->dev, "%s: prepare stream dir %d\n", __func__, substream->stream);
@@ -460,6 +465,8 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 
 	/* set internal flag for BE */
 	list_for_each_entry(s, &bus->stream_list, list) {
+		struct sof_intel_hda_stream *hda_stream;
+
 		hext_stream = stream_to_hdac_ext_stream(s);
 
 		/*
@@ -489,13 +496,20 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 				stream_tag = hdac_stream(hext_stream)->stream_tag;
 				snd_hdac_ext_link_clear_stream_id(link, stream_tag);
 			}
+			snd_soc_dai_set_dma_data(cpu_dai, hext_stream->link_substream, NULL);
+			snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
 			hext_stream->link_prepared = 0;
 
+			/* free the host DMA channel reserved by hostless streams */
+			hda_stream = hstream_to_sof_hda_stream(hext_stream);
+			hda_stream->host_reserved = 0;
+
 			/* for consistency with TRIGGER_SUSPEND we free DAI resources */
 			ret = hda_dai_hw_free_ipc(hdac_stream(hext_stream)->direction, cpu_dai);
 			if (ret < 0)
 				return ret;
 		}
+
 	}
 
 	return 0;
-- 
2.30.2

[PATCH 13/14] ASoC: SOF: Intel: add helper for link DMA cleanups

[Pierre-Louis Bossart]

We do the same operations from different places, add a helper to
enforce consistency and make the programming sequences clearer.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 112 ++++++++++++++--------------------
 1 file changed, 45 insertions(+), 67 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 644f75081edd3..53600c6c29116 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -128,6 +128,40 @@ hda_link_stream_assign(struct hdac_bus *bus,
 	return res;
 }
 
+static int hda_link_dma_cleanup(struct snd_pcm_substream *substream,
+				struct hdac_stream *hstream,
+				struct snd_soc_dai *cpu_dai,
+				struct snd_soc_dai *codec_dai,
+				bool trigger_suspend_stop)
+{
+	struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
+	struct hdac_bus *bus = hstream->bus;
+	struct sof_intel_hda_stream *hda_stream;
+	struct hdac_ext_link *link;
+	int stream_tag;
+
+	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
+	if (!link)
+		return -EINVAL;
+
+	if (trigger_suspend_stop)
+		snd_hdac_ext_link_stream_clear(hext_stream);
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+		stream_tag = hdac_stream(hext_stream)->stream_tag;
+		snd_hdac_ext_link_clear_stream_id(link, stream_tag);
+	}
+	snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
+	snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
+	hext_stream->link_prepared = 0;
+
+	/* free the host DMA channel reserved by hostless streams */
+	hda_stream = hstream_to_sof_hda_stream(hext_stream);
+	hda_stream->host_reserved = 0;
+
+	return 0;
+}
+
 static int hda_link_dma_params(struct hdac_ext_stream *hext_stream,
 			       struct hda_pipe_params *params)
 {
@@ -221,13 +255,7 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
 	struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	struct hdac_ext_link *link;
-	struct hdac_bus *bus = hstream->bus;
-	int stream_tag;
-
-	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
-	if (!link)
-		return -EINVAL;
+	int ret;
 
 	dev_dbg(cpu_dai->dev, "%s: cmd=%d\n", __func__, cmd);
 	if (!hext_stream)
@@ -240,15 +268,9 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
 	case SNDRV_PCM_TRIGGER_STOP:
-		snd_hdac_ext_link_stream_clear(hext_stream);
-
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-			stream_tag = hdac_stream(hext_stream)->stream_tag;
-			snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-		}
-		snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
-		snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
-		hext_stream->link_prepared = 0;
+		ret = hda_link_dma_cleanup(substream, hstream, cpu_dai, codec_dai, true);
+		if (ret < 0)
+			return ret;
 
 		break;
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
@@ -267,36 +289,13 @@ static int hda_link_dma_hw_free(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
 	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
-	struct sof_intel_hda_stream *hda_stream;
-	struct hdac_bus *bus = hstream->bus;
 	struct hdac_ext_stream *hext_stream;
-	struct hdac_ext_link *link;
-	int stream_tag;
 
 	hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	if (!hext_stream) {
-		dev_dbg(cpu_dai->dev, "%s: hext_stream is not assigned\n", __func__);
-		return -EINVAL;
-	}
-
-	link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name);
-	if (!link)
-		return -EINVAL;
-
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
-		stream_tag = hdac_stream(hext_stream)->stream_tag;
-		snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-	}
-
-	snd_soc_dai_set_dma_data(cpu_dai, substream, NULL);
-	snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
-	hext_stream->link_prepared = 0;
-
-	/* free the host DMA channel reserved by hostless streams */
-	hda_stream = hstream_to_sof_hda_stream(hext_stream);
-	hda_stream->host_reserved = 0;
+	if (!hext_stream)
+		return 0;
 
-	return 0;
+	return hda_link_dma_cleanup(substream, hstream, cpu_dai, codec_dai, false);
 }
 
 static int hda_dai_widget_update(struct snd_soc_dapm_widget *w,
@@ -457,15 +456,11 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 {
 	struct snd_soc_pcm_runtime *rtd;
 	struct hdac_ext_stream *hext_stream;
-	struct hdac_ext_link *link;
 	struct hdac_stream *s;
-	const char *name;
-	int stream_tag;
 	int ret;
 
 	/* set internal flag for BE */
 	list_for_each_entry(s, &bus->stream_list, list) {
-		struct sof_intel_hda_stream *hda_stream;
 
 		hext_stream = stream_to_hdac_ext_stream(s);
 
@@ -482,34 +477,17 @@ static int hda_dai_suspend(struct hdac_bus *bus)
 			rtd = asoc_substream_to_rtd(hext_stream->link_substream);
 			cpu_dai = asoc_rtd_to_cpu(rtd, 0);
 			codec_dai = asoc_rtd_to_codec(rtd, 0);
-			name = codec_dai->component->name;
-			link = snd_hdac_ext_bus_get_link(bus, name);
-			if (!link)
-				return -EINVAL;
 
-			/*
-			 * we don't need to call snd_hdac_ext_link_stream_clear(he_stream)
-			 * since we can only reach this case in the pause_push state, and
-			 * the TRIGGER_PAUSE_PUSH already stops the DMA
-			 */
-			if (hdac_stream(hext_stream)->direction == SNDRV_PCM_STREAM_PLAYBACK) {
-				stream_tag = hdac_stream(hext_stream)->stream_tag;
-				snd_hdac_ext_link_clear_stream_id(link, stream_tag);
-			}
-			snd_soc_dai_set_dma_data(cpu_dai, hext_stream->link_substream, NULL);
-			snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK);
-			hext_stream->link_prepared = 0;
-
-			/* free the host DMA channel reserved by hostless streams */
-			hda_stream = hstream_to_sof_hda_stream(hext_stream);
-			hda_stream->host_reserved = 0;
+			ret = hda_link_dma_cleanup(hext_stream->link_substream, s,
+						   cpu_dai, codec_dai, false);
+			if (ret < 0)
+				return ret;
 
 			/* for consistency with TRIGGER_SUSPEND we free DAI resources */
 			ret = hda_dai_hw_free_ipc(hdac_stream(hext_stream)->direction, cpu_dai);
 			if (ret < 0)
 				return ret;
 		}
-
 	}
 
 	return 0;
-- 
2.30.2

[PATCH 14/14] ASoC: SOF: Intel: hda-dai: protect hw_params against successive calls

[Pierre-Louis Bossart]

Once we've set-up the HDA stream and its format, we currently don't
support additional format changes. We already have a protection in the
.prepare case, but this needs to be added in the hw_params too.

In mixing use cases where two DPCM FEs are connected to the same BE,
if can happen that there are multiple calls to the BE hw_params when
the two FEs are configured simultaneously.

This could alternatively be fixed at the DPCM level but that's a more
intrusive change requiring infrastructure changes: this would need to
be paired with the definition of fixed hw_params at the mixer level.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 sound/soc/sof/intel/hda-dai.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 53600c6c29116..9823230d2ef4a 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -336,8 +336,13 @@ static int hda_dai_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params,
 			     struct snd_soc_dai *dai)
 {
+	struct hdac_ext_stream *hext_stream =
+				snd_soc_dai_get_dma_data(dai, substream);
 	int ret;
 
+	if (hext_stream && hext_stream->link_prepared)
+		return 0;
+
 	ret = hda_link_dma_hw_params(substream, params);
 	if (ret < 0)
 		return ret;
-- 
2.30.2

Re: [PATCH 00/14] ASoC: SOF: Intel: improve HDaudio DAI support

[Mark Brown]

On Thu, 21 Apr 2022 15:31:47 -0500, Pierre-Louis Bossart wrote:
> The SOF CI and daily tests exposed a number of issues with corner
> cases on platforms using the HDaudio DAI, such as UpExtreme boards or
> usual HDaudio+DMIC laptops.
> 
> This patchset provides improvements for pause_push/pause_release,
> suspend-resume, mixing use cases and combinations of all three.
> 
> [...]

Applied to

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

Thanks!

[01/14] ASoC: SOF: remove incorrect clearing of prepared flag
        commit: 8e84b6a4e7f188638748d2ac0455a94799530aa1
[02/14] ASoC: SOF: Intel: Add IPC-specific dai ops for IPC3
        commit: 51ec71dc0cc90e6683ebda7f5ea0ddb71265ab23
[03/14] ASOC: SOF: Intel: hda-dai: consistent naming for HDA DAI and HDA link DMA
        commit: 309e6e557482415c32338b118f0eb17600d98060
[04/14] ASoC: SOF: Intel: hda-dai: simplify hda_dai_widget_update() prototype
        commit: b44c99f11de2c9f01b5526e668c476de976fd14d
[05/14] ASoC: SOF: Intel: hda-dai: use snd_soc_dai_get_widget() helper
        commit: 5ef85c9e42e5fc549e934669fdca352b3da97ec4
[06/14] ASoC: SOF: Intel: hda-dai: split link DMA and dai operations
        commit: f321ffc8d93639181af0512938e2b0630ca28051
[07/14] ASoC: SOF: Intel: hda-dai: regroup dai and link DMA operations
        commit: 9272d6c2af6427df8d7fe665ede6a1bf97d0ca8c
[08/14] ASoC: SOF: sof-audio: flag errors on pipeline teardown
        commit: d1c73a213b462058e91654b5d1d493b3003375cd
[09/14] ASOC: SOF: Intel: hda-dai: add hda_dai_hw_free_ipc() helper
        commit: 81622503229943363858cd7ae1330f49b131dfbc
[10/14] ASoC: SOF: Intel: hda-dai: move code to deal with hda dai/dailink suspend
        commit: f09e92844eabd6a65feab0c548a7cf6741cfa39d
[11/14] ASoC: SOF: Intel: hda-dai: improve suspend case
        commit: 23b1944e46ab4cd7cbd4ef999f814fc6c6f2eb88
[12/14] ASoC: SOF: Intel: hda-dai: reset dma_data and release stream
        commit: 722cbbfaed2a290b1de1fb0ec4ee9a15ec240f7c
[13/14] ASoC: SOF: Intel: add helper for link DMA cleanups
        commit: 880924cad12e96092364467cb7b3ad7a689bec55
[14/14] ASoC: SOF: Intel: hda-dai: protect hw_params against successive calls
        commit: c4eb48f7739fc0dae7e6b8319a77261fc1b61d74

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