[PATCH] Video mode fixes in linux loader

[PATCH] Video mode fixes in linux loader

[Pavel Roskin]

Hello!

This is an attempt to fix all issues with the video mode handling in the
new Linux loader.

First of all, free_page() doesn't belong to grub_linux_unload().  The
later function is called after the new kernel has been loaded, just
before the boot.  Thus it obliterates the data set up by the last
"linux" command if the previous Linux boot failed.

Instead, free_page() should be called from allocate_pages() to reset not
only real_mode_mem and prot_mode_mem, but also initrd_mem.

Next problem is that grub_linux_boot() can access linux_vesafb_modes
with a wrong index.  I believe we should threat modes that don't fit the
array as text modes.

I introduced GRUB_LINUX_VID_MODE_VESA_START for the first VESA mode we
support.  Also, I introduced a macro ARRAY_SIZE to calculate the array
size.

The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
the kernel we tried to load before.  Finally, "vga=ask" is now
recognized.

ChangeLog:

        * include/grub/misc.h (ARRAY_SIZE): New macro.
        * include/grub/i386/linux.h (GRUB_LINUX_VID_MODE_VESA_START):
        New macro.
        * loader/i386/linux.c (allocate_pages): Use free_pages().
        (grub_linux_unload): Don't use free_pages().
        (grub_linux_boot): Prevent accessing linux_vesafb_modes with a
        wrong index.  Treat all other modes as text modes.
        (grub_cmd_linux): Initialize vid_mode unconditionally to
        GRUB_LINUX_VID_MODE_NORMAL.  Recognize and support "vga=ask".
	
Index: include/grub/misc.h
===================================================================
--- include/grub/misc.h	(revision 2068)
+++ include/grub/misc.h	(working copy)
@@ -26,6 +26,7 @@
 #include <grub/err.h>
 
 #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1) & ~(align - 1))
+#define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
 
 #define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__, __LINE__, condition, fmt, ## args);
 /* XXX: If grub_memmove is too slow, we must implement grub_memcpy.  */
Index: include/grub/i386/linux.h
===================================================================
--- include/grub/i386/linux.h	(revision 2068)
+++ include/grub/i386/linux.h	(working copy)
@@ -38,6 +38,7 @@
 #define GRUB_LINUX_VID_MODE_NORMAL	0xFFFF
 #define GRUB_LINUX_VID_MODE_EXTENDED	0xFFFE
 #define GRUB_LINUX_VID_MODE_ASK		0xFFFD
+#define GRUB_LINUX_VID_MODE_VESA_START	0x0301
 
 #define GRUB_LINUX_SETUP_MOVE_SIZE	0x9100
 #define GRUB_LINUX_CL_MAGIC		0xA33F
Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2068)
+++ loader/i386/linux.c	(working copy)
@@ -206,8 +206,7 @@ allocate_pages (grub_size_t prot_size)
   prot_mode_pages = (prot_size >> 12);
   
   /* Initialize the memory pointers with NULL for convenience.  */
-  real_mode_mem = 0;
-  prot_mode_mem = 0;
+  free_pages();
   
   /* FIXME: Should request low memory from the heap when this feature is
      implemented.  */
@@ -332,7 +331,9 @@ grub_linux_boot (void)
   
   params = real_mode_mem;
 
-  if (vid_mode == GRUB_LINUX_VID_MODE_NORMAL || vid_mode == GRUB_LINUX_VID_MODE_EXTENDED)
+  if (vid_mode < GRUB_LINUX_VID_MODE_VESA_START ||
+      vid_mode >= GRUB_LINUX_VID_MODE_VESA_START +
+		  ARRAY_SIZE (linux_vesafb_modes))
     grub_video_restore ();
   else if (vid_mode)
     {
@@ -340,7 +341,7 @@ grub_linux_boot (void)
       int depth, flags;
       
       flags = 0;
-      linux_mode = &linux_vesafb_modes[vid_mode - 0x301];
+      linux_mode = &linux_vesafb_modes[vid_mode - GRUB_LINUX_VID_MODE_VESA_START];
       depth = linux_mode->depth;
       
       /* If we have 8 or less bits, then assume that it is indexed color mode.  */
@@ -443,7 +444,6 @@ grub_linux_boot (void)
 static grub_err_t
 grub_linux_unload (void)
 {
-  free_pages ();
   grub_dl_unref (my_mod);
   loaded = 0;
   return GRUB_ERR_NONE;
@@ -570,6 +570,7 @@ grub_cmd_linux (grub_command_t cmd __att
 	       (unsigned) real_size, (unsigned) prot_size);
 
   /* Detect explicitly specified memory size, if any.  */
+  vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
   linux_mem_size = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "vga=", 4) == 0)
