All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Sridharan, Ranjani" <ranjani.sridharan@intel.com>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH 4/7] ASoC: soc-pcm: Merge for_each_rtd_cpu/codec_dais()
Date: Tue, 10 Mar 2020 18:30:29 -0700	[thread overview]
Message-ID: <CAFQqKeXR-ymj-5Xz=+LEKbPP1_eqVfoLNF5a88Yf10An3FdoFw@mail.gmail.com> (raw)
In-Reply-To: <87sgifvgvj.wl-kuninori.morimoto.gx@renesas.com>

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

  reply	other threads:[~2020-03-11  1:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFQqKeXR-ymj-5Xz=+LEKbPP1_eqVfoLNF5a88Yf10An3FdoFw@mail.gmail.com' \
    --to=ranjani.sridharan@intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kuninori.morimoto.gx@renesas.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.