Re: Fix for "MT2032 Fatal Error: PLLs didn't lock"

Re: Fix for "MT2032 Fatal Error: PLLs didn't lock"

[Gerd Knorr]

> I saw others report same issues but didnt see any
> fixes/patches/solutions. With the debug option on for bttv and tuner in
> /etc/modules.conf and the TV frequency set to 133.25MHz and then
> 140.25MHz, the sign extension defect pops up in rfin value below. This
> is because 133.25MHz is 0x7F13BD0 and 140.25MHz is 0x85C0B90. The high
> bit gets sign extended in as 0xFFFFFFFFF85C0B90 (-128185456)

Huh?  I can't see where a sign extension happens.  0x85C0B90 has the bit
#27 set to one, not #31 ...

Beside that the values are passed around as unsigned values everythere.
Can you please explain in more detail what is going on?

> Nov 16 21:45:56 localhost kernel: tuner: tv freq set to 133.25
> Nov 16 21:45:56 localhost kernel: mt2032_set_if_freq rfin=133250000
> if1=1090000000 if2=38900000 from=32900000 to=39900000

> Nov 16 21:45:58 localhost kernel: tuner: tv freq set to 140.25
> Nov 16 21:45:58 localhost kernel: mt2032_set_if_freq rfin=-128185456
> if1=1090000000 if2=38900000 from=32900000 to=39900000

Works fine here:

tuner: tv freq set to 140.25
mt2032_set_if_freq rfin=140250000 if1=1090000000 if2=38900000 from=32900000 to=39900000

>  	case VIDIOCSFREQ:
>  	{
> -		unsigned long *v = arg;
> +		unsigned int *v = arg;

Wrong, VIDIOCSFREQ is "_IOW('v',15, unsigned long)".  Beside that I'm
very surprised that this actually makes a difference.  What architecture
is that?  And what size int/long have there?

> -				(*iarg)/16,(*iarg)%16*100/16);
> +				(*v)/16,(*v)%16*100/16);

That is ok.

  Gerd

-- 
You have a new virus in /var/mail/kraxel

Re: Fix for "MT2032 Fatal Error: PLLs didn't lock"

[Subbu K. K.]

On Mon, 2003-11-24 at 17:16, Gerd Knorr wrote: 
> > I saw others report same issues but didnt see any
> > fixes/patches/solutions. With the debug option on for bttv and tuner in
> > /etc/modules.conf and the TV frequency set to 133.25MHz and then
> > 140.25MHz, the sign extension defect pops up in rfin value below. This
> > is because 133.25MHz is 0x7F13BD0 and 140.25MHz is 0x85C0B90. The high
> > bit gets sign extended in as 0xFFFFFFFFF85C0B90 (-128185456)
> 
> Huh?  I can't see where a sign extension happens.  0x85C0B90 has the bit
> #27 set to one, not #31 ...
I know this sounds weird, but sign extension is happening between
invocation of set_tv_freq and the dprintk in mt2032_set_tv_freq.

> Beside that the values are passed around as unsigned values everythere.
> Can you please explain in more detail what is going on?
Except in one place where there is an implicit conversion to signed ints
in line 746:

