Hi Alex, I'm addressing this to you (Alex), as you seem to be The Man to talk to about r6xx dri support at the moment. CC'ing both the radeonhd and the dri-devel mailing lists.
Good. I tried running the r6xx-r7xx-support branch from drm/xf86-video-radeonhd with my RV670 card (HD 3870) on an amd64 platform, but it failed, giving me an irrecoverable black screen, and an OOPS because of a null pointer dereference. It became apparent the cause was an ioremap in r600_cp.c failing, and the result wasn't checked, so it didn't quite fail gracefully. It seems to me (I must admit I don't really know what I'm talking about though, I haven't looked into the way memory is handled in x86 cpus yet) that the correct fix would be to use ioremap_wc (which maps to ioremap_nocache on non-PAT-enabled kernels) instead. It works for me: with EXA enabled, I get a working X instance, with lots of slowness and glitches but I guess that's expected behavior at the moment. Below is a patch, which does four things: - Change drm_core_ioremap to drm_core_ioremap_wc (note that this is already the function that's called in the r3xx code in the exact same place; I don't know what went wrong here, but someone missed something) - Add a check for this specific drm_core_ioremap(_wc) invocation failing; any failure should return -EINVAL. Also, when dev_priv->pcigart_offset_set isn't set, it should return -EINVAL, and not just continue as if nothing's amiss - Add many debugging messages. Your opinion on this may be different from mine, so apply whatever you deem useful. Note particularly that I added messages for succeeding or failing ioremaps and ioremapfrees - drm_core_ioremap_wc was enabled only on 2.6.26+ kernels, which seems odd. I checked, and ioremap_wc has been in the kernel a *long* time (on x86); there was a #define to check whether it exists on the current arch, so I changed drm_core_ioremap_wc to be always present (it's used unconditionally in radeon_cp.c anyway, so it seems to be a sane idea to define drm_core_ioremap_wc to do the right thing whether ioremap_wc is present or not). I hope this patch helps. Signed-off-by: Sytse Wielinga <[email protected]> diff --git a/linux-core/drm_memory.c b/linux-core/drm_memory.c index d1a88c8..f3dd693 100644 --- a/linux-core/drm_memory.c +++ b/linux-core/drm_memory.c @@ -343,24 +343,48 @@ static void *agp_remap(unsigned long offset, unsigned long size, void drm_core_ioremap(struct drm_map *map, struct drm_device *dev) { if (drm_core_has_AGP(dev) && - dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP) + dev->agp && dev->agp->cant_use_aperture && map->type == _DRM_AGP) { map->handle = agp_remap(map->offset, map->size, dev); - else + if (!map->handle) + DRM_ERROR("agp_remap at 0x%08lx size 0x%04lx failed\n", + map->offset, map->size); + else + DRM_DEBUG("0x%p: agp_remap at 0x%08lx size 0x%04lx\n", + map->handle, map->offset, map->size); + } else { map->handle = ioremap(map->offset, map->size); + if (!map->handle) + DRM_ERROR("ioremap at 0x%08lx size 0x%04lx failed\n", + map->offset, map->size); + else + DRM_DEBUG("0x%p: ioremap at 0x%08lx size 0x%04lx\n", + map->handle, map->offset, map->size); + } } EXPORT_SYMBOL_GPL(drm_core_ioremap); -#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,26) void drm_core_ioremap_wc(struct drm_map *map, struct drm_device *dev) { +#ifdef ARCH_HAS_IOREMAP_WC map->handle = ioremap_wc(map->offset, map->size); +#else + map->handle = ioremap_nocache(map->offset, map->size); +#endif + if (!map->handle) + DRM_ERROR("ioremap_wc at 0x%08lx size 0x%04lx failed\n", + map->offset, map->size); + else + DRM_DEBUG("0x%p: ioremap_wc at 0x%08lx size 0x%04lx\n", + map->handle, map->offset, map->size); } EXPORT_SYMBOL_GPL(drm_core_ioremap_wc); -#endif void drm_core_ioremapfree(struct drm_map *map, struct drm_device *dev) { + DRM_DEBUG("ioremapfree at 0x%p size 0x%04lx\n", + map->handle, map->size); + if (!map->handle || !map->size) return; diff --git a/shared-core/r600_cp.c b/shared-core/r600_cp.c index 6436ffc..03a64a7 100644 --- a/shared-core/r600_cp.c +++ b/shared-core/r600_cp.c @@ -2297,11 +2297,14 @@ int r600_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init) - (unsigned long)dev->sg->virtual + dev_priv->gart_vm_start); - DRM_DEBUG("fb 0x%08x size %d\n", dev_priv->fb_location, dev_priv->fb_size); + DRM_DEBUG("fb 0x%08x size %d\n", + (unsigned int) dev_priv->fb_location, + (unsigned int) dev_priv->fb_size); DRM_DEBUG("dev_priv->gart_size %d\n", dev_priv->gart_size); - DRM_DEBUG("dev_priv->gart_vm_start 0x%x\n", dev_priv->gart_vm_start); - DRM_DEBUG("dev_priv->gart_buffers_offset 0x%lx\n", - dev_priv->gart_buffers_offset); + DRM_DEBUG("dev_priv->gart_vm_start 0x%08x\n", + (unsigned int) dev_priv->gart_vm_start); + DRM_DEBUG("dev_priv->gart_buffers_offset 0x%08lx\n", + dev_priv->gart_buffers_offset); dev_priv->ring.start = (u32 *) dev_priv->cp_ring->handle; dev_priv->ring.end = ((u32 *) dev_priv->cp_ring->handle @@ -2321,8 +2324,13 @@ int r600_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init) dev_priv->gart_info.table_mask = DMA_BIT_MASK(32); /* if we have an offset set from userspace */ - if (!dev_priv->pcigart_offset_set) + if (!dev_priv->pcigart_offset_set) { DRM_ERROR("Need gart offset from userspace\n"); + r600_do_cleanup_cp(dev); + return -EINVAL; + } + + DRM_DEBUG("Using gart offset 0x%08lx\n", dev_priv->pcigart_offset); dev_priv->gart_info.bus_addr = dev_priv->pcigart_offset + dev_priv->fb_location; @@ -2330,15 +2338,20 @@ int r600_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init) dev_priv->pcigart_offset + dev_priv->fb_aper_offset; dev_priv->gart_info.mapping.size = dev_priv->gart_info.table_size; - - drm_core_ioremap(&dev_priv->gart_info.mapping, dev); + + drm_core_ioremap_wc(&dev_priv->gart_info.mapping, dev); + if (!dev_priv->gart_info.mapping.handle) { + DRM_ERROR("ioremap failed.\n"); + r600_do_cleanup_cp(dev); + return -EINVAL; + } dev_priv->gart_info.addr = dev_priv->gart_info.mapping.handle; DRM_DEBUG("Setting phys_pci_gart to %p %08lX\n", - dev_priv->gart_info.addr, - dev_priv->pcigart_offset); + dev_priv->gart_info.addr, + dev_priv->pcigart_offset); r600_page_table_init(dev); if (((dev_priv->flags & RADEON_FAMILY_MASK) >= CHIP_RV770)) { diff --git a/shared-core/radeon_cp.c b/shared-core/radeon_cp.c index 19ff1b6..33dcf81 100644 --- a/shared-core/radeon_cp.c +++ b/shared-core/radeon_cp.c @@ -1165,9 +1165,10 @@ static int radeon_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init) + dev_priv->gart_vm_start); DRM_DEBUG("dev_priv->gart_size %d\n", dev_priv->gart_size); - DRM_DEBUG("dev_priv->gart_vm_start 0x%x\n", dev_priv->gart_vm_start); + DRM_DEBUG("dev_priv->gart_vm_start 0x%x\n", + (unsigned int) dev_priv->gart_vm_start); DRM_DEBUG("dev_priv->gart_buffers_offset 0x%lx\n", - dev_priv->gart_buffers_offset); + dev_priv->gart_buffers_offset); dev_priv->ring.start = (u32 *) dev_priv->cp_ring->handle; dev_priv->ring.end = ((u32 *) dev_priv->cp_ring->handle @@ -1203,6 +1204,12 @@ static int radeon_do_init_cp(struct drm_device * dev, drm_radeon_init_t * init) dev_priv->gart_info.table_size; drm_core_ioremap_wc(&dev_priv->gart_info.mapping, dev); + if (!dev_priv->gart_info.mapping.handle) { + DRM_ERROR("ioremap failed.\n"); + radeon_do_cleanup_cp(dev); + return -EINVAL; + } + dev_priv->gart_info.addr = dev_priv->gart_info.mapping.handle; ------------------------------------------------------------------------------ This SF.net email is sponsored by: SourcForge Community SourceForge wants to tell your story. http://p.sf.net/sfu/sf-spreadtheword -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
