[linux-dvb] [RFC] Change dvb-pll handling of IF frequency

[Michael Krufky]

Trent Piepho wrote:
> The basic frequency calculation done in dvb-pll is to take the desired
> frequency, add the IF frequency, and then divide by the reference
> frequency.
> 
> When dvb-pll does the division, it just rounds the result down.  It would
> make more sense to round to the nearest integer, which is done by adding
> 1/2 the divisor (the reference frequency) before dividing.
> 
> Some PLL definitions already do this, by adding one half the reference
> frequency to the IF frequency.  The effect is the same, except one looking
> at the code doesn't know if they are seeing the IF frequency, or the IF
> frequency plus 1/2 the reference frequency.  Of course some PLL defintions
> do it one way and some do it the other and there is no record of which is
> which.
> 
> I think it makes a lot more sense put just the IF frequency in the PLL
> definition, and take care of rounding properly in the dvb-pll code.  So I
> have attached a patch which does this.  In order to do this, I had to guess
> which PLLs had stepsize/2 added to the offset and which didn't.
> 
> The second patch moves the IF frequency out of the per-band data.  There is
> no reason for the IF to change from one band to another, and in all of the
> PLLs defined the IF is constant across bands.
> 
> As far as which PLLs had offsets with stepsize/2 added and which didn't,
> I've made a list of all the IF frequencies defined in dvb-pll:
> 
> dvb_pll_thomson_dtt7610         44
> dvb_pll_microtune_4042          44
> dvb_pll_thomson_dtt761x         44
> dvb_pll_lg_tdvs_h06xf           44
> dvb_pll_tdhu2                   44
> dvb_pll_tuv1236d                44
> dvb_pll_samsung_tbmv            44
> dvb_pll_tua6010xs               36 1/8
> dvb_pll_thomson_dtt7579         36 1/6
> dvb_pll_tded4                   36 1/6
> dvb_pll_tua6034                 36 1/6
> dvb_pll_unknown_1               36 1/6
> dvb_pll_thomson_dtt759x         36 1/6
> dvb_pll_lg_z201                 36 1/6
> dvb_pll_philips_td1316          36 1/6 (rounded to nearest kHz)
> dvb_pll_tda665x                 36 1/6 (rounded to nearest kHz) + step/2
> dvb_pll_fmd1216me               36 1/8 (rounded to two decimals) + step/2
> dvb_pll_thomson_fe6600          36 1/8 (rounded to two decimals) + step/2
> dvb_pll_env57h1xd5              36 1/8 + step
> dvb_pll_philips_sd1878_tda8261  499
> 
> Anyway, from this list I think it's clear that the PLLs with a pre-adjusted
> offset are dvb_pll_tda665x, dvb_pll_fmd1216me, dvb_pll_thomson_fe6600, and
> dvb_pll_env57h1xd5.
> 
> dvb_pll_philips_sd1878_tda8261 is an odd one, as it's the only satellite
> PLL with zero-IF output in the list.  It uses an offset of 499 with a
> stepsize of 500 kHz so that it rounds _up_.  I've left this PLL rounding
> up, assuming there is a reason it was this way.
> 
> Also, dvb_pll_env57h1xd5 is not 36 1/8 + step/2, but 36 1/8 + step.  I
> think this is a bug.  If you go back to patch 1740 which created the
> dvb-pll definition, you'll see it was taken from the dibusb driver.  The
> original code for this pll was:
> 
> 	u32 freq_khz = fep->frequency / 1000;
> 	u32 tfreq = ((freq_khz + 36125)*6 + 500) / 1000;
> 
> If you multiple that out and reduce it, you get:
>    tfreq = (fep->frequency + 36125000 + 166667/2) / 166667;
> 
> But the dvb-pll definition is using 36125000 + 166667.

Trent,

This looks good to me, pending review from the other dvb developers.

Acked-by: Michael Krufky <mkrufky at linuxtv dot org>