[linux-dvb] Large arrays

Trent Piepho xyzzy at speakeasy.org
Wed Aug 29 09:19:37 CEST 2007


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);
}



More information about the linux-dvb mailing list