[PATCH 0/8] ASoC: Intel: avs: PCM code cleanup

[PATCH 0/8] ASoC: Intel: avs: PCM code cleanup

[Cezary Rojewski]

A set of changes that aims to improve readability of cohesiveness of the
pcm code for the avs-driver.

Start off with a change that synchronizes DAI open/close - DAIs are
started up in ascending order yet their shutdown does not follow the
scheme - it is done in the ascending order too, rather than desceding
one. This patch is a dependency for the next one in line.

To align the HDAudio DAI startup/shutdown with the non-HDAudio
equivalents, relocate the code from component to DAI. The reason above
is a dependency stems from codec driver requirements - HDAudio code
found in sound/pci/hda/ expects substream->runtime->private_data to
point to a valid stream (HOST) pointer.

With the hard part done, the follow up changes update the existing code
to reduce it is complexity - removal of duplicates, renaming of
ambiguous functions and adding new fields to DAI-data object so that the
number of local variables and casts is reduced.

Cezary Rojewski (8):
  ASoC: pcm: Reverse iterate DAIs when shutting them down
  ASoC: Intel: avs: Relocate HDA BE DAI specific operations
  ASoC: Intel: avs: Remove redundancy around DAI shutdown
  ASoC: Intel: avs: Store pointer to adev in DAI dma_data
  ASoC: Intel: avs: Remove redundancy around DAI startup
  ASoC: Intel: avs: Remove redundancy around DAI prepare
  ASoC: Intel: avs: Store pointer to link_stream in dma_data
  ASoC: Intel: avs: Clean up hw constraints initialization

 include/sound/soc.h       |   4 +
 sound/soc/intel/avs/pcm.c | 250 +++++++++++++++++---------------------
 sound/soc/soc-pcm.c       |   2 +-
 3 files changed, 117 insertions(+), 139 deletions(-)

-- 
2.25.1

[PATCH 1/8] ASoC: pcm: Reverse iterate DAIs when shutting them down

[Cezary Rojewski]

During startup snd_soc_dai_startup() is launched in ascending order and
the exact same thing is done during shutdown procedure. Reverse the
order in the latter so that it is symmetric to the former.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 include/sound/soc.h | 4 ++++
 sound/soc/soc-pcm.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 0376f7e4c15d..a4747c3d0856 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1225,6 +1225,10 @@ struct snd_soc_pcm_runtime {
 	     ((i) < (rtd)->dai_link->num_cpus + (rtd)->dai_link->num_codecs) &&	\
 		     ((dai) = (rtd)->dais[i]);				\
 	     (i)++)
+#define for_each_rtd_dais_reverse(rtd, i, dai)					\
+	for ((i) = (rtd)->dai_link->num_cpus + (rtd)->dai_link->num_codecs - 1;	\
+	     (i) >= 0 && ((dai) = (rtd)->dais[i]);				\
+	     (i)--)
 #define for_each_rtd_ch_maps(rtd, i, ch_maps) for_each_link_ch_maps(rtd->dai_link, i, ch_maps)
 
 void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index b0e1bd7f588b..711b2f49ed88 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -724,7 +724,7 @@ static int soc_pcm_clean(struct snd_soc_pcm_runtime *rtd,
 		}
 	}
 
-	for_each_rtd_dais(rtd, i, dai)
+	for_each_rtd_dais_reverse(rtd, i, dai)
 		snd_soc_dai_shutdown(dai, substream, rollback);
 
 	snd_soc_link_shutdown(substream, rollback);
-- 
2.25.1

[PATCH 2/8] ASoC: Intel: avs: Relocate HDA BE DAI specific operations

[Cezary Rojewski]

DAI's startup()/shutdown() shall deal with allocation and freeing of
resources needed to facilitate streaming over it. Currently for HDAudio
BE DAIs some of that task is done in component->open()/close(). Relocate
the relevant pieces to address that.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 55 ++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 29 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 77a7e8f93951..f3cd54f355ef 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -277,12 +277,36 @@ static const struct snd_soc_dai_ops avs_dai_hda_be_ops;
 
 static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
