Skip to contentSkip to conversation

FIXME: These should be dynamically obtained from a terminal.

PATCH Superseded Vincent Pelletier 4 participants 14 messages Jul 3, 2005 – Aug 9, 2005

lore
Share & tools
lore

Vincent Pelletier

author
#0 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I'm trying to fix that thing.
I'm only working on the i386 part, because I have no ppc.

Here is how I plan to do it (almost done, but I have some questions) :

- -add an asm function to get the current vga mode without changing it (or maybe a global var to keep the current vga mode)
- -add a field in struct grub_term
- -implement this function in different terminal handlers
- -implement grub_getwh
- -redefine GRUB_TERM_WIDTH & GRUB_TERM_HEIGHT macros to use grub_getwh

Here are my questions :
Should we foresee, in vga mode, that the user might someday choose his vga mode ?
Should we redefine all the macros we use to draw the menu to be less intensive on grub_getwh ?

In the attached patch, the vga function is disabled, because for now the asm function does not work (returns always 0). As I'm really not familiar with x86 asm, I think I had something wrong adapting from grub_vga_set_mode.

nb: I only give the patch as draft, a preview of what I'm working on.

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCyFNXFEQoKRQyjtURAtHIAKCDMtJhev1Z0mXvFm2Rg8p49rk4aQCgl2UR C3TbKukpU4T4dHFIjyTv6hI=
=wg2h
-----END PGP SIGNATURE-----

___________________________________________________________________________
Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger Téléchargez cette version sur http://fr.messenger.yahoo.com

author
#1 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Vincent Pelletier

In the attached patch,

*cough* *cough*

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCyFXJFEQoKRQyjtURAjFRAKCyEeBiF6HDVeeFajmy8oGfVWIrvQCfUiCG hX6z+XtVX9llG10YKEi6t7A=
=tmOI
-----END PGP SIGNATURE-----

Index: include/grub/term.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/term.h,v retrieving revision 1.7

grub/term.h +8 -2
diff -u -p -r1.7 term.h
@@ -72,8 +72,10 @@ grub_term_color_state;
 /* Menu-related geometrical constants.  */
 
 /* FIXME: These should be dynamically obtained from a terminal.  */
-#define GRUB_TERM_WIDTH		80
-#define GRUB_TERM_HEIGHT	25
+/*#define GRUB_TERM_WIDTH		80
+#define GRUB_TERM_HEIGHT	25*/
+#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
+#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)
 
 /* The number of lines of "GRUB version..." at the top.  */
 #define GRUB_TERM_INFO_HEIGHT	1

@@ -142,6 +144,9 @@ struct grub_term
   /* Get a character.  */
   int (*getkey) (void);
   
+  /* Get the screen size. The return value is ((Width << 8) | Height).  */
+  grub_uint16_t (*getwh) (void);
+
   /* Get the cursor position. The return value is ((X << 8) | Y).  */
   grub_uint16_t (*getxy) (void);
   

@@ -183,6 +188,7 @@ void EXPORT_FUNC(grub_putchar) (int c);
 void EXPORT_FUNC(grub_putcode) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
 void EXPORT_FUNC(grub_cls) (void);

Index: include/grub/i386/pc/vga.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/i386/pc/vga.h,v retrieving revision 1.3

grub/i386/pc/vga.h +3 -0
diff -u -p -r1.3 vga.h
@@ -25,6 +25,9 @@
 /* Set the video mode to MODE and return the previous mode.  */
 unsigned char EXPORT_FUNC(grub_vga_set_mode) (unsigned char mode);
 
+/* Return the current video mode.  */
+unsigned char EXPORT_FUNC(grub_vga_get_mode) (void);
+
 /* Return a pointer to the ROM font table.  */
 unsigned char *EXPORT_FUNC(grub_vga_get_font) (void);

Index: kern/term.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/term.c,v
retrieving revision 1.7

term.c +6 -0
diff -u -p -r1.7 term.c
@@ -213,6 +213,12 @@ grub_checkkey (void)
 }
 
 grub_uint16_t
