[PATCH] FBDev: fix for fbcon Oops

[PATCH] FBDev: fix for fbcon Oops

[Antonino Daplas]

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

Hi,

Attached is a patch against 2.5.49 + James Simmons' latest fbdev.diff.

This fixes fbcon oopsing at load time which is due to the fb_cursor's
palette index entries being improperly updated.

A new kind of 'emacs glitch'  appeared, though. This can be described as
a block of text that is incompletely  copied, ie when "tabbing" a line
of text.  This is present with all hardware I tested, soft accel and
hardware accel, which indicates a problem in the higher layers (probably
fbcon). vgacon works okay.  

Other changes:

1. optimization of fbcon_accel_putcs()
2. enabling logo displays at all packed pixel formats (as long as
fb_imageblit is supported)
3. Various fbcon_accel_cursor() fixes which would have resulted in
wrong cursor colors or an invisible cursor.



Tony



[-- Attachment #2: fbcon.c.diff --]
[-- Type: text/x-patch, Size: 9458 bytes --]

diff -Naur linux-2.5.49/drivers/video/Makefile linux/drivers/video/Makefile
--- linux-2.5.49/drivers/video/Makefile	2002-11-28 06:29:45.000000000 +0000
+++ linux/drivers/video/Makefile	2002-11-28 06:30:35.000000000 +0000
@@ -69,7 +69,7 @@
 obj-$(CONFIG_FB_TX3912)           += tx3912fb.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 
 obj-$(CONFIG_FB_MATROX)		  += matrox/
-obj-$(CONFIG_FB_RIVA)		  += riva/
+obj-$(CONFIG_FB_RIVA)		  += riva/ cfbfillrect.o cfbimgblt.o cfbcopyarea.o
 obj-$(CONFIG_FB_SIS)		  += sis/
 obj-$(CONFIG_FB_ATY)		  += aty/ cfbimgblt.o
 
diff -Naur linux-2.5.49/drivers/video/console/fbcon.c linux/drivers/video/console/fbcon.c
--- linux-2.5.49/drivers/video/console/fbcon.c	2002-11-28 06:30:09.000000000 +0000
+++ linux/drivers/video/console/fbcon.c	2002-11-28 06:33:39.000000000 +0000
@@ -153,6 +153,8 @@
 static int cursor_on;
 static int cursor_blink_rate;
 
+static u8 fbcon_pixmap[8192];
+
 static inline void cursor_undrawn(void)
 {
 	vbl_cursor_cnt = 0;
@@ -353,7 +355,8 @@
 {
 	struct fb_info *info = p->fb_info;
 	unsigned short charmask = p->charmask;
-	unsigned int width = ((vc->vc_font.width + 7) >> 3);
+	unsigned int width = ((vc->vc_font.width + 7)/8);
+	unsigned int cellsize = vc->vc_font.height * width;
 	struct fb_image image;
 	u16 c = scr_readw(s);
 
@@ -361,16 +364,37 @@
 	image.bg_color = attr_bgcol(p, c);
 	image.dx = xx * vc->vc_font.width;
 	image.dy = yy * vc->vc_font.height;
-	image.width = vc->vc_font.width;
 	image.height = vc->vc_font.height;
 	image.depth = 1;
 
-	while (count--) {
-		image.data = p->fontdata + 
-			(scr_readw(s++) & charmask) * vc->vc_font.height * width;
+	if (!(vc->vc_font.width & 7) && fbcon_pixmap != NULL) {
+		unsigned int pitch = width * count, i, j;
+		char *src, *dst, *dst0;
+
+		dst0 = fbcon_pixmap;
+		image.width = vc->vc_font.width * count;
+		image.data = fbcon_pixmap;
+		while (count--) {
+			src = p->fontdata + (scr_readw(s++) & charmask) * cellsize;
+			dst = dst0;
+			for (i = image.height; i--; ) {
+				for (j = 0; j < width; j++) 
+					dst[j] = *src++;
+				dst += pitch;
+			}
+			dst0 += width;
+		}
 		info->fbops->fb_imageblit(info, &image);
-		image.dx += vc->vc_font.width;
-	}	
+	}
+	else {
+		image.width = vc->vc_font.width;
+		while (count--) {
+			image.data = p->fontdata + 
+				(scr_readw(s++) & charmask) * vc->vc_font.height * width;
+			info->fbops->fb_imageblit(info, &image);
+			image.dx += vc->vc_font.width;
+		}	
+	}
 }
 
 void fbcon_accel_clear_margins(struct vc_data *vc, struct display *p,
@@ -425,7 +449,7 @@
 
 	if ((vc->vc_cursor_type & 0x0f) != shape) {
 		shape = vc->vc_cursor_type & 0x0f;
-		cursor.set |= FB_CUR_SETSIZE;
+		cursor.set |= FB_CUR_SETSHAPE;
 	}
 
 	c = scr_readw((u16 *) vc->vc_pos);
@@ -435,10 +459,7 @@
 		fgcolor = (int) attr_fgcol(p, c);
 		bgcolor = (int) attr_bgcol(p, c);
 		cursor.set |= FB_CUR_SETCMAP;
-		cursor.image.bg_color = bgcolor;
-		cursor.image.fg_color = fgcolor;
 	}
-
 	c &= p->charmask;
 	font = p->fontdata + (c * ((width + 7) / 8) * height);
 	if (font != dest) {
@@ -453,7 +474,7 @@
 
 	if (cursor.set & FB_CUR_SETSIZE) {
 		memset(image, 0xff, 64);
-		cursor.set |= FB_CUR_SETSIZE;
+		cursor.set |= FB_CUR_SETSHAPE;
 	}		
 
 	if (cursor.set & FB_CUR_SETSHAPE) {
@@ -497,6 +518,8 @@
 	cursor.image.dy = yy * height;
 	cursor.image.depth = 1;
 	cursor.image.data = image;
+	cursor.image.bg_color = bgcolor;
+	cursor.image.fg_color = fgcolor;
 	cursor.mask = mask;
 	cursor.dest = dest;
 	cursor.rop = ROP_XOR;
@@ -2487,20 +2510,16 @@
 	struct display *p = &fb_display[fg_console];	/* draw to vt in foreground */
 	struct fb_info *info = p->fb_info;
 	struct vc_data *vc = info->display_fg;
-#ifdef CONFIG_FBCON_ACCEL
 	struct fb_image image;
 	u32 *palette = NULL, *saved_palette = NULL;
-#endif
 	int depth = info->var.bits_per_pixel;
-	int line = info->fix.line_length;
 	unsigned char *fb = info->screen_base;
-	unsigned char *logo;
-	unsigned char *dst, *src;
-	int i, j, n, x1, y1, x;
+	unsigned char *logo, *logo_new = NULL;
+	int i, j, n, x;
 	int logo_depth, done = 0;
 
 	/* Return if the frame buffer is not mapped */
-	if (!fb)
+	if (!fb || !info->fbops->fb_imageblit)
 		return 0;
 
 	/*
@@ -2543,65 +2562,114 @@
 		logo_depth = 1;
 	}
 
-#if defined(CONFIG_FBCON_ACCEL)
-	if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
-		unsigned char mask[9] =
-		    { 0, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff };
+    {
+	    int needs_truepalette = 0;
+	    int needs_directpalette = 0;
+	    int needs_logo = 0;
+
+	    switch (info->fix.visual) {
+	    case FB_VISUAL_TRUECOLOR:
+		    needs_truepalette = 1;
+		    break;
+	    case FB_VISUAL_DIRECTCOLOR:
+		    if (depth >= 24) 
+			    needs_directpalette = 1;
+		    /* 16 colors */
+		    else if (depth >= 16)
+			    needs_logo = 4;
+		    /* 2 colors */
+		    else
+			    needs_logo = 1;
+		    break;
+	    case FB_VISUAL_PSEUDOCOLOR:
+	    default:
+		    /* 16 colors */
+		    if (depth >= 4 && depth < 8)
+			    needs_logo = 4;
+		    /* 4 colors */
+		    else if (depth < 4)
+			    needs_logo = 1;
+		    break;
+	    }		    
+	if (needs_truepalette || needs_directpalette) {
+		unsigned char mask[9] = { 0,0x80,0xc0,0xe0,0xf0,0xf8,0xfc,0xfe,0xff };
 		unsigned char redmask, greenmask, bluemask;
 		int redshift, greenshift, blueshift;
 
-		/* Bug: Doesn't obey msb_right ... (who needs that?) */
-		redmask =
-		    mask[info->var.red.length <
-			 8 ? info->var.red.length : 8];
-		greenmask =
-		    mask[info->var.green.length <
-			 8 ? info->var.green.length : 8];
-		bluemask =
-		    mask[info->var.blue.length <
-			 8 ? info->var.blue.length : 8];
-		redshift =
-		    info->var.red.offset - (8 - info->var.red.length);
-		greenshift =
-		    info->var.green.offset - (8 - info->var.green.length);
-		blueshift =
-		    info->var.blue.offset - (8 - info->var.blue.length);
-
 		/*
-		 * We have to create a temporary palette since console palette is only
+	 	 * We have to create a temporary palette since console palette is only
 		 * 16 colors long.
-		 */
+	 	 */
 		palette = kmalloc(256 * 4, GFP_KERNEL);
 		if (palette == NULL)
-			return (LOGO_H + vc->vc_font.height - 1)/vc->vc_font.height;
+			return 1;
+
+		if (needs_truepalette) {
+			/* Bug: Doesn't obey msb_right ... (who needs that?) */
+			redmask   = mask[info->var.red.length   < 8 ? info->var.red.length   : 8];
+			greenmask = mask[info->var.green.length < 8 ? info->var.green.length : 8];
+			bluemask  = mask[info->var.blue.length  < 8 ? info->var.blue.length  : 8];
+			redshift   = info->var.red.offset   - (8 - info->var.red.length);
+			greenshift = info->var.green.offset - (8 - info->var.green.length);
+			blueshift  = info->var.blue.offset  - (8 - info->var.blue.length);
+
 
-		for (i = 0; i < LINUX_LOGO_COLORS; i++) {
-			palette[i + 32] =
-			    (safe_shift
-			     ((linux_logo_red[i] & redmask),
-			      redshift) | safe_shift((linux_logo_green[i] &
-						      greenmask),
-						     greenshift) |
-			     safe_shift((linux_logo_blue[i] & bluemask),
-					blueshift));
+			for ( i = 0; i < LINUX_LOGO_COLORS; i++) {
+				palette[i+32] = (safe_shift((linux_logo_red[i]   & redmask), redshift) |
+						 safe_shift((linux_logo_green[i] & greenmask), greenshift) |
+						 safe_shift((linux_logo_blue[i]  & bluemask), blueshift));
+			}
+		}
+		else {
+			redshift = info->var.red.offset;
+			greenshift = info->var.green.offset;
+			blueshift = info->var.blue.offset;
+
+			for (i = 32; i < LINUX_LOGO_COLORS; i++) 
+				palette[i] = i << redshift | i << greenshift | i << blueshift;
 		}
 		saved_palette = info->pseudo_palette;
 		info->pseudo_palette = palette;
 	}
+	if (needs_logo) {
+		logo_new = kmalloc(LOGO_W * LOGO_H, GFP_KERNEL);
+		if (logo_new == NULL)
+			return 1;
+
+		logo = logo_new;
+		switch (needs_logo) {
+		case 4:
+		{
+			for (i = 0; i < (LOGO_W * LOGO_H)/2; i++) {
+				logo_new[i*2] = linux_logo16[i] >> 4;
+				logo_new[(i*2)+1] = linux_logo16[i] & 0xf;
+			}
+		}
+		break;
+		case 1:
+		default:
+		{
+			int j;
+			for (i = 0; i < (LOGO_W * LOGO_H)/8; i++) 
+				for (j = 0; j < 8; j++) 
+					logo_new[i*2] = linux_logo_bw[i] &  (7 - j);
+		}
+		break;
+		} 
+	}
 	image.width = LOGO_W;
 	image.height = LOGO_H;
 	image.depth = depth;
 	image.data = logo;
 	image.dy = 0;
-#endif
+    }
 
 	for (x = 0; x < num_online_cpus() * (LOGO_W + 8) &&
 	     x < info->var.xres - (LOGO_W + 8); x += (LOGO_W + 8)) {
-#if defined (CONFIG_FBCON_ACCEL)
 		image.dx = x;
 		info->fbops->fb_imageblit(info, &image);
 		done = 1;
-#endif
+#if 0 /* Turn off, should be removed soon enough... */
 #if defined(CONFIG_FBCON_AFB) || defined(CONFIG_FBCON_ILBM) || \
     defined(CONFIG_FBCON_IPLAN2P2) || defined(CONFIG_FBCON_IPLAN2P4) || \
     defined(CONFIG_FBCON_IPLAN2P8)
@@ -2733,14 +2801,14 @@
 			done = 1;
 		}
 #endif
+#endif /* Turn off for now... */
 	}
-
-#if defined (CONFIG_FBCON_ACCEL)
 	if (palette != NULL)
 		kfree(palette);
 	if (saved_palette != NULL)
 		info->pseudo_palette = saved_palette;
-#endif
+	if (logo_new != NULL)
+		kfree(logo_new);
 	/* Modes not yet supported: packed pixels with depth != 8 (does such a
 	 * thing exist in reality?) */
 

Re: [Linux-fbdev-devel] [PATCH] FBDev: fix for fbcon Oops

[James Simmons]

> This fixes fbcon oopsing at load time which is due to the fb_cursor's
> palette index entries being improperly updated.

Fixed.

> A new kind of 'emacs glitch'  appeared, though. This can be described as
> a block of text that is incompletely  copied, ie when "tabbing" a line
> of text.  This is present with all hardware I tested, soft accel and
> hardware accel, which indicates a problem in the higher layers (probably
> fbcon). vgacon works okay.  

Its is fbcon. If you play with the low level driver to draw things you 
noticed everything is okay. I have to track it down.
 
> Other changes:
> 
> 1. optimization of fbcon_accel_putcs()

Applied. If you look at my patch I attempted to make fbcon_pixmap dynamic 
instead of a static memeory area. It oops so I have that code commented 
out for now. Actually I applied it when the fix came out rigth away.

> 2. enabling logo displays at all packed pixel formats (as long as
> fb_imageblit is supported)

This didn't apply well. Only thing missing.

> 3. Various fbcon_accel_cursor() fixes which would have resulted in
> wrong cursor colors or an invisible cursor.

Applied. Worked now :-)