Subject: agp_i810.c review
To: None <>
From: Eric Anholt <>
List: tech-x11
Date: 04/17/2007 14:48:38
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.

Eric Anholt                                  

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

Version: GnuPG v1.4.7 (FreeBSD)