Messages in this thread | | | Subject | Re: [RFC] Make AGP work with IOMMU | From | David Woodhouse <> | Date | Wed, 29 Jul 2009 10:01:08 +0100 |
| |
On Wed, 2009-07-29 at 18:43 +1000, Dave Airlie wrote: > On Wed, Jul 29, 2009 at 5:15 PM, David Woodhouse<dwmw2@infradead.org> wrote: > > On Wed, 2009-07-29 at 16:28 +1000, Dave Airlie wrote: > >> Yup pretty much we always got lucky, its not like AGP and IOMMU systems > >> are a huge item, its really only Intel IGPs which use the AGP > >> subsystem these days. > > > > Ah, really? One thing which was bothering me was what happens when I use > > non-onboard graphics in one of these beasts -- are individual gfx > > drivers going to need to be fixed too? > > Yes, more than likely. I think radeon and radeon kms drivers dtrt, at > least they call pci_map_single for the pages they use in their on-chip > translation units, and the cards have been used in sparc64 and ppc64 > with iommu stuff happening.
It's going to perform a lot better if we use pci_map_sg() to map as many pages as possible at once.
> nouveau might work when it gets upstream but it may not. > > Other PCI and/or PCIE1x cards still around would be MGA and SiS based, > and I suspect they would need work, but they might just work. > > nvidia/fglrx binary drivers, who knows.
... or cares.
> > > >> > cur_gatt = GET_GATT(addr); > >> > writel(agp_generic_mask_memory(agp_bridge, > >> > - mem->pages[i], mem->type), cur_gatt+GET_GATT_OFF(addr)); > >> > + phys_to_gart(page_to_phys(mem->pages[i])), > >> > >> don't suppose we want page_to_gart or is the double function nicer? > > > > I pondered that briefly. But then observed that phys_to_gart() and > > gart_to_phys() _always_ describe an identity mapping, so perhaps they > > could just be ditched completely? > > Yeah that could work too, no idea why they were introduced, well before my time. > > > > >> > @@ -150,8 +150,17 @@ static int agp_backend_initialize(struct agp_bridge_data *bridge) > >> > } > >> > > >> > bridge->scratch_page_real = phys_to_gart(page_to_phys(page)); > >> > - bridge->scratch_page = > >> > - bridge->driver->mask_memory(bridge, page, 0); > >> > + bridge->scratch_page = bridge->driver->mask_memory(bridge, > >> > + phys_to_gart(page_to_phys(page)), 0); > >> > + > >> > + if (bridge->driver->agp_map_page && > >> > + bridge->driver->agp_map_page(phys_to_virt(page_to_phys(page)), > >> > >> and maybe page_to_virt. > > > > That's called page_address(), and it (as well as the above construct) is > > broken with highmem pages. It's actually OK here, since this page is > > allocated with GFP_DMA32 -- but for cleanliness' sake I should probably > > switch agp_map_page() to take a 'struct page *' rather than a virtual > > address. > > Yes that might be best, having a function called map_page taking an > address seems wrong.
http://git.infradead.org/users/dwmw2/iommu-agp.git?a=commitdiff;h=71fc2ae5
> > > >> > + if ((mem->page_count * sizeof(*mem->sg_list)) < 2*PAGE_SIZE) > >> > + mem->sg_list = kcalloc(mem->page_count, sizeof(*mem->sg_list), > >> > + GFP_KERNEL); > >> > + > >> > + if (mem->sg_list == NULL) { > >> > + mem->sg_list = vmalloc(mem->page_count * sizeof(*mem->sg_list)); > >> > + mem->sg_vmalloc_flag = 1; > >> > >> Can we drop vmalloc_flag and use is_vmalloc_addr on the free function? > > > > I suppose so -- we could eliminate the other vmalloc_flag field in > > 'struct agp_memory' that way too? Doesn't shrink the structure any -- > > we'd just end up with padding where the flags were. > > Yeah we should drop both of them maybe I can do that as a cleanup > after this patch. > > > > >> (aside: yet another place that wants a kmalloc/vmalloc allocator. I suspect > >> vmalloc here to be slow but I suppose there isn't much we can do.) > > > > http://lwn.net/Articles/342915/ ? > > > > In fact, can't scatterlists do something like that already? > > no idea, I think I've seen vmalloc used in other places before.
This fixes it for scatterlists: http://git.infradead.org/users/dwmw2/iommu-agp.git?a=commitdiff;h=89187864
-- David Woodhouse Open Source Technology Centre David.Woodhouse@intel.com Intel Corporation
| |