[alsa-devel] [PATCH] ASoC: twl6040: Workaround for headset DC offset caused pop noise
Both Headset DAC need to be enabled at the same time, before any of the output drivers are enabled.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Liam Girdwood lrg@ti.com --- include/linux/mfd/twl6040.h | 1 + sound/soc/codecs/twl6040.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h index d9e05ea..0bbb1e7 100644 --- a/include/linux/mfd/twl6040.h +++ b/include/linux/mfd/twl6040.h @@ -122,6 +122,7 @@
/* HSLCTL/R (0x10/0x11) fields */
+#define TWL6040_HSDACENA (1 << 0) #define TWL6040_HSDACMODE (1 << 1) #define TWL6040_HSDRVMODE (1 << 3)
diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index 93f8a59..924eaf3 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -91,6 +91,7 @@ struct twl6040_data { int pll_power_mode; int hs_power_mode; int hs_power_mode_locked; + int hs_dac_enabled; unsigned int clk_in; unsigned int sysclk; u16 hs_left_step; @@ -654,6 +655,43 @@ static int headset_power_mode(struct snd_soc_codec *codec, int high_perf) static int twl6040_hs_dac_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *kcontrol, int event) { + struct snd_soc_codec *codec = w->codec; + struct twl6040_data *priv = snd_soc_codec_get_drvdata(codec); + u8 hslctl, hsrctl; + + /* + * Workaround for Headset DC offset caused pop noise: + * Both HS DAC need to be turned on (before the HS driver) and off at + * the same time. + */ + if (SND_SOC_DAPM_EVENT_ON(event)) { + if (!priv->hs_dac_enabled++) { + hslctl = twl6040_read_reg_cache(codec, + TWL6040_REG_HSLCTL); + hslctl |= TWL6040_HSDACENA; + + hsrctl = twl6040_read_reg_cache(codec, + TWL6040_REG_HSRCTL); + hsrctl |= TWL6040_HSDACENA; + + twl6040_write(codec, TWL6040_REG_HSLCTL, hslctl); + twl6040_write(codec, TWL6040_REG_HSRCTL, hsrctl); + } + } else { + if (!--priv->hs_dac_enabled) { + hslctl = twl6040_read_reg_cache(codec, + TWL6040_REG_HSLCTL); + hslctl &= ~TWL6040_HSDACENA; + + hsrctl = twl6040_read_reg_cache(codec, + TWL6040_REG_HSRCTL); + hsrctl &= ~TWL6040_HSDACENA; + + twl6040_write(codec, TWL6040_REG_HSLCTL, hslctl); + twl6040_write(codec, TWL6040_REG_HSRCTL, hsrctl); + } + } + msleep(1); return 0; } @@ -1061,11 +1099,11 @@ static const struct snd_soc_dapm_widget twl6040_dapm_widgets[] = {
/* DACs */ SND_SOC_DAPM_DAC_E("HSDAC Left", "Headset Playback", - TWL6040_REG_HSLCTL, 0, 0, + SND_SOC_NOPM, 0, 0, twl6040_hs_dac_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_DAC_E("HSDAC Right", "Headset Playback", - TWL6040_REG_HSRCTL, 0, 0, + SND_SOC_NOPM, 0, 0, twl6040_hs_dac_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD), SND_SOC_DAPM_DAC_E("HFDAC Left", "Handsfree Playback",
On Tue, Oct 11, 2011 at 02:00:13PM +0300, Peter Ujfalusi wrote:
Both Headset DAC need to be enabled at the same time, before any of the output drivers are enabled.
This sounds like you should be using a supply widget to do the reference counting here?
On Tuesday 11 October 2011 14:29:21 Mark Brown wrote:
On Tue, Oct 11, 2011 at 02:00:13PM +0300, Peter Ujfalusi wrote:
Both Headset DAC need to be enabled at the same time, before any of the output drivers are enabled.
This sounds like you should be using a supply widget to do the reference counting here?
Probably it is better to move the whole thing to be handled by a supply widget (attached to the HS DACs). It will not mess up the sequence, and I'm not going to need to count in the driver. Will rewrite the patch.
Thanks for the suggestion, Péter
On Wednesday 12 October 2011 11:18:50 Péter Ujfalusi wrote:
This sounds like you should be using a supply widget to do the reference counting here?
Probably it is better to move the whole thing to be handled by a supply widget (attached to the HS DACs). It will not mess up the sequence, and I'm not going to need to count in the driver. Will rewrite the patch.
I can not move the HS DAC power on/off to be handled by a supply widget... Refcounting in a supply widget will not work for the power down, since the supply widget will be turned off after the DAC widget. At time when the DAC receives the PMD event the attached supply is still on. I can update the commit message to explicitly say that we need to turn on both HS DAC at the same time, and we need to turn tem off at the same time.
-- Péter
On Wed, Oct 12, 2011 at 11:55:15AM +0300, Péter Ujfalusi wrote:
I can not move the HS DAC power on/off to be handled by a supply widget... Refcounting in a supply widget will not work for the power down, since the supply widget will be turned off after the DAC widget. At time when the DAC receives the PMD event the attached supply is still on. I can update the commit message to explicitly say that we need to turn on both HS DAC at the same time, and we need to turn tem off at the same time.
I still don't understand what the problem is. If nothing else you can do random register writes from an event on a supply widget...
On Wednesday 12 October 2011 10:46:25 Mark Brown wrote:
On Wed, Oct 12, 2011 at 11:55:15AM +0300, Péter Ujfalusi wrote:
I can not move the HS DAC power on/off to be handled by a supply widget... Refcounting in a supply widget will not work for the power down, since the supply widget will be turned off after the DAC widget. At time when the DAC receives the PMD event the attached supply is still on. I can update the commit message to explicitly say that we need to turn on both HS DAC at the same time, and we need to turn tem off at the same time.
I still don't understand what the problem is. If nothing else you can do random register writes from an event on a supply widget...
I still have the code for the out of order McPDM shutdown which is using a supply widget attached to the codec's DAC/ADC. The ordering of the two supply is not written in stone when both are attached to the same DAC. Yes, I'm evaluating the possibility of the reordering the shutdown sequence in the core as you have suggested a while back.
For this patch: I will merge the two DAC to one, and handle both of them in a single DAC widget. I have tested this and works fine.
-- Péter
On Wed, Oct 12, 2011 at 01:45:33PM +0300, Péter Ujfalusi wrote:
On Wednesday 12 October 2011 10:46:25 Mark Brown wrote:
I still don't understand what the problem is. If nothing else you can do random register writes from an event on a supply widget...
I still have the code for the out of order McPDM shutdown which is using a supply widget attached to the codec's DAC/ADC. The ordering of the two supply is not written in stone when both are attached to the same DAC.
SUPPLY_S.
participants (2)
-
Mark Brown -
Peter Ujfalusi