On Tue, 28 Aug 2007, Manu Abraham wrote: > Trent Piepho wrote: > > That's really inefficient. You've got about 250k of table there. I don't > > think a 250k+ module is going to be very popular. > > > > I do agree that's not the most effecient way. But given the short time > span and lack of FP operations, the quick way that came to my mind was > precomputed values. > > That said, the popularity of a module is not defined by it's size. as an > eg: the nVidia module is over 1 Meg. Considering the quality/performance > of the tuners in the tree we have, i don't think there would even be a > popularity contest, coming anywhere near. But nobody likes how huge the nvidia module is. > That said, if it can be optimized, there is no reason why it shouldn't > be done. > > I had a comment from Andrew Morton as well on it: "It's large, but heck, > it's not a lot of memory. The nvidia driver is a meg or something." Huge tables like that can be a problem just for the code itself. Huge patches, show checkouts and diffs, slow compiles, etc. There are also problems with maintaining such code. Say you need to tweak the tuning algorithm? Instead of just changing one constant or making an expression round up instead of down, you have to create a whole new table. And what if there is a new device revision, which is slightly different? You have to have another huge table, when you could probably support both with just a small addition to the code. Or say someone has a fake Russian copy, that uses a 26 Mhz crystal instead of 27 MHz? Much harder to support if you look at the driver and just see some huge table with no idea what made it, instead of one simple constant to change. For instance, when radio support for my tuner didn't work, I was able to figure out from the tuner-simple code that it was using the wrong 1st IF offset for the tda9887 radio demodulator and 2nd IF output. If there was just some huge table of random settings, it would have been much harder to fix. > If anyone's interested, the original version with FP is here: > > http://paste.debian.net/35554 > > It 's a userspace application with which i did some crude tests. > > The tables are here > > http://jusst.de/manu/1086M_36.125M.c > http://jusst.de/manu/1086M_36.166M.c I looked at this code, but there is code there that isn't used, so I'm not sure exactly what it's supposed to. There is a 'optimal_settings_index' parameter to mc44s80x_set_1st_lo(), but it's never used. There is still a huge table, 'tuner_step', where did that come from? The whole table isn't used, only index 1. And lo2_divider isn't used either. Is there some kind of formula that can calculate this table? The 'lo1_adjustment' code is very odd. Are you sure it's really doing what it's supposed to do? The actual code in mc44s80x_set_1st_lo() is just a round-about way of doing a rather simple bit of math. So, here is a integer version of your floating point code. It's actually _more_ accurate than the FP version. For your sample (226500000 Hz, lo1_divider 13, lo1_adjustment 0), my code finds the same divider as yours, 630. But the 'first_lo' value I find is 1,312,615,385 Hz, while your FP code returns 1,312,615,336 Hz. Mine is within 1 Hz of the true value of 1312615384.615384. #ifdef __KERNEL__ #include <asm/div64.h> #else #define do_div(n, base) ({ unsigned long __rem = (n)%(base); (n)/=(base); __rem; }) typedef unsigned long long u64; #endif /* freq = input frequency in Hz refdiv1 = first reference frequency divisor (10 to 15, or 26) adj1 = mystery adjustment (signed number!) */ /* To turn the lo1_adjustment value into adj1, do this: adj1 = (lo1_adjustment+1) / ((lo1_adjustment&1)?-2:2); */ #define REF_FREQ 27000000 /* 27 MHz reference */ #define IF1 1086000000 void mc44s80x_set_1st_lo(unsigned int freq, unsigned int refdiv1, int adj1) { u64 div1; unsigned int remainder, freq1, if1, lo1ref; signed int if1_offset; div1 = ((u64)freq + IF1) * refdiv1 + REF_FREQ/2; /* Subtract 1 to round XXX.5 down instead of up */ printf("freq+IF * refdiv1 = %llu\n", div1); remainder = do_div(div1, REF_FREQ); /* adjustment sign is flipped if we didn't round div1 down. Very stange, I don't understand what this is for. */ if (remainder <= REF_FREQ/2) adj1 *= -1; div1 += adj1; /* Actual frequency PLL tuned, taking into account step size, the IF * offset, and the mystery adjustment. */ freq1 = (div1 * REF_FREQ + refdiv1/2) / refdiv1; /* Actual IF used, taking into acount everything freq1 does */ if1 = freq1 - freq; /* Offset of actual IF from nominal IF */ if1_offset = if1 - IF1; /* LO1REF, whatever that is */ /* BTW, do you really want to round down, not to the nearest? */ lo1ref = 15 * (8/2) * div1 / refdiv1; /* The "n+2" divider */ div1 = (div1<2) ? 0 : div1-2; printf("frequency: %u lo1: %u ", freq, freq1); printf("ref div1: %u ", refdiv1); printf("if1: %u off: %d\n", if1, if1_offset); printf("n=%d\n", (int)div1); printf("LO1REF: %d\n", lo1ref); } _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
