September 2019 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [RFC] ASoC: soc-pcm: Use different sequence for start/stop trigger
by Peter Ujfalusi 25 Sep '19

25 Sep '19

On stream stop currently we stop the DMA first followed by the CPU DAI. This can cause underflow (playback) or overflow (capture) on the DAI side as the DMA is no longer feeding data while the DAI is still active. It can be observed easily if the DAI side does not have FIFO (or it is disabled) to survive the time while the DMA is stopped, but still can happen on relatively slow CPUs when relatively high sampling rate is used: the FIFO is drained between the time the DMA is stopped and the DAI is stopped. It can only fixed by using different sequence within trigger for 'stop' and 'start': case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: Start DMA first followed by CPU DAI (currently used sequence) case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: Stop CPU DAI first followed by DMA Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)ti.com> --- Hi, on a new TI platform (j721e) where we can disable the McASP FIFO (the CPU dai) since the UDMA + PDMA provides the buffering I have started to see error interrupts right after pcm_trigger:STOP and in rare cases even on PAUSE that McASP underruns. I was also able to reproduce the same issue on am335x board, but it is much harder to trigger it. With this patch the underrun after trigger:STOP is gone. If I think about the issue, I'm not sure why it was not noticed before as the behavior makes sense: we stop the DMA first then we stop the CPU DAI. If between the DMA stop and DAI stop we would need a sample in the DAI (which is still running) then for sure we will underrun in the HW (or overrun in case of capture). When I run the ALSA conformance test [1] it is easier to trigger. Not sure if anyone else have seen such underrun/overrun when stopping a stream, but the fact that I have seen it with both UDMA+PDMA and EDMA on different platforms makes me wonder if the issue can be seen on other platforms as well. [1] https://chromium.googlesource.com/chromiumos/platform/audiotest/+/master/al… Regards, Peter --- sound/soc/soc-pcm.c | 66 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e163dde5eab1..c96430e70752 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1047,7 +1047,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) return 0; } -static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) +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; @@ -1056,24 +1056,60 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) struct snd_soc_dai *codec_dai; int i, ret; + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + ret = snd_soc_component_trigger(component, substream, cmd); + if (ret < 0) + return ret; + } + for_each_rtd_codec_dai(rtd, i, codec_dai) { ret = snd_soc_dai_trigger(codec_dai, substream, cmd); if (ret < 0) return ret; } - for_each_rtdcom(rtd, rtdcom) { - component = rtdcom->component; + snd_soc_dai_trigger(cpu_dai, substream, cmd); + if (ret < 0) + return ret; - ret = snd_soc_component_trigger(component, substream, cmd); + if (rtd->dai_link->ops->trigger) { + ret = rtd->dai_link->ops->trigger(substream, cmd); if (ret < 0) return ret; } + return 0; +} + +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_rtdcom_list *rtdcom; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dai *codec_dai; + int i, ret; + snd_soc_dai_trigger(cpu_dai, substream, cmd); if (ret < 0) return ret; + for_each_rtd_codec_dai(rtd, i, codec_dai) { + ret = snd_soc_dai_trigger(codec_dai, substream, cmd); + if (ret < 0) + return ret; + } + + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + ret = snd_soc_component_trigger(component, substream, cmd); + if (ret < 0) + return ret; + } + if (rtd->dai_link->ops->trigger) { ret = rtd->dai_link->ops->trigger(substream, cmd); if (ret < 0) @@ -1083,6 +1119,28 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return 0; } +static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) +{ + int ret; + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + ret = soc_pcm_trigger_start(substream, cmd); + break; + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + ret = soc_pcm_trigger_stop(substream, cmd); + break; + default: + return -EINVAL; + } + + return ret; +} + static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, int cmd) { -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

3 5

[alsa-devel] [PATCH] ASoC: soc-pcm: Use different sequence for start/stop trigger
by Peter Ujfalusi 25 Sep '19

25 Sep '19