@@ -581,6 +582,8 @@ grub_cmd_linux (grub_command_t cmd __att
 	  vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
 	else if (grub_strcmp (val, "ext") == 0)
 	  vid_mode = GRUB_LINUX_VID_MODE_EXTENDED;
+	else if (grub_strcmp (val, "ask") == 0)
+	  vid_mode = GRUB_LINUX_VID_MODE_ASK;
 	else
 	  vid_mode = (grub_uint16_t) grub_strtoul (val, 0, 0);
 


-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[Yoshinori K. Okuji]

On Tuesday 07 April 2009 00:34:03 Pavel Roskin wrote:
> Hello!
>
> This is an attempt to fix all issues with the video mode handling in the
> new Linux loader.
>
> First of all, free_page() doesn't belong to grub_linux_unload().  The
> later function is called after the new kernel has been loaded, just
> before the boot.  Thus it obliterates the data set up by the last
> "linux" command if the previous Linux boot failed.
>
> Instead, free_page() should be called from allocate_pages() to reset not
> only real_mode_mem and prot_mode_mem, but also initrd_mem.
>
> Next problem is that grub_linux_boot() can access linux_vesafb_modes
> with a wrong index.  I believe we should threat modes that don't fit the
> array as text modes.
>
> I introduced GRUB_LINUX_VID_MODE_VESA_START for the first VESA mode we
> support.  Also, I introduced a macro ARRAY_SIZE to calculate the array
> size.
>
> The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
> the kernel we tried to load before.  Finally, "vga=ask" is now
> recognized.
>
> ChangeLog:
>
>         * include/grub/misc.h (ARRAY_SIZE): New macro.
>         * include/grub/i386/linux.h (GRUB_LINUX_VID_MODE_VESA_START):
>         New macro.
>         * loader/i386/linux.c (allocate_pages): Use free_pages().
>         (grub_linux_unload): Don't use free_pages().
>         (grub_linux_boot): Prevent accessing linux_vesafb_modes with a
>         wrong index.  Treat all other modes as text modes.
>         (grub_cmd_linux): Initialize vid_mode unconditionally to
>         GRUB_LINUX_VID_MODE_NORMAL.  Recognize and support "vga=ask".
>
> Index: include/grub/misc.h
> ===================================================================
> --- include/grub/misc.h	(revision 2068)
> +++ include/grub/misc.h	(working copy)
> @@ -26,6 +26,7 @@
>  #include <grub/err.h>
>
>  #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1) & ~(align
> - 1)) +#define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
>
>  #define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__,
> __LINE__, condition, fmt, ## args); /* XXX: If grub_memmove is too slow, we
> must implement grub_memcpy.  */ Index: include/grub/i386/linux.h
> ===================================================================
> --- include/grub/i386/linux.h	(revision 2068)
> +++ include/grub/i386/linux.h	(working copy)
> @@ -38,6 +38,7 @@
>  #define GRUB_LINUX_VID_MODE_NORMAL	0xFFFF
>  #define GRUB_LINUX_VID_MODE_EXTENDED	0xFFFE
>  #define GRUB_LINUX_VID_MODE_ASK		0xFFFD
> +#define GRUB_LINUX_VID_MODE_VESA_START	0x0301
>
>  #define GRUB_LINUX_SETUP_MOVE_SIZE	0x9100
>  #define GRUB_LINUX_CL_MAGIC		0xA33F
> Index: loader/i386/linux.c
> ===================================================================
> --- loader/i386/linux.c	(revision 2068)
> +++ loader/i386/linux.c	(working copy)
> @@ -206,8 +206,7 @@ allocate_pages (grub_size_t prot_size)
>    prot_mode_pages = (prot_size >> 12);
>
>    /* Initialize the memory pointers with NULL for convenience.  */
> -  real_mode_mem = 0;
> -  prot_mode_mem = 0;
> +  free_pages();

Please insert a space before the parenthesis.

