Re: [PATCH] split/refurbish vga_text.mod

[Robert Millan]

Committed.

On Sun, Nov 09, 2008 at 11:06:36PM +0100, Robert Millan wrote:
> 
> This patch refurbishes the current vga_text / console roles:
> 
>   - vga_text.c used only in coreboot; provides functions that on BIOS
>     are in kern/i386/pc/startup.S
> 
>   - console.c shared between coreboot and BIOS; confusing mixture of support
>     code for vga_text.c on coreboot, or console (both input and output), and
>     device registration on both coreboot (only output) and BIOS (both input
>     and output).
> 
> to:
> 
>   - vga_text.c: still provides functions that on BIOS are in
>     kern/i386/pc/startup.S, but now also handles device registration
>     using functions from vga_common.c (and from vga_text.c itself)
>     (this works either as a module or as built-in when on coreboot)
> 
>   - vga_common.c: support code for either vga_text.c or startup.S, no
>     device registration.
> 
>   - console.c: BIOS console.  handles device registration (both input and
>     output) using functions provided by vga_common.c and startup.S.
> 
> The inmediate results are:
> 
>   - We get rid of the GRUB_MACHINE_PCBIOS hack.
> 
>   - vga_text.mod can be loaded and used on non-coreboot platforms.
> 
> -- 
> Robert Millan
> 
>   The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>   how) you may access your data; but nobody's threatening your freedom: we
>   still allow you to remove your data and not access it at all."

