On Tue, 2003-07-29 at 22:41, Ian Romanick wrote:
1. I don't like the hard-coding of 2*1024*1024 as the size of the indirect buffers. This was copied directly from the R200 driver, but I don't like it. We may want to change the size of this buffer at some point, and hard-coding the value into the client-side driver will make that difficult.
2. I don't like the hackish handing of the pre-1.3 DRM case. Are there other PCI IDs that need the 128MB offset? Do we even support the pre-1.3 DRM anymore? If we don't support the pre-1.3 DRM (and don't intend to fix the support), I'd like to chop all the pre-1.3 stuff out. That will make the Radeon driver "look" a lot more like the R200 driver. That's a good thing IMHO.
Why not always use
( ( INREG( RADEON_MC_AGP_LOCATION ) & 0xffff ) << 16 ) + dri_priv->agpTexOffset
as discussed on IRC? This should work with any chip, memory layout, ...
Here's my inner conflict about that. If there's a perfectly good way to get this value with a simple INREG, why is there an ioctl to get it as well?
In any case, here's an updated version of the patch that uses that method.
Index: lib/GL/mesa/src/drv/radeon/radeon_screen.c
===================================================================
RCS file: /cvsroot/dri/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_screen.c,v
retrieving revision 1.20
diff -u -d -r1.20 radeon_screen.c
--- lib/GL/mesa/src/drv/radeon/radeon_screen.c 21 May 2003 17:32:09 -0000 1.20
+++ lib/GL/mesa/src/drv/radeon/radeon_screen.c 4 Aug 2003 18:45:44 -0000
@@ -112,6 +112,7 @@
ret = drmCommandWriteRead( sPriv->fd, DRM_RADEON_GETPARAM,
&gp, sizeof(gp));
if (ret) {
+ FREE( screen );
fprintf(stderr, "drmRadeonGetParam (RADEON_PARAM_AGP_BUFFER_OFFSET): %d\n",
ret);
return NULL;
}
@@ -165,6 +166,8 @@
}
if ( !screen->IsPCI ) {
+ unsigned char *RADEONMMIO = screen->mmio.map;
+
screen->agpTextures.handle = dri_priv->agpTexHandle;
screen->agpTextures.size = dri_priv->agpTexMapSize;
if ( drmMap( sPriv->fd,
@@ -178,6 +181,9 @@
__driUtilMessage("%s: IsPCI failed\n", __FUNCTION__);
return NULL;
}
+
+ screen->agp_texture_offset = dri_priv->agpTexOffset
+ + ((INREG( RADEON_MC_AGP_LOCATION ) & 0x0ffffU) << 16);
}
screen->chipset = 0;
@@ -221,8 +227,7 @@
screen->logTexGranularity[RADEON_AGP_HEAP] = 0;
} else {
screen->numTexHeaps = RADEON_NR_TEX_HEAPS;
- screen->texOffset[RADEON_AGP_HEAP] =
- dri_priv->agpTexOffset + RADEON_AGP_TEX_OFFSET;
+ screen->texOffset[RADEON_AGP_HEAP] = screen->agp_texture_offset;
screen->texSize[RADEON_AGP_HEAP] = dri_priv->agpTexMapSize;
screen->logTexGranularity[RADEON_AGP_HEAP] =
dri_priv->log2AGPTexGran;
Index: lib/GL/mesa/src/drv/radeon/radeon_screen.h
===================================================================
RCS file: /cvsroot/dri/xc/xc/lib/GL/mesa/src/drv/radeon/radeon_screen.h,v
retrieving revision 1.14
diff -u -d -r1.14 radeon_screen.h
--- lib/GL/mesa/src/drv/radeon/radeon_screen.h 30 Apr 2003 01:50:52 -0000 1.14
+++ lib/GL/mesa/src/drv/radeon/radeon_screen.h 4 Aug 2003 18:45:44 -0000
@@ -92,6 +92,7 @@
__DRIscreenPrivate *driScreen;
unsigned int sarea_priv_offset;
unsigned int agp_buffer_offset; /* offset in card memory space */
+ unsigned int agp_texture_offset; /* offset in card memory space */
} radeonScreenRec, *radeonScreenPtr;
extern radeonScreenPtr radeonCreateScreen( __DRIscreenPrivate *sPriv );
Index: programs/Xserver/hw/xfree86/drivers/ati/radeon_reg.h
===================================================================
RCS file: /cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/drivers/ati/radeon_reg.h,v
retrieving revision 1.29
diff -u -d -r1.29 radeon_reg.h
--- programs/Xserver/hw/xfree86/drivers/ati/radeon_reg.h 10 Jun 2003 18:52:57
-0000 1.29
+++ programs/Xserver/hw/xfree86/drivers/ati/radeon_reg.h 4 Aug 2003 18:45:45
-0000
@@ -1882,7 +1882,15 @@
/* Constants */
+/* Do not use this value. For the R100, which can have upto 64MB of on-card
+ * memory, this should be 0x04000000. However, I believe that for the RV200
+ * (i.e., Radeon 7500), which can have upto 128MB of on-card memory, this
+ * should be 0x08000000. Use "(INREG( RADEON_MC_AGP_LOCATION ) & 0x0ffffU)
+ * << 16" instead.
+ */
+#if 0
#define RADEON_AGP_TEX_OFFSET 0x02000000
+#endif
#define RADEON_LAST_FRAME_REG RADEON_GUI_SCRATCH_REG0
#define RADEON_LAST_CLEAR_REG RADEON_GUI_SCRATCH_REG2