+grub_getwh (void)
+{
+  return (grub_cur_term->getwh) ();
+}
+
+grub_uint16_t
 grub_getxy (void)
 {
   return (grub_cur_term->getxy) ();

Index: kern/i386/pc/startup.S
===================================================================
RCS file: /cvsroot/grub/grub2/kern/i386/pc/startup.S,v retrieving revision 1.13

i386/pc/startup.S +22 -0
diff -u -p -r1.13 startup.S
@@ -1570,6 +1570,28 @@ FUNCTION(grub_vga_set_mode)
 
 
 /*
+ * unsigned char grub_vga_get_mode (void)
+ */
+FUNCTION(grub_vga_get_mode)
+	pushl	%ebp
+	pushl	%ebx
+
+	call	prot_to_real
+	.code16
+	/* get current mode */
+	xorw	%bx, %bx
+	movb	$0x0f, %ah
+	int	$0x10
+
+	DATA32	call	real_to_prot
+	.code32
+
+	popl	%ebx
+	popl	%ebp
+	ret
+
+
+/*
  * unsigned char *grub_vga_get_font (void)
  */
 FUNCTION(grub_vga_get_font)

Index: term/i386/pc/console.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/console.c,v retrieving revision 1.5

i386/pc/console.c +8 -0
diff -u -p -r1.5 console.c
@@ -20,6 +20,7 @@
 #include <grub/machine/console.h>
 #include <grub/term.h>
 #include <grub/types.h>
+#include <grub/misc.h>
 
 grub_uint8_t grub_console_cur_color = 0x7;
 static grub_uint8_t grub_console_standard_color = 0x7;

@@ -74,6 +75,12 @@ grub_console_putchar (grub_uint32_t c)
   grub_console_real_putchar (c);
 }
 
+static grub_uint16_t
+grub_console_getwh (void)
+{
+  return (80 << 8) | 25; /* FIXME: Always true ?  */
+}
+
 static void
 grub_console_setcolorstate (grub_term_color_state state)
 {

@@ -107,6 +114,7 @@ static struct grub_term grub_console_ter
     .putchar = grub_console_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_console_getwh,
     .getxy = grub_console_getxy,
     .gotoxy = grub_console_gotoxy,
     .cls = grub_console_cls,

Index: term/i386/pc/vga.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/vga.c,v retrieving revision 1.7

i386/pc/vga.c +54 -0

66 lines not on this page · raw message

#2 lore raw
In reply to Vincent Pelletier
Quoted 2 lines · Vincent Pelletier

-add an asm function to get the current vga mode without changing it (or maybe a global var to keep the current vga mode)

Probably it is better to keep the current mode in a variable. It is not funny to go back to real mode every time.

Review

Quoted 4 lines
(quoted diff) +0 -4
-add a field in struct grub_term
-implement this function in different terminal handlers
-implement grub_getwh
-redefine GRUB_TERM_WIDTH & GRUB_TERM_HEIGHT macros to use grub_getwh

Sound good.

Quoted 3 lines

Here are my questions :
Should we foresee, in vga mode, that the user might someday choose his vga mode ?

No. Since the BIOS does not provide the geometry for each VGA mode, we must maintain a table in that case. And, the table differs among video cards. So I do not want to allow the user to change it to an arbitrary mode.

What we really need is to support VESA. The screen of the VGA mode is too small to do fancy stuff.

Quoted 2 lines

Should we redefine all the macros we use to draw the menu to be less intensive on grub_getwh ?

We need to call grub_getwh only once. Possibly in grub_menu_init_page.

Quoted 4 lines

In the attached patch, the vga function is disabled, because for now the asm function does not work (returns always 0). As I'm really not familiar with x86 asm, I think I had something wrong adapting from grub_vga_set_mode.

As the VGA mode is hardcoded, you can hardcode the geometry in vga.c as well.

BTW, is the VGA terminal working well? Since Marco changed the mode to 0x10 from 0x12, the screen is a bit smaller than before, and I'm not sure if the menu entry editor works with this.

Okuji

author
#3 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Yoshinori K. Okuji

We need to call grub_getwh only once. Possibly in grub_menu_init_page.

So we have to redesign the way sizes are computed to use something else than macros.

Quoted 3 lines

BTW, is the VGA terminal working well? Since Marco changed the mode to 0x10 from 0x12, the screen is a bit smaller than before, and I'm not sure if the menu entry editor works with this.

It wasn't working, and that's why I started studying the getwh idea :) . There is one line missing at the bottom of the screen, so when it's initialy drawn (frame + comments) it scrolls up one line. Then, when entries are drawn, they are drawn at the right absolute place, so one line below the top of the menu.

Maybe see you tomorr^Wtoday at the LSM :).

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCycP7FEQoKRQyjtURAvB6AJwMZ1ppB3e+sVxF0o2933WjFunNvgCfcaj0 ZpJw03aghGX1VP4v81oLvkc=
=7FD8
-----END PGP SIGNATURE-----

