asoc tlv320aic3x: set power bits correctly
Subsystems (3)
PATCH 1/1 asoc tlv320aic3x: set power bits correctly
Set power bits in output stage control registers to reflect the "!muted" bits. The codec needs them set in order to operate correctly.
From: Daniel Mack <[email protected]>
Set power bits in output stage control registers to reflect the "!muted" bits. The codec needs them set in order to operate correctly.
sound/soc/codecs/tlv320aic3x.c +24 -0
@@ -93,6 +93,28 @@ static const u8 aic3x_reg[AIC3X_CACHEREGNUM] = { };/* +* power up/down aic3x functions +*/ +static inline void aic3x_handle_pwr_bits (unsigned int reg,unsigned int *val) +{ +switch (reg) { +case HPLOUT_CTRL : +case HPROUT_CTRL : +case HPLCOM_CTRL : +case HPRCOM_CTRL : +case LLOPM_CTRL : +case RLOPM_CTRL : +case MONOLOPM_CTRL : +if (*val &0x08 ) + *val |=0x01 ; +else + *val &= ~0x01 ; + +break ; + } +} + +/* * read aic3x register cache */ static inline unsigned int aic3x_read_reg_cache(struct snd_soc_codec *codec,
@@ -124,6 +146,8 @@ static int aic3x_write(struct snd_soc_codec *codec, unsigned int reg, {u8 data[2 ]; +aic3x_handle_pwr_bits (reg, &value); +/* data is * D15..D8 aic3x register offset * D7...D0 register data
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Quoted 2 lines · Daniel Mack
Set power bits in output stage control registers to reflect the "!muted" bits. The codec needs them set in order to operate correctly.
Could you explain in more detail what this patch is doing, please (ideally in comments in the code or at least the commit message)?
sound/soc/codecs/tlv320aic3x.c static const u8 aic3x_reg[AIC3X_CACHEREGNUM] = { · line 114
Quoted 8 lines
sound/soc/codecs/tlv320aic3x.c +8
-0+case MONOLOPM_CTRL : +if (*val &0x08 ) + *val |=0x01 ; +else + *val &= ~0x01 ; + +break ; + }
It would be good if the code were clearer about what these register bits mean and why this isn't being handled via DAPM.
If what you're trying to do is avoid powering on amplifiers when they are muted then this is best avoided since it often creates problems with pops and clicks due to things getting powered on and off out of the sequence generated by DAPM. If the amplifier is powered on before its inputs then it will amplify any noise generaetd by the inputs when they change power state. This is why ASoC mute controls for amplifiers are normally independent of their power controls.
Quoted 9 lines · Mark Brown
If what you're trying to do is avoid powering on amplifiers when they are muted then this is best avoided since it often creates problems with pops and clicks due to things getting powered on and off out of the sequence generated by DAPM. If the amplifier is powered on before its inputs then it will amplify any noise generaetd by the inputs when they change power state. This is why ASoC mute controls for amplifiers are normally independent of their power controls.
I was looking do we have possible driver bug here and answer is no. Driver keeps output state powered during playback if associated endpoint is enabled with snd_soc_dapm_set_endpoint and unpowered otherwise.
So as with Mark's comment, the patch is unnecessary.
Jarkko
Hi Jarkko,
Quoted 6 lines · Jarkko Nikula
I was looking do we have possible driver bug here and answer is no. Driver keeps output state powered during playback if associated endpoint is enabled with
snd_soc_dapm_set_endpointand unpowered otherwise.So as with Mark's comment, the patch is unnecessary.
Hmm, ok - I might have a lack of knowledge here. All I was seeing with an I2C hardware analyzer is that those registers in question were written without the power bit set which makes them remain muted. What's the way to power up this entity properly?
And as we're on it - aic3x_dapm_event(SNDRV_CTL_POWER_D3cold) is called shortly after PCM playback has finished which clears the power bits of many registers. This in turn also makes the current PGA mixer setting become unfunctional as the outputs are switched off. Is there a proper way of preventing the glue layer to power down the chip in this case?
Thanks,
Daniel
Quoted 4 lines · Daniel Mack
Hmm, ok - I might have a lack of knowledge here. All I was seeing with an I2C hardware analyzer is that those registers in question were written without the power bit set which makes them remain muted. What's the way to power up this entity properly?
You need to ensure that the entire DAPM path from the DAC to the output is powered by ensuring that the final output endpoint is marked as active. Your machine driver should be using snd_soc_dapm_set_endpoint() to mark either the output pin of the codec or any external device (eg, a jack) that you've told DAPM about as enabled. If you do not do this then DAPM will notice that none output paths are connected and not power on any of the components in the output path in order to save power.
This should be integrated with any jack detection you are doing so that, for example, the headphone output will be marked as disabled when there is no headphone present. The core will automatically manage the power state for DACs and ADCs but the machine driver needs to manage the power state for everything else.
Quoted 6 lines
And as we're on it -
aic3x_dapm_event(SNDRV_CTL_POWER_D3cold)is called shortly after PCM playback has finished which clears the power bits of many registers. This in turn also makes the current PGA mixer setting become unfunctional as the outputs are switched off. Is there a proper way of preventing the glue layer to power down the chip in this case?
Are you sure about that? The chip should only be being placed into D3cold when the system is being suspended or powered off. The chip will be placed into D3hot when the DACs and ADCs go idle but not D3cold. You will see similar behaviour if you haven't marked appropriate endpoints as active (since DAPM will power off anything it thinks is not in use, triggered when the DAC or ADC goes idle) but it shouldn't be done by the driver D3cold.