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? :) >>> >> >> 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 >> > Correct. >> 2. 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 >> 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 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); >>>> >>>> >> >> > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel