Bug 3553 - [mga] Processing PInS (BIOS) information horribly broken
Summary: [mga] Processing PInS (BIOS) information horribly broken
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/mga (show other bugs)
Version: unspecified
Hardware: All All
: high normal
Assignee: Ian Romanick
QA Contact:
Depends on:
Blocks: 1690
  Show dependency treegraph
Reported: 2005-06-16 11:30 UTC by Ian Romanick
Modified: 2005-06-30 17:04 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Description Ian Romanick 2005-06-16 11:30:52 UTC
The code in MGAReadBios (mga_driver.c) for processing the PInS information
embedded in the card's BIOS is horribly broken in several ways.

1. It does not support PowerPC at all (the code is #ifdef'ed out).
2. It does not support version 4 or 5 if the PInS structure.  These versions
have a structure size of 128 bytes, but the code rejects any structure that is
not 64 bytes.
3. Version 3 isn't really supported either.  It is 64 bytes, but many of the
fields are in different locations.

Basically, MGAReadBios needs to be gutted and replaced with code more like what
is in the Linux kernel's matroxfb driver.  All of the device specific and PInS
version specific processing is done in one place.  All of the information is
processed and used to fill in a single datastructure that is used throughout the
code for all chips.

Doing this would allow us to eliminate a lot of 'switch( pMga->Chipset ) { ...
}' blocks.  This is especially true in mga_dacG.c.
Comment 1 Ian Romanick 2005-06-20 13:51:11 UTC
Created attachment 2934 [details] [review]
Proposed patch - just skip size zero (and near) fonts

This patch dumps all of the old BIOS processing code from the MGA DDX.	The new
code, located in mga_bios.c, is modeled after the code in matroxfb_misc.c
(though no actual code was copied).  Basically, the BIOS is processed in one
place, with "device independent" values stored in a data structure.  This data
is then used, without extra switch-statments, throughout the driver.

In addition, this patch adds support for processing the BIOS on PowerPC
systems.  On PPC cards, the magic offset values (that give the location of the
PInS data) is not in the BIOS.	Instead the driver has to search the BIOS for
the PInS structure signature.  The patch does this and correctly handles
byte-ordering (and data alignement) issues.

This code has been tested on an AGP G400 on x86 and a PCI G450 on PowerPC.  I
would *really* appreciate it if people could test this patch on older (e.g.,
G200, G100, Millenium II, etc.) hardware.

I intend to remove the extra log messages at the end of
mga_read_and_process_bios (mga_bios.c) before committing the code.

NOTE: The file mga_bios.h is also removed.  The "documentation" in that file
was moved to the file mga_PInS.txt.  This file documents, as much as possible,
the layout of the various PInS datastructure versions.	The information in that
file is 100% based on the old mga_bios.h and the code in matroxfb_misc.c.  No
additional information from Matrox documentation is included in that file. 
This just puts the information that was already known in one place.
Comment 2 Ian Romanick 2005-07-01 10:04:25 UTC
Page #2934, with the changes noted when it was posted, was committed to CVS
today.  Closing bug as fixed.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.