patch fixing background next to penguin logo in monochrome

Hi all,

I have developed a simple workaround for a problem existing in
framebuffer when it interprets VGA attribute bits in monochrome mode.
Instead of empty chars we get underlines - you can see what I mean at
http://joy.sophics.cz/horizlines.jpg - the image shows the background
where the penguin logo will appear during linux kernel boot up. The logo
then looks like incorrectly drawn (due to the horizontal lines) and
people are complaining or reporting this as a sign of a kernel problems
with their hardware setup or something.

When Geert told me in linux-m68k that this issue was a years known
problem and a quite complicated thing to fix I tried to come up with a
simpler solution and here it is: the patch attached below simply erases
the background next to the penguin(s) while it paints them. The result
is clear screen with just the penguin logo. 

The patch has been tested on my Atari Falcon both in normal and inverse
monochrome mode. It's so simple that I hope it gets accepted in the
kernel.

Please CC: me in replies as I am not subscribed to this list.

Thanks.

Petr

Index: drivers/video/fbcon.c
===================================================================
RCS file: /home/linux-m68k/cvsroot/linux/drivers/video/Attic/fbcon.c,v
retrieving revision 1.1.1.2.2.6
diff -u -r1.1.1.2.2.6 fbcon.c

--- drivers/video/fbcon.c	26 Aug 2003 03:13:09 -0000	1.1.1.2.2.6
+++ drivers/video/fbcon.c	11 Aug 2004 19:50:16 -0000

@@ -2433,12 +2433,16 @@
 		    dst = fb + (y1%4)*8192 + (y1>>2)*line + x/8;
 		else
 		    dst = fb + y1*line + x/8;
-		for( x1 = 0; x1 < LOGO_LINE; ++x1 ) {
+		for( x1 = 0; x1 < (p->var.xres-x)/8; ++x1 ) {
+		    char c;
+		    if (x1 < LOGO_LINE)
+			c = *src++ ^ inverse; /* logo data */
+		    else
+			c = ~inverse; /* erase background to end of line */
 #ifndef CONFIG_HP300
-		    fb_writeb(*src++ ^ inverse, dst++);
+		    fb_writeb(c, dst++);
 #else /* CONFIG_HP300 */
 		    /* hack hack -- make it work with topcat in pseudomono mode */
-		    char c = *src++ ^ inverse;
 		    fb_writeb((c >> 7) & 1, dst++);
 		    fb_writeb((c >> 6) & 1, dst++);
 		    fb_writeb((c >> 5) & 1, dst++);




-------------------------------------------------------

SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Thursday 12 August 2004 03:56, Petr Stehlik wrote:

Hi all,

I have developed a simple workaround for a problem existing in
framebuffer when it interprets VGA attribute bits in monochrome mode.
Instead of empty chars we get underlines - you can see what I mean at
http://joy.sophics.cz/horizlines.jpg - the image shows the background
where the penguin logo will appear during linux kernel boot up. The logo
then looks like incorrectly drawn (due to the horizontal lines) and
people are complaining or reporting this as a sign of a kernel problems
with their hardware setup or something.

During the take_over_console() part in drivers/char/console.c, the
the framebuffer is initialized first before the character attributes are
updated. So, during the framebuffer initialization, when the space for
the logo is created, the space is erased using vc->vc_video_erase_char.  And
because the attributes are not updated yet, the vc_video_erase_char has still
the attributes of the initial console. If the initial console happens to be
color capable (vc->vc_can_do_color != 0), then the attributes will be 
misinterpreted when the monochrome console redraws the space for the logo.
You get the underline.

Anyway, the simplest solution is to use a vc->vc_video_erase_char with the
underline attribute removed.  Check drivers/video/fbcon.c and look at the
function fbcon_setup. Note, the logo space is erased using the scr_memsetw
function with conp->vc_video_erase_char. Just replace them with
(conp->vc_video_erase_char & ~0x400) if conp->can_do_color is false. (0x400
is the underline attribute). 

I haven't tried this in 2.4, but a similar fix is already present in the latest 2.6 rc
and mm tree. (I still need to add the bold attribute support though).

Tony




-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

V Ne, 12. 09. 2004 v 00:16, Antonino A. Daplas pí¹e:

On Thursday 12 August 2004 03:56, Petr Stehlik wrote:

quoted

I have developed a simple workaround for a problem existing in
framebuffer when it interprets VGA attribute bits in monochrome mode.
Instead of empty chars we get underlines

Anyway, the simplest solution is to use a vc->vc_video_erase_char with the
underline attribute removed.  Check drivers/video/fbcon.c and look at the
function fbcon_setup. Note, the logo space is erased using the scr_memsetw
function with conp->vc_video_erase_char. Just replace them with
(conp->vc_video_erase_char & ~0x400) if conp->can_do_color is false. (0x400
is the underline attribute). 

Good idea. Tested, works. Included below.

Thanks.

Petr

Index: drivers/video/fbcon.c
===================================================================
RCS file: /home/linux-m68k/cvsroot/linux/drivers/video/Attic/fbcon.c,v
retrieving revision 1.1.1.2.2.6
diff -u -r1.1.1.2.2.6 fbcon.c

--- drivers/video/fbcon.c	26 Aug 2003 03:13:09 -0000	1.1.1.2.2.6
+++ drivers/video/fbcon.c	12 Aug 2004 11:59:22 -0000

@@ -692,9 +692,13 @@
     		conp->vc_pos += logo_lines * conp->vc_size_row;
     	    }
     	}
