From: Jon M. Taylor <taylorj@ecs.csus.edu>
  To  : GGI mailing list <ggi-develop@eskimo.com>
  Date: Sun, 2 Aug 1998 22:26:07 -0700 (PDT)

Re: More progress with kgicon

Jonas writes:

>>On Sun, Aug 02, 1998 at 01:21:27AM +1000, Andrew Apted wrote:
>>> Hi !
>>>
>>> I've committed a new'n'improved fbcon-kgi.c which fixes many bugs
>>> (palette setting, panning, splitline) but more importantly changes the
>>> way initialization works : it is no longer "init_kgi() sets mode and
>>> then we create a var_screeninfo" it is now "first create a 
>>> var_screeninfo 
>>> and then set the mode in kgifb_set_var()".
>>
>> This change broke the Matrox Millennium driver. It says MMIO == NULL.
>
> Oh, sorry I thought everything was supposed to set up the MMIO
> information probably (except maybe the stock VGA driver).

	No, AFAIK MMIO information as it pertains to the card as a whole
is internal to the KGI drivers and should not be exposed to the outside
world except in the context of a kgi_mode. I took a look this afternoon at
fbcon-kgi.c and the et6000 chipset driver, and I see what you are doing
but I don't think it is the correct way to handle default modes.

	The ET6000 chipset driver currently sets a default mode by
scanning the hardware settings and building a kgi_mode out of them.  It
seems like this is designed to ensure that the video mode before driver
insertion is the same mode that is set after driver insertion.  But this
is not always a valid thing to do.  What if your driver is to be run on a
system not based on VGA-style hardware?  What if the card is hooked to
another monitor for multiheading, and that monitor cannot handle the same
mode as the other one?  What if the user wants to start off with a 
different mode?

	Also, in fbcon-kgi.c in kgi_init(), you commented out the call to
kgim_check_mode(), thus passing the default kgi_mode set by your chipset
driver through unmodified.  Problem is, that kgi_mode is set by the
chipset driver at chipset init time, and the chipset doesn't know anything
about the other driver subsections.  The mode MUST be passed through
kgim_check_mode() to give each of those subsections a crack at it before
its info can be assumed valid and be used for anything.  The various 
check_*_mode() functions can tweak the mode in many ways, and it is 
possible that something important like the LFB base or the resolution 
could change.
 
> I made the switch to using kgi_display.mmio->size because
> kgi_display.mode.fb_size was wrong (just the size of the current mode,
> which hasn't been set yet 

	It should be checked_mode()ed before use, which will give you 
valid info in mode.fb_size.

> so the driver assumes text mode and we get a
> value like 131072 -- what is needed is the total size of the linear
> area -- e.g. 2 MB).

	But not all modes occupy the whole LFB.  mode.fb_size *should* be
set to the size of the LFB *for that mode*, not the size of the MMIO
region for the whole card.  That MMIO info is internal to the KGI driver, 
and fbcon doesn't need it.  The KGI driver is what should be zeroing out 
the whole MMIO region.

>>  This version seems to use the defmode from the driver instead of the 
>>  defmode structure in fbcon-kgi.c, but the Matrox driver
>
> No previously we would set KGIFB_640x400 as the default mode in
> init_kgi(), and fill out the default_var structure from that, but now we
> first set up the default_var structure, and *then* convert that to a
> ggi_mode (via convert_to_ggimode) when kgifb_set_var() calls
> kgi_do_set_mode().  The old way was not the right way.

	Correct.  Only fbcon modes are relevant here.  kgi_modes should 
only be used to translate fbcon modes right before they are fed to the 
KGI driver routines.

>>>  doesn't save any defmode, because the driver is MMIO only and can't 
>>>  use the
>>>  ordinary VGA text mode, it uses the real framebuffer with a different 
>>>  layout 
>>>  and doesn't use the vga framebuffer.

	This is correct behavior.  If it is desired to set the default
mode of the new display to the mode of the VC it was mapped from, that
information should be passed in to kgicon by fbcon when the CON2FBMAP
ioctl is recieved.  That would be the proper way to do what the ET6000
driver is trying to do.  If this cannot be done, then you fall back to 
the driver's default mode unless THAT fails a check_mode(), in which case 
the console migration should fail.

>>>  I tried to set the mode->mmio to the correct value, but then insmod 
>>>  crashed
>>>  with "unable to handle kernel NULL pointer dereference at virtual
>>>  address 00000000"
>>>
>>>  Any Idea?

	It might be getting overridden by the zero value in the default
structure at some point.

> I'm thinking that setting up kgi_display.mmio should always be done --
> is that right ?

	It shouldn't be necessary for basic framebuffering or acceleration
via ioctl.  If you want to poke the MMIO registers directly or bypass a
pseudo-linear framebuffer mapping or something like that, then you would
need the MMIO region info.  But for kgicon, it isn't of much use outside
the KGI driver.  Inside the driver, you could use this info for all kinds
of things like setting MTRRs or partitioning the video RAM into multiple
regions for double-buffering, font/bitmap caches, S3 STREAMS buffer
separation, etc.

> (Note the current code assumes *1* mmio structure which is the
> framebuffer -- if there are others then they would need to be *after*
> the first one.  Probably kgicon should scan the list looking for the
> FRAMEBUFFER tag...).

	Again, you shouldn't need to look at the MMIO regions to get the 
base and size of the LFB if your mode is set up correctly.

Jon

---
'Cloning and the reprogramming of DNA is the first serious step in 
becoming one with God.'
	- Scientist G. Richard Seed