2007-11-25 23:43:09

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: [RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

This patch fixes bits of the DRM so to make the radeon DRI work on
non-cache coherent PCI DMA variants of the PowerPC processors.

It moves the few places that needs change to wrappers to that
other architectures with similar issues can easily add their
own changes to those wrappers, at least until we have more useful
generic kernel API.

Signed-off-by: Benjamin Herrenschmidt <[email protected]>
---

drivers/char/drm/ati_pcigart.c | 6 ++++++
drivers/char/drm/drm_scatter.c | 12 +++++++++++-
drivers/char/drm/drm_vm.c | 20 +++++++++++++++-----
3 files changed, 32 insertions(+), 6 deletions(-)

Index: linux-work/drivers/char/drm/ati_pcigart.c
===================================================================
--- linux-work.orig/drivers/char/drm/ati_pcigart.c 2007-11-26 10:07:29.000000000 +1100
+++ linux-work/drivers/char/drm/ati_pcigart.c 2007-11-26 10:21:33.000000000 +1100
@@ -214,6 +214,12 @@ int drm_ati_pcigart_init(struct drm_devi
}
}

+ if (gart_info->gart_table_location == DRM_ATI_GART_MAIN)
+ dma_sync_single_for_device(&dev->pdev->dev,
+ bus_address,
+ max_pages * sizeof(u32),
+ PCI_DMA_TODEVICE);
+
ret = 1;

#if defined(__i386__) || defined(__x86_64__)
Index: linux-work/drivers/char/drm/drm_scatter.c
===================================================================
--- linux-work.orig/drivers/char/drm/drm_scatter.c 2007-11-26 10:07:29.000000000 +1100
+++ linux-work/drivers/char/drm/drm_scatter.c 2007-11-26 10:20:08.000000000 +1100
@@ -36,6 +36,16 @@

#define DEBUG_SCATTER 0

