[PATCH 0/1] ASoC: TWL4030: Possible fix for the Beep at the end of the audio file.

[PATCH 0/1] ASoC: TWL4030: Possible fix for the Beep at the end of the audio file.

[Peter Ujfalusi]

Hello,

I have finally managed to reproduce and test the 'tuck' phenomena.
The following patch seamingly fixes the issue at least in my hardware setup.

Note that I still can not test this on Beagle and on the HW that I have testing
the headset output is kind of tricky, so this might very well not fix the issue.

Anuj, Jarkko, Arun: Would you be so kind and test it on your boards?

---
Peter Ujfalusi (1):
  ASoC: TWL4030: Headset VMID ramp fix

 sound/soc/codecs/twl4030.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

[PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Peter Ujfalusi]

This patch fixes the 'tuck' phenomena observed on the headset
output after the streams are stopped and DAPM send the codec
to standby mode.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 sound/soc/codecs/twl4030.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
index fd392c6..06042a1 100644
--- a/sound/soc/codecs/twl4030.c
+++ b/sound/soc/codecs/twl4030.c
@@ -599,7 +599,7 @@ static int headsetl_event(struct snd_soc_dapm_widget *w,
 		/* Is this needed? Can we just use whatever gain here? */
 		twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET,
 				(hs_gain & (~0x0f)) | 0x0a);
-		hs_pop |= TWL4030_RAMP_EN;
+		hs_pop &= ~TWL4030_VMID_EN;
 		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
 
 		/* Restore the original volume */
@@ -613,7 +613,7 @@ static int headsetl_event(struct snd_soc_dapm_widget *w,
 		twl4030_i2c_write_u8(TWL4030_MODULE_AUDIO_VOICE,
 					hs_gain & (~0x0f),
 					TWL4030_REG_HS_GAIN_SET);
-		hs_pop &= ~TWL4030_VMID_EN;
+		hs_pop |= TWL4030_RAMP_EN;
 		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET, hs_pop);
 		break;
 	}
-- 
1.6.2.3

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Jarkko Nikula]

On Fri,  8 May 2009 09:11:17 +0300
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> This patch fixes the 'tuck' phenomena observed on the headset
> output after the streams are stopped and DAPM send the codec
> to standby mode.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
> ---
>  sound/soc/codecs/twl4030.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/twl4030.c b/sound/soc/codecs/twl4030.c
> index fd392c6..06042a1 100644
> --- a/sound/soc/codecs/twl4030.c
> +++ b/sound/soc/codecs/twl4030.c
> @@ -599,7 +599,7 @@ static int headsetl_event(struct
> snd_soc_dapm_widget *w, /* Is this needed? Can we just use whatever
> gain here? */ twl4030_write(w->codec, TWL4030_REG_HS_GAIN_SET,
>  				(hs_gain & (~0x0f)) | 0x0a);
> -		hs_pop |= TWL4030_RAMP_EN;
> +		hs_pop &= ~TWL4030_VMID_EN;
>  		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET,
> hs_pop); 
>  		/* Restore the original volume */
> @@ -613,7 +613,7 @@ static int headsetl_event(struct
> snd_soc_dapm_widget *w, twl4030_i2c_write_u8
> (TWL4030_MODULE_AUDIO_VOICE, hs_gain & (~0x0f),
>  					TWL4030_REG_HS_GAIN_SET);
> -		hs_pop &= ~TWL4030_VMID_EN;
> +		hs_pop |= TWL4030_RAMP_EN;
>  		twl4030_write(w->codec, TWL4030_REG_HS_POPN_SET,

It really does fix the power-up & -down pops on Beagle. By listening
carefully I can hear slight power-up and -down pops but only if really
focusing on it.

But now the audio output is distorted and volume level is a little bit
lower. Without the patch it plays fine. Is it possible that now the
output voltage is not correctly biased? Worth to check with a scope.


-- 
Jarkko

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Peter Ujfalusi]

On Friday 08 May 2009 09:40:44 ext Jarkko Nikula wrote:
> It really does fix the power-up & -down pops on Beagle. By listening
> carefully I can hear slight power-up and -down pops but only if really
> focusing on it.
>
> But now the audio output is distorted and volume level is a little bit
> lower. Without the patch it plays fine. Is it possible that now the
> output voltage is not correctly biased? Worth to check with a scope.

Well, that's what you get, when you are testing audio with /dev/zero and 
/dev/urandom... I have not noticed that the random noise got more random ;)
What you have described is correct, it is heavily distorted.

But I have debugged this further and I believe I know what is going on:
The sequence for disable according to the documentation:
HS_POP_SET:RAMP_EN = 0
HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0
HS_POP_SET:VMID_EN = 0

Now what is missing from this is that after setting the HS_POP_SET:RAMP_EN = 0 
it will take RAMP_DELAY time for the VMID to reach 0.
Since we are not waiting between RAMP_EN = 0 and VMID_EN = 0, the VMID has 
been cut, which causes the 'tuck' on the headset output.