>
>    /* FIXME: Should request low memory from the heap when this feature is
>       implemented.  */
> @@ -332,7 +331,9 @@ grub_linux_boot (void)
>
>    params = real_mode_mem;
>
> -  if (vid_mode == GRUB_LINUX_VID_MODE_NORMAL || vid_mode ==
> GRUB_LINUX_VID_MODE_EXTENDED) +  if (vid_mode <
> GRUB_LINUX_VID_MODE_VESA_START ||
> +      vid_mode >= GRUB_LINUX_VID_MODE_VESA_START +
> +		  ARRAY_SIZE (linux_vesafb_modes))
>      grub_video_restore ();
>    else if (vid_mode)
>      {
> @@ -340,7 +341,7 @@ grub_linux_boot (void)
>        int depth, flags;
>
>        flags = 0;
> -      linux_mode = &linux_vesafb_modes[vid_mode - 0x301];
> +      linux_mode = &linux_vesafb_modes[vid_mode -
> GRUB_LINUX_VID_MODE_VESA_START]; depth = linux_mode->depth;
>
>        /* If we have 8 or less bits, then assume that it is indexed color
> mode.  */ @@ -443,7 +444,6 @@ grub_linux_boot (void)
>  static grub_err_t
>  grub_linux_unload (void)
>  {
> -  free_pages ();
>    grub_dl_unref (my_mod);
>    loaded = 0;
>    return GRUB_ERR_NONE;
> @@ -570,6 +570,7 @@ grub_cmd_linux (grub_command_t cmd __att
>  	       (unsigned) real_size, (unsigned) prot_size);
>
>    /* Detect explicitly specified memory size, if any.  */

This is not due to you, but the comment above is not synchronized with the 
code apparently. Can you fix it?

> +  vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
>    linux_mem_size = 0;
>    for (i = 1; i < argc; i++)
>      if (grub_memcmp (argv[i], "vga=", 4) == 0)
> @@ -581,6 +582,8 @@ grub_cmd_linux (grub_command_t cmd __att
>  	  vid_mode = GRUB_LINUX_VID_MODE_NORMAL;
>  	else if (grub_strcmp (val, "ext") == 0)
>  	  vid_mode = GRUB_LINUX_VID_MODE_EXTENDED;
> +	else if (grub_strcmp (val, "ask") == 0)
> +	  vid_mode = GRUB_LINUX_VID_MODE_ASK;
>  	else
>  	  vid_mode = (grub_uint16_t) grub_strtoul (val, 0, 0);

Regards,
Okuji

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Tue, 2009-04-07 at 09:31 +0900, Yoshinori K. Okuji wrote:
> > +  free_pages();
> 
> Please insert a space before the parenthesis.

Done.

> This is not due to you, but the comment above is not synchronized with the 
> code apparently. Can you fix it?

Fixed and committed.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[phcoder]

Does it actually work? I don't see the code which either handles 
GRUB_LINUX_VID_MODE_ASK or passes it to the kernel. Also it looks like 
vga= parameter is parsed by grub and grub passes only video mode values 
as resolution and color depth. Wouldn't it be better to move to a more 
modern video mode specification method? As gfxpayload variable 
previously discussed on that list
Pavel Roskin wrote:
> On Tue, 2009-04-07 at 09:31 +0900, Yoshinori K. Okuji wrote:
>>> +  free_pages();
>> Please insert a space before the parenthesis.
> 
> Done.
> 
>> This is not due to you, but the comment above is not synchronized with the 
>> code apparently. Can you fix it?
> 
> Fixed and committed.
> 


-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Tue, 2009-04-07 at 09:31 +0200, phcoder wrote:
> Does it actually work?

No.  But at least it loads the kernel.

>  I don't see the code which either handles 
> GRUB_LINUX_VID_MODE_ASK or passes it to the kernel.

We could set params->vid_mode, but the mode setting in the kernel is
done in the 16-bit code that we don't use.

>  Also it looks like 
> vga= parameter is parsed by grub and grub passes only video mode values 
> as resolution and color depth. Wouldn't it be better to move to a more 
> modern video mode specification method? As gfxpayload variable 
> previously discussed on that list

OK, I just fixed the issue that was annoying me.  Being a PC user, I
don't really care about the graphical boot.

After touching that code, I believe the 32-bit Linux loader should not
parse the vga option at all.  It's irrelevant.

Instead, the loader should either load the kernel in the current mode
(text of gfxterm) or it should restore the text mode.  I believe not all
kernels can start in graphical mode, so using the graphical mode should
be enabled explicitly by some option.

If the platform doesn't have text mode, then obviously the current mode
should be used.

I don't see any strong need to use different video modes in GRUB and in
the kernel.  If somebody needs that, the mode in GRUB can be changed
before loading the kernel.  Also, a kernel video driver can change the
mode upon initialization using driver-specific parameters.

GRUB could implement video mode handling compatible with the Linux
internal bootloader, including "vga=ask", but I'm not sure we need it.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[phcoder]

Pavel Roskin wrote:
>>  I don't see the code which either handles 
>> GRUB_LINUX_VID_MODE_ASK or passes it to the kernel.
> 
> We could set params->vid_mode, but the mode setting in the kernel is
> done in the 16-bit code that we don't use.
Or we could do the actual asking like linux16 does
> 
>>  Also it looks like 
>> vga= parameter is parsed by grub and grub passes only video mode values 
>> as resolution and color depth. Wouldn't it be better to move to a more 
>> modern video mode specification method? As gfxpayload variable 
>> previously discussed on that list
> 
> OK, I just fixed the issue that was annoying me.  Being a PC user, I
> don't really care about the graphical boot.
> 
> After touching that code, I believe the 32-bit Linux loader should not
> parse the vga option at all.  It's irrelevant.
> 
This option may be kept for backward compatibility a possible fallback 
chain is:
gfxpayload->vga->previousvideo mode
> Instead, the loader should either load the kernel in the current mode
> (text of gfxterm) or it should restore the text mode.  I believe not all
> kernels can start in graphical mode, so using the graphical mode should
> be enabled explicitly by some option.
> 
> If the platform doesn't have text mode, then obviously the current mode
> should be used.
> 
> I don't see any strong need to use different video modes in GRUB and in
> the kernel.  If somebody needs that, the mode in GRUB can be changed
> before loading the kernel.  Also, a kernel video driver can change the
> mode upon initialization using driver-specific parameters.
> 
Every OS has its specific needs. E.g. xnu needs to boot in video mode. 
Perhaps it's just me not knowing how to boot it in text mode but this 
limitation will have to be respected. And I don't feel like initialising 
  video console just to set a supported mode is a good idea.
> GRUB could implement video mode handling compatible with the Linux
> internal bootloader, including "vga=ask", but I'm not sure we need it.
> 
I don't think we need it either just no system should be rendered 
unbootable by this

-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Sun, 2009-04-12 at 21:11 +0200, phcoder wrote:
> Pavel Roskin wrote:
> >>  I don't see the code which either handles 
> >> GRUB_LINUX_VID_MODE_ASK or passes it to the kernel.
> > 
> > We could set params->vid_mode, but the mode setting in the kernel is
> > done in the 16-bit code that we don't use.
> Or we could do the actual asking like linux16 does

I don't see that in the code.

Anyway, gfxterm and graphical boot is not something I'm very interested
in.  It's just the breakage with vga=1 that made fix the code.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

Hi Pavel,

On Mon, Apr 06, 2009 at 11:34:03AM -0400, Pavel Roskin wrote:
> Hello!
> 
> This is an attempt to fix all issues with the video mode handling in the
> new Linux loader.

Please, in general, when you modify code that has been actively worked on
by someone, try to get that person involved.  If I'm to busy to check the
list at a given time, feel free to CC me or msg me on IRC.  Some things
in your patch were not right.

> First of all, free_page() doesn't belong to grub_linux_unload().  The
> later function is called after the new kernel has been loaded, just
> before the boot.  Thus it obliterates the data set up by the last
> "linux" command if the previous Linux boot failed.
> 
> Instead, free_page() should be called from allocate_pages() to reset not
> only real_mode_mem and prot_mode_mem, but also initrd_mem.

Ok.

> Next problem is that grub_linux_boot() can access linux_vesafb_modes
> with a wrong index.  I believe we should threat modes that don't fit the
> array as text modes.
> 
> I introduced GRUB_LINUX_VID_MODE_VESA_START for the first VESA mode we
> support.  Also, I introduced a macro ARRAY_SIZE to calculate the array
> size.

Ok.

> The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
> the kernel we tried to load before.

Ok, BUT if we're already in vesa mode, and we know it works (since we're using
it), there's no point in wasting time only to get a worse mode.

We should just make sure subsequent calls to "linux" command override the
previous one.

> Finally, "vga=ask" is now recognized.

With the new loader, Linux' 16-bit entry code is no longer responsible for
setting vesa modes, therefore vga=ask can't work.  There's no point in
recognizing it (except to warn the user).

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

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

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

On Mon, Apr 13, 2009 at 04:16:57PM +0200, Robert Millan wrote:
> > The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
> > the kernel we tried to load before.
> 
> Ok, BUT if we're already in vesa mode, and we know it works (since we're using
> it), there's no point in wasting time only to get a worse mode.
> 
> We should just make sure subsequent calls to "linux" command override the
> previous one.

I think this should do it.  Can you confirm this works for you?

(note: this patch is intentionally missing indentation, for readability)

-- 
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: 1533 bytes --]


	* loader/i386/linux.c (grub_linux_boot): Only go back to text
	mode when `vid_mode' wasn't initialized.  Otherwise leave the
	current mode (whatever that is).
	(grub_cmd_linux): Default to `vid_mode = 0' rather than
	`GRUB_LINUX_VID_MODE_NORMAL'.

Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2097)
+++ loader/i386/linux.c	(working copy)
@@ -331,12 +331,14 @@ 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))
-    grub_video_restore ();
-  else if (vid_mode)
+  if (vid_mode)
     {
+      if (vid_mode < GRUB_LINUX_VID_MODE_VESA_START ||
+	  vid_mode >= GRUB_LINUX_VID_MODE_VESA_START +
+	  ARRAY_SIZE (linux_vesafb_modes))
+	grub_video_restore ();
+      else
+	{
       struct linux_vesafb_mode *linux_mode;
       int depth, flags;
       
@@ -366,6 +368,7 @@ grub_linux_boot (void)
 	  return grub_errno;
 	}
     }
+    }
 
   if (! grub_linux_setup_video (params))
     params->have_vga = GRUB_VIDEO_TYPE_VLFB;
@@ -570,8 +573,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;
-  linux_mem_size = 0;
+  vid_mode = linux_mem_size = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "vga=", 4) == 0)
       {

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Mon, 2009-04-13 at 16:45 +0200, Robert Millan wrote:
> On Mon, Apr 13, 2009 at 04:16:57PM +0200, Robert Millan wrote:
> > > The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
> > > the kernel we tried to load before.
> > 
> > Ok, BUT if we're already in vesa mode, and we know it works (since we're using
> > it), there's no point in wasting time only to get a worse mode.
> > 
> > We should just make sure subsequent calls to "linux" command override the
> > previous one.
> 
> I think this should do it.  Can you confirm this works for you?

I actually installed GRUB with gfxterm on a laptop that has Intel
framebuffer support.  Now the kernel starts in VESA mode and then the
screen goes blank because intelfb cannot deal with it.  Sure, intelfb
should be fixed, but we should be liberal in what we accept.  Some
kernels may not support VESA modes at all.

Adding vga=0 to the kernel command line didn't fix it.  That's bad.
"vga=0" means text mode 80x25.  Adding "vga=1" fixed the problem.  The
text mode was 80x25, not 80x50, so that's another issue.

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

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Mon, Apr 13, 2009 at 11:41:36AM -0400, Pavel Roskin wrote:
> I actually installed GRUB with gfxterm on a laptop that has Intel
> framebuffer support.  Now the kernel starts in VESA mode and then the
> screen goes blank because intelfb cannot deal with it.  Sure, intelfb
> should be fixed, but we should be liberal in what we accept.

We could detect this situation by checking video= parameter, and setting
text mode if intelfb is found.  But then again do we want to prevent
future versions of intelfb from gracefuly transitioning from vesa mode
without screen glitch?

> Some
> kernels may not support VESA modes at all.

I don't think this is applicable;  all modern versions of Linux include
vesa modesetting in its 16-bit entry code, and older versions are already
detected by the new loader (user is prompted to use linux16).

> Adding vga=0 to the kernel command line didn't fix it.  That's bad.
> "vga=0" means text mode 80x25.  Adding "vga=1" fixed the problem.  The
> text mode was 80x25, not 80x50, so that's another issue.

Shouldn't be hard to fix.  Do you know how to switch to 80x50 mode?

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

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

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Mon, 2009-04-13 at 21:05 +0200, Robert Millan wrote:
> On Mon, Apr 13, 2009 at 11:41:36AM -0400, Pavel Roskin wrote:
> > I actually installed GRUB with gfxterm on a laptop that has Intel
> > framebuffer support.  Now the kernel starts in VESA mode and then the
> > screen goes blank because intelfb cannot deal with it.  Sure, intelfb
> > should be fixed, but we should be liberal in what we accept.
> 
> We could detect this situation by checking video= parameter, and setting
> text mode if intelfb is found.  But then again do we want to prevent
> future versions of intelfb from gracefuly transitioning from vesa mode
> without screen glitch?

No, that would be bad.  It's even possible that intelfb would work
correctly in other configurations.  The laptop has resolution 1440x900
that doesn't match any VESA mode.

> > Some
> > kernels may not support VESA modes at all.
> 
> I don't think this is applicable;  all modern versions of Linux include
> vesa modesetting in its 16-bit entry code, and older versions are already
> detected by the new loader (user is prompted to use linux16).

I can disable CONFIG_FB, and then the screen remains blank until X
starts.  It's entirely possible that some distros don't enable CONFIG_FB
to save memory, and I don't always enable it in the kernels I configure
myself.

> > Adding vga=0 to the kernel command line didn't fix it.  That's bad.
> > "vga=0" means text mode 80x25.  Adding "vga=1" fixed the problem.  The
> > text mode was 80x25, not 80x50, so that's another issue.
> 
> Shouldn't be hard to fix.  Do you know how to switch to 80x50 mode?

Well, load 8x8 font.  It's done in the 16-bit code.

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

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[phcoder]

Pavel Roskin wrote:
> On Mon, 2009-04-13 at 21:05 +0200, Robert Millan wrote:
>> On Mon, Apr 13, 2009 at 11:41:36AM -0400, Pavel Roskin wrote:
>>> I actually installed GRUB with gfxterm on a laptop that has Intel
>>> framebuffer support.  Now the kernel starts in VESA mode and then the
>>> screen goes blank because intelfb cannot deal with it.  Sure, intelfb
>>> should be fixed, but we should be liberal in what we accept.
>> We could detect this situation by checking video= parameter, and setting
>> text mode if intelfb is found.  But then again do we want to prevent
>> future versions of intelfb from gracefuly transitioning from vesa mode
>> without screen glitch?
> 
> No, that would be bad.  It's even possible that intelfb would work
> correctly in other configurations.  The laptop has resolution 1440x900
> that doesn't match any VESA mode.
> 
With proposed autodetections bootloader becomes overzealous. I think it 
should be configirable but in a more modern way. I think gfxpayload with 
the same syntax as gfxmode plus additional platform-specific keywords 
like text80x25 and text80x50 should be fine. If to gfxpayload variable 
set, then use current mode.
I object against a solution of just using current mode because of the 
current state on EFI: code to retrieve framebuffer address isn't yet 
used for gfxterm. If this is fixed then I could agree with "last mode" 
solution.

vga= option could be deleted altogether and replaced by warning if 
someone passes it. If we do so it's better to do before debian switches

-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Tue, Apr 14, 2009 at 02:02:46AM +0200, phcoder wrote:
> With proposed autodetections bootloader becomes overzealous. I think it  
> should be configirable but in a more modern way. I think gfxpayload with  
> the same syntax as gfxmode plus additional platform-specific keywords  
> like text80x25 and text80x50 should be fine.

Sounds nice.  Will you implement this?

> If to gfxpayload variable  
> set, then use current mode.

I think based on what Pavel said, and on what you say below, that using
current mode should be avoided in official GRUB, because of the need to
support a wide variety of build configurations.  If gfxpayload is unset I
would just reset it.

Someone who's also distributing builds of Linux and has intimate knowledge
about their setup, can also make similar decisions when it comes to setting
up GRUB build options.

> I object against a solution of just using current mode because of the  
> current state on EFI: code to retrieve framebuffer address isn't yet  
> used for gfxterm.

Can this be fixed?

> vga= option could be deleted altogether and replaced by warning if  
> someone passes it.

A warning is fine, but please keep the vga=.  It's useful to provide
backward compatibility, even if it's a deprecated interface.

(Though, since it's backward compatibility, I would only provide it on BIOS
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."

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Mon, Apr 13, 2009 at 07:20:20PM -0400, Pavel Roskin wrote:
> > We could detect this situation by checking video= parameter, and setting
> > text mode if intelfb is found.  But then again do we want to prevent
> > future versions of intelfb from gracefuly transitioning from vesa mode
> > without screen glitch?
> 
> No, that would be bad.  It's even possible that intelfb would work
> correctly in other configurations.  The laptop has resolution 1440x900
> that doesn't match any VESA mode.

Alright.

> > > Some
> > > kernels may not support VESA modes at all.
> > 
> > I don't think this is applicable;  all modern versions of Linux include
> > vesa modesetting in its 16-bit entry code, and older versions are already
> > detected by the new loader (user is prompted to use linux16).
> 
> I can disable CONFIG_FB, and then the screen remains blank until X
> starts.  It's entirely possible that some distros don't enable CONFIG_FB
> to save memory, and I don't always enable it in the kernels I configure
> myself.

Makes sense for official GRUB.

However, I'd still like to add a macro check that can be enabled on distros
that ship Linux builds with CONFIG_FB and want to enable seamless mode
transition (this will be the case for e.g. Debian).

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

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

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

[-- 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;
       }

Re: [PATCH] Video mode fixes in linux loader

[BandiPat]

Robert Millan wrote:
> 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.
> 
> 
> 
> ------------------------------------------------------------------------
This may follow the last little problem we are having getting the splash 
screen to show in Zenwalk.  We were in IRC last weekend trying to come 
up with a solution as to why we had to use linux16 & initrd16 with the 
vga= command in the grub.cfg, but nothing he suggested seemed to work. 
Here is a section from my grub.cfg:
#
# DO NOT EDIT THIS FILE
#
# It is automatically generated by /usr/sbin/grub-mkconfig using templates
# from  and settings from /etc/default/grub
#

### BEGIN /etc/grub.d/00_header ###
set default=0
set timeout=10
set root=(hd0,1)
search --fs-uuid --set 82d81803-e0a2-487d-9c83-ec6d8d35cf2a
if loadfont /usr/share/grub/ascii.pf2 ; then
   set gfxmode=1024x768x32
   insmod gfxterm
   insmod video
   insmod vbe
   if terminal_output gfxterm ; then true ; else
     # For backward compatibility with versions of terminal.mod that don't
     # understand terminal_output
     terminal gfxterm
   fi
fi
insmod png
background_image /boot/grub/ZenBlack.png
### END /etc/grub.d/00_header ###

### BEGIN /etc/grub.d/10_linux ###
menuentry "Zenwalk 6.0 GNU/Linux" {
	set root=(hd0,1)
	search --fs-uuid --set 82d81803-e0a2-487d-9c83-ec6d8d35cf2a
	linux16	/boot/vmlinuz root=/dev/sda1 ro resume=/dev/sda4 splash=silent 
vga=794
	initrd16 /boot/initrd.splash
}
-------------------------
My first question for you Robert is; will your vid_mode.diff correct the 
need for using vga= and linux16 to get the splash screen to display in 
the correct resolution?  Second, what is needed in the grub.cfg to be 
able to not use the deprecated linux16 & vga= anymore?  Does someone 
have a sample grub.cfg for x86 Linux that works to display the splash 
screen using linux, but without the vga= ?  If you do, please send it to 
me, so I might try your solution.  I can't believe this is as big a 
problem as it appears to be, but no one seems to have mentioned a viable 
solution yet!

Thanks for your help!
Pat

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Sat, May 02, 2009 at 08:03:23PM -0400, BandiPat wrote:
> 	linux16	/boot/vmlinuz root=/dev/sda1 ro resume=/dev/sda4 splash=silent  
> vga=794

This means vga=0x31a, aka 16-bit 1280x1024.  Does 24-bit (vga=0x31b) work?

I suspect there's some fuzzy matching here.

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

Re: [PATCH] Video mode fixes in linux loader

[BandiPat]

Robert Millan wrote:
> On Sat, May 02, 2009 at 08:03:23PM -0400, BandiPat wrote:
>> 	linux16	/boot/vmlinuz root=/dev/sda1 ro resume=/dev/sda4 splash=silent  
>> vga=794
> 
> This means vga=0x31a, aka 16-bit 1280x1024.  Does 24-bit (vga=0x31b) work?
> 
> I suspect there's some fuzzy matching here.
> 
=======
Thanks Robert for the suggestion.  I had renewed hopes this might work, 
but it would not.  After editing the menuentry to use either the 
vga=0x31b or vga=795, the machine booted to a blinking cursor.  It would 
not go beyond that.

Does that help in determining where maybe the problem lies?  Also, I 
thought both linux16 and vga were deprecated, no longer to be used?

Thanks,
Pat

-- 
        ---Zenwalk v6.0--Linux 2.6.28---
         Registered Linux User #225206
"Ever tried Zen computing?"  http://www.zenwalk.org

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Mon, May 04, 2009 at 11:15:13AM -0400, BandiPat wrote:
> Robert Millan wrote:
>> On Sat, May 02, 2009 at 08:03:23PM -0400, BandiPat wrote:
>>> 	linux16	/boot/vmlinuz root=/dev/sda1 ro resume=/dev/sda4 
>>> splash=silent  vga=794
>>
>> This means vga=0x31a, aka 16-bit 1280x1024.  Does 24-bit (vga=0x31b) work?
>>
>> I suspect there's some fuzzy matching here.
>>
> =======
> Thanks Robert for the suggestion.  I had renewed hopes this might work,  
> but it would not.  After editing the menuentry to use either the  
> vga=0x31b or vga=795, the machine booted to a blinking cursor.  It would  
> not go beyond that.

What suitable modes does "vbeinfo" list?

> Does that help in determining where maybe the problem lies?  Also, I  
> thought both linux16 and vga were deprecated, no longer to be used?

"vga" is deprecated, but we don't have a replacement for it yet.

"linux16" will eventually become completely obsolete, but some users
(e.g. you) still need to rely on it, which is why we didn't remove it.

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

Re: [PATCH] Video mode fixes in linux loader

[BandiPat]

Robert Millan wrote:

>> =======
>> Thanks Robert for the suggestion.  I had renewed hopes this might work,  
>> but it would not.  After editing the menuentry to use either the  
>> vga=0x31b or vga=795, the machine booted to a blinking cursor.  It would  
>> not go beyond that.
> 
> What suitable modes does "vbeinfo" list?
> 
===========
A little help here.  Where do you run this "vbeinfo" command to get a 
listing of suitable modes?  I am going to load the vbeinfo.mod in the 
grub.cfg, but what after that to get a listing?

Thanks

Re: [PATCH] Video mode fixes in linux loader

[Felix Zielcke]

Am Montag, den 04.05.2009, 14:16 -0400 schrieb BandiPat:

> A little help here.  Where do you run this "vbeinfo" command to get a 
> listing of suitable modes?  I am going to load the vbeinfo.mod in the 
> grub.cfg, but what after that to get a listing?
> 

Just press `c' to get to commandline and type vbeinfo.
Normally you shouldn't ever need to insmod command modules. GRUB should
load them automatically as long as you have /boot/grub/command.lst.
-- 
Felix Zielcke

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

Btw, I increased the mode list considerably (using documented modes from
Wikipedia).  Chances that your ultra-weird mode of choice is supported are
much greater now.

There's still no fuzzy matching, though.  I'm not sure if we'd want to do
that at all for vga= modes, since we already do it properly in our video
subsystem.

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

Re: [PATCH] Video mode fixes in linux loader

[BandiPat]

Robert Millan wrote:
> Btw, I increased the mode list considerably (using documented modes from
> Wikipedia).  Chances that your ultra-weird mode of choice is supported are
> much greater now.
> 
> There's still no fuzzy matching, though.  I'm not sure if we'd want to do
> that at all for vga= modes, since we already do it properly in our video
> subsystem.
> 
=========
I'll give them a look as soon as I can get something new built.  I'll 
have to admit, I didn't expect you guys to go to so much trouble, since 
the vga= will be going away anyhow.  We can all live with the "fuzzy 
matching" for now, until you get the new video subsystem operating 
properly.  Don't put a lot of time into something that won't be used 
much longer, as we can live with things as they are for the moment.

Thanks for everything.
Pat


-- 
        ---Zenwalk v6.0--Linux 2.6.28---
         Registered Linux User #225206
"Ever tried Zen computing?"  http://www.zenwalk.org

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Mon, May 04, 2009 at 05:59:11PM -0400, BandiPat wrote:
> Robert Millan wrote:
>> Btw, I increased the mode list considerably (using documented modes from
>> Wikipedia).  Chances that your ultra-weird mode of choice is supported are
>> much greater now.
>>
>> There's still no fuzzy matching, though.  I'm not sure if we'd want to do
>> that at all for vga= modes, since we already do it properly in our video
>> subsystem.
>>
> =========
> I'll give them a look as soon as I can get something new built.  I'll  
> have to admit, I didn't expect you guys to go to so much trouble, since  
> the vga= will be going away anyhow.  We can all live with the "fuzzy  
> matching" for now, until you get the new video subsystem operating  
> properly.  Don't put a lot of time into something that won't be used  
> much longer, as we can live with things as they are for the moment.

Maybe you're right :-/

Hopefuly this will be less of a problem when we can set it from a variable
using a sane interface...

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

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Sat, May 02, 2009 at 05:32:01PM +0200, Robert Millan wrote:
> 
> 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.

Committed.

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

Re: [PATCH] Video mode fixes in linux loader

[Isaac Dupree]

Robert Millan wrote:
> > > > Some
> > > > kernels may not support VESA modes at all.
> > >
> > > I don't think this is applicable;  all modern versions of Linux include
> > > vesa modesetting in its 16-bit entry code, and older versions are
> > > already detected by the new loader (user is prompted to use linux16).
> >
> > I can disable CONFIG_FB, and then the screen remains blank until X
> > starts.  It's entirely possible that some distros don't enable CONFIG_FB
> > to save memory, and I don't always enable it in the kernels I configure
> > myself.
>
> Makes sense for official GRUB.
>
> However, I'd still like to add a macro check that can be enabled on distros
> that ship Linux builds with CONFIG_FB and want to enable seamless mode
> transition (this will be the case for e.g. Debian).

Can we have GRUB2 not be distro-dependent, because I often depend on it to 
launch multiple different distros (and I'd rather not worry too much about 
which distro installed my grub2)?  It [i.e., whether to use seamless mode 
transition] can be a setting in the grub.cfg if necessary.

