[PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[Pierre-Louis Bossart]

From: Jack Yu <jack.yu@realtek.com>

Add module parameter "power_up_delay" to fix pop noise on capture. The
power_up_delay value is set with a default value of 400ms, smaller
values are not recommended.

BugLink: https://github.com/thesofproject/linux/issues/1969
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Signed-off-by: Jack Yu <jack.yu@realtek.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/codecs/rt715.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
index 099c8bd20006..0cf10dec1e3b 100644
--- a/sound/soc/codecs/rt715.c
+++ b/sound/soc/codecs/rt715.c
@@ -37,6 +37,10 @@
 
 #include "rt715.h"
 
+static int power_up_delay = 400;
+module_param(power_up_delay, int, 0444);
+MODULE_PARM_DESC(power_up_delay, "RT715 power up delay time in ms");
+
 static int rt715_index_write(struct regmap *regmap, unsigned int reg,
 		unsigned int value)
 {
@@ -498,6 +502,7 @@ static int rt715_set_bias_level(struct snd_soc_component *component,
 			regmap_write(rt715->regmap,
 						RT715_SET_AUDIO_POWER_STATE,
 						AC_PWRST_D0);
+			msleep(power_up_delay);
 		}
 		break;
 
-- 
2.25.1

Re: [PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 543 bytes --]

On Wed, Sep 16, 2020 at 03:47:58PM -0500, Pierre-Louis Bossart wrote:
> From: Jack Yu <jack.yu@realtek.com>
> 
> Add module parameter "power_up_delay" to fix pop noise on capture. The
> power_up_delay value is set with a default value of 400ms, smaller
> values are not recommended.

Normally we would just add a delay in the driver unconditionally, why
make this a module paramter?  If there are board variations then we
should be getting them from board data, not forcing individual users to
bodge things with a module parameter.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[Pierre-Louis Bossart]

On 9/17/20 6:25 AM, Mark Brown wrote:
> On Wed, Sep 16, 2020 at 03:47:58PM -0500, Pierre-Louis Bossart wrote:
>> From: Jack Yu <jack.yu@realtek.com>
>>
>> Add module parameter "power_up_delay" to fix pop noise on capture. The
>> power_up_delay value is set with a default value of 400ms, smaller
>> values are not recommended.
> 
> Normally we would just add a delay in the driver unconditionally, why
> make this a module paramter?  If there are board variations then we
> should be getting them from board data, not forcing individual users to
> bodge things with a module parameter.

that wasn't the intent. 400ms is the recommended value, but the 
parameter provides a way to experiment without having to recompile 
during integration/debug stages.

It was my recommendation to add this parameter, I don't mind removing it 
if you prefer it that way. Or I can respin the commit message and 
comments to make it clearer what the intended use was.

Re: [PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

On Thu, Sep 17, 2020 at 08:00:39AM -0500, Pierre-Louis Bossart wrote:
> On 9/17/20 6:25 AM, Mark Brown wrote:

> > Normally we would just add a delay in the driver unconditionally, why
> > make this a module paramter?  If there are board variations then we
> > should be getting them from board data, not forcing individual users to
> > bodge things with a module parameter.

> that wasn't the intent. 400ms is the recommended value, but the parameter
> provides a way to experiment without having to recompile during
> integration/debug stages.

> It was my recommendation to add this parameter, I don't mind removing it if
> you prefer it that way. Or I can respin the commit message and comments to
> make it clearer what the intended use was.

It'd be better to just remove it.  If you want a facility to experiment
for testing debugfs is a better fit I think, unless this is a delay that
only gets done once during probe of course in which case that wouldn't
work.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rt715: Add module parameter to fix dmic pop sound issue.

[Pierre-Louis Bossart]

On 9/17/20 8:06 AM, Mark Brown wrote:
> On Thu, Sep 17, 2020 at 08:00:39AM -0500, Pierre-Louis Bossart wrote:
>> On 9/17/20 6:25 AM, Mark Brown wrote:
> 
>>> Normally we would just add a delay in the driver unconditionally, why
>>> make this a module paramter?  If there are board variations then we
>>> should be getting them from board data, not forcing individual users to
>>> bodge things with a module parameter.
> 
>> that wasn't the intent. 400ms is the recommended value, but the parameter
>> provides a way to experiment without having to recompile during
>> integration/debug stages.
> 
>> It was my recommendation to add this parameter, I don't mind removing it if
>> you prefer it that way. Or I can respin the commit message and comments to
>> make it clearer what the intended use was.
> 
> It'd be better to just remove it.  If you want a facility to experiment
> for testing debugfs is a better fit I think, unless this is a delay that
> only gets done once during probe of course in which case that wouldn't
> work.

ok, I'll send a v2.