[alsa-devel] [PATCH -next] ASoC: pandora: Add DAC regulator support
Pandora's external DAC is connected to VSIM TWL4030 supply, so let's start switching it too to save more power.
Also DAC got it's own DAPM handler and the delay was removed (it gave on audible improvement).
Signed-off-by: Grazvydas Ignotas notasas@gmail.com --- sound/soc/omap/omap3pandora.c | 37 +++++++++++++++++++++++++++++++------ 1 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/sound/soc/omap/omap3pandora.c b/sound/soc/omap/omap3pandora.c index 68980c1..b978411 100644 --- a/sound/soc/omap/omap3pandora.c +++ b/sound/soc/omap/omap3pandora.c @@ -22,7 +22,7 @@ #include <linux/clk.h> #include <linux/platform_device.h> #include <linux/gpio.h> -#include <linux/delay.h> +#include <linux/regulator/consumer.h>
#include <sound/core.h> #include <sound/pcm.h> @@ -40,6 +40,8 @@
#define PREFIX "ASoC omap3pandora: "
+static struct regulator *omap3pandora_dac_reg; + static int omap3pandora_cmn_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, unsigned int fmt) { @@ -106,21 +108,31 @@ static int omap3pandora_in_hw_params(struct snd_pcm_substream *substream, SND_SOC_DAIFMT_CBS_CFS); }
-static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w, +static int omap3pandora_dac_event(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { if (SND_SOC_DAPM_EVENT_ON(event)) { + regulator_enable(omap3pandora_dac_reg); gpio_set_value(OMAP3_PANDORA_DAC_POWER_GPIO, 1); - gpio_set_value(OMAP3_PANDORA_AMP_POWER_GPIO, 1); } else { - gpio_set_value(OMAP3_PANDORA_AMP_POWER_GPIO, 0); - mdelay(1); gpio_set_value(OMAP3_PANDORA_DAC_POWER_GPIO, 0); + regulator_disable(omap3pandora_dac_reg); }
return 0; }
+static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *k, int event) +{ + if (SND_SOC_DAPM_EVENT_ON(event)) + gpio_set_value(OMAP3_PANDORA_AMP_POWER_GPIO, 1); + else + gpio_set_value(OMAP3_PANDORA_AMP_POWER_GPIO, 0); + + return 0; +} + /* * Audio paths on Pandora board: * @@ -130,7 +142,9 @@ static int omap3pandora_hp_event(struct snd_soc_dapm_widget *w, * |P| <--- TWL4030 <--------- Line In and MICs */ static const struct snd_soc_dapm_widget omap3pandora_out_dapm_widgets[] = { - SND_SOC_DAPM_DAC("PCM DAC", "HiFi Playback", SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_DAC_E("PCM DAC", "HiFi Playback", SND_SOC_NOPM, + 0, 0, omap3pandora_dac_event, + SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), SND_SOC_DAPM_PGA_E("Headphone Amplifier", SND_SOC_NOPM, 0, 0, NULL, 0, omap3pandora_hp_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), @@ -306,8 +320,18 @@ static int __init omap3pandora_soc_init(void) goto fail2; }
+ omap3pandora_dac_reg = regulator_get(&omap3pandora_snd_device->dev, "vcc"); + if (IS_ERR(omap3pandora_dac_reg)) { + pr_err(PREFIX "Failed to get DAC regulator from %s: %ld\n", + dev_name(&omap3pandora_snd_device->dev), + PTR_ERR(omap3pandora_dac_reg)); + goto fail3; + } + return 0;
+fail3: + platform_device_del(omap3pandora_snd_device); fail2: platform_device_put(omap3pandora_snd_device); fail1: @@ -320,6 +344,7 @@ module_init(omap3pandora_soc_init);
static void __exit omap3pandora_soc_exit(void) { + regulator_put(omap3pandora_dac_reg); platform_device_unregister(omap3pandora_snd_device); gpio_free(OMAP3_PANDORA_AMP_POWER_GPIO); gpio_free(OMAP3_PANDORA_DAC_POWER_GPIO);
On Fri, 2010-02-05 at 15:13 +0200, Grazvydas Ignotas wrote:
Pandora's external DAC is connected to VSIM TWL4030 supply, so let's start switching it too to save more power.
Also DAC got it's own DAPM handler and the delay was removed (it gave on audible improvement).
That's good to hear (pun intended).
Signed-off-by: Grazvydas Ignotas notasas@gmail.com
sound/soc/omap/omap3pandora.c | 37 +++++++++++++++++++++++++++++++------ 1 files changed, 31 insertions(+), 6 deletions(-)
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Fri, Feb 5, 2010 at 4:08 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Fri, 2010-02-05 at 15:13 +0200, Grazvydas Ignotas wrote:
Pandora's external DAC is connected to VSIM TWL4030 supply, so let's start switching it too to save more power.
Also DAC got it's own DAPM handler and the delay was removed (it gave on audible improvement).
That's good to hear (pun intended).
Uh, I meant "no audible improvement". However I've just checked the DAC datasheet again and it requires 1ms delay between turning on/off the supply and switching /PD pin (connected to OMAP3_PANDORA_DAC_POWER_GPIO). Even though I hear no difference, adding the delays back just in case. Updated patch attached.
On Fri, 2010-02-05 at 16:29 +0200, Grazvydas Ignotas wrote:
On Fri, Feb 5, 2010 at 4:08 PM, Liam Girdwood lrg@slimlogic.co.uk wrote:
On Fri, 2010-02-05 at 15:13 +0200, Grazvydas Ignotas wrote:
Pandora's external DAC is connected to VSIM TWL4030 supply, so let's start switching it too to save more power.
Also DAC got it's own DAPM handler and the delay was removed (it gave on audible improvement).
That's good to hear (pun intended).
Uh, I meant "no audible improvement". However I've just checked the DAC datasheet again and it requires 1ms delay between turning on/off the supply and switching /PD pin (connected to OMAP3_PANDORA_DAC_POWER_GPIO). Even though I hear no difference, adding the delays back just in case. Updated patch attached. _______________________________________________
Btw, have you tried turning the volume to mute when the DAC is enabled. It may lessen the pop at switch on.
Acked-by: Liam Girdwood lrg@slimlogic.co.uk
On Fri, Feb 05, 2010 at 04:29:53PM +0200, Grazvydas Ignotas wrote:
Uh, I meant "no audible improvement". However I've just checked the DAC datasheet again and it requires 1ms delay between turning on/off the supply and switching /PD pin (connected to OMAP3_PANDORA_DAC_POWER_GPIO). Even though I hear no difference, adding the delays back just in case. Updated patch attached.
Which step of the powerup is the one that generates the pop? As Liam says if you can mute the DAC while powering up might help if that's possible, though looking I don't think it is.
From 72cdf36e432a9929baff8841df979f13492dc813 Mon Sep 17 00:00:00 2001 From: Grazvydas Ignotas notasas@gmail.com Date: Fri, 5 Feb 2010 12:57:37 +0200 Subject: [PATCH -next] ASoC: pandora: Add DAC regulator support
Please don't append patches to messages like this, it makes them harder to apply since the e-mail text ends up as part of the commit message as does the patch header.
Applied, thanks.
On Fri, Feb 5, 2010 at 7:20 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, Feb 05, 2010 at 04:29:53PM +0200, Grazvydas Ignotas wrote:
Uh, I meant "no audible improvement". However I've just checked the DAC datasheet again and it requires 1ms delay between turning on/off the supply and switching /PD pin (connected to OMAP3_PANDORA_DAC_POWER_GPIO). Even though I hear no difference, adding the delays back just in case. Updated patch attached.
Which step of the powerup is the one that generates the pop? As Liam says if you can mute the DAC while powering up might help if that's possible, though looking I don't think it is.
It's on powerdown, there is a faint pop. It appears its the amp (final part in the path), I tried to toggle it while playing /dev/zero manually and it always pops on powerdown. On powerup there is a small sound cutoff too, probably because of amp capacitors charging, but we decided to leave it instead of halting every sound playing program until the amp gets ready.
From 72cdf36e432a9929baff8841df979f13492dc813 Mon Sep 17 00:00:00 2001 From: Grazvydas Ignotas notasas@gmail.com Date: Fri, 5 Feb 2010 12:57:37 +0200 Subject: [PATCH -next] ASoC: pandora: Add DAC regulator support
Please don't append patches to messages like this, it makes them harder to apply since the e-mail text ends up as part of the commit message as does the patch header.
Applied, thanks.
noted, thanks.
On Sat, 6 Feb 2010 19:33:08 +0200 Grazvydas Ignotas notasas@gmail.com wrote:
It's on powerdown, there is a faint pop. It appears its the amp (final part in the path), I tried to toggle it while playing /dev/zero manually and it always pops on powerdown. On powerup there is a small sound cutoff too, probably because of amp capacitors charging, but we decided to leave it instead of halting every sound playing program until the amp gets ready.
My side comment for the /dev/zero test. You may hear pops also if /dev/zero is played as unsigned 8-bit samples on signed DAC since then samples gets converted to maximum negative value.
So there is a difference how the /dev/zero is played. First command below produces a pop because it's playing a pop and second produces a pop only if HW is producing it.
aplay /dev/zero aplay -f S16_LE /dev/zero
On Sun, Feb 7, 2010 at 11:07 AM, Jarkko Nikula jhnikula@gmail.com wrote:
On Sat, 6 Feb 2010 19:33:08 +0200 Grazvydas Ignotas notasas@gmail.com wrote:
It's on powerdown, there is a faint pop. It appears its the amp (final part in the path), I tried to toggle it while playing /dev/zero manually and it always pops on powerdown. On powerup there is a small sound cutoff too, probably because of amp capacitors charging, but we decided to leave it instead of halting every sound playing program until the amp gets ready.
My side comment for the /dev/zero test. You may hear pops also if /dev/zero is played as unsigned 8-bit samples on signed DAC since then samples gets converted to maximum negative value.
So there is a difference how the /dev/zero is played. First command below produces a pop because it's playing a pop and second produces a pop only if HW is producing it.
aplay /dev/zero aplay -f S16_LE /dev/zero
I was testing with '-f cd' which should also be signed. Actually doing just 'aplay /dev/zero' makes matters worse, there is some additional 'squeek' noise audible when I turn the volume up.
On Mon, Feb 8, 2010 at 1:39 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
If this goes off too much you might want to tweak the delay ASoC uses to defer power down of DAPM widgets after streams stop. There's a command line option pmdown_time, it really ought to have a sysfs file to tweak but there's not been any demand.
Yes we are already using that (set to 1 minute), and the sysfs file would be useful for us (we have a hack for that in our tree).
On Sat, Feb 06, 2010 at 07:33:08PM +0200, Grazvydas Ignotas wrote:
On Fri, Feb 5, 2010 at 7:20 PM, Mark Brown
Which step of the powerup is the one that generates the pop? As Liam says if you can mute the DAC while powering up might help if that's possible, though looking I don't think it is.
It's on powerdown, there is a faint pop. It appears its the amp (final part in the path), I tried to toggle it while playing /dev/zero manually and it always pops on powerdown. On powerup there is a small
Like Jarkko says be careful what you're actually playing there - especially given the ramp you appear to have at startup which would mask any sudden DC offset in the samples at startup.
sound cutoff too, probably because of amp capacitors charging, but we decided to leave it instead of halting every sound playing program until the amp gets ready.
If this goes off too much you might want to tweak the delay ASoC uses to defer power down of DAPM widgets after streams stop. There's a command line option pmdown_time, it really ought to have a sysfs file to tweak but there's not been any demand.
participants (4)
-
Grazvydas Ignotas -
Jarkko Nikula -
Liam Girdwood -
Mark Brown