All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
@ 2020-03-11  1:06 Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 1/7] ASoC: soc-core: " Kuninori Morimoto
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:06 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


Hi Mark

Current ALSA SoC has CPU/Codec categorized DAIs.
But it will be no longer good much for modern device.

Currently, rtd has both CPU/Codec DAIs array.

	rtd->cpu_dais   = [][][][][][][][][]
	rtd->codec_dais = [][][][][][][][][]

This patch merges these, like below.
It still keeps rtd->cpu_dais, rtd->codec_dais

	rtd->dais = [][][][][][][][][][][][][][][][][][]
	            ^cpu_dais         ^codec_dais
	            |--- num_cpus ---|--- num_codecs --|

After this merging, we can merge for_each_rtd_cpu/codec_dais().

-	for_each_rtd_cpu_dais() {
-		...
-	}
-	for_each_rtd_codec_dais() {
-		...
-	}
+	for_each_rtd_dais() {
+		...
+	}


Kuninori Morimoto (7):
  ASoC: soc-core: Merge CPU/Codec DAIs
  ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais()
  ASoC: soc-dapm: Merge for_each_rtd_cpu/codec_dais()
  ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
  ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new()
  ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb()
  ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()

 include/sound/soc.h  |   7 +-
 sound/soc/soc-core.c |  66 +++-----
 sound/soc/soc-dapm.c |   9 +-
 sound/soc/soc-pcm.c  | 367 +++++++++++--------------------------------
 4 files changed, 123 insertions(+), 326 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/7] ASoC: soc-core: Merge CPU/Codec DAIs
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 2/7] ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais() Kuninori Morimoto
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ALSA SoC is currently categorizing CPU/Codec DAIs,
and it works well.

But modern devices require more complex connections,
for example Codec to Codec, etc, and future devices will
enable to more complex connections.
Because of these background, CPU/Codec DAIs categorizing is
no longer good much to modern device.

Currently, rtd has both CPU/Codec DAIs pointer.

	rtd->cpu_dais   = [][][][][][][][][]
	rtd->codec_dais = [][][][][][][][][]

This patch merges these into DAIs pointer.

	rtd->dais = [][][][][][][][][][][][][][][][][][]
	            ^cpu_dais         ^codec_dais
	            |--- num_cpus ---|--- num_codecs --|

Then, we can merge for_each_rtd_cpu/codec_dais() from this patch.

