tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mike Looijmans]

My general question: If "my" codec needs to alter the state of the DAPM 
tree, how can the codec ask or inform the DAPM components of this?


This problem occurs on tlv320aic32x4 codecs, but probably applies to 
many others as well.

Using the tlv320aic32x4 a simple loop like this may "crash" the codec:

while true
do
    speaker-test -c 2 -t sine -l 1 -r 48000 > /dev/null
    speaker-test -c 2 -t sine -l 1 -r 44100 > /dev/null
done 


A "crash" means, in this respect that the chip loses some of its 
settings, malfunctions and often it results in NACKs on I2C commands.

I've been trying to properly fix this, but it just won't fit in the 
framework.

The cause is that the DAPM starts the DAC and/or ADC cores while the 
clocks aren't running, and stops the clocks before shutting down the cores.

There is the overall bug of the driver that starts and stops all the 
clocks in the aic32x4_set_bias_level in the wrong order - it should 
start them from source to sink, and stop them in the opposite direction, 
so the PLL must be the first to start and last to stop. Apparently this 
isn't really critical as I've never observed failure as a result of this.

What I cannot fix is that changing the parameters (like clock frequency) 
must stop the DAC core.

The DAPM framework just sets the BIAS to "ON" only once, and does not 
shut down until after a time out. Which is good.
But if the next stream to play needs a different clock setting, the DAC 
must be stopped first, the code must wait until the DAC has powered 
down, and then the clocks can be stopped and altered. When all settings 
are done, the DAC can be restarted.

Starting the DAC is the DAPM's task, it's this line that does the job:

SND_SOC_DAPM_DAC("Left DAC", "Left Playback", AIC32X4_DACSETUP, 7, 0),

I can detect the change in the hw_params, and on the first stream to 
play, this is called while still in STANDBY or lower levels. However, 
when the codec is already in ON bias level, how do I tell DAPM that I 
had to shut down the DAC in order to change the parameters? Just 
toggling the related bits will keep the DAC powered down. A routine to 
store and restore the power state is possible, but very ineffective. 
Especially since the "I must shutdown" conclusion can also be in the 
clock setting routine. That would result in multiple down-up cycles.


-- 
Mike Looijmans

Re: tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1415 bytes --]

On Wed, Mar 12, 2014 at 04:08:40PM +0100, Mike Looijmans wrote:

> What I cannot fix is that changing the parameters (like clock
> frequency) must stop the DAC core.

> The DAPM framework just sets the BIAS to "ON" only once, and does
> not shut down until after a time out. Which is good.
> But if the next stream to play needs a different clock setting, the
> DAC must be stopped first, the code must wait until the DAC has
> powered down, and then the clocks can be stopped and altered. When
> all settings are done, the DAC can be restarted.

As a first order fix the driver should just refuse to reconfigure if the
clocks are active.

> I can detect the change in the hw_params, and on the first stream to
> play, this is called while still in STANDBY or lower levels.
> However, when the codec is already in ON bias level, how do I tell
> DAPM that I had to shut down the DAC in order to change the
> parameters? Just toggling the related bits will keep the DAC powered
> down. A routine to store and restore the power state is possible,
> but very ineffective. Especially since the "I must shutdown"
> conclusion can also be in the clock setting routine. That would
> result in multiple down-up cycles.

The big problem with shutting down is disruption to other activity - if
there's something going on using the clocks.  For some devices that
don't have long startup times ignore_pmdow_time may help a lot.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mike Looijmans]

On 03/13/2014 12:31 AM, Mark Brown wrote:
> On Wed, Mar 12, 2014 at 04:08:40PM +0100, Mike Looijmans wrote:
>
>> What I cannot fix is that changing the parameters (like clock
>> frequency) must stop the DAC core.
>
>> The DAPM framework just sets the BIAS to "ON" only once, and does
>> not shut down until after a time out. Which is good.
>> But if the next stream to play needs a different clock setting, the
>> DAC must be stopped first, the code must wait until the DAC has
>> powered down, and then the clocks can be stopped and altered. When
>> all settings are done, the DAC can be restarted.
>
> As a first order fix the driver should just refuse to reconfigure if the
> clocks are active.

