[PATCH] fbcon: "Do not touch hardware if vc_mode != KD_TEXT: fix

Sigh, this patch uncovered a can of worms.  I tested different combinations,
those with/without xxxfb_blank implementation, framebuffers in directcolor
or truecolor, etc. I find that there is a problem unblanking if the hardware
has no xxxfb_blank() implementation, and also that the generic fb_blank() in
fbmem.c is problematic with drivers in directcolor or pseudocolor mode when
called by an fb application such as X.

Display blanking is implemented in three ways:

    1. using the drivers blanking implementation - info->fbops->fb_blank()
    2. clearing the screen with the console erase character - fbcon_blank()
    3. setting the color map to all black - fb_blank()

The third method is problematic for these reasons:

    - Setting the colormap to all black will not work in truecolor mode
    - In directcolor or pseudocolor, it will overwrite the fb application's
      color map, producing wrong colors.

So, remove the generic implementation in fb_blank() and just return -EINVAL
if there is no hardware implementation.  This will be only used by apps doing
an FBIO_BLANK ioctl, and is a more robust approach.

Other changes:

- Consolidated all tests and created an inlined helper function to check if
  the framebuffer console is inactive or not.
- fix unblanking if driver has no xxxfb_blank() hook.

I'm probably missing a few more things, but this patch should be generally
correct. Please apply after the entire patch series to avoid rejects.
Signed-off-by: Antonino Daplas <adaplas@pol.net>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

drivers/video/console/fbcon.c

@@ -200,6 +200,13 @@ static irqreturn_t fb_vbl_detect(int irq, void *dummy, struct pt_regs *fp)
 }
 #endif
 
+static inline int fbcon_is_inactive(struct vc_data *vc, struct fb_info *info)
+{
+	return (info->state != FBINFO_STATE_RUNNING ||
+		vt_cons[vc->vc_num]->vc_mode != KD_TEXT ||
+		(console_blanked && info->fbops->fb_blank));
+}
+
 static inline int get_color(struct vc_data *vc, struct fb_info *info,
 	      u16 c, int is_fg)
 {
@@ -252,9 +259,8 @@ static void fb_flashcursor(void *private)
 	if (ops->currcon != -1)
 		vc = vc_cons[ops->currcon].d;
 
-	if (info->state != FBINFO_STATE_RUNNING ||
-	    !vc || !CON_IS_VISIBLE(vc) ||
-	    vt_cons[vc->vc_num]->vc_mode != KD_TEXT ||
+	if (!vc || !CON_IS_VISIBLE(vc) ||
+	    fbcon_is_inactive(vc, info) ||
  	    registered_fb[con2fb_map[vc->vc_num]] != info)
 		return;
 	acquire_console_sem();
@@ -988,12 +994,7 @@ static void fbcon_clear(struct vc_data *vc, int sy, int sx, int height,
 	struct display *p = &fb_display[vc->vc_num];
 	u_int y_break;
 
-	if (!info->fbops->fb_blank && console_blanked)
-		return;
-	if (info->state != FBINFO_STATE_RUNNING)
-		return;
-
-	if (vt_cons[vc->vc_num]->vc_mode != KD_TEXT)
+	if (fbcon_is_inactive(vc, info))
 		return;
 
 	if (!height || !width)
@@ -1018,18 +1019,10 @@ static void fbcon_putcs(struct vc_data *vc, const unsigned short *s,
 	struct display *p = &fb_display[vc->vc_num];
 	struct fbcon_ops *ops = info->fbcon_par;
 
-	if (!info->fbops->fb_blank && console_blanked)
-		return;
-
-	if (info->state != FBINFO_STATE_RUNNING)
-		return;
-
-	if (vt_cons[vc->vc_num]->vc_mode != KD_TEXT)
-		return;
-
-	ops->putcs(vc, info, s, count, real_y(p, ypos), xpos,
-		   get_color(vc, info, scr_readw(s), 1),
-		   get_color(vc, info, scr_readw(s), 0));
+	if (!fbcon_is_inactive(vc, info))
+		ops->putcs(vc, info, s, count, real_y(p, ypos), xpos,
+			   get_color(vc, info, scr_readw(s), 1),
+			   get_color(vc, info, scr_readw(s), 0));
 }
 
 static void fbcon_putc(struct vc_data *vc, int c, int ypos, int xpos)
@@ -1045,7 +1038,8 @@ static void fbcon_clear_margins(struct vc_data *vc, int bottom_only)
 	struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
 	struct fbcon_ops *ops = info->fbcon_par;
 
-	ops->clear_margins(vc, info, bottom_only);
+	if (!fbcon_is_inactive(vc, info))
+		ops->clear_margins(vc, info, bottom_only);
 }
 
 static void fbcon_cursor(struct vc_data *vc, int mode)
@@ -1056,7 +1050,7 @@ static void fbcon_cursor(struct vc_data *vc, int mode)
 	int y = real_y(p, vc->vc_y);
  	int c = scr_readw((u16 *) vc->vc_pos);
 
-	if (vt_cons[vc->vc_num]->vc_mode != KD_TEXT)
+	if (fbcon_is_inactive(vc, info))
 		return;
 
 	ops->cursor_flash = 1;
@@ -1511,11 +1505,8 @@ static int fbcon_scroll(struct vc_data *vc, int t, int b, int dir,
 	struct fbcon_ops *ops = info->fbcon_par;
 	int scroll_partial = info->flags & FBINFO_PARTIAL_PAN_OK;
 
-	if (!info->fbops->fb_blank && console_blanked)
-		return 0;
-
-	if (!count || vt_cons[vc->vc_num]->vc_mode != KD_TEXT)
-		return 0;
+	if (fbcon_is_inactive(vc, info))
+		return -EINVAL;
 
 	fbcon_cursor(vc, CM_ERASE);
 
@@ -1704,10 +1695,7 @@ static void fbcon_bmove(struct vc_data *vc, int sy, int sx, int dy, int dx,
 	struct fb_info *info = registered_fb[con2fb_map[vc->vc_num]];
 	struct display *p = &fb_display[vc->vc_num];
 	
-	if (!info->fbops->fb_blank && console_blanked)
-		return;
-
-	if (vt_cons[vc->vc_num]->vc_mode != KD_TEXT)
+	if (fbcon_is_inactive(vc, info))
 		return;
 
 	if (!width || !height)
@@ -1999,8 +1987,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
 		return 0;
 	}
 
-	ops->cursor_flash = (!blank);
 	fbcon_cursor(vc, blank ? CM_ERASE : CM_DRAW);
+	ops->cursor_flash = (!blank);
 
 	if (!info->fbops->fb_blank) {
 		if (blank) {
@@ -2019,10 +2007,10 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
 			} else
 				fbcon_clear(vc, 0, 0, height, vc->vc_cols);
 			vc->vc_video_erase_char = oldc;
-		} else
+ 		} else if (!fbcon_is_inactive(vc, info))
 			update_screen(vc->vc_num);
 	} else if (vt_cons[vc->vc_num]->vc_mode == KD_TEXT)
-		retval = fb_blank(info, blank);
+ 		retval = info->fbops->fb_blank(blank, info);
 
 	return retval;
 }