+static inline void *drm_vmalloc_dma(unsigned long size)
+{
+#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
+ return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM,
+ PAGE_KERNEL | _PAGE_NO_CACHE);
+#else
+ return vmalloc_32(size);
+#endif
+}
+
void drm_sg_cleanup(struct drm_sg_mem * entry)
{
struct page *page;
@@ -104,7 +114,7 @@ int drm_sg_alloc(struct drm_device *dev,
}
memset((void *)entry->busaddr, 0, pages * sizeof(*entry->busaddr));

- entry->virtual = vmalloc_32(pages << PAGE_SHIFT);
+ entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT);
if (!entry->virtual) {
drm_free(entry->busaddr,
entry->pages * sizeof(*entry->busaddr), DRM_MEM_PAGES);
Index: linux-work/drivers/char/drm/drm_vm.c
===================================================================
--- linux-work.orig/drivers/char/drm/drm_vm.c 2007-11-26 10:07:29.000000000 +1100
+++ linux-work/drivers/char/drm/drm_vm.c 2007-11-26 10:11:09.000000000 +1100
@@ -54,13 +54,24 @@ static pgprot_t drm_io_prot(uint32_t map
pgprot_val(tmp) |= _PAGE_NO_CACHE;
if (map_type == _DRM_REGISTERS)
pgprot_val(tmp) |= _PAGE_GUARDED;
-#endif
-#if defined(__ia64__)
+#elif defined(__ia64__)
if (efi_range_is_wc(vma->vm_start, vma->vm_end -
vma->vm_start))
tmp = pgprot_writecombine(tmp);
else
tmp = pgprot_noncached(tmp);
+#elif defined(__sparc__)
+ tmp = pgprot_noncached(tmp);
+#endif
+ return tmp;
+}
+
+static pgprot_t drm_dma_prot(uint32_t map_type, struct vm_area_struct *vma)
+{
+ pgprot_t tmp = vm_get_page_prot(vma->vm_flags);
+
+#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
+ tmp |= _PAGE_NO_CACHE;
#endif
return tmp;
}
@@ -617,9 +628,6 @@ static int drm_mmap_locked(struct file *
offset = dev->driver->get_reg_ofs(dev);
vma->vm_flags |= VM_IO; /* not in core dump */
vma->vm_page_prot = drm_io_prot(map->type, vma);
-#ifdef __sparc__
- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-#endif
if (io_remap_pfn_range(vma, vma->vm_start,
(map->offset + offset) >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
@@ -638,6 +646,7 @@ static int drm_mmap_locked(struct file *
page_to_pfn(virt_to_page(map->handle)),
vma->vm_end - vma->vm_start, vma->vm_page_prot))
return -EAGAIN;
+ vma->vm_page_prot = drm_dma_prot(map->type, vma);
/* fall through to _DRM_SHM */
case _DRM_SHM:
vma->vm_ops = &drm_vm_shm_ops;
@@ -650,6 +659,7 @@ static int drm_mmap_locked(struct file *
vma->vm_ops = &drm_vm_sg_ops;
vma->vm_private_data = (void *)map;
vma->vm_flags |= VM_RESERVED;
+ vma->vm_page_prot = drm_dma_prot(map->type, vma);
break;
default:
return -EINVAL; /* This should never happen. */


2008-03-02 11:05:28

by Gerhard Pircher

[permalink] [raw]
Subject: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

Hi,

-------- Original-Nachricht --------
> Datum: Mon, 26 Nov 2007 10:41:58 +1100
> Von: Benjamin Herrenschmidt <[email protected]>
> An: David Airlie <[email protected]>
> CC: [email protected], [email protected], [email protected]
> Betreff: [RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

> This patch fixes bits of the DRM so to make the radeon DRI work on
> non-cache coherent PCI DMA variants of the PowerPC processors.
>
> It moves the few places that needs change to wrappers to that
> other architectures with similar issues can easily add their
> own changes to those wrappers, at least until we have more useful
> generic kernel API.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>

Xorg (v7.1.1, Debian Etch) crashes with this patch (applied to 2.6.25-rc3)
on my AmigaOne with a Radeon 9200 (PCIGART mode enabled). See the attached
log file for the stack trace.

regards,

Gerhard
--
GMX startet ShortView.de. Hier findest Du Leute mit Deinen Interessen!
Jetzt dabei sein: http://www.shortview.de/?mc=sv_ext_mf@gmx


Attachments:
dri_crash.log (65.45 kB)

2008-03-02 20:49:11

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC


> Xorg (v7.1.1, Debian Etch) crashes with this patch (applied to 2.6.25-rc3)
> on my AmigaOne with a Radeon 9200 (PCIGART mode enabled). See the attached
> log file for the stack trace.

That doesn't look possible, which is weird... looks like we are passing
0 to clean_dcache_range().

Interestingly enough, I can -see- possible issues with the ppc32 DMA API
when trying to use HIGHMEM but that isn't the case here...

Can you add printk's to ati_pcigart.c to print the arguments passed to

+ dma_sync_single_for_device(&dev->pdev->dev,
+ bus_address,
+ max_pages * sizeof(u32),
+ PCI_DMA_TODEVICE);
+

And also print the result of bus_to_virt(bus_address) ?

Ben.

2008-03-02 22:30:32

by Gerhard Pircher

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

-------- Original-Nachricht --------
> Datum: Mon, 03 Mar 2008 07:47:09 +1100
> Von: Benjamin Herrenschmidt <[email protected]>
> An: Gerhard Pircher <[email protected]>
> CC: [email protected], [email protected], [email protected], [email protected]
> Betreff: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

> That doesn't look possible, which is weird... looks like we are passing
> 0 to clean_dcache_range().
>
> Interestingly enough, I can -see- possible issues with the ppc32 DMA API
> when trying to use HIGHMEM but that isn't the case here...
>
> Can you add printk's to ati_pcigart.c to print the arguments passed to
>
> + dma_sync_single_for_device(&dev->pdev->dev,
> + bus_address,
> + max_pages * sizeof(u32),
> + PCI_DMA_TODEVICE);
> +
>
> And also print the result of bus_to_virt(bus_address) ?
>
> Ben.

Okay, I changed the code to this:

>DRM_DEBUG("dev = 0x%x, bus_address = 0x%x, bus_to_virt = 0x%lx, max_pages = 0x%x\n",
> (unsigned int)&dev->pdev->dev, bus_address,
> (unsigned long)virt_to_bus(bus_address), max_pages);
>
>if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) {
> DRM_DEBUG("calling dma_sync_single_for_device()\n");
> dma_sync_single_for_device(&dev->pdev->dev,
> bus_address,
> max_pages * sizeof(u32),
> PCI_DMA_TODEVICE);
>}

It looks like dma_sync_single_for_device() is not called here (the debug
messages don't show up in kernel log). I also included the Xorg.0.log
file.

Gerhard
--
Psssst! Schon vom neuen GMX MultiMessenger geh?rt?
Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger


Attachments:
dri_crash2.log (87.47 kB)
Xorg.0.log (51.67 kB)
Download all attachments

2008-03-02 22:54:51

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC


> Okay, I changed the code to this:
>
> >DRM_DEBUG("dev = 0x%x, bus_address = 0x%x, bus_to_virt = 0x%lx, max_pages = 0x%x\n",
> > (unsigned int)&dev->pdev->dev, bus_address,
> > (unsigned long)virt_to_bus(bus_address), max_pages);
> >
> >if (gart_info->gart_table_location == DRM_ATI_GART_MAIN) {
> > DRM_DEBUG("calling dma_sync_single_for_device()\n");
> > dma_sync_single_for_device(&dev->pdev->dev,
> > bus_address,
> > max_pages * sizeof(u32),
> > PCI_DMA_TODEVICE);
> >}
>
> It looks like dma_sync_single_for_device() is not called here (the debug
> messages don't show up in kernel log). I also included the Xorg.0.log
> file.

Ok, try adding more debug then, around the calls to pci_map_single() in
that same function and dump the arguments. I'm especially interested
in the result of the various page_address().

Ben.

2008-03-02 22:56:49

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

Bah, I think I found the problem:

+static inline void *drm_vmalloc_dma(unsigned long size)
+{
+#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
+ return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM,
+ PAGE_KERNEL | _PAGE_NO_CACHE);
+#else
+ return vmalloc_32(size);
+#endif
+}
+

Remove the GFP_HIGHMEM from the above. It looks like our cache
flushing isn't going to work for highmem, it would need some
kmap's for that.

Ben.



2008-03-03 19:51:30

by Gerhard Pircher

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC


-------- Original-Nachricht --------
> Datum: Mon, 03 Mar 2008 09:56:09 +1100
> Von: Benjamin Herrenschmidt <[email protected]>
> An: Gerhard Pircher <[email protected]>
> CC: [email protected], [email protected], [email protected], [email protected]
> Betreff: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

> Bah, I think I found the problem:
>
> +static inline void *drm_vmalloc_dma(unsigned long size)
> +{
> +#if defined(__powerpc__) && defined(CONFIG_NOT_COHERENT_CACHE)
> + return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM,
> + PAGE_KERNEL | _PAGE_NO_CACHE);
> +#else
> + return vmalloc_32(size);
> +#endif
> +}
> +
>
> Remove the GFP_HIGHMEM from the above. It looks like our cache
> flushing isn't going to work for highmem, it would need some
> kmap's for that.
Yes, it looks like this was the problem. No kernel oops anymore.
The machine locks up anyway (which is a well known hardware problem).
It doesn't lock up with CPPIOMode=true, but probably only because the
initialization of DRI fails with "BAD cp_mode (f0000000)!".

Gerhard
--
Psssst! Schon vom neuen GMX MultiMessenger geh?rt?
Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

2008-03-03 20:45:02

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC


On Mon, 2008-03-03 at 20:51 +0100, Gerhard Pircher wrote:
> > Remove the GFP_HIGHMEM from the above. It looks like our cache
> > flushing isn't going to work for highmem, it would need some
> > kmap's for that.

> Yes, it looks like this was the problem. No kernel oops anymore.
> The machine locks up anyway (which is a well known hardware problem).
> It doesn't lock up with CPPIOMode=true, but probably only because the
> initialization of DRI fails with "BAD cp_mode (f0000000)!".

Damn, I wonder why you insist trying to make that machine work :-) The
hardware is just totally busted.

Ben.

2008-03-03 21:37:47

by Gerhard Pircher

[permalink] [raw]
Subject: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC


-------- Original-Nachricht --------
> Datum: Tue, 04 Mar 2008 07:44:11 +1100
> Von: Benjamin Herrenschmidt <[email protected]>
> An: Gerhard Pircher <[email protected]>
> CC: [email protected], [email protected], [email protected], [email protected]
> Betreff: Re: [BUG/RFC/PATCH] drm: Fix for non-coherent DMA PowerPC

> On Mon, 2008-03-03 at 20:51 +0100, Gerhard Pircher wrote:
> > > Remove the GFP_HIGHMEM from the above. It looks like our cache
> > > flushing isn't going to work for highmem, it would need some
> > > kmap's for that.
>
> > Yes, it looks like this was the problem. No kernel oops anymore.
> > The machine locks up anyway (which is a well known hardware problem).
> > It doesn't lock up with CPPIOMode=true, but probably only because the
> > initialization of DRI fails with "BAD cp_mode (f0000000)!".
>
> Damn, I wonder why you insist trying to make that machine work :-) The
> hardware is just totally busted.
Because it's a challenge! :) Or because the OS4 developers say that
PCIGART works.

Gerhard
--
Psst! Geheimtipp: Online Games kostenlos spielen bei den GMX Free Games!
http://games.entertainment.web.de/de/entertainment/games/free