Which would result in the driver often failing to work and the user 
having no clue why.

>> I can detect the change in the hw_params, and on the first stream to
>> play, this is called while still in STANDBY or lower levels.
>> However, when the codec is already in ON bias level, how do I tell
>> DAPM that I had to shut down the DAC in order to change the
>> parameters? Just toggling the related bits will keep the DAC powered
>> down. A routine to store and restore the power state is possible,
>> but very ineffective. Especially since the "I must shutdown"
>> conclusion can also be in the clock setting routine. That would
>> result in multiple down-up cycles.
>
> The big problem with shutting down is disruption to other activity - if
> there's something going on using the clocks.  For some devices that
> don't have long startup times ignore_pmdow_time may help a lot.

The bias level can remain where it is, it's just the DACSETUP or 
ADCSETUP bits that need toggling. They're independent, so only the 
affected parts need to stop. Using ignore_pmdow_time would result in the 
codec going to a lower bias state, and that is not what should happen 
here. Restarting the ADC or DAC is just a matter of milliseconds, but a 
complete power-down will result in reference voltage instability and 
capacitors recharging, and it may take several minutes for the system to 
completely recover from that.

I'm mostly asking to think about the "big picture" here. As a generic 
case, not specific to this codec.

-- 
Mike Looijmans

Re: tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]

On Thu, Mar 13, 2014 at 08:09:25AM +0100, Mike Looijmans wrote:
> On 03/13/2014 12:31 AM, Mark Brown wrote:

> >As a first order fix the driver should just refuse to reconfigure if the
> >clocks are active.

> Which would result in the driver often failing to work and the user having
> no clue why.

Print an error message.

> >The big problem with shutting down is disruption to other activity - if
> >there's something going on using the clocks.  For some devices that
> >don't have long startup times ignore_pmdow_time may help a lot.

> The bias level can remain where it is, it's just the DACSETUP or ADCSETUP
> bits that need toggling. They're independent, so only the affected parts
> need to stop. Using ignore_pmdow_time would result in the codec going to a
> lower bias state, and that is not what should happen here. Restarting the
> ADC or DAC is just a matter of milliseconds, but a complete power-down will
> result in reference voltage instability and capacitors recharging, and it
> may take several minutes for the system to completely recover from that.

*Any* glitch in an active digital audio path will be noticable to users, 

If you're taking several minutes to do a bias level transition there's
something seriously wrong with either the hardware or with the way it's
being managed by the driver.

> I'm mostly asking to think about the "big picture" here. As a generic case,
> not specific to this codec.

The general solution here is essentially don't do that; if the hardware
is fragile on reconfiguration you will tend to end up hitting cases
where trying to do a reconfiguration will glitch or fail so the sound
server will generally fix the configuration and handle things in
software.  Otherwise pmdown_time is the general solution for modern
devices with quick startup/shutdown times, older devices probably need
some custom hacks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mike Looijmans]

On 03/13/2014 03:04 PM, Mark Brown wrote:
> On Thu, Mar 13, 2014 at 08:09:25AM +0100, Mike Looijmans wrote:
>> On 03/13/2014 12:31 AM, Mark Brown wrote:
>
>>> As a first order fix the driver should just refuse to reconfigure if the
>>> clocks are active.
>
>> Which would result in the driver often failing to work and the user having
>> no clue why.
>
> Print an error message.
>
>>> The big problem with shutting down is disruption to other activity - if
>>> there's something going on using the clocks.  For some devices that
>>> don't have long startup times ignore_pmdow_time may help a lot.
>
>> The bias level can remain where it is, it's just the DACSETUP or ADCSETUP
>> bits that need toggling. They're independent, so only the affected parts
>> need to stop. Using ignore_pmdow_time would result in the codec going to a
>> lower bias state, and that is not what should happen here. Restarting the
>> ADC or DAC is just a matter of milliseconds, but a complete power-down will
>> result in reference voltage instability and capacitors recharging, and it
>> may take several minutes for the system to completely recover from that.
>
> *Any* glitch in an active digital audio path will be noticable to users,

