* [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin
@ 2021-05-24 14:21 Srinivasa Rao Mandadapu
2021-05-28 8:47 ` Srinivas Kandagatla
2021-06-01 15:17 ` Doug Anderson
0 siblings, 2 replies; 4+ messages in thread
From: Srinivasa Rao Mandadapu @ 2021-05-24 14:21 UTC (permalink / raw)
To: agross, bjorn.andersson, lgirdwood, broonie, robh+dt, plai,
bgoswami, perex, tiwai, srinivas.kandagatla, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
Cc: Srinivasa Rao Mandadapu
This patch fixes PoP noise of around 15ms observed during audio capture begin.
Enables BCLK and LRCLK in snd_soc_dai_ops prepare call for introducing some delay
before capture start and clock enable.
Co-developed-by: Judy Hsiao <judyhsiao@chromium.org>
Signed-off-by: Judy Hsiao <judyhsiao@chromium.org>
Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
---
Changes Since V2:
-- Updated comments as per linux style
-- Removed unrelated changes.
Changes Since V1:
-- Enableed BCLK and LRCLK in dai ops prepare API instead of startup API
-- Added comments
sound/soc/qcom/lpass-cpu.c | 54 +++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index 28c7497344e3..1855eae22aad 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -93,9 +93,21 @@ static void lpass_cpu_daiops_shutdown(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+ struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+ unsigned int id = dai->driver->id;
clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]);
- clk_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
+ /*
+ * To ensure BCLK/LRCLK disabled even in device node validation
+ * Will not impact if disabled in lpass_cpu_daiops_trigger()
+ * suspend.
+ */
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ regmap_fields_write(i2sctl->spken, id, LPAIF_I2SCTL_SPKEN_DISABLE);
+ else
+ regmap_fields_write(i2sctl->micen, id, LPAIF_I2SCTL_MICEN_DISABLE);
+
+ clk_disable_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
}
static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
@@ -275,6 +287,10 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ /*
+ * To ensure lpass BCLK/LRCLK is enabled during
+ * device resume. Will not impact if enabled in lpass_cpu_daiops_prepare().
+ */
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
ret = regmap_fields_write(i2sctl->spken, id,
LPAIF_I2SCTL_SPKEN_ENABLE);
@@ -296,6 +312,10 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ /*
+ * To ensure lpass BCLK/LRCLK is disabled during
+ * device suspend.
+ */
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
ret = regmap_fields_write(i2sctl->spken, id,
LPAIF_I2SCTL_SPKEN_DISABLE);
@@ -315,12 +335,44 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
return ret;
}
+static int lpass_cpu_daiops_prepare(struct snd_pcm_substream *substream,
+ struct snd_soc_dai *dai)
+{
+ struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
+ struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+ unsigned int id = dai->driver->id;
+ int ret;
+ /*
+ * To ensure lpass BCLK/LRCLK is enabled bit before
+ * playback/capture data flow starts.
+ */
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+ ret = regmap_fields_write(i2sctl->spken, id, LPAIF_I2SCTL_SPKEN_ENABLE);
+ else
+ ret = regmap_fields_write(i2sctl->micen, id, LPAIF_I2SCTL_MICEN_ENABLE);
+
+ if (ret) {
+ dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_enable(drvdata->mi2s_bit_clk[id]);
+
+ if (ret) {
+ dev_err(dai->dev, "error in enabling mi2s bit clk: %d\n", ret);
+ clk_disable(drvdata->mi2s_osr_clk[id]);
+ return ret;
+ }
+ return 0;
+}
+
const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops = {
.set_sysclk = lpass_cpu_daiops_set_sysclk,
.startup = lpass_cpu_daiops_startup,
.shutdown = lpass_cpu_daiops_shutdown,
.hw_params = lpass_cpu_daiops_hw_params,
.trigger = lpass_cpu_daiops_trigger,
+ .prepare = lpass_cpu_daiops_prepare,
};
EXPORT_SYMBOL_GPL(asoc_qcom_lpass_cpu_dai_ops);
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin
2021-05-24 14:21 [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin Srinivasa Rao Mandadapu
@ 2021-05-28 8:47 ` Srinivas Kandagatla
2021-06-01 15:17 ` Doug Anderson
1 sibling, 0 replies; 4+ messages in thread
From: Srinivas Kandagatla @ 2021-05-28 8:47 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu, agross, bjorn.andersson, lgirdwood,
broonie, robh+dt, plai, bgoswami, perex, tiwai, rohitkr,
linux-arm-msm, alsa-devel, devicetree, linux-kernel, swboyd,
judyhsiao
On 24/05/2021 15:21, Srinivasa Rao Mandadapu wrote:
> This patch fixes PoP noise of around 15ms observed during audio capture begin.
> Enables BCLK and LRCLK in snd_soc_dai_ops prepare call for introducing some delay
> before capture start and clock enable.
>
> Co-developed-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> Changes Since V2:
> -- Updated comments as per linux style
> -- Removed unrelated changes.
> Changes Since V1:
> -- Enableed BCLK and LRCLK in dai ops prepare API instead of startup API
> -- Added comments
>
> sound/soc/qcom/lpass-cpu.c | 54 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
> index 28c7497344e3..1855eae22aad 100644
> --- a/sound/soc/qcom/lpass-cpu.c
> +++ b/sound/soc/qcom/lpass-cpu.c
> @@ -93,9 +93,21 @@ static void lpass_cpu_daiops_shutdown(struct snd_pcm_substream *substream,
> struct snd_soc_dai *dai)
> {
> struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> + struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
> + unsigned int id = dai->driver->id;
>
> clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]);
> - clk_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
> + /*
> + * To ensure BCLK/LRCLK disabled even in device node validation
> + * Will not impact if disabled in lpass_cpu_daiops_trigger()
> + * suspend.
> + */
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + regmap_fields_write(i2sctl->spken, id, LPAIF_I2SCTL_SPKEN_DISABLE);
> + else
> + regmap_fields_write(i2sctl->micen, id, LPAIF_I2SCTL_MICEN_DISABLE);
> +
> + clk_disable_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
> }
>
> static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
> @@ -275,6 +287,10 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
> case SNDRV_PCM_TRIGGER_START:
> case SNDRV_PCM_TRIGGER_RESUME:
> case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> + /*
> + * To ensure lpass BCLK/LRCLK is enabled during
> + * device resume. Will not impact if enabled in lpass_cpu_daiops_prepare().
> + */
prepare should have already ensured that isn't it?
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> ret = regmap_fields_write(i2sctl->spken, id,
> LPAIF_I2SCTL_SPKEN_ENABLE);
> @@ -296,6 +312,10 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
> case SNDRV_PCM_TRIGGER_STOP:
> case SNDRV_PCM_TRIGGER_SUSPEND:
> case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> + /*
> + * To ensure lpass BCLK/LRCLK is disabled during
> + * device suspend.
> + */
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> ret = regmap_fields_write(i2sctl->spken, id,
> LPAIF_I2SCTL_SPKEN_DISABLE);
> @@ -315,12 +335,44 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
> return ret;
> }
>
[Snip ...
> +static int lpass_cpu_daiops_prepare(struct snd_pcm_substream *substream,
> + struct snd_soc_dai *dai)
> +{
> + struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
> + struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
> + unsigned int id = dai->driver->id;
> + int ret;
> + /*
> + * To ensure lpass BCLK/LRCLK is enabled bit before
> + * playback/capture data flow starts.
> + */
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + ret = regmap_fields_write(i2sctl->spken, id, LPAIF_I2SCTL_SPKEN_ENABLE);
> + else
> + ret = regmap_fields_write(i2sctl->micen, id, LPAIF_I2SCTL_MICEN_ENABLE);
> +
> + if (ret) {
> + dev_err(dai->dev, "error writing to i2sctl reg: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_enable(drvdata->mi2s_bit_clk[id]);
> +
> + if (ret) {
> + dev_err(dai->dev, "error in enabling mi2s bit clk: %d\n", ret);
> + clk_disable(drvdata->mi2s_osr_clk[id]);
> + return ret;
> + }
> + return 0;
> +}
> +
----]
If prepare is enabling the clk and the i2s enable bits, then why do we
need to do the same thing in trigger?
Also the clk enable count is going up twice (once in prepare and
trigger) and we can never be able to disable the clk as there is is no
corresponding disable for this prepare path.
Also note that prepare can be called multiple times.
--srini
> const struct snd_soc_dai_ops asoc_qcom_lpass_cpu_dai_ops = {
> .set_sysclk = lpass_cpu_daiops_set_sysclk,
> .startup = lpass_cpu_daiops_startup,
> .shutdown = lpass_cpu_daiops_shutdown,
> .hw_params = lpass_cpu_daiops_hw_params,
> .trigger = lpass_cpu_daiops_trigger,
> + .prepare = lpass_cpu_daiops_prepare,
> };
> EXPORT_SYMBOL_GPL(asoc_qcom_lpass_cpu_dai_ops);
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin
2021-05-24 14:21 [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin Srinivasa Rao Mandadapu
@ 2021-06-01 15:17 ` Doug Anderson
2021-06-01 15:17 ` Doug Anderson
1 sibling, 0 replies; 4+ messages in thread
From: Doug Anderson @ 2021-06-01 15:17 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: Andy Gross, Bjorn Andersson, Liam Girdwood, Mark Brown,
Rob Herring, Patrick Lai, Banajit Goswami, Jaroslav Kysela,
Takashi Iwai, Srinivas Kandagatla, Rohit kumar, linux-arm-msm,
ALSA Development Mailing List,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Stephen Boyd, Judy Hsiao
Hi,
On Mon, May 24, 2021 at 7:21 AM Srinivasa Rao Mandadapu
<srivasam@codeaurora.org> wrote:
>
> This patch fixes PoP noise of around 15ms observed during audio capture begin.
> Enables BCLK and LRCLK in snd_soc_dai_ops prepare call for introducing some delay
> before capture start and clock enable.
>
> Co-developed-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> Changes Since V2:
> -- Updated comments as per linux style
> -- Removed unrelated changes.
> Changes Since V1:
> -- Enableed BCLK and LRCLK in dai ops prepare API instead of startup API
> -- Added comments
>
> sound/soc/qcom/lpass-cpu.c | 54 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
Presumably related with Srinivas Kandagatla's comments: this patch
causes a bad splat at bootup. Specifically, here's what I saw when
this patch was in the Chrome OS 5.4 kernel:
------------[ cut here ]------------
lpass_audio_core_lpaif_pri_ibit_clk already disabled
WARNING: CPU: 0 PID: 2066 at drivers/clk/clk.c:958 clk_core_disable+0x4a0/0x670
CPU: 0 PID: 2066 Comm: cras Not tainted 5.4.122-lockdep #4
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
pstate: 60400089 (nZCv daIf +PAN -UAO)
pc : clk_core_disable+0x4a0/0x670
lr : clk_core_disable+0x4a0/0x670
sp : ffffff81727cf8a0
x29: ffffff81727cf8a0 x28: 1ffffff033d2b8d5
x27: dfffffd000000000 x26: ffffff81a3781980
x25: 1ffffff03493d3b3 x24: ffffff816f433480
x23: dfffffd000000000 x22: 1ffffff035b66e2f
x21: 00000000ffffffff x20: ffffff81adb3717c
x19: ffffff81adb37100 x18: 0000000000000339
x17: ffffffffffffffff x16: 0000000000000006
x15: 0000000000000001 x14: 0720072007200720
x13: 0000000000000000 x12: ffffffd0132c1e00
x11: 0000000000000001 x10: 0000000000000000
x9 : cff2cb5e2cb22e00 x8 : cff2cb5e2cb22e00
x7 : ffffffd010288f74 x6 : 0000000000000000
x5 : 0000000000000080 x4 : 0000000000000001
x3 : ffffffd010429d10 x2 : 0000000000000001
x1 : 0000000000000008 x0 : 0000000000000034
Call trace:
clk_core_disable+0x4a0/0x670
clk_disable+0x50/0x64
lpass_cpu_daiops_shutdown+0x190/0x204 [snd_soc_lpass_cpu]
snd_soc_dai_shutdown+0x8c/0x9c
soc_pcm_close+0x258/0x478
snd_pcm_release_substream+0xfc/0x1e8
snd_pcm_release+0x8c/0x124
__fput+0x1e4/0x4fc
____fput+0x1c/0x28
task_work_run+0x12c/0x164
do_notify_resume+0x1e04/0x2c48
work_pending+0x8/0x14
irq event stamp: 101552
-Doug
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin
@ 2021-06-01 15:17 ` Doug Anderson
0 siblings, 0 replies; 4+ messages in thread
From: Doug Anderson @ 2021-06-01 15:17 UTC (permalink / raw)
To: Srinivasa Rao Mandadapu
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
ALSA Development Mailing List, Banajit Goswami, linux-arm-msm,
Patrick Lai, Takashi Iwai, Andy Gross, Rob Herring,
Liam Girdwood, Mark Brown, Rohit kumar, Stephen Boyd,
Bjorn Andersson, Judy Hsiao, LKML
Hi,
On Mon, May 24, 2021 at 7:21 AM Srinivasa Rao Mandadapu
<srivasam@codeaurora.org> wrote:
>
> This patch fixes PoP noise of around 15ms observed during audio capture begin.
> Enables BCLK and LRCLK in snd_soc_dai_ops prepare call for introducing some delay
> before capture start and clock enable.
>
> Co-developed-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Judy Hsiao <judyhsiao@chromium.org>
> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
> ---
> Changes Since V2:
> -- Updated comments as per linux style
> -- Removed unrelated changes.
> Changes Since V1:
> -- Enableed BCLK and LRCLK in dai ops prepare API instead of startup API
> -- Added comments
>
> sound/soc/qcom/lpass-cpu.c | 54 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 53 insertions(+), 1 deletion(-)
Presumably related with Srinivas Kandagatla's comments: this patch
causes a bad splat at bootup. Specifically, here's what I saw when
this patch was in the Chrome OS 5.4 kernel:
------------[ cut here ]------------
lpass_audio_core_lpaif_pri_ibit_clk already disabled
WARNING: CPU: 0 PID: 2066 at drivers/clk/clk.c:958 clk_core_disable+0x4a0/0x670
CPU: 0 PID: 2066 Comm: cras Not tainted 5.4.122-lockdep #4
Hardware name: Google Lazor (rev1 - 2) with LTE (DT)
pstate: 60400089 (nZCv daIf +PAN -UAO)
pc : clk_core_disable+0x4a0/0x670
lr : clk_core_disable+0x4a0/0x670
sp : ffffff81727cf8a0
x29: ffffff81727cf8a0 x28: 1ffffff033d2b8d5
x27: dfffffd000000000 x26: ffffff81a3781980
x25: 1ffffff03493d3b3 x24: ffffff816f433480
x23: dfffffd000000000 x22: 1ffffff035b66e2f
x21: 00000000ffffffff x20: ffffff81adb3717c
x19: ffffff81adb37100 x18: 0000000000000339
x17: ffffffffffffffff x16: 0000000000000006
x15: 0000000000000001 x14: 0720072007200720
x13: 0000000000000000 x12: ffffffd0132c1e00
x11: 0000000000000001 x10: 0000000000000000
x9 : cff2cb5e2cb22e00 x8 : cff2cb5e2cb22e00
x7 : ffffffd010288f74 x6 : 0000000000000000
x5 : 0000000000000080 x4 : 0000000000000001
x3 : ffffffd010429d10 x2 : 0000000000000001
x1 : 0000000000000008 x0 : 0000000000000034
Call trace:
clk_core_disable+0x4a0/0x670
clk_disable+0x50/0x64
lpass_cpu_daiops_shutdown+0x190/0x204 [snd_soc_lpass_cpu]
snd_soc_dai_shutdown+0x8c/0x9c
soc_pcm_close+0x258/0x478
snd_pcm_release_substream+0xfc/0x1e8
snd_pcm_release+0x8c/0x124
__fput+0x1e4/0x4fc
____fput+0x1c/0x28
task_work_run+0x12c/0x164
do_notify_resume+0x1e04/0x2c48
work_pending+0x8/0x14
irq event stamp: 101552
-Doug
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-01 15:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-24 14:21 [PATCH v3] ASoC: qcom: lpass-cpu: Fix pop noise during audio capture begin Srinivasa Rao Mandadapu
2021-05-28 8:47 ` Srinivas Kandagatla
2021-06-01 15:17 ` Doug Anderson
2021-06-01 15:17 ` Doug Anderson
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.