[alsa-devel] [PATCH] Patched to allow fine-grained control of PLL and clock dividers
Changes: 1. allow the driver to accept S32_LE PCM streams 2. fine-grained control of the PLL generation WM8804 can run with PLL frequencies of 256xfs and 128xfs for most sample rates. At 192kHz only 128xfs is supported. The existing driver selects 128xfs automatically for some lower samples rates. By using the "pllid" argument of the "set_pll" function is is now possible to control the behaviour. This allows using 256xfs PLL frequency on all sample rates up to 96kHz. It should allow lower jitter and better signal quality. When pllid=0, the behaviour of the driver does not change.
--- sound/soc/codecs/wm8804.c | 25 +++++++++++++++++-------- sound/soc/codecs/wm8804.h | 7 +++++++ 2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c index 1704b1e..f3f73b2 100644 --- a/sound/soc/codecs/wm8804.c +++ b/sound/soc/codecs/wm8804.c @@ -2,6 +2,7 @@ * wm8804.c -- WM8804 S/PDIF transceiver driver * * Copyright 2010-11 Wolfson Microelectronics plc + * patched by Daniel Matuschek info@crazyaudio.com to allow fine-grained + * control of PLL and dividers * * Author: Dimitris Papastamos dp@opensource.wolfsonmicro.com * @@ -277,6 +278,7 @@ static int wm8804_hw_params(struct snd_pcm_substream *substream, blen = 0x1; break; case SNDRV_PCM_FORMAT_S24_LE: + case SNDRV_PCM_FORMAT_S32_LE: blen = 0x2; break; default: @@ -318,7 +320,7 @@ static struct {
#define FIXED_PLL_SIZE ((1ULL << 22) * 10) static int pll_factors(struct pll_div *pll_div, unsigned int target, - unsigned int source) + unsigned int source, int mclk_div) { u64 Kpart; unsigned long int K, Ndiv, Nmod, tmp; @@ -332,9 +334,14 @@ static int pll_factors(struct pll_div *pll_div, unsigned int target, tmp = target * post_table[i].div; if (tmp >= 90000000 && tmp <= 100000000) { pll_div->freqmode = post_table[i].freqmode; - pll_div->mclkdiv = post_table[i].mclkdiv; - target *= post_table[i].div; - break; + if ((mclk_div == WM8804_MCLKDIV_DONTCARE) || + ((post_table[i].mclkdiv == 1) && + (mclk_div == WM8804_MCLKDIV_1)) || + ((post_table[i].mclkdiv == 0) && + (mclk_div == WM8804_MCLKDIV_0))) { + pll_div->mclkdiv = post_table[i].mclkdiv; + target *= post_table[i].div; + break; + } } }
@@ -388,7 +395,7 @@ static int wm8804_set_pll(struct snd_soc_dai *dai, int pll_id, int ret; struct pll_div pll_div;
- ret = pll_factors(&pll_div, freq_out, freq_in); + ret = pll_factors(&pll_div, freq_out, freq_in, pll_id); if (ret) return ret;
@@ -641,7 +648,8 @@ static const struct snd_soc_dai_ops wm8804_dai_ops = { };
#define WM8804_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \ - SNDRV_PCM_FMTBIT_S24_LE) + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE | \ + SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE)
#define WM8804_RATES (SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_64000 | \ @@ -674,7 +682,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8804 = { .suspend = wm8804_suspend, .resume = wm8804_resume, .set_bias_level = wm8804_set_bias_level, - .idle_bias_off = true, + .idle_bias_off = false,
.controls = wm8804_snd_controls, .num_controls = ARRAY_SIZE(wm8804_snd_controls), @@ -760,6 +768,7 @@ static int wm8804_i2c_probe(struct i2c_client *i2c,
ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_wm8804, &wm8804_dai, 1); + return ret; }
diff --git a/sound/soc/codecs/wm8804.h b/sound/soc/codecs/wm8804.h index 8ec14f5..0365177 100644 --- a/sound/soc/codecs/wm8804.h +++ b/sound/soc/codecs/wm8804.h @@ -58,4 +58,11 @@
#define WM8804_CLKOUT_DIV 1
+#define WM8804_MCLKDIV_DONTCARE 0 +#define WM8804_MCLKDIV_0 1 +#define WM8804_MCLKDIV_1 2 +#define WM8804_PLL_MCLKDIV_DONTCARE WM8804_MCLKDIV_DONTCARE +#define WM8804_PLL_MCLKDIV_0 WM8804_MCLKDIV_0 +#define WM8804_PLL_MCLKDIV_1 WM8804_MCLKDIV_1 + #endif /* _WM8804_H */
Please make sure you CC the maintainers on patch submission otherwise they may miss the patch. Please also use the correct subject prefix too e.g.
ASoC: wm8804: allow fine-grained control of PLL and clock dividers
Btw, you should use git format-patch to create your patch otherwise it cant easily be applied.
Some minor comments below :-
On Sun, 2014-01-05 at 20:45 +0100, Daniel Matuschek wrote:
Changes:
- allow the driver to accept S32_LE PCM streams
- fine-grained control of the PLL generation
WM8804 can run with PLL frequencies of 256xfs and 128xfs for most sample rates. At 192kHz only 128xfs is supported. The existing driver selects 128xfs automatically for some lower samples rates. By using the "pllid" argument of the "set_pll" function is is now possible to control the behaviour. This allows using 256xfs PLL frequency on all sample rates up to 96kHz. It should allow lower jitter and better signal quality. When pllid=0, the behaviour of the driver does not change.
sound/soc/codecs/wm8804.c | 25 +++++++++++++++++-------- sound/soc/codecs/wm8804.h | 7 +++++++ 2 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/sound/soc/codecs/wm8804.c b/sound/soc/codecs/wm8804.c index 1704b1e..f3f73b2 100644 --- a/sound/soc/codecs/wm8804.c +++ b/sound/soc/codecs/wm8804.c @@ -2,6 +2,7 @@
- wm8804.c -- WM8804 S/PDIF transceiver driver
- Copyright 2010-11 Wolfson Microelectronics plc
- patched by Daniel Matuschek info@crazyaudio.com to allow fine-grained
- control of PLL and dividers
- Author: Dimitris Papastamos dp@opensource.wolfsonmicro.com
@@ -277,6 +278,7 @@ static int wm8804_hw_params(struct snd_pcm_substream *substream, blen = 0x1; break; case SNDRV_PCM_FORMAT_S24_LE:
- case SNDRV_PCM_FORMAT_S32_LE: blen = 0x2; break; default:
@@ -318,7 +320,7 @@ static struct {
#define FIXED_PLL_SIZE ((1ULL << 22) * 10) static int pll_factors(struct pll_div *pll_div, unsigned int target,
unsigned int source)
unsigned int source, int mclk_div)
{ u64 Kpart; unsigned long int K, Ndiv, Nmod, tmp; @@ -332,9 +334,14 @@ static int pll_factors(struct pll_div *pll_div, unsigned int target, tmp = target * post_table[i].div; if (tmp >= 90000000 && tmp <= 100000000) { pll_div->freqmode = post_table[i].freqmode;
pll_div->mclkdiv = post_table[i].mclkdiv;
target *= post_table[i].div;
break;
if ((mclk_div == WM8804_MCLKDIV_DONTCARE) ||
((post_table[i].mclkdiv == 1) &&
(mclk_div == WM8804_MCLKDIV_1)) ||
((post_table[i].mclkdiv == 0) &&
(mclk_div == WM8804_MCLKDIV_0))) {
pll_div->mclkdiv = post_table[i].mclkdiv;
target *= post_table[i].div;
break;
} }}
@@ -388,7 +395,7 @@ static int wm8804_set_pll(struct snd_soc_dai *dai, int pll_id, int ret; struct pll_div pll_div;
ret = pll_factors(&pll_div, freq_out, freq_in);
if (ret) return ret;ret = pll_factors(&pll_div, freq_out, freq_in, pll_id);
@@ -641,7 +648,8 @@ static const struct snd_soc_dai_ops wm8804_dai_ops = { };
#define WM8804_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE | \
SNDRV_PCM_FMTBIT_S24_LE)
SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE | \
SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE)
You don't mention adding support for IEC format in your patch description.
#define WM8804_RATES (SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_64000 | \ @@ -674,7 +682,7 @@ static struct snd_soc_codec_driver soc_codec_dev_wm8804 = { .suspend = wm8804_suspend, .resume = wm8804_resume, .set_bias_level = wm8804_set_bias_level,
- .idle_bias_off = true,
- .idle_bias_off = false,
Why does idle bias change here ?
.controls = wm8804_snd_controls, .num_controls = ARRAY_SIZE(wm8804_snd_controls), @@ -760,6 +768,7 @@ static int wm8804_i2c_probe(struct i2c_client *i2c,
ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_wm8804, &wm8804_dai, 1);
Any need for this ?
return ret; }
diff --git a/sound/soc/codecs/wm8804.h b/sound/soc/codecs/wm8804.h index 8ec14f5..0365177 100644 --- a/sound/soc/codecs/wm8804.h +++ b/sound/soc/codecs/wm8804.h @@ -58,4 +58,11 @@
#define WM8804_CLKOUT_DIV 1
+#define WM8804_MCLKDIV_DONTCARE 0 +#define WM8804_MCLKDIV_0 1 +#define WM8804_MCLKDIV_1 2 +#define WM8804_PLL_MCLKDIV_DONTCARE WM8804_MCLKDIV_DONTCARE +#define WM8804_PLL_MCLKDIV_0 WM8804_MCLKDIV_0 +#define WM8804_PLL_MCLKDIV_1 WM8804_MCLKDIV_1
#endif /* _WM8804_H */
On Wed, Jan 08, 2014 at 12:08:08PM +0000, Liam Girdwood wrote:
Please make sure you CC the maintainers on patch submission otherwise they may miss the patch. Please also use the correct subject prefix too e.g.
In general follow the process in SubmittingPatches.
Changes:
- allow the driver to accept S32_LE PCM streams
- fine-grained control of the PLL generation
These should be sent as two separate patches, each patch should only do one thing (as covered in SubmittingPatches).
participants (3)
-
Daniel Matuschek
-
Liam Girdwood
-
Mark Brown