[alsa-devel] PLL computation in TLV320AIC3x SoC driver
Hello,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable
thanks, regards, p.
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..e84e473 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -766,9 +766,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0; - u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; - u16 pll_d = 1; + u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; + u16 d, pll_d = 1; u8 reg; + int clk;
/* select data word length */ data = @@ -835,47 +836,62 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, return 0;
/* Use PLL - * find an apropriate setup for j, d, r and p by iterating over - * p and r - j and d are calculated for each fraction. - * Up to 128 values are probed, the closest one wins the game. + * find an appropriate setup for j, d, r and p by iterating over + * p, r and j first, then trying to compute the fraction d. + * Up to 6528 values are probed, the closest one wins the game. * The sysclk is divided by 1000 to prevent integer overflows. */ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
- for (r = 1; r <= 16; r++) - for (p = 1; p <= 8; p++) { - int clk, tmp = (codec_clk * pll_r * 10) / pll_p; - u8 j = tmp / 10000; - u16 d = tmp % 10000; - - if (j > 63) - continue; - - if (d != 0 && aic3x->sysclk < 10000000) - continue; - - /* This is actually 1000 * ((j + (d/10000)) * r) / p - * The term had to be converted to get rid of the - * division by 10000 */ - clk = ((10000 * j * r) + (d * r)) / (10 * p); - - /* check whether this values get closer than the best - * ones we had before */ - if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { - pll_j = j; pll_d = d; pll_r = r; pll_p = p; - last_clk = clk; - } - - /* Early exit for exact matches */ - if (clk == codec_clk) - break; - } + for (r = 1; r <= 16; r++) + for (p = 1; p <= 8; p++) { + for (j = 4; j <= 55; j++) { + /* This is actually 1000 * ((j + (d/10000)) * r) / p + * The term had to be converted to get rid of the + * division by 10000; d = 0 here */ + int clk = (1000 * j * r) / p; + + /* check whether this values get closer than the best + * ones we had before */ + if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { + pll_j = j; pll_d = 0; pll_r = r; pll_p = p; + last_clk = clk; + } + + /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + } + } + + /* try with d != 0 */ + for (p = 1; p <= 8; p++) { + j = codec_clk * p / 1000; + + if (j < 4 || j > 11) continue; + + /* do not use codec_clk here since we'd loose precision */ + d = ((2048 * fsref * 10) / (aic3x->sysclk / 1000)) % 10000; + clk = (10000 * j + d) / (10 * p); + + /* check whether this values get closer than the best + * ones we had before */ + if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { + pll_j = j; pll_d = d; pll_r = 1; pll_p = 1; + last_clk = clk; + } + + /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + }
if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
Hello Peter,
Peter Meerwald wrote:
Hello,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I've made the simple test application to calculate pll_p/r/j/d values using current tlv320aic3x clock calculation scheme and I've got:
samplerate=8000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=(sysclk * [pll_j].[pll_d] * pll_r) / (2048 * pll_p) = 48000 samplerate=11025: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=16000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=22050: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=32000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=44100: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=48000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=64000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE, hence FSREF=96000 samplerate=88200: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096, note that here we used DUAL_RATE_MODE hence FSREF=88192 samplerate=96000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE hence FSREF = 96000
Then the AIC3X_SAMPLE_RATE_SEL_REG is calculated by simple formula based on fsref.
Hence according to current code the calculations of desired FSREF is correct for sysclk=16384000
I've not tried it with your patch since I don't actually understand what do you fix? :)
Regards, Vladimir
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable
thanks, regards, p.
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..e84e473 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -766,9 +766,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0;
u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 pll_d = 1;
u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 d, pll_d = 1; u8 reg;int clk; /* select data word length */ data =@@ -835,47 +836,62 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, return 0;
/* Use PLL
* find an apropriate setup for j, d, r and p by iterating over* p and r - j and d are calculated for each fraction.* Up to 128 values are probed, the closest one wins the game.
* find an appropriate setup for j, d, r and p by iterating over* p, r and j first, then trying to compute the fraction d.* Up to 6528 values are probed, the closest one wins the game. * The sysclk is divided by 1000 to prevent integer overflows. */ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
for (r = 1; r <= 16; r++)for (p = 1; p <= 8; p++) {int clk, tmp = (codec_clk * pll_r * 10) / pll_p;u8 j = tmp / 10000;u16 d = tmp % 10000;if (j > 63)continue;if (d != 0 && aic3x->sysclk < 10000000)continue;/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000 */clk = ((10000 * j * r) + (d * r)) / (10 * p);/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = d; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)break;}
for (r = 1; r <= 16; r++)
for (p = 1; p <= 8; p++) {for (j = 4; j <= 55; j++) {/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000; d = 0 here */int clk = (1000 * j * r) / p;/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = 0; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;}}/* try with d != 0 */
for (p = 1; p <= 8; p++) {
j = codec_clk * p / 1000;if (j < 4 || j > 11) continue;/* do not use codec_clk here since we'd loose precision */d = ((2048 * fsref * 10) / (aic3x->sysclk / 1000)) % 10000;clk = (10000 * j + d) / (10 * p);/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = d; pll_r = 1; pll_p = 1;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;}
if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
Hello Vladimir,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I've made the simple test application to calculate pll_p/r/j/d values using current tlv320aic3x clock calculation scheme and I've got: samplerate=8000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=(sysclk * [pll_j].[pll_d] * pll_r) / (2048 * pll_p) = 48000 samplerate=11025: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=16000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=22050: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=32000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=44100: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=48000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=64000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE, hence FSREF=96000 samplerate=88200: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096, note that here we used DUAL_RATE_MODE hence FSREF=88192 samplerate=96000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE hence FSREF = 96000
Hence according to current code the calculations of desired FSREF is correct for sysclk=16384000 I've not tried it with your patch since I don't actually understand what do you fix? :)
1. loop variables are r and p but for computation of j the variables pll_r and pll_p are used; I think r and p should be used in the computation of j, your code below:
for (r = 1; r <= 16; r++) for (p = 1; p <= 8; p++) { int clk, tmp = (codec_clk * pll_r * 10) / pll_p; u8 j = tmp / 10000;
further, the codec datasheet mandates certain ranges for 'good performance', 4 <= j <= 55 (for d==0) and 4 <= j <= 11 (for d!=0) which are not checked for
2. d should actually be 5125 and not 5120
3. appropriate is spell appropriately :)
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable thanks, regards, p.
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..e84e473 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -766,9 +766,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0;
u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 pll_d = 1;
u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 d, pll_d = 1; u8 reg;int clk; /* select data word length */ data =@@ -835,47 +836,62 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, return 0; /* Use PLL
* find an apropriate setup for j, d, r and p by iterating over* p and r - j and d are calculated for each fraction.* Up to 128 values are probed, the closest one wins the game.
* find an appropriate setup for j, d, r and p by iterating over* p, r and j first, then trying to compute the fraction d.* Up to 6528 values are probed, the closest one wins the game. * The sysclk is divided by 1000 to prevent integer overflows. */ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
for (r = 1; r <= 16; r++)for (p = 1; p <= 8; p++) {int clk, tmp = (codec_clk * pll_r * 10) / pll_p;u8 j = tmp / 10000;u16 d = tmp % 10000;if (j > 63)continue;if (d != 0 && aic3x->sysclk < 10000000)continue;/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000 */clk = ((10000 * j * r) + (d * r)) / (10 * p);/* check whether this values get closer than thebest
* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk -last_clk)) {
pll_j = j; pll_d = d; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)break;}
- for (r = 1; r <= 16; r++)
for (p = 1; p <= 8; p++) {for (j = 4; j <= 55; j++) {/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000; d = 0 here */int clk = (1000 * j * r) / p;/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = 0; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;}}- /* try with d != 0 */
- for (p = 1; p <= 8; p++) {
j = codec_clk * p / 1000;if (j < 4 || j > 11) continue;/* do not use codec_clk here since we'd loose precision */d = ((2048 * fsref * 10) / (aic3x->sysclk / 1000)) % 10000;clk = (10000 * j + d) / (10 * p);/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = d; pll_r = 1; pll_p = 1;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;- } if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
Peter Meerwald wrote:
Hello Vladimir,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I've made the simple test application to calculate pll_p/r/j/d values using current tlv320aic3x clock calculation scheme and I've got: samplerate=8000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=(sysclk * [pll_j].[pll_d] * pll_r) / (2048 * pll_p) = 48000 samplerate=11025: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=16000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=22050: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=32000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=44100: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=48000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=64000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE, hence FSREF=96000 samplerate=88200: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096, note that here we used DUAL_RATE_MODE hence FSREF=88192 samplerate=96000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE hence FSREF = 96000
Hence according to current code the calculations of desired FSREF is correct for sysclk=16384000 I've not tried it with your patch since I don't actually understand what do you fix? :)
- loop variables are r and p but for computation of j the variables
pll_r and pll_p are used; I think r and p should be used in the computation of j, your code below:
for (r = 1; r <= 16; r++) for (p = 1; p <= 8; p++) { int clk, tmp = (codec_clk * pll_r * 10) / pll_p; u8 j = tmp / 10000;further, the codec datasheet mandates certain ranges for 'good performance', 4 <= j <= 55 (for d==0) and 4 <= j <= 11 (for d!=0) which are not checked for
Correct.
- d should actually be 5125 and not 5120
Now I see, I've verified your code handles this. This actually produce more accuracy for FSREF.
I just tested your patch works fine with sysclk=16384000 and 12288000 but fails with 11286900: it returns FSREF=45467 instead of desired 44100. I think that this is a valuable difference. Please check it.
Regards, Vladimir
- appropriate is spell appropriately :)
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable thanks, regards, p.
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..e84e473 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -766,9 +766,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0;
u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 pll_d = 1;
u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 d, pll_d = 1; u8 reg;int clk; /* select data word length */ data =@@ -835,47 +836,62 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, return 0; /* Use PLL
* find an apropriate setup for j, d, r and p by iterating over* p and r - j and d are calculated for each fraction.* Up to 128 values are probed, the closest one wins the game.
* find an appropriate setup for j, d, r and p by iterating over* p, r and j first, then trying to compute the fraction d.* Up to 6528 values are probed, the closest one wins the game. * The sysclk is divided by 1000 to prevent integer overflows. */ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
for (r = 1; r <= 16; r++)for (p = 1; p <= 8; p++) {int clk, tmp = (codec_clk * pll_r * 10) / pll_p;u8 j = tmp / 10000;u16 d = tmp % 10000;if (j > 63)continue;if (d != 0 && aic3x->sysclk < 10000000)continue;/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000 */clk = ((10000 * j * r) + (d * r)) / (10 * p);/* check whether this values get closer than thebest
* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk -last_clk)) {
pll_j = j; pll_d = d; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)break;}
- for (r = 1; r <= 16; r++)
for (p = 1; p <= 8; p++) {for (j = 4; j <= 55; j++) {/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000; d = 0 here */int clk = (1000 * j * r) / p;/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = 0; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;}}- /* try with d != 0 */
- for (p = 1; p <= 8; p++) {
j = codec_clk * p / 1000;if (j < 4 || j > 11) continue;/* do not use codec_clk here since we'd loose precision */d = ((2048 * fsref * 10) / (aic3x->sysclk / 1000)) % 10000;clk = (10000 * j + d) / (10 * p);/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = d; pll_r = 1; pll_p = 1;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;- } if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
Vladimir Barinov wrote:
Peter Meerwald wrote:
Hello Vladimir,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I've made the simple test application to calculate pll_p/r/j/d values using current tlv320aic3x clock calculation scheme and I've got: samplerate=8000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=(sysclk * [pll_j].[pll_d] * pll_r) / (2048 * pll_p) = 48000 samplerate=11025: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=16000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=22050: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=32000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=44100: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096 samplerate=48000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000 samplerate=64000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE, hence FSREF=96000 samplerate=88200: pll_p=1, pll_r=1, pll_j=5, pll_d=5120, hence FSREF= 44096, note that here we used DUAL_RATE_MODE hence FSREF=88192 samplerate=96000: pll_p=1, pll_r=1, pll_j=6, pll_d=0, hence FSREF=48000, note that here we used DUAL_RATE_MODE hence FSREF = 96000
Hence according to current code the calculations of desired FSREF is correct for sysclk=16384000 I've not tried it with your patch since I don't actually understand what do you fix? :)
- loop variables are r and p but for computation of j the variables
pll_r and pll_p are used; I think r and p should be used in the computation of j, your code below:
for (r = 1; r <= 16; r++) for (p = 1; p <= 8; p++) { int clk, tmp = (codec_clk * pll_r * 10) / pll_p; u8 j = tmp / 10000;further, the codec datasheet mandates certain ranges for 'good performance', 4 <= j <= 55 (for d==0) and 4 <= j <= 11 (for d!=0) which are not checked for
Correct.
- d should actually be 5125 and not 5120
Now I see, I've verified your code handles this. This actually produce more accuracy for FSREF.
I just tested your patch works fine with sysclk=16384000 and 12288000 but fails with 11286900: it returns FSREF=45467 instead of desired 44100. I think that this is a valuable difference. Please check it.
Please check also 33868800 sysclk, it returns FSREF=47545 instead of 48000.
Regards, Vladimir
- appropriate is spell appropriately :)
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable thanks, regards, p.
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 3395cf9..e84e473 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -766,9 +766,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0;
u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 pll_d = 1;
u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1;u16 d, pll_d = 1; u8 reg;int clk; /* select data word length */ data =@@ -835,47 +836,62 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, return 0; /* Use PLL
* find an apropriate setup for j, d, r and p by iteratingover
* p and r - j and d are calculated for each fraction.* Up to 128 values are probed, the closest one wins the game.
* find an appropriate setup for j, d, r and p by iteratingover
* p, r and j first, then trying to compute the fraction d.* Up to 6528 values are probed, the closest one wins thegame. * The sysclk is divided by 1000 to prevent integer overflows. */ codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
for (r = 1; r <= 16; r++)for (p = 1; p <= 8; p++) {int clk, tmp = (codec_clk * pll_r * 10) /pll_p;
u8 j = tmp / 10000;u16 d = tmp % 10000;if (j > 63)continue;if (d != 0 && aic3x->sysclk < 10000000)continue;/* This is actually 1000 * ((j + (d/10000))
- r) / p
* The term had to be converted to get ridof the
* division by 10000 */clk = ((10000 * j * r) + (d * r)) / (10 * p);/* check whether this values get closerthan the best
* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk -last_clk)) {
pll_j = j; pll_d = d; pll_r = r;pll_p = p;
last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)break;}
- for (r = 1; r <= 16; r++)
for (p = 1; p <= 8; p++) {for (j = 4; j <= 55; j++) {/* This is actually 1000 * ((j + (d/10000)) * r) / p* The term had to be converted to get rid of the* division by 10000; d = 0 here */int clk = (1000 * j * r) / p;/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk -last_clk)) {
pll_j = j; pll_d = 0; pll_r = r; pll_p = p;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;}}- /* try with d != 0 */
- for (p = 1; p <= 8; p++) {
j = codec_clk * p / 1000;if (j < 4 || j > 11) continue;/* do not use codec_clk here since we'd loose precision */d = ((2048 * fsref * 10) / (aic3x->sysclk / 1000)) % 10000;clk = (10000 * j + d) / (10 * p);/* check whether this values get closer than the best* ones we had before */if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) {pll_j = j; pll_d = d; pll_r = 1; pll_p = 1;last_clk = clk;}/* Early exit for exact matches */if (clk == codec_clk)goto found;- } if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n",
__func__); return -EINVAL; } +found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
Hello,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I just tested your patch works fine with sysclk=16384000 and 12288000 but fails with 11286900: it returns FSREF=45467 instead of desired 44100. I think that this is a valuable difference. Please check it.
Please check also 33868800 sysclk, it returns FSREF=47545 instead of 48000.
I have put the proposed code at http://pmeerw.net/clk/ for review;
'make test' gives me: clk(16384000,48000): r 1 p 1 d 0 j 6; 6000 == 6000; 48000.000000 clk_orig(16384000,48000): r 1 p 1 d 0 j 6; 6000 == 6000; 48000.000000 clk(16384000,44100): r 1 p 1 d 5125 j 5; 5512 == 5512; 44100.000000 clk_orig(16384000,44100): r 1 p 1 d 5120 j 5; 5512 == 5512; 44096.000000 clk(11286900,48000): r 1 p 1 d 7095 j 8; 8709 == 8710; 47999.636499 clk_orig(11286900,48000): r 1 p 1 d 7100 j 8; 8710 == 8710; 48002.392090 clk(11286900,44100): r 1 p 1 d 19 j 8; 8001 == 8002; 44099.924370 clk_orig(11286900,44100): r 1 p 1 d 20 j 8; 8002 == 8002; 44100.475488 clk(12288000,48000): r 1 p 1 d 0 j 8; 8000 == 8000; 48000.000000 clk_orig(12288000,48000): r 1 p 1 d 0 j 8; 8000 == 8000; 48000.000000 clk(12288000,44100): r 1 p 1 d 3500 j 7; 7350 == 7350; 44100.000000 clk_orig(12288000,44100): r 1 p 1 d 3500 j 7; 7350 == 7350; 44100.000000 clk(33868800,48000): r 1 p 4 d 6099 j 11; 2902 == 2902; 47999.680313 clk_orig(33868800,48000): r 1 p 1 d 9020 j 2; 2902 == 2902; 47991.825000 clk(33868800,44100): r 1 p 3 d 0 j 8; 2666 == 2666; 44100.000000 clk_orig(33868800,44100): r 1 p 1 d 6660 j 2; 2666 == 2666; 44088.975000 clk(12000000,48000): r 1 p 1 d 1920 j 8; 8192 == 8192; 48000.000000 clk_orig(12000000,48000): r 1 p 1 d 1920 j 8; 8192 == 8192; 48000.000000 clk(12000000,44100): r 1 p 1 d 5264 j 7; 7526 == 7526; 44100.000000 clk_orig(12000000,44100): r 1 p 1 d 5260 j 7; 7526 == 7526; 44097.656250
where clk is my proposed code and clk_orig is the code currently in tlv320aic3x.c; results include all test cases mentioned
in parenthesis is the input (sysclk, fsref), the last column is the approximation to fsref
note that the proposed code has less deviation from fsref in all cases and manages to stay within recommended settings for sysclk 33868800
I have modified the following line to get rid of rounding errors as much as possible: /* do not use codec_clk here since we'd loose precision */ d = ((2048 * p * fsref) - j * sysclk) * 100 / (sysclk/100); and fixed a bug which always set pll_p = 1 in case d!=0
if code looks good, I'll submit a patch against linux-sound-2.6 as Mark suggested
regards, p.
Hello Peter,
Peter Meerwald wrote:
Hello,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
I just tested your patch works fine with sysclk=16384000 and 12288000 but fails with 11286900: it returns FSREF=45467 instead of desired 44100. I think that this is a valuable difference. Please check it.
Please check also 33868800 sysclk, it returns FSREF=47545 instead of 48000.
I have put the proposed code at http://pmeerw.net/clk/ for review;
note that the proposed code has less deviation from fsref in all cases and manages to stay within recommended settings for sysclk 33868800
I have modified the following line to get rid of rounding errors as much as possible: /* do not use codec_clk here since we'd loose precision */ d = ((2048 * p * fsref) - j * sysclk) * 100 / (sysclk/100); and fixed a bug which always set pll_p = 1 in case d!=0
I confirm that it handles all cases with best precision now.
if code looks good, I'll submit a patch against linux-sound-2.6 as Mark suggested
I'm ok to submit.
> I'm trying to use the SoC TLV320AIC3x codec driver with sysclk > 16384000 and
I have put the proposed code at http://pmeerw.net/clk/ for review;
I confirm that it handles all cases with best precision now. I'm ok to submit.
fix precision of PLL computation for TLV320AIC3x SoC driver, test results are at http://pmeerw.net/clk
Signed-off-by: Peter Meerwald pmeerw@pmeerw.net
--- sound/soc/codecs/tlv320aic3x.c | 80 ++++++++++++++++++++++++--------------- 1 files changed, 49 insertions(+), 31 deletions(-)
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 2b4dc2b..6f81a00 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -765,9 +765,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0; - u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; - u16 pll_d = 1; + u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; + u16 d, pll_d = 1; u8 reg; + int clk;
/* select data word length */ data = @@ -833,48 +834,65 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, if (bypass_pll) return 0;
- /* Use PLL - * find an apropriate setup for j, d, r and p by iterating over - * p and r - j and d are calculated for each fraction. - * Up to 128 values are probed, the closest one wins the game. + /* Use PLL, compute apropriate setup for j, d, r and p, the closest + * one wins the game. Try with d==0 first, next with d!=0. + * Constraints for j are according to the datasheet. * The sysclk is divided by 1000 to prevent integer overflows. */ + codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
- for (r = 1; r <= 16; r++) - for (p = 1; p <= 8; p++) { - int clk, tmp = (codec_clk * pll_r * 10) / pll_p; - u8 j = tmp / 10000; - u16 d = tmp % 10000; + for (r = 1; r <= 16; r++) + for (p = 1; p <= 8; p++) { + for (j = 4; j <= 55; j++) { + /* This is actually 1000 * ((j + (d/10000)) * r) / p + * The term had to be converted to get rid of the + * division by 10000; d = 0 here */ + int clk = (1000 * j * r) / p;
- if (j > 63) - continue; + /* check whether this values get closer than the best + * ones we had before */ + if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { + pll_j = j; pll_d = 0; pll_r = r; pll_p = p; + last_clk = clk; + }
- if (d != 0 && aic3x->sysclk < 10000000) - continue; + /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + } + }
- /* This is actually 1000 * ((j + (d/10000)) * r) / p - * The term had to be converted to get rid of the - * division by 10000 */ - clk = ((10000 * j * r) + (d * r)) / (10 * p); - - /* check whether this values get closer than the best - * ones we had before */ - if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { - pll_j = j; pll_d = d; pll_r = r; pll_p = p; - last_clk = clk; - } - - /* Early exit for exact matches */ - if (clk == codec_clk) - break; - } + /* try with d != 0 */ + for (p = 1; p <= 8; p++) { + + j = codec_clk * p / 1000; + + if (j < 4 || j > 11) continue; + + /* do not use codec_clk here since we'd loose precision */ + d = ((2048 * p * fsref) - j * aic3x->sysclk) * 100 / (aic3x->sysclk/100); + + clk = (10000 * j + d) / (10 * p); + + /* check whether this values get closer than the best + * ones we had before */ + if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { + pll_j = j; pll_d = d; pll_r = 1; pll_p = p; + last_clk = clk; + } + + /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + }
if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
On Mon, Dec 14, 2009 at 11:24:58AM +0100, Peter Meerwald wrote:
- /* try with d != 0 */
- for (p = 1; p <= 8; p++) {
j = codec_clk * p / 1000;
There are many codign style issues around the indentation in this code - could you please run the patch through scripts/checkpatch.pl and clean up the issues it identifies?
There are many codign style issues around the indentation in this code - could you please run the patch through scripts/checkpatch.pl and clean up the issues it identifies?
cleaned up patch below...
fix precision of PLL computation for TLV320AIC3x SoC driver, test results are at http://pmeerw.net/clk
Signed-off-by: Peter Meerwald pmeerw@pmeerw.net Acked-by: Vladimir Barinov vova.barinov@gmail.com
---
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c index 2b4dc2b..5a8f53c 100644 --- a/sound/soc/codecs/tlv320aic3x.c +++ b/sound/soc/codecs/tlv320aic3x.c @@ -765,9 +765,10 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, struct snd_soc_codec *codec = socdev->card->codec; struct aic3x_priv *aic3x = codec->private_data; int codec_clk = 0, bypass_pll = 0, fsref, last_clk = 0; - u8 data, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; - u16 pll_d = 1; + u8 data, j, r, p, pll_q, pll_p = 1, pll_r = 1, pll_j = 1; + u16 d, pll_d = 1; u8 reg; + int clk;
/* select data word length */ data = @@ -833,48 +834,70 @@ static int aic3x_hw_params(struct snd_pcm_substream *substream, if (bypass_pll) return 0;
- /* Use PLL - * find an apropriate setup for j, d, r and p by iterating over - * p and r - j and d are calculated for each fraction. - * Up to 128 values are probed, the closest one wins the game. + /* Use PLL, compute apropriate setup for j, d, r and p, the closest + * one wins the game. Try with d==0 first, next with d!=0. + * Constraints for j are according to the datasheet. * The sysclk is divided by 1000 to prevent integer overflows. */ + codec_clk = (2048 * fsref) / (aic3x->sysclk / 1000);
for (r = 1; r <= 16; r++) for (p = 1; p <= 8; p++) { - int clk, tmp = (codec_clk * pll_r * 10) / pll_p; - u8 j = tmp / 10000; - u16 d = tmp % 10000; + for (j = 4; j <= 55; j++) { + /* This is actually 1000*((j+(d/10000))*r)/p + * The term had to be converted to get + * rid of the division by 10000; d = 0 here + */ + int clk = (1000 * j * r) / p; + + /* Check whether this values get closer than + * the best ones we had before + */ + if (abs(codec_clk - clk) < + abs(codec_clk - last_clk)) { + pll_j = j; pll_d = 0; + pll_r = r; pll_p = p; + last_clk = clk; + } + + /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + } + }
- if (j > 63) - continue; + /* try with d != 0 */ + for (p = 1; p <= 8; p++) { + j = codec_clk * p / 1000;
- if (d != 0 && aic3x->sysclk < 10000000) - continue; + if (j < 4 || j > 11) + continue;
- /* This is actually 1000 * ((j + (d/10000)) * r) / p - * The term had to be converted to get rid of the - * division by 10000 */ - clk = ((10000 * j * r) + (d * r)) / (10 * p); + /* do not use codec_clk here since we'd loose precision */ + d = ((2048 * p * fsref) - j * aic3x->sysclk) + * 100 / (aic3x->sysclk/100);
- /* check whether this values get closer than the best - * ones we had before */ - if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { - pll_j = j; pll_d = d; pll_r = r; pll_p = p; - last_clk = clk; - } + clk = (10000 * j + d) / (10 * p);
- /* Early exit for exact matches */ - if (clk == codec_clk) - break; + /* check whether this values get closer than the best + * ones we had before */ + if (abs(codec_clk - clk) < abs(codec_clk - last_clk)) { + pll_j = j; pll_d = d; pll_r = 1; pll_p = p; + last_clk = clk; }
+ /* Early exit for exact matches */ + if (clk == codec_clk) + goto found; + } + if (last_clk == 0) { printk(KERN_ERR "%s(): unable to setup PLL\n", __func__); return -EINVAL; }
+found: data = aic3x_read_reg_cache(codec, AIC3X_PLL_PROGA_REG); aic3x_write(codec, AIC3X_PLL_PROGA_REG, data | (pll_p << PLLP_SHIFT)); aic3x_write(codec, AIC3X_OVRF_STATUS_AND_PLLR_REG, pll_r << PLLR_SHIFT);
On Mon, Dec 14, 2009 at 02:44:56PM +0100, Peter Meerwald wrote:
There are many codign style issues around the indentation in this code - could you please run the patch through scripts/checkpatch.pl and clean up the issues it identifies?
cleaned up patch below...
Applied, thanks.
Hello Peter,
Peter Meerwald wrote:
fix precision of PLL computation for TLV320AIC3x SoC driver, test results are at http://pmeerw.net/clk
Signed-off-by: Peter Meerwald pmeerw@pmeerw.net
if you could make checkpatch.pl passed then: Acked-by: Vladimir Barinov vova.barinov@gmail.com
Regards, Vladimir
On Tue, Dec 08, 2009 at 12:28:30PM +0100, Peter Meerwald wrote:
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
Please submit patches against the most recent for- branch of
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git
You've also forgotten your Signed-off-by.
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
Might be helpful to point out what these constraints are and how the code fails to respect them - given that your change is a total rewrite of the relevant code it's not as easy as it might be to spot what's changed.
On Tue, 2009-12-08 at 12:28 +0100, Peter Meerwald wrote:
Hello,
I'm trying to use the SoC TLV320AIC3x codec driver with sysclk 16384000 and ran into some problems with setting PLL; below is a patch against linux-2.6-asoc
note that the original code uses variables pll_r and pll_p instead of the loop variable r and p to compute tmp, this seems broken
further, the original code does not respect the constraints on j (>= 4, <= 55 for d==0) according to the codec's datasheet, and similarly for d!=0
I've tested the code with a number of reasonable sysclk values and got sane PLL values; please apply if acceptable
Can you confirm you also tested against 12.288MHz and 11.2869MHz ?
If so, Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Thanks
Liam
participants (5)
-
Liam Girdwood -
Mark Brown -
Peter Meerwald -
Vladimir Barinov -
Vladimir Barinov