Re: 2.6 fbcon and 15/16 bpp

[John Zielinski <grim@undead.cc>]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

Benjamin Herrenschmidt wrote:

>>Little.  It's an single processor Athalon box.  I'll do the test later 
>>tonight as I don't want to reboot the other machine at the moment.
>>    
>>
>
>Ok, so this isn't an endian problem in the cfb_ routines. Thanks.
>I'll try to figure out what's up here as well later this week, I'm
>a bit too busy with other things at the moment.
>
>Ben.
>  
>

Here's a patch that fixed things on my machine.  I found it by trial and 
error since I don't have a hardware reference document and the code is a 
little confusing to me.  I don't know if this is the root cause or just 
a fix for the symptoms.  Changing the u16's to u32's fixed the verticle 
grille appearance and most of the color corruption.  Commenting out that 
small block of code fixed the rest of the color problems.

I have a few questions.  Why are we going through the palette instead of 
just writing the proper directcolor/truecolor bitvalues for that color?  
Does that have to do with the disabled HW acceleration code?   Or is it 
because the hw palette color components are 8 bits wide so the displayed 
colors are more accurate?

And the code that I commented out, why is it accesing palette array 
elements that have been set on a previous call or haven't been set yet?  
Why is it trying to compensate for the wider green field when we're 
programming 8 bit values to the hw palette?

My understanding of the low level video hardware control is very limited 
beyond standard VGA register programming so I'm hoping to learn 
something with all my questions.  :)

John


[-- Attachment #2: patch.16bppfix --]
[-- Type: text/plain, Size: 1462 bytes --]

diff -urNX dontdiff linux.old/drivers/video/aty/radeon_base.c linux/drivers/video/aty/radeon_base.c
--- linux.old/drivers/video/aty/radeon_base.c	2003-12-21 17:54:33.000000000 -0500
+++ linux/drivers/video/aty/radeon_base.c	2003-12-23 17:41:07.000000000 -0500
@@ -1179,12 +1179,12 @@
 				return 1;
 
 			/* For 565, the green component is mixed one order below */
-			if (rinfo->depth == 16) {
-		                OUTREG(PALETTE_INDEX, pindex>>1);
-	       	         	OUTREG(PALETTE_DATA, (rinfo->palette[regno>>1].red << 16) |
-	                        	(green << 8) | (rinfo->palette[regno>>1].blue));
-	                	green = rinfo->palette[regno<<1].green;
-	        	}
+//			if (rinfo->depth == 16) {
+//		                OUTREG(PALETTE_INDEX, pindex>>1);
+//	       	         	OUTREG(PALETTE_DATA, (rinfo->palette[regno>>1].red << 16) |
+//	                        	(green << 8) | (rinfo->palette[regno>>1].blue));
+//	                	green = rinfo->palette[regno<<1].green;
+//	        	}
 		}
 
 		if (rinfo->depth != 16 || regno < 32) {
@@ -1197,11 +1197,11 @@
  	if (regno < 16) {
         	switch (rinfo->depth) {
 		case 15:
-			((u16 *) (info->pseudo_palette))[regno] =
+			((u32 *) (info->pseudo_palette))[regno] =
 			    (regno << 10) | (regno << 5) | regno;
 			break;
 		case 16:
-			((u16 *) (info->pseudo_palette))[regno] =
+			((u32 *) (info->pseudo_palette))[regno] =
 			    (regno << 11) | (regno << 6) | regno;
 			break;
 		case 24: