Subject: agp_i810.c review
To: None <tech-x11@netbsd.org>
From: Eric Anholt <eric@anholt.net>
List: tech-x11
Date: 04/17/2007 14:48:38
--=-y+FrHcimb6LAuecemWj5
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

Quick review of your current agp_i810.c code, since I've been hearing
about bugs with it:

- While only 8MB is common, there do exist many other stolen memory
sizes I believe (up to 64M on i915), which you should probably handle
appropriately in attach.
- On the i915, you've got a hardcoded 256k GTT size (256k + 4k for bios
popup), but the GTT size is actually the graphics (AGP) aperture size /
1024.  Check the size of the appropriate PCI BAR to find the graphics
aperture size (this is what the rman_get_size() is doing in FreeBSD).
The code you have in get_aperture() attempts to get at this information
in a different way, but the appropriate bits keep moving around between
generations, when all you need is a single "get pci resource size" call
that your kernel provides you.
- For physical (type =3D=3D 2) mappings, you allow any of either PAGE_SIZE
or 4*PAGE_SIZE in alloc_memory.  However, in bind_memory, you only bind
one page.  This'll probably result in scribbling on system memory when
people do HW cursors.

--=20
Eric Anholt                             anholt@FreeBSD.org
eric@anholt.net                         eric.anholt@intel.com


--=-y+FrHcimb6LAuecemWj5
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (FreeBSD)

iD8DBQBGJUC2HUdvYGzw6vcRAhl8AJ4/slMcFEIesvUaR7NAvwT9DvUBHACeMZdE
VG7MnTuVHm4CsU05NYyIOrs=
=nV2x
-----END PGP SIGNATURE-----

--=-y+FrHcimb6LAuecemWj5--