From: Michel <mi...@da...> - 2003-08-04 21:01:28
|
On Mon, 2003-08-04 at 20:48, Ian Romanick wrote: > Michel Dänzer wrote: > > > 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? Because nobody thought of the simple solution? :) Or maybe we wanted to get rid of the MMIO mapping? I don't know. If in doubt, I'd consider the chip register authoritative though. :) > 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 Can't we just get rid of this? The value it was supposed to represent was never a constant in the first place. -- Earthling Michel Dänzer \ Debian (powerpc), XFree86 and DRI developer Software libre enthusiast \ http://svcs.affero.net/rm.php?r=daenzer |