[SeaBIOS] [PATCH 3/3]

[Kevin O'Connor]

On Sat, Jan 07, 2012 at 12:28:55AM +0100, Nils wrote:
> Add the GX2 functionality.
> Improve Kconfig VGA options.
> Conditionally build VGA bios rom if selected in Kconfig. 

Thanks.

I'd like to see Kconfig select whether or not the vgabios is built.
However, that should really be in a patch separate from the addition
of GX2 support.

Also, I'd prefer to avoid #ifdefs where possible.

The first two patches look good to me.

[...]
> +ifneq ($(CONFIG_NO_VGABIOS),y)
> +all: $(OUT) $(OUT)bios.bin $(OUT)vgabios.bin
> +else
>  all: $(OUT) $(OUT)bios.bin
> +endif

all-y: $(OUT) $(OUT)bios.bin
all-$(CONFIG_BUILD_VGABIOS) += $(OUT)vgabios.bin

[...]
>  SRCVGA=src/output.c src/util.c vgasrc/vgabios.c vgasrc/vgafb.c \
>      vgasrc/vgatables.c vgasrc/vgafonts.c vgasrc/vbe.c \
> -    vgasrc/stdvga.c vgasrc/clext.c vgasrc/bochsvga.c vgasrc/geodevga.c
> +    vgasrc/stdvga.c vgasrc/clext.c vgasrc/bochsvga.c
> +
> +ifeq ($(CONFIG_VGA_GEODEGX2),y)
> +SRCVGA +=  vgasrc/geodevga.c
> +endif
> +ifeq ($(CONFIG_VGA_GEODELX),y)
> +SRCVGA +=  vgasrc/geodevga.c
> +endif

Lets just build vgasrc/geodevga.c unconditionally - the
compiler/linker will strip it out if it isn't configured.

[...]
> --- vgasrc/geodevga.c
> +++ vgasrc/geodevga.c
> @@ -87,7 +87,12 @@
>      int ret=0;
>      union u64_u32_u val;
>  
> +#if (CONFIG_VGA_GEODEGX2)
> +    val=geode_msrRead(GLIU0_P2D_BM_4);
> +#endif
> +#if (CONFIG_VGA_GEODELX)
>      val=geode_msrRead(MSR_GLIU0_BASE4);
> +#endif

       if (CONFIG_VGA_GEODEGX2)
           val=geode_msrRead(GLIU0_P2D_BM_4);
       else
           val=geode_msrRead(MSR_GLIU0_BASE4);

[...]
> +#if (CONFIG_VGA_GEODEGX2)
> +    #define VP_MSR_CONFIG               0xc0002001  /* GX2 */
> +#endif
> +#if (CONFIG_VGA_GEODELX)
> +    #define VP_MSR_CONFIG               0x48002001  /* LX */
> +#endif

#define VP_MSR_CONFIG_GX2               0xc0002001  /* GX2 */
#define VP_MSR_CONFIG_LX                0x48002001  /* LX */

and update the code to use an if statement?

[...]
> --- vgasrc/vgahw.h
> +++ vgasrc/vgahw.h
> @@ -22,6 +22,8 @@
>          return clext_init();
>      if (CONFIG_VGA_BOCHS)
>          return bochsvga_init();
> +    if (CONFIG_VGA_GEODEGX2)
> +        return geodevga_init();
>      if (CONFIG_VGA_GEODELX)
>          return geodevga_init();

       if (CONFIG_VGA_GEODELX || CONFIG_VGA_GEODEGX2)
           return geodevga_init();

-Kevin