From: Benjamin H. <be...@ke...> - 2001-08-09 15:52:37
|
Hi ! Back to an old problem... On Macs, we have that weird Apple's UniNorth AGP controller which has the "feature" of not letting the CPU access the AGP aperture. (It's not documented, but I've done all sort of tests lately and couldn't get it to do anything but Machine Check on an access to the AGP aperture, also Apple themselves do not use the AGP aperture from the CPU and map it at bus address 0 from the video card, thus overlapping RAM). Currently, we have DRI working using PCI GART on ATI r128 cards (and possibly Radeon's as well but this was not tested AFAIK). I'd like to get AGP working. I've looked at the various thing that would need change to acheive that, but before I start hacking in the quite complex DRM code, I was wondering if anybody already started work in this direction. Basically, it appears the the API exposed to userland by the DRM will not need change as the X server relies on drmMap return virtual address. What need to be done, afaik, are 3 things: - Have agpgart export some kind of flags telling about the limitation of the controller. It would be nice if allocation of the AGP memory could be done once with a single vmalloc to simplify in-kernel virtual mapping of the aperture. - DRM should respond to userland map requests by using a special nopage() routine (maybe similar to what is used by PCI GART, I didn't look at that implementation in detail yet) that basically maps the real RAM pages into the process space - DRM should respond to in-kernel ioremap macro of AGP space differently than it does today, by either building it's own virtual space mapped to the RAM pages, or better, letting agpgart do it. This would be made easier if the following limitation to agpgart was considered acceptable: let only a single chunk AGP memory to be allocated & bound once for all instead of letting segments be allocated & bound dynamically. I don't think that dynamic feature is used by DRM today (at least not by the ATI drivers). This would allow a single chunk of vmalloc'ed memory to be used, giving us the in-kernel virtual mapping for free so we only have to bother about the mapping of those pages to userland. I'd appreciate any comments, clues, guidelines, directions, etc... as I'm still a bit confused by the DRM code :) Regards, Ben. |
From: Jeff H. <jha...@va...> - 2001-08-09 16:18:02
Attachments:
alpha.patch
|
Benjamin Herrenschmidt wrote: > Hi ! > > Back to an old problem... > > On Macs, we have that weird Apple's UniNorth AGP controller which has the > "feature" of not letting the CPU access the AGP aperture. (It's not > documented, but I've done all sort of tests lately and couldn't get it to > do anything but Machine Check on an access to the AGP aperture, also > Apple themselves do not use the AGP aperture from the CPU and map it at > bus address 0 from the video card, thus overlapping RAM). > > Currently, we have DRI working using PCI GART on ATI r128 cards (and > possibly Radeon's as well but this was not tested AFAIK). > > I'd like to get AGP working. I've looked at the various thing that would > need change to acheive that, but before I start hacking in the quite > complex DRM code, I was wondering if anybody already started work in this > direction. Actually there is a patch floating around which accomplishes this for the alpha. I will attach it to this email, but its has other problems (with the mga driver) so isn't merged into the mainline yet. > > Basically, it appears the the API exposed to userland by the DRM will not > need change as the X server relies on drmMap return virtual address. What > need to be done, afaik, are 3 things: > > - Have agpgart export some kind of flags telling about the limitation of > the controller. It would be nice if allocation of the AGP memory could be > done once with a single vmalloc to simplify in-kernel virtual mapping of > the aperture. This is all going to be reworked in 2.5. For now I don't want to make any large changes to agpgart. > > - DRM should respond to userland map requests by using a special > nopage() routine (maybe similar to what is used by PCI GART, I didn't > look at that implementation in detail yet) that basically maps the real > RAM pages into the process space > > - DRM should respond to in-kernel ioremap macro of AGP space differently > than it does today, by either building it's own virtual space mapped to > the RAM pages, or better, letting agpgart do it. This would be made > easier if the following limitation to agpgart was considered acceptable: > let only a single chunk AGP memory to be allocated & bound once for all > instead of letting segments be allocated & bound dynamically. I don't > think that dynamic feature is used by DRM today (at least not by the ATI > drivers). This would allow a single chunk of vmalloc'ed memory to be > used, giving us the in-kernel virtual mapping for free so we only have to > bother about the mapping of those pages to userland. > > I'd appreciate any comments, clues, guidelines, directions, etc... as I'm > still a bit confused by the DRM code :) > > Regards, > Ben. > > > > _______________________________________________ > Dri-devel mailing list > Dri...@li... > http://lists.sourceforge.net/lists/listinfo/dri-devel > I'm attaching the alpha patch which was sent to me. It looks like it accomplishes what you need for not accessing the aperture directly. Using it as a base you should be able to get a Uni-North agpgart driver working with the drm. It won't work for the agpgart userland API, but the kernel level agpgart API will work just fine. In my opinion thats okay until 2.5 (where I will add code to fix this for the userland API.) -Jeff |
From: Benjamin H. <be...@ke...> - 2001-08-10 14:22:18
|
I've looked at this patch, and still have a question. From my understanding, the only important piece in this patch for me is the nopage function for mmap()'ing AGP pages properly without using the CPU view of the aperture. However, the r128_cce.c also uses DRM_IOREMAP to get an in-kernel mapping of the AGP aperture, which in my case also fails. I'm trying to figure out if the kernel driver really need to access this ioremap'ed region, in which case, I would also need to replace DRM_IOREMAP with a function that creates an in-kernel virtual mapping for AGP. Also, but I'm note quite sure yet as I don't have it working enough to test so far, some Apple older chipsets may require the AGP base on the bus to be 0 (overlapping RAM from the card's point of view). There's at least one routine in DRM that won't like that: mmap() considers a 0 base as beeing a DMA buffer. Ben. |
From: Chris A. <chr...@in...> - 2001-08-14 05:33:52
|
On Thu, Aug 09, 2001 at 10:17:01AM -0600, Jeff Hartmann wrote: > This is all going to be reworked in 2.5. For now I don't want to make > any large changes to agpgart. Does this mean that support for chipsets not providing 'host translation' won't exist in any form in the 2.4 series? I'd like to eventually get DRI support for ia64 integrated into the official sources (XFree86/DRI CVS, Linus' kernel tree), and I'm wondering whether this'll have to wait for 2.5. Chris |
From: Jeff H. <jha...@va...> - 2001-08-14 15:45:55
|
Chris Ahna wrote: > On Thu, Aug 09, 2001 at 10:17:01AM -0600, Jeff Hartmann wrote: > >> This is all going to be reworked in 2.5. For now I don't want to make >> any large changes to agpgart. > > > Does this mean that support for chipsets not providing 'host > translation' won't exist in any form in the 2.4 series? No it will exist but it will be hacked up. In 2.5 I will approach this problem in a completely different way so that there is a clean solution. Everyone will directly access a vmalloc'ed area when accessing agp memory. This requires that PAT support is in place for the ia32, and I am not submiting that work till 2.5. > > I'd like to eventually get DRI support for ia64 integrated into the > official sources (XFree86/DRI CVS, Linus' kernel tree), and I'm wondering > whether this'll have to wait for 2.5. > > Chris > Btw can you send me your patches again, I apologize for being slow to get them integrated. Are they still based on the 4.0.3 drm? Or have you ported them to 4.1.0 yet? -Jeff |
From: Chris A. <chr...@in...> - 2001-08-14 16:33:58
|
On Tue, Aug 14, 2001 at 09:44:55AM -0600, Jeff Hartmann wrote: > > Does this mean that support for chipsets not providing 'host > > translation' won't exist in any form in the 2.4 series? > > No it will exist but it will be hacked up. In 2.5 I will approach this > problem in a completely different way so that there is a clean > solution. Everyone will directly access a vmalloc'ed area when > accessing agp memory. This requires that PAT support is in place for > the ia32, and I am not submiting that work till 2.5. OK. Thanks. > > I'd like to eventually get DRI support for ia64 integrated into the > > official sources (XFree86/DRI CVS, Linus' kernel tree), and I'm wondering > > whether this'll have to wait for 2.5. > > > > Chris > > > Btw can you send me your patches again, I apologize for being slow to > get them integrated. Are they still based on the 4.0.3 drm? Or have > you ported them to 4.1.0 yet? I did an initial port to the 4.1.0 drm a few months back, this seems to be working pretty well, however I'll clean it up today. I'll make a point to get some patches to you soon. Thanks, Chris |
From: Chris A. <chr...@in...> - 2001-08-14 05:30:48
|
On Thu, Aug 09, 2001 at 06:52:40PM +0200, Benjamin Herrenschmidt wrote: > On Macs, we have that weird Apple's UniNorth AGP controller which has the > "feature" of not letting the CPU access the AGP aperture. (It's not > documented, but I've done all sort of tests lately and couldn't get it to > do anything but Machine Check on an access to the AGP aperture, also > Apple themselves do not use the AGP aperture from the CPU and map it at > bus address 0 from the video card, thus overlapping RAM). ... > I'd appreciate any comments, clues, guidelines, directions, etc... as I'm > still a bit confused by the DRM code :) The Intel 460GX chipset (appears only in ia64 boxen) has the same limitations you've described above. In writing an agpgart driver for 460, I worked around this problem by: i) manually altering the PTEs corresponding to user space mmaps which overlapped the aperture so that the virtual pages were mapped to the proper physical ones. ii) adding a version of vmalloc to agpgart which, instead of allocating pages, simply wired in virtual -> physical mappings in the page tables that matched those found in the GART. Basically, this driver works by replicating pieces of the GART table in the processor page tables. Right now this driver is in the ia64 kernel tree and seems to work pretty well. The changes to agpgart and drm weren't all that invasive either. If you're interested in this approach, I can send along the relevant patches. Chris |
From: Benjamin H. <be...@ke...> - 2001-08-14 07:36:14
|
>The Intel 460GX chipset (appears only in ia64 boxen) has the same >limitations you've described above. In writing an agpgart driver for >460, I worked around this problem by: > >i) manually altering the PTEs corresponding to user space mmaps which >overlapped the aperture so that the virtual pages were mapped to the >proper physical ones. > >ii) adding a version of vmalloc to agpgart which, instead of allocating >pages, simply wired in virtual -> physical mappings in the page tables >that matched those found in the GART. > >Basically, this driver works by replicating pieces of the GART table in >the processor page tables. > >Right now this driver is in the ia64 kernel tree and seems to work >pretty well. The changes to agpgart and drm weren't all that invasive >either. If you're interested in this approach, I can send along the >relevant patches. I'm interested in your patches, yes. I've started working a different way now. It doesn't quite work yet, but I'm suspecting another problem in the way I'm setting up UniNorth. Currently, I'm using the alpha patch to DRM that implements mmap'ing of AGP to userland via a nopage function that maps the physical pages, and I added an ioremap_agp function to DRM for the in-kernel mapping that wire the memory pages to vmalloc'ed space. It's probably similar to what you are doing, but I'd still like to see your code. There is another problem with my approach, which is probably specific to PPC, that is cache aliasing issues. AGP pages have to be non-cachable. However, the kernel always have all RAM mapped cachable using a BAT (a kind of very big page table entry that is decoded by the MMU before any hash table translation). That means our AGP memory is mapped both non-cachable by the 2 above mecanisms, and cachable with the BAT (but at different virtual addresses). The problem is that the speculative loads of the PPC may cause some of these pages to end up in the cache if some code accessed an adjacent page via the cachable mapping, and the CPU "speculatively" loaded bits of the next page. It's not easy to remove that cachable mapping of all RAM, I may end up turning everything into cachable state and adding proper flush/invalidate routines to the various bits of code that access the AGP memory. Regards, Ben. |
From: Chris A. <chr...@in...> - 2001-08-14 08:23:36
|
On Tue, Aug 14, 2001 at 09:35:45AM +0200, Benjamin Herrenschmidt wrote: > I'm interested in your patches, yes. I've started working a different > way now. It doesn't quite work yet, but I'm suspecting another problem > in the way I'm setting up UniNorth. Currently, I'm using the alpha > patch to DRM that implements mmap'ing of AGP to userland via a nopage > function that maps the physical pages, and I added an ioremap_agp > function to DRM for the in-kernel mapping that wire the memory pages > to vmalloc'ed space. It's probably similar to what you are doing, but > I'd still like to see your code. My 'ioremap_agp' code is below. This is a straight copy of the code in mm/vmalloc.c, it's just altered to do mapping instead of allocation. Thinking about it more, the nopage method is indeed a _very_ clean way to handle the user space maps. I'll have to convert my driver over that way soon. One comment about ioremap_agp. What happens if the GART table changes while some client has obtained and is using a chunk of memory returned by ioremap_agp? The original mapping is now incorrect, so we have to fix up all existing ioremap_agp's every time the GART table changes. I'm not sure if this makes sense (it's rather late here :-) or if you've considered it already. I've written code to do such fixups, but I don't have a good looking diff lying around to send. Unless you have a reason why this doesn't have to be done, I'll send along some sample fixup code tomorrow. Thanks, Chris --- vmmap.c (my version of ioremap_agp) --- /* * vmmap.c * * Hack to allow virtual addressing fixups in the kernel's * vmalloc area. This is needed so that the page tables in the * kernel's vmalloc area can be used to emulate GART translation * This file is currently good for 460GX only. * * This file is basically a copy of mm/vmalloc.c modified to not * allocate and free pages. * * Chris Ahna <chr...@in...> */ #include <linux/config.h> #include <linux/slab.h> #include <linux/vmalloc.h> #include <linux/spinlock.h> #include <linux/module.h> #include <linux/agp_backend.h> #include <linux/highmem.h> #include <asm/uaccess.h> #include <asm/pgalloc.h> #include "agp.h" #ifdef CONFIG_AGP_PTE_FIXUPS EXPORT_SYMBOL(agp_vmmap); EXPORT_SYMBOL(agp_vmunmap); EXPORT_SYMBOL(agp_flush_tlb_all); static inline void agp_free_area_pte(pmd_t * pmd, unsigned long address, unsigned long size) { pte_t * pte; unsigned long end; if (pmd_none(*pmd)) return; if (pmd_bad(*pmd)) { pmd_ERROR(*pmd); pmd_clear(pmd); return; } pte = pte_offset(pmd, address); address &= ~PMD_MASK; end = address + size; if (end > PMD_SIZE) end = PMD_SIZE; do { pte_t page; page = ptep_get_and_clear(pte); address += PAGE_SIZE; pte++; if (pte_none(page) || pte_present(page)) { continue; } printk(KERN_CRIT "Whee.. Swapped out page in kernel page table\n"); } while (address < end); } static inline void agp_free_area_pmd(pgd_t * dir, unsigned long address, unsigned long size) { pmd_t * pmd; unsigned long end; if (pgd_none(*dir)) return; if (pgd_bad(*dir)) { pgd_ERROR(*dir); pgd_clear(dir); return; } pmd = pmd_offset(dir, address); address &= ~PGDIR_MASK; end = address + size; if (end > PGDIR_SIZE) end = PGDIR_SIZE; do { agp_free_area_pte(pmd, address, end - address); address = (address + PMD_SIZE) & PMD_MASK; pmd++; } while (address < end); } void agp_vmfree_area_pages(unsigned long address, unsigned long size) { pgd_t * dir; unsigned long end = address + size; dir = pgd_offset_k(address); flush_cache_all(); do { agp_free_area_pmd(dir, address, end - address); address = (address + PGDIR_SIZE) & PGDIR_MASK; dir++; } while (address && (address < end)); flush_tlb_all(); } static inline int agp_alloc_area_pte (pte_t * pte, unsigned long address, unsigned long size, unsigned long target, pgprot_t prot) { unsigned long end; address &= ~PMD_MASK; end = address + size; if (end > PMD_SIZE) end = PMD_SIZE; target = (unsigned long) __va(target); do { struct page * page; page = virt_to_page(target); if (!pte_none(*pte)) printk(KERN_ERR "alloc_area_pte: page already exists\n"); if (!page) return -ENOMEM; set_pte(pte, mk_pte(page, prot)); address += PAGE_SIZE; target += PAGE_SIZE; pte++; } while (address < end); return 0; } static inline int agp_alloc_area_pmd(pmd_t * pmd, unsigned long address, unsigned long size, unsigned long target, pgprot_t prot) { unsigned long end; address &= ~PGDIR_MASK; end = address + size; if (end > PGDIR_SIZE) end = PGDIR_SIZE; do { pte_t * pte = pte_alloc(&init_mm, pmd, address); if (!pte) return -ENOMEM; if (agp_alloc_area_pte(pte, address, end - address, target, prot)) return -ENOMEM; target += ((address + PMD_SIZE) & PMD_MASK) - address; address = (address + PMD_SIZE) & PMD_MASK; pmd++; } while (address < end); return 0; } inline int agp_vmalloc_area_pages (unsigned long address, unsigned long size, unsigned long target, pgprot_t prot) { pgd_t * dir; unsigned long end = address + size; int ret; dir = pgd_offset_k(address); flush_cache_all(); spin_lock(&init_mm.page_table_lock); do { pmd_t *pmd; pmd = pmd_alloc(&init_mm, dir, address); ret = -ENOMEM; if (!pmd) break; ret = -ENOMEM; if (agp_alloc_area_pmd(pmd, address, end - address, target, prot)) break; target += ((address + PGDIR_SIZE) & PGDIR_MASK) - address; address = (address + PGDIR_SIZE) & PGDIR_MASK; dir++; ret = 0; } while (address && (address < end)); spin_unlock(&init_mm.page_table_lock); flush_tlb_all(); return ret; } void agp_vmunmap(void *addr) { struct vm_struct **p, *tmp; if (!addr) return; if ((PAGE_SIZE-1) & (unsigned long) addr) { printk(KERN_ERR "Trying to vfree() bad address (%p)\n", addr); return; } write_lock(&vmlist_lock); for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) { if (tmp->addr == addr) { *p = tmp->next; agp_vmfree_area_pages(VMALLOC_VMADDR(tmp->addr), tmp->size); write_unlock(&vmlist_lock); kfree(tmp); return; } } write_unlock(&vmlist_lock); printk(KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n", addr); } void *__agp_vmmap (unsigned long size, unsigned long target, pgprot_t prot) { void * addr; struct vm_struct *area; size = PAGE_ALIGN(size); if (!size) { BUG(); return NULL; } area = get_vm_area(size, VM_ALLOC); if (!area) return NULL; addr = area->addr; if (agp_vmalloc_area_pages(VMALLOC_VMADDR(addr), size, target, prot)) { vfree(addr); return NULL; } return addr; } void *agp_vmmap(unsigned long offset, unsigned long size) { return __agp_vmmap(size, offset, PAGE_KERNEL); } void agp_flush_tlb_all(void) { flush_tlb_all(); } #endif /* CONFIG_AGP_PTE_FIXUPS */ |
From: Benjamin H. <be...@ke...> - 2001-08-14 09:05:37
|
>My 'ioremap_agp' code is below. This is a straight copy of the code >in mm/vmalloc.c, it's just altered to do mapping instead of allocation. > >Thinking about it more, the nopage method is indeed a _very_ clean way >to handle the user space maps. I'll have to convert my driver over that >way soon. > >One comment about ioremap_agp. What happens if the GART table changes >while some client has obtained and is using a chunk of memory returned >by ioremap_agp? The original mapping is now incorrect, so we have to >fix up all existing ioremap_agp's every time the GART table changes. > >I'm not sure if this makes sense (it's rather late here :-) or if you've >considered it already. I've written code to do such fixups, but I don't >have a good looking diff lying around to send. Unless you have a reason >why this doesn't have to be done, I'll send along some sample fixup code >tomorrow. Thanks, I decided arbitrarily that I didn't support that "feature" ;) For now, I'm mostly working with the r128 and radeon drivers, and they both allocate the AGP memory once for all. Ben. |
From: Benjamin H. <be...@ke...> - 2001-08-14 13:38:28
|
Ok, I've made some progress. It sort of works, but dies after a few seconds. Here are the details: - Machine is a PowerBook "Pismo" with a rev. of UniNorth that can do x2 AGP only and no fast write (it does SBA fortunately). I use the alpha nopage() code for userland mappings and a home made ioremap_agp() for in- kernel mappings. Everything is mapped uncached, which is a performance hit on PPC, I'll have to see how I can make things cachable by adding the proper flush routines all over the place. X is 4.1 from top of 4.1 branch in XFree CVS, DRM is hacked from the 2.4.8 kernel version. - glxgears display is sorta broken (but that happened with PCI GART as well, so I beleive it's a problem with the GL driver vs. the Rage M3 (r128 mobility). - It's a bit slower than PCI GART (about 430 fps in gears, but it's not stable, while PCI GART gives me a stable 474 fps). This may be partially do to the fact that we have all the AGP memory uncachable, I think that will prevent the PPC from doing write-combining. - After running for a few seconds with any GL app (I tried glxgears and tuxracer), X dies with the message "drmR128SwapBuffers: return = -16". In dmesg, I also have a bunch of: [drm:r128_cce_vertex] *ERROR* ring space check failed! [drm:r128_cce_vertex] *ERROR* ring space check failed! [drm:r128_cce_vertex] *ERROR* ring space check failed! [drm:r128_cce_vertex] *ERROR* ring space check failed! [drm:r128_cce_vertex] *ERROR* ring space check failed! etc... and one [drm:r128_cce_swap] *ERROR* ring space check failed! It seems to stop at this point. Any clue what's up here ? I beleive I will have better perfs with more recent Apple machines that can do x4 AGP and support fast write. Also, the rage M3 is not a very fast chip anyway... For those interested, the hacked code is available in my rsync kernel tree at penguinppc.org::linux-2.4-benh Regards, Ben. |
From: Michel L. <ml...@cp...> - 2001-08-15 07:24:14
|
Hi all, On 14 Aug, this message from Benjamin Herrenschmidt echoed through cyberspace: > - Machine is a PowerBook "Pismo" with a rev. of UniNorth that can do x2 > AGP only and no fast write (it does SBA fortunately). I use the alpha > nopage() code for userland mappings and a home made ioremap_agp() for in- > kernel mappings. Everything is mapped uncached, which is a performance > hit on PPC, I'll have to see how I can make things cachable by adding the > proper flush routines all over the place. Depending on cache mode and data direction, you might not need cache flushing. I mapped the 'control' framebuffer write-through; since only the cpu writes there (it's not accelerated), that's ok without cache flushing, but it does enable store gathering in the processor (and maybe also in the PCI bridge; not sure...) Not sure whether that applies to AGP as well. > - It's a bit slower than PCI GART (about 430 fps in gears, but it's not > stable, while PCI GART gives me a stable 474 fps). This may be partially > do to the fact that we have all the AGP memory uncachable, I think that > will prevent the PPC from doing write-combining. Yes, that's how I understand all the docs I read so far. I'd suggest tackling cacheing issues first; that will probably provide the biggest benefit. Cheers, and happy hacking ;-) Michel ------------------------------------------------------------------------- Michel Lanners | " Read Philosophy. Study Art. 23, Rue Paul Henkes | Ask Questions. Make Mistakes. L-1710 Luxembourg | email ml...@cp... | http://www.cpu.lu/~mlan | Learn Always. " |
From: Chris A. <chr...@in...> - 2001-08-16 20:32:33
|
On Tue, Aug 14, 2001 at 03:38:18PM +0200, Benjamin Herrenschmidt wrote: > I use the alpha > nopage() code for userland mappings and a home made ioremap_agp() for in- > kernel mappings. With Ben's work on the UniNorth and my work on 460GX, it has become clear that we'll need some kind of agp_ioremap() function for kernel maps which overlap the AGP aperture on systems that don't allow accesses to the aperture from the host processors. I'd like to finalize an agp_ioremap() implementation and get it integrated so that Ben and I can add our chipset specific bits to the kernel mainline. I propose that agp_ioremap() work in the following way: i) Reserve a portion of the vmalloc area and wire the corresponding PTEs to aperture addresses. This can be a generic function that gets built into the kernel (since it would need access to get_vm_area, vmlist, and vmlist_lock). ii) Fix up these PTEs to point to the correct physical pages. The code to do this would reside in the agpgart module and rely on a new function in struct agp_bridge_data that returns the physical address corresponding to an address in the aperture. The aperture address -> physical address function would look like pgoff = (aperture_addr - aperture_base) >> PAGE_SHIFT; physical_addr = agp_bridge.unmask_memory( agp_bridge.gatt_table[pgoff]); for most chipsets. It needs to be chipset specific since the above method doesn't work when PAGE_SIZE is smaller than the number of bytes mapped by each GATT entry (this actually happens on 460GX). The in-kernel code for agp_ioremap() can be almost identical to the code for vmalloc(). Similarly, and agp_iounmap() function can be quickly adapted from the code for vfree(). I think the process should have two distinct steps in order to keep agp_bridge hidden from the in-kernel mapping code. Ben, Jeff, what do think about this approach? If it seems alright, I can have a patch ready pretty quickly (agp_ioremap() already exists and works largely like this in the ia64 kernel tree). Chris |
From: Jeff H. <jha...@va...> - 2001-08-16 21:09:59
Attachments:
drm-2.4.9-pre4.patch
|
Chris Ahna wrote: > On Tue, Aug 14, 2001 at 03:38:18PM +0200, Benjamin Herrenschmidt wrote: > >> I use the alpha >> nopage() code for userland mappings and a home made ioremap_agp() for in- >> kernel mappings. > > > With Ben's work on the UniNorth and my work on 460GX, it has become > clear that we'll need some kind of agp_ioremap() function for kernel maps > which overlap the AGP aperture on systems that don't allow accesses to > the aperture from the host processors. I'd like to finalize an > agp_ioremap() implementation and get it integrated so that Ben and I can > add our chipset specific bits to the kernel mainline. > > I propose that agp_ioremap() work in the following way: > > i) Reserve a portion of the vmalloc area and wire the corresponding > PTEs to aperture addresses. This can be a generic function that gets > built into the kernel (since it would need access to get_vm_area, > vmlist, and vmlist_lock). > > ii) Fix up these PTEs to point to the correct physical pages. The code > to do this would reside in the agpgart module and rely on a new function > in struct agp_bridge_data that returns the physical address corresponding > to an address in the aperture. > > The aperture address -> physical address function would look like > > pgoff = (aperture_addr - aperture_base) >> PAGE_SHIFT; > physical_addr = agp_bridge.unmask_memory( > agp_bridge.gatt_table[pgoff]); > > for most chipsets. It needs to be chipset specific since the above > method doesn't work when PAGE_SIZE is smaller than the number of bytes > mapped by each GATT entry (this actually happens on 460GX). > > The in-kernel code for agp_ioremap() can be almost identical to the code > for vmalloc(). Similarly, and agp_iounmap() function can be quickly > adapted from the code for vfree(). > > I think the process should have two distinct steps in order to keep > agp_bridge hidden from the in-kernel mapping code. > > Ben, Jeff, what do think about this approach? If it seems alright, I > can have a patch ready pretty quickly (agp_ioremap() already exists and > works largely like this in the ia64 kernel tree). > > Chris > I was thinking about this today in fact, I have a patch that missed the window for 2.4.9 which provides some functionality to support user land mappings in a generic way. I'll attach this patch (against 2.4.9-pre4) to the end of this email. I'd like to have a way to deal with the in kernel mappings in a generic way as well. I'd like to see what your proposing in patch form. I really want to just have the drm writing to the agp memory structure directly in the future. I really can't do that on ia32 just yet (because I am not submitting PAT till 2.5), so I guess an agp_ioremap could work. However perhaps we could make the API available on arch's that need this and just write to the vmalloc'ed memory ourselves. All agp memory allocations become vmalloc's if we can't write to the aperture. We populate the agp aperture by walking the page tables of the vmalloc'ed region inside agpgart. We can deal with page size issues here. We put a link to the vmalloc'ed region in the agp_memory structure. If the cant_use_aperture flag is set, we will read/write to the vmalloc'ed pointer. On arch's that can write to the aperture we just do things the same way we have been doing them. The only thing I'm concerned about is that we can get a write combining cache type on ia64 from vmalloc. I guess I'm willing to do either one, however the second solution does not require changes to the mainline kernel outside of agpgart. Would this work for everyone? Does anyone see problems with it? -Jeff |
From: Chris A. <chr...@in...> - 2001-08-16 21:54:25
|
On Thu, Aug 16, 2001 at 03:08:59PM -0600, Jeff Hartmann wrote: > I was thinking about this today in fact, I have a patch that missed the > window for 2.4.9 which provides some functionality to support user land > mappings in a generic way. I'll attach this patch (against 2.4.9-pre4) > to the end of this email. I'd like to have a way to deal with the in > kernel mappings in a generic way as well. I'd like to see what your > proposing in patch form. OK, I'll send a patch soon. One comment about the patch however, in DRM(vm_nopage)() you're adding a line that says > + agpmem->memory->memory[offset] &= dev->agp->page_mask; This won't work for 460GX (which is really getting to be a PITA :-) where unmask_memory looks like addr = (entry & 0xffffff) << 12 So I see two solutions to this problem: i) special case 460GX in the drm code ii) actually export agp_bridge.unmask_memory to drm in some form I prefer the latter, let me know if you think this is possible. > I really want to just have the drm writing to the agp memory structure > directly in the future. I really can't do that on ia32 just yet > (because I am not submitting PAT till 2.5), so I guess an agp_ioremap > could work. However perhaps we could make the API available on arch's > that need this and just write to the vmalloc'ed memory ourselves. Just as a point of interest, what's PAT? > All agp memory allocations become vmalloc's if we can't write to the > aperture. > We populate the agp aperture by walking the page tables of the > vmalloc'ed region inside agpgart. We can deal with page size issues here. > We put a link to the vmalloc'ed region in the agp_memory structure. > If the cant_use_aperture flag is set, we will read/write to the > vmalloc'ed pointer. On arch's that can write to the aperture we just do > things the same way we have been doing them. The only thing I'm > concerned about is that we can get a write combining cache type on ia64 > from vmalloc. With this approach the aperture would only be contiguous within single bound sections, correct? Two consecutive agp_allocate_memory calls will probably result in two discontiguous vmalloc'ed regions. This means no one will be able to do (what used to be) an ioremap() that covers more than one section of the aperture. Is this analysis correct? If it is, is this a problem at all? > I guess I'm willing to do either one, however the second solution does > not require changes to the mainline kernel outside of agpgart. Would > this work for everyone? Does anyone see problems with it? drm will have to change a little, right? I mean, we'll have to do something with the current ioremap() calls to make sure the drivers use the vmalloc'ed regions. I'll guess I'll send my patch and leave it to you to make the decision. Both approaches seem reasonable to me. Chris |
From: Jeff H. <jha...@va...> - 2001-08-16 22:32:58
|
Chris Ahna wrote: > On Thu, Aug 16, 2001 at 03:08:59PM -0600, Jeff Hartmann wrote: > >> I was thinking about this today in fact, I have a patch that missed the >> window for 2.4.9 which provides some functionality to support user land >> mappings in a generic way. I'll attach this patch (against 2.4.9-pre4) >> to the end of this email. I'd like to have a way to deal with the in >> kernel mappings in a generic way as well. I'd like to see what your >> proposing in patch form. > > > OK, I'll send a patch soon. One comment about the patch however, in > DRM(vm_nopage)() you're adding a line that says > >> + agpmem->memory->memory[offset] &= dev->agp->page_mask; > > > This won't work for 460GX (which is really getting to be a PITA :-) > where unmask_memory looks like > > addr = (entry & 0xffffff) << 12 > > So I see two solutions to this problem: > > i) special case 460GX in the drm code > ii) actually export agp_bridge.unmask_memory to drm in some form Okay I'll have to make some sort of external api for this it seems. > > I prefer the latter, let me know if you think this is possible. > >> I really want to just have the drm writing to the agp memory structure >> directly in the future. I really can't do that on ia32 just yet >> (because I am not submitting PAT till 2.5), so I guess an agp_ioremap >> could work. However perhaps we could make the API available on arch's >> that need this and just write to the vmalloc'ed memory ourselves. > > > Just as a point of interest, what's PAT? The page attribute table on ia32. It allows you to basically set mtrr's on individual pages. Its required if we want to write to vmalloc'ed regions instead of the aperture itself. > >> All agp memory allocations become vmalloc's if we can't write to the >> aperture. >> We populate the agp aperture by walking the page tables of the >> vmalloc'ed region inside agpgart. We can deal with page size issues here. >> We put a link to the vmalloc'ed region in the agp_memory structure. >> If the cant_use_aperture flag is set, we will read/write to the >> vmalloc'ed pointer. On arch's that can write to the aperture we just do >> things the same way we have been doing them. The only thing I'm >> concerned about is that we can get a write combining cache type on ia64 >> from vmalloc. > > > With this approach the aperture would only be contiguous within single > bound sections, correct? Correct. > Two consecutive agp_allocate_memory calls will > probably result in two discontiguous vmalloc'ed regions. This means no > one will be able to do (what used to be) an ioremap() that covers more > than one section of the aperture. Is this analysis correct? If it is, > is this a problem at all? No, this is not a problem. Inside the kernel we deal with each region seperately now. The original mga driver might have done this, but that code was rewritten by Gareth. > >> I guess I'm willing to do either one, however the second solution does >> not require changes to the mainline kernel outside of agpgart. Would >> this work for everyone? Does anyone see problems with it? > > > drm will have to change a little, right? Not much actually. We just modify the ioremap macros so they check if they must use the vmalloc'ed region. We might also want to make an easy way to associate the agp memory structures with the individual maps. > I mean, we'll have to do > something with the current ioremap() calls to make sure the drivers use > the vmalloc'ed regions. > > I'll guess I'll send my patch and leave it to you to make the decision. > Both approaches seem reasonable to me. > > Chris Okay sounds like this approach will work for you. Benjamin does it make sense to you? -Jeff |
From: Jeff H. <jha...@va...> - 2001-08-16 22:46:06
|
Jeff Hartmann wrote: > > No, this is not a problem. Inside the kernel we deal with each > region seperately now. The original mga driver might have done this, > but that code was rewritten by Gareth. I also forgot to mention that the only dri driver that actually even has multiple agp memory allocations is the i810 driver. But as I said above it deals with each of these regions seperately (plus its ia32 only anyhow.) -Jeff |
From: Chris A. <chr...@in...> - 2001-08-16 23:13:28
|
On Thu, Aug 16, 2001 at 04:31:40PM -0600, Jeff Hartmann wrote: ... > > i) special case 460GX in the drm code > > ii) actually export agp_bridge.unmask_memory to drm in some form > > Okay I'll have to make some sort of external api for this it seems. Great! ... > > Two consecutive agp_allocate_memory calls will > > probably result in two discontiguous vmalloc'ed regions. This means no > > one will be able to do (what used to be) an ioremap() that covers more > > than one section of the aperture. Is this analysis correct? If it is, > > is this a problem at all? > > No, this is not a problem. Inside the kernel we deal with each region > seperately now. The original mga driver might have done this, but that > code was rewritten by Gareth. Alright. ... > > drm will have to change a little, right? > > Not much actually. We just modify the ioremap macros so they check if > they must use the vmalloc'ed region. We might also want to make an easy > way to associate the agp memory structures with the individual maps. These are the same drm changes that would go with my approach. ... > Okay sounds like this approach will work for you. Benjamin does it make > sense to you? Earlier you voiced a concern about not being able to get a writecombined attribute out of vmallocon ia64. I don't think this is a problem. Since we're going to be walking the page tables to fill in the GART table, we can rewrite the PTEs to include a writecombined attribute along the way if we want to. By the way, sre you going to code the vmalloc aperture approach or would you like me to cook something up? Just for the record, my earlier proposal in patch form is attached (diff is against 2.4.9 + drm-2.4.9-pre4.patch) I didn't include the Makefile changes needed to build vmmap.o into to kernel, but otherwise the patch should be complete. To drop this into drm, calls to ioremap() and iounmap() would need to be replaced with calls to agp_ioremap() and agp_iounmap(). This code has been compiled but hasn't been tested in any way. Chris diff -X /cvs/excludes -urN linux-2.4.9-clean/drivers/char/agp/agp.h linux-2.4.9/drivers/char/agp/agp.h --- linux-2.4.9-clean/drivers/char/agp/agp.h Thu Aug 16 15:24:32 2001 +++ linux-2.4.9/drivers/char/agp/agp.h Thu Aug 16 16:07:21 2001 @@ -111,6 +111,7 @@ void (*cleanup) (void); void (*tlb_flush) (agp_memory *); unsigned long (*mask_memory) (unsigned long, int); + unsigned long (*unmask_memory) (unsigned long); void (*cache_flush) (void); int (*create_gatt_table) (void); int (*free_gatt_table) (void); @@ -120,7 +121,11 @@ void (*free_by_type) (agp_memory *); unsigned long (*agp_alloc_page) (void); void (*agp_destroy_page) (unsigned long); + unsigned long (*aper_to_phys) (unsigned long); }; + +extern void *agp_vmmap(unsigned long offset, unsigned long size); +extern void agp_vmunmap(void *handle); #define OUTREG32(mmap, addr, val) __raw_writel((val), (mmap)+(addr)) #define OUTREG16(mmap, addr, val) __raw_writew((val), (mmap)+(addr)) diff -X /cvs/excludes -urN linux-2.4.9-clean/drivers/char/agp/agpgart_be.c linux-2.4.9/drivers/char/agp/agpgart_be.c --- linux-2.4.9-clean/drivers/char/agp/agpgart_be.c Thu Aug 16 15:24:34 2001 +++ linux-2.4.9/drivers/char/agp/agpgart_be.c Thu Aug 16 15:58:58 2001 @@ -56,6 +56,8 @@ EXPORT_SYMBOL(agp_enable); EXPORT_SYMBOL(agp_backend_acquire); EXPORT_SYMBOL(agp_backend_release); +EXPORT_SYMBOL(agp_ioremap); +EXPORT_SYMBOL(agp_iounmap); static void flush_cache(void); @@ -812,6 +814,103 @@ /* End Basic Page Allocation Routines */ +/* Generic routines needed to build specialized kernel maps when + * agp_bridge.cant_use_aperture == 1 */ + +#define IN_VMALLOC(_x) (((_x) >= VMALLOC_START) && ((_x) < VMALLOC_END)) +#define APER_SIZE (((unsigned long)A_SIZE_8(agp_bridge.current_size) \ + ->size) << 20) +#define HIGH_ENOUGH(addr) ((addr) >= agp_bridge.gart_bus_addr) +#define LOW_ENOUGH(addr) ((addr) < agp_bridge.gart_bus_addr + APER_SIZE) +#define IN_APER(addr) (HIGH_ENOUGH(addr) && LOW_ENOUGH(addr)) + +static pte_t * agp_lookup_pte(unsigned long addr) +{ + pgd_t *dir; + pmd_t *pmd; + pte_t *pte; + + if(!IN_VMALLOC(addr)) + return NULL; + + dir = pgd_offset_k(addr); + pmd = pmd_offset(dir, addr); + + if(pmd) { + pte = pte_offset(pmd, addr); + + if(pte) { + return pte; + } else { + return NULL; + } + } else { + return NULL; + } +} + +void *agp_ioremap(unsigned long offset, unsigned long size) +{ + unsigned long handle, vaddr, new; + pte_t *pte; + + if(agp_bridge.cant_use_aperture == 0 || !IN_APER(offset)) + return ioremap(offset, size); + + handle = (unsigned long) agp_vmmap(offset, size); + + for(vaddr = handle; vaddr < handle + size; + vaddr += PAGE_SIZE, offset += PAGE_SIZE) { + pte = agp_lookup_pte(vaddr); + if(pte == NULL) + goto err_exit; + + new = agp_bridge.aper_to_phys(offset); + +#if defined(__ia64__) + *pte = mk_pte_phys(new, __pgprot(pte_val(*pte) & ~_PFN_MASK)); +#else + *pte = mk_pte_phys(new, __pgprot(pte_val(*pte) & ~PAGE_MASK)); +#endif + } + + return ((void *) handle); + +err_exit: + return NULL; +} + +void agp_iounmap(void *handle) +{ + if(agp_bridge.cant_use_aperture == 0 || + !IN_VMALLOC((unsigned long) handle)) { + iounmap(handle); + return; + } + + agp_vmunmap(handle); +} + +unsigned long agp_generic_aper_to_phys(unsigned long apaddr) +{ + unsigned long pgoff, ret; + + if(!IN_APER(apaddr)) + return 0; + + pgoff = (apaddr - agp_bridge.gart_bus_addr) >> PAGE_SHIFT; + ret = agp_bridge.unmask_memory(agp_bridge.gatt_table[pgoff]); + + return ret; +} + +unsigned long agp_generic_unmask_memory(unsigned long entry) +{ + return (entry & PAGE_MASK); +} + +/* End cant_use_aperture kernel map support */ + void agp_enable(u32 mode) { if (agp_bridge.type == NOT_SUPPORTED) return; @@ -1082,6 +1181,7 @@ agp_bridge.cleanup = intel_i810_cleanup; agp_bridge.tlb_flush = intel_i810_tlbflush; agp_bridge.mask_memory = intel_i810_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = intel_i810_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1093,6 +1193,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; } @@ -1275,6 +1376,7 @@ agp_bridge.cleanup = intel_cleanup; agp_bridge.tlb_flush = intel_tlbflush; agp_bridge.mask_memory = intel_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1286,6 +1388,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -1306,6 +1409,7 @@ agp_bridge.cleanup = intel_cleanup; agp_bridge.tlb_flush = intel_tlbflush; agp_bridge.mask_memory = intel_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1317,6 +1421,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -1337,6 +1442,7 @@ agp_bridge.cleanup = intel_cleanup; agp_bridge.tlb_flush = intel_tlbflush; agp_bridge.mask_memory = intel_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1348,6 +1454,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -1455,6 +1562,7 @@ agp_bridge.cleanup = via_cleanup; agp_bridge.tlb_flush = via_tlbflush; agp_bridge.mask_memory = via_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1466,6 +1574,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -1567,6 +1676,7 @@ agp_bridge.cleanup = sis_cleanup; agp_bridge.tlb_flush = sis_tlbflush; agp_bridge.mask_memory = sis_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -1578,6 +1688,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; } @@ -1943,6 +2054,7 @@ agp_bridge.cleanup = amd_irongate_cleanup; agp_bridge.tlb_flush = amd_irongate_tlbflush; agp_bridge.mask_memory = amd_irongate_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = amd_create_gatt_table; @@ -1954,6 +2066,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -2187,6 +2300,7 @@ agp_bridge.cleanup = ali_cleanup; agp_bridge.tlb_flush = ali_tlbflush; agp_bridge.mask_memory = ali_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = agp_generic_agp_enable; agp_bridge.cache_flush = ali_cache_flush; agp_bridge.create_gatt_table = agp_generic_create_gatt_table; @@ -2198,6 +2312,7 @@ agp_bridge.agp_alloc_page = ali_alloc_page; agp_bridge.agp_destroy_page = ali_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; return 0; @@ -2773,6 +2888,7 @@ agp_bridge.cleanup = serverworks_cleanup; agp_bridge.tlb_flush = serverworks_tlbflush; agp_bridge.mask_memory = serverworks_mask_memory; + agp_bridge.unmask_memory = agp_generic_unmask_memory; agp_bridge.agp_enable = serverworks_agp_enable; agp_bridge.cache_flush = global_cache_flush; agp_bridge.create_gatt_table = serverworks_create_gatt_table; @@ -2784,6 +2900,7 @@ agp_bridge.agp_alloc_page = agp_generic_alloc_page; agp_bridge.agp_destroy_page = agp_generic_destroy_page; agp_bridge.cant_use_aperture = 0; + agp_bridge.aper_to_phys = agp_generic_aper_to_phys; pci_read_config_dword(agp_bridge.dev, SVWRKS_APSIZE, @@ -3440,7 +3557,9 @@ &agp_enable, &agp_backend_acquire, &agp_backend_release, - &agp_copy_info + &agp_copy_info, + &agp_ioremap, + &agp_iounmap }; static int __init agp_init(void) diff -X /cvs/excludes -urN linux-2.4.9-clean/drivers/char/agp/vmmap.c linux-2.4.9/drivers/char/agp/vmmap.c --- linux-2.4.9-clean/drivers/char/agp/vmmap.c Wed Dec 31 16:00:00 1969 +++ linux-2.4.9/drivers/char/agp/vmmap.c Thu Aug 16 15:02:41 2001 @@ -0,0 +1,225 @@ +/* + * vmmap.c + * + * Hack to allow virtual addressing fixups in the kernel's + * vmalloc area. This is needed so that the page tables in the + * kernel's vmalloc area can be used to emulate GART translation + * + * This file is basically a copy of mm/vmalloc.c modified to not + * allocate and free pages. + * + * Chris Ahna <chr...@in...> + */ + +#include <linux/config.h> +#include <linux/slab.h> +#include <linux/vmalloc.h> +#include <linux/spinlock.h> +#include <linux/module.h> +#include <linux/agp_backend.h> +#include <linux/highmem.h> + +#include <asm/uaccess.h> +#include <asm/pgalloc.h> + +#include "agp.h" + +EXPORT_SYMBOL(agp_vmmap); +EXPORT_SYMBOL(agp_vmunmap); + +static inline void agp_free_area_pte(pmd_t * pmd, unsigned long address, unsigned long size) +{ + pte_t * pte; + unsigned long end; + + if (pmd_none(*pmd)) + return; + if (pmd_bad(*pmd)) { + pmd_ERROR(*pmd); + pmd_clear(pmd); + return; + } + pte = pte_offset(pmd, address); + address &= ~PMD_MASK; + end = address + size; + if (end > PMD_SIZE) + end = PMD_SIZE; + do { + pte_t page; + page = ptep_get_and_clear(pte); + address += PAGE_SIZE; + pte++; + if (pte_none(page) || pte_present(page)) { + continue; + } + printk(KERN_CRIT "Whee.. Swapped out page in kernel page table\n"); + } while (address < end); +} + +static inline void agp_free_area_pmd(pgd_t * dir, unsigned long address, unsigned long size) +{ + pmd_t * pmd; + unsigned long end; + + if (pgd_none(*dir)) + return; + if (pgd_bad(*dir)) { + pgd_ERROR(*dir); + pgd_clear(dir); + return; + } + pmd = pmd_offset(dir, address); + address &= ~PGDIR_MASK; + end = address + size; + if (end > PGDIR_SIZE) + end = PGDIR_SIZE; + do { + agp_free_area_pte(pmd, address, end - address); + address = (address + PMD_SIZE) & PMD_MASK; + pmd++; + } while (address < end); +} + +void agp_vmfree_area_pages(unsigned long address, unsigned long size) +{ + pgd_t * dir; + unsigned long end = address + size; + + dir = pgd_offset_k(address); + flush_cache_all(); + do { + agp_free_area_pmd(dir, address, end - address); + address = (address + PGDIR_SIZE) & PGDIR_MASK; + dir++; + } while (address && (address < end)); + flush_tlb_all(); +} + +static inline int agp_alloc_area_pte (pte_t * pte, unsigned long address, + unsigned long size, unsigned long target, pgprot_t prot) +{ + unsigned long end; + + address &= ~PMD_MASK; + end = address + size; + if (end > PMD_SIZE) + end = PMD_SIZE; + target = (unsigned long) __va(target); + do { + struct page * page; + page = virt_to_page(target); + if (!pte_none(*pte)) + printk(KERN_ERR "alloc_area_pte: page already exists\n"); + if (!page) + return -ENOMEM; + set_pte(pte, mk_pte(page, prot)); + address += PAGE_SIZE; + target += PAGE_SIZE; + pte++; + } while (address < end); + return 0; +} + +static inline int agp_alloc_area_pmd(pmd_t * pmd, unsigned long address, unsigned long size, unsigned long target, pgprot_t prot) +{ + unsigned long end; + + address &= ~PGDIR_MASK; + end = address + size; + if (end > PGDIR_SIZE) + end = PGDIR_SIZE; + do { + pte_t * pte = pte_alloc(&init_mm, pmd, address); + if (!pte) + return -ENOMEM; + if (agp_alloc_area_pte(pte, address, end - address, target, prot)) + return -ENOMEM; + target += ((address + PMD_SIZE) & PMD_MASK) - address; + address = (address + PMD_SIZE) & PMD_MASK; + pmd++; + } while (address < end); + return 0; +} + +inline int agp_vmalloc_area_pages (unsigned long address, unsigned long size, + unsigned long target, pgprot_t prot) +{ + pgd_t * dir; + unsigned long end = address + size; + int ret; + + dir = pgd_offset_k(address); + flush_cache_all(); + spin_lock(&init_mm.page_table_lock); + do { + pmd_t *pmd; + + pmd = pmd_alloc(&init_mm, dir, address); + ret = -ENOMEM; + if (!pmd) + break; + + ret = -ENOMEM; + if (agp_alloc_area_pmd(pmd, address, end - address, target, prot)) + break; + + target += ((address + PGDIR_SIZE) & PGDIR_MASK) - address; + address = (address + PGDIR_SIZE) & PGDIR_MASK; + dir++; + + ret = 0; + } while (address && (address < end)); + spin_unlock(&init_mm.page_table_lock); + flush_tlb_all(); + return ret; +} + +void agp_vmunmap(void *addr) +{ + struct vm_struct **p, *tmp; + + if (!addr) + return; + if ((PAGE_SIZE-1) & (unsigned long) addr) { + printk(KERN_ERR "Trying to vfree() bad address (%p)\n", addr); + return; + } + write_lock(&vmlist_lock); + for (p = &vmlist ; (tmp = *p) ; p = &tmp->next) { + if (tmp->addr == addr) { + *p = tmp->next; + agp_vmfree_area_pages(VMALLOC_VMADDR(tmp->addr), tmp->size); + write_unlock(&vmlist_lock); + kfree(tmp); + return; + } + } + write_unlock(&vmlist_lock); + printk(KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n", addr); +} + +void *__agp_vmmap (unsigned long size, unsigned long target, pgprot_t prot) +{ + void * addr; + struct vm_struct *area; + + size = PAGE_ALIGN(size); + if (!size) { + BUG(); + return NULL; + } + area = get_vm_area(size, VM_ALLOC); + if (!area) + return NULL; + addr = area->addr; + if (agp_vmalloc_area_pages(VMALLOC_VMADDR(addr), size, target, prot)) { + vfree(addr); + return NULL; + } + return addr; +} + +void *agp_vmmap(unsigned long offset, unsigned long size) +{ + return __agp_vmmap(size, offset, pgprot_writecombine(PAGE_KERNEL)); +} diff -X /cvs/excludes -urN linux-2.4.9-clean/include/linux/agp_backend.h linux-2.4.9/include/linux/agp_backend.h --- linux-2.4.9-clean/include/linux/agp_backend.h Thu Aug 16 15:24:34 2001 +++ linux-2.4.9/include/linux/agp_backend.h Thu Aug 16 15:46:05 2001 @@ -239,6 +239,25 @@ * */ +extern void *agp_ioremap(unsigned long offset, unsigned long size); + +/* + * agp_ioremap : + * + * This Function handles ioremap's which overlap + * the AGP aperture for systems where the AGP + * aperture is not directly accessible. + */ + +extern void agp_iounmap(void *handle); + +/* + * agp_iounmap : + * + * This Function releases a previously acquired + * agp_ioremap. + */ + typedef struct { void (*free_memory)(agp_memory *); agp_memory *(*allocate_memory)(size_t, u32); @@ -248,6 +267,8 @@ int (*acquire)(void); void (*release)(void); void (*copy_info)(agp_kern_info *); + void *(*ioremap)(unsigned long, unsigned long); + void (*iounmap)(void *); } drm_agp_t; extern const drm_agp_t *drm_agp_p; |
From: Benjamin H. <be...@ke...> - 2001-08-17 10:07:56
|
> >Okay sounds like this approach will work for you. Benjamin does it make >sense to you? Your vmalloc approach makes sense and would fit my needs. I will probably have to tweak the caching attributes, but that's all. Ben. |
From: Benjamin H. <be...@ke...> - 2001-08-17 10:07:20
|
I'm a bit late on my implemetation, I'm having some problems with the ring not emptying. It seem to be some kind of coherency problem, maybe related to cache aliasing problems. I'm investigating. >I was thinking about this today in fact, I have a patch that missed the >window for 2.4.9 which provides some functionality to support user land >mappings in a generic way. I'll attach this patch (against 2.4.9-pre4) >to the end of this email. I'd like to have a way to deal with the in >kernel mappings in a generic way as well. I'd like to see what your >proposing in patch form. I use your nopage function with only slight change to the mmap routine to set the _PAGE_NO_CACHE bit on PPC. >I really want to just have the drm writing to the agp memory structure >directly in the future. I really can't do that on ia32 just yet >(because I am not submitting PAT till 2.5), so I guess an agp_ioremap >could work. However perhaps we could make the API available on arch's >that need this and just write to the vmalloc'ed memory ourselves. > >All agp memory allocations become vmalloc's if we can't write to the >aperture. >We populate the agp aperture by walking the page tables of the >vmalloc'ed region inside agpgart. We can deal with page size issues here. >We put a link to the vmalloc'ed region in the agp_memory structure. >If the cant_use_aperture flag is set, we will read/write to the >vmalloc'ed pointer. On arch's that can write to the aperture we just do >things the same way we have been doing them. The only thing I'm >concerned about is that we can get a write combining cache type on ia64 >from vmalloc. Sounds a bit simpler than the agp_ioremap solution. If I'm not mistaken that is similar to what PCIGART does on ATI ? The write combining might be solvable using __vmalloc. But it leads to another problem I'd like to deal with. Basically, AGP memory is supposed to be non-coherent, and so has to be set non-cachable. On PPC, without cache, you also lose write combining, at least on the CPU side (the bridge can still try to write combine). But this is also a big performance hit. If it was possible to keep the cache enabled for AGP memory (at least in write through mode, causing cache lines to be loaded only on reads), perfs would certainly be better, but that require apropriate flush & invalidate operations, especially if reading back from the aperture. I don't know the DRI well enough to decide if this is a realistic option. >I guess I'm willing to do either one, however the second solution does >not require changes to the mainline kernel outside of agpgart. Would >this work for everyone? Does anyone see problems with it? > >-Jeff |
From: Chris A. <chr...@in...> - 2001-08-17 18:42:22
|
On Fri, Aug 17, 2001 at 12:30:53AM +0200, Benjamin Herrenschmidt wrote: > But it leads to another problem I'd like to deal with. > > Basically, AGP memory is supposed to be non-coherent, and so has to be > set non-cachable. On PPC, without cache, you also lose write combining, > at least on the CPU side (the bridge can still try to write combine). > But this is also a big performance hit. If it was possible to keep the > cache enabled for AGP memory (at least in write through mode, causing > cache lines to be loaded only on reads), perfs would certainly be better, > but that require apropriate flush & invalidate operations, especially if > reading back from the aperture. > I don't know the DRI well enough to decide if this is a realistic option. As long as you can make <driver>_flush_write_combine() do the needed flush and invalidate actions, you should be able to map AGP memory with an attribute other than _PAGE_NO_CACHE. On ia64 this flexibility allows all AGP memory to be mapped writecombined without any changes to DRM. One other thing to checkout, on 460GX there's a coherency bit in each GART table entry that makes accesses from the card into the aperture cache coherent. This is how direct rendering was done for a few months on ia64, with all memory mapped writeback and the coherency bit on in every GART entry. UniNorth probably doesn't support this, but if it did I think it'd help solve your problems. Chris |
From: Benjamin H. <be...@ke...> - 2001-08-17 19:30:56
|
> >As long as you can make <driver>_flush_write_combine() do the needed >flush and invalidate actions, you should be able to map AGP memory with >an attribute other than _PAGE_NO_CACHE. On ia64 this flexibility >allows all AGP memory to be mapped writecombined without any changes to >DRM. Hrm... who exactly touch the AGP memory ? I beleive both the in-kernel DRM driver and the userland part. They must take care of it. Also, it looks like the r128 driver also reads from the AGP aperture for the ring pointer. That can be handled with a simple flush, the cache line must be properly invalidated, and any possible contention between card access and CPU access solved, which might even be impossible. >One other thing to checkout, on 460GX there's a coherency bit in each >GART table entry that makes accesses from the card into the aperture >cache coherent. This is how direct rendering was done for a few months >on ia64, with all memory mapped writeback and the coherency bit on in >every GART entry. UniNorth probably doesn't support this, but if it did >I think it'd help solve your problems. I wouldn't bet on it ;) Newer revisions of UniNorth may allow that (they do allow at least x4 and fast write), unfortunately, this is not documented, and the Darwin's driver contains no mention of such things. Ben. |
From: Chris A. <chr...@in...> - 2001-08-17 20:41:46
|
On Fri, Aug 17, 2001 at 09:29:20PM +0200, Benjamin Herrenschmidt wrote: > Hrm... who exactly touch the AGP memory ? I beleive both the in-kernel > DRM driver and the userland part. They must take care of it. Also, it > looks like the r128 driver also reads from the AGP aperture for the > ring pointer. That can be handled with a simple flush, the cache line > must be properly invalidated, and any possible contention between > card access and CPU access solved, which might even be impossible. I think you're right, bother user and kernel level code touches AGP memory. However, it seems that a <driver>_flush_write_combine() is issued every time the driver wants the card to pick up new data. On most hardware, <driver>_flush_write_combine() will push all writecombined transactions out to memory. It would probably be tough to make this efficiently flush exactly the memory you need though. As far as the ring_read_ptr goes, on ia64 I had to relocate this outside of the AGP aperture to work around a deficiency of the chipset. I believe writes from the card to memory outside of the AGP aperture are required to be cache coherent, so maybe this relocation can help you out as well. > I wouldn't bet on it ;) Newer revisions of UniNorth may allow that > (they do allow at least x4 and fast write), unfortunately, this is not > documented, and the Darwin's driver contains no mention of such things. Are you writing the driver without specs on the UniNorth? If you are, then I guess it'd be real tough to know if this was supported. Chris |
From: Benjamin H. <be...@ke...> - 2001-08-17 20:57:59
|
>On Fri, Aug 17, 2001 at 09:29:20PM +0200, Benjamin Herrenschmidt wrote: > >I think you're right, bother user and kernel level code touches AGP >memory. However, it seems that a <driver>_flush_write_combine() is >issued every time the driver wants the card to pick up new data. On >most hardware, <driver>_flush_write_combine() will push all >writecombined transactions out to memory. It would probably be tough to >make this efficiently flush exactly the memory you need though. > >As far as the ring_read_ptr goes, on ia64 I had to relocate this outside >of the AGP aperture to work around a deficiency of the chipset. I >believe writes from the card to memory outside of the AGP aperture are >required to be cache coherent, so maybe this relocation can help you >out as well. That's interesting. Your workaround may help me figure out if the bug I'm chasing is really a coherency problem or not. Basically, I'm getting error messages ([drm:r128_cce_vertex] *ERROR* ring space check failed! and friends) after a few seconds of running 3D stuffs, and I think they can be related to a problem of coherency. Well, maybe my chipset have the same kind of bug you are talking about. Could you elaborate on the bug you had ? > >> I wouldn't bet on it ;) Newer revisions of UniNorth may allow that >> (they do allow at least x4 and fast write), unfortunately, this is not >> documented, and the Darwin's driver contains no mention of such things. > >Are you writing the driver without specs on the UniNorth? If you are, >then I guess it'd be real tough to know if this was supported. Yes. All I have is the Darwin source which is pretty primitive and seem to only concern older versions of UniNorth (Apple have been silently "hiding" some drivers from Darwin lately, I hope it's just temporary). That was enough to figure out the couple of registers to tell the chipset where the aperture is, what size it is, and the format of the entries. Ben. |
From: Michel <mic...@ii...> - 2001-08-17 21:24:02
|
Chris Ahna wrote: > > On Fri, Aug 17, 2001 at 09:29:20PM +0200, Benjamin Herrenschmidt wrote: > > Hrm... who exactly touch the AGP memory ? I beleive both the in-kernel > > DRM driver and the userland part. They must take care of it. Also, it > > looks like the r128 driver also reads from the AGP aperture for the > > ring pointer. That can be handled with a simple flush, the cache line > > must be properly invalidated, and any possible contention between > > card access and CPU access solved, which might even be impossible. > > I think you're right, bother user and kernel level code touches AGP > memory. However, it seems that a <driver>_flush_write_combine() is > issued every time the driver wants the card to pick up new data. On > most hardware, <driver>_flush_write_combine() will push all > writecombined transactions out to memory. It would probably be tough to > make this efficiently flush exactly the memory you need though. At least r128_flush_write_combine() is defined as mb(), does that serve the purpose on PPC? -- Earthling Michel Dänzer (MrCooper) \ Debian GNU/Linux (powerpc) developer CS student, Free Software enthusiast \ XFree86 and DRI project member |