[coreboot] VGA cleanup advice

[ron minnich]

On Tue, Oct 27, 2015 at 10:52 AM Nicky Sielicki <sielicki at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20151027/27793691/attachment.html>