___________________________________________________________________________
Appel audio GRATUIT partout dans le monde avec le nouveau Yahoo! Messenger Téléchargez cette version sur http://fr.messenger.yahoo.com

#4 lore raw
In reply to Yoshinori K. Okuji
Quoted 6 lines · Yoshinori K. Okuji

No. Since the BIOS does not provide the geometry for each VGA mode, we must maintain a table in that case. And, the table differs among video cards. So I do not want to allow the user to change it to an arbitrary mode.

What we really need is to support VESA. The screen of the VGA mode is too small to do fancy stuff.

For VGA the first modes are all the same, so we can use a fixed table. For VESA modes we can get the width and height. But that should be done when setting the mode.

Quoted 3 lines

BTW, is the VGA terminal working well? Since Marco changed the mode to 0x10 from 0x12, the screen is a bit smaller than before, and I'm not sure if the menu entry editor works with this.

The menu should support all kinds of screen sizes anyway. For example on the PPC the text screen is really big.

--
Marco

#5 lore raw
Quoted 2 lines · Marco Gerards

The menu should support all kinds of screen sizes anyway. For example on the PPC the text screen is really big.

I think there are 2 different issues in a high resolution:

- How to adapt the menu without making it too ugly

- How to use bigger fonts

When I use yaboot on my iBook, I always feel that the font is too small. It is almost unreadable. So there must be a way to increase the font size.

Okuji

#6 lore raw
Quoted 4 lines · Yoshinori K. Okuji
Quoted 2 lines · Marco Gerards

The menu should support all kinds of screen sizes anyway. For example on the PPC the text screen is really big.

Quoted 8 lines

I think there are 2 different issues in a high resolution:

- How to adapt the menu without making it too ugly

- How to use bigger fonts

When I use yaboot on my iBook, I always feel that the font is too small. It is almost unreadable. So there must be a way to increase the font size.

Yes, I agree. It is very small. It should be possible to increase it.

If I have the time I will write framebuffer support for GRUB 2 on the PPC. But my todo list is big already so I do not know when. I hope someone else is interested in doing this. :)

Thanks,
Marco

author
#7 lore raw

PATCH 1/1 FIXME: These should be dynamically obtained from a terminal.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Here is a patch that adds grub_getwh functions.
Warning : terms other than console.c & vga.c will be broken by this patch, as long as they don't have the function.

2005-07-12 Vincent Pelletier <[email protected]>

* include/grub/term.h: (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
Redefined to use grub_getwh.
(struct grub_term): New field named getwh. (grub_getwh): New exported prototype.
* term/i386/pc/console.c: (grub_console_getwh): New function. (grub_console_term): New field and new initial value.
* term/i386/pc/vga.c: (grub_vga_getwh): New function. (grub_vga_term): New field and new initial value.

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC1CKkFEQoKRQyjtURAhQOAJ9kxitoVirko0thecc0gpAzguTyvgCcDyuS VE/VAYMisJ+VwyzyOLjhT5Y=
=Yygw
-----END PGP SIGNATURE-----

Index: include/grub/term.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/term.h,v retrieving revision 1.7

grub/term.h +7 -3
diff -u -p -r1.7 term.h
@@ -71,9 +71,9 @@ grub_term_color_state;
 
 /* Menu-related geometrical constants.  */
 
-/* FIXME: These should be dynamically obtained from a terminal.  */
-#define GRUB_TERM_WIDTH		80
-#define GRUB_TERM_HEIGHT	25
+/* FIXME: Ugly way to get them form terminal.  */
+#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
+#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)
 
 /* The number of lines of "GRUB version..." at the top.  */
 #define GRUB_TERM_INFO_HEIGHT	1

@@ -142,6 +142,9 @@ struct grub_term
   /* Get a character.  */
   int (*getkey) (void);
   
+  /* Get the screen size. The return value is ((Width << 8) | Height).  */
+  grub_uint16_t (*getwh) (void);
+
   /* Get the cursor position. The return value is ((X << 8) | Y).  */
   grub_uint16_t (*getxy) (void);
   

@@ -183,6 +186,7 @@ void EXPORT_FUNC(grub_putchar) (int c);
 void EXPORT_FUNC(grub_putcode) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
 void EXPORT_FUNC(grub_cls) (void);

Index: term/i386/pc/console.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/console.c,v retrieving revision 1.5

i386/pc/console.c +7 -0
diff -u -p -r1.5 console.c
@@ -74,6 +74,12 @@ grub_console_putchar (grub_uint32_t c)
   grub_console_real_putchar (c);
 }
 
