Re: [PATCH] Fixed ieee1275 console

[Robert Millan <rmh@aybabtu.com>]

On Mon, Oct 01, 2007 at 08:14:16PM +0200, Marcin Kurek wrote:
> diff -urN grub2.org/include/grub/term.h grub2/include/grub/term.h
> --- grub2.org/include/grub/term.h	2007-07-22 01:32:22.000000000 +0200
> +++ grub2/include/grub/term.h	2007-10-01 19:20:49.734330521 +0200
> @@ -51,10 +51,13 @@
>  #define GRUB_TERM_NO_EDIT	(1 << 1)
>  /* Set when the terminal cannot do fancy things.  */
>  #define GRUB_TERM_DUMB		(1 << 2)
> +/* Set to use ascii menu borders.  */
> +#define GRUB_TERM_SIMPLE_MENU	(1 << 3) 
> +/* Set to use cp437 menu borders.  */
> +#define GRUB_TERM_CP437_MENU	(1 << 4)
>  /* Set when the terminal needs to be initialized.  */
>  #define GRUB_TERM_NEED_INIT	(1 << 16)

Is this a bitmask?  In this case, looks like you aren't using the right
numbers.  Now GRUB_TERM_SIMPLE_MENU == GRUB_TERM_DUMB | GRUB_TERM_NO_EDIT
OTOH if that's what you intended, I think it's better to build one macro
using the others.

> diff -urN grub2.org/term/i386/pc/vesafb.c grub2/term/i386/pc/vesafb.c
> --- grub2.org/term/i386/pc/vesafb.c	2007-07-22 01:32:31.000000000 +0200
> +++ grub2/term/i386/pc/vesafb.c	2007-10-01 19:37:38.160330521 +0200
> @@ -214,45 +214,7 @@
>  			       unsigned *width)
>  {
>    if (code > 0x7f)
> -    {
> -      /* Map some unicode characters to the VGA font, if possible.  */
> -      switch (code)
> -	{
> -	case 0x2190:	/* left arrow */
> -	  code = 0x1b;
> -	  break;
> -	case 0x2191:	/* up arrow */
> -	  code = 0x18;
> -	  break;
> -	case 0x2192:	/* right arrow */
> -	  code = 0x1a;
> -	  break;
> -	case 0x2193:	/* down arrow */
> -	  code = 0x19;
> -	  break;
> -	case 0x2501:	/* horizontal line */
> -	  code = 0xc4;
> -	  break;
> -	case 0x2503:	/* vertical line */
> -	  code = 0xb3;
> -	  break;
> -	case 0x250F:	/* upper-left corner */
> -	  code = 0xda;
> -	  break;
> -	case 0x2513:	/* upper-right corner */
> -	  code = 0xbf;
> -	  break;
> -	case 0x2517:	/* lower-left corner */
> -	  code = 0xc0;
> -	  break;
> -	case 0x251B:	/* lower-right corner */
> -	  code = 0xd9;
> -	  break;
> -
> -	default:
> -	  return grub_font_get_glyph (code, bitmap, width);
> -	}
> -    }
> +    return grub_font_get_glyph (code, bitmap, width);
>  
>    if (bitmap)
>      grub_memcpy (bitmap,
> @@ -601,7 +563,7 @@
>      .setcolorstate = grub_virtual_screen_setcolorstate,
>      .setcolor = grub_virtual_screen_setcolor,
>      .setcursor = grub_vesafb_setcursor,
> -    .flags = 0,
> +    .flags = GRUB_TERM_CP437_MENU,
>      .next = 0
>    };
>  
> diff -urN grub2.org/term/i386/pc/vga.c grub2/term/i386/pc/vga.c
> --- grub2.org/term/i386/pc/vga.c	2007-07-22 01:32:31.000000000 +0200
> +++ grub2/term/i386/pc/vga.c	2007-10-01 19:38:10.883330521 +0200
> @@ -198,45 +198,7 @@
>  get_vga_glyph (grub_uint32_t code, unsigned char bitmap[32], unsigned *width)
>  {
>    if (code > 0x7f)
> -    {
> -      /* Map some unicode characters to the VGA font, if possible.  */
> -      switch (code)
> -	{
> -	case 0x2190:	/* left arrow */
> -	  code = 0x1b;
> -	  break;
> -	case 0x2191:	/* up arrow */
> -	  code = 0x18;
> -	  break;
> -	case 0x2192:	/* right arrow */
> -	  code = 0x1a;
> -	  break;
> -	case 0x2193:	/* down arrow */
> -	  code = 0x19;
> -	  break;
> -	case 0x2501:	/* horizontal line */
> -	  code = 0xc4;
> -	  break;
> -	case 0x2503:	/* vertical line */
> -	  code = 0xb3;
> -	  break;
> -	case 0x250F:	/* upper-left corner */
> -	  code = 0xda;
> -	  break;
> -	case 0x2513:	/* upper-right corner */
> -	  code = 0xbf;
> -	  break;
> -	case 0x2517:	/* lower-left corner */
> -	  code = 0xc0;
> -	  break;
> -	case 0x251B:	/* lower-right corner */
> -	  code = 0xd9;
> -	  break;
> -
> -	default:
> -	  return grub_font_get_glyph (code, bitmap, width);
> -	}
> -    }
> +    return grub_font_get_glyph (code, bitmap, width);

This part doesn't look right.  vesa/vga is a whole different history.  At the
least vesa *does* support unicode.

> diff -urN grub2.org/term/ieee1275/ofconsole.c grub2/term/ieee1275/ofconsole.c
> --- grub2.org/term/ieee1275/ofconsole.c	2007-07-22 11:05:11.000000000 +0200
> +++ grub2/term/ieee1275/ofconsole.c	2007-10-01 19:01:49.820330521 +0200
> @@ -75,6 +75,7 @@
>  grub_ofconsole_putchar (grub_uint32_t c)
>  {
>    char chr = c;
> +  
>    if (c == '\n')
>      {
>        grub_curr_y++;
> @@ -86,6 +87,7 @@
>        if (grub_curr_x > grub_ofconsole_width)
>  	grub_putcode ('\n');
>      }
> +
>    grub_ieee1275_write (stdout_ihandle, &chr, 1, 0);
>  }

Please try to avoid cosmetical changes.

> diff -urN grub2.org/util/console.c grub2/util/console.c
> --- grub2.org/util/console.c	2007-07-22 01:32:31.000000000 +0200
> +++ grub2/util/console.c	2007-10-01 19:01:49.821330521 +0200
> @@ -44,47 +44,6 @@
>  static void
>  grub_ncurses_putchar (grub_uint32_t c)
>  {

Have you verified that grub-emu still builds / works?

> diff -urN grub2.org/term/ieee1275/ofconsole.c grub2/term/ieee1275/ofconsole.c
> --- grub2.org/term/ieee1275/ofconsole.c	2007-10-01 18:02:39.000000000 +0200
> +++ grub2/term/ieee1275/ofconsole.c	2007-10-01 18:06:33.000000000 +0200
> @@ -135,43 +135,56 @@
>  
>    grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
>  
> -  if (actual > 0 && c == '\e')
> +  if (actual > 0)
>      {
> -      grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
> -      if (actual <= 0)
> -	{
> -	  *key = '\e';
> -	  return 1;
> -	}
> +      if (c != '\e')
> +      {
> +        switch(c)
> +        {
> +          case 127:
> +            /* Backspace */
> +            c = '\b';
> +            break;

What is 127 normally?  For firmware bugs, we have a flag mechanism so that it
only affects the buggy firmware.  Check my latest commit to
kern/powerpc/ieee1275/cmain.c.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)