-    	scr_memsetw((unsigned short *)conp->vc_origin,
-		    conp->vc_video_erase_char, 
-		    conp->vc_size_row * logo_lines);
+	{
+		int erase_char = conp->vc_video_erase_char;
+		if (! conp->vc_can_do_color)
+			erase_char &= ~0x400; /* disable underline */
+		scr_memsetw((unsigned short *)conp->vc_origin,
+			erase_char, conp->vc_size_row * logo_lines);
+	}
     }
     
     /*




-------------------------------------------------------

SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Thu, 12 Aug 2004, Petr Stehlik wrote:

V Ne, 12. 09. 2004 v 00:16, Antonino A. Daplas pí¨e:

quoted

On Thursday 12 August 2004 03:56, Petr Stehlik wrote:

quoted

I have developed a simple workaround for a problem existing in
framebuffer when it interprets VGA attribute bits in monochrome mode.
Instead of empty chars we get underlines

quoted

Anyway, the simplest solution is to use a vc->vc_video_erase_char with the
underline attribute removed.  Check drivers/video/fbcon.c and look at the
function fbcon_setup. Note, the logo space is erased using the scr_memsetw
function with conp->vc_video_erase_char. Just replace them with
(conp->vc_video_erase_char & ~0x400) if conp->can_do_color is false. (0x400
is the underline attribute).

Good idea. Tested, works. Included below.

Thanks a lot! Works fine on Amiga as well!

BTW, I checked in a slightly modified version in Linux/m68k CVS
(http://linux-m68k-cvs.ubb.ca/~geert/linux-m68k-2.4.x-merging/162-logo.diff).

Gr{oetje,eeting}s,

						Geert

P.S. There are still problems when switching from color to monochrome
     (underlines appear everywhere) or vice versa (text becomes blue), but I
     cannot expect you to fix all old bugs in one week ;-)
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Friday 13 August 2004 04:01, Geert Uytterhoeven wrote:

P.S. There are still problems when switching from color to monochrome
     (underlines appear everywhere) or vice versa (text becomes blue), but
I cannot expect you to fix all old bugs in one week ;-)

This one is a little difficult since there is currently no method to convert
monochrome attributes to color and vice versa.  The best one can do is to
clear all attributes when switching from mono<->color.

When switching, one will get plain text only.  This is  probably better than
having text with incorrect attributes.

Attached patch is completely untested, but it's the same one in 2.6.

Tony

diff -uprN linux-2.4-orig/drivers/char/console.c linux-2.4/drivers/char/console.c

--- linux-2.4-orig/drivers/char/console.c	2004-09-13 06:09:05.000000000 +0800
+++ linux-2.4/drivers/char/console.c	2004-09-13 06:13:40.072568112 +0800

@@ -693,6 +693,17 @@ static inline void save_screen(int currc
  *	Redrawing of screen
  */
 
