Subject: Re: VGA driver changes for machines without built-in font
To: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
From: Bang Jun-Young <junyoung@netbsd.org>
List: tech-kern
Date: 01/25/2003 00:51:58
Hi,

Sorry for late.

On Mon, Jan 20, 2003 at 11:19:02PM +0900, Izumi Tsutsui wrote:
> In article <20030120044009.GA568@krishna>
> junyoung@netbsd.org wrote:
> 
> > On Wed, Jan 15, 2003 at 10:42:05PM +0900, Izumi Tsutsui wrote:
> > > - In vga_raster.c (for raster console), use wsfont for console fontset
> > >   rather than loading builtin font in vga_raster_init_screen().
> > 
> > Builtin font is used to give a user the (nearly) same look and feel as
> > he's running on the real text console. I think process of loading builtin
> > font can be skipped by checking if font is found in the ROM at boot time,
> > or it can be configured to do so.
> 
> Yes, wsfont is used only if vga_no_builtinfont (which would be set in
> MD consinit()) is true. Otherwise builtin fonts are still used even for
> raster console. I'm wondering whether there is any way to detect
> if the adapter has builtin font or not, though.

I read a VGA manual again but couldn't find how to do that in MI way. :-(
If your bios didn't load builtin font during bootup process...maybe there's
no luck (yet).

I have applied your patch and now am running with a patched kernel. While
building kernel, I had to modify sources a bit to fix some compile errors:

cc  -ffreestanding  -O2 -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wno-sign-compare -Wno-uninitialized  -Di386 -I.  -I../../../../arch -I../../../.. -nostdinc -DLKM -DMAXUSERS=8 -D_KERNEL -D_KERNEL_OPT   -c ../../../../dev/ic/vga_raster.c
../../../../dev/ic/vga_raster.c:140: `MC6845_NREGS' undeclared here (not in a function)
cc1: warnings being treated as errors

This was fixed by adding MC6845_NREGS to mc6845reg.h.

../../../../dev/ic/vga_raster.c:226: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:232: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:238: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:244: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:250: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:256: warning: initialization discards qualifiers from pointer target type
../../../../dev/ic/vga_raster.c:262: warning: initialization discards qualifiers from pointer target type
../../../../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.

Index: dev/ic/vga.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/vga.c,v
retrieving revision 1.64
diff -u -r1.64 vga.c
--- dev/ic/vga.c	2002/10/15 17:30:43	1.64
+++ dev/ic/vga.c	2003/01/18 21:23:59
@@ -54,13 +54,19 @@
 /* for WSCONS_SUPPORT_PCVTFONTS and WSDISPLAY_CHARFUNCS */
 #include "opt_wsdisplay_compat.h"
 
+int vga_no_builtinfont = 0;
+
 static struct wsdisplay_font _vga_builtinfont = {
-	"builtin",
-	0, 256,
-	WSDISPLAY_FONTENC_IBM,
-	8, 16, 1,
-	WSDISPLAY_FONTORDER_L2R, 0,
-	0
+	"builtin",			/* typeface name */
+	0,				/* firstchar */
+	256,				/* numbhars */
				              ^	
					    typo

Index: dev/ic/vga_raster.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/vga_raster.c,v
retrieving revision 1.5
diff -u -r1.5 vga_raster.c
--- dev/ic/vga_raster.c	2002/11/28 07:02:20	1.5
+++ dev/ic/vga_raster.c	2003/01/18 21:24:01
@@ -135,7 +137,7 @@
 
 struct vga_moderegs {
 	u_int8_t miscout;		/* Misc. output */
-	u_int8_t crtc[VGA_CRTC_NREGS];	/* CRTC controller */
+	u_int8_t crtc[MC6845_NREGS];	/* CRTC controller */
 	u_int8_t atc[VGA_ATC_NREGS];	/* Attribute controller */
 	u_int8_t ts[VGA_TS_NREGS];	/* Time sequencer */
 	u_int8_t gdc[VGA_GDC_NREGS];	/* Graphics display controller */
@@ -145,7 +147,7 @@
 static struct vgascreen vga_console_screen;
 static struct vga_config vga_console_vc;
 static struct vga_raster_font vga_console_fontset_ascii;
-static struct videomode vga_console_modes[2] = {
+static const struct videomode vga_console_modes[2] = {
	^^^^^
	removed

@@ -986,19 +1003,19 @@
 		 */
 		memt = vh->vh_memt;
 		memh = vh->vh_memh;
-		off = (scr->cursorrow * scr->type->ncols + scr->cursorcol) + 
+		off = (scr->cursorrow * scr->type->ncols + scr->cursorcol) +
 		    scr->dispoffset / 8;
 
 		scr->cursortmp = scr->mem[off];
-		vga_raster_putchar(scr, scr->cursorrow, scr->cursorcol, 
+		vga_raster_putchar(scr, scr->cursorrow, scr->cursorcol,
 		    scr->cursortmp.ch, scr->cursortmp.attr ^ 0x77);
 	} else {
 		scr->cursortmp.ch = 0;
 		scr->cursortmp.attr = 0;
 		scr->cursortmp.second = 0;
-		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. 

 	scr->mem[off].ch = c;
 	scr->mem[off].attr = attr;
 	scr->mem[off].second = 0;
-	scr->mem[off].enc = WSDISPLAY_FONTENC_IBM;
+	scr->mem[off].enc = scr->encoding;

... And here,

-		scr->mem[off + i].enc = WSDISPLAY_FONTENC_IBM;
+		scr->mem[off + i].enc = scr->encoding;

Here, too.

+
+/*
+ * vga_reset():
+ *	Reset VGA registers to put it into 80x25 text mode. (mode 3)
+ *	This function should be called from MD consinit() on ports
+ *	whose firmware does not use text mode at boot time.
+ */

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)?

The rest of the patch looks okay.

Jun-Young

-- 
Bang Jun-Young <junyoung@netbsd.org>