In case of the Beagle board the ramp delay can be configured(MCLK=26MHz): 20ms 
- 2581ms, by default it is 20ms.
To verify this I have added mdelay(30) after the RAMP_EN = 0, and there were 
no 'tuck'. However when I increased the RAMP_DELAY (to 40ms, or longer) the 
'tuck' reappeared.

How to solve this?
Realistically we can not add 2581ms delay to the headsetl_event function...
We could keep the VMID enabled all the time (which increases the idle power 
consumption by ~0.001A), but then the muting of the headset 
(HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0) will give the 'tuck'...
In power consumption the difference between
HS_POP_GAIN:HSL_GAIN, HSR_GAIN == 0 and
HS_POP_GAIN:HSL_GAIN, HSR_GAIN != 0 is about ~0.01A, which is significant in 
my opinion.
Using a timer to finish the ramp down?

Anyways, this seams quite bad. I will think about it over the weekend...

Any ideas?

-- 
Péter

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Peter Ujfalusi]

On Friday 08 May 2009 14:56:50 Ujfalusi Peter (Nokia-D/Tampere) wrote:
> On Friday 08 May 2009 09:40:44 ext Jarkko Nikula wrote:
> > It really does fix the power-up & -down pops on Beagle. By listening
> > carefully I can hear slight power-up and -down pops but only if really
> > focusing on it.
> >
> > But now the audio output is distorted and volume level is a little bit
> > lower. Without the patch it plays fine. Is it possible that now the
> > output voltage is not correctly biased? Worth to check with a scope.
>
> Well, that's what you get, when you are testing audio with /dev/zero and
> /dev/urandom... I have not noticed that the random noise got more random ;)
> What you have described is correct, it is heavily distorted.
>
> But I have debugged this further and I believe I know what is going on:
> The sequence for disable according to the documentation:
> HS_POP_SET:RAMP_EN = 0
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0
> HS_POP_SET:VMID_EN = 0
>
> Now what is missing from this is that after setting the HS_POP_SET:RAMP_EN
> = 0 it will take RAMP_DELAY time for the VMID to reach 0.
> Since we are not waiting between RAMP_EN = 0 and VMID_EN = 0, the VMID has
> been cut, which causes the 'tuck' on the headset output.
>
> In case of the Beagle board the ramp delay can be configured(MCLK=26MHz):
> 20ms - 2581ms, by default it is 20ms.
> To verify this I have added mdelay(30) after the RAMP_EN = 0, and there
> were no 'tuck'. However when I increased the RAMP_DELAY (to 40ms, or
> longer) the 'tuck' reappeared.
>
> How to solve this?
> Realistically we can not add 2581ms delay to the headsetl_event function...
> We could keep the VMID enabled all the time (which increases the idle power
> consumption by ~0.001A), but then the muting of the headset
> (HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0) will give the 'tuck'...
> In power consumption the difference between
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN == 0 and
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN != 0 is about ~0.01A, which is significant
> in my opinion.
> Using a timer to finish the ramp down?
>
> Anyways, this seams quite bad. I will think about it over the weekend...
>
> Any ideas?

We could fixate the RAMP_DELAY to 0 (27/20/14 ms, MCLK = 19.2/26/38.4 MHz) and 
add mdelay(28/21/15) in the headsetl_event function... But I'm not sure is it 
allowed, at least it looks hackish. Mark?

-- 
Péter

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Mark Brown]

On Fri, May 08, 2009 at 02:56:50PM +0300, Peter Ujfalusi wrote:

> Realistically we can not add 2581ms delay to the headsetl_event function...
> We could keep the VMID enabled all the time (which increases the idle power 
> consumption by ~0.001A), but then the muting of the headset 

Generally you want to leave VMID up all the time for exactly this reason
- ramping it cleanly with that generation of hardware just takes too
long to do it regularly during normal operation.

> (HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0) will give the 'tuck'...

Have you tried ramping the gain down gradually?  DAPM can do that for you
automatically if the volume control is attached to the PGA.

> In power consumption the difference between
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN == 0 and
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN != 0 is about ~0.01A, which is significant in 
> my opinion.

Yup, that's definitely going to be noticable.

> Using a timer to finish the ramp down?

How well does the device cope with aborting the ramp down?  If it can
handle it then that seems like a reasonable approach.  Othwerise it's
sounding like you may have to give people the option of either living
the pop or having the very slow ramp.

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Mark Brown]

On Fri, May 08, 2009 at 03:14:17PM +0300, Peter Ujfalusi wrote:

> We could fixate the RAMP_DELAY to 0 (27/20/14 ms, MCLK = 19.2/26/38.4 MHz) and 
> add mdelay(28/21/15) in the headsetl_event function... But I'm not sure is it 
> allowed, at least it looks hackish. Mark?

A short delay there wouldn't be ideal but shouldn't break anything
either - the power down will be happening out of line after the pop wait
so it's not as critical as it might be.

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Peter Ujfalusi]

