This patch fixes PoP noise of around 15ms observed during audio capture begin.
Enables BCLK and LRCLK in snd_soc_dai_ops startup call for introducing some delay
before capture start and clock enable.
Signed-off-by: Judy Hsiao <[email protected]>
Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
sound/soc/qcom/lpass-cpu.c | 37 +++++++++++++++++++++++++++++++++----
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c
index c62d2612e8f5..3dcfc2f115a7 100644
--- a/sound/soc/qcom/lpass-cpu.c
+++ b/sound/soc/qcom/lpass-cpu.c
@@ -73,14 +73,28 @@ static int lpass_cpu_daiops_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct lpass_data *drvdata = snd_soc_dai_get_drvdata(dai);
- int ret;
+ struct lpaif_i2sctl *i2sctl = drvdata->i2sctl;
+ unsigned int id = dai->driver->id;
+ int ret = -EINVAL;
ret = clk_prepare_enable(drvdata->mi2s_osr_clk[dai->driver->id]);
if (ret) {
dev_err(dai->dev, "error in enabling mi2s osr clk: %d\n", ret);
return ret;
}
- ret = clk_prepare(drvdata->mi2s_bit_clk[dai->driver->id]);
+
+ 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);
+
+ ret = clk_prepare_enable(drvdata->mi2s_bit_clk[dai->driver->id]);
if (ret) {
dev_err(dai->dev, "error in enabling mi2s bit clk: %d\n", ret);
clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]);
@@ -93,9 +107,23 @@ 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;
+ int ret = -EINVAL;
clk_disable_unprepare(drvdata->mi2s_osr_clk[dai->driver->id]);
- clk_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
+ clk_disable_unprepare(drvdata->mi2s_bit_clk[dai->driver->id]);
+
+ if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+ ret = regmap_fields_write(i2sctl->spken, id,
+ LPAIF_I2SCTL_SPKEN_DISABLE);
+ } else {
+ ret = regmap_fields_write(i2sctl->micen, id,
+ LPAIF_I2SCTL_MICEN_DISABLE);
+ }
+ if (ret)
+ dev_err(dai->dev, "error writing to i2sctl reg: %d\n",
+ ret);
}
static int lpass_cpu_daiops_hw_params(struct snd_pcm_substream *substream,
@@ -308,7 +336,8 @@ static int lpass_cpu_daiops_trigger(struct snd_pcm_substream *substream,
ret);
clk_disable(drvdata->mi2s_bit_clk[dai->driver->id]);
-
+ break;
+ default:
break;
}
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On Thu, May 13, 2021 at 05:15:39PM +0530, Srinivasa Rao Mandadapu wrote:
> + 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);
> + }
This commit doesn't remove the matching update in triger() so we'd have
two redundant updates. I guess it's unlikely to be harmful but it looks
wrong/confusing.
Hi Brown,
Thanks for your review comments!!!
On 5/13/2021 7:01 PM, Mark Brown wrote:
> On Thu, May 13, 2021 at 05:15:39PM +0530, Srinivasa Rao Mandadapu wrote:
>
>> + 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);
>> + }
> This commit doesn't remove the matching update in triger() so we'd have
> two redundant updates. I guess it's unlikely to be harmful but it looks
> wrong/confusing.
Yes, It's not harmful, as clk_prepare_enable is enabling clock only once
but maintaining count.
As Some times in Suspend/resume Sequence not hitting startup/shutdown,
but Trigger, so for maintaining
consistency not removed in trigger.
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On Fri, May 14, 2021 at 12:20:46PM +0530, Srinivasa Rao Mandadapu wrote:
> On 5/13/2021 7:01 PM, Mark Brown wrote:
> > This commit doesn't remove the matching update in triger() so we'd have
> > two redundant updates. I guess it's unlikely to be harmful but it looks
> > wrong/confusing.
> Yes, It's not harmful, as clk_prepare_enable is enabling clock only once but
> maintaining count.
> As Some times in Suspend/resume Sequence not hitting startup/shutdown, but
> Trigger, so for maintaining
> consistency not removed in trigger.
This at least needs some commenting so that it's clear, it looks buggy
at the minute.