-Isaac

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Sun, May 03, 2009 at 09:18:02AM -0400, Isaac Dupree wrote:
> Robert Millan wrote:
> > > > > Some
> > > > > kernels may not support VESA modes at all.
> > > >
> > > > I don't think this is applicable;  all modern versions of Linux include
> > > > vesa modesetting in its 16-bit entry code, and older versions are
> > > > already detected by the new loader (user is prompted to use linux16).
> > >
> > > I can disable CONFIG_FB, and then the screen remains blank until X
> > > starts.  It's entirely possible that some distros don't enable CONFIG_FB
> > > to save memory, and I don't always enable it in the kernels I configure
> > > myself.
> >
> > Makes sense for official GRUB.
> >
> > However, I'd still like to add a macro check that can be enabled on distros
> > that ship Linux builds with CONFIG_FB and want to enable seamless mode
> > transition (this will be the case for e.g. Debian).
> 
> Can we have GRUB2 not be distro-dependent,

We aren't talking about making GRUB distro-dependant, only about what is the
default behaviour when user didn't specify any.

Official GRUB, as discussed, will default to text mode.  I don't see why
downstream distributors would need to do the same, since it's so easy to
override in case a user needs that.

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

Re: [PATCH] Video mode fixes in linux loader

[Pavel Roskin]