The reconfiguration only occurs when switching from say 44100 to 48000 
sampling rates (but not when going from 24 to 16 bits, for example). I don't 
think users will actually expect a switch from one sampling rate to another to 
proceed without any glitch or delay.

> If you're taking several minutes to do a bias level transition there's
> something seriously wrong with either the hardware or with the way it's
> being managed by the driver.

Good enough for human ears, but this project uses codecs for measurement and 
is sensitive to microvolt changes in reference voltages and such. The audible 
effects are gone in less than a second, but the measurable effects last a lot 
longer.


>> I'm mostly asking to think about the "big picture" here. As a generic case,
>> not specific to this codec.
>
> The general solution here is essentially don't do that; if the hardware
> is fragile on reconfiguration you will tend to end up hitting cases
> where trying to do a reconfiguration will glitch or fail so the sound
> server will generally fix the configuration and handle things in
> software.  Otherwise pmdown_time is the general solution for modern
> devices with quick startup/shutdown times, older devices probably need
> some custom hacks.

It's not about "fragile", it's about managing codec power states. For some 
transistions, like clock changes, a codec will likely need to go to a lower 
state. The current core does not even attempt to mute the DAC for example.

Well, custom hacks is 95% of what i've done to the driver (and I hacked a few 
lines in the core as well to get what I wanted). I was hoping to lower the 
"custom" part a bit in future.



Met vriendelijke groet / kind regards,

Mike Looijmans

TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: tlv320aic32x4 Codec ADC and DAC must shutdown to alter clocks

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2943 bytes --]

On Fri, Mar 14, 2014 at 10:47:50AM +0100, Mike Looijmans wrote:

Don't drop CCs on kernel lists if you want people to read replies - the
bit about always CCing maintainers applies too...

> >*Any* glitch in an active digital audio path will be noticable to users,

> The reconfiguration only occurs when switching from say 44100 to 48000
> sampling rates (but not when going from 24 to 16 bits, for example). I don't
> think users will actually expect a switch from one sampling rate to another
> to proceed without any glitch or delay.

This has a wider effect than just the digital playback path though
doesn't it - the capture path for example, and if this is a modern
device there's doubtless charge pumps in play.

> >If you're taking several minutes to do a bias level transition there's
> >something seriously wrong with either the hardware or with the way it's
> >being managed by the driver.

> Good enough for human ears, but this project uses codecs for measurement and
> is sensitive to microvolt changes in reference voltages and such. The
> audible effects are gone in less than a second, but the measurable effects
> last a lot longer.

Less than a second still sounds like an awfully long time here and if
you've got constraints like this it sounds like you need to force the
device to be always running anyway.  Glancing at the code we're only
managing clocks in the bias level operations...

> >The general solution here is essentially don't do that; if the hardware
> >is fragile on reconfiguration you will tend to end up hitting cases
> >where trying to do a reconfiguration will glitch or fail so the sound
> >server will generally fix the configuration and handle things in
> >software.  Otherwise pmdown_time is the general solution for modern
> >devices with quick startup/shutdown times, older devices probably need
> >some custom hacks.

> It's not about "fragile", it's about managing codec power states. For some
> transistions, like clock changes, a codec will likely need to go to a lower

It certainly sounds a lot like it's fragile - there seem to be fairly
tight constraints on reconfiguring, you're saying it's hard to implement
something that bounces the power around reconfiguration, the device
requires symmetric rates so it won't cope if there's capture activity
too and who knows what happens with the analogue bypass paths.

Equally well you've not been specific about anything so it's possible
that the restrictions and/or what needs doing are looser than you're
making them sound; looking at the code I'm not seeing anything except
clocks being managed as part of the bias level management and there's no
effort to selectively manage the clocks.

> state. The current core does not even attempt to mute the DAC for example.

Yes it does - you can only reconfigure the sample rate for the audio
stream when it's not running and we mute (if the device supports it)
whenever the audio stream is halted.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]