-	return avs_dai_startup(substream, dai, false, &avs_dai_hda_be_ops);
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct hdac_ext_stream *link_stream;
+	struct hda_codec *codec;
+	int ret;
+
+	ret = avs_dai_startup(substream, dai, false, &avs_dai_hda_be_ops);
+	if (ret)
+		return ret;
+
+	codec = dev_to_hda_codec(snd_soc_rtd_to_codec(rtd, 0)->dev);
+	link_stream = snd_hdac_ext_stream_assign(&codec->bus->core, substream,
+						 HDAC_EXT_STREAM_TYPE_LINK);
+	if (!link_stream) {
+		avs_dai_nonhda_be_shutdown(substream, dai);
+		return -EBUSY;
+	}
+
+	substream->runtime->private_data = link_stream;
+	return 0;
 }
 
 static void avs_dai_hda_be_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
-	return avs_dai_nonhda_be_shutdown(substream, dai);
+	struct hdac_ext_stream *link_stream;
+
+	link_stream = substream->runtime->private_data;
+	snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);
+	substream->runtime->private_data = NULL;
+
+	avs_dai_nonhda_be_shutdown(substream, dai);
 }
 
 static int avs_dai_hda_be_hw_params(struct snd_pcm_substream *substream,
@@ -1576,8 +1600,6 @@ static int avs_component_hda_open(struct snd_soc_component *component,
 				  struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct hdac_ext_stream *link_stream;
-	struct hda_codec *codec;
 
 	if (!rtd->dai_link->no_pcm) {
 		struct snd_pcm_hardware hwparams = avs_pcm_hardware;
@@ -1609,30 +1631,6 @@ static int avs_component_hda_open(struct snd_soc_component *component,
 		return snd_soc_set_runtime_hwparams(substream, &hwparams);
 	}
 
-	codec = dev_to_hda_codec(snd_soc_rtd_to_codec(rtd, 0)->dev);
-	link_stream = snd_hdac_ext_stream_assign(&codec->bus->core, substream,
-					     HDAC_EXT_STREAM_TYPE_LINK);
-	if (!link_stream)
-		return -EBUSY;
-
-	substream->runtime->private_data = link_stream;
-	return 0;
-}
-
-static int avs_component_hda_close(struct snd_soc_component *component,
-				   struct snd_pcm_substream *substream)
-{
-	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct hdac_ext_stream *link_stream;
-
-	/* only BE DAI links are handled here */
-	if (!rtd->dai_link->no_pcm)
-		return 0;
-
-	link_stream = substream->runtime->private_data;
-	snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);
-	substream->runtime->private_data = NULL;
-
 	return 0;
 }
 
@@ -1643,7 +1641,6 @@ static const struct snd_soc_component_driver avs_hda_component_driver = {
 	.suspend		= avs_component_suspend,
 	.resume			= avs_component_resume,
 	.open			= avs_component_hda_open,
-	.close			= avs_component_hda_close,
 	.pointer		= avs_component_pointer,
 	.mmap			= avs_component_mmap,
 	.pcm_construct		= avs_component_construct,
-- 
2.25.1

[PATCH 3/8] ASoC: Intel: avs: Remove redundancy around DAI shutdown

[Cezary Rojewski]

Move avs_dai_nonhda_be_shutdown() to avs_dai_shutdown() as the function
is common for all transfer types, not just non-HDAudio ones. Use it
to simplify avs_dai_fe_shutdown().

While at it, fix explicit kfree(data) and use the destructor instead.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 44 +++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index f3cd54f355ef..23f7e0fae817 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -85,6 +85,21 @@ static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 	return 0;
 }
 
+static void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+	struct avs_dev *adev = to_avs_dev(dai->dev);
+	struct avs_dma_data *data;
+
+	data = snd_soc_dai_get_dma_data(dai, substream);
+
+	if (rtd->dai_link->ignore_suspend)
+		adev->num_lp_paths--;
+
+	snd_soc_dai_set_dma_data(dai, substream, NULL);
+	kfree(data);
+}
+
 static int avs_dai_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *fe_hw_params,
 			     struct snd_pcm_hw_params *be_hw_params, struct snd_soc_dai *dai,