On Mon, 2009-04-13 at 16:16 +0200, Robert Millan wrote:
> Hi Pavel,
> 
> On Mon, Apr 06, 2009 at 11:34:03AM -0400, Pavel Roskin wrote:
> > Hello!
> > 
> > This is an attempt to fix all issues with the video mode handling in the
> > new Linux loader.
> 
> Please, in general, when you modify code that has been actively worked on
> by someone, try to get that person involved.

I tried, but probably not hard enough.  Sorry.

> > The default VGA mode is now GRUB_LINUX_VID_MODE_NORMAL, not the mode of
> > the kernel we tried to load before.
> 
> Ok, BUT if we're already in vesa mode, and we know it works (since we're using
> it), there's no point in wasting time only to get a worse mode.

Maybe we should introduce another macro, e.g.
GRUB_LINUX_VID_MODE_CURRENT, recognize it in the loader and default to
it.

> We should just make sure subsequent calls to "linux" command override the
> previous one.

Yes, failed loading of one kernel should not affect loading of another
kernel.  That was the most annoying bug, and I hope it's fixed now.

> > Finally, "vga=ask" is now recognized.
> 
> With the new loader, Linux' 16-bit entry code is no longer responsible for
> setting vesa modes, therefore vga=ask can't work.  There's no point in
> recognizing it (except to warn the user).

