[alsa-devel] [PATCH] ASoC: adau1701: Reset codec based on sample rate changes
From: Pascal Huerst pascal.huerst@gmail.com
Instead of checking if mclk/lrclk ratio has changed, check if sample rate has changed. In certain cases, the mclk might be changed in the machine driver, which can lead to the same mclk/lrclk ration, eventhow the sample rate has changed.
Since the codec has to be programmed differently for every sample rate, its better to check for samplerate changes instead of mclk/lrclk ration changes.
Signed-off-by: Pascal Huerst pascal.huerst@gmail.com --- sound/soc/codecs/adau1701.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c index 202dea1..1dbbcda 100644 --- a/sound/soc/codecs/adau1701.c +++ b/sound/soc/codecs/adau1701.c @@ -111,6 +111,7 @@ struct adau1701 { unsigned int dai_fmt; unsigned int pll_clkdiv; unsigned int sysclk; + unsigned int current_rate; struct regmap *regmap; struct i2c_client *client; u8 pin_config[12]; @@ -436,22 +437,24 @@ static int adau1701_hw_params(struct snd_pcm_substream *substream, { struct snd_soc_codec *codec = dai->codec; struct adau1701 *adau1701 = snd_soc_codec_get_drvdata(codec); - unsigned int clkdiv = adau1701->sysclk / params_rate(params); + unsigned int rate = params_rate(params); + unsigned int clkdiv = adau1701->sysclk / rate; unsigned int val; int ret;
/* - * If the mclk/lrclk ratio changes, the chip needs updated PLL + * If the sample rate changes, the chip needs updated PLL * mode GPIO settings, and a full reset cycle, including a new * firmware upload. */ - if (clkdiv != adau1701->pll_clkdiv) { - ret = adau1701_reset(codec, clkdiv, params_rate(params)); + if (adau1701->current_rate != rate) { + adau1701->current_rate = rate; + ret = adau1701_reset(codec, clkdiv, rate); if (ret < 0) return ret; }
- switch (params_rate(params)) { + switch (rate) { case 192000: val = ADAU1701_DSPCTRL_SR_192; break; -- 2.5.0
On Wed, Mar 23, 2016 at 12:58:23PM +0100, pascal.huerst@gmail.com wrote:
Instead of checking if mclk/lrclk ratio has changed, check if sample rate has changed. In certain cases, the mclk might be changed in the machine driver, which can lead to the same mclk/lrclk ration, eventhow the sample rate has changed.
Since the codec has to be programmed differently for every sample rate, its better to check for samplerate changes instead of mclk/lrclk ration changes.
Why does this mean we have to reset the CODEC rather than just reprogramming it?
On 03/23/2016 01:48 PM, Mark Brown wrote:
On Wed, Mar 23, 2016 at 12:58:23PM +0100, pascal.huerst@gmail.com wrote:
Instead of checking if mclk/lrclk ratio has changed, check if sample rate has changed. In certain cases, the mclk might be changed in the machine driver, which can lead to the same mclk/lrclk ration, eventhow the sample rate has changed.
Since the codec has to be programmed differently for every sample rate, its better to check for samplerate changes instead of mclk/lrclk ration changes.
Why does this mean we have to reset the CODEC rather than just reprogramming it?
Quoting from the datasheet:
The clock mode should not be changed without also resetting the ADAU1701. If the mode is changed during operation, a click or pop can result in the output signals. The state of the PLL_MODEx pins should be changed while RESET is held low.
- Lars
On Wed, Mar 23, 2016 at 02:37:10PM +0100, Lars-Peter Clausen wrote:
On 03/23/2016 01:48 PM, Mark Brown wrote:
Why does this mean we have to reset the CODEC rather than just reprogramming it?
Quoting from the datasheet:
The clock mode should not be changed without also resetting the ADAU1701. If the mode is changed during operation, a click or pop can result in the output signals. The state of the PLL_MODEx pins should be changed while RESET is held low.
That should be in the commit log and/or code since that's *extremely* unusual and is likely to surprise people.
On 03/23/2016 12:58 PM, pascal.huerst@gmail.com wrote:
From: Pascal Huerst pascal.huerst@gmail.com
Instead of checking if mclk/lrclk ratio has changed, check if sample rate has changed. In certain cases, the mclk might be changed in the machine driver, which can lead to the same mclk/lrclk ration, eventhow the sample rate has changed.
Since the codec has to be programmed differently for every sample rate, its better to check for samplerate changes instead of mclk/lrclk ration changes.
Mark's comment made me give this some additional though. Do we actually need to reset the device if the clkdiv did not change. Stopping the DSP, uploading the new firmware and then restarting it should be sufficient. But on the other hand the time the reset takes should be negligible compared to programming the firmware, so it might be ok to always do it. Let me know what you think.
On 23.03.2016 14:44, Lars-Peter Clausen wrote:
On 03/23/2016 12:58 PM, pascal.huerst@gmail.com wrote:
From: Pascal Huerst pascal.huerst@gmail.com
Instead of checking if mclk/lrclk ratio has changed, check if sample rate has changed. In certain cases, the mclk might be changed in the machine driver, which can lead to the same mclk/lrclk ration, eventhow the sample rate has changed.
Since the codec has to be programmed differently for every sample rate, its better to check for samplerate changes instead of mclk/lrclk ration changes.
Mark's comment made me give this some additional though. Do we actually need to reset the device if the clkdiv did not change. Stopping the DSP, uploading the new firmware and then restarting it should be sufficient. But on the other hand the time the reset takes should be negligible compared to programming the firmware, so it might be ok to always do it. Let me know what you think.
Ok, I see your point. So I did some measurements.
On our devices,
a firmware download takes about: 844ms Resetting the pll settings takes about: 87ms
I'm not sure, if this worth the effort, but certainly it could be done. I would probably leave it for now, be a bit more precise in the comment above and add a note to the commit message, as Mark suggested (?)
While at it I also saw, that we should keep the reset line low, while changing the pll settings. (Which we don't right now)
The datasheet states:
... "The state of the PLL_MODEx pins should be changed while RESET is held low." ...
participants (4)
-
Lars-Peter Clausen -
Mark Brown -
Pascal Huerst -
pascal.huerst＠gmail.com