Subject: Re: agp_i810.c review
To: None <tech-x11@netbsd.org>
From: Christos Zoulas <christos@astron.com>
List: tech-x11
Date: 04/17/2007 23:30:15
In article <1176846518.34672.32.camel@vonnegut>,
Eric Anholt  <eric@anholt.net> wrote:
>-=-=-=-=-=-
>
>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 == 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.

Can you send a patch please?

christos