rivafb color fix?

rivafb color fix?

[Antonino Daplas]

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

Hi, 

I happened to find my old Riva128 card and tried the rivafb driver. 
Noticed that color (and the logo)is wrong at > 8bpp.

Looking at the code, it exports the visual as FB_VISUAL_DIRECTCOLOR at
16 and 32 bpp, but the DAC was not set up as such.  Also the palette is
set up as Truecolor.

What I found interesting was that accessing the framebuffer directly,
the pixel must be in directcolor (with a properly configured DAC) but
using the accel functions, the pixel value that is passed must be in
truecolor format.  

I'm not sure if this is true for all NV hardware, but I'm attaching a
patch that fixed this problem for the Riva 128 at least.  If no other
archs are affected, I'll fix the patch to be specific for NV_ARCH_03
only. (patch is against 2.4.19)

Tony

PS:

James,

While doing this, I was able to modify the rivafb 2.5 driver and included
hardware supported drawing functions.  I'm planning to port the hardware cursor as well.
You may wish to work with other cards then.

(Ani, only if you don't mind -- since I'm planning to revive an old box using the 
Riva card and rivafb)

[-- Attachment #2: riva_colorfix.diff --]
[-- Type: text/x-patch, Size: 4556 bytes --]

diff -Naur linux-2.4.19/drivers/video/riva/fbdev.c linux/drivers/video/riva/fbdev.c
--- linux-2.4.19/drivers/video/riva/fbdev.c	2002-11-26 04:07:50.000000000 +0000
+++ linux/drivers/video/riva/fbdev.c	2002-11-26 04:13:42.000000000 +0000
@@ -1122,24 +1122,20 @@
 
 	assert(var != NULL);
 
-	switch (var->bits_per_pixel) {
-#ifdef FBCON_HAS_CFB8
+	/* assume green component has the most weight */
+	switch (var->green.length) {
+#if defined (FBCON_HAS_CFB8) || defined (FBCON_HAS_CFB32)
 	case 8:
-		rc = 256;	/* pseudocolor... 256 entries HW palette */
+		rc = 256;	/* pseudocolor or truecolor >= 24bpp... 256 entries HW palette */
 		break;
 #endif
 #ifdef FBCON_HAS_CFB16
-	case 15:
-		rc = 15;	/* fix for 15 bpp depths on Riva 128 based cards */
+	case 5:
+	        rc = 32;	/* 5-bit - 32 entries spaced 8 indices apart */
 		break;
-	case 16:
-		rc = 16;	/* directcolor... 16 entries SW palette */
-		break;		/* Mystique: truecolor, 16 entries SW palette, HW palette hardwired into 1:1 mapping */
-#endif
-#ifdef FBCON_HAS_CFB32
-	case 32:
-		rc = 16;	/* directcolor... 16 entries SW palette */
-		break;		/* Mystique: truecolor, 16 entries SW palette, HW palette hardwired into 1:1 mapping */
+	case 6:
+		rc = 64;	/* 6-bit - 64 entries spaced 4 indices apart */
+		break;		
 #endif
 	default:
 		/* should not occur */
@@ -1222,6 +1218,7 @@
 	struct rivafb_info *rivainfo = (struct rivafb_info *)info;
 	RIVA_HW_INST *chip = &rivainfo->riva;
 	struct display *p;
+	int i;
 
 	DPRINTK("ENTER\n");
 
@@ -1233,6 +1230,15 @@
 	if (regno >= riva_get_cmap_len(&p->var))
 		return -EINVAL;
 
+	if (!regno) {
+		/* Reset palette */
+		for (i = 0; i < 256; i++) {
+			rivainfo->palette[regno].red = 0;
+			rivainfo->palette[regno].green = 0;
+			rivainfo->palette[regno].blue = 0;
+		}
+	}
+
 	rivainfo->palette[regno].red = red;
 	rivainfo->palette[regno].green = green;
 	rivainfo->palette[regno].blue = blue;
@@ -1243,40 +1249,56 @@
 		    (red * 77 + green * 151 + blue * 28) >> 8;
 	}
 
-	switch (p->var.bits_per_pixel) {
-#ifdef FBCON_HAS_CFB8
-	case 8:
+	if (p->var.green.length == 5) {
+		for (i = 0; i < 8; i++) {
+			riva_wclut(chip, (regno*8)+i, (u8) red, (u8) green, (u8) blue);
+		}
+	}
+	else if (p->var.green.length == 6) {
+		if (regno < 32) {
+			for (i = 0; i < 8; i++) {
+				riva_wclut(chip, (regno*8)+i, (u8) red, 
+					   rivainfo->palette[regno*2].green,
+					   (u8) blue);
+			}
+		}
+		for (i = 0; i < 4; i++) {
+			riva_wclut(chip, (regno*4)+i, rivainfo->palette[regno/2].red,
+				   (u8) green, rivainfo->palette[regno/2].blue);
+		}
+	}
+	else {
 		/* "transparent" stuff is completely ignored. */
 		riva_wclut(chip, regno, red >> 8, green >> 8, blue >> 8);
-		break;
-#endif /* FBCON_HAS_CFB8 */
+	}		   
+	if (regno < 16) {
+		switch (p->var.bits_per_pixel) {
 #ifdef FBCON_HAS_CFB16
-	case 16:
-		assert(regno < 16);
-		if (p->var.green.length == 5) {
-			/* 0rrrrrgg gggbbbbb */
-			rivainfo->con_cmap.cfb16[regno] =
-			    ((red & 0xf800) >> 1) |
-			    ((green & 0xf800) >> 6) | ((blue & 0xf800) >> 11);
-		} else {
-			/* rrrrrggg gggbbbbb */
-			rivainfo->con_cmap.cfb16[regno] =
-			    ((red & 0xf800) >> 0) |
-			    ((green & 0xf800) >> 5) | ((blue & 0xf800) >> 11);
-		}
-		break;
+		case 16:
+			if (p->var.green.length == 5) {
+				/* 0rrrrrgg gggbbbbb */
+				rivainfo->con_cmap.cfb16[regno] =
+					((red & 0xf800) >> 1) |
+					((green & 0xf800) >> 6) | ((blue & 0xf800) >> 11);
+			} else {
+				/* rrrrrggg gggbbbbb */
+				rivainfo->con_cmap.cfb16[regno] =
+					((red & 0xf800) >> 0) |
+					((green & 0xf800) >> 5) | ((blue & 0xf800) >> 11);
+			}
+			break;
 #endif /* FBCON_HAS_CFB16 */
 #ifdef FBCON_HAS_CFB32
-	case 32:
-		assert(regno < 16);
-		rivainfo->con_cmap.cfb32[regno] =
-		    ((red & 0xff00) << 8) |
-		    ((green & 0xff00)) | ((blue & 0xff00) >> 8);
-		break;
+		case 32:
+			rivainfo->con_cmap.cfb32[regno] =
+				((red & 0xff00) << 8) |
+				((green & 0xff00)) | ((blue & 0xff00) >> 8);
+			break;
 #endif /* FBCON_HAS_CFB32 */
-	default:
-		/* do nothing */
-		break;
+		default:
+			/* do nothing */
+			break;
+		}
 	}
 
 	return 0;
@@ -1403,6 +1425,13 @@
 
 	memcpy(&v, var, sizeof(v));
 
+	/*
+	 * Riva 128 supports RGB555 only 
+	 */
+	if (rivainfo->riva.Architecture == NV_ARCH_03 &&
+	    v.bits_per_pixel == 16)
+		v.bits_per_pixel = 15;
+
 	accel = v.accel_flags & FB_ACCELF_TEXT;
 
 	switch (v.bits_per_pixel) {