From e04e7b8ccd4912e6c823bf7e66f302a53396fb77 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 22 Jan 2021 10:13:32 +0900 Subject: [PATCH 1/5] ASoC: soc-pcm: tidyup pcm setting Current soc_new_pcm() setups pcm randomly. This patch tidyup it. Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/878s8laigt.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 7079a301ec31..d5f1f653ec9b 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2760,8 +2760,8 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) else rtd->close_delayed_work_func = snd_soc_close_delayed_work; - pcm->nonatomic = rtd->dai_link->nonatomic; rtd->pcm = pcm; + pcm->nonatomic = rtd->dai_link->nonatomic; pcm->private_data = rtd; if (rtd->dai_link->no_pcm || rtd->dai_link->params) { From 7fc6bebd5831a788a74e019e39c43c014a96a110 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 22 Jan 2021 10:13:38 +0900 Subject: [PATCH 2/5] ASoC: soc-pcm: add soc_get_playback_capture() and simplify soc_new_pcm() soc_new_pcm() implementation is very long / verbose / complex, thus, it is very difficult to read. If we read it carefully, we can notice that it is consisted by int soc_new_pcm(...) { (1) judging playback/caputre part (2) creating the PCM part (3) setup pcm/rtd part } This patch adds new soc_get_playback_capture() for (1) part and offload it from soc_new_pcm(). Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/877do5aign.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index d5f1f653ec9b..46818b3319f6 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2631,15 +2631,11 @@ open_end: return ret; } -/* create a new pcm */ -int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) +static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, + int *playback, int *capture) { struct snd_soc_dai *codec_dai; struct snd_soc_dai *cpu_dai; - struct snd_soc_component *component; - struct snd_pcm *pcm; - char new_name[64]; - int ret = 0, playback = 0, capture = 0; int stream; int i; @@ -2655,12 +2651,11 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) for_each_rtd_cpu_dais(rtd, i, cpu_dai) { if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - playback = 1; + *playback = 1; break; } } - - if (!playback) { + if (!*playback) { dev_err(rtd->card->dev, "No CPU DAIs support playback for stream %s\n", rtd->dai_link->stream_name); @@ -2672,12 +2667,12 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) for_each_rtd_cpu_dais(rtd, i, cpu_dai) { if (snd_soc_dai_stream_valid(cpu_dai, stream)) { - capture = 1; + *capture = 1; break; } } - if (!capture) { + if (!*capture) { dev_err(rtd->card->dev, "No CPU DAIs support capture for stream %s\n", rtd->dai_link->stream_name); @@ -2704,23 +2699,39 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_PLAYBACK) && snd_soc_dai_stream_valid(cpu_dai, cpu_playback)) - playback = 1; + *playback = 1; if (snd_soc_dai_stream_valid(codec_dai, SNDRV_PCM_STREAM_CAPTURE) && snd_soc_dai_stream_valid(cpu_dai, cpu_capture)) - capture = 1; + *capture = 1; } } if (rtd->dai_link->playback_only) { - playback = 1; - capture = 0; + *playback = 1; + *capture = 0; } if (rtd->dai_link->capture_only) { - playback = 0; - capture = 1; + *playback = 0; + *capture = 1; } + return 0; +} + +/* create a new pcm */ +int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) +{ + struct snd_soc_component *component; + struct snd_pcm *pcm; + char new_name[64]; + int ret = 0, playback = 0, capture = 0; + int i; + + ret = soc_get_playback_capture(rtd, &playback, &capture); + if (ret < 0) + return ret; + /* create the PCM */ if (rtd->dai_link->params) { snprintf(new_name, sizeof(new_name), "codec2codec(%s)", From 2b39123b134e10a3817156bd9b157c9b8f950d6f Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 22 Jan 2021 10:13:43 +0900 Subject: [PATCH 3/5] ASoC: soc-pcm: add soc_create_pcm() and simplify soc_new_pcm() soc_new_pcm() implementation is very long / verbose / complex, thus, it is very difficult to read. If we read it carefully, we can notice that it is consisted by int soc_new_pcm(...) { (1) judging playback/caputre part (2) creating the PCM part (3) setup pcm/rtd part } This patch adds new soc_create_pcm() for (2) part and offload it from snd_pcm_new(). Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/875z3paigi.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 46818b3319f6..10c5e0beecd8 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2719,18 +2719,12 @@ static int soc_get_playback_capture(struct snd_soc_pcm_runtime *rtd, return 0; } -/* create a new pcm */ -int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) +static int soc_create_pcm(struct snd_pcm **pcm, + struct snd_soc_pcm_runtime *rtd, + int playback, int capture, int num) { - struct snd_soc_component *component; - struct snd_pcm *pcm; char new_name[64]; - int ret = 0, playback = 0, capture = 0; - int i; - - ret = soc_get_playback_capture(rtd, &playback, &capture); - if (ret < 0) - return ret; + int ret; /* create the PCM */ if (rtd->dai_link->params) { @@ -2738,13 +2732,13 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) rtd->dai_link->stream_name); ret = snd_pcm_new_internal(rtd->card->snd_card, new_name, num, - playback, capture, &pcm); + playback, capture, pcm); } else if (rtd->dai_link->no_pcm) { snprintf(new_name, sizeof(new_name), "(%s)", rtd->dai_link->stream_name); ret = snd_pcm_new_internal(rtd->card->snd_card, new_name, num, - playback, capture, &pcm); + playback, capture, pcm); } else { if (rtd->dai_link->dynamic) snprintf(new_name, sizeof(new_name), "%s (*)", @@ -2756,7 +2750,7 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) "multicodec" : asoc_rtd_to_codec(rtd, 0)->name, num); ret = snd_pcm_new(rtd->card->snd_card, new_name, num, playback, - capture, &pcm); + capture, pcm); } if (ret < 0) { dev_err(rtd->card->dev, "ASoC: can't create pcm %s for dailink %s: %d\n", @@ -2765,6 +2759,25 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) } dev_dbg(rtd->card->dev, "ASoC: registered pcm #%d %s\n",num, new_name); + return 0; +} + +/* create a new pcm */ +int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) +{ + struct snd_soc_component *component; + struct snd_pcm *pcm; + int ret = 0, playback = 0, capture = 0; + int i; + + ret = soc_get_playback_capture(rtd, &playback, &capture); + if (ret < 0) + return ret; + + ret = soc_create_pcm(&pcm, rtd, playback, capture, num); + if (ret < 0) + return ret; + /* DAPM dai link stream work */ if (rtd->dai_link->params) rtd->close_delayed_work_func = codec2codec_close_delayed_work; @@ -2825,8 +2838,8 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) ret = snd_soc_pcm_component_new(rtd); if (ret < 0) { - dev_err(rtd->dev, "ASoC: pcm %s constructor failed for dailink %s: %d\n", - new_name, rtd->dai_link->name, ret); + dev_err(rtd->dev, "ASoC: pcm constructor failed for dailink %s: %d\n", + rtd->dai_link->name, ret); return ret; } From 4b260f425497b105acc2baa9d97ef781ef0c667d Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 22 Jan 2021 10:13:48 +0900 Subject: [PATCH 4/5] ASoC: soc-pcm: use snd_pcm_hardware at dpcm_runtime_merge_xxx() soc-pcm has dpcm_runtime_merge_xxx() functions, but uses parameters are very verbose. dpcm_runtime_merge_format(..., &runtime->hw.formats); dpcm_runtime_merge_chan(..., &runtime->hw.channels_min, &runtime->hw.channels_max); dpcm_runtime_merge_rate(..., &runtime->hw.rates, &runtime->hw.rate_min, &runtime->hw.rate_max); We want to replace it into dpcm_runtime_merge_format(..., runtime); dpcm_runtime_merge_chan(..., runtime); dpcm_runtime_merge_rate(..., runtime); This patch do it. Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/874kj9aigd.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 10c5e0beecd8..2a625ce0bacb 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1526,9 +1526,10 @@ static void dpcm_init_runtime_hw(struct snd_pcm_runtime *runtime, } static void dpcm_runtime_merge_format(struct snd_pcm_substream *substream, - u64 *formats) + struct snd_pcm_runtime *runtime) { struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream); + struct snd_pcm_hardware *hw = &runtime->hw; struct snd_soc_dpcm *dpcm; struct snd_soc_dai *dai; int stream = substream->stream; @@ -1556,16 +1557,16 @@ static void dpcm_runtime_merge_format(struct snd_pcm_substream *substream, codec_stream = snd_soc_dai_get_pcm_stream(dai, stream); - *formats &= codec_stream->formats; + hw->formats &= codec_stream->formats; } } } static void dpcm_runtime_merge_chan(struct snd_pcm_substream *substream, - unsigned int *channels_min, - unsigned int *channels_max) + struct snd_pcm_runtime *runtime) { struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream); + struct snd_pcm_hardware *hw = &runtime->hw; struct snd_soc_dpcm *dpcm; int stream = substream->stream; @@ -1594,10 +1595,10 @@ static void dpcm_runtime_merge_chan(struct snd_pcm_substream *substream, cpu_stream = snd_soc_dai_get_pcm_stream(dai, stream); - *channels_min = max(*channels_min, - cpu_stream->channels_min); - *channels_max = min(*channels_max, - cpu_stream->channels_max); + hw->channels_min = max(hw->channels_min, + cpu_stream->channels_min); + hw->channels_max = min(hw->channels_max, + cpu_stream->channels_max); } /* @@ -1607,20 +1608,19 @@ static void dpcm_runtime_merge_chan(struct snd_pcm_substream *substream, if (be->num_codecs == 1) { codec_stream = snd_soc_dai_get_pcm_stream(asoc_rtd_to_codec(be, 0), stream); - *channels_min = max(*channels_min, - codec_stream->channels_min); - *channels_max = min(*channels_max, - codec_stream->channels_max); + hw->channels_min = max(hw->channels_min, + codec_stream->channels_min); + hw->channels_max = min(hw->channels_max, + codec_stream->channels_max); } } } static void dpcm_runtime_merge_rate(struct snd_pcm_substream *substream, - unsigned int *rates, - unsigned int *rate_min, - unsigned int *rate_max) + struct snd_pcm_runtime *runtime) { struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(substream); + struct snd_pcm_hardware *hw = &runtime->hw; struct snd_soc_dpcm *dpcm; int stream = substream->stream; @@ -1648,9 +1648,9 @@ static void dpcm_runtime_merge_rate(struct snd_pcm_substream *substream, pcm = snd_soc_dai_get_pcm_stream(dai, stream); - *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); + hw->rate_min = max(hw->rate_min, pcm->rate_min); + hw->rate_max = min_not_zero(hw->rate_max, pcm->rate_max); + hw->rates = snd_pcm_rate_mask_intersect(hw->rates, pcm->rates); } } } @@ -1675,11 +1675,9 @@ static void dpcm_set_fe_runtime(struct snd_pcm_substream *substream) substream->stream)); } - dpcm_runtime_merge_format(substream, &runtime->hw.formats); - dpcm_runtime_merge_chan(substream, &runtime->hw.channels_min, - &runtime->hw.channels_max); - dpcm_runtime_merge_rate(substream, &runtime->hw.rates, - &runtime->hw.rate_min, &runtime->hw.rate_max); + dpcm_runtime_merge_format(substream, runtime); + dpcm_runtime_merge_chan(substream, runtime); + dpcm_runtime_merge_rate(substream, runtime); } static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, From dd5abc7834ffae1ca6c399583353e00886817181 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Fri, 22 Jan 2021 10:13:53 +0900 Subject: [PATCH 5/5] ASoC: soc-pcm: fixup snd_pcm_limit_hw_rates() timing soc-pcm has snd_pcm_limit_hw_rates() which determine rate_min/rate_max. It updates runtime->hw.rate_min/max (A) based on hw->rates (B). int snd_pcm_limit_hw_rates(...) { int i; for (...) { (B) if (runtime->hw.rates & (1 << i)) { (A) runtime->hw.rate_min = ... break; } } for (...) { (B) if (runtime->hw.rates & (1 << i)) { (A) runtime->hw.rate_max = ... break; } } return 0; } This means, setup order is 1) set hw->rates 2) call snd_pcm_limit_hw_rates() 3) update hw->rate_min/max soc_pcm_init_runtime_hw() is calling it in good order static void soc_pcm_init_runtime_hw(xxx) { ... 1) hw->rates = snd_pcm_rate_mask_intersect(...); 2) snd_pcm_limit_hw_rates(...); 3) hw->rate_min = max(...); hw->rate_min = max(...); hw->rate_max = min_not_zero(...); hw->rate_max = min_not_zero(...); } But, dpcm_fe_dai_startup() is not. static int dpcm_fe_dai_startup(xxx) { ... 1) 3) dpcm_set_fe_runtime(...); 2) snd_pcm_limit_hw_rates(...); ... } More detail of dpcm_set_fe_runtime() is static void dpcm_set_fe_runtime() { ... for_each_rtd_cpu_dais(rtd, i, cpu_dai) { ... 3) 1) dpcm_init_runtime_hw(...); } ... 3) 1) dpcm_runtime_merge_rate(...); } This patch fixup these into static void dpcm_set_fe_runtime() { ... for_each_rtd_cpu_dais(rtd, i, cpu_dai) { ... 1) 2) 3) dpcm_init_runtime_hw(...); } ... 1) 2) 3) dpcm_runtime_merge_rate(...); } static int dpcm_fe_dai_startup(xxx) { ... dpcm_set_fe_runtime(...); - snd_pcm_limit_hw_rates(...); ... } Link: https://lore.kernel.org/r/87k15l7ewd.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Kuninori Morimoto Link: https://lore.kernel.org/r/8735ytaig8.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/soc-pcm.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 2a625ce0bacb..b79f064887d4 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1514,6 +1514,10 @@ unwind: static void dpcm_init_runtime_hw(struct snd_pcm_runtime *runtime, struct snd_soc_pcm_stream *stream) { + runtime->hw.rates = stream->rates; + + snd_pcm_limit_hw_rates(runtime); + runtime->hw.rate_min = stream->rate_min; runtime->hw.rate_max = min_not_zero(stream->rate_max, UINT_MAX); runtime->hw.channels_min = stream->channels_min; @@ -1522,7 +1526,6 @@ static void dpcm_init_runtime_hw(struct snd_pcm_runtime *runtime, runtime->hw.formats &= stream->formats; else runtime->hw.formats = stream->formats; - runtime->hw.rates = stream->rates; } static void dpcm_runtime_merge_format(struct snd_pcm_substream *substream, @@ -1648,9 +1651,12 @@ static void dpcm_runtime_merge_rate(struct snd_pcm_substream *substream, pcm = snd_soc_dai_get_pcm_stream(dai, stream); + hw->rates = snd_pcm_rate_mask_intersect(hw->rates, pcm->rates); + + snd_pcm_limit_hw_rates(runtime); + hw->rate_min = max(hw->rate_min, pcm->rate_min); hw->rate_max = min_not_zero(hw->rate_max, pcm->rate_max); - hw->rates = snd_pcm_rate_mask_intersect(hw->rates, pcm->rates); } } } @@ -1738,7 +1744,6 @@ static int dpcm_apply_symmetry(struct snd_pcm_substream *fe_substream, static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) { struct snd_soc_pcm_runtime *fe = asoc_substream_to_rtd(fe_substream); - struct snd_pcm_runtime *runtime = fe_substream->runtime; int stream = fe_substream->stream, ret = 0; dpcm_set_fe_update_state(fe, stream, SND_SOC_DPCM_UPDATE_FE); @@ -1761,7 +1766,6 @@ static int dpcm_fe_dai_startup(struct snd_pcm_substream *fe_substream) fe->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN; dpcm_set_fe_runtime(fe_substream); - snd_pcm_limit_hw_rates(runtime); ret = dpcm_apply_symmetry(fe_substream, stream); if (ret < 0)