Subject: Re: agp_i810.c review
To: None <firstname.lastname@example.org>
From: Christos Zoulas <email@example.com>
Date: 04/17/2007 23:30:15
In article <1176846518.34672.32.camel@vonnegut>,
Eric Anholt <firstname.lastname@example.org> 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?