@@ -2328,8 +2316,9 @@ static int fbcon_set_palette(struct vc_data *vc, unsigned char *table)
 	int i, j, k, depth;
 	u8 val;
 
-	if (!info->fbops->fb_blank && console_blanked)
+	if (fbcon_is_inactive(vc, info))
 		return -EINVAL;
+
 	depth = fb_get_color_depth(info);
 	if (depth > 3) {
 		for (i = j = 0; i < 16; i++) {

drivers/video/fbmem.c

@@ -743,33 +743,14 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
 int
 fb_blank(struct fb_info *info, int blank)
 {	
-	/* ??? Variable sized stack allocation.  */
-	struct fb_cmap cmap;
-	u16 *black = NULL;
-	int err = 0;
+	int err = -EINVAL;
 	
 	/* Workaround for broken X servers */
 	if (blank > VESA_POWERDOWN)
 		blank = VESA_POWERDOWN;
 
-	if (info->fbops->fb_blank && !info->fbops->fb_blank(blank, info))
-		return 0;
-
-	cmap = info->cmap;
-
-	if (blank) { 
-		black = kmalloc(sizeof(u16) * info->cmap.len, GFP_KERNEL);
-		if (black) {
-			memset(black, 0, info->cmap.len * sizeof(u16));
-			cmap.red = cmap.green = cmap.blue = black;
-			cmap.transp = info->cmap.transp ? black : NULL;
-			cmap.start = info->cmap.start;
-			cmap.len = info->cmap.len;
-		}
-	}
-
-	err = fb_set_cmap(&cmap, info);
-	kfree(black);
+	if (info->fbops->fb_blank)
+ 		err = info->fbops->fb_blank(blank, info);
 
 	return err;
 }