+static grub_uint16_t
+grub_console_getwh (void)
+{
+  return (80 << 8) | 25; /* FIXME: Always true ?  */
+}
+
 static void
 grub_console_setcolorstate (grub_term_color_state state)
 {

@@ -107,6 +113,7 @@ static struct grub_term grub_console_ter
     .putchar = grub_console_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_console_getwh,
     .getxy = grub_console_getxy,
     .gotoxy = grub_console_gotoxy,
     .cls = grub_console_cls,

Index: term/i386/pc/vga.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/vga.c,v retrieving revision 1.7

i386/pc/vga.c +7 -0
diff -u -p -r1.7 vga.c
@@ -473,6 +473,12 @@ grub_vga_putchar (grub_uint32_t c)
 }
 
 static grub_uint16_t
+grub_vga_getwh (void)
+{
+  return (TEXT_WIDTH << 8) | TEXT_HEIGHT;
+}
+
+static grub_uint16_t
 grub_vga_getxy (void)
 {
   return ((xpos << 8) | ypos);

@@ -566,6 +572,7 @@ static struct grub_term grub_vga_term =
     .putchar = grub_vga_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_vga_getwh,
     .getxy = grub_vga_getxy,
     .gotoxy = grub_vga_gotoxy,
     .cls = grub_vga_cls,
author
#8 lore raw

PATCH v2 1/1 FIXME: These should be dynamically obtained from a terminal.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 2 lines · Vincent Pelletier

Warning : terms other than console.c & vga.c will be broken by this patch, as long as they don't have the function.

It's fixed in this patch for sparc64 port (see sparc_getwh.diff, which is a diff between term/powerpc and term/sparc64). I think the same code can be used for ppc.

It also contains the grub_getwh function I forgot in the previous patch.

2005-07-13 Vincent Pelletier <[email protected]>

* include/grub/term.h: (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
Redefined to use grub_getwh.
(struct grub_term): New field named getwh. (grub_getwh): New exported prototype.
* kern/term.c : (grub_getwh): New function.
* term/i386/pc/console.c: (grub_console_getwh): New function. (grub_console_term): New field and new initial value.
* term/i386/pc/vga.c: (grub_vga_getwh): New function. (grub_vga_term): New field and new initial value.

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC1NNnFEQoKRQyjtURAtibAJoDv9ya6nHL1JHkyIxLfWOeC8paaQCfcO9K fRM2BWoPzNOCr2NMZ+P3z9I=
=g1su
-----END PGP SIGNATURE-----

ieee1275/ofconsole.c +49 -3
diff -rupN powerpc/ieee1275/ofconsole.c sparc64/ieee1275/ofconsole.c
@@ -120,7 +120,7 @@ static int
 grub_ofconsole_readkey (int *key)
 {
   char c;
-  int actual = 0;
+  grub_ssize_t actual = 0;
 
   grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
 

@@ -207,6 +207,49 @@ grub_ofconsole_getxy (void)
   return ((grub_curr_x - 1) << 8) | grub_curr_y;
 }
 
+static grub_uint16_t
+grub_ofconsole_getwh (void)
+{
+  grub_ieee1275_ihandle_t config;
+  char *val;
+  grub_ssize_t lval;
+  static grub_uint8_t w, h;
+
+  if (w && h) /* Once we have them, don't ask them again.  */
+    {
+      if (! grub_ieee1275_open ("/config", &config) &&
+          config != -1)
+        {
+          if (! grub_ieee1275_get_property_length (config, "screen-#columns",
+                                                   &lval) && lval != -1)
+            {
+              val = grub_malloc (lval);
+              if (! grub_ieee1275_get_property (config, "screen-#columns",
+                                            val, lval, 0))
+                w = (grub_uint8_t) grub_strtoul (val, val + lval, 10);
+              grub_free (val);
+            }
+          if (! grub_ieee1275_get_property_length (config, "screen-#rows",
+                                                   &lval) && lval != -1)
+            {
+              val = grub_malloc (lval);
+              if (! grub_ieee1275_get_property (config, "screen-#rows",
+                                                val, lval, 0))
+                h = (grub_uint8_t) grub_strtoul (val, val + lval, 10);
+              grub_free (val);
+            }
+        }
+    }
+
+  /* XXX: Default values from OpenBoot on my U10.  */
+  if (! w)
+    w = 80;
+  if (! h)
+    h = 34;
+
+  return (w << 8) | h;
+}
+
 static void
 grub_ofconsole_gotoxy (grub_uint8_t x, grub_uint8_t y)
 {

@@ -222,6 +265,8 @@ static void
 grub_ofconsole_cls (void)
 {
   /* Clear the screen.  */
+  if (grub_env_get ("nocls"))
+    return;
   grub_ofconsole_writeesc ("\e[2J");
   grub_gotoxy (0, 0);
 }

@@ -241,8 +286,8 @@ grub_ofconsole_refresh (void)
 static grub_err_t
 grub_ofconsole_init (void)
 {
-  char data[4];
-  grub_size_t actual;
+  unsigned char data[4];
+  grub_ssize_t actual;
   int col;
 
   if (grub_ieee1275_get_property (grub_ieee1275_chosen, "stdout", data,

@@ -287,6 +332,7 @@ static struct grub_term grub_ofconsole_t
     .checkkey = grub_ofconsole_checkkey,
     .getkey = grub_ofconsole_getkey,
     .getxy = grub_ofconsole_getxy,
+    .getwh = grub_ofconsole_getwh,
     .gotoxy = grub_ofconsole_gotoxy,
     .cls = grub_ofconsole_cls,
     .setcolorstate = grub_ofconsole_setcolorstate,

Index: include/grub/term.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/term.h,v retrieving revision 1.7

grub/term.h +7 -3
diff -u -p -r1.7 term.h
@@ -71,9 +71,9 @@ grub_term_color_state;
 
 /* Menu-related geometrical constants.  */
 
-/* FIXME: These should be dynamically obtained from a terminal.  */
-#define GRUB_TERM_WIDTH		80
-#define GRUB_TERM_HEIGHT	25
+/* FIXME: Ugly way to get them form terminal.  */
+#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
+#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)
 
 /* The number of lines of "GRUB version..." at the top.  */
 #define GRUB_TERM_INFO_HEIGHT	1

@@ -142,6 +142,9 @@ struct grub_term
   /* Get a character.  */
   int (*getkey) (void);
   
+  /* Get the screen size. The return value is ((Width << 8) | Height).  */
+  grub_uint16_t (*getwh) (void);
+
   /* Get the cursor position. The return value is ((X << 8) | Y).  */
   grub_uint16_t (*getxy) (void);
   

@@ -183,6 +186,7 @@ void EXPORT_FUNC(grub_putchar) (int c);
 void EXPORT_FUNC(grub_putcode) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
 void EXPORT_FUNC(grub_cls) (void);

Index: kern/term.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/term.c,v
retrieving revision 1.7

term.c +6 -0
diff -u -p -r1.7 term.c
@@ -218,6 +218,12 @@ grub_getxy (void)
   return (grub_cur_term->getxy) ();
 }
 
+grub_uint16_t
+grub_getwh (void)
+{
+  return (grub_cur_term->getwh) ();
+}
+
 void
 grub_gotoxy (grub_uint8_t x, grub_uint8_t y)
 {

Index: term/i386/pc/console.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/console.c,v retrieving revision 1.5

i386/pc/console.c +7 -0
diff -u -p -r1.5 console.c
@@ -74,6 +74,12 @@ grub_console_putchar (grub_uint32_t c)
   grub_console_real_putchar (c);
 }
 
+static grub_uint16_t
+grub_console_getwh (void)
+{
+  return (80 << 8) | 25; /* FIXME: Always true ?  */
+}
+
 static void
 grub_console_setcolorstate (grub_term_color_state state)
 {

@@ -107,6 +113,7 @@ static struct grub_term grub_console_ter
     .putchar = grub_console_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_console_getwh,
     .getxy = grub_console_getxy,
     .gotoxy = grub_console_gotoxy,
     .cls = grub_console_cls,

Index: term/i386/pc/vga.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/vga.c,v retrieving revision 1.7

i386/pc/vga.c +7 -0
diff -u -p -r1.7 vga.c
@@ -473,6 +473,12 @@ grub_vga_putchar (grub_uint32_t c)
 }
 
 static grub_uint16_t
+grub_vga_getwh (void)
+{
+  return (TEXT_WIDTH << 8) | TEXT_HEIGHT;
+}
+
+static grub_uint16_t
 grub_vga_getxy (void)
 {
   return ((xpos << 8) | ypos);

@@ -566,6 +572,7 @@ static struct grub_term grub_vga_term =
     .putchar = grub_vga_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_vga_getwh,
     .getxy = grub_vga_getxy,
     .gotoxy = grub_vga_gotoxy,
     .cls = grub_vga_cls,
#9 lore raw
Quoted 7 lines · Vincent Pelletier
Quoted 2 lines · Vincent Pelletier

Warning : terms other than console.c & vga.c will be broken by this patch, as long as they don't have the function.

It's fixed in this patch for sparc64 port (see sparc_getwh.diff, which is a diff between term/powerpc and term/sparc64). I think the same code can be used for ppc.

Nice! I will test this on both the PPC boxes I have.

Quoted 5 lines

It also contains the grub_getwh function I forgot in the previous patch.

2005-07-13 Vincent Pelletier <[email protected]>

Quoted 9 lines

* include/grub/term.h: (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
Redefined to use grub_getwh.
(struct grub_term): New field named getwh. (grub_getwh): New exported prototype.
* kern/term.c : (grub_getwh): New function.
* term/i386/pc/console.c: (grub_console_getwh): New function. (grub_console_term): New field and new initial value.
* term/i386/pc/vga.c: (grub_vga_getwh): New function. (grub_vga_term): New field and new initial value.

This looks fine. Please remove the `:' after the filename in case something follows between the ()'s. For grub_vga_term you did not say which member was added.

Quoted 7 lines
ieee1275/ofconsole.c +7 -0
+static grub_uint16_t
+grub_ofconsole_getwh (void)
+{
+  grub_ieee1275_ihandle_t config;
+  char *val;
+  grub_ssize_t lval;
+  static grub_uint8_t w, h;

ieee1275/ofconsole.c grub_ofconsole_getxy (void) · line 226

Quoted 10 lines
ieee1275/ofconsole.c +10 -0
+
+  if (w && h) /* Once we have them, don't ask them again.  */
+    {
+      if (! grub_ieee1275_open ("/config", &config) &&
+          config != -1)
+        {
+          if (! grub_ieee1275_get_property_length (config, "screen-#columns",
+                                                   &lval) && lval != -1)
+            {
+              val = grub_malloc (lval);

Please check for errors here.

ieee1275/ofconsole.c grub_ofconsole_getxy (void) · line 235

Quoted 9 lines
ieee1275/ofconsole.c +9 -0
+              if (! grub_ieee1275_get_property (config, "screen-#columns",
+                                            val, lval, 0))
+                w = (grub_uint8_t) grub_strtoul (val, val + lval, 10);
+              grub_free (val);
+            }
+          if (! grub_ieee1275_get_property_length (config, "screen-#rows",
+                                                   &lval) && lval != -1)
+            {
+              val = grub_malloc (lval);

Same here.

i386/pc/console.c grub_console_putchar (grub_uint32_t c) · line 81

Quoted 5 lines
i386/pc/console.c +5 -0
+static grub_uint16_t
+grub_console_getwh (void)
+{
+  return (80 << 8) | 25; /* FIXME: Always true ?  */
+}

In mode 3 it is. Which is something we can assume, I hope. :)

Thanks,
Marco

author
#10 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 1 line · Marco Gerards

Nice! I will test this on both the PPC boxes I have.

I hope it works too. I haven't see any variable naming standard in the ieee1275 docs, so I'm afraid it won't find the variables.

Quoted 3 lines

This looks fine. Please remove the `:' after the filename in case something follows between the ()'s. For grub_vga_term you did not say which member was added.

Oops. Fixed below.

Quoted 1 line

Please check for errors here.

[...]

Quoted 1 line

Same here.

Done.
I thought I checked all the possible errors though :'( . And I forgot those simple ones.

Quoted 1 line

In mode 3 it is. Which is something we can assume, I hope. :)

Removed the comment.

First patch :

2005-07-13 Vincent Pelletier <[email protected]>

* include/grub/term.h (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
Redefined to use grub_getwh.
(struct grub_term): New field named getwh. (grub_getwh): New exported prototype.
* kern/term.c (grub_getwh): New function.
* term/i386/pc/console.c (grub_console_getwh): New function. (grub_console_term): New field named getwh and new initial value.
* term/i386/pc/vga.c (grub_vga_getwh): New function. (grub_vga_term): New field named getwh and new initial value.

Second patch :

2005-07-13 Vincent Pelletier <[email protected]>
* term/sparc64/ofconsole.c (grub_ofconsole_readkey): Use grub_ssize_t.
(grub_ofconsole_getw): New function. (grub_ofconsole_cls): Don't clear screen when debug environment variable is set, regardless of its value. (grub_ofconsole_init): Use grub_ssize_t and unsigned char. (grub_ofconsole_term): New field named getwh and new initial value.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC1YfMFEQoKRQyjtURAtliAJ9iZKLpn48u8arpEXsgLlm5GBSFPwCcCOMX ECZ8+QD/tK5cmdvciVRrnN8=
=EbEg
-----END PGP SIGNATURE-----

Index: include/grub/term.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/term.h,v retrieving revision 1.7

grub/term.h +7 -3
diff -u -p -r1.7 term.h
@@ -71,9 +71,9 @@ grub_term_color_state;
 
 /* Menu-related geometrical constants.  */
 
-/* FIXME: These should be dynamically obtained from a terminal.  */
-#define GRUB_TERM_WIDTH		80
-#define GRUB_TERM_HEIGHT	25
+/* FIXME: Ugly way to get them form terminal.  */
+#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
+#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)
 
 /* The number of lines of "GRUB version..." at the top.  */
 #define GRUB_TERM_INFO_HEIGHT	1

@@ -142,6 +142,9 @@ struct grub_term
   /* Get a character.  */
   int (*getkey) (void);
   
+  /* Get the screen size. The return value is ((Width << 8) | Height).  */
+  grub_uint16_t (*getwh) (void);
+
   /* Get the cursor position. The return value is ((X << 8) | Y).  */
   grub_uint16_t (*getxy) (void);
   

@@ -183,6 +186,7 @@ void EXPORT_FUNC(grub_putchar) (int c);
 void EXPORT_FUNC(grub_putcode) (grub_uint32_t code);
 int EXPORT_FUNC(grub_getkey) (void);
 int EXPORT_FUNC(grub_checkkey) (void);
+grub_uint16_t EXPORT_FUNC(grub_getwh) (void);
 grub_uint16_t EXPORT_FUNC(grub_getxy) (void);
 void EXPORT_FUNC(grub_gotoxy) (grub_uint8_t x, grub_uint8_t y);
 void EXPORT_FUNC(grub_cls) (void);

Index: kern/term.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/term.c,v
retrieving revision 1.7

term.c +6 -0
diff -u -p -r1.7 term.c
@@ -218,6 +218,12 @@ grub_getxy (void)
   return (grub_cur_term->getxy) ();
 }
 
+grub_uint16_t
+grub_getwh (void)
+{
+  return (grub_cur_term->getwh) ();
+}
+
 void
 grub_gotoxy (grub_uint8_t x, grub_uint8_t y)
 {

Index: term/i386/pc/console.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/console.c,v retrieving revision 1.5

i386/pc/console.c +7 -0
diff -u -p -r1.5 console.c
@@ -74,6 +74,12 @@ grub_console_putchar (grub_uint32_t c)
   grub_console_real_putchar (c);
 }
 
+static grub_uint16_t
+grub_console_getwh (void)
+{
+  return (80 << 8) | 25;
+}
+
 static void
 grub_console_setcolorstate (grub_term_color_state state)
 {

@@ -107,6 +113,7 @@ static struct grub_term grub_console_ter
     .putchar = grub_console_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_console_getwh,
     .getxy = grub_console_getxy,
     .gotoxy = grub_console_gotoxy,
     .cls = grub_console_cls,

Index: term/i386/pc/vga.c
===================================================================
RCS file: /cvsroot/grub/grub2/term/i386/pc/vga.c,v retrieving revision 1.7

i386/pc/vga.c +7 -0
diff -u -p -r1.7 vga.c
@@ -473,6 +473,12 @@ grub_vga_putchar (grub_uint32_t c)
 }
 
 static grub_uint16_t
+grub_vga_getwh (void)
+{
+  return (TEXT_WIDTH << 8) | TEXT_HEIGHT;
+}
+
+static grub_uint16_t
 grub_vga_getxy (void)
 {
   return ((xpos << 8) | ypos);

@@ -566,6 +572,7 @@ static struct grub_term grub_vga_term =
     .putchar = grub_vga_putchar,
     .checkkey = grub_console_checkkey,
     .getkey = grub_console_getkey,
+    .getwh = grub_vga_getwh,
     .getxy = grub_vga_getxy,
     .gotoxy = grub_vga_gotoxy,
     .cls = grub_vga_cls,
ieee1275/ofconsole.c +53 -3

86 lines not on this page · raw message

#11 lore raw

Vincent Pelletier <[email protected]> writes:

Hi Vincent,

Finally I had the time and energy to both proofread and test your patch. It did work for me after making some changes.

Quoted 15 lines

First patch :

2005-07-13 Vincent Pelletier <[email protected]>

* include/grub/term.h (GRUB_TERM_WIDTH, GRUB_TERM_HEIGHT):
Redefined to use grub_getwh.
(struct grub_term): New field named getwh. (grub_getwh): New exported prototype.
* kern/term.c (grub_getwh): New function.
* term/i386/pc/console.c (grub_console_getwh): New function. (grub_console_term): New field named getwh and new initial value.
* term/i386/pc/vga.c (grub_vga_getwh): New function. (grub_vga_term): New field named getwh and new initial value.

Quoted 5 lines

Second patch :

2005-07-13 Vincent Pelletier <[email protected]>
* term/sparc64/ofconsole.c (grub_ofconsole_readkey): Use grub_ssize_t.

You forgot a newline. The sparc64 directory does not exist. Hollis changes this to term/ieee1275, can you make that change?

Quoted 2 lines

(grub_ofconsole_cls): Don't clear screen when debug environment variable is set, regardless of its value.

Please do not check in this change. If we have to make such change, I think it should be in grub_cls ().

Quoted 3 lines

(grub_ofconsole_init): Use grub_ssize_t and unsigned char. (grub_ofconsole_term): New field named getwh and new initial value.

This should be a single changelog entry. As I see it, it is one patch.

Quoted 15 lines

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFC1YfMFEQoKRQyjtURAtliAJ9iZKLpn48u8arpEXsgLlm5GBSFPwCcCOMX ECZ8+QD/tK5cmdvciVRrnN8=
=EbEg
-----END PGP SIGNATURE-----
Index: include/grub/term.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/term.h,v retrieving revision 1.7
diff -u -p -r1.7 term.h

grub/term.h grub_term_color_state; · line 76

Quoted 9 lines
grub/term.h +3 -3
@@ -71,9 +71,9 @@ grub_term_color_state;
 
 /* Menu-related geometrical constants.  */
 
-/* FIXME: These should be dynamically obtained from a terminal.  */
-#define GRUB_TERM_WIDTH		80
-#define GRUB_TERM_HEIGHT	25
+/* FIXME: Ugly way to get them form terminal.  */
+#define GRUB_TERM_WIDTH         ((grub_getwh()&0xFF00)>>8)
+#define GRUB_TERM_HEIGHT        (grub_getwh()&0xFF)

Please use the GCS, so a space before the () and spaces surrounding the operators.

Quoted 2 lines

diff -rup powerpc/ieee1275/ofconsole.c sparc64/ieee1275/ofconsole.c
--- powerpc/ieee1275/ofconsole.c 2005-06-21 04:33:52.000000000 +0200

[...]

grub/term.h

Quoted 9 lines
grub/term.h +9 -0
+static grub_uint16_t
+grub_ofconsole_getwh (void)
+{
+  grub_ieee1275_ihandle_t config;
+  char *val;
+  grub_ssize_t lval;
+  static grub_uint8_t w, h;
+
+  if (w && h) /* Once we have them, don't ask them again.  */

The check is wrong, it should be:
if (! (w && h))

grub/term.h

Quoted 2 lines
grub/term.h +2 -0
+    {
+      if (! grub_ieee1275_open ("/config", &config) &&

This did not work for me, I changed this to:

if (! grub_ieee1275_finddevice ("/options", &config) &&

Does that change work for you? If it does not we have to figure out how to make it work on both the sparc and on the PPC.

grub/term.h

Quoted 6 lines
grub/term.h +6 -0
+          config != -1)
+        {
+          if (! grub_ieee1275_get_property_length (config, "screen-#columns",
+                                                   &lval) && lval != -1)
+            {
+              if ((val = grub_malloc (lval)) != 0)

I do not like this. Just use:

val = grub_malloc (...);
if (! val)
...

grub/term.h

Quoted 5 lines
grub/term.h +5 -0
+  /* XXX: Default values from OpenBoot on my U10.  */
+  if (! w)
+    w = 80;
+  if (! h)
+    h = 34;

Can you change that to 80x24, the defaults on the pegasos? It would be better to use values that are too small instead of too big.

Thanks,
Marco

#12 lore raw
Quoted 6 lines · Marco Gerards

Vincent Pelletier <[email protected]> writes:

Hi Vincent,

Finally I had the time and energy to both proofread and test your patch. It did work for me after making some changes.

Hi Vincent,

I have committed your patch with some changes and some extra code so the pager can use the screen height.

--
Marco

#13 lore raw
Quoted 11 lines · Marco Gerards
Quoted 6 lines · Marco Gerards

Vincent Pelletier <[email protected]> writes:

Hi Vincent,

Finally I had the time and energy to both proofread and test your patch. It did work for me after making some changes.

I have committed your patch with some changes and some extra code so the pager can use the screen height.

This patch introduced some build warnings which I've just taken care of.

-Hollis