On stream stop currently we stop the DMA first followed by the CPU DAI. This can cause underflow (playback) or overflow (capture) on the DAI side as the DMA is no longer feeding data while the DAI is still active. It can be observed easily if the DAI side does not have FIFO (or it is disabled) to survive the time while the DMA is stopped, but still can happen on relatively slow CPUs when relatively high sampling rate is used: the FIFO is drained between the time the DMA is stopped and the DAI is stopped. It can only fixed by using different sequence within trigger for 'stop' and 'start': case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: Trigger order: dai_link, DMA, CPU DAI then the codec case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: Trigger order: codec, CPU DAI, DMA then dai_link Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)ti.com> --- sound/soc/soc-pcm.c | 66 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index e163dde5eab1..41e081e95108 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1047,7 +1047,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) return 0; } -static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) +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; @@ -1056,8 +1056,8 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) struct snd_soc_dai *codec_dai; int i, ret; - for_each_rtd_codec_dai(rtd, i, codec_dai) { - ret = snd_soc_dai_trigger(codec_dai, substream, cmd); + if (rtd->dai_link->ops->trigger) { + ret = rtd->dai_link->ops->trigger(substream, cmd); if (ret < 0) return ret; } @@ -1070,10 +1070,46 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return ret; } - snd_soc_dai_trigger(cpu_dai, substream, cmd); + ret = snd_soc_dai_trigger(cpu_dai, substream, cmd); if (ret < 0) return ret; + for_each_rtd_codec_dai(rtd, i, codec_dai) { + ret = snd_soc_dai_trigger(codec_dai, substream, cmd); + if (ret < 0) + return ret; + } + + return 0; +} + +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_rtdcom_list *rtdcom; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + struct snd_soc_dai *codec_dai; + int i, ret; + + for_each_rtd_codec_dai(rtd, i, codec_dai) { + ret = snd_soc_dai_trigger(codec_dai, substream, cmd); + if (ret < 0) + return ret; + } + + ret = snd_soc_dai_trigger(cpu_dai, substream, cmd); + if (ret < 0) + return ret; + + for_each_rtdcom(rtd, rtdcom) { + component = rtdcom->component; + + ret = snd_soc_component_trigger(component, substream, cmd); + if (ret < 0) + return ret; + } + if (rtd->dai_link->ops->trigger) { ret = rtd->dai_link->ops->trigger(substream, cmd); if (ret < 0) @@ -1083,6 +1119,28 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) return 0; } +static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd) +{ + int ret; + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_RESUME: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + ret = soc_pcm_trigger_start(substream, cmd); + break; + case SNDRV_PCM_TRIGGER_STOP: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + ret = soc_pcm_trigger_stop(substream, cmd); + break; + default: + return -EINVAL; + } + + return ret; +} + static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream, int cmd) { -- Peter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

1 0

Re: [alsa-devel] [PATCH] ASoC: Intel: Skylake: prevent memory leak in snd_skl_parse_uuids
by Andy Shevchenko 25 Sep '19

25 Sep '19

On Tue, Sep 24, 2019 at 11:08:38PM -0500, Navid Emamdoost wrote: > In snd_skl_parse_uuids if allocation for module->instance_id fails, the > allocated memory for module shoulde be released. Since it's using devm_*() for the second allocation it would be cleaner to either switch it to regular kcalloc() or to switch the first one to devm_kzalloc(), whatever suits better. > module->instance_id = devm_kzalloc(ctx->dev, size, GFP_KERNEL); > if (!module->instance_id) { > + kfree(module); -- With Best Regards, Andy Shevchenko

1 0

[alsa-devel] [PATCH 0/3] Make pcm_ioplug check lock status before locking (fixes pcm_jack lockups)
by Ben Russell 25 Sep '19

25 Sep '19

This is my first time contributing a patch to a mailing list. If I've made a mess, please let me know so I can learn how to avoid it in future. The purpose of this patchset is to fix a specific common lockup in the pcm_jack plugin (from alsa-plugins). I'm not sure if this is the right approach to take, but at the very least it should make the pcm_ioplug code a bit more resilient. The problem is this: When using the pcm_jack plugin, if a program attempts to play audio using the SND_PCM_FORMAT_FLOAT format, said program locks up. This should be enough to reproduce the bug: pcm.rawjack { type jack playback_ports { 0 system:playback_1 1 system:playback_2 } capture_ports { 0 system:capture_1 1 system:capture_2 } } pcm.!default { type plug slave.pcm "rawjack" } What's happening is that several snd_pcm_ioplug_* functions assume that the pcm mutex is locked already. It then proceeds to unlock the mutex, call a function, and then relock the mutex. When the mutex isn't locked already, the initial unlock results in a silently ignored pthread error, and the lock results in the program eventually deadlocking as it doesn't expect the mutex to be held at that point. Patch 2 modifies pcm_ioplug to check if the mutex is held before doing the unlock-act-lock sequence, and if the mutex is not held then it skips the unlock and lock stages. This depends on Patch 1, which adds a snd_pcm_is_locked function to give the state of the mutex. Patch 3 is completely optional. It adds assertions which make sure that all uses of snd_pcm_lock/snd_pcm_unlock are correct. On one hand this will likely result in crashes in some of the less refined parts of the code. On the other hand, when that happens, you'll know which parts need a bit more love. I know it was useful for finding this issue in the first place. These patches fix the problems I am having, but if you have a more suitable approach to fixing this problem then please let me know. Regards, Ben R Ben Russell (3): pcm_local: Add snd_pcm_is_locked pcm_ioplug: Don't unlock+lock if we're not locked pcm_local: assert() when using mutexes incorrectly src/pcm/pcm_ioplug.c | 50 +++++++++++++++++++++++----------- src/pcm/pcm_local.h | 64 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 91 insertions(+), 23 deletions(-) -- 2.23.0

2 7

[alsa-devel] [PATCH v2 0/2] Add CML M/C for RT5682 and RT1011
by sathya.prakash.m.r＠intel.com 25 Sep '19

25 Sep '19

From: Sathya Prakash M R <sathya.prakash.m.r(a)intel.com> The series adds CML m/c driver with RT5682 on SSP0 and RT1011 on SSP1. The RT1011 is smart AMP which supports 2 Whoofers and 2 tweeter support. RT5682 is headset codec supporting playback and record. The m/c supports HDMI playback and DMIC record. Naveen Manohar (2): ASoC: Intel: boards: Add CML m/c using RT1011 and RT5682 ASoC: Intel: Add acpi match for rt1011 based m/c driver sound/soc/intel/boards/Kconfig | 18 + sound/soc/intel/boards/Makefile | 2 + sound/soc/intel/boards/cml_rt1011_rt5682.c | 473 ++++++++++++++++++++++ sound/soc/intel/common/soc-acpi-intel-cnl-match.c | 7 + 4 files changed, 500 insertions(+) create mode 100644 sound/soc/intel/boards/cml_rt1011_rt5682.c -- Changes from V1: Addressed comments on ASoC: Intel: boards: Add CML m/c using RT1011 and RT5682 1.9.1

1 2

Re: [alsa-devel] [PATCH V4 4/4] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
by S.j. Wang 25 Sep '19

25 Sep '19

Hi > On Tue, Sep 24, 2019 at 06:52:35PM +0800, Shengjiu Wang wrote: > > There is error "aplay: pcm_write:2023: write error: Input/output error" > > on i.MX8QM/i.MX8QXP platform for S24_3LE format. > > > > In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit > > sample, but we didn't add any constraint, that cause issues. > > > > So we need to query the caps of dma, then update the hw parameters > > according to the caps. > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> > > --- > > sound/soc/fsl/fsl_asrc.c | 4 +-- > > sound/soc/fsl/fsl_asrc.h | 3 ++ > > sound/soc/fsl/fsl_asrc_dma.c | 59 > > +++++++++++++++++++++++++++++++----- > > 3 files changed, 56 insertions(+), 10 deletions(-) > > > > @@ -270,12 +268,17 @@ static int fsl_asrc_dma_hw_free(struct > > snd_pcm_substream *substream) > > > > static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) > > { > > + bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; > > struct snd_soc_pcm_runtime *rtd = substream->private_data; > > struct snd_pcm_runtime *runtime = substream->runtime; > > struct snd_soc_component *component = > snd_soc_rtdcom_lookup(rtd, > > DRV_NAME); > > + struct snd_dmaengine_dai_dma_data *dma_data; > > struct device *dev = component->dev; > > struct fsl_asrc *asrc_priv = dev_get_drvdata(dev); > > struct fsl_asrc_pair *pair; > > + struct dma_chan *tmp_chan = NULL; > > + u8 dir = tx ? OUT : IN; > > + int ret = 0; > > > > pair = kzalloc(sizeof(struct fsl_asrc_pair), GFP_KERNEL); > > Sorry, I didn't catch it previously. We would need to release this memory > also for all error-out paths, as the code doesn't have any error-out routine, > prior to applying this change. > > > if (!pair) > > @@ -285,11 +288,51 @@ static int fsl_asrc_dma_startup(struct > > snd_pcm_substream *substream) > > > + /* Request a dummy pair, which will be released later. > > + * Request pair function needs channel num as input, for this > > + * dummy pair, we just request "1" channel temporary. > > + */ > > "temporary" => "temporarily" > > > + ret = fsl_asrc_request_pair(1, pair); > > + if (ret < 0) { > > + dev_err(dev, "failed to request asrc pair\n"); > > + return ret; > > + } > > + > > + /* Request a dummy dma channel, which will be release later. */ > > "release" => "released" Ok, will update them. Best regards Wang shengjiu

1 0

[alsa-devel] [PATCH V4 0/4] update supported sample format
by Shengjiu Wang 24 Sep '19

24 Sep '19

This patch serial is to update the supported format for fsl_asrc and fix some format issue. Shengjiu Wang (4): ASoC: fsl_asrc: Use in(out)put_format instead of in(out)put_word_width ASoC: fsl_asrc: update supported sample format ASoC: pcm_dmaengine: Extract snd_dmaengine_pcm_refine_runtime_hwparams ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8 changes in v2 - extract snd_dmaengine_pcm_set_runtime_hwparams in one separate path. - 4th patch depends on 3rd patch changes in v3 - Fix build report by kbuild test robot <lkp(a)intel.com> - change snd_dmaengine_pcm_set_runtime_hwparams to snd_dmaengine_pcm_refine_runtime_hwparams changes in v4 - update according to Nicolin's comments. include/sound/dmaengine_pcm.h | 5 ++ sound/core/pcm_dmaengine.c | 83 +++++++++++++++++++++++++++ sound/soc/fsl/fsl_asrc.c | 65 ++++++++++++++------- sound/soc/fsl/fsl_asrc.h | 7 ++- sound/soc/fsl/fsl_asrc_dma.c | 59 ++++++++++++++++--- sound/soc/soc-generic-dmaengine-pcm.c | 61 ++------------------ 6 files changed, 193 insertions(+), 87 deletions(-) -- 2.21.0

2 6

[alsa-devel] Making alsamixer configurable, adding mouse support + minor fixes
by GitHub pull_request - opened 24 Sep '19

24 Sep '19

alsa-project/alsa-utils pull request #26 was opened from braph: Hello, I'm using the alsamixer, but I am missing mouse support and the ability to configure it. I implemented those things for myself now, maybe it would also be useful for others. There are some commits that would even fit in the original alsamixer code without my rather heavy modifications, these are: 6eb0bc8f0848567f302e94ea5ee3a6d7c6966e65: _alsamixer: remove redundant sprintf() calls_ bef5b723a43f310ba73bcaa52f3da74ef5e08dde: _alsamixer: use mvwhline/mvwvline for drawing lines_ ae0604c93aa4bacbb678eb8ec832ad9e17c8b56a: _alsamixer: device_name.c: recognize DEL (ASCII 127) as backspace_ And maybe 485f3b0da1ffead2f88cffd9f2203516000415b5: _alsamixer: fix window_size_changed()_ Regards, Benjamin Request URL : https://github.com/alsa-project/alsa-utils/pull/26 Patch URL : https://github.com/alsa-project/alsa-utils/pull/26.patch Repository URL: https://github.com/alsa-project/alsa-utils

1 0

[alsa-devel] alsa-ucm-conf repository - initial import
by Jaroslav Kysela 24 Sep '19

24 Sep '19

Hello all, I just made an initial import to to alsa-ucm-conf repository which is in the 'import' branch: https://github.com/alsa-project/alsa-ucm-conf/tree/import I do not have the BSD-3-Clause licence acknowledgements from some minor contributors. I do not think that it's a problem. I would expect that if we do not have a disagreement, it is fine. The question is, if we need automake/autoconf tools to package those files. It seems to me that it might be clever to keep this ASIS for now. Users/packagers can just do 'cp' to the appropriate directory. Jaroslav -- Jaroslav Kysela <perex(a)perex.cz> Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

2 1

[alsa-devel] [PATCH] ASoC: samsung: i2s: Add clocks' macros descriptions
by Marek Szyprowski 24 Sep '19

24 Sep '19

From: Maciej Falkowski <m.falkowski(a)samsung.com> To increase macro readability added descriptions to clocks macros. Signed-off-by: Maciej Falkowski <m.falkowski(a)samsung.com> Signed-off-by: Marek Szyprowski <m.szyprowski(a)samsung.com> --- include/dt-bindings/sound/samsung-i2s.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/include/dt-bindings/sound/samsung-i2s.h b/include/dt-bindings/sound/samsung-i2s.h index 77545f14c379..fd752475c762 100644 --- a/include/dt-bindings/sound/samsung-i2s.h +++ b/include/dt-bindings/sound/samsung-i2s.h @@ -2,8 +2,13 @@ #ifndef _DT_BINDINGS_SAMSUNG_I2S_H #define _DT_BINDINGS_SAMSUNG_I2S_H -#define CLK_I2S_CDCLK 0 -#define CLK_I2S_RCLK_SRC 1 -#define CLK_I2S_RCLK_PSR 2 +/* the CDCLK (CODECLKO) gate clock */ +#define CLK_I2S_CDCLK 0 + +/* the RCLKSRC mux clock (corresponding to RCLKSRC bit in IISMOD register) */ +#define CLK_I2S_RCLK_SRC 1 + +/* the RCLK prescaler divider clock (corresponding to the IISPSR register) */ +#define CLK_I2S_RCLK_PSR 2 #endif /* _DT_BINDINGS_SAMSUNG_I2S_H */ -- 2.17.1

3 2