Radeon 9000 Pro corruption problem on PPC when DRI is enabled

[Ari Entlich]

On Tue, 2006-08-22 at 01:42 +0200, Roland Scheidegger wrote:
> Ari Entlich wrote:
> > On Mon, 2006-08-21 at 09:20 +1000, Benjamin Herrenschmidt wrote:
> >>>> (II) RADEON(0): PLL parameters: rf=0 rd=12 min=0 max=2097152; 
> >>>> xclk=0
> >>> These parameters look very bogus. Either your video bios has 
> >>> simply screwed up values there or the driver fails to read it 
> >>> correctly (for instance different encoding than assumed).
> > 
> > Here's something PLL-related from a 6.8.2 logfile I have kicking 
> > around:
> > 
> > (WW) RADEON(0): Video BIOS not detected, using default clock 
> > settings! (II) RADEON(0): Probed PLL values: xtal: 27.000000 Mhz, 
> > sclk: 274.500000 Mhz, mclk: 249.750000 Mhz
> Yes, according to Ben that's what should happen. It seems that a check
> is missing for what sort of bios it is (xorg only checks signature, but
> not type, radeonfb checks signature and type). That said, I'm wondering
> why the bios check failed with xorg 6.8.2, as it didn't test for type
> neither. Maybe it failed to map the rom completely. Do you have a full
> log available from xorg 6.8.2?
> 

Yeah, I was going to say (before you beat me to it :p) that it looks to
me than nothing relevant changed in the Radeon driver itself (functions
RADEONGetClockInfoFromBIOS, RADEONGetClockInfo, and RADEONGetBIOSInfo),
but that something might've changed along the chain of calls that
xf86ReadPciBIOS starts. 

You can get the complete 6.8.2 logfile at
http://home.twcny.rr.com/fandaswebsite/xorg_6_8_2_log .

> > Hmm...
> > 
> >> If it's a "mac" card, it has no x86 BIOS with the expected tables 
> >> in it. radeonfb can get some of the values from the Open Firmware,
> >>  X doesn't know how to do that. However, X should detect that case
> >>  and fallback to "measuring" the values from the card. It looks
> >> like that didn't happen which I find a bit strange. Worth looking
> >> at what's going on in the code. If it's not a "mac" card (an x86
> >> card in Pegasos machine for example), then I don't know for sure, 
> >> reading the ROM should have worked.
> If I understand that correctly, the driver can't use _anything_ it reads
> from the bios (clock info, connector info,...)? A true wonder it almost
> works then I guess with all those bogus values...
> 

I can now confirm that this is a mac card (can you even use non-mac
cards in a mac?). I checked the specs on ATI's website - the non-mac
cards have a VGA port, something which this thing has not. The lspci
output does have that "expansion rom" line. It says "disabled" though,
hmm...

Well, I'd like to point out that the screen is perfectly usable with DRI
off, so using those bogus BIOS values can't be messing stuff up too much
(unless they're only used by the DRI? when UseFBDev is off?). Also, all
that PLL stuff happens when UseFBDev is on too...

> 
> >> so RADEONGetClockInfoFromBIOS() should have returned FALSE and for
> >>  some reason didn't. I suspect we aren't doing a necessary sanity 
> >> check on the ROM image before reading things from it (not even 
> >> checking it's actually an ATI x86 BIOS ROM image). We should add 
> >> that (radeonfb does some basic checkings like that too, at least 
> >> checks it's an x86 BIOS ROM image). We should also add some sanity
> >>  checking of the values read from the ROM.
> It looks like adding a check for the bios type is the right thing to do.
> I'd hope it's not possible that it might fail wrongly on some cards?
> 
> >> I'm in a hurry at the moment and thus can't produce a patch, so 
> >> feel free to beat me to it there. If not, I'll have a look later 
> >> this week.
> For producing proper patches, I need to install git first (later this
> week)...
> 
> 
> > I could /try/ to look at the code, but I know nothing about Radeon 
> > hardware and my C skills aren't quite professional-grade just yet, so
> >  I can't promise anything :-).
> The attached patch might fix things. Completely untested, I think it
> should apply to pretty much any version of radeon_bios.c you can find...
> 

Yeah, ok, I'll try that. God, I love Gentoo :-). Is that the test
radeonfb uses?

> 
> > I don't think it is strict about modelines.
> Ok, it looks like this wasn't an issue.
> 
> > However, above all, I don't want to get sidetracked off of my 
> > original complaint. I would much rather have an un-corrupted screen 
> > (see original message for links to pictures) with UseFBDev true (even
> >  if it is ugly and unnecessary) than a corrupted screen with UseFBDev
> >  false.
> I'm not exactly sure if reading wrong bios stuff could cause issues even 
> with UseFBDev, but there is some hope you won't get corruption when not 
> using UseFBDev (if it would work at all...). No promises, though, I'm a 
> bit lost what could cause that corruption, but indeed the broken bios 
> stuff sounds a bit like an unlikely reason. But you never know, and the 
> bios bug needs fixing anyway.

Hehe. Yeah, ok.

> 
> Roland
> plain text document attachment (radeon_bios.c.diff)
> --- src-radeon_bios.c	2006-08-22 01:20:14.000000000 +0200
> +++ src-radeon_bios.c.patched	2006-08-22 01:32:00.000000000 +0200
> @@ -77,7 +77,15 @@
>  	xfree (info->VBIOS);
>  	info->VBIOS = NULL;
>  	return FALSE;
> -    } 
> +    }
> +
> +    if (info->VBIOS[14] != 0x0) {
> +	xf86DrvMsg(pScrn->scrnIndex, X_WARNING,
> +		   "Not a x86 BIOS ROM Image, BIOS data will not be used\n");
> +	xfree (info->VBIOS);
> +	info->VBIOS = NULL;
> +	return FALSE;
> +    }
>  
>      if (info->VBIOS) info->ROMHeaderStart = RADEON_BIOS16(0x48);
>  

Thanks,
 Ari