Re: "no scatter/gather memory" ?
On Friday, March 04, 2005 6:01 pm, Roland Scheidegger wrote: > Paul Mackerras wrote: > > Note that that check is also wrong for ppc64. I think it is going to > > be wrong for most 64-bit platforms, since it is assuming that you can > > never have ram at a higher physical address than any I/O devices. On > > 64-bit platforms it is quite common to have some ram and some I/O > > below 4GB, and some more ram above 4GB. > > > > I don't see why we need the check anyway, unless some architecture > > (x86?) will actually panic if you try to ioremap a physical address > > that is below virt_to_phys(high_memory) or something. > > Wouldn't this check even break on x86 with PAE? Those boxes certainly > have parts of their ram mapped above io memory too. Or does that > high_memory variable stay below 4GB with PAE? I think the start of high memory will stay below 4G, but the check should probably be removed anyway. If we really want to make sure that a given offset is in I/O space, we should check that explicitly, and not rely on some 'top of real memory' type variable, since that's inherently non-portable. Jesse --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
Paul Mackerras wrote: Note that that check is also wrong for ppc64. I think it is going to be wrong for most 64-bit platforms, since it is assuming that you can never have ram at a higher physical address than any I/O devices. On 64-bit platforms it is quite common to have some ram and some I/O below 4GB, and some more ram above 4GB. I don't see why we need the check anyway, unless some architecture (x86?) will actually panic if you try to ioremap a physical address that is below virt_to_phys(high_memory) or something. Wouldn't this check even break on x86 with PAE? Those boxes certainly have parts of their ram mapped above io memory too. Or does that high_memory variable stay below 4GB with PAE? Roland --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
Paul Mackerras wrote: Stephane Marchesin writes: Ok, it looks like drm cvs (core and non core) has been broken on ia64 since august. Patch attached. Stephane Index: linux/drm_bufs.h === RCS file: /cvs/dri/drm/linux/drm_bufs.h,v retrieving revision 1.45 diff -u -r1.45 drm_bufs.h --- linux/drm_bufs.h16 Jan 2005 05:40:12 - 1.45 +++ linux/drm_bufs.h4 Mar 2005 14:10:26 - @@ -74,7 +74,7 @@ if ( (offset & (~PAGE_MASK)) || (size & (~PAGE_MASK)) ) return -EINVAL; -#if !defined(__sparc__) && !defined(__alpha__) +#if !defined(__sparc__) && !defined(__alpha__) && !defined(__ia64__) if ( offset + size < offset || offset < virt_to_phys(high_memory) ) return -EINVAL; #endif Note that that check is also wrong for ppc64. I think it is going to be wrong for most 64-bit platforms, since it is assuming that you can never have ram at a higher physical address than any I/O devices. On 64-bit platforms it is quite common to have some ram and some I/O below 4GB, and some more ram above 4GB. There are other places where this is done as well. You might want to check. Also, as we said, you'll need to change offset into an unsigned long. Btw, does drm cvs work for you on ppc64 ? I don't see why we need the check anyway, unless some architecture (x86?) will actually panic if you try to ioremap a physical address that is below virt_to_phys(high_memory) or something. No idea. Stephane --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
Stephane Marchesin writes: > Ok, it looks like drm cvs (core and non core) has been broken on ia64 > since august. Patch attached. > > Stephane > > Index: linux/drm_bufs.h > === > RCS file: /cvs/dri/drm/linux/drm_bufs.h,v > retrieving revision 1.45 > diff -u -r1.45 drm_bufs.h > --- linux/drm_bufs.h 16 Jan 2005 05:40:12 - 1.45 > +++ linux/drm_bufs.h 4 Mar 2005 14:10:26 - > @@ -74,7 +74,7 @@ > > if ( (offset & (~PAGE_MASK)) || (size & (~PAGE_MASK)) ) > return -EINVAL; > -#if !defined(__sparc__) && !defined(__alpha__) > +#if !defined(__sparc__) && !defined(__alpha__) && !defined(__ia64__) > if ( offset + size < offset || offset < virt_to_phys(high_memory) ) > return -EINVAL; > #endif Note that that check is also wrong for ppc64. I think it is going to be wrong for most 64-bit platforms, since it is assuming that you can never have ram at a higher physical address than any I/O devices. On 64-bit platforms it is quite common to have some ram and some I/O below 4GB, and some more ram above 4GB. I don't see why we need the check anyway, unless some architecture (x86?) will actually panic if you try to ioremap a physical address that is below virt_to_phys(high_memory) or something. Paul. --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
On Friday, March 4, 2005 10:31 am, Jesse Barnes wrote: > Seems like we need something like this instead? > > Index: linux-core/drm_dma.c > === > RCS file: /cvs/dri/drm/linux-core/drm_dma.c,v > retrieving revision 1.39 > diff -u -r1.39 drm_dma.c > --- linux-core/drm_dma.c31 Oct 2004 15:16:44 - 1.39 > +++ linux-core/drm_dma.c4 Mar 2005 18:29:03 - > @@ -141,6 +141,9 @@ > buf->filp = NULL; > buf->used = 0; > > + pci_unmap_page(dev->pdev, buf->bus_address, buf->total, > + PCI_DMA_FROMDEVICE); > + This is wrong since we may get here with AGP or other memory that wasn't pci_map*'d in the first place. It should also use pci_unmap_single instead. > if (drm_core_check_feature(dev, DRIVER_DMA_QUEUE) > && waitqueue_active(&buf->dma_wait)) { > wake_up_interruptible(&buf->dma_wait); > Index: linux-core/drm_bufs.c > === > RCS file: /cvs/dri/drm/linux-core/drm_bufs.c,v > retrieving revision 1.54 > diff -u -r1.54 drm_bufs.c > --- linux-core/drm_bufs.c 5 Feb 2005 08:00:14 - 1.54 > +++ linux-core/drm_bufs.c 4 Mar 2005 18:29:04 - > @@ -752,7 +752,9 @@ > buf->used = 0; > buf->offset = (dma->byte_count + byte_count + > offset); buf->address = (void *)(page + offset); > - buf->bus_address = virt_to_bus(buf->address); > + buf->bus_address = pci_map_page(dev->pdev, page, > offset, + buf->total, > + PCI_DMA_TODEVICE); > buf->next = NULL; This should be pci_map_single instead I think, since the buffer may be more than one page like you said? buf->bus_address = pci_map_single(dev->pdev, bus->address, buf->total, PCI_DMA_BIDIRECTIONAL); Jesse --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
On Friday, March 4, 2005 10:05 am, Stephane Marchesin wrote: > >It also looks like drm_addbufs_pci uses virt_to_bus, which won't work on > > many non-x86 platforms. Hmm... the version in latest 2.6 kernel tree > > doesn't seem to have these problems, which one is the master copy? > > It's drm cvs, which gets merged into 2.6 from time to time. Well, we'd better fix it before it gets merged then! :) In particular, this snippet from linux-core/drm_bufs.c:drm_addbufs_pci: buf->order = order; buf->used = 0; buf->offset = (dma->byte_count + byte_count + offset); buf->address = (void *)(page + offset); buf->bus_address = virt_to_bus(buf->address); buf->next = NULL; buf->waiting = 0; buf->pending = 0; It looks like the bus_address field usage here is new? At least I don't see it in the same routine in the latest 2.6 kernel tree. The problem with virt_to_bus is that it doesn't specify *which* bus, so it's only suitable on machines with very simple bus layouts (and I/O cache coherent ones at that). Seems like we need something like this instead? Index: linux-core/drm_dma.c === RCS file: /cvs/dri/drm/linux-core/drm_dma.c,v retrieving revision 1.39 diff -u -r1.39 drm_dma.c --- linux-core/drm_dma.c31 Oct 2004 15:16:44 - 1.39 +++ linux-core/drm_dma.c4 Mar 2005 18:29:03 - @@ -141,6 +141,9 @@ buf->filp = NULL; buf->used = 0; + pci_unmap_page(dev->pdev, buf->bus_address, buf->total, + PCI_DMA_FROMDEVICE); + if (drm_core_check_feature(dev, DRIVER_DMA_QUEUE) && waitqueue_active(&buf->dma_wait)) { wake_up_interruptible(&buf->dma_wait); Index: linux-core/drm_bufs.c === RCS file: /cvs/dri/drm/linux-core/drm_bufs.c,v retrieving revision 1.54 diff -u -r1.54 drm_bufs.c --- linux-core/drm_bufs.c 5 Feb 2005 08:00:14 - 1.54 +++ linux-core/drm_bufs.c 4 Mar 2005 18:29:04 - @@ -752,7 +752,9 @@ buf->used = 0; buf->offset = (dma->byte_count + byte_count + offset); buf->address = (void *)(page + offset); - buf->bus_address = virt_to_bus(buf->address); + buf->bus_address = pci_map_page(dev->pdev, page, offset, + buf->total, + PCI_DMA_TODEVICE); buf->next = NULL; buf->waiting = 0; buf->pending = 0; --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
Jesse Barnes wrote: On Friday, March 4, 2005 6:20 am, Stephane Marchesin wrote: Stephane Marchesin wrote: Hi, I upgraded drm (non core) to current cvs (previous one was from 2004-07-15) on an ia64 with a radeon 7000 (pci) and now on inserting the module I get : [drm:radeon_ati_pcigart_cleanup] *ERROR* no scatter/gather memory! [drm:radeon_do_cleanup_cp] *ERROR* failed to cleanup PCI GART! Ok, it looks like drm cvs (core and non core) has been broken on ia64 since august. Patch attached. What is this code trying to do anyway? It looks like it's checking for overflow and trying to make sure that the offset is in I/O space? Are those checks really necessary? Well... they probably check that you can fit the result of pci_resource_start into an unsigned int :) (I'm only half joking - see radeon_cp.c which passes that as the offset argument for initmap). So yes, initmap should have an unsigned long or some kind of pointer as its second arg instead... It also looks like drm_addbufs_pci uses virt_to_bus, which won't work on many non-x86 platforms. Hmm... the version in latest 2.6 kernel tree doesn't seem to have these problems, which one is the master copy? It's drm cvs, which gets merged into 2.6 from time to time. Stephane --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
On Friday, March 4, 2005 6:20 am, Stephane Marchesin wrote: > Stephane Marchesin wrote: > > Hi, > > > > I upgraded drm (non core) to current cvs (previous one was from > > 2004-07-15) on an ia64 with a radeon 7000 (pci) and now on inserting > > the module I get : > > [drm:radeon_ati_pcigart_cleanup] *ERROR* no scatter/gather memory! > > [drm:radeon_do_cleanup_cp] *ERROR* failed to cleanup PCI GART! > > Ok, it looks like drm cvs (core and non core) has been broken on ia64 > since august. Patch attached. What is this code trying to do anyway? It looks like it's checking for overflow and trying to make sure that the offset is in I/O space? Are those checks really necessary? It also looks like drm_addbufs_pci uses virt_to_bus, which won't work on many non-x86 platforms. Hmm... the version in latest 2.6 kernel tree doesn't seem to have these problems, which one is the master copy? Jesse --- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click -- ___ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: "no scatter/gather memory" ?
Stephane Marchesin wrote: Hi, I upgraded drm (non core) to current cvs (previous one was from 2004-07-15) on an ia64 with a radeon 7000 (pci) and now on inserting the module I get : [drm:radeon_ati_pcigart_cleanup] *ERROR* no scatter/gather memory! [drm:radeon_do_cleanup_cp] *ERROR* failed to cleanup PCI GART! Ok, it looks like drm cvs (core and non core) has been broken on ia64 since august. Patch attached. Stephane Index: linux/drm_bufs.h === RCS file: /cvs/dri/drm/linux/drm_bufs.h,v retrieving revision 1.45 diff -u -r1.45 drm_bufs.h --- linux/drm_bufs.h16 Jan 2005 05:40:12 - 1.45 +++ linux/drm_bufs.h4 Mar 2005 14:10:26 - @@ -74,7 +74,7 @@ if ( (offset & (~PAGE_MASK)) || (size & (~PAGE_MASK)) ) return -EINVAL; -#if !defined(__sparc__) && !defined(__alpha__) +#if !defined(__sparc__) && !defined(__alpha__) && !defined(__ia64__) if ( offset + size < offset || offset < virt_to_phys(high_memory) ) return -EINVAL; #endif Index: linux-core/drm_bufs.c === RCS file: /cvs/dri/drm/linux-core/drm_bufs.c,v retrieving revision 1.54 diff -u -r1.54 drm_bufs.c --- linux-core/drm_bufs.c 5 Feb 2005 08:00:14 - 1.54 +++ linux-core/drm_bufs.c 4 Mar 2005 14:10:27 - @@ -63,7 +63,7 @@ if ((offset & (~PAGE_MASK)) || (size & (~PAGE_MASK))) return -EINVAL; -#if !defined(__sparc__) && !defined(__alpha__) +#if !defined(__sparc__) && !defined(__alpha__) && !defined(__ia64__) if (offset + size < offset || offset < virt_to_phys(high_memory)) return -EINVAL; #endif