I agree.  Generally, "vga=random_string" causes an error now.  Perhaps
any unrecognized values of "vga" should cause a warning, not an error.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Video mode fixes in linux loader

[Robert Millan]

On Mon, Apr 13, 2009 at 11:16:46AM -0400, Pavel Roskin wrote:
> On Mon, 2009-04-13 at 16:16 +0200, Robert Millan wrote:
> > Please, in general, when you modify code that has been actively worked on
> > by someone, try to get that person involved.
> 
> I tried, but probably not hard enough.  Sorry.

No problem :-)

> > Ok, BUT if we're already in vesa mode, and we know it works (since we're using
> > it), there's no point in wasting time only to get a worse mode.
> 
> Maybe we should introduce another macro, e.g.
> GRUB_LINUX_VID_MODE_CURRENT, recognize it in the loader and default to
> it.

If we're going to get creative and add new options that weren't present in
the legacy interface, then I'd really prefer if we design a new one, and
better a kernel-independant one like an env variable (we discussed this in
another thread, but didn't agree on which name to use yet).

> Yes, failed loading of one kernel should not affect loading of another
> kernel.  That was the most annoying bug, and I hope it's fixed now.

Yeah I don't see anything wrong with this situation now, but I could be
missing something, since there are many ways to "fail".

> > > Finally, "vga=ask" is now recognized.
> > 
> > With the new loader, Linux' 16-bit entry code is no longer responsible for
> > setting vesa modes, therefore vga=ask can't work.  There's no point in
> > recognizing it (except to warn the user).
> 
> I agree.  Generally, "vga=random_string" causes an error now.  Perhaps
> any unrecognized values of "vga" should cause a warning, not an error.

Probably better when it comes to headless machines, yes...

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