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
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
