[LinuxBIOS] VGA support for Geode GX1/CS5530

Uwe Hermann uwe at hermann-uwe.de
Sun Oct 7 23:13:13 CEST 2007


On Sat, Oct 06, 2007 at 12:21:08PM +0200, Juergen Beisert wrote:
> > Nice! So this is in-LinuxBIOS VGA support? I.e. without running any VGA
> > option ROM blob in an emulator?
> 
> This "driver" really knows the hardware, so it does not need any help from any 
> kind of SMM.

Very nice!

 
> > I'll give it a try on my CS5530 box in a few days (no access right now).
> 
> Visit my BCOM WINNET100 Tutorial, scroll to the buttom of this document and 
> download the Board Support Package. Extract it, you will need some patches to 
> use the VGA console feature within Linux. Also for the Linux kernel you will 
> need a specific console driver for the GX1, as the generic driver expects 
> some help from the SMM. See patches/linux-2.6.22/generic for the whole patch 
> stack, apply it to a fresh 2.6.22 kernel and build it with the 
> igel316-kernelconfig.target as its .config. Also if you like to run X on it, 

Is this scheduled to go into the mainline kernel one day? Would be great!


> you will need a special driver. You can find it in the BSP in the  
> local_src/xf86-video-geode_gx1 directory.

Same here, will it be merged into xorg?


> > > +#if CONFIG_GX1_VIDEO == 1
> > > +/*
> > > + * Some register descriptions that are no listed in cpu/amd/gx1def.h
> > > + */
> >
> > Would it make sense to add them to cpu/amd/gx1def.h?
> > Probably not as they're not CPU related but VGA related?
> 
> To be discussed. As this CPU and the chipset is one silicon (Note: only VGA 
> interface support is in the companion chip, but the graphic feature is part 
> of the Geode device!) you can put both defines together.

Hm, ok, it's fine either way, I guess.


> > > +/*
> > > + * what colour depth should be used as default (in bpp)
> > > + * Note: Currently no other value than 16 is supported
> > > + */
> > > +#define COLOUR_DEPTH 16
> >
> > Maybe this should be a user-visible config-option in Confib.lb too?
> 
> Makes no sense yet, as my routines do not support the 8 bit CLUT mode. And the 
> Geode hardware only supports 16 and 8 bit mode. Nothing else.

Yep, ok. Should be a config option as soon as 8 bit CLUT mode works.


> > > +/*
> > > + * Support for a few basic video modes
> > > + * Note: all modes only for CRT. The flatpanel feature is
> > > + * not supported here (due to the lack of hardware to test)
> > > + */
> > > +struct video_mode {
> > > +	int pixel_clock;		/*<< pixel clock in Hz */
> >
> > What is the '/*<<' supposed to mean?
> 
> Doxygen style. I like it, but I was not sure if it will be accepted in LBv2. 
> But it seems I forgot it to remove...

Ah, so another way to write /** Foo */ ? If so, let's please use the
standard /** Foo */ notation as in the rest of the code.


> > > +	u32 val;
> > > +
> > > +	val = mode->sync_pol;
> > > +	val <<= 8;
> >
> > Why not this?
> >
> >   val = mode->sync_pol << 8;
> >
> > Or maybe even
> >
> >   u32 val = mode->sync_pol << 8;
> 
> Hmmm, mode->sync_pol is an signed int. So shift operations are a bad idea?

Hm, maybe. But why is sync_pol not u32 (or so) in the first place then?
Does it _have_ to be signed?

 
> > > +	writel(val | 0x0020002F, io_base + CS5530_DISPLAY_CONFIG);
> > > +}
> > > +
> > > +/*
> > > + * This bitmap file must provide:
> > > + * int width: pixel count in one line
> > > + * int height: line count
> > > + * int colours: ount of used colour
> > > + * unsigned long colour_map[]: RGB 565 colours to be used
> > > + * unsigned char bitmap[]: index per pixel into colour_map[],
> > > width*height pixels + */
> > > +#include "bitmap.c"
> >
> > Should be a bit more configurable later (specify filename in Config.lb
> > or so). In v3 this should probably be handled in Kconfig.
> 
> Yeees. This bitmap is a gimmick only. I was sure everyone would reject it on 
> this list.

No, not rejected, it's fine. It should just be a config option.

 
> > > Index: LinuxBIOSv2/src/southbridge/amd/cs5530/bitmap.c
> > > ===================================================================
> > > --- /dev/null
> > > +++ LinuxBIOSv2/src/southbridge/amd/cs5530/bitmap.c
> > > @@ -0,0 +1,304 @@
> > > +/* do not edit
> > > +This is an image of size 51 x 60 with 234 colours */
> >
> > How was this image generated? What's the source format?
> >
> > Please also attach a license to it if you created it. If you didn't we
> > must get permission from the author, I guess (and/or state the license
> > which applies to it).
> 
> Gimmick only. This graphic file is autogenerated with a small program that 
> converts xpm into C code.

Doesn't gimp have a similar option, too? If your code could handle the
output of that gimp plugin (it's shipped per default, I think) that
would be a great and easy way for users to create their own icons
without requiring extra command line tools.


> This one shows one of the icons from the linuxbios 
> website (one of the chips with the penguin in it). So I have no clue what 
> kind of license this file should have.

The small icons in the wiki were created by me and are GPL'd (version 2
or later). I still don't know the license of the penguin logo (which is
part of one of the icons) but the rest is definately all GPL'd.


Thanks, Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20071007/19e5abc4/attachment.sig>


More information about the coreboot mailing list