[alsa-devel] [PATCH] ASoC: fsl: imx-wm8962: Do FLL configuration in hw_params() and hw_free()
Previously, we couldn't use hw_params() and hw_free() to open and close FLL becuase there might be race between two simmultaneous substreams and cause FLL configuration being changed and accordingly mulfunction. So we adopted DAPM to control it since there won't be any race in DAPM. However, if we want to playback a different sample rate file, we need to wait for DAPM to change its bias_level to reconfigure FLL.
But after we introduced full symmetry protection in the soc-pcm, we don't need to worry about the race any more. And the instance by using hw_params() and hw_free() to control FLL would allow us to support flexible use cases, 'aplay -Dhw:0 44k16bit.wav 48k24bit.wav 32k32bit.wav' for example.
Thus this patch mainly moves FLL configuration from set_bias_level() to hw_params() and hw_free() so as to enchance the sound card's capability.
Signed-off-by: Nicolin Chen b42378@freescale.com --- sound/soc/fsl/imx-wm8962.c | 147 +++++++++++++++++++++------------------------ 1 file changed, 70 insertions(+), 77 deletions(-)
diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c index 61e4885..df67a27 100644 --- a/sound/soc/fsl/imx-wm8962.c +++ b/sound/soc/fsl/imx-wm8962.c @@ -28,6 +28,9 @@
#define DAI_NAME_SIZE 32
+#define IMX_WM8962_DAI_FMT \ + (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | SND_SOC_DAIFMT_CBM_CFM) + struct imx_wm8962_data { struct snd_soc_dai_link dai; struct snd_soc_card card; @@ -49,92 +52,84 @@ static const struct snd_soc_dapm_widget imx_wm8962_dapm_widgets[] = { SND_SOC_DAPM_MIC("DMIC", NULL), };
-static int sample_rate = 44100; -static snd_pcm_format_t sample_format = SNDRV_PCM_FORMAT_S16_LE;
static int imx_hifi_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { - sample_rate = params_rate(params); - sample_format = params_format(params); + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct imx_priv *priv = &card_priv; + struct device *dev = &priv->pdev->dev; + struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev); + snd_pcm_format_t sample_format = params_format(params); + u32 pll_out, sample_rate = params_rate(params); + int ret = 0; + + /* set codec DAI configuration */ + ret = snd_soc_dai_set_fmt(codec_dai, IMX_WM8962_DAI_FMT); + if (ret) { + dev_err(dev, "failed to set codec dai fmt: %d\n", ret); + return ret; + } + + if (sample_format == SNDRV_PCM_FORMAT_S24_LE) + pll_out = sample_rate * 384; + else + pll_out = sample_rate * 256; + + ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, WM8962_FLL_MCLK, + data->clk_frequency, pll_out); + if (ret) { + dev_err(dev, "failed to start FLL: %d\n", ret); + return ret; + } + + ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_FLL, + pll_out, SND_SOC_CLOCK_IN); + if (ret) { + dev_err(dev, "failed to set SYSCLK: %d\n", ret); + return ret; + } + + return 0; +} + +static int imx_hifi_hw_free(struct snd_pcm_substream *substream) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *codec_dai = rtd->codec_dai; + struct imx_priv *priv = &card_priv; + struct device *dev = &priv->pdev->dev; + int ret; + + /* Don't diable FLL if still having multiple substreams running */ + if (codec_dai->active != 1) + return 0; + + /* Switch to MCLK as sysclk once so as to disable FLL */ + ret = snd_soc_dai_set_sysclk(codec_dai, WM8962_SYSCLK_MCLK, + 0, SND_SOC_CLOCK_IN); + if (ret < 0) { + dev_err(dev, "failed to switch away from FLL: %d\n", ret); + return ret; + } + + /* Disable FLL so that we can reset its output freq later */ + ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, + WM8962_FLL_MCLK, 0, 0); + if (ret < 0) { + dev_err(dev, "failed to stop FLL: %d\n", ret); + return ret; + }
return 0; }
static struct snd_soc_ops imx_hifi_ops = { .hw_params = imx_hifi_hw_params, + .hw_free = imx_hifi_hw_free, };
-static int imx_wm8962_set_bias_level(struct snd_soc_card *card, - struct snd_soc_dapm_context *dapm, - enum snd_soc_bias_level level) -{ - struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; - struct imx_priv *priv = &card_priv; - struct imx_wm8962_data *data = platform_get_drvdata(priv->pdev); - struct device *dev = &priv->pdev->dev; - unsigned int pll_out; - int ret; - - if (dapm->dev != codec_dai->dev) - return 0; - - switch (level) { - case SND_SOC_BIAS_PREPARE: - if (dapm->bias_level == SND_SOC_BIAS_STANDBY) { - if (sample_format == SNDRV_PCM_FORMAT_S24_LE) - pll_out = sample_rate * 384; - else - pll_out = sample_rate * 256; - - ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, - WM8962_FLL_MCLK, data->clk_frequency, - pll_out); - if (ret < 0) { - dev_err(dev, "failed to start FLL: %d\n", ret); - return ret; - } - - ret = snd_soc_dai_set_sysclk(codec_dai, - WM8962_SYSCLK_FLL, pll_out, - SND_SOC_CLOCK_IN); - if (ret < 0) { - dev_err(dev, "failed to set SYSCLK: %d\n", ret); - return ret; - } - } - break; - - case SND_SOC_BIAS_STANDBY: - if (dapm->bias_level == SND_SOC_BIAS_PREPARE) { - ret = snd_soc_dai_set_sysclk(codec_dai, - WM8962_SYSCLK_MCLK, data->clk_frequency, - SND_SOC_CLOCK_IN); - if (ret < 0) { - dev_err(dev, - "failed to switch away from FLL: %d\n", - ret); - return ret; - } - - ret = snd_soc_dai_set_pll(codec_dai, WM8962_FLL, - 0, 0, 0); - if (ret < 0) { - dev_err(dev, "failed to stop FLL: %d\n", ret); - return ret; - } - } - break; - - default: - break; - } - - dapm->bias_level = level; - - return 0; -} - static int imx_wm8962_late_probe(struct snd_soc_card *card) { struct snd_soc_dai *codec_dai = card->rtd[0].codec_dai; @@ -248,8 +243,7 @@ static int imx_wm8962_probe(struct platform_device *pdev) data->dai.cpu_dai_name = dev_name(&ssi_pdev->dev); data->dai.platform_of_node = ssi_np; data->dai.ops = &imx_hifi_ops; - data->dai.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | - SND_SOC_DAIFMT_CBM_CFM; + data->dai.dai_fmt = IMX_WM8962_DAI_FMT;
data->card.dev = &pdev->dev; ret = snd_soc_of_parse_card_name(&data->card, "model"); @@ -264,7 +258,6 @@ static int imx_wm8962_probe(struct platform_device *pdev) data->card.num_dapm_widgets = ARRAY_SIZE(imx_wm8962_dapm_widgets);
data->card.late_probe = imx_wm8962_late_probe; - data->card.set_bias_level = imx_wm8962_set_bias_level;
ret = devm_snd_soc_register_card(&pdev->dev, &data->card); if (ret) {
On Thu, Dec 05, 2013 at 12:54:08AM +0800, Nicolin Chen wrote:
Previously, we couldn't use hw_params() and hw_free() to open and close FLL becuase there might be race between two simmultaneous substreams and cause FLL configuration being changed and accordingly mulfunction. So we adopted DAPM to control it since there won't be any race in DAPM. However, if we want to playback a different sample rate file, we need to wait for DAPM to change its bias_level to reconfigure FLL.
But after we introduced full symmetry protection in the soc-pcm, we don't need to worry about the race any more. And the instance by using hw_params() and hw_free() to control FLL would allow us to support flexible use cases, 'aplay -Dhw:0 44k16bit.wav 48k24bit.wav 32k32bit.wav' for example.
Thus this patch mainly moves FLL configuration from set_bias_level() to hw_params() and hw_free() so as to enchance the sound card's capability.
That's not the only reason for using set_bias_level(), it's also used so that bypass paths from the CODEC inputs to outputs get the FLL started - all the outputs need the device to be clocked.
On Wed, Dec 04, 2013 at 07:02:11PM +0000, Mark Brown wrote:
On Thu, Dec 05, 2013 at 12:54:08AM +0800, Nicolin Chen wrote:
Previously, we couldn't use hw_params() and hw_free() to open and close FLL becuase there might be race between two simmultaneous substreams and cause FLL configuration being changed and accordingly mulfunction. So we adopted DAPM to control it since there won't be any race in DAPM. However, if we want to playback a different sample rate file, we need to wait for DAPM to change its bias_level to reconfigure FLL.
But after we introduced full symmetry protection in the soc-pcm, we don't need to worry about the race any more. And the instance by using hw_params() and hw_free() to control FLL would allow us to support flexible use cases, 'aplay -Dhw:0 44k16bit.wav 48k24bit.wav 32k32bit.wav' for example.
Thus this patch mainly moves FLL configuration from set_bias_level() to hw_params() and hw_free() so as to enchance the sound card's capability.
That's not the only reason for using set_bias_level(), it's also used so that bypass paths from the CODEC inputs to outputs get the FLL started
- all the outputs need the device to be clocked.
Thank you for reminding me of this case. And I still have a concern.
As far as I can understand, the point that all the outputs needs the device to be clocked is undeniably true, but the external MCLK should be sufficient to source the SYSCLK of WM8962. Does the bypass mode also need to set FLL to get an accurate frequency?
Sorry I am not so familiar with the bypass path. So what I can imagine is WM8962 just bypass input (DAC) to outputs (DAC) via MIXER or some routes else, then there's no need for WM8962 to generate accurate BCLK and LRCLK. Sir, could you please also shed some light on this point if it's not a comprehensive one?
Thank you, Nicolin Chen
On Thu, Dec 05, 2013 at 10:56:31AM +0800, Nicolin Chen wrote:
As far as I can understand, the point that all the outputs needs the device to be clocked is undeniably true, but the external MCLK should be sufficient to source the SYSCLK of WM8962. Does the bypass mode also need to set FLL to get an accurate frequency?
The device is going to need a clock that's in spec to make sure that the device performs in spec, if the device is clocked from a 32kHz clock (which is quite a common configuration and one that's supported by the machine driver since it gets the clock from DT) that's definitely not going to be the case and I'd not expect it to work at all sensibly.
On Thu, Dec 05, 2013 at 11:15:08AM +0000, Mark Brown wrote:
On Thu, Dec 05, 2013 at 10:56:31AM +0800, Nicolin Chen wrote:
As far as I can understand, the point that all the outputs needs the device to be clocked is undeniably true, but the external MCLK should be sufficient to source the SYSCLK of WM8962. Does the bypass mode also need to set FLL to get an accurate frequency?
The device is going to need a clock that's in spec to make sure that the device performs in spec, if the device is clocked from a 32kHz clock (which is quite a common configuration and one that's supported by the machine driver since it gets the clock from DT) that's definitely not going to be the case and I'd not expect it to work at all sensibly.
Thank you for the explain. I understand now.
Then I think it would be plausible if I could refine this patch to allow both set_bias_level() and hw_params/free() to control the FLL while making them not break each other but coexist, right?
Thank you, Nicolin Chen
On Thu, Dec 05, 2013 at 07:07:28PM +0800, Nicolin Chen wrote:
Then I think it would be plausible if I could refine this patch to allow both set_bias_level() and hw_params/free() to control the FLL while making them not break each other but coexist, right?
Yes, that makes sense. It might even be useful to have some support for this along the style of the bias level support in the core since modern CODECs all use things like charge pumps and class D amplifiers so this is becoming a more and more common problem.
On Thu, Dec 05, 2013 at 12:21:13PM +0000, Mark Brown wrote:
On Thu, Dec 05, 2013 at 07:07:28PM +0800, Nicolin Chen wrote:
Then I think it would be plausible if I could refine this patch to allow both set_bias_level() and hw_params/free() to control the FLL while making them not break each other but coexist, right?
Yes, that makes sense. It might even be useful to have some support for this along the style of the bias level support in the core since modern CODECs all use things like charge pumps and class D amplifiers so this is becoming a more and more common problem.
That's a nice suggestion. But currently I don't think I have the proficiency to do this task to the core. So I'd like to start revising from this machine driver first to gain the knowledge :)
Thank you for your reply, Nicolin Chen
participants (2)
-
Mark Brown -
Nicolin Chen