@@ -166,21 +181,6 @@ static int avs_dai_nonhda_be_startup(struct snd_pcm_substream *substream, struct
 	return avs_dai_startup(substream, dai, false, &avs_dai_nonhda_be_ops);
 }
 
-static void avs_dai_nonhda_be_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
-{
-	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct avs_dev *adev = to_avs_dev(dai->dev);
-	struct avs_dma_data *data;
-
-	if (rtd->dai_link->ignore_suspend)
-		adev->num_lp_paths--;
-
-	data = snd_soc_dai_get_dma_data(dai, substream);
-
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
-	kfree(data);
-}
-
 static int avs_dai_nonhda_be_hw_params(struct snd_pcm_substream *substream,
 				       struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *dai)
 {
@@ -266,7 +266,7 @@ static int avs_dai_nonhda_be_trigger(struct snd_pcm_substream *substream, int cm
 
 static const struct snd_soc_dai_ops avs_dai_nonhda_be_ops = {
 	.startup = avs_dai_nonhda_be_startup,
-	.shutdown = avs_dai_nonhda_be_shutdown,
+	.shutdown = avs_dai_shutdown,
 	.hw_params = avs_dai_nonhda_be_hw_params,
 	.hw_free = avs_dai_nonhda_be_hw_free,
 	.prepare = avs_dai_nonhda_be_prepare,
@@ -290,7 +290,7 @@ static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct sn
 	link_stream = snd_hdac_ext_stream_assign(&codec->bus->core, substream,
 						 HDAC_EXT_STREAM_TYPE_LINK);
 	if (!link_stream) {
-		avs_dai_nonhda_be_shutdown(substream, dai);
+		avs_dai_shutdown(substream, dai);
 		return -EBUSY;
 	}
 
@@ -306,7 +306,7 @@ static void avs_dai_hda_be_shutdown(struct snd_pcm_substream *substream, struct
 	snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);
 	substream->runtime->private_data = NULL;
 
-	avs_dai_nonhda_be_shutdown(substream, dai);
+	avs_dai_shutdown(substream, dai);
 }
 
 static int avs_dai_hda_be_hw_params(struct snd_pcm_substream *substream,
@@ -558,18 +558,12 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so
 
 static void avs_dai_fe_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
-	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct avs_dev *adev = to_avs_dev(dai->dev);
 	struct avs_dma_data *data;
 
-	if (rtd->dai_link->ignore_suspend)
-		adev->num_lp_paths--;
-
 	data = snd_soc_dai_get_dma_data(dai, substream);
 
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
 	snd_hdac_ext_stream_release(data->host_stream, HDAC_EXT_STREAM_TYPE_HOST);
-	kfree(data);
+	avs_dai_shutdown(substream, dai);
 }
 
 static int avs_dai_fe_hw_params(struct snd_pcm_substream *substream,
-- 
2.25.1

[PATCH 4/8] ASoC: Intel: avs: Store pointer to adev in DAI dma_data

[Cezary Rojewski]

Reduce the number of to_avs_dev() casts by storing the driver context in
DAI's dma_data.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 23f7e0fae817..a3a04115216c 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -22,6 +22,7 @@
 struct avs_dma_data {
 	struct avs_tplg_path_template *template;
 	struct avs_path *path;
+	struct avs_dev *adev;
 	/*
 	 * link stream is stored within substream's runtime
 	 * private_data to fulfill the needs of codec BE path
@@ -60,7 +61,7 @@ static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 			   const struct snd_soc_dai_ops *ops)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct avs_dev *adev = to_avs_dev(dai->dev);
+	struct avs_dev *adev = to_avs_dev(dai->component->dev);
 	struct avs_tplg_path_template *template;
 	struct avs_dma_data *data;
 
@@ -77,6 +78,7 @@ static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 
 	data->substream = substream;
 	data->template = template;
+	data->adev = adev;
 	snd_soc_dai_set_dma_data(dai, substream, data);
 
 	if (rtd->dai_link->ignore_suspend)
@@ -88,13 +90,12 @@ static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 static void avs_dai_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct avs_dev *adev = to_avs_dev(dai->dev);
 	struct avs_dma_data *data;
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
 
 	if (rtd->dai_link->ignore_suspend)
-		adev->num_lp_paths--;
+		data->adev->num_lp_paths--;
 
 	snd_soc_dai_set_dma_data(dai, substream, NULL);
 	kfree(data);
@@ -107,7 +108,6 @@ static int avs_dai_hw_params(struct snd_pcm_substream *substream,
 {
 	struct avs_dma_data *data;
 	struct avs_path *path;
-	struct avs_dev *adev = to_avs_dev(dai->dev);
 	int ret;
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
@@ -124,7 +124,7 @@ static int avs_dai_hw_params(struct snd_pcm_substream *substream,
 		params_rate(be_hw_params), params_channels(be_hw_params),
 		params_width(be_hw_params), params_physical_width(be_hw_params));
 
-	path = avs_path_create(adev, dma_id, data->template, fe_hw_params, be_hw_params);
+	path = avs_path_create(data->adev, dma_id, data->template, fe_hw_params, be_hw_params);
 	if (IS_ERR(path)) {
 		ret = PTR_ERR(path);
 		dev_err(dai->dev, "create path failed: %d\n", ret);
@@ -505,8 +505,7 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct avs_dma_data *data;
-	struct avs_dev *adev = to_avs_dev(dai->dev);
-	struct hdac_bus *bus = &adev->base.core;
+	struct hdac_bus *bus;
 	struct hdac_ext_stream *host_stream;
 	int ret;
 
@@ -515,6 +514,7 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so
 		return ret;
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
+	bus = &data->adev->base.core;
 
 	host_stream = snd_hdac_ext_stream_assign(bus, substream, HDAC_EXT_STREAM_TYPE_HOST);
 	if (!host_stream) {
@@ -655,7 +655,6 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_pcm_stream *stream_info;
 	struct avs_dma_data *data;
-	struct avs_dev *adev = to_avs_dev(dai->dev);
 	struct hdac_ext_stream *host_stream;
 	unsigned int format_val;
 	struct hdac_bus *bus;
@@ -685,7 +684,7 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so
 	if (ret < 0)
 		return ret;
 
-	ret = avs_dai_prepare(adev, substream, dai);
+	ret = avs_dai_prepare(data->adev, substream, dai);
 	if (ret)
 		return ret;
 
-- 
2.25.1

[PATCH 5/8] ASoC: Intel: avs: Remove redundancy around DAI startup

[Cezary Rojewski]

Half of the arguments in avs_dai_startup() are unused and can be
dropped. With the function updated, it matches its template in
snd_soc_dai_ops and can be referenced throughout the pcm.c file without
need of any wrappers.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index a3a04115216c..0771a9716f4b 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -57,15 +57,14 @@ avs_dai_find_path_template(struct snd_soc_dai *dai, bool is_fe, int direction)
 	return dw->priv;
 }
 
-static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai, bool is_fe,
-			   const struct snd_soc_dai_ops *ops)
+static int avs_dai_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct avs_dev *adev = to_avs_dev(dai->component->dev);
 	struct avs_tplg_path_template *template;
 	struct avs_dma_data *data;
 
-	template = avs_dai_find_path_template(dai, is_fe, substream->stream);
+	template = avs_dai_find_path_template(dai, !rtd->dai_link->no_pcm, substream->stream);
 	if (!template) {
 		dev_err(dai->dev, "no %s path for dai %s, invalid tplg?\n",
 			snd_pcm_stream_str(substream), dai->name);
@@ -174,13 +173,6 @@ static int avs_dai_prepare(struct avs_dev *adev, struct snd_pcm_substream *subst
 	return ret;
 }
 
-static const struct snd_soc_dai_ops avs_dai_nonhda_be_ops;
-
-static int avs_dai_nonhda_be_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
-{
-	return avs_dai_startup(substream, dai, false, &avs_dai_nonhda_be_ops);
-}
-
 static int avs_dai_nonhda_be_hw_params(struct snd_pcm_substream *substream,
 				       struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *dai)
 {
@@ -265,7 +257,7 @@ static int avs_dai_nonhda_be_trigger(struct snd_pcm_substream *substream, int cm
 }
 
 static const struct snd_soc_dai_ops avs_dai_nonhda_be_ops = {
-	.startup = avs_dai_nonhda_be_startup,
+	.startup = avs_dai_startup,
 	.shutdown = avs_dai_shutdown,
 	.hw_params = avs_dai_nonhda_be_hw_params,
 	.hw_free = avs_dai_nonhda_be_hw_free,
@@ -273,8 +265,6 @@ static const struct snd_soc_dai_ops avs_dai_nonhda_be_ops = {
 	.trigger = avs_dai_nonhda_be_trigger,
 };
 
-static const struct snd_soc_dai_ops avs_dai_hda_be_ops;
-
 static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
@@ -282,7 +272,7 @@ static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct sn
 	struct hda_codec *codec;
 	int ret;
 
-	ret = avs_dai_startup(substream, dai, false, &avs_dai_hda_be_ops);
+	ret = avs_dai_startup(substream, dai);
 	if (ret)
 		return ret;
 
@@ -479,8 +469,6 @@ static const struct snd_pcm_hw_constraint_list hw_rates = {
 	.mask = 0,
 };
 
-const struct snd_soc_dai_ops avs_dai_fe_ops;
-
 static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule)
 {
 	struct snd_interval *interval = hw_param_interval(params, rule->var);
@@ -509,7 +497,7 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so
 	struct hdac_ext_stream *host_stream;
 	int ret;
 
-	ret = avs_dai_startup(substream, dai, true, &avs_dai_fe_ops);
+	ret = avs_dai_startup(substream, dai);
 	if (ret)
 		return ret;
 
-- 
2.25.1

[PATCH 6/8] ASoC: Intel: avs: Remove redundancy around DAI prepare

[Cezary Rojewski]

Drop unused arguments in the avs_dai_prepare() function. With the
function updated, it matches its template in snd_soc_dai_ops and can be
referenced throughout the pcm.c file without need of any wrappers.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 0771a9716f4b..d4557b7b1c6c 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -151,8 +151,7 @@ static int avs_dai_be_hw_params(struct snd_pcm_substream *substream,
 	return avs_dai_hw_params(substream, fe_hw_params, be_hw_params, dai, dma_id);
 }
 
-static int avs_dai_prepare(struct avs_dev *adev, struct snd_pcm_substream *substream,
-			   struct snd_soc_dai *dai)
+static int avs_dai_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct avs_dma_data *data;
 	int ret;
@@ -201,11 +200,6 @@ static int avs_dai_nonhda_be_hw_free(struct snd_pcm_substream *substream, struct
 	return 0;
 }
 
-static int avs_dai_nonhda_be_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
-{
-	return avs_dai_prepare(to_avs_dev(dai->dev), substream, dai);
-}
-
 static int avs_dai_nonhda_be_trigger(struct snd_pcm_substream *substream, int cmd,
 				     struct snd_soc_dai *dai)
 {
@@ -261,7 +255,7 @@ static const struct snd_soc_dai_ops avs_dai_nonhda_be_ops = {
 	.shutdown = avs_dai_shutdown,
 	.hw_params = avs_dai_nonhda_be_hw_params,
 	.hw_free = avs_dai_nonhda_be_hw_free,
-	.prepare = avs_dai_nonhda_be_prepare,
+	.prepare = avs_dai_prepare,
 	.trigger = avs_dai_nonhda_be_trigger,
 };
 
@@ -381,7 +375,7 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		snd_hdac_ext_bus_link_set_stream_id(link, hdac_stream(link_stream)->stream_tag);
 
-	ret = avs_dai_prepare(to_avs_dev(dai->dev), substream, dai);
+	ret = avs_dai_prepare(substream, dai);
 	if (ret)
 		return ret;
 
@@ -672,7 +666,7 @@ static int avs_dai_fe_prepare(struct snd_pcm_substream *substream, struct snd_so
 	if (ret < 0)
 		return ret;
 
-	ret = avs_dai_prepare(data->adev, substream, dai);
+	ret = avs_dai_prepare(substream, dai);
 	if (ret)
 		return ret;
 
-- 
2.25.1

[PATCH 7/8] ASoC: Intel: avs: Store pointer to link_stream in dma_data

[Cezary Rojewski]

While the HDAudio codec driver expectations must be met - store valid
pointer to HDAudio LINK stream in substream->runtime->private_data - the
code is more readable and easier to maintain if dma_data stores pointers
to both HOST and LINK stream.

DAI BE operations can refer to the LINK stream with data->link_stream,
similarly to how DAI FE operations access the HOST stream with
data->host_stream.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 40 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index d4557b7b1c6c..168e16e82116 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -23,13 +23,12 @@ struct avs_dma_data {
 	struct avs_tplg_path_template *template;
 	struct avs_path *path;
 	struct avs_dev *adev;
-	/*
-	 * link stream is stored within substream's runtime
-	 * private_data to fulfill the needs of codec BE path
-	 *
-	 * host stream assigned
-	 */
-	struct hdac_ext_stream *host_stream;
+
+	/* LINK-stream utilized in BE operations while HOST in FE ones. */
+	union {
+		struct hdac_ext_stream *link_stream;
+		struct hdac_ext_stream *host_stream;
+	};
 
 	struct snd_pcm_substream *substream;
 };
@@ -263,6 +262,7 @@ static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct sn
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
 	struct hdac_ext_stream *link_stream;
+	struct avs_dma_data *data;
 	struct hda_codec *codec;
 	int ret;
 
@@ -278,18 +278,18 @@ static int avs_dai_hda_be_startup(struct snd_pcm_substream *substream, struct sn
 		return -EBUSY;
 	}
 
+	data = snd_soc_dai_get_dma_data(dai, substream);
+	data->link_stream = link_stream;
 	substream->runtime->private_data = link_stream;
 	return 0;
 }
 
 static void avs_dai_hda_be_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
-	struct hdac_ext_stream *link_stream;
+	struct avs_dma_data *data = snd_soc_dai_get_dma_data(dai, substream);
 
-	link_stream = substream->runtime->private_data;
-	snd_hdac_ext_stream_release(link_stream, HDAC_EXT_STREAM_TYPE_LINK);
+	snd_hdac_ext_stream_release(data->link_stream, HDAC_EXT_STREAM_TYPE_LINK);
 	substream->runtime->private_data = NULL;
-
 	avs_dai_shutdown(substream, dai);
 }
 
@@ -297,16 +297,13 @@ static int avs_dai_hda_be_hw_params(struct snd_pcm_substream *substream,
 				    struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *dai)
 {
 	struct avs_dma_data *data;
-	struct hdac_ext_stream *link_stream;
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
 	if (data->path)
 		return 0;
 
-	link_stream = substream->runtime->private_data;
-
 	return avs_dai_be_hw_params(substream, hw_params, dai,
-				    hdac_stream(link_stream)->stream_tag - 1);
+				    hdac_stream(data->link_stream)->stream_tag - 1);
 }
 
 static int avs_dai_hda_be_hw_free(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
@@ -323,7 +320,7 @@ static int avs_dai_hda_be_hw_free(struct snd_pcm_substream *substream, struct sn
 	if (!data->path)
 		return 0;
 
-	link_stream = substream->runtime->private_data;
+	link_stream = data->link_stream;
 	link_stream->link_prepared = false;
 	avs_path_free(data->path);
 	data->path = NULL;
@@ -347,13 +344,16 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn
 	struct snd_soc_pcm_stream *stream_info;
 	struct hdac_ext_stream *link_stream;
 	struct hdac_ext_link *link;
+	struct avs_dma_data *data;
 	struct hda_codec *codec;
 	struct hdac_bus *bus;
 	unsigned int format_val;
 	unsigned int bits;
 	int ret;
 
-	link_stream = runtime->private_data;
+	data = snd_soc_dai_get_dma_data(dai, substream);
+	link_stream = data->link_stream;
+
 	if (link_stream->link_prepared)
 		return 0;
 
@@ -387,14 +387,12 @@ static int avs_dai_hda_be_trigger(struct snd_pcm_substream *substream, int cmd,
 				  struct snd_soc_dai *dai)
 {
 	struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
-	struct hdac_ext_stream *link_stream;
 	struct avs_dma_data *data;
 	int ret = 0;
 
 	dev_dbg(dai->dev, "entry %s cmd=%d\n", __func__, cmd);
 
 	data = snd_soc_dai_get_dma_data(dai, substream);
-	link_stream = substream->runtime->private_data;
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_RESUME:
@@ -403,7 +401,7 @@ static int avs_dai_hda_be_trigger(struct snd_pcm_substream *substream, int cmd,
 		fallthrough;
 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		snd_hdac_ext_stream_start(link_stream);
+		snd_hdac_ext_stream_start(data->link_stream);
 
 		ret = avs_path_pause(data->path);
 		if (ret < 0) {
@@ -426,7 +424,7 @@ static int avs_dai_hda_be_trigger(struct snd_pcm_substream *substream, int cmd,
 		if (ret < 0)
 			dev_err(dai->dev, "pause BE path failed: %d\n", ret);
 
-		snd_hdac_ext_stream_clear(link_stream);
+		snd_hdac_ext_stream_clear(data->link_stream);
 
 		ret = avs_path_reset(data->path);
 		if (ret < 0)
-- 
2.25.1

[PATCH 8/8] ASoC: Intel: avs: Clean up hw constraints initialization

[Cezary Rojewski]

Provide a separate function that initializes all PCM hardware
constraints for the driver. No functional changes.

Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/pcm.c | 84 ++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 40 deletions(-)

diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
index 168e16e82116..845b5ed9eb1b 100644
--- a/sound/soc/intel/avs/pcm.c
+++ b/sound/soc/intel/avs/pcm.c
@@ -448,19 +448,6 @@ static const struct snd_soc_dai_ops avs_dai_hda_be_ops = {
 	.trigger = avs_dai_hda_be_trigger,
 };
 
-static const unsigned int rates[] = {
-	8000, 11025, 12000, 16000,
-	22050, 24000, 32000, 44100,
-	48000, 64000, 88200, 96000,
-	128000, 176400, 192000,
-};
-
-static const struct snd_pcm_hw_constraint_list hw_rates = {
-	.count = ARRAY_SIZE(rates),
-	.list = rates,
-	.mask = 0,
-};
-
 static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule)
 {
 	struct snd_interval *interval = hw_param_interval(params, rule->var);
@@ -481,40 +468,33 @@ static int hw_rule_param_size(struct snd_pcm_hw_params *params, struct snd_pcm_h
 	return snd_interval_refine(interval, &to);
 }
 
-static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+static int avs_pcm_hw_constraints_init(struct snd_pcm_substream *substream)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct avs_dma_data *data;
-	struct hdac_bus *bus;
-	struct hdac_ext_stream *host_stream;
+	static const unsigned int rates[] = {
+		8000, 11025, 12000, 16000,
+		22050, 24000, 32000, 44100,
+		48000, 64000, 88200, 96000,
+		128000, 176400, 192000,
+	};
+	static const struct snd_pcm_hw_constraint_list rate_list = {
+		.count = ARRAY_SIZE(rates),
+		.list = rates,
+	};
 	int ret;
 
-	ret = avs_dai_startup(substream, dai);
-	if (ret)
-		return ret;
-
-	data = snd_soc_dai_get_dma_data(dai, substream);
-	bus = &data->adev->base.core;
-
-	host_stream = snd_hdac_ext_stream_assign(bus, substream, HDAC_EXT_STREAM_TYPE_HOST);
-	if (!host_stream) {
-		ret = -EBUSY;
-		goto err;
-	}
-
-	data->host_stream = host_stream;
 	ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	if (ret < 0)
-		goto err;
+		return ret;
 
-	/* avoid wrap-around with wall-clock */
+	/* Avoid wrap-around with wall-clock. */
 	ret = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_TIME, 20, 178000000);
 	if (ret < 0)
-		goto err;
+		return ret;
 
-	ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &hw_rates);
+	ret = snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE, &rate_list);
 	if (ret < 0)
-		goto err;
+		return ret;
 
 	/* Adjust buffer and period size based on the audio format. */
 	snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_SIZE, hw_rule_param_size, NULL,
@@ -524,16 +504,40 @@ static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_so
 			    SNDRV_PCM_HW_PARAM_FORMAT, SNDRV_PCM_HW_PARAM_CHANNELS,
 			    SNDRV_PCM_HW_PARAM_RATE, -1);
 
+	return ret;
+}
+
+static int avs_dai_fe_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
+{
+	struct hdac_ext_stream *host_stream;
+	struct avs_dma_data *data;
+	struct hdac_bus *bus;
+	int ret;
+
+	ret = avs_pcm_hw_constraints_init(substream);
+	if (ret)
+		return ret;
+
+	ret = avs_dai_startup(substream, dai);
+	if (ret)
+		return ret;
+
+	data = snd_soc_dai_get_dma_data(dai, substream);
+	bus = &data->adev->base.core;
+
+	host_stream = snd_hdac_ext_stream_assign(bus, substream, HDAC_EXT_STREAM_TYPE_HOST);
+	if (!host_stream) {
+		avs_dai_shutdown(substream, dai);
+		return -EBUSY;
+	}
+
+	data->host_stream = host_stream;
 	snd_pcm_set_sync(substream);
 
 	dev_dbg(dai->dev, "%s fe STARTUP tag %d str %p",
 		__func__, hdac_stream(host_stream)->stream_tag, substream);
 
 	return 0;
-
-err:
-	kfree(data);
-	return ret;
 }
 
 static void avs_dai_fe_shutdown(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
-- 
2.25.1

Re: [PATCH 0/8] ASoC: Intel: avs: PCM code cleanup

[Mark Brown]

On Fri, 26 Apr 2024 11:57:25 +0200, Cezary Rojewski wrote:
> A set of changes that aims to improve readability of cohesiveness of the
> pcm code for the avs-driver.
> 
> Start off with a change that synchronizes DAI open/close - DAIs are
> started up in ascending order yet their shutdown does not follow the
> scheme - it is done in the ascending order too, rather than desceding
> one. This patch is a dependency for the next one in line.
> 
> [...]

Applied to

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

Thanks!

[1/8] ASoC: pcm: Reverse iterate DAIs when shutting them down
      commit: 31a70a71b3a730aa703bbd05713d21115dd6d33a
[2/8] ASoC: Intel: avs: Relocate HDA BE DAI specific operations
      commit: 140df6d4d5f541e950a35cad2e3dffb49186ed74
[3/8] ASoC: Intel: avs: Remove redundancy around DAI shutdown
      commit: b9d59f970ea7772957f6da02ca1ba272ef4495b8
[4/8] ASoC: Intel: avs: Store pointer to adev in DAI dma_data
      commit: c303a994e5d0f7d297cb6ac56052dce8f412ee67
[5/8] ASoC: Intel: avs: Remove redundancy around DAI startup
      commit: 3a48d146aa761bc591272bc453eda64743128a31
[6/8] ASoC: Intel: avs: Remove redundancy around DAI prepare
      commit: 0f8843ca4f6cbf0efb8c2d5516a3b92fb2771a04
[7/8] ASoC: Intel: avs: Store pointer to link_stream in dma_data
      commit: cdcb770a60e8e6b9fbb737ebe21b2daadaba1744
[8/8] ASoC: Intel: avs: Clean up hw constraints initialization
      commit: e85e75b67993c1fb0c80306783c31266261170d4

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