Re: [PATCH] Improve handling of "keep" in gfxpayload

[Robert Millan]

On Mon, Aug 10, 2009 at 12:01:02PM +0100, Colin Watson wrote:
> If the user set "keep" in gfxpayload, as I understand it, that indicates
> that they want the graphical mode set by GRUB to persist through to the
> kernel. In order for this to actually work with Linux, we need to set up
> the vid_mode boot parameter to indicate that it should keep the current
> video mode, otherwise it'll just reset the mode to text at boot and
> nothing much is gained. This patch fixes that.
> 
> Note that I'm explicitly not enabling GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> in the Ubuntu grub2 package. I can understand why support for it was
> added, and I'd be willing to assume that the Ubuntu kernel has the
> necessary support, but I'm not willing to assume that every Linux kernel
> booted by the Ubuntu grub2 build has the necessary support; that seems
> one assumption too far for me. As such I'm interested in making this
> work properly with run-time configuration as well as build-time
> configuration.
> 
> This doesn't quite work perfectly yet. It's better than before - I've
> tested this, and if everything works properly then the result is a
> smooth zero-flicker transition, which is wonderful. However, if
> something goes wrong before the kernel starts a framebuffer then it has
> no way to display any text at all, and it doesn't seem to start one
> until relatively late for me. It may be that the next step here is to
> try to explicitly tell the kernel to set the correct VESA mode rather
> than using 0x0F04, but I thought I'd send this patch anyway in the
> meantime ...
> 
> 2009-08-10  Colin Watson  <address@hidden>
> 
>         * include/grub/i386/linux.h (GRUB_LINUX_VID_MODE_CURRENT): New
>         macro.
>         * loader/i386/linux.c (grub_linux_boot): If gfxpayload starts
>         with "keep", or if GRUB_ASSUME_LINUX_HAS_FB_SUPPORT is defined,
>         then set the vid_mode boot parameter to
>         GRUB_LINUX_VID_MODE_CURRENT.

Hi Colin,

We don't have GRUB_ASSUME_LINUX_HAS_FB_SUPPORT anymore.  Does this patch
still make sense?  (or some part of it?  I suspect GRUB_LINUX_VID_MODE_CURRENT
is still useful).

> diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/include/grub/i386/linux.h 
> grub2-1.96+20090725.new/include/grub/i386/linux.h
> --- grub2-1.96+20090725/include/grub/i386/linux.h     2009-07-06 
> 03:10:57.000000000 +0100
> +++ grub2-1.96+20090725.new/include/grub/i386/linux.h 2009-08-08 
> 19:45:10.000000000 +0100
> @@ -39,6 +39,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_CURRENT  0x0F04
>  #define GRUB_LINUX_VID_MODE_VESA_START       0x0300
>  
>  #define GRUB_LINUX_SETUP_MOVE_SIZE   0x9100
> diff -Nur -x '*.orig' -x '*~' grub2-1.96+20090725/loader/i386/linux.c 
> grub2-1.96+20090725.new/loader/i386/linux.c
> --- grub2-1.96+20090725/loader/i386/linux.c   2009-08-08 19:31:00.000000000 
> +0100
> +++ grub2-1.96+20090725.new/loader/i386/linux.c       2009-08-08 
> 19:45:09.000000000 +0100
> @@ -446,6 +446,7 @@
>    int e820_num;
>    grub_err_t err = 0;
>    char *modevar, *tmp;
> +  int keep_mode = 0;
>  
>    params = real_mode_mem;
>  
> @@ -460,11 +461,19 @@
>        if (! tmp)
>       return grub_errno;
>        grub_sprintf (tmp, "%s;" DEFAULT_VIDEO_MODE, modevar);
> +#ifndef GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> +      if (grub_memcmp (modevar, "keep", sizeof ("keep")) == 0
> +       || grub_memcmp (modevar, "keep,", sizeof ("keep,") - 1) == 0
> +       || grub_memcmp (modevar, "keep;", sizeof ("keep;") - 1) == 0)
> +     keep_mode = 1;
> +#endif
>        err = grub_video_set_mode (tmp, 0);
>        grub_free (tmp);
>      }
> -#ifndef GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
>    else
> +#ifdef GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> +    keep_mode = 1;
> +#else
>      err = grub_video_set_mode (DEFAULT_VIDEO_MODE, 0);
>  #endif
>  
> @@ -474,6 +483,8 @@
>        grub_printf ("Booting however\n");
>        grub_errno = GRUB_ERR_NONE;
>      }
> +  else if (keep_mode)
> +    params->vid_mode = GRUB_LINUX_VID_MODE_CURRENT;
>  
>    if (! grub_linux_setup_video (params))
>      params->have_vga = GRUB_VIDEO_TYPE_VLFB;

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