-	for_each_rtd_cpu_dais() {
-		...
-	}
-	for_each_rtd_codec_dais() {
-		...
-	}
+	for_each_rtd_dais() {
+		...
+	}

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |  7 ++++++-
 sound/soc/soc-core.c | 18 +++++++++---------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 03054bf9cd37..efa12256bb33 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1144,6 +1144,7 @@ struct snd_soc_pcm_runtime {
 	struct snd_compr *compr;
 	struct snd_soc_dai *codec_dai;
 	struct snd_soc_dai *cpu_dai;
+	struct snd_soc_dai **dais;
 
 	struct snd_soc_dai **codec_dais;
 	unsigned int num_codecs;
@@ -1183,7 +1184,11 @@ struct snd_soc_pcm_runtime {
 	     (i)++)
 #define for_each_rtd_codec_dais_rollback(rtd, i, dai)		\
 	for (; (--(i) >= 0) && ((dai) = rtd->codec_dais[i]);)
-
+#define for_each_rtd_dais(rtd, i, dai)					\
+	for ((i) = 0;							\
+	     ((i) < (rtd)->num_cpus + (rtd)->num_codecs) &&		\
+		     ((dai) = (rtd)->dais[i]);				\
+	     (i)++)
 
 void snd_soc_close_delayed_work(struct snd_soc_pcm_runtime *rtd);
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 4e0f55555e37..511f6b0cb2e0 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -475,22 +475,22 @@ static struct snd_soc_pcm_runtime *soc_new_pcm_runtime(
 	INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
 
 	/*
-	 * for rtd->codec_dais
+	 * for rtd->dais
 	 */
-	rtd->codec_dais = devm_kcalloc(dev, dai_link->num_codecs,
+	rtd->dais = devm_kcalloc(dev, dai_link->num_cpus + dai_link->num_codecs,
 					sizeof(struct snd_soc_dai *),
 					GFP_KERNEL);
-	if (!rtd->codec_dais)
+	if (!rtd->dais)
 		goto free_rtd;
 
 	/*
-	 * for rtd->cpu_dais
+	 * dais = [][][][][][][][][][][][][][][][][][]
+	 *	  ^cpu_dais         ^codec_dais
+	 *	  |--- num_cpus ---|--- num_codecs --|
 	 */
-	rtd->cpu_dais = devm_kcalloc(dev, dai_link->num_cpus,
-				     sizeof(struct snd_soc_dai *),
-				     GFP_KERNEL);
-	if (!rtd->cpu_dais)
-		goto free_rtd;
+	rtd->cpu_dais	= &rtd->dais[0];
+	rtd->codec_dais	= &rtd->dais[dai_link->num_cpus];
+
 	/*
 	 * rtd remaining settings
 	 */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/7] ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 1/7] ASoC: soc-core: " Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 3/7] ASoC: soc-dapm: " Kuninori Morimoto
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Now we can use for_each_rtd_dais().
Let's use it instead of for_each_rtd_cpu/codec_dais().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 511f6b0cb2e0..333cbbd268b4 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1313,26 +1313,22 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order)
 static void soc_remove_link_dais(struct snd_soc_card *card)
 {
 	int i;
-	struct snd_soc_dai *codec_dai;
-	struct snd_soc_dai *cpu_dai;
+	struct snd_soc_dai *dai;
 	struct snd_soc_pcm_runtime *rtd;
 	int order;
 
 	for_each_comp_order(order) {
 		for_each_card_rtds(card, rtd) {
-			/* remove the CODEC DAI */
-			for_each_rtd_codec_dais(rtd, i, codec_dai)
-				soc_remove_dai(codec_dai, order);
-
-			for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-				soc_remove_dai(cpu_dai, order);
+			/* remove DAIs */
+			for_each_rtd_dais(rtd, i, dai)
+				soc_remove_dai(dai, order);
 		}
 	}
 }
 
 static int soc_probe_link_dais(struct snd_soc_card *card)
 {
-	struct snd_soc_dai *codec_dai, *cpu_dai;
+	struct snd_soc_dai *dai;
 	struct snd_soc_pcm_runtime *rtd;
 	int i, order, ret;
 
@@ -1344,15 +1340,8 @@ static int soc_probe_link_dais(struct snd_soc_card *card)
 				card->name, rtd->num, order);
 
 			/* probe the CPU DAI */
-			for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-				ret = soc_probe_dai(cpu_dai, order);
-				if (ret)
-					return ret;
-			}
-
-			/* probe the CODEC DAI */
-			for_each_rtd_codec_dais(rtd, i, codec_dai) {
-				ret = soc_probe_dai(codec_dai, order);
+			for_each_rtd_dais(rtd, i, dai) {
+				ret = soc_probe_dai(dai, order);
 				if (ret)
 					return ret;
 			}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/7] ASoC: soc-dapm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 1/7] ASoC: soc-core: " Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 2/7] ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais() Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:27   ` Sridharan, Ranjani
  2020-03-11  1:07 ` [PATCH 4/7] ASoC: soc-pcm: " Kuninori Morimoto
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Now we can use for_each_rtd_dais().
Let's use it instead of for_each_rtd_cpu/codec_dais().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-dapm.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index e00a465a7c32..3a3fbf167383 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -4433,14 +4433,11 @@ void snd_soc_dapm_connect_dai_link_widgets(struct snd_soc_card *card)
 static void soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int stream,
 	int event)
 {
-	struct snd_soc_dai *codec_dai;
-	struct snd_soc_dai *cpu_dai;
+	struct snd_soc_dai *dai;
 	int i;
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		soc_dapm_dai_stream_event(cpu_dai, stream, event);
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		soc_dapm_dai_stream_event(codec_dai, stream, event);
+	for_each_rtd_cpu_dais(rtd, i, dai)
+		soc_dapm_dai_stream_event(dai, stream, event);
 
 	dapm_power_widgets(rtd->card, event);
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/7] ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
                   ` (2 preceding siblings ...)
  2020-03-11  1:07 ` [PATCH 3/7] ASoC: soc-dapm: " Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:30   ` Sridharan, Ranjani
  2020-03-11  1:07 ` [PATCH 5/7] ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new() Kuninori Morimoto
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Now we can use for_each_rtd_dais().
Let's use it instead of for_each_rtd_cpu/codec_dais().

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 314 +++++++++++---------------------------------
 1 file changed, 75 insertions(+), 239 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 733d7e8a0e55..dae1821c78dc 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -259,25 +259,15 @@ static int soc_rtd_trigger(struct snd_soc_pcm_runtime *rtd,
 static void snd_soc_runtime_action(struct snd_soc_pcm_runtime *rtd,
 				   int stream, int action)
 {
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i;
 
 	lockdep_assert_held(&rtd->card->pcm_mutex);
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		cpu_dai->stream_active[stream] += action;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		codec_dai->stream_active[stream] += action;
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		cpu_dai->active += action;
-		cpu_dai->component->active += action;
-	}
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		codec_dai->active += action;
-		codec_dai->component->active += action;
+	for_each_rtd_dais(rtd, i, dai) {
+		dai->stream_active[stream] += action;
+		dai->active += action;
+		dai->component->active += action;
 	}
 }
 
@@ -444,8 +434,8 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
 				struct snd_pcm_hw_params *params)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *dai;
 	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
 	unsigned int rate, channels, sample_bits, symmetry, i;
 
 	rate = params_rate(params);
@@ -455,11 +445,8 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
 	/* reject unmatched parameters when applying symmetry */
 	symmetry = rtd->dai_link->symmetric_rates;
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		symmetry |= cpu_dai->driver->symmetric_rates;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		symmetry |= codec_dai->driver->symmetric_rates;
+	for_each_rtd_cpu_dais(rtd, i, dai)
+		symmetry |= dai->driver->symmetric_rates;
 
 	if (symmetry) {
 		for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
@@ -473,11 +460,8 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
 
 	symmetry = rtd->dai_link->symmetric_channels;
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		symmetry |= cpu_dai->driver->symmetric_channels;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		symmetry |= codec_dai->driver->symmetric_channels;
+	for_each_rtd_dais(rtd, i, dai)
+		symmetry |= dai->driver->symmetric_channels;
 
 	if (symmetry) {
 		for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
@@ -492,11 +476,8 @@ static int soc_pcm_params_symmetry(struct snd_pcm_substream *substream,
 
 	symmetry = rtd->dai_link->symmetric_samplebits;
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		symmetry |= cpu_dai->driver->symmetric_samplebits;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		symmetry |= codec_dai->driver->symmetric_samplebits;
+	for_each_rtd_dais(rtd, i, dai)
+		symmetry |= dai->driver->symmetric_samplebits;
 
 	if (symmetry) {
 		for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
@@ -516,25 +497,18 @@ static bool soc_pcm_has_symmetry(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_dai_link *link = rtd->dai_link;
-	struct snd_soc_dai *codec_dai;
-	struct snd_soc_dai *cpu_dai;
+	struct snd_soc_dai *dai;
 	unsigned int symmetry, i;
 
 	symmetry = link->symmetric_rates ||
 		link->symmetric_channels ||
 		link->symmetric_samplebits;
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		symmetry = symmetry ||
-			cpu_dai->driver->symmetric_rates ||
-			cpu_dai->driver->symmetric_channels ||
-			cpu_dai->driver->symmetric_samplebits;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
+	for_each_rtd_dais(rtd, i, dai)
 		symmetry = symmetry ||
-			codec_dai->driver->symmetric_rates ||
-			codec_dai->driver->symmetric_channels ||
-			codec_dai->driver->symmetric_samplebits;
+			dai->driver->symmetric_rates ||
+			dai->driver->symmetric_channels ||
+			dai->driver->symmetric_samplebits;
 
 	return symmetry;
 }
@@ -772,19 +746,15 @@ static int soc_pcm_close(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
 	snd_soc_runtime_deactivate(rtd, substream->stream);
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		snd_soc_dai_shutdown(cpu_dai, substream);
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		snd_soc_dai_shutdown(codec_dai, substream);
+	for_each_rtd_dais(rtd, i, dai)
+		snd_soc_dai_shutdown(dai, substream);
 
 	soc_rtd_shutdown(rtd, substream);
 
@@ -816,8 +786,7 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_soc_component *component;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	const char *codec_dai_name = "multicodec";
 	const char *cpu_dai_name = "multicpu";
 	int i, ret = 0;
@@ -842,28 +811,19 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	}
 
 	/* startup the audio subsystem */
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_startup(cpu_dai, substream);
-		if (ret < 0) {
-			dev_err(cpu_dai->dev, "ASoC: can't open interface %s: %d\n",
-				cpu_dai->name, ret);
-			goto cpu_dai_err;
-		}
-	}
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_startup(codec_dai, substream);
+	for_each_rtd_dais(rtd, i, dai) {
+		ret = snd_soc_dai_startup(dai, substream);
 		if (ret < 0) {
-			dev_err(codec_dai->dev,
-				"ASoC: can't open codec %s: %d\n",
-				codec_dai->name, ret);
+			dev_err(dai->dev,
+				"ASoC: can't open DAI %s: %d\n",
+				dai->name, ret);
 			goto config_err;
 		}
 
 		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			codec_dai->tx_mask = 0;
+			dai->tx_mask = 0;
 		else
-			codec_dai->rx_mask = 0;
+			dai->rx_mask = 0;
 	}
 
 	/* Dynamic PCM DAI links compat checks use dynamic capabilities */
@@ -903,17 +863,9 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	soc_pcm_apply_msb(substream);
 
 	/* Symmetry only applies if we've already got an active stream. */
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		if (cpu_dai->active) {
-			ret = soc_pcm_apply_symmetry(substream, cpu_dai);
-			if (ret != 0)
-				goto config_err;
-		}
-	}
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		if (codec_dai->active) {
-			ret = soc_pcm_apply_symmetry(substream, codec_dai);
+	for_each_rtd_dais(rtd, i, dai) {
+		if (dai->active) {
+			ret = soc_pcm_apply_symmetry(substream, dai);
 			if (ret != 0)
 				goto config_err;
 		}
@@ -935,11 +887,8 @@ static int soc_pcm_open(struct snd_pcm_substream *substream)
 	return 0;
 
 config_err:
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		snd_soc_dai_shutdown(codec_dai, substream);
-cpu_dai_err:
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		snd_soc_dai_shutdown(cpu_dai, substream);
+	for_each_rtd_dais(rtd, i, dai)
+		snd_soc_dai_shutdown(dai, substream);
 
 	soc_rtd_shutdown(rtd, substream);
 rtd_startup_err:
@@ -978,8 +927,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i, ret = 0;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
@@ -1000,21 +948,11 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 		}
 	}
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_prepare(codec_dai, substream);
-		if (ret < 0) {
-			dev_err(codec_dai->dev,
-				"ASoC: codec DAI prepare error: %d\n",
-				ret);
-			goto out;
-		}
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_prepare(cpu_dai, substream);
+	for_each_rtd_dais(rtd, i, dai) {
+		ret = snd_soc_dai_prepare(dai, substream);
 		if (ret < 0) {
-			dev_err(cpu_dai->dev,
-				"ASoC: cpu DAI prepare error: %d\n", ret);
+			dev_err(dai->dev,
+				"ASoC: DAI prepare error: %d\n", ret);
 			goto out;
 		}
 	}
@@ -1029,11 +967,8 @@ static int soc_pcm_prepare(struct snd_pcm_substream *substream)
 	snd_soc_dapm_stream_event(rtd, substream->stream,
 			SND_SOC_DAPM_STREAM_START);
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai)
-		snd_soc_dai_digital_mute(codec_dai, 0,
-					 substream->stream);
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai)
-		snd_soc_dai_digital_mute(cpu_dai, 0, substream->stream);
+	for_each_rtd_dais(rtd, i, dai)
+		snd_soc_dai_digital_mute(dai, 0, substream->stream);
 
 out:
 	mutex_unlock(&rtd->card->pcm_mutex);
@@ -1217,44 +1152,23 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream,
 static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i;
 
 	mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
 
 	/* clear the corresponding DAIs parameters when going to be inactive */
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		if (cpu_dai->active == 1) {
-			cpu_dai->rate = 0;
-			cpu_dai->channels = 0;
-			cpu_dai->sample_bits = 0;
-		}
-	}
+	for_each_rtd_dais(rtd, i, dai) {
+		int active = dai->stream_active[substream->stream];
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		if (codec_dai->active == 1) {
-			codec_dai->rate = 0;
-			codec_dai->channels = 0;
-			codec_dai->sample_bits = 0;
+		if (dai->active == 1) {
+			dai->rate = 0;
+			dai->channels = 0;
+			dai->sample_bits = 0;
 		}
-	}
-
-	/* apply codec digital mute */
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		int active = codec_dai->stream_active[substream->stream];
-
-		if (active == 1)
-			snd_soc_dai_digital_mute(codec_dai, 1,
-						 substream->stream);
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		int active = cpu_dai->stream_active[substream->stream];
 
 		if (active == 1)
-			snd_soc_dai_digital_mute(cpu_dai, 1,
-						 substream->stream);
+			snd_soc_dai_digital_mute(dai, 1, substream->stream);
 	}
 
 	/* free any machine hw params */
@@ -1264,18 +1178,11 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
 	soc_pcm_components_hw_free(substream, NULL);
 
 	/* now free hw params for the DAIs  */
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		if (!snd_soc_dai_stream_valid(codec_dai, substream->stream))
+	for_each_rtd_dais(rtd, i, dai) {
+		if (!snd_soc_dai_stream_valid(dai, substream->stream))
 			continue;
 
-		snd_soc_dai_hw_free(codec_dai, substream);
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		if (!snd_soc_dai_stream_valid(cpu_dai, substream->stream))
-			continue;
-
-		snd_soc_dai_hw_free(cpu_dai, substream);
+		snd_soc_dai_hw_free(dai, substream);
 	}
 
 	mutex_unlock(&rtd->card->pcm_mutex);
@@ -1286,8 +1193,7 @@ static int soc_pcm_trigger_start(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i, ret;
 
 	ret = soc_rtd_trigger(rtd, substream, cmd);
@@ -1300,14 +1206,8 @@ static int soc_pcm_trigger_start(struct snd_pcm_substream *substream, int cmd)
 			return ret;
 	}
 
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_trigger(cpu_dai, substream, cmd);
-		if (ret < 0)
-			return ret;
-	}
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_trigger(codec_dai, substream, cmd);
+	for_each_rtd_dais(rtd, i, dai) {
+		ret = snd_soc_dai_trigger(dai, substream, cmd);
 		if (ret < 0)
 			return ret;
 	}
@@ -1319,18 +1219,11 @@ static int soc_pcm_trigger_stop(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct snd_soc_component *component;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i, ret;
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_trigger(codec_dai, substream, cmd);
-		if (ret < 0)
-			return ret;
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_trigger(cpu_dai, substream, cmd);
+	for_each_rtd_dais(rtd, i, dai) {
+		ret = snd_soc_dai_trigger(dai, substream, cmd);
 		if (ret < 0)
 			return ret;
 	}
@@ -1374,18 +1267,11 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
 				   int cmd)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	int i, ret;
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		ret = snd_soc_dai_bespoke_trigger(codec_dai, substream, cmd);
-		if (ret < 0)
-			return ret;
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		ret = snd_soc_dai_bespoke_trigger(cpu_dai, substream, cmd);
+	for_each_rtd_dais(rtd, i, dai) {
+		ret = snd_soc_dai_bespoke_trigger(dai, substream, cmd);
 		if (ret < 0)
 			return ret;
 	}
@@ -1544,7 +1430,7 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card,
 		if (!be->dai_link->no_pcm)
 			continue;
 
-		for_each_rtd_cpu_dais(be, i, dai) {
+		for_each_rtd_dais(be, i, dai) {
 			w = snd_soc_dai_get_widget(dai, stream);
 
 			dev_dbg(card->dev, "ASoC: try BE : %s\n",
@@ -1553,13 +1439,6 @@ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card,
 			if (w == widget)
 				return be;
 		}
-
-		for_each_rtd_codec_dais(be, i, dai) {
-			w = snd_soc_dai_get_widget(dai, stream);
-
-			if (w == widget)
-				return be;
-		}
 	}
 
 	/* Widget provided is not a BE */
@@ -1640,13 +1519,13 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream,
 	for_each_dpcm_be(fe, stream, dpcm) {
 		unsigned int i;
 
-		/* is there a valid CPU DAI widget for this BE */
+		/* is there a valid DAI widget for this BE */
 		do_prune = 1;
-		for_each_rtd_cpu_dais(dpcm->be, i, dai) {
+		for_each_rtd_dais(dpcm->be, i, dai) {
 			widget = snd_soc_dai_get_widget(dai, stream);
 
 			/*
-			 * The BE is pruned only if none of the cpu_dai
+			 * The BE is pruned only if none of the DAI
 			 * widgets are in the active list.
 			 */
 			if (widget && widget_in_list(list, widget))
@@ -1655,18 +1534,6 @@ static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream,
 		if (!do_prune)
 			continue;
 
-		/* is there a valid CODEC DAI widget for this BE */
-		do_prune = 1;
-		for_each_rtd_codec_dais(dpcm->be, i, dai) {
-			widget = snd_soc_dai_get_widget(dai, stream);
-
-			/* prune the BE if it's no longer in our active list */
-			if (widget && widget_in_list(list, widget))
-				do_prune = 0;
-		}
-		if (!do_prune)
-			continue;
-
 		dev_dbg(fe->dev, "ASoC: pruning %s BE %s for %s\n",
 			stream ? "capture" : "playback",
 			dpcm->be->dai_link->name, fe->dai_link->name);
@@ -1998,43 +1865,23 @@ static void dpcm_runtime_merge_rate(struct snd_pcm_substream *substream,
 
 	for_each_dpcm_be(fe, stream, dpcm) {
 		struct snd_soc_pcm_runtime *be = dpcm->be;
-		struct snd_soc_pcm_stream *codec_stream;
-		struct snd_soc_pcm_stream *cpu_stream;
+		struct snd_soc_pcm_stream *pcm;
 		struct snd_soc_dai *dai;
 		int i;
 
-		for_each_rtd_cpu_dais(be, i, dai) {
+		for_each_rtd_dais(be, i, dai) {
 			/*
-			 * Skip CPUs which don't support the current stream
+			 * Skip DAIs which don't support the current stream
 			 * type. See soc_pcm_init_runtime_hw() for more details
 			 */
 			if (!snd_soc_dai_stream_valid(dai, stream))
 				continue;
 
-			cpu_stream = snd_soc_dai_get_pcm_stream(dai, stream);
+			pcm = snd_soc_dai_get_pcm_stream(dai, stream);
 
-			*rate_min = max(*rate_min, cpu_stream->rate_min);
-			*rate_max = min_not_zero(*rate_max,
-						 cpu_stream->rate_max);
-			*rates = snd_pcm_rate_mask_intersect(*rates,
-						cpu_stream->rates);
-		}
-
-		for_each_rtd_codec_dais(be, i, dai) {
-			/*
-			 * Skip CODECs which don't support the current stream
-			 * type. See soc_pcm_init_runtime_hw() for more details
-			 */
-			if (!snd_soc_dai_stream_valid(dai, stream))
-				continue;
-
-			codec_stream = snd_soc_dai_get_pcm_stream(dai, stream);
-
-			*rate_min = max(*rate_min, codec_stream->rate_min);
-			*rate_max = min_not_zero(*rate_max,
-						 codec_stream->rate_max);
-			*rates = snd_pcm_rate_mask_intersect(*rates,
-						codec_stream->rates);
+			*rate_min = max(*rate_min, pcm->rate_min);
+			*rate_max = min_not_zero(*rate_max, pcm->rate_max);
+			*rates = snd_pcm_rate_mask_intersect(*rates, pcm->rates);
 		}
 	}
 }
@@ -2117,8 +1964,7 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
 		struct snd_pcm_substream *be_substream =
 			snd_soc_dpcm_get_substream(be, stream);
 		struct snd_soc_pcm_runtime *rtd;
-		struct snd_soc_dai *codec_dai;
-		struct snd_soc_dai *cpu_dai;
+		struct snd_soc_dai *dai;
 		int i;
 
 		/* A backend may not have the requested substream */
@@ -2133,19 +1979,9 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream,
 			be_substream->runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX;
 
 		/* Symmetry only applies if we've got an active stream. */
-		for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-			if (cpu_dai->active) {
-				err = soc_pcm_apply_symmetry(fe_substream,
-							     cpu_dai);
-				if (err < 0)
-					return err;
-			}
-		}
-
-		for_each_rtd_codec_dais(rtd, i, codec_dai) {
-			if (codec_dai->active) {
-				err = soc_pcm_apply_symmetry(fe_substream,
-							     codec_dai);
+		for_each_rtd_dais(rtd, i, dai) {
+			if (dai->active) {
+				err = soc_pcm_apply_symmetry(fe_substream, dai);
 				if (err < 0)
 					return err;
 			}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/7] ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
                   ` (3 preceding siblings ...)
  2020-03-11  1:07 ` [PATCH 4/7] ASoC: soc-pcm: " Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 6/7] ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb() Kuninori Morimoto
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Now CPU/Codec DAIs are alias for dais.
Thus, we can directly use for_each_rtd_dais() macro
for soc_dai_pcm_new().
This patch merge CPU/Codec for it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 333cbbd268b4..38c2b0434f8f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1037,20 +1037,20 @@ int snd_soc_add_pcm_runtime(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_add_pcm_runtime);
 
-static int soc_dai_pcm_new(struct snd_soc_dai **dais, int num_dais,
-			   struct snd_soc_pcm_runtime *rtd)
+static int soc_dai_pcm_new(struct snd_soc_pcm_runtime *rtd)
 {
+	struct snd_soc_dai *dai;
 	int i, ret = 0;
 
-	for (i = 0; i < num_dais; ++i) {
-		struct snd_soc_dai_driver *drv = dais[i]->driver;
+	for_each_rtd_dais(rtd, i, dai) {
+		struct snd_soc_dai_driver *drv = dai->driver;
 
 		if (drv->pcm_new)
-			ret = drv->pcm_new(rtd, dais[i]);
+			ret = drv->pcm_new(rtd, dai);
 		if (ret < 0) {
-			dev_err(dais[i]->dev,
+			dev_err(dai->dev,
 				"ASoC: Failed to bind %s with pcm device\n",
-				dais[i]->name);
+				dai->name);
 			return ret;
 		}
 	}
@@ -1121,13 +1121,8 @@ static int soc_init_pcm_runtime(struct snd_soc_card *card,
 			dai_link->stream_name, ret);
 		return ret;
 	}
-	ret = soc_dai_pcm_new(rtd->cpu_dais,
-			      rtd->num_cpus, rtd);
-	if (ret < 0)
-		return ret;
-	ret = soc_dai_pcm_new(rtd->codec_dais,
-			      rtd->num_codecs, rtd);
-	return ret;
+
+	return soc_dai_pcm_new(rtd);
 }
 
 static void soc_set_name_prefix(struct snd_soc_card *card,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/7] ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
                   ` (4 preceding siblings ...)
  2020-03-11  1:07 ` [PATCH 5/7] ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new() Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  1:07 ` [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer() Kuninori Morimoto
  2020-03-11  1:51 ` [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Pierre-Louis Bossart
  7 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_apply_msb() is setting msb for both CPU/Codec,
but we can merge these into one.
This patch do it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 26 +++++++-------------------
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index dae1821c78dc..c0f318699fe4 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -530,35 +530,23 @@ static void soc_pcm_set_msb(struct snd_pcm_substream *substream, int bits)
 static void soc_pcm_apply_msb(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
-	struct snd_soc_pcm_stream *pcm_codec, *pcm_cpu;
+	struct snd_soc_dai *dai;
+	struct snd_soc_pcm_stream *pcm;
 	int stream = substream->stream;
 	int i;
-	unsigned int bits = 0, cpu_bits = 0;
+	unsigned int bits = 0;
 
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		pcm_codec = snd_soc_dai_get_pcm_stream(codec_dai, stream);
+	for_each_rtd_dais(rtd, i, dai) {
+		pcm = snd_soc_dai_get_pcm_stream(dai, stream);
 
-		if (pcm_codec->sig_bits == 0) {
+		if (pcm->sig_bits == 0) {
 			bits = 0;
 			break;
 		}
-		bits = max(pcm_codec->sig_bits, bits);
-	}
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		pcm_cpu = snd_soc_dai_get_pcm_stream(cpu_dai, stream);
-
-		if (pcm_cpu->sig_bits == 0) {
-			cpu_bits = 0;
-			break;
-		}
-		cpu_bits = max(pcm_cpu->sig_bits, cpu_bits);
+		bits = max(pcm->sig_bits, bits);
 	}
 
 	soc_pcm_set_msb(substream, bits);
-	soc_pcm_set_msb(substream, cpu_bits);
 }
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
                   ` (5 preceding siblings ...)
  2020-03-11  1:07 ` [PATCH 6/7] ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb() Kuninori Morimoto
@ 2020-03-11  1:07 ` Kuninori Morimoto
  2020-03-11  2:00   ` Pierre-Louis Bossart
  2020-03-11  1:51 ` [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Pierre-Louis Bossart
  7 siblings, 1 reply; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:07 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA


From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_pcm_pointer() is getting eash DAI's delay via snd_soc_dai_delay(),
but, it is adding CPU delay and Codec delay.
We need is not "added delay", but "max delay" of CPU/Codec.
This patch finds maximum delay from CPU/Codec.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-pcm.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index c0f318699fe4..675de7c0eaa4 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -1274,13 +1274,10 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
 static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct snd_soc_dai *cpu_dai;
-	struct snd_soc_dai *codec_dai;
+	struct snd_soc_dai *dai;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	snd_pcm_uframes_t offset = 0;
-	snd_pcm_sframes_t delay = 0;
-	snd_pcm_sframes_t codec_delay = 0;
-	snd_pcm_sframes_t cpu_delay = 0;
+	snd_pcm_sframes_t add_delay = 0;
 	int i;
 
 	/* clearing the previous total delay */
@@ -1288,22 +1285,12 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
 
 	offset = snd_soc_pcm_component_pointer(substream);
 
-	/* base delay if assigned in pointer callback */
-	delay = runtime->delay;
-
-	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
-		cpu_delay = max(cpu_delay,
-				snd_soc_dai_delay(cpu_dai, substream));
-	}
-	delay += cpu_delay;
-
-	for_each_rtd_codec_dais(rtd, i, codec_dai) {
-		codec_delay = max(codec_delay,
-				  snd_soc_dai_delay(codec_dai, substream));
-	}
-	delay += codec_delay;
+	for_each_rtd_dais(rtd, i, dai)
+		add_delay = max(add_delay,
+				snd_soc_dai_delay(dai, substream));
 
-	runtime->delay = delay;
+	/* base delay if assigned in pointer callback */
+	runtime->delay += add_delay;
 
 	return offset;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] ASoC: soc-dapm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:07 ` [PATCH 3/7] ASoC: soc-dapm: " Kuninori Morimoto
@ 2020-03-11  1:27   ` Sridharan, Ranjani
  2020-03-11  1:30     ` Kuninori Morimoto
  0 siblings, 1 reply; 19+ messages in thread
From: Sridharan, Ranjani @ 2020-03-11  1:27 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown

On Tue, Mar 10, 2020 at 6:10 PM Kuninori Morimoto <
kuninori.morimoto.gx@renesas.com> wrote:

>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Now we can use for_each_rtd_dais().
> Let's use it instead of for_each_rtd_cpu/codec_dais().
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-dapm.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
> index e00a465a7c32..3a3fbf167383 100644
> --- a/sound/soc/soc-dapm.c
> +++ b/sound/soc/soc-dapm.c
> @@ -4433,14 +4433,11 @@ void snd_soc_dapm_connect_dai_link_widgets(struct
> snd_soc_card *card)
>  static void soc_dapm_stream_event(struct snd_soc_pcm_runtime *rtd, int
> stream,
>         int event)
>  {
> -       struct snd_soc_dai *codec_dai;
> -       struct snd_soc_dai *cpu_dai;
> +       struct snd_soc_dai *dai;
>         int i;
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               soc_dapm_dai_stream_event(cpu_dai, stream, event);
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               soc_dapm_dai_stream_event(codec_dai, stream, event);
> +       for_each_rtd_cpu_dais(rtd, i, dai)
>
typo here? should be for_each_rtd_dais()?
Thanks,
Ranjani

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/7] ASoC: soc-dapm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:27   ` Sridharan, Ranjani
@ 2020-03-11  1:30     ` Kuninori Morimoto
  0 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  1:30 UTC (permalink / raw)
  To: Sridharan, Ranjani; +Cc: Linux-ALSA, Mark Brown


Hi Sridharan

Thank you for your review

>     From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>    
>     Now we can use for_each_rtd_dais().
>     Let's use it instead of for_each_rtd_cpu/codec_dais().
>    
>     Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>     ---
(snip)
>     -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
>     -               soc_dapm_dai_stream_event(cpu_dai, stream, event);
>     -       for_each_rtd_codec_dais(rtd, i, codec_dai)
>     -               soc_dapm_dai_stream_event(codec_dai, stream, event);
>     +       for_each_rtd_cpu_dais(rtd, i, dai)
> 
> typo here? should be for_each_rtd_dais()?

Oops !? Indeed.
Thank you for pointing it.
Will fix in v2

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:07 ` [PATCH 4/7] ASoC: soc-pcm: " Kuninori Morimoto
@ 2020-03-11  1:30   ` Sridharan, Ranjani
  2020-03-11  2:06     ` Kuninori Morimoto
  0 siblings, 1 reply; 19+ messages in thread
