[PATCH 0/7] ASoC: Merge CPU/Codec DAIs
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(-)
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 */
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; }
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); }
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
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
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; }
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
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
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,
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); }
/**
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; }
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; }
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
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(-)
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
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
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.
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
participants (4)
-
Kuninori Morimoto
-
Mark Brown
-
Pierre-Louis Bossart
-
Sridharan, Ranjani