[PATCH] Improve handling of "keep" in gfxpayload

[PATCH] Improve handling of "keep" in gfxpayload

[Colin Watson]

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

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  <cjwatson@ubuntu.com>

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

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

[-- Attachment #2: grub2-gfxpayload-keep.diff --]
[-- Type: text/x-diff, Size: 1912 bytes --]

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;

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.

Nice catch, I wasn't aware an extra flag was needed.

> Note that I'm explicitly not enabling GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> in the Ubuntu grub2 package.

Actually, the Debian package stopped enabling it too.  I had my reservations
when "keep" was introduced, but now I see that Vladimir's idea is a much
better option than hardcoding in the binary.

I think I'll remove GRUB_ASSUME_LINUX_HAS_FB_SUPPORT.  It seems that nobody
is going to use it, so all it does is add complexity.  This will also make
your patch simpler.

Is there any objection to 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] Improve handling of "keep" in gfxpayload

[Colin Watson]

On Mon, Aug 10, 2009 at 01:49:40PM +0200, Robert Millan wrote:
> 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.
> 
> Nice catch, I wasn't aware an extra flag was needed.

It also seems that doing this right is tricky on the kernel side (see
https://lists.ubuntu.com/archives/kernel-team/2009-August/006773.html
and thread), so I probably won't be turning this on by default yet in
our configuration - but of course if you explicitly say "keep" then you
should get what you asked for.

> > Note that I'm explicitly not enabling GRUB_ASSUME_LINUX_HAS_FB_SUPPORT
> > in the Ubuntu grub2 package.
> 
> Actually, the Debian package stopped enabling it too.  I had my reservations
> when "keep" was introduced, but now I see that Vladimir's idea is a much
> better option than hardcoding in the binary.
> 
> I think I'll remove GRUB_ASSUME_LINUX_HAS_FB_SUPPORT.  It seems that nobody
> is going to use it, so all it does is add complexity.  This will also make
> your patch simpler.
> 
> Is there any objection to that?

Not from me at least.

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Robert Millan]

On Mon, Aug 10, 2009 at 12:58:35PM +0100, Colin Watson wrote:
> On Mon, Aug 10, 2009 at 01:49:40PM +0200, Robert Millan wrote:
> > 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.
> > 
> > Nice catch, I wasn't aware an extra flag was needed.
> 
> It also seems that doing this right is tricky on the kernel side (see
> https://lists.ubuntu.com/archives/kernel-team/2009-August/006773.html
> and thread), so I probably won't be turning this on by default yet in
> our configuration - but of course if you explicitly say "keep" then you
> should get what you asked for.

I read that thread.  Note that in the future, GRUB will include video
drivers, and when those are ready we don't have to use vesa, so I wouldn't
spend a lot of effort with this vesa->other video driver transition that
according to Matthew can be complicated.

-- 
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] Improve handling of "keep" in gfxpayload

[Colin Watson]

On Mon, Aug 10, 2009 at 05:15:41PM +0200, Robert Millan wrote:
> On Mon, Aug 10, 2009 at 12:58:35PM +0100, Colin Watson wrote:
> > It also seems that doing this right is tricky on the kernel side (see
> > https://lists.ubuntu.com/archives/kernel-team/2009-August/006773.html
> > and thread), so I probably won't be turning this on by default yet in
> > our configuration - but of course if you explicitly say "keep" then you
> > should get what you asked for.
> 
> I read that thread.  Note that in the future, GRUB will include video
> drivers, and when those are ready we don't have to use vesa, so I wouldn't
> spend a lot of effort with this vesa->other video driver transition that
> according to Matthew can be complicated.

I don't think it's relevant to Matthew's argument what GRUB happens to
use to implement the graphical mode. The only way that Linux can write
text to the screen in graphical mode is if it has a framebuffer driver
to do so, which needs some kind of substrate (be that VESA or something
smarter implemented by KMS); it doesn't matter what mode was previously
programmed by GRUB.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

>
> I don't think it's relevant to Matthew's argument what GRUB happens to
> use to implement the graphical mode. The only way that Linux can write
> text to the screen in graphical mode is if it has a framebuffer driver
> to do so, which needs some kind of substrate (be that VESA or something
> smarter implemented by KMS); it doesn't matter what mode was previously
> programmed by GRUB.
Framebuffer console works just fine with no KMS if video mode is
already set and video parameters passed. At least technically. Just
that you won't be able to do fancy stuff like acceleration before you
load complete drivers. An example of OS which boots in such mode is
OSX - it continues in the mode set by booter and uses the video for
showing either scroll wheel or text.
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Colin Watson]

On Mon, Aug 10, 2009 at 06:27:04PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Framebuffer console works just fine with no KMS if video mode is
> already set and video parameters passed. At least technically.