From: Sridharan, Ranjani @ 2020-03-11  1:30 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown

On Tue, Mar 10, 2020 at 6:10 PM Kuninori Morimoto <
kuninori.morimoto.gx@renesas.com> wrote:

>
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> Now we can use for_each_rtd_dais().
> Let's use it instead of for_each_rtd_cpu/codec_dais().
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  sound/soc/soc-pcm.c | 314 +++++++++++---------------------------------
>  1 file changed, 75 insertions(+), 239 deletions(-)
>
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 733d7e8a0e55..dae1821c78dc 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -259,25 +259,15 @@ static int soc_rtd_trigger(struct
> snd_soc_pcm_runtime *rtd,
>  static void snd_soc_runtime_action(struct snd_soc_pcm_runtime *rtd,
>                                    int stream, int action)
>  {
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i;
>
>         lockdep_assert_held(&rtd->card->pcm_mutex);
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               cpu_dai->stream_active[stream] += action;
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               codec_dai->stream_active[stream] += action;
> -
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               cpu_dai->active += action;
> -               cpu_dai->component->active += action;
> -       }
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               codec_dai->active += action;
> -               codec_dai->component->active += action;
> +       for_each_rtd_dais(rtd, i, dai) {
> +               dai->stream_active[stream] += action;
> +               dai->active += action;
> +               dai->component->active += action;
>         }
>  }
>
> @@ -444,8 +434,8 @@ static int soc_pcm_params_symmetry(struct
> snd_pcm_substream *substream,
>                                 struct snd_pcm_hw_params *params)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +       struct snd_soc_dai *dai;
>         struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
>         unsigned int rate, channels, sample_bits, symmetry, i;
>
>         rate = params_rate(params);
> @@ -455,11 +445,8 @@ static int soc_pcm_params_symmetry(struct
> snd_pcm_substream *substream,
>         /* reject unmatched parameters when applying symmetry */
>         symmetry = rtd->dai_link->symmetric_rates;
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               symmetry |= cpu_dai->driver->symmetric_rates;
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               symmetry |= codec_dai->driver->symmetric_rates;
> +       for_each_rtd_cpu_dais(rtd, i, dai)
> +               symmetry |= dai->driver->symmetric_rates;
>
>         if (symmetry) {
>                 for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> @@ -473,11 +460,8 @@ static int soc_pcm_params_symmetry(struct
> snd_pcm_substream *substream,
>
>         symmetry = rtd->dai_link->symmetric_channels;
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               symmetry |= cpu_dai->driver->symmetric_channels;
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               symmetry |= codec_dai->driver->symmetric_channels;
> +       for_each_rtd_dais(rtd, i, dai)
> +               symmetry |= dai->driver->symmetric_channels;
>
>         if (symmetry) {
>                 for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> @@ -492,11 +476,8 @@ static int soc_pcm_params_symmetry(struct
> snd_pcm_substream *substream,
>
>         symmetry = rtd->dai_link->symmetric_samplebits;
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               symmetry |= cpu_dai->driver->symmetric_samplebits;
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               symmetry |= codec_dai->driver->symmetric_samplebits;
> +       for_each_rtd_dais(rtd, i, dai)
> +               symmetry |= dai->driver->symmetric_samplebits;
>
>         if (symmetry) {
>                 for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> @@ -516,25 +497,18 @@ static bool soc_pcm_has_symmetry(struct
> snd_pcm_substream *substream)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_dai_link *link = rtd->dai_link;
> -       struct snd_soc_dai *codec_dai;
> -       struct snd_soc_dai *cpu_dai;
> +       struct snd_soc_dai *dai;
>         unsigned int symmetry, i;
>
>         symmetry = link->symmetric_rates ||
>                 link->symmetric_channels ||
>                 link->symmetric_samplebits;
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               symmetry = symmetry ||
> -                       cpu_dai->driver->symmetric_rates ||
> -                       cpu_dai->driver->symmetric_channels ||
> -                       cpu_dai->driver->symmetric_samplebits;
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> +       for_each_rtd_dais(rtd, i, dai)
>                 symmetry = symmetry ||
> -                       codec_dai->driver->symmetric_rates ||
> -                       codec_dai->driver->symmetric_channels ||
> -                       codec_dai->driver->symmetric_samplebits;
> +                       dai->driver->symmetric_rates ||
> +                       dai->driver->symmetric_channels ||
> +                       dai->driver->symmetric_samplebits;
>
>         return symmetry;
>  }
> @@ -772,19 +746,15 @@ static int soc_pcm_close(struct snd_pcm_substream
> *substream)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_component *component;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i;
>
>         mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
>
>         snd_soc_runtime_deactivate(rtd, substream->stream);
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               snd_soc_dai_shutdown(cpu_dai, substream);
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               snd_soc_dai_shutdown(codec_dai, substream);
> +       for_each_rtd_dais(rtd, i, dai)
> +               snd_soc_dai_shutdown(dai, substream);
>
>         soc_rtd_shutdown(rtd, substream);
>
> @@ -816,8 +786,7 @@ static int soc_pcm_open(struct snd_pcm_substream
> *substream)
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_pcm_runtime *runtime = substream->runtime;
>         struct snd_soc_component *component;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         const char *codec_dai_name = "multicodec";
>         const char *cpu_dai_name = "multicpu";
>         int i, ret = 0;
> @@ -842,28 +811,19 @@ static int soc_pcm_open(struct snd_pcm_substream
> *substream)
>         }
>
>         /* startup the audio subsystem */
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               ret = snd_soc_dai_startup(cpu_dai, substream);
> -               if (ret < 0) {
> -                       dev_err(cpu_dai->dev, "ASoC: can't open interface
> %s: %d\n",
> -                               cpu_dai->name, ret);
> -                       goto cpu_dai_err;
> -               }
> -       }
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               ret = snd_soc_dai_startup(codec_dai, substream);
> +       for_each_rtd_dais(rtd, i, dai) {
> +               ret = snd_soc_dai_startup(dai, substream);
>                 if (ret < 0) {
> -                       dev_err(codec_dai->dev,
> -                               "ASoC: can't open codec %s: %d\n",
> -                               codec_dai->name, ret);
> +                       dev_err(dai->dev,
> +                               "ASoC: can't open DAI %s: %d\n",
> +                               dai->name, ret);
>                         goto config_err;
>                 }
>
>                 if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> -                       codec_dai->tx_mask = 0;
> +                       dai->tx_mask = 0;
>                 else
> -                       codec_dai->rx_mask = 0;
> +                       dai->rx_mask = 0;
>         }
>
>         /* Dynamic PCM DAI links compat checks use dynamic capabilities */
> @@ -903,17 +863,9 @@ static int soc_pcm_open(struct snd_pcm_substream
> *substream)
>         soc_pcm_apply_msb(substream);
>
>         /* Symmetry only applies if we've already got an active stream. */
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               if (cpu_dai->active) {
> -                       ret = soc_pcm_apply_symmetry(substream, cpu_dai);
> -                       if (ret != 0)
> -                               goto config_err;
> -               }
> -       }
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               if (codec_dai->active) {
> -                       ret = soc_pcm_apply_symmetry(substream, codec_dai);
> +       for_each_rtd_dais(rtd, i, dai) {
> +               if (dai->active) {
> +                       ret = soc_pcm_apply_symmetry(substream, dai);
>                         if (ret != 0)
>                                 goto config_err;
>                 }
> @@ -935,11 +887,8 @@ static int soc_pcm_open(struct snd_pcm_substream
> *substream)
>         return 0;
>
>  config_err:
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               snd_soc_dai_shutdown(codec_dai, substream);
> -cpu_dai_err:
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               snd_soc_dai_shutdown(cpu_dai, substream);
> +       for_each_rtd_dais(rtd, i, dai)
> +               snd_soc_dai_shutdown(dai, substream);
>
>         soc_rtd_shutdown(rtd, substream);
>  rtd_startup_err:
> @@ -978,8 +927,7 @@ static int soc_pcm_prepare(struct snd_pcm_substream
> *substream)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_component *component;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i, ret = 0;
>
>         mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
> @@ -1000,21 +948,11 @@ static int soc_pcm_prepare(struct snd_pcm_substream
> *substream)
>                 }
>         }
>
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               ret = snd_soc_dai_prepare(codec_dai, substream);
> -               if (ret < 0) {
> -                       dev_err(codec_dai->dev,
> -                               "ASoC: codec DAI prepare error: %d\n",
> -                               ret);
> -                       goto out;
> -               }
> -       }
> -
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               ret = snd_soc_dai_prepare(cpu_dai, substream);
> +       for_each_rtd_dais(rtd, i, dai) {
> +               ret = snd_soc_dai_prepare(dai, substream);
>                 if (ret < 0) {
> -                       dev_err(cpu_dai->dev,
> -                               "ASoC: cpu DAI prepare error: %d\n", ret);
> +                       dev_err(dai->dev,
> +                               "ASoC: DAI prepare error: %d\n", ret);
>                         goto out;
>                 }
>         }
> @@ -1029,11 +967,8 @@ static int soc_pcm_prepare(struct snd_pcm_substream
> *substream)
>         snd_soc_dapm_stream_event(rtd, substream->stream,
>                         SND_SOC_DAPM_STREAM_START);
>
> -       for_each_rtd_codec_dais(rtd, i, codec_dai)
> -               snd_soc_dai_digital_mute(codec_dai, 0,
> -                                        substream->stream);
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> -               snd_soc_dai_digital_mute(cpu_dai, 0, substream->stream);
> +       for_each_rtd_dais(rtd, i, dai)
> +               snd_soc_dai_digital_mute(dai, 0, substream->stream);
>
>  out:
>         mutex_unlock(&rtd->card->pcm_mutex);
> @@ -1217,44 +1152,23 @@ static int soc_pcm_hw_params(struct
> snd_pcm_substream *substream,
>  static int soc_pcm_hw_free(struct snd_pcm_substream *substream)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i;
>
>         mutex_lock_nested(&rtd->card->pcm_mutex, rtd->card->pcm_subclass);
>
>         /* clear the corresponding DAIs parameters when going to be
> inactive */
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               if (cpu_dai->active == 1) {
> -                       cpu_dai->rate = 0;
> -                       cpu_dai->channels = 0;
> -                       cpu_dai->sample_bits = 0;
> -               }
> -       }
> +       for_each_rtd_dais(rtd, i, dai) {
> +               int active = dai->stream_active[substream->stream];
>
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               if (codec_dai->active == 1) {
> -                       codec_dai->rate = 0;
> -                       codec_dai->channels = 0;
> -                       codec_dai->sample_bits = 0;
> +               if (dai->active == 1) {
> +                       dai->rate = 0;
> +                       dai->channels = 0;
> +                       dai->sample_bits = 0;
>                 }
> -       }
> -
> -       /* apply codec digital mute */
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               int active = codec_dai->stream_active[substream->stream];
> -
> -               if (active == 1)
> -                       snd_soc_dai_digital_mute(codec_dai, 1,
> -                                                substream->stream);
> -       }
> -
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               int active = cpu_dai->stream_active[substream->stream];
>
>                 if (active == 1)
> -                       snd_soc_dai_digital_mute(cpu_dai, 1,
> -                                                substream->stream);
> +                       snd_soc_dai_digital_mute(dai, 1,
> substream->stream);
>         }
>
>         /* free any machine hw params */
> @@ -1264,18 +1178,11 @@ static int soc_pcm_hw_free(struct
> snd_pcm_substream *substream)
>         soc_pcm_components_hw_free(substream, NULL);
>
>         /* now free hw params for the DAIs  */
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               if (!snd_soc_dai_stream_valid(codec_dai,
> substream->stream))
> +       for_each_rtd_dais(rtd, i, dai) {
> +               if (!snd_soc_dai_stream_valid(dai, substream->stream))
>                         continue;
>
> -               snd_soc_dai_hw_free(codec_dai, substream);
> -       }
> -
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               if (!snd_soc_dai_stream_valid(cpu_dai, substream->stream))
> -                       continue;
> -
> -               snd_soc_dai_hw_free(cpu_dai, substream);
> +               snd_soc_dai_hw_free(dai, substream);
>         }
>
>         mutex_unlock(&rtd->card->pcm_mutex);
> @@ -1286,8 +1193,7 @@ static int soc_pcm_trigger_start(struct
> snd_pcm_substream *substream, int cmd)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_component *component;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i, ret;
>
>         ret = soc_rtd_trigger(rtd, substream, cmd);
> @@ -1300,14 +1206,8 @@ static int soc_pcm_trigger_start(struct
> snd_pcm_substream *substream, int cmd)
>                         return ret;
>         }
>
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               ret = snd_soc_dai_trigger(cpu_dai, substream, cmd);
> -               if (ret < 0)
> -                       return ret;
> -       }
> -
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               ret = snd_soc_dai_trigger(codec_dai, substream, cmd);
> +       for_each_rtd_dais(rtd, i, dai) {
> +               ret = snd_soc_dai_trigger(dai, substream, cmd);
>                 if (ret < 0)
>                         return ret;
>         }
> @@ -1319,18 +1219,11 @@ static int soc_pcm_trigger_stop(struct
> snd_pcm_substream *substream, int cmd)
>  {
>         struct snd_soc_pcm_runtime *rtd = substream->private_data;
>         struct snd_soc_component *component;
> -       struct snd_soc_dai *cpu_dai;
> -       struct snd_soc_dai *codec_dai;
> +       struct snd_soc_dai *dai;
>         int i, ret;
>
> -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -               ret = snd_soc_dai_trigger(codec_dai, substream, cmd);
> -               if (ret < 0)
> -                       return ret;
> -       }
> -
> -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -               ret = snd_soc_dai_trigger(cpu_dai, substream, cmd);
> +       for_each_rtd_dais(rtd, i, dai) {
>
Morimoto-san,

We are switching the order in which the codec dais and cpu dais are stopped
here with this new macro no. Does it make a difference? The same comment
applies to some other changes as well.

If the trigger_start() started cpu dais first and then codec dais, do we
need to stop in the reverse order?

Thanks,
Ranjani

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
  2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
                   ` (6 preceding siblings ...)
  2020-03-11  1:07 ` [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer() Kuninori Morimoto
@ 2020-03-11  1:51 ` Pierre-Louis Bossart
  2020-03-11  2:27   ` Kuninori Morimoto
  7 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-11  1:51 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA

Hi Morimoto-san,

On 3/10/20 8:06 PM, Kuninori Morimoto wrote:
> 
> Hi Mark
> 
> Current ALSA SoC has CPU/Codec categorized DAIs.
> But it will be no longer good much for modern device.
> 
> Currently, rtd has both CPU/Codec DAIs array.
> 
> 	rtd->cpu_dais   = [][][][][][][][][]
> 	rtd->codec_dais = [][][][][][][][][]
> 
> This patch merges these, like below.
> It still keeps rtd->cpu_dais, rtd->codec_dais

Sorry, but I perceive a contradiction here, or I am missing the bigger 
picture.

Is the end-goal to remove the cpu_dais and codec_dais, and fold them as 
non-descript 'dais'? This is what I understand by "it will be no longer 
good much for modern device"

Or is this 'merge' a simple data handling change to avoid using two 
"for" loops instead of one, and we are going to keep the distinction 
between dais?

more specifically I am concerned about the tons of code we have, e.g. a 
random machine driver:

	struct snd_soc_dai *codec_dai = rtd->codec_dai;
	struct snd_soc_jack *jack;
	int ret;

	/* Configure sysclk for codec */
	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK,

If the rtd structure only has an array of dais, how would the codecs be 
configured then?
	
> 
> 	rtd->dais = [][][][][][][][][][][][][][][][][][]
> 	            ^cpu_dais         ^codec_dais
> 	            |--- num_cpus ---|--- num_codecs --|
> 
> After this merging, we can merge for_each_rtd_cpu/codec_dais().
> 
> -	for_each_rtd_cpu_dais() {
> -		...
> -	}
> -	for_each_rtd_codec_dais() {
> -		...
> -	}
> +	for_each_rtd_dais() {
> +		...
> +	}
> 
> 
> Kuninori Morimoto (7):
>    ASoC: soc-core: Merge CPU/Codec DAIs
>    ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais()
>    ASoC: soc-dapm: Merge for_each_rtd_cpu/codec_dais()
>    ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
>    ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new()
>    ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb()
>    ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()
> 
>   include/sound/soc.h  |   7 +-
>   sound/soc/soc-core.c |  66 +++-----
>   sound/soc/soc-dapm.c |   9 +-
>   sound/soc/soc-pcm.c  | 367 +++++++++++--------------------------------
>   4 files changed, 123 insertions(+), 326 deletions(-)
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()
  2020-03-11  1:07 ` [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer() Kuninori Morimoto
@ 2020-03-11  2:00   ` Pierre-Louis Bossart
  2020-03-11  2:39     ` Kuninori Morimoto
  0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-11  2:00 UTC (permalink / raw)
  To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA



On 3/10/20 8:07 PM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> soc_pcm_pointer() is getting eash DAI's delay via snd_soc_dai_delay(),
> but, it is adding CPU delay and Codec delay.
> We need is not "added delay", but "max delay" of CPU/Codec.
> This patch finds maximum delay from CPU/Codec.

The max() used between all cpu or codec dais for the same dailink is 
largely defensive programming, in practice it's not clear to me if we 
have different delays reported by differents codec or cpu_dais. I would 
expect all cpu dais in the same dailink to report the same delay, and 
likewise all codecs dais in the same dailink should provide the same value.

Now doing a max between cpu and codec dais does not seem right to me. 
You may have a delay in a DSP and a delay in a codec, and worst case the 
delay is the total of the two. It wouldn't matter too much with a 
'simple' codec with limited buffering, but the moment the codec itself 
has a DSP and internal buffering this change in accounting would 
introduce a real offset.


> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>   sound/soc/soc-pcm.c | 27 +++++++--------------------
>   1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index c0f318699fe4..675de7c0eaa4 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1274,13 +1274,10 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
>   static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
>   {
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> -	struct snd_soc_dai *cpu_dai;
> -	struct snd_soc_dai *codec_dai;
> +	struct snd_soc_dai *dai;
>   	struct snd_pcm_runtime *runtime = substream->runtime;
>   	snd_pcm_uframes_t offset = 0;
> -	snd_pcm_sframes_t delay = 0;
> -	snd_pcm_sframes_t codec_delay = 0;
> -	snd_pcm_sframes_t cpu_delay = 0;
> +	snd_pcm_sframes_t add_delay = 0;
>   	int i;
>   
>   	/* clearing the previous total delay */
> @@ -1288,22 +1285,12 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
>   
>   	offset = snd_soc_pcm_component_pointer(substream);
>   
> -	/* base delay if assigned in pointer callback */
> -	delay = runtime->delay;
> -
> -	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> -		cpu_delay = max(cpu_delay,
> -				snd_soc_dai_delay(cpu_dai, substream));
> -	}
> -	delay += cpu_delay;
> -
> -	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -		codec_delay = max(codec_delay,
> -				  snd_soc_dai_delay(codec_dai, substream));
> -	}
> -	delay += codec_delay;
> +	for_each_rtd_dais(rtd, i, dai)
> +		add_delay = max(add_delay,
> +				snd_soc_dai_delay(dai, substream));
>   
> -	runtime->delay = delay;
> +	/* base delay if assigned in pointer callback */
> +	runtime->delay += add_delay;
>   
>   	return offset;
>   }
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/7] ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
  2020-03-11  1:30   ` Sridharan, Ranjani
@ 2020-03-11  2:06     ` Kuninori Morimoto
  0 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  2:06 UTC (permalink / raw)
  To: Sridharan, Ranjani; +Cc: Linux-ALSA, Mark Brown


Hi Sridharan

Thank you for reviewing

>     @@ -1319,18 +1219,11 @@ static int soc_pcm_trigger_stop(struct snd_pcm_substream *substream, int
>     cmd)
>      {
>             struct snd_soc_pcm_runtime *rtd = substream->private_data;
>             struct snd_soc_component *component;
>     -       struct snd_soc_dai *cpu_dai;
>     -       struct snd_soc_dai *codec_dai;
>     +       struct snd_soc_dai *dai;
>             int i, ret;
>    
>     -       for_each_rtd_codec_dais(rtd, i, codec_dai) {
>     -               ret = snd_soc_dai_trigger(codec_dai, substream, cmd);
>     -               if (ret < 0)
>     -                       return ret;
>     -       }
>     -
>     -       for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
>     -               ret = snd_soc_dai_trigger(cpu_dai, substream, cmd);
>     +       for_each_rtd_dais(rtd, i, dai) {
> 
> Morimoto-san,
> 
> We are switching the order in which the codec dais and cpu dais are stopped here with this new macro
> no. Does it make a difference? The same comment applies to some other changes as well.
> 
> If the trigger_start() started cpu dais first and then codec dais, do we need to stop in the reverse
> order?

I'm sorry but I didn't mention detail.
Yes, it exchanged the order.

But I'm thinking that the current order was
implemented by just a coincidence,
and I'm hoping hardware doesn't matter order of same layer (= DAI).

Maybe Mark answered already.
https://lore.kernel.org/alsa-devel/20200225014039.GA21366@sirena.org.uk/

But if the order was important for some platform,
we need/should consider "order", like for_each_comp_order().

	for_each_comp_order(order) {
		for_each_rtd_dais(rtd, i, dai, order) {
			...

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
  2020-03-11  1:51 ` [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Pierre-Louis Bossart
@ 2020-03-11  2:27   ` Kuninori Morimoto
  2020-03-11 14:36     ` Pierre-Louis Bossart
  0 siblings, 1 reply; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  2:27 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown


Hi Pierre-Louis

> > 	rtd->cpu_dais   = [][][][][][][][][]
> > 	rtd->codec_dais = [][][][][][][][][]
(snip)
> > 	rtd->dais = [][][][][][][][][][][][][][][][][][]
> > 	            ^cpu_dais         ^codec_dais
> > 	            |--- num_cpus ---|--- num_codecs --|
(snip)
> Is the end-goal to remove the cpu_dais and codec_dais, and fold them
> as non-descript 'dais'? This is what I understand by "it will be no
> longer good much for modern device"

Yes. We want to have non-descript DAIs in the future.
I think this was indicated by Lars-Peter before at ELCE.
But, I think we *can't* do it right now.
Because many drivers are considering CPU and Codec separately.

> Or is this 'merge' a simple data handling change to avoid using two
> "for" loops instead of one, and we are going to keep the distinction
> between dais?

Yes.
There are some functions which is doing something only for CPU or Codec.
This patch-set do nothing to such functions.
Maybe it can be updated in the future, maybe not (can't).

I hope this patch-set can be 1st step for non-descript DAIs.
But the main purpose of this patch so far is that
keeping current CPU / Codec DAIs method,
but, simply merge such code *if possible*.

> more specifically I am concerned about the tons of code we have,
> e.g. a random machine driver:
> 
> 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
> 	struct snd_soc_jack *jack;
> 	int ret;
> 
> 	/* Configure sysclk for codec */
> 	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK,
> 
> If the rtd structure only has an array of dais, how would the codecs
> be configured then?

The image is like this.
It still can use rtd->cpu_dais, rtd->codec_dais.
Of course for_each_rtd_cpu/codec_dais() macro too.

	rtd->dais = [][][][][][][][][][][][][][][][][][]
	            ^cpu_dais         ^codec_dais
	            |--- num_cpus ---|--- num_codecs --|

	rtd->cpu_dais   = &rtd->dais[0];
	rtd->codec_dais = &rtd->dais[dai_link->num_cpus];

So we can use/keep existing code/method same as before.
Is this good answer for you ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()
  2020-03-11  2:00   ` Pierre-Louis Bossart
@ 2020-03-11  2:39     ` Kuninori Morimoto
  0 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-11  2:39 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown


Hi Pierre-Louis

Thank you for your review

> The max() used between all cpu or codec dais for the same dailink is
> largely defensive programming, in practice it's not clear to me if we
> have different delays reported by differents codec or cpu_dais. I
> would expect all cpu dais in the same dailink to report the same
> delay, and likewise all codecs dais in the same dailink should provide
> the same value.
> 
> Now doing a max between cpu and codec dais does not seem right to
> me. You may have a delay in a DSP and a delay in a codec, and worst
> case the delay is the total of the two. It wouldn't matter too much
> with a 'simple' codec with limited buffering, but the moment the codec
> itself has a DSP and internal buffering this change in accounting
> would introduce a real offset.
(snip)
> > -	for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> > -		cpu_delay = max(cpu_delay,
> > -				snd_soc_dai_delay(cpu_dai, substream));
> > -	}
> > -	delay += cpu_delay;
> > -
> > -	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> > -		codec_delay = max(codec_delay,
> > -				  snd_soc_dai_delay(codec_dai, substream));
> > -	}
> > -	delay += codec_delay;
> > +	for_each_rtd_dais(rtd, i, dai)
> > +		add_delay = max(add_delay,
> > +				snd_soc_dai_delay(dai, substream));
> >   -	runtime->delay = delay;
> > +	/* base delay if assigned in pointer callback */
> > +	runtime->delay += add_delay;
> >     	return offset;

Hmm.. ? Indeed...
Why I merged these ??
Thank you for pointing it. This patch is indeed wrong.
Will remove it in v2.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
  2020-03-11  2:27   ` Kuninori Morimoto
@ 2020-03-11 14:36     ` Pierre-Louis Bossart
  2020-03-11 17:10       ` Mark Brown
  0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-11 14:36 UTC (permalink / raw)
  To: Kuninori Morimoto; +Cc: Linux-ALSA, Mark Brown



On 3/10/20 9:27 PM, Kuninori Morimoto wrote:
> 
> Hi Pierre-Louis
> 
>>> 	rtd->cpu_dais   = [][][][][][][][][]
>>> 	rtd->codec_dais = [][][][][][][][][]
> (snip)
>>> 	rtd->dais = [][][][][][][][][][][][][][][][][][]
>>> 	            ^cpu_dais         ^codec_dais
>>> 	            |--- num_cpus ---|--- num_codecs --|
> (snip)
>> Is the end-goal to remove the cpu_dais and codec_dais, and fold them
>> as non-descript 'dais'? This is what I understand by "it will be no
>> longer good much for modern device"
> 
> Yes. We want to have non-descript DAIs in the future.
> I think this was indicated by Lars-Peter before at ELCE.
> But, I think we *can't* do it right now.
> Because many drivers are considering CPU and Codec separately.
> 
>> Or is this 'merge' a simple data handling change to avoid using two
>> "for" loops instead of one, and we are going to keep the distinction
>> between dais?
> 
> Yes.
> There are some functions which is doing something only for CPU or Codec.
> This patch-set do nothing to such functions.
> Maybe it can be updated in the future, maybe not (can't).
> 
> I hope this patch-set can be 1st step for non-descript DAIs.
> But the main purpose of this patch so far is that
> keeping current CPU / Codec DAIs method,
> but, simply merge such code *if possible*.
> 
>> more specifically I am concerned about the tons of code we have,
>> e.g. a random machine driver:
>>
>> 	struct snd_soc_dai *codec_dai = rtd->codec_dai;
>> 	struct snd_soc_jack *jack;
>> 	int ret;
>>
>> 	/* Configure sysclk for codec */
>> 	ret = snd_soc_dai_set_sysclk(codec_dai, DA7219_CLKSRC_MCLK,
>>
>> If the rtd structure only has an array of dais, how would the codecs
>> be configured then?
> 
> The image is like this.
> It still can use rtd->cpu_dais, rtd->codec_dais.
> Of course for_each_rtd_cpu/codec_dais() macro too.
> 
> 	rtd->dais = [][][][][][][][][][][][][][][][][][]
> 	            ^cpu_dais         ^codec_dais
> 	            |--- num_cpus ---|--- num_codecs --|
> 
> 	rtd->cpu_dais   = &rtd->dais[0];
> 	rtd->codec_dais = &rtd->dais[dai_link->num_cpus];
> 
> So we can use/keep existing code/method same as before.
> Is this good answer for you ?

Thanks for the precisions.

I have no objections to the addition of the rtd->dais, it can indeed 
simplify the code by just processing all dais in the same loop.

I would still like to make sure we have an broadbrush idea of what the 
2nd step might be. It seems to me it's not possible to avoid having a 
notion of source/sink inside of a dailink (the wording is probably not 
right e.g. for full duplex, maybe domain1/domain2 component1/component2 
are more accurate). The dais are exposed by different components and are 
really the hook by which the components can be configured with 
compatible settings.

Thanks
-Pierre



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
  2020-03-11 14:36     ` Pierre-Louis Bossart
@ 2020-03-11 17:10       ` Mark Brown
  2020-03-12  1:17         ` Kuninori Morimoto
  0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2020-03-11 17:10 UTC (permalink / raw)
  To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Kuninori Morimoto

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

On Wed, Mar 11, 2020 at 09:36:17AM -0500, Pierre-Louis Bossart wrote:

> I would still like to make sure we have an broadbrush idea of what the 2nd
> step might be. It seems to me it's not possible to avoid having a notion of
> source/sink inside of a dailink (the wording is probably not right e.g. for
> full duplex, maybe domain1/domain2 component1/component2 are more accurate).
> The dais are exposed by different components and are really the hook by
> which the components can be configured with compatible settings.

We at least have the notion of clock master but that's something that
could potentially vary at runtime so not stable.  Like you say finding
appropriate language to talk about these things is tricky and I'm also
struggling to think of anything better than numbered components.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/7] ASoC: Merge CPU/Codec DAIs
  2020-03-11 17:10       ` Mark Brown
@ 2020-03-12  1:17         ` Kuninori Morimoto
  0 siblings, 0 replies; 19+ messages in thread
From: Kuninori Morimoto @ 2020-03-12  1:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: Linux-ALSA, Pierre-Louis Bossart


Hi Mark, Pierre-Louis

Thank you for your feedback

> > I would still like to make sure we have an broadbrush idea of what the 2nd
> > step might be. It seems to me it's not possible to avoid having a notion of
> > source/sink inside of a dailink (the wording is probably not right e.g. for
> > full duplex, maybe domain1/domain2 component1/component2 are more accurate).
> > The dais are exposed by different components and are really the hook by
> > which the components can be configured with compatible settings.
> 
> We at least have the notion of clock master but that's something that
> could potentially vary at runtime so not stable.  Like you say finding
> appropriate language to talk about these things is tricky and I'm also
> struggling to think of anything better than numbered components.

If my understanding was correct,
Lars-Peter's presentation at ELCE was using idea of "Bridge".
This idea is not exist in current ALSA SoC,
but current DAI can be updated to it ?
Then, current notion can be udated also?
I'm not 100% understanding how to update,
how to use, etc, but my Image is...

	sink     source
	+---+	 +---+
	|DAI|--->|DAI|
	+---+	 +---+

	    ---->
	+------------+
	|   Bridge   |
	+------------+

Thank you for your help !!

Best regards
---
Kuninori Morimoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-03-12  1:18 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11  1:06 [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 1/7] ASoC: soc-core: " Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 2/7] ASoC: soc-core: Merge for_each_rtd_cpu/codec_dais() Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 3/7] ASoC: soc-dapm: " Kuninori Morimoto
2020-03-11  1:27   ` Sridharan, Ranjani
2020-03-11  1:30     ` Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 4/7] ASoC: soc-pcm: " Kuninori Morimoto
2020-03-11  1:30   ` Sridharan, Ranjani
2020-03-11  2:06     ` Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 5/7] ASoC: soc-core: Merge CPU/Codec for soc_dai_pcm_new() Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 6/7] ASoC: soc-pcm: Merge CPU/Codec MSB at soc_pcm_apply_msb() Kuninori Morimoto
2020-03-11  1:07 ` [PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer() Kuninori Morimoto
2020-03-11  2:00   ` Pierre-Louis Bossart
2020-03-11  2:39     ` Kuninori Morimoto
2020-03-11  1:51 ` [PATCH 0/7] ASoC: Merge CPU/Codec DAIs Pierre-Louis Bossart
2020-03-11  2:27   ` Kuninori Morimoto
2020-03-11 14:36     ` Pierre-Louis Bossart
2020-03-11 17:10       ` Mark Brown
2020-03-12  1:17         ` Kuninori Morimoto

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.