746         mt2032_set_if_freq(c, freq*62500 /* freq*1000*1000/16 */, 
> > Nov 16 21:45:56 localhost kernel: tuner: tv freq set to 133.25
> > Nov 16 21:45:56 localhost kernel: mt2032_set_if_freq rfin=133250000
> > if1=1090000000 if2=38900000 from=32900000 to=39900000
> 
> > Nov 16 21:45:58 localhost kernel: tuner: tv freq set to 140.25
> > Nov 16 21:45:58 localhost kernel: mt2032_set_if_freq rfin=-128185456
> > if1=1090000000 if2=38900000 from=32900000 to=39900000
> 
> Works fine here:
> 
> tuner: tv freq set to 140.25
> mt2032_set_if_freq rfin=140250000 if1=1090000000 if2=38900000 from=32900000 to=39900000
Thanks for the clue. Perhaps the problem is gcc/athlon related?

> >  	case VIDIOCSFREQ:
> >  	{
> > -		unsigned long *v = arg;
> > +		unsigned int *v = arg;
> 
> Wrong, VIDIOCSFREQ is "_IOW('v',15, unsigned long)".  Beside that I'm
> very surprised that this actually makes a difference.  What architecture
> is that?  And what size int/long have there?
I agree that the change is rather kludgy. I was using stock Mandrake
9.1/gcc3.2.2 on AMD Athlon 1.4GHz. I have seen the error pop up on the
Knoppix (2.4.18..2.4.20 kernels) also on the same box so it doesnt seem
to be distro-specific. I dont have a Intel system to check if the
problem is related to the cpu. Clues anyone?

Let me poke around in the assembly code tonight to find out why the sign
change happens on my box. I suspect this could be due to gcc/athlon
combo.

Subbu K. K.

Re: Fix for "MT2032 Fatal Error: PLLs didn't lock"

[Gerd Knorr]

> Except in one place where there is an implicit conversion to signed ints
> in line 746:
> 
> 746         mt2032_set_if_freq(c, freq*62500 /* freq*1000*1000/16 */, 

Sure?  Both freq and second mt2032_set_if_freq argument are declared
unsigned int ...
What happens with "62500" changed to "(unsigned)62500" ?

> > tuner: tv freq set to 140.25
> > mt2032_set_if_freq rfin=140250000 if1=1090000000 if2=38900000 from=32900000 to=39900000
> Thanks for the clue. Perhaps the problem is gcc/athlon related?

Maybe.  gcc 3.3.1 here.

  Gerd

-- 
You have a new virus in /var/mail/kraxel

Re: Fix for "MT2032 Fatal Error: PLLs didn't lock"

[Subbu K. K.]

On Tue, 2003-11-25 at 12:28, Subbu K. K. wrote:
> Let me poke around in the assembly code tonight to find out why the sign
> change happens on my box. I suspect this could be due to gcc/athlon
> combo.
> 
ok. Found the error.

bttv-0.7.103 and earlier (kernel 2.4.20 and earlier) had the following
lines in two places:
  mt2032_set_if_freq(c,freq* 1000*1000/16, ...
When freq is defined as int, this computation causes a sign extension
error when freq is 2244 (140.25MHz). The error disappears if
1000*1000/16 is replaced with 62500 and/or if freq is defined as
unsigned int.

Here is a simple test to surface this error:
// test with gcc -o signext signext.c && ./signext
main(int argc, char *argv[])
{
	int s; unsigned f;

	s = 2244; f = s;
	printf("freq=%d rfin=%d\n", s*62500, s*1000*1000/16);
	printf("freq=%d rfin=%d\n", f*62500, f*1000*1000/16);
}
the output shows:
freq=14025000 rfin=-128185456
freq=14025000 rfin=140250000

bttv-0.7.104+ and kernel 2.4.21+ have the fix. Still, I thought of
recording the error for those still on older kernels.

Subbu K. K.

Fix for "MT2032 Fatal Error: PLLs didn't lock"

[Subbu K. K.]

Hi,

I found a sign extension defect in the bttv driver code. The defect
will manifest itself when using any TV Channels above 133.25MHz using
xawtv or any other TV viewer. I just got a white noise hiss. I was able
to reproduce the defect on Mandrake 9.1, Knoppix (Oct 2003) even with
the latest bttv 0.7.107 and in the latest test10 bttv driver code.  The
symptoms for this defect in the mesage log are:
====/var/log/messages==
Nov 16 21:42:51 localhost kernel: mt2032: re-init PLLs by LINT
Nov 16 21:42:51 localhost kernel: mt2032: re-init PLLs by LINT
Nov 16 21:42:51 localhost kernel: MT2032 Fatal Error: PLLs didn't
lock.
=================

I saw others report same issues but didnt see any
fixes/patches/solutions. With the debug option on for bttv and tuner in
/etc/modules.conf and the TV frequency set to 133.25MHz and then
140.25MHz, the sign extension defect pops up in rfin value below. This
is because 133.25MHz is 0x7F13BD0 and 140.25MHz is 0x85C0B90. The high
bit gets sign extended in as 0xFFFFFFFFF85C0B90 (-128185456)
===
Nov 16 21:45:56 localhost kernel: tuner: tv freq set to 133.25
Nov 16 21:45:56 localhost kernel: mt2032_set_if_freq rfin=133250000
if1=1090000000 if2=38900000 from=32900000 to=39900000
..
Nov 16 21:45:58 localhost kernel: tuner: tv freq set to 140.25
Nov 16 21:45:58 localhost kernel: mt2032_set_if_freq rfin=-128185456
if1=1090000000 if2=38900000 from=32900000 to=39900000
===

The patch is fairly small and involves only three lines of code to
tuner.c. The first change is required and the next two changes are
strongly recommended. Now, I am able to watch all the cable channels
using Pinnacle PCTV/Rave (branded as PCTV Plus in India) on Mandrake 9.1
without any issues. Can others confirm if the patch works for them on
the recent kernel builds.

--- bttv-0.7.107/driver/tuner.c	2003-06-24 21:52:11.000000000
+0530
+++ bttv-0.7.107-fix/driver/tuner.c	2003-11-16 23:30:47.000000000
+0530
@@ -951,15 +951,15 @@
 	}
 	case VIDIOCSFREQ:
 	{
-		unsigned long *v = arg;
+		unsigned int *v = arg;
 
 		if (t->radio) {
 			dprintk("tuner: radio freq set to %d.%02d\n",
-				(*iarg)/16,(*iarg)%16*100/16);
+				(*v)/16,(*v)%16*100/16);
 			set_radio_freq(client,*v);
 		} else {
 			dprintk("tuner: tv freq set to %d.%02d\n",
-				(*iarg)/16,(*iarg)%16*100/16);
+				(*v)/16,(*v)%16*100/16);
 			set_tv_freq(client,*v);
 		}
 		t->freq = *v;
=====

Subbu K. K.