Re: [PATCH] VBE support

[Alexander Thiel]

Hello,

here is the VBE patch against the current CVS. I did my best to write a 
conforming ChangeLog entry. Let me know if there is something wrong - it's 
the first time that I contribute to an open source project.
I will send a patch that fills in the remaining fields in the multiboot info 
header later.

What do you think the format of this 'videomode' builtin should be? I wanted 
to pass only the VBE mode number, but it seems that this is not general 
enough.

I am also thinking about handling graphics requests where some fields are set 
to zero: choosing an appropriate mode would require quite an amount of code 
and I would not like to squeeze all of it into load_image which is quite hard 
to read already. Don't you think this function needs some clean-up to remove 
all these nested if/else constructs? If you want to add something new it's 
quite hard to see where it should go.

Regards,

        Alex

On Tuesday 13 November 2001 17:45, you wrote:
> At Tue, 13 Nov 2001 12:37:29 +0100,
>
> Alexander Thiel wrote:
> > as the latest version of GRUB did not support VBE graphics as decribed in
> > the multiboot specification, but I wanted it for my own toy kernel, I
> > have added it myself. I could not find any hint about ongoing work for
> > VBE support in the mailing list archive, so I decided to post my patch
> > (against grub-0.90). It moves some existing code around and adds some new
> > chunks. Here is a summary of the changes:
>
> Thanks for your contribution very much! Could you write ChangeLog
> entries, as descibed in the GNU Coding Standards?
>
> > As of yet, there is only the default text mode supported, and the info on
> > the protected mode interface is not available. I am also thinking about
> > adding a builtin function 'setvbe' to explicitly set the mode passed to
> > the kernel.
>
> Agreed. I think "videomode" would be more appropriate, since we may
> add support for other modes than VBE later.
>
> Okuji

vbe.diff
Description: Text Data