Re: [PATCH] Video mode fixes in linux loader

[Robert Millan <rmh@aybabtu.com>]

[-- Attachment #1: Type: text/plain, Size: 1226 bytes --]

On Sat, May 02, 2009 at 01:31:14PM +0200, Robert Millan wrote:
> > > > "vga=ask" is not a warning now.  It causes "error: You need to load the
> > > > kernel first", apparently from initrd.  In other words, the "linux"
> > > > command fails and there is no visible warning.
> > > 
> > > Sounds like my error code is wrong, but we could turn it into a warning
> > > like you suggested.
> > 
> > I was editing the command line from the menu, so I  could not see the
> > message.  Waiting for input is a fair game for an option that implies
> > waiting for input.
> 
> Spot on.  Will do that.

Fixed.

See also this new patch.  It restructures the checks so that
"vid_mode == 0" indicates lack of "vga=" parameter.  For user requesting
text mode (vga=normal or vga=0) we already have GRUB_LINUX_VID_MODE_NORMAL
so there's no need to handle both values in GRUB.

It also introduces the GRUB_ASSUME_LINUX_HAS_FB_SUPPORT macro, which allows
easy override of the "fallback to text mode" setting.

-- 
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."

[-- Attachment #2: vid_mode.diff --]
[-- Type: text/x-diff, Size: 2849 bytes --]

2009-05-02  Robert Millan  <rmh.grub@aybabtu.com>

	* loader/i386/linux.c (GRUB_ASSUME_LINUX_HAS_FB_SUPPORT): New macro.
	(grub_linux_boot): Don't check for `linux_vesafb_modes' bounds (this
	is done by grub_cmd_linux() now).
	[! GRUB_ASSUME_LINUX_HAS_FB_SUPPORT]: If "vga=" parameter wasn't set,
	restore video to text mode.
	(grub_cmd_linux): Default `vid_mode' initialization to 0, which
	indicates lack of "vga=" parameter.  "vga=0" is mapped to
	`GRUB_LINUX_VID_MODE_NORMAL'.

Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2156)
+++ loader/i386/linux.c	(working copy)
@@ -39,6 +39,14 @@
 #define GRUB_LINUX_CL_OFFSET		0x1000
 #define GRUB_LINUX_CL_END_OFFSET	0x2000
 
+/* This macro is useful for distributors, who can be certain they built FB support
+   into Linux, and therefore can benefit from seamless mode transition between
+   GRUB and Linux (saving boot time and visual glitches).  Official GRUB, OTOH,
+   needs to be conservative.  */
+#ifndef GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
+#define GRUB_ASSUME_LINUX_HAS_FB_SUPPORT 0
+#endif
+
 static grub_dl_t my_mod;
 
 static grub_size_t linux_mem_size;
@@ -332,9 +340,7 @@ grub_linux_boot (void)
   
   params = real_mode_mem;
 
-  if (vid_mode < GRUB_LINUX_VID_MODE_VESA_START ||
-      vid_mode >= GRUB_LINUX_VID_MODE_VESA_START +
-		  ARRAY_SIZE (linux_vesafb_modes))
+  if (vid_mode == GRUB_LINUX_VID_MODE_NORMAL || vid_mode == GRUB_LINUX_VID_MODE_EXTENDED)
     grub_video_restore ();
   else if (vid_mode)
     {
@@ -367,6 +373,12 @@ grub_linux_boot (void)
 	  return grub_errno;
 	}
     }
+#if ! GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
+  else
+    /* If user didn't request a video mode, and we can't assume Linux supports FB,
+       then we go back to text mode.  */
+    grub_video_restore ();
+#endif
 
   if (! grub_linux_setup_video (params))
     params->have_vga = GRUB_VIDEO_TYPE_VLFB;
@@ -571,7 +583,7 @@ grub_cmd_linux (grub_command_t cmd __att
 	       (unsigned) real_size, (unsigned) prot_size);
 
   /* Look for memory size and video mode specified on the command line.  */
-  vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
+  vid_mode = 0;
   linux_mem_size = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "vga=", 4) == 0)
@@ -596,6 +608,22 @@ grub_cmd_linux (grub_command_t cmd __att
 	else
 	  vid_mode = (grub_uint16_t) grub_strtoul (val, 0, 0);
 
+	switch (vid_mode)
+	  {
+	  case 0:
+	    vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
+	    break;
+	  case 1:
+	    vid_mode = GRUB_LINUX_VID_MODE_EXTENDED;
+	    break;
+	  default:
+	    /* Ignore invalid values.  */
+	    if (vid_mode < GRUB_LINUX_VID_MODE_VESA_START ||
+		vid_mode >= GRUB_LINUX_VID_MODE_VESA_START +
+		ARRAY_SIZE (linux_vesafb_modes))
+	      vid_mode = 0;
+	  }
+
 	if (grub_errno)
 	  goto fail;
       }