Feel free to show me what I'm doing wrong, but it demonstrably does not
work right now and right here, and Matthew Garrett (whose opinion I
value very highly when it comes to kernelspace) says that the code
necessary to make this work does not exist in Linux right now.

> Just that you won't be able to do fancy stuff like acceleration before
> you load complete drivers. An example of OS which boots in such mode
> is OSX - it continues in the mode set by booter and uses the video for
> showing either scroll wheel or text.

That means it's theoretically possible, certainly, but that wasn't my
point - the fact is that it doesn't work with Linux today, and whether
OS X can do it is not really directly relevant to that.

Regards,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

On Mon, Aug 10, 2009 at 6:41 PM, Colin Watson<cjwatson@ubuntu.com> wrote:
> On Mon, Aug 10, 2009 at 06:27:04PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> Framebuffer console works just fine with no KMS if video mode is
>> already set and video parameters passed. At least technically.
>
> Feel free to show me what I'm doing wrong, but it demonstrably does not
> work right now and right here, and Matthew Garrett (whose opinion I
> value very highly when it comes to kernelspace) says that the code
> necessary to make this work does not exist in Linux right now.
>

AFAIK efifb does exactly that. Try adding efifb keyword to linux command line.

>> Just that you won't be able to do fancy stuff like acceleration before
>> you load complete drivers. An example of OS which boots in such mode
>> is OSX - it continues in the mode set by booter and uses the video for
>> showing either scroll wheel or text.
>
> That means it's theoretically possible, certainly, but that wasn't my
> point - the fact is that it doesn't work with Linux today, and whether
> OS X can do it is not really directly relevant to that.
>
> Regards,
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Colin Watson]

On Mon, Aug 24, 2009 at 04:26:09PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> On Mon, Aug 10, 2009 at 6:41 PM, Colin Watson<cjwatson@ubuntu.com> wrote:
> > On Mon, Aug 10, 2009 at 06:27:04PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> >> Framebuffer console works just fine with no KMS if video mode is
> >> already set and video parameters passed. At least technically.
> >
> > Feel free to show me what I'm doing wrong, but it demonstrably does not
> > work right now and right here, and Matthew Garrett (whose opinion I
> > value very highly when it comes to kernelspace) says that the code
> > necessary to make this work does not exist in Linux right now.
> 
> AFAIK efifb does exactly that. Try adding efifb keyword to linux command line.

Right, that's fine once the kernel has woken up to some extent, but it
doesn't load in early boot when it can be important to be able to see
output (drivers only get initialised once the kernel gets up far enough
to call driver_init) and it doesn't address the problem of automatically
unloading the boot console driver later to put a better framebuffer in
place once the kernel has scanned the PCI bus and so on. Besides,
hardcoding the framebuffer module to load in grub.cfg would be bad.

Yes, there are various ways in which this can be hacked up for a given
environment, as long as you don't mind some warts. However, making it
work with no user configuration (as it does right now, aside from the
multiple video mode transitions, if GRUB doesn't do anything special)
really does need actual kernel work. Look at where CONFIG_VGA_CONSOLE is
handled in arch/x86/kernel/setup.c for an example of the sort of thing I
mean.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

On Mon, Aug 24, 2009 at 6:38 PM, Colin Watson<cjwatson@ubuntu.com> wrote:
> On Mon, Aug 24, 2009 at 04:26:09PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> On Mon, Aug 10, 2009 at 6:41 PM, Colin Watson<cjwatson@ubuntu.com> wrote:
>> > On Mon, Aug 10, 2009 at 06:27:04PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> >> Framebuffer console works just fine with no KMS if video mode is
>> >> already set and video parameters passed. At least technically.
>> >
>> > Feel free to show me what I'm doing wrong, but it demonstrably does not
>> > work right now and right here, and Matthew Garrett (whose opinion I
>> > value very highly when it comes to kernelspace) says that the code
>> > necessary to make this work does not exist in Linux right now.
>>
>> AFAIK efifb does exactly that. Try adding efifb keyword to linux command line.
>
> Right, that's fine once the kernel has woken up to some extent, but it
> doesn't load in early boot when it can be important to be able to see
I haven't done tests but efifb uses the data supplied by grub and
doesn't try to modeswitch (the corresponding EFI functions are
unavailable once kernel is loaded) so it should work on any firmware
even if name suggests otherwise.
Additionally after kernel is loaded EFI test functions can'tbe used so
efifb is the only way to show diagnostic messages on EFI so I expect
it to be inited early
-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Vladimir 'phcoder' Serbinenko]

