Subject: Re: VGA driver changes for machines without built-in font
To: None <junyoung@netbsd.org>
From: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
List: tech-kern
Date: 01/25/2003 02:39:10
Hi, thank you for your comments.

In article <20030124155158.GA271@krishna>
junyoung@netbsd.org wrote:

> ../../../../dev/ic/vga_raster.c:140: `MC6845_NREGS' undeclared here
> (not in a function)
> cc1: warnings being treated as errors

Ah, sorry, I forgot to add a diff for mc6845reg.h in the latest patch..

> ../../../../dev/ic/vga_raster.c:268: warning: initialization discards
> qualifiers from pointer target type
> 
> To fix that, I had to remove const qualifier you added to definition of
> struct videomode vga_console_modes[]. And then the kernel was successfully
> built.

Ok, I'll revert this.

> +	256,				/* numbhars */
> 				              ^	
> 					    typo

Oops, Fixed.

> -		scr->cursortmp.enc = WSDISPLAY_FONTENC_IBM;
> +		scr->cursortmp.enc = scr->encoding;
> 
> Please leave encoding related code as it is. I'm going to resume working
> on this soon. 

Hmm. My intention of these changes is to use encoding
of default wsfont in vga_no_builtinfont case.
Without these changes, if the kernel uses wsfonts for default font
other than WSDISPLAY_FONTENC_IBM, it shows some garbages.
(for example, it fills screen with '@' (0x40) instead ' ' (0x20)
 on initialization with FONT_VT220ISO8x16, whose firstchar is 0x20.)

I guess these implecit FONTENC_IBMs are used for the encoding of
the builtin font (which is default), but is it wrong?
In my patch, scr->encoding is set in vga_raster_setup_font().

> vga_reset() and its friends are not necessary for i386. How about making
> it optionally compile via 'options VGA_RESET_ON_BOOT' (or something with 
> a better name)?

Yeah, I also wonder if it should be optional or not..

At the first time I added 'options VGA_NO_BUILTIN_FONT'
to disable all related changes, but I removed it because
it was also useful to use optional fonts even on i386.
But actually vga_reset() is required by only some machines,
so I'll add options VGA_RESET to enable it.
(or needs a better name? :-)

I've put the latest patch here:
http://www.ceres.dti.ne.jp/~tsutsui/netbsd/arc-vga-cirrus-20030124.diff
---
Izumi Tsutsui
tsutsui@ceres.dti.ne.jp