[coreboot] Geode LX VGA BIOS Patch
Chris Kindt
chriskindt at umbc.edu
Sat Aug 15 06:00:36 CEST 2009
Kevin O'Connor wrote:
> On Wed, Aug 12, 2009 at 06:02:08AM -0400, Chris Kindt wrote:
>> These initial patches enable booting with the SeaBIOS VGA BIOS on
>> Geode LX hardware.
>
> Wow. I was unaware that there was a GSOC project for this.
>
> I have a few comments - see below.
>> +static union u64_u32_u lx_msrRead(u32 msrAddr)
>> +{
>> + union u64_u32_u val;
>> + asm __volatile__ (
>> + "movw $0x0AC1C, %%dx \n"
>> + "movl $0xFC530007, %%eax \n"
>> + "outl %%eax, %%dx \n"
>> + "addb $2, %%dl \n"
>> + "inw %%dx, %%ax \n"
>> + : "=a" (val.lo), "=d"(val.hi)
>> + : "c"(msrAddr)
>
> Does the geode do something "magical" with the above sequence?
> Otherwise, I'm a bit confused on how the asm works.
This is a magic port. The geode SMM (VSA2) provides a few functions from
this interface.
>
> [...]
>> +static inline void
>> +call16_vgaint_lx(u32 eax, u32 ebx,u32 ecx, u32 edx)
>> +{
>> + asm volatile(
>> + "int $0x10\n"
>> + "cli\n"
>> + "cld"
>> + :
>> + : "a"(eax), "b"(ebx),"c"(ecx), "d"(edx)
>> + : "cc", "memory");
>> +}
>
> It looks like you're not indicating that the registers could get
> clobberred. Also, the existing call16_simpint() or direct calls to
> the bios handlers may work.
I am replacing it with direct calls.
>
>> +/* PCI Header
>> +*
>> +* This might deserve a pci_header.S file, it is here for now
>> +*/
>> +
>> +ASM16(
>> + " .globl _rom_pcidata \n"
>> + "_rom_pcidata: \n"
>> + "_rom_pcidata_sig: \n"
>> + " .ascii \"PCIR\" \n"
>> + "_rom_pcidata_venderid: \n"
>> + " .word 0x1022 \n"
> [...]
>
> I think it would be better to define this with a C struct - see the
> "struct pci_data" defined in src/optionroms.c.
>
This was originally added in vgaentry.S. It was relocated in an attempt
to contain everything in geodelx.* files. I agree that a struct would be
a much better format.
>> + if(CONFIG_GEODELX)
>> + geodelx_demo();
>> +
>> + // Fixup checksum
>> + extern u8 _rom_header_size, _rom_header_checksum;
>> // Fixup checksum
>> extern u8 _rom_header_size, _rom_header_checksum;
>
> Bad merge?
>
I missed that.
> [... out of order ...]
>> // XXX
>> #define CONFIG_VBE 0
>> #define CONFIG_CIRRUS 0
>> +#define CONFIG_GEODELX 1
>
> For SeaBIOS, I don't thing geode should be the default.
I intended this patch to be used by someone wishing to build a geode
rom. I wasn't thinking about inclusion yet.
>
> Finally, I'd be happy to commit this to SeaBIOS. I think it does
> re-raise the question of whether SeaBIOS' VGA support should be
> spun-off into it's own repository, though.
>
> -Kevin
>
I am happy that you would be willing to commit. An updated file is on
the way.
Thanks,
Chris Kindt
More information about the coreboot
mailing list