> I think I'll remove GRUB_ASSUME_LINUX_HAS_FB_SUPPORT.  It seems that nobody
> is going to use it, so all it does is add complexity.  This will also make
> your patch simpler.
>
> Is there any objection to that?
I'm ok with that. Generally I think that whenever we have the same
configuration available at build and config time we should remove
build time as it adds complexity for nothing. The only exception I
could think of is disabling some features to make code smaller.
However I hope we won't have to introduce cherrypicking compiling when
user is required to choose core features - it's simply too easy to get
wrong and hard to debug
>
> --
> 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."
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Vladimir 'phcoder' Serbinenko]

> 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 ...
What is the linux behaviour with 0x0F04? Does it just keep the mode?
What if the same mode is passed as a value? Does linux redoes the
modesetting? If 0x0F04 works ok I would prefer to always pass it when
kernel is booted in graphical mode. VESA mode numbers are an artifact
and when grub2 has its own graphical drivers it won't correspond to
anything.
>
> 2009-08-10  Colin Watson  <cjwatson@ubuntu.com>
>
>        * 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.
>
> Thanks,
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Colin Watson]

On Mon, Aug 10, 2009 at 02:05:15PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Colin Watson wrote:
> > 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 ...
> 
> What is the linux behaviour with 0x0F04? Does it just keep the mode?

Yes, it completely skips modesetting.

> What if the same mode is passed as a value? Does linux redoes the
> modesetting?

Yes, which permits it to display text since now it's set up a linear
framebuffer.

> If 0x0F04 works ok I would prefer to always pass it when kernel is
> booted in graphical mode. VESA mode numbers are an artifact and when
> grub2 has its own graphical drivers it won't correspond to anything.

It's not the best solution; it's just closer than what's there right
now. The problem with 0x0F04 is that the kernel doesn't know how to
display text until it brings up its own framebuffer, and we need to
figure out how to tell vesafb to come up early so that the kernel can
display text if necessary. At the moment it seems that (a) either 0x0F04
or a VESA mode number is appropriate if GRUB is booting in graphical
mode; (b) kernel work is needed to do better; (c) as such it is probably
not appropriate for GRUB to boot Linux in graphical mode by default just
yet.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

> Yes, it completely skips modesetting.
>
>> What if the same mode is passed as a value? Does linux redoes the
>> modesetting?
>
> Yes, which permits it to display text since now it's set up a linear
> framebuffer.
Why can't linux use linear framebuffer for its framebuffer console?
>
>> If 0x0F04 works ok I would prefer to always pass it when kernel is
>> booted in graphical mode. VESA mode numbers are an artifact and when
>> grub2 has its own graphical drivers it won't correspond to anything.
>
> It's not the best solution; it's just closer than what's there right
> now. The problem with 0x0F04 is that the kernel doesn't know how to
> display text until it brings up its own framebuffer, and we need to
> figure out how to tell vesafb to come up early so that the kernel can
> display text if necessary. At the moment it seems that (a) either 0x0F04
> or a VESA mode number is appropriate if GRUB is booting in graphical
> mode; (b) kernel work is needed to do better; (c) as such it is probably
> not appropriate for GRUB to boot Linux in graphical mode by default just
> yet.
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Colin Watson]

On Mon, Aug 10, 2009 at 02:38:42PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> Colin Watson wrote:
> > Yes, it completely skips modesetting.
> >
> >> What if the same mode is passed as a value? Does linux redoes the
> >> modesetting?
> >
> > Yes, which permits it to display text since now it's set up a linear
> > framebuffer.
> 
> Why can't linux use linear framebuffer for its framebuffer console?

It's probably easiest if you just catch up on the thread on the Ubuntu
kernel list that I referenced earlier, where this is covered. Matthew
Garrett's pretty authoritative on this.

  https://lists.ubuntu.com/archives/kernel-team/2009-August/006773.html

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Robert Millan]

On Mon, Aug 10, 2009 at 02:05:15PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> What is the linux behaviour with 0x0F04? Does it just keep the mode?
> What if the same mode is passed as a value? Does linux redoes the
> modesetting? If 0x0F04 works ok I would prefer to always pass it when
> kernel is booted in graphical mode. VESA mode numbers are an artifact
> and when grub2 has its own graphical drivers it won't correspond to
> anything.

I might be missing something, but AFAICT VESA mode numbers are only passed
by the user in the "vga=" option.

There's also the `vid_mode' parameter in Linux header, but it's only used by
legacy code, and with the 32-bit boot protocol Linux doesn't read it.

Does 0x0F04 really do something when used in combination with Linux' vesafb?

-- 
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] Improve handling of "keep" in gfxpayload

[Colin Watson]