On Friday 08 May 2009 15:32:08 ext Mark Brown wrote:
> On Fri, May 08, 2009 at 02:56:50PM +0300, Peter Ujfalusi wrote:
> > Realistically we can not add 2581ms delay to the headsetl_event
> > function... We could keep the VMID enabled all the time (which increases
> > the idle power consumption by ~0.001A), but then the muting of the
> > headset
>
> Generally you want to leave VMID up all the time for exactly this reason
> - ramping it cleanly with that generation of hardware just takes too
> long to do it regularly during normal operation.
>
> > (HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0) will give the 'tuck'...
>
> Have you tried ramping the gain down gradually?  DAPM can do that for you
> automatically if the volume control is attached to the PGA.

I'll try that on Monday, but is it a problem that the 
HSL_GAIN and HSR_GAIN looks like this:
0 == Off
1 == 6 dB
2 == 0 dB
3 == -6 dB

-- 
Péter

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Jarkko Nikula]

On Fri, 8 May 2009 14:56:50 +0300
Peter Ujfalusi <peter.ujfalusi@nokia.com> wrote:

> But I have debugged this further and I believe I know what is going
> on: The sequence for disable according to the documentation:
> HS_POP_SET:RAMP_EN = 0
> HS_POP_GAIN:HSL_GAIN, HSR_GAIN = 0
> HS_POP_SET:VMID_EN = 0
> 
> Now what is missing from this is that after setting the
> HS_POP_SET:RAMP_EN = 0 it will take RAMP_DELAY time for the VMID to
> reach 0. Since we are not waiting between RAMP_EN = 0 and VMID_EN =
> 0, the VMID has been cut, which causes the 'tuck' on the headset
> output.
> 
I was looking the power-up phase and there, as far as I understand,
RAMP_DELAY is HW driven and no SW delays required like in power-down
path, code does power-up path correctly but still there is a pop. This
made me thinking the idea below.

I don't have TWL4030 TRM but AFAIK, the TPS65950 is the same chip and
I was looking HS power sequencing from page 691 in TPS65950 TRM.

http://focus.ti.com/docs/prod/folders/print/tps65950.html
http://www.ti.com/litv/pdf/swcu050d

> Anyways, this seams quite bad. I will think about it over the
> weekend...
> 
> Any ideas?
> 
Would it be possible that pop is not caused by the HS output but
inside the chip, like from DAC?

One nice method to hunt these is to use gpio to trigger scope at the
beginning of sequencing and toggle it at each step so it would be easy
to see what step is causing the strong pop and what kind of delays
are required to mask it.


-- 
Jarkko

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Jarkko Nikula]

On Fri, 8 May 2009 16:55:01 +0300
Jarkko Nikula <jhnikula@gmail.com> wrote:

> Would it be possible that pop is not caused by the HS output but
> inside the chip, like from DAC?
> 
Seems to be not the case. Pops are there even if I put msleep(250)
before and after playing with the HS.

A digital scope would definitely help here to see are there ramp and is
the coupling configuration correct.


-- 
Jarkko

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Peter Ujfalusi]

On Friday 08 May 2009 17:23:22 ext Jarkko Nikula wrote:
> On Fri, 8 May 2009 16:55:01 +0300
>
> Jarkko Nikula <jhnikula@gmail.com> wrote:
> > Would it be possible that pop is not caused by the HS output but
> > inside the chip, like from DAC?
>
> Seems to be not the case. Pops are there even if I put msleep(250)
> before and after playing with the HS.
>
> A digital scope would definitely help here to see are there ramp and is
> the coupling configuration correct.

OK I got the scope hooked up.
Startup sequence on HS looks fine:
TWL4030_VMID_EN
TWL4030_RAMP_EN
HS gain restore.

On the shutdown sequence part I have the following:
~TWL4030_RAMP_EN
msleep(30)
~TWL4030_VMID_EN
HS gain mute

When the RAMP_DELAY = 0 (27/20/14 ms) the voltage on the HSOL line gradually 
ramps down.
If I set the RAMP_DELAY = 1 (55/40/27 ms) the voltage on HSOL line starts to 
gradually ramp down, but close to the end there is a clear cut (voltage jumps 
to 0, after around 30ms from the start of the ramp down). If I further 
increase the RAMP_DELAY, the cut happens at the same time, but it falls from 
even higher voltage to 0.

I think the solution for now is to fix the RAMP_DELAY to 0 (27/20/14 ms), and 
add the delay for the shutdown case.

Also I have tried gradually decrease the volume on the HS output. It does not 
seams to be working on the twl4030 codec: when the HSL_GAIN, HSR_GAIN is set 
to 0 it effectively cuts the output (HSOL line drops to zero -> 'tuck' can be 
heard)

Mark: what do you think, is it acceptable to add a maximum of 27ms delay to 
the hsol_event:SND_SOC_DAPM_POST_PMD event?

-- 
Péter

Re: [PATCH 1/1] ASoC: TWL4030: Headset VMID ramp fix

[Mark Brown]

On Mon, May 11, 2009 at 09:22:41AM +0300, Peter Ujfalusi wrote:

> Mark: what do you think, is it acceptable to add a maximum of 27ms delay to 
> the hsol_event:SND_SOC_DAPM_POST_PMD event?

Yes, that's fine.