On Tue, Oct 27, 2015 at 10:52 AM Nicky Sielicki sielicki@nicky.io wrote:
Hello list,
I'm interested in cleaning up some of the VGA programming in coreboot.
Good :-)
I was trying to figure out why GM45 "native" textmode isn't working. I noticed the following bit of code
northbridge/intel/gm45/gma.c:144
const u8 cr[] = { 0x5f, 0x4f, 0x50, 0x82, 0x55, 0x81, 0xbf, 0x1f, 0x00, 0x4f, 0x0d, 0x0e, 0x00, 0x00, 0x00, 0x00, 0x9c, 0x8e, 0x8f, 0x28, 0x1f, 0x96, 0xb9, 0xa3, 0xff }; vga_cr_write(0x11, 0); for (i = 0; i <= 0x18; i++) vga_cr_write(i, cr[i]); <...> vga_textmode_init();
Next question: why isn't that 0x18 a sizeof(cr)? You might fix that too :-)
That for-loop generates the following statement:
'vga_cr_write(0x11, 0x8e)'
Looking at some VGA documentation, and noticing that 0x8e has a MSB of 1, I realized that this statement just locked CRTC registers 00h-07h! (http://www.osdever.net/FreeVGA/vga/crtcreg.htm#11)
Here is something I don't know. Once locked, can they be unlocked, or is it a one way trip?
That's no good, because the subsequent call to vga_textmode_init() tries to write to some of those CRTC registers, and it's not checking if they're locked. It assumes that no one else is working with VGA registers.
yikes. Would it be possible to split the cr[] into two pieces, one to be called once all vga setup is done, and lock it later?
It's worth noting that GM45 isn't the only platform where this CRTC locking is occuring. A couple others follow this same practice of loading the CR registers into an array, writing them, and then calling vga_textmode_init(). That's probably not what we want, and I'm looking for advice on cleaning this up.
I think what we want to do is avoid having the VGA registers touched from outside drivers/pc80/vga/. That means that we need to add or extend functions to it in order to disincentivize writing them directly-- but is it possible to accomodate every use case?
these are good ideas. One thing I've decided with graphics is that it's probably impossible to cover all use cases. The hardware guys are too creative.
On the other hand, an easier fix is to just have vga_textmode_init() check that the registers it writes to are unlocked.
I wonder about just separating the locking from the setup bytes and making locking the last thing called. That way, if people need to add more functions at some point, they don't have to worry about locking at all.
thanks
ron