+static void clear_buffer_attributes(int currcons)
+{
+	unsigned short *p = (unsigned short *) origin;
+	int count = screenbuf_size/2;
+	int mask = hi_font_mask | 0xff;
+
+	for (; count > 0; count--, p++) {
+		scr_writew((scr_readw(p)&mask) | (video_erase_char&~mask), p);
+	}
+}
+
 void redraw_screen(int new_console, int is_switch)
 {
 	int redraw = 1;

@@ -728,9 +739,21 @@ void redraw_screen(int new_console, int 
 
 	if (redraw) {
 		int update;
+		int old_was_color = vc_cons[currcons].d->vc_can_do_color;
+
 		set_origin(currcons);
 		update = sw->con_switch(vc_cons[currcons].d);
 		set_palette(currcons);
+		/*
+		 * If console changed from mono<->color, the best we can do
+		 * is to clear the buffer attributes. As it currently stands,
+		 * rebuilding new attributes from the old buffer is not doable
+		 * without overly complex code.
+		 */
+		if (old_was_color != vc_cons[currcons].d->vc_can_do_color) {
+			update_attr(currcons);
+			clear_buffer_attributes(currcons);
+		}
 		if (update && vcmode != KD_GRAPHICS)
 			do_update_region(currcons, origin, screenbuf_size/2);
 	}

@@ -2750,18 +2773,6 @@ void __init con_init(void)
 }
 
 #ifndef VT_SINGLE_DRIVER
-
-static void clear_buffer_attributes(int currcons)
-{
-	unsigned short *p = (unsigned short *) origin;
-	int count = screenbuf_size/2;
-	int mask = hi_font_mask | 0xff;
-
-	for (; count > 0; count--, p++) {
-		scr_writew((scr_readw(p)&mask) | (video_erase_char&~mask), p);
-	}
-}
-
 /*
  *	If we support more console drivers, this function is used
  *	when a driver wants to take over some existing consoles







-------------------------------------------------------

SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Monday 13 September 2004 06:25, Antonino A. Daplas wrote:

On Friday 13 August 2004 04:01, Geert Uytterhoeven wrote:

quoted

P.S. There are still problems when switching from color to monochrome
     (underlines appear everywhere) or vice versa (text becomes blue),
but I cannot expect you to fix all old bugs in one week ;-)

This one is a little difficult since there is currently no method to
convert monochrome attributes to color and vice versa.  The best one can do
is to clear all attributes when switching from mono<->color.

When switching, one will get plain text only.  This is  probably better
than having text with incorrect attributes.

Attached patch is completely untested, but it's the same one in 2.6.

I think the conp->vc_can_do_color field also needs to be updated during fbcon_switch.

Tony
---

diff -uprN linux-2.4-orig/drivers/video/fbcon.c linux-2.4/drivers/video/fbcon.c

--- linux-2.4-orig/drivers/video/fbcon.c	2004-09-13 07:40:18.194333488 +0800
+++ linux-2.4/drivers/video/fbcon.c	2004-09-13 07:41:52.302026944 +0800

@@ -1930,6 +1930,7 @@ static int fbcon_switch(struct vc_data *
 	(*info->switch_con)(unit, info);
     if (p->dispsw->clear_margins && vt_cons[unit]->vc_mode == KD_TEXT)
 	p->dispsw->clear_margins(conp, p, 0);
+    conp->vc_can_do_color = p->var.bits_per_pixel != 1;
     if (logo_shown == -2) {
 	/* G.S.: Display a line above the Boot Logo to state what
 	* version of the kernel we are booting.




-------------------------------------------------------

SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Mon, 13 Sep 2004, Antonino A. Daplas wrote:

I don't think it's needed: I saw no artifacts when switching between a
monochrome and a color VC or vice versa.

When looking at the source, conp->vc_can_do_color is initialized in
fbcon_setup(), which is called from fbcon_changevar(), which is called by the
fbdev if the resolution or color depth was changed.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Friday 13 August 2004 16:05, Geert Uytterhoeven wrote:

quoted

diff -uprN linux-2.4-orig/drivers/video/fbcon.c
linux-2.4/drivers/video/fbcon.c ---
linux-2.4-orig/drivers/video/fbcon.c	2004-09-13 07:40:18.194333488 +0800

+++ linux-2.4/drivers/video/fbcon.c	2004-09-13 07:41:52.302026944 +0800

@@ -1930,6 +1930,7 @@ static int fbcon_switch(struct vc_data *
 	(*info->switch_con)(unit, info);
     if (p->dispsw->clear_margins && vt_cons[unit]->vc_mode == KD_TEXT)
 	p->dispsw->clear_margins(conp, p, 0);
+    conp->vc_can_do_color = p->var.bits_per_pixel != 1;
     if (logo_shown == -2) {
 	/* G.S.: Display a line above the Boot Logo to state what
 	* version of the kernel we are booting.

I don't think it's needed: I saw no artifacts when switching between a
monochrome and a color VC or vice versa.

When looking at the source, conp->vc_can_do_color is initialized in
fbcon_setup(), which is called from fbcon_changevar(), which is called by
the fbdev if the resolution or color depth was changed.

Okay.  I wasn't too sure of that since I'm not very familiar with the 2.4
code.

Tony




-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Mon, 13 Sep 2004, Antonino A. Daplas wrote:

On Friday 13 August 2004 04:01, Geert Uytterhoeven wrote:

quoted

P.S. There are still problems when switching from color to monochrome
     (underlines appear everywhere) or vice versa (text becomes blue), but
I cannot expect you to fix all old bugs in one week ;-)

This one is a little difficult since there is currently no method to convert
monochrome attributes to color and vice versa.  The best one can do is to
clear all attributes when switching from mono<->color.

When switching, one will get plain text only.  This is  probably better than
having text with incorrect attributes.

Attached patch is completely untested, but it's the same one in 2.6.

Doesn't help, still the same behavior :-(

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Sunday 15 August 2004 20:25, Geert Uytterhoeven wrote:

quoted

This one is a little difficult since there is currently no method to
convert monochrome attributes to color and vice versa.  The best one can
do is to clear all attributes when switching from mono<->color.

When switching, one will get plain text only.  This is  probably better
than having text with incorrect attributes.

Attached patch is completely untested, but it's the same one in 2.6.

Doesn't help, still the same behavior :-(

I looked at the code, and the flow seems to be like this:

fbcon_switch->fb_set_var->changevar->fbcon_setup->update_screen/redraw_screen

However, fbcon_setup() updates conp->vc_can_do_color, so by the time
the redraw_screen function in console.c checks for the vc_can_do_color field,
it always returns equal.

So instead of clearing the buffer attributes in redraw_screen(), we do it 
instead in fbcon_setup().

Try the patch below.  You might have to apply them manually as I'm using
Suse-9.0 linux kernel, which has the bootsplash code.  Also, all the previous
patches need to be reversed.

Tony

PS:  If we can change the clear_buffer_attribute to rebuild attributes from
the old buffer instead of just clearing the attributes, that would be perfect.
Is there an easy way to do that?
---

diff -uprN linux-2.4-orig/drivers/video/fbcon.c linux-2.4/drivers/video/fbcon.c

--- linux-2.4-orig/drivers/video/fbcon.c	2004-09-16 10:50:58.093218288 +0800
+++ linux-2.4/drivers/video/fbcon.c	2004-09-16 10:53:12.826735680 +0800

@@ -618,12 +618,25 @@ static void fbcon_font_widths(struct dis
 
 #define fontwidthvalid(p,w) ((p)->dispsw->fontwidthmask & FONTWIDTH(w))
 
+static void clear_buffer_attributes(struct vc_data *conp)
+{
+	unsigned short *p = (unsigned short *) conp->vc_origin;
+	int count = conp->vc_screenbuf_size/2;
+	int mask = conp->vc_hi_font_mask | 0xff;
+	int erase_char = conp->vc_video_erase_char;
+
+	for (; count > 0; count--, p++) {
+		scr_writew((scr_readw(p)&mask) | (erase_char&~mask), p);
+	}
+}
+
 static void fbcon_setup(int con, int init, int logo)
 {
     struct display *p = &fb_display[con];
     struct vc_data *conp = p->conp;
     int nr_rows, nr_cols;
     int old_rows, old_cols;
+    int old_was_color = conp->vc_can_do_color;
     unsigned short *save = NULL, *r, *q;
     int i, charcnt = 256;
     struct fbcon_font_desc *font;

@@ -803,6 +816,8 @@ static void fbcon_setup(int con, int ini
     p->bgcol = 0;
 
     if (!init) {
+	if (old_was_color != conp->vc_can_do_color)
+	    clear_buffer_attributes(conp);
 	if (conp->vc_cols != nr_cols || conp->vc_rows != nr_rows)
 	    vc_resize_con(nr_rows, nr_cols, con);
 	else if (CON_IS_VISIBLE(conp) &&
 




-------------------------------------------------------

SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

On Thu, 16 Sep 2004, Antonino A. Daplas wrote:

On Sunday 15 August 2004 20:25, Geert Uytterhoeven wrote:

quoted

quoted

This one is a little difficult since there is currently no method to
convert monochrome attributes to color and vice versa.  The best one can
do is to clear all attributes when switching from mono<->color.

When switching, one will get plain text only.  This is  probably better
than having text with incorrect attributes.

Attached patch is completely untested, but it's the same one in 2.6.

Doesn't help, still the same behavior :-(

I looked at the code, and the flow seems to be like this:

fbcon_switch->fb_set_var->changevar->fbcon_setup->update_screen/redraw_screen

However, fbcon_setup() updates conp->vc_can_do_color, so by the time
the redraw_screen function in console.c checks for the vc_can_do_color field,
it always returns equal.

So instead of clearing the buffer attributes in redraw_screen(), we do it
instead in fbcon_setup().

Try the patch below.  You might have to apply them manually as I'm using
Suse-9.0 linux kernel, which has the bootsplash code.  Also, all the previous
patches need to be reversed.

Applied fine, but still the same result.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285