Subject: agp_i810.c review
To: None <firstname.lastname@example.org>
From: Eric Anholt <email@example.com>
Date: 04/17/2007 14:48:38
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.
Eric Anholt anholt@FreeBSD.org
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)
-----END PGP SIGNATURE-----