On Mon, Aug 10, 2009 at 05:27:25PM +0200, Robert Millan wrote:
> On Mon, Aug 10, 2009 at 02:05:15PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> > What is the linux behaviour with 0x0F04? Does it just keep the mode?
> > What if the same mode is passed as a value? Does linux redoes the
> > modesetting? If 0x0F04 works ok I would prefer to always pass it when
> > kernel is booted in graphical mode. VESA mode numbers are an artifact
> > and when grub2 has its own graphical drivers it won't correspond to
> > anything.
> 
> I might be missing something, but AFAICT VESA mode numbers are only passed
> by the user in the "vga=" option.
> 
> There's also the `vid_mode' parameter in Linux header, but it's only used by
> legacy code, and with the 32-bit boot protocol Linux doesn't read it.

Hmm. You seem to be right, on investigation (I'd missed the fact that
the 32-bit boot protocol skips a whole bunch of initialisation code),
but in that case I'm thoroughly confused about why setting it made any
difference at all in my tests. Back to the drawing board, I guess.

(It's not actually quite true that vid_mode isn't used *at all* with the
32-bit boot protocol; it's still saved for use with ACPI sleep.)

> Does 0x0F04 really do something when used in combination with Linux' vesafb?

It seems to be 16-bit boot protocol only. I withdraw this patch for the
time being as I'm obviously confused.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

>
> Hmm. You seem to be right, on investigation (I'd missed the fact that
> the 32-bit boot protocol skips a whole bunch of initialisation code),
> but in that case I'm thoroughly confused about why setting it made any
> difference at all in my tests. Back to the drawing board, I guess.
>
> (It's not actually quite true that vid_mode isn't used *at all* with the
> 32-bit boot protocol; it's still saved for use with ACPI sleep.)
>
>> Does 0x0F04 really do something when used in combination with Linux' vesafb?
>
> It seems to be 16-bit boot protocol only. I withdraw this patch for the
> time being as I'm obviously confused.
If these numbers are still needed it needs to be uniform between
gfxpayload=1024x768 or gfxpayload=keep + gfxmode=1024x768. Derive the
number from video mode information if you really need it
>
> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Colin Watson]

On Mon, Aug 10, 2009 at 10:01:06PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> If these numbers are still needed it needs to be uniform between
> gfxpayload=1024x768 or gfxpayload=keep + gfxmode=1024x768. Derive the
> number from video mode information if you really need it

I couldn't find terribly clear documentation of "keep". Isn't the point
that if you say "keep" then the mode is carried over to the booted
kernel, and if you don't then it isn't? If my understanding is correct
then that would mean that there's no point setting up vid_mode to match
gfxpayload unless "keep" is given.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

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

[Vladimir 'phcoder' Serbinenko]

On Mon, Aug 10, 2009 at 10:16 PM, Colin Watson<cjwatson@ubuntu.com> wrote:
> On Mon, Aug 10, 2009 at 10:01:06PM +0200, Vladimir 'phcoder' Serbinenko wrote:
>> If these numbers are still needed it needs to be uniform between
>> gfxpayload=1024x768 or gfxpayload=keep + gfxmode=1024x768. Derive the
>> number from video mode information if you really need it
>
> I couldn't find terribly clear documentation of "keep". Isn't the point
> that if you say "keep" then the mode is carried over to the booted
> kernel, and if you don't then it isn't? If my understanding is correct
> then that would mean that there's no point setting up vid_mode to match
> gfxpayload unless "keep" is given.
>
"keep" is a GRUB directive and mean "try to use the mode already set
to boot kernel". Kernel has no need to know if video mode was set
specifically for it or kept from grub operation, only what the mode
is. So kernel shouldn't be able to distinguish between:
insmod vbe
set gfxmode=1024x768
terminal_output.gfxterm
set gfxpayload=keep
linux /vmlinuz

and

insmod vbe
set gfxpayload=1024x768
linux /vmlinuz

> --
> Colin Watson                                       [cjwatson@ubuntu.com]
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

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

[Robert Millan]

On Mon, Aug 10, 2009 at 05:18:42PM +0100, Colin Watson wrote:
> 
> (It's not actually quite true that vid_mode isn't used *at all* with the
> 32-bit boot protocol; it's still saved for use with ACPI sleep.)

In this case, I suppose we should set it.  Currently we don't, because I was
under the impression that it wasn't used.

-- 
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] 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  <cjwatson@ubuntu.com>
> 
>         * 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."

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

[Colin Watson]

On Mon, Nov 09, 2009 at 06:18:41PM +0100, Robert Millan wrote:
> 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).

Given the kernel issues with it, I've withdrawn this patch for the time
being. There may be cunning things we can do in future on systems that
support kernel modesetting, but it's not worth it right now.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]