[PATCH v1] ASoC: sof_es8336: reduce pop noise on speaker
The Speaker GPIO needs to be turned on slightly behind the codec turned on. It also need to be turned off slightly before the codec turned down. Current code uses delay in DAPM_EVENT to do it but the mdelay delays the DAPM itself and thus has no effect. A delayed_work is added to turn on the speaker. The Speaker is turned off in .trigger since trigger is called slightly before the DAPM events.
Signed-off-by: Zhu Ning zhuning@everest-semi.com
------------
v1: cancel delayed work while disabling speaker. --- sound/soc/intel/boards/sof_es8336.c | 60 ++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/sound/soc/intel/boards/sof_es8336.c b/sound/soc/intel/boards/sof_es8336.c index fbb42e54947a..70713e4b07dc 100644 --- a/sound/soc/intel/boards/sof_es8336.c +++ b/sound/soc/intel/boards/sof_es8336.c @@ -63,6 +63,7 @@ struct sof_es8336_private { struct snd_soc_jack jack; struct list_head hdmi_pcm_list; bool speaker_en; + struct delayed_work pcm_pop_work; };
struct sof_hdmi_pcm { @@ -111,6 +112,46 @@ static void log_quirks(struct device *dev) dev_info(dev, "quirk headset at mic1 port enabled\n"); }
+static void pcm_pop_work_events(struct work_struct *work) +{ + struct sof_es8336_private *priv = + container_of(work, struct sof_es8336_private, pcm_pop_work.work); + + gpiod_set_value_cansleep(priv->gpio_speakers, priv->speaker_en); + + if (quirk & SOF_ES8336_HEADPHONE_GPIO) + gpiod_set_value_cansleep(priv->gpio_headphone, priv->speaker_en); + +} + +static int sof_8336_trigger(struct snd_pcm_substream *substream, int cmd) +{ + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_card *card = rtd->card; + struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card); + + switch (cmd) { + case SNDRV_PCM_TRIGGER_START: + case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + case SNDRV_PCM_TRIGGER_RESUME: + break; + + case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_STOP: + if (priv->speaker_en == false) + if (substream->stream == 0) { + cancel_delayed_work(&priv->pcm_pop_work); + gpiod_set_value_cansleep(priv->gpio_speakers, true); + } + break; + default: + return -EINVAL; + } + + return 0; +} + static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { @@ -122,19 +163,7 @@ static int sof_es8316_speaker_power_event(struct snd_soc_dapm_widget *w,
priv->speaker_en = !SND_SOC_DAPM_EVENT_ON(event);
- if (SND_SOC_DAPM_EVENT_ON(event)) - msleep(70); - - gpiod_set_value_cansleep(priv->gpio_speakers, priv->speaker_en); - - if (!(quirk & SOF_ES8336_HEADPHONE_GPIO)) - return 0; - - if (SND_SOC_DAPM_EVENT_ON(event)) - msleep(70); - - gpiod_set_value_cansleep(priv->gpio_headphone, priv->speaker_en); - + queue_delayed_work(system_wq, &priv->pcm_pop_work, msecs_to_jiffies(70)); return 0; }
@@ -344,6 +373,7 @@ static int sof_es8336_hw_params(struct snd_pcm_substream *substream, /* machine stream operations */ static struct snd_soc_ops sof_es8336_ops = { .hw_params = sof_es8336_hw_params, + .trigger = sof_8336_trigger, };
static struct snd_soc_dai_link_component platform_component[] = { @@ -723,7 +753,8 @@ static int sof_es8336_probe(struct platform_device *pdev) }
INIT_LIST_HEAD(&priv->hdmi_pcm_list); - + INIT_DELAYED_WORK(&priv->pcm_pop_work, + pcm_pop_work_events); snd_soc_card_set_drvdata(card, priv);
if (mach->mach_params.dmic_num > 0) { @@ -752,6 +783,7 @@ static int sof_es8336_remove(struct platform_device *pdev) struct snd_soc_card *card = platform_get_drvdata(pdev); struct sof_es8336_private *priv = snd_soc_card_get_drvdata(card);
+ cancel_delayed_work(&priv->pcm_pop_work); gpiod_put(priv->gpio_speakers); device_remove_software_node(priv->codec_dev); put_device(priv->codec_dev);
On Fri, 28 Oct 2022 10:04:56 +0800, Zhu Ning wrote:
The Speaker GPIO needs to be turned on slightly behind the codec turned on. It also need to be turned off slightly before the codec turned down. Current code uses delay in DAPM_EVENT to do it but the mdelay delays the DAPM itself and thus has no effect. A delayed_work is added to turn on the speaker. The Speaker is turned off in .trigger since trigger is called slightly before the DAPM events.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: sof_es8336: reduce pop noise on speaker commit: 89cdb224f2abe37ec4ac21ba0d9ddeb5a6a9cf68
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
participants (2)
-
Mark Brown
-
Zhu Ning