> 2008-11-09  Robert Millan  <address@hidden>
> 
>       * conf/i386-pc.rmk (kernel_img_SOURCES): Add `term/i386/vga_common.c'.
>       * conf/i386.rmk (pkglib_MODULES): Add `vga_text.mod'.
>       (vga_text_mod_SOURCES, vga_text_mod_CFLAGS, vga_text_mod_LDFLAGS): New
>       variables.
>       * conf/i386-coreboot.rmk (kernel_elf_SOURCES): Replace
>       `term/i386/pc/console.c' with `term/i386/vga_common.c'.
> 
>       * kern/i386/coreboot/init.c (grub_machine_init): Replace call to
>       grub_console_init() with call to grub_vga_text_init().
>       (grub_machine_fini): Replace call to
>       grub_console_fini() with call to grub_vga_text_fini() and
>       grub_at_keyboard_fini().
> 
>       * include/grub/i386/pc/console.h: Include `<grub/term.h>'.
>       (grub_console_putchar, grub_console_getcharwidth, grub_console_getwh)
>       (grub_console_setcolorstate, grub_console_setcolor)
>       (grub_console_getcolor): New function prototypes.
> 
>       * term/i386/pc/vga_text.c: Include `<grub/dl.h>'.
>       (grub_vga_text_getxy, grub_vga_text_gotoxy, grub_vga_text_cls)
>       (grub_vga_text_setcursor): Static-ize.
>       (grub_vga_text_term): New structure.
>       (GRUB_MOD_INIT(vga_text), GRUB_MOD_FINI(vga_text)): New functions.
> 
>       * term/i386/pc/console.c: Remove `<grub/machine/machine.h>'.
>       (grub_console_cur_color, grub_console_standard_color)
>       (grub_console_normal_color, grub_console_highlight_color)
>       (map_char, grub_console_putchar, grub_console_getcharwidth)
>       (grub_console_getwh, grub_console_setcolorstate, grub_console_setcolor)
>       (grub_console_getcolor): Move from here ...
>       * term/i386/vga_common.c: ... to here (same function names).
> 
> Index: conf/i386-pc.rmk
> ===================================================================
> --- conf/i386-pc.rmk  (revision 1905)
> +++ conf/i386-pc.rmk  (working copy)
> @@ -49,7 +49,7 @@ kernel_img_SOURCES = kern/i386/pc/startu
>       kern/generic/rtc_get_time_ms.c \
>       kern/generic/millisleep.c \
>       kern/env.c \
> -     term/i386/pc/console.c \
> +     term/i386/pc/console.c term/i386/vga_common.c \
>       symlist.c
>  kernel_img_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h 
> elfload.h \
>       env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
> Index: conf/i386.rmk
> ===================================================================
> --- conf/i386.rmk     (revision 1905)
> +++ conf/i386.rmk     (working copy)
> @@ -9,3 +9,8 @@ pkglib_MODULES += at_keyboard.mod
>  at_keyboard_mod_SOURCES = term/i386/pc/at_keyboard.c
>  at_keyboard_mod_CFLAGS = $(COMMON_CFLAGS)
>  at_keyboard_mod_LDFLAGS = $(COMMON_LDFLAGS)
> +
> +pkglib_MODULES += vga_text.mod
> +vga_text_mod_SOURCES = term/i386/pc/vga_text.c term/i386/vga_common.c
> +vga_text_mod_CFLAGS = $(COMMON_CFLAGS)
> +vga_text_mod_LDFLAGS = $(COMMON_LDFLAGS)
> Index: conf/i386-coreboot.rmk
> ===================================================================
> --- conf/i386-coreboot.rmk    (revision 1905)
> +++ conf/i386-coreboot.rmk    (working copy)
> @@ -23,8 +23,8 @@ kernel_elf_SOURCES = kern/i386/coreboot/
>       kern/generic/rtc_get_time_ms.c \
>       kern/generic/millisleep.c \
>       kern/env.c \
> -     term/i386/pc/console.c \
> -     term/i386/pc/at_keyboard.c term/i386/pc/vga_text.c \
> +     term/i386/pc/vga_text.c term/i386/vga_common.c \
> +     term/i386/pc/at_keyboard.c \
>       symlist.c
>  kernel_elf_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h 
> elfload.h \
>       env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
> Index: kern/i386/coreboot/init.c
> ===================================================================
> --- kern/i386/coreboot/init.c (revision 1905)
> +++ kern/i386/coreboot/init.c (working copy)
> @@ -78,7 +78,7 @@ void
>  grub_machine_init (void)
>  {
>    /* Initialize the console as early as possible.  */
> -  grub_console_init ();
> +  grub_vga_text_init ();
>  
>    grub_lower_mem = GRUB_MEMORY_MACHINE_LOWER_USABLE;
>    grub_upper_mem = 0;
> @@ -151,7 +151,8 @@ grub_machine_set_prefix (void)
>  void
>  grub_machine_fini (void)
>  {
> -  grub_console_fini ();
> +  grub_at_keyboard_fini ();
> +  grub_vga_text_fini ();
>  }
>  
>  /* Return the end of the core image.  */
> Index: include/grub/i386/pc/console.h
> ===================================================================
> --- include/grub/i386/pc/console.h    (revision 1905)
> +++ include/grub/i386/pc/console.h    (working copy)
> @@ -1,6 +1,6 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2005,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2005,2007,2008  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -36,6 +36,7 @@
>  
>  #include <grub/types.h>
>  #include <grub/symbol.h>
> +#include <grub/term.h>
>  
>  /* These are global to share code between C and asm.  */
>  extern grub_uint8_t grub_console_cur_color;
> @@ -47,6 +48,14 @@ void grub_console_gotoxy (grub_uint8_t x
>  void grub_console_cls (void);
>  void grub_console_setcursor (int on);
>  
> +/* Provided by vga_common.c.  */
> +void grub_console_putchar (grub_uint32_t c);
> +grub_ssize_t grub_console_getcharwidth (grub_uint32_t c);
> +grub_uint16_t grub_console_getwh (void);
> +void grub_console_setcolorstate (grub_term_color_state state);
> +void grub_console_setcolor (grub_uint8_t normal_color, grub_uint8_t 
> highlight_color);
> +void grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t 
> *highlight_color);
> +
>  /* Initialize the console system.  */
>  void grub_console_init (void);
>  
> Index: term/i386/pc/vga_text.c
> ===================================================================
> --- term/i386/pc/vga_text.c   (revision 1905)
> +++ term/i386/pc/vga_text.c   (working copy)
> @@ -1,6 +1,6 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2007, 2008  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -16,6 +16,7 @@
>   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <grub/dl.h>
>  #include <grub/machine/console.h>
>  #include <grub/cpu/io.h>
>  #include <grub/types.h>
> @@ -106,31 +107,31 @@ grub_console_real_putchar (int c)
>    update_cursor ();
>  }
>  
> -grub_uint16_t
> -grub_console_getxy (void)
> +static grub_uint16_t
> +grub_vga_text_getxy (void)
>  {
>    return (grub_curr_x << 8) | grub_curr_y;
>  }
>  
> -void
> -grub_console_gotoxy (grub_uint8_t x, grub_uint8_t y)
> +static void
> +grub_vga_text_gotoxy (grub_uint8_t x, grub_uint8_t y)
>  {
>    grub_curr_x = x;
>    grub_curr_y = y;
>    update_cursor ();
>  }
>  
> -void
> -grub_console_cls (void)
> +static void
> +grub_vga_text_cls (void)
>  {
>    int i;
>    for (i = 0; i < ROWS * COLS; i++)
>      ((short *) VGA_TEXT_SCREEN)[i] = ' ' | (grub_console_cur_color << 8);
> -  grub_console_gotoxy (0, 0);
> +  grub_vga_text_gotoxy (0, 0);
>  }
>  
> -void
> -grub_console_setcursor (int on)
> +static void
> +grub_vga_text_setcursor (int on)
>  {
>    grub_uint8_t old;
>    grub_outb (CRTC_CURSOR, CRTC_ADDR_PORT);
> @@ -140,3 +141,30 @@ grub_console_setcursor (int on)
>    else
>      grub_outb (old | CRTC_CURSOR_DISABLE, CRTC_DATA_PORT);
>  }
> +
> +static struct grub_term_output grub_vga_text_term =
> +  {
> +    .name = "vga_text",
> +    .init = grub_vga_text_cls,
> +    .fini = grub_vga_text_cls,
> +    .putchar = grub_console_putchar,
> +    .getcharwidth = grub_console_getcharwidth,
> +    .getwh = grub_console_getwh,
> +    .getxy = grub_vga_text_getxy,
> +    .gotoxy = grub_vga_text_gotoxy,
> +    .cls = grub_vga_text_cls,
> +    .setcolorstate = grub_console_setcolorstate,
> +    .setcolor = grub_console_setcolor,
> +    .getcolor = grub_console_getcolor,
> +    .setcursor = grub_vga_text_setcursor,
> +  };
> +
> +GRUB_MOD_INIT(vga_text)
> +{
> +  grub_term_register_output (&grub_vga_text_term);
> +}
> +
> +GRUB_MOD_FINI(vga_text)
> +{
> +  grub_term_unregister_output (&grub_vga_text_term);
> +}
> Index: term/i386/pc/console.c
> ===================================================================
> --- term/i386/pc/console.c    (revision 1905)
> +++ term/i386/pc/console.c    (working copy)
> @@ -16,125 +16,16 @@
>   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <grub/machine/machine.h>
>  #include <grub/machine/console.h>
>  #include <grub/term.h>
>  #include <grub/types.h>
>  
> -grub_uint8_t grub_console_cur_color = 0x7;
> -static grub_uint8_t grub_console_standard_color = 0x7;
> -static grub_uint8_t grub_console_normal_color = 0x7;
> -static grub_uint8_t grub_console_highlight_color = 0x70;
> -
> -static grub_uint32_t
> -map_char (grub_uint32_t c)
> -{
> -  if (c > 0x7f)
> -    {
> -      /* Map some unicode characters to the VGA font, if possible.  */
> -      switch (c)
> -     {
> -     case 0x2190:    /* left arrow */
> -       c = 0x1b;
> -       break;
> -     case 0x2191:    /* up arrow */
> -       c = 0x18;
> -       break;
> -     case 0x2192:    /* right arrow */
> -       c = 0x1a;
> -       break;
> -     case 0x2193:    /* down arrow */
> -       c = 0x19;
> -       break;
> -     case 0x2501:    /* horizontal line */
> -       c = 0xc4;
> -       break;
> -     case 0x2503:    /* vertical line */
> -       c = 0xb3;
> -       break;
> -     case 0x250F:    /* upper-left corner */
> -       c = 0xda;
> -       break;
> -     case 0x2513:    /* upper-right corner */
> -       c = 0xbf;
> -       break;
> -     case 0x2517:    /* lower-left corner */
> -       c = 0xc0;
> -       break;
> -     case 0x251B:    /* lower-right corner */
> -       c = 0xd9;
> -       break;
> -
> -     default:
> -       c = '?';
> -       break;
> -     }
> -    }
> -
> -  return c;
> -}
> -
> -static void
> -grub_console_putchar (grub_uint32_t c)
> -{
> -  grub_console_real_putchar (map_char (c));
> -}
> -
> -static grub_ssize_t
> -grub_console_getcharwidth (grub_uint32_t c __attribute__ ((unused)))
> -{
> -  /* For now, every printable character has the width 1.  */
> -  return 1;
> -}
> -
> -static grub_uint16_t
> -grub_console_getwh (void)
> -{
> -  return (80 << 8) | 25;
> -}
> -
> -static void
> -grub_console_setcolorstate (grub_term_color_state state)
> -{
> -  switch (state) {
> -    case GRUB_TERM_COLOR_STANDARD:
> -      grub_console_cur_color = grub_console_standard_color;
> -      break;
> -    case GRUB_TERM_COLOR_NORMAL:
> -      grub_console_cur_color = grub_console_normal_color;
> -      break;
> -    case GRUB_TERM_COLOR_HIGHLIGHT:
> -      grub_console_cur_color = grub_console_highlight_color;
> -      break;
> -    default:
> -      break;
> -  }
> -}
> -
> -static void
> -grub_console_setcolor (grub_uint8_t normal_color, grub_uint8_t 
> highlight_color)
> -{
> -  grub_console_normal_color = normal_color;
> -  grub_console_highlight_color = highlight_color;
> -}
> -
> -static void
> -grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t 
> *highlight_color)
> -{
> -  *normal_color = grub_console_normal_color;
> -  *highlight_color = grub_console_highlight_color;
> -}
> -
> -/* On non-BIOS platforms, console.c is used in combination with vga_text.c
> -   (only to handle output).  */
> -#ifdef GRUB_MACHINE_PCBIOS
>  static struct grub_term_input grub_console_term_input =
>    {
>      .name = "console",
>      .checkkey = grub_console_checkkey,
>      .getkey = grub_console_getkey,
>    };
> -#endif
>  
>  static struct grub_term_output grub_console_term_output =
>    {
> @@ -157,19 +48,14 @@ grub_console_init (void)
>  {
>    grub_term_register_output (&grub_console_term_output);
>    grub_term_set_current_output (&grub_console_term_output);
> -#ifdef GRUB_MACHINE_PCBIOS
>    grub_term_register_input (&grub_console_term_input);
>    grub_term_set_current_input (&grub_console_term_input);
> -#endif
>  }
>  
>  void
>  grub_console_fini (void)
>  {
>    grub_term_set_current_output (&grub_console_term_output);
> -#ifdef GRUB_MACHINE_PCBIOS
> -  grub_term_set_current_input (&grub_console_term_input);
>    grub_term_unregister_input (&grub_console_term_input);
> -#endif
>    grub_term_unregister_output (&grub_console_term_output);
>  }
> Index: term/i386/vga_common.c
> ===================================================================
> --- term/i386/vga_common.c    (revision 1905)
> +++ term/i386/vga_common.c    (working copy)
> @@ -16,7 +16,6 @@
>   *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <grub/machine/machine.h>
>  #include <grub/machine/console.h>
>  #include <grub/term.h>
>  #include <grub/types.h>
> @@ -74,26 +73,26 @@ map_char (grub_uint32_t c)
>    return c;
>  }
>  
> -static void
> +void
>  grub_console_putchar (grub_uint32_t c)
>  {
>    grub_console_real_putchar (map_char (c));
>  }
>  
> -static grub_ssize_t
> +grub_ssize_t
>  grub_console_getcharwidth (grub_uint32_t c __attribute__ ((unused)))
>  {
>    /* For now, every printable character has the width 1.  */
>    return 1;
>  }
>  
> -static grub_uint16_t
> +grub_uint16_t
>  grub_console_getwh (void)
>  {
>    return (80 << 8) | 25;
>  }
>  
> -static void
> +void
>  grub_console_setcolorstate (grub_term_color_state state)
>  {
>    switch (state) {
> @@ -111,65 +110,16 @@ grub_console_setcolorstate (grub_term_co
>    }
>  }
>  
> -static void
> +void
>  grub_console_setcolor (grub_uint8_t normal_color, grub_uint8_t 
> highlight_color)
>  {
>    grub_console_normal_color = normal_color;
>    grub_console_highlight_color = highlight_color;
>  }
>  
> -static void
> +void
>  grub_console_getcolor (grub_uint8_t *normal_color, grub_uint8_t 
> *highlight_color)
>  {
>    *normal_color = grub_console_normal_color;
>    *highlight_color = grub_console_highlight_color;
>  }
> -
> -/* On non-BIOS platforms, console.c is used in combination with vga_text.c
> -   (only to handle output).  */
> -#ifdef GRUB_MACHINE_PCBIOS
> -static struct grub_term_input grub_console_term_input =
> -  {
> -    .name = "console",
> -    .checkkey = grub_console_checkkey,
> -    .getkey = grub_console_getkey,
> -  };
> -#endif
> -
> -static struct grub_term_output grub_console_term_output =
> -  {
> -    .name = "console",
> -    .putchar = grub_console_putchar,
> -    .getcharwidth = grub_console_getcharwidth,
> -    .getwh = grub_console_getwh,
> -    .getxy = grub_console_getxy,
> -    .gotoxy = grub_console_gotoxy,
> -    .cls = grub_console_cls,
> -    .setcolorstate = grub_console_setcolorstate,
> -    .setcolor = grub_console_setcolor,
> -    .getcolor = grub_console_getcolor,
> -    .setcursor = grub_console_setcursor,
> -    .flags = 0,
> -  };
> -
> -void
> -grub_console_init (void)
> -{
> -  grub_term_register_output (&grub_console_term_output);
> -  grub_term_set_current_output (&grub_console_term_output);
> -#ifdef GRUB_MACHINE_PCBIOS
> -  grub_term_register_input (&grub_console_term_input);
> -  grub_term_set_current_input (&grub_console_term_input);
> -#endif
> -}
> -
> -void
> -grub_console_fini (void)
> -{
> -  grub_term_set_current_output (&grub_console_term_output);
> -#ifdef GRUB_MACHINE_PCBIOS
> -  grub_term_set_current_input (&grub_console_term_input);
> -  grub_term_unregister_input (&grub_console_term_input);
> -#endif
> -  grub_term_unregister_output (&grub_console_term_output);
> -}

> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."