Radeon DRM GART mapping bogosity

[Benjamin Herrenschmidt]

On Tue, 2005-05-03 at 15:24 +1000, Benjamin Herrenschmidt wrote: 
> Hi !
> 
> The radeon DRM has some interesting bug that paul and I discovered to
> cause all sort of problems like crashing the machine on suspend/resume
> (go figure ...) etc...
> 
> 	dev_priv->gart_vm_start = dev_priv->fb_location
> 				+ RADEON_READ( RADEON_CONFIG_APER_SIZE );
> 
> The "aim" of this code is to setup the card memory map so that the GART
> sits just after the framebuffer. However, CONFIG_APER_SIZE is _not_ a
> good indication of the framebuffer size.
>
> CONFIG_APER_SIZE is only the size of the visible aperture on the PCI
> bus. Some setups (like some Macs for example) can use the dual split
> aperture mecanism, in which case CONFIG_APER_SIZE is only half of the
> VRAM. I can imagine cards overloaded with memory to have more vram that
> is directly accessible from PCI in other circumstances too (though the
> split aperture case is a real world scenario we encountered on paul's
> laptop at least).
> 
> The result is we end up putting the GART right in the middle of VRAM in
> card's space. The card's memory controller at best does nothing of it,
> at worst blows up in funny way when the engine is reset, or in some
> case, when re-initializing from suspend/resume cycle.

   .../...

Ok, I'm cross posting here because X.org is doing it wrong too. On R300,
for some reason I don't fully understand, it just goes back to the "old"
way of putting the FB at 0 (though it does properly use CONFIG_MEMSIZE
to set the size part of MC_FB_LOCATION), but for non R300, it does try
to put the framebuffer at the same address as the BAR ... and then tries
to use CONFIG_APER_SIZE for the size part of MC_FB_LOCATION, which is
incorrect.

The result is that on !r300, it won't crash since X.Org and DRI won't
create an overlapping mapping, but they won't be able to use all of VRAM
of some cards where CONFIG_APER_SIZE < CONFIG_MEM_SIZE, and on r300, it
will possibly create overlapping mappings and will cause all sorts of
troubles if you get the above case.

Another problem I haven't checked is that we should make sure, before
changing MC_FB_LOCATION, to actually disable scanning of memory by both
CRTCs (and possibly disable tiling, I had some problems in radeonfb due
to mac firmware enabling tiling, that would explode when playing with
MC_FB_LOCATION).

In the meantime, here's a patch against current Linus "git" that I'm
tempted to push asap so that at least 2.6.12 avoids the problem of
overlapping which causes random stuffs to happen with lockups. The
"issue" here is even if you don't have an r300-friendly DRM, it will
still try to initialize those things, even if it ultimately fails,
provided you have a new enough X.org, and thus will screw up the
mapping.

Index: linux-work/drivers/char/drm/radeon_drv.h
===================================================================
--- linux-work.orig/drivers/char/drm/radeon_drv.h	2005-05-02 10:48:09.000000000 +1000
+++ linux-work/drivers/char/drm/radeon_drv.h	2005-05-03 17:51:55.000000000 +1000
@@ -346,6 +346,7 @@
 #define RADEON_CLOCK_CNTL_DATA		0x000c
 #	define RADEON_PLL_WR_EN			(1 << 7)
 #define RADEON_CLOCK_CNTL_INDEX		0x0008
+#define RADEON_CONFIG_MEMSIZE		0x00f8
 #define RADEON_CONFIG_APER_SIZE		0x0108
 #define RADEON_CRTC_OFFSET		0x0224
 #define RADEON_CRTC_OFFSET_CNTL		0x0228
Index: linux-work/drivers/char/drm/radeon_cp.c
===================================================================
--- linux-work.orig/drivers/char/drm/radeon_cp.c	2005-05-02 10:48:09.000000000 +1000
+++ linux-work/drivers/char/drm/radeon_cp.c	2005-05-03 17:49:25.000000000 +1000
@@ -1269,6 +1269,7 @@
 {
 	drm_radeon_private_t *dev_priv = dev->dev_private;;
 	DRM_DEBUG( "\n" );
+	u32 gart_loc;
 
 	dev_priv->is_pci = init->is_pci;
 
@@ -1476,8 +1477,12 @@
 
 
 	dev_priv->gart_size = init->gart_size;
-	dev_priv->gart_vm_start = dev_priv->fb_location
-				+ RADEON_READ( RADEON_CONFIG_APER_SIZE );
+	gart_loc = dev_priv->fb_location + RADEON_READ( RADEON_CONFIG_MEMSIZE );
+	/* overflow ? */
+	if ((gart_loc + dev_priv->gart_size) < dev_priv->fb_location)
+		gart_loc = dev_priv->fb_location - dev_priv->gart_size;
+	
+	dev_priv->gart_vm_start = gart_loc;
 
 #if __OS_HAS_AGP
 	if ( !dev_priv->is_pci )