[PATCH 0/5] ASoC: soc-pcm: cleanup soc_new_pcm() and bugfix

newer
[PATCH v1 0/2] ASoC: tegra: Add...

older
ucm2: Separate the configuration...

Kuninori Morimoto

22 Jan 2021 22 Jan '21

2:13 a.m.

Hi Mark

These are soc-pcm cleanup patchset.

1) - 3) : cleanup soc_new_pcm() function 4) : cleanup dpcm_runtime_merge_xxx() function 5) : bugfix of snd_pcm_limit_hw_rates() order

Kuninori Morimoto (5): 1) ASoC: soc-pcm: tidyup pcm setting 2) ASoC: soc-pcm: add soc_get_playback_capture() and simplify soc_new_pcm() 3) ASoC: soc-pcm: add soc_create_pcm() and simplify soc_new_pcm() 4) ASoC: soc-pcm: use snd_pcm_hardware at dpcm_runtime_merge_xxx() 5) ASoC: soc-pcm: fixup snd_pcm_limit_hw_rates() timing

sound/soc/soc-pcm.c | 124 +++++++++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 49 deletions(-)

-- 2.25.1

Show replies by date

Kuninori Morimoto

22 Jan 22 Jan

2:13 a.m.

New subject: [PATCH 1/5] ASoC: soc-pcm: tidyup pcm setting

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

Current soc_new_pcm() setups pcm randomly. This patch tidyup it.

Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- 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) {

-- 2.25.1

Kuninori Morimoto

2:13 a.m.

New subject: [PATCH 2/5] ASoC: soc-pcm: add soc_get_playback_capture() and simplify soc_new_pcm()

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

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 kuninori.morimoto.gx@renesas.com --- 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 @@ static int dpcm_fe_dai_open(struct snd_pcm_substream *fe_substream) 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)",

-- 2.25.1

Kuninori Morimoto

2:13 a.m.

New subject: [PATCH 3/5] ASoC: soc-pcm: add soc_create_pcm() and simplify soc_new_pcm()

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

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 kuninori.morimoto.gx@renesas.com --- 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; }

-- 2.25.1

Kuninori Morimoto

2:13 a.m.

New subject: [PATCH 4/5] ASoC: soc-pcm: use snd_pcm_hardware at dpcm_runtime_merge_xxx()

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

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 kuninori.morimoto.gx@renesas.com --- 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,

-- 2.25.1

Kuninori Morimoto

2:13 a.m.

New subject: [PATCH 5/5] ASoC: soc-pcm: fixup snd_pcm_limit_hw_rates() timing

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

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 kuninori.morimoto.gx@renesas.com --- 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 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) 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)

-- 2.25.1

Mark Brown

3 Feb 3 Feb

10:07 p.m.

On 22 Jan 2021 10:13:04 +0900, Kuninori Morimoto wrote:

...

These are soc-pcm cleanup patchset.
: cleanup soc_new_pcm() function
 : cleanup dpcm_runtime_merge_xxx() function
 : bugfix of snd_pcm_limit_hw_rates() order
Kuninori Morimoto (5):

ASoC: soc-pcm: tidyup pcm setting

ASoC: soc-pcm: add soc_get_playback_capture() and simplify soc_new_pcm()

ASoC: soc-pcm: add soc_create_pcm() and simplify soc_new_pcm()

ASoC: soc-pcm: use snd_pcm_hardware at dpcm_runtime_merge_xxx()

ASoC: soc-pcm: fixup snd_pcm_limit_hw_rates() timing

[...]

Applied to

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

Thanks!

[1/5] ASoC: soc-pcm: tidyup pcm setting commit: e04e7b8ccd4912e6c823bf7e66f302a53396fb77 [2/5] ASoC: soc-pcm: add soc_get_playback_capture() and simplify soc_new_pcm() commit: 7fc6bebd5831a788a74e019e39c43c014a96a110 [3/5] ASoC: soc-pcm: add soc_create_pcm() and simplify soc_new_pcm() commit: 2b39123b134e10a3817156bd9b157c9b8f950d6f [4/5] ASoC: soc-pcm: use snd_pcm_hardware at dpcm_runtime_merge_xxx() commit: 4b260f425497b105acc2baa9d97ef781ef0c667d [5/5] ASoC: soc-pcm: fixup snd_pcm_limit_hw_rates() timing commit: dd5abc7834ffae1ca6c399583353e00886817181

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

1418

Age (days ago)

1430

Last active (days ago)

List overview

Download

6 comments

2 participants

tags (0)

participants (2)

Kuninori Morimoto
Mark Brown