From: Keith W. <ke...@tu...> - 2003-10-21 10:09:36
|
Michel Dänzer wrote: > On Wed, 2003-10-08 at 04:34, Vladimir Dergachev wrote: > >>On Wed, 8 Oct 2003, Michel [ISO-8859-1] Dnzer wrote: >> >> >>>>>Does the aperture ever (have to) move during the X server life though? >>>> >>>>I would not care. However, I know that at least Window 98 drivers have >>>>default position (0) unless capture is enabled. Also, I suspect that when >>>>one calls Video BIOS with framebuffer position anywhere other than 0 the >>>>BIOS then toggles the hard reset line. >>> >>>Why? (CC'ing Hui Yu, who should be able to comment on this) >> >>No idea why it does it. > > > Any news on this? I haven't taken this into account in > > http://penguinppc.org/~daenzer/DRI/radeon-memory-transition.diff In r200_screen.c, you check for drmMinor >= 10 before issuing the FB_LOCATION ioctl, but it's not clear what happens if drmMinor < 10 -- will the driver function correctly? If not, what should happen? Also, it seems this patch does two things, one is the fblocation stuff, but there's also some chipset_id changes relating to the R200_CHIPSET_RS300. I'd be happier to see these in separate patches. In radeon_state.c, the tests in radeon_emit_packets() are just ugly. The normal usage for this code on a properly installed system is that (filp_priv->fboffset == 0), right? So all those tests are going to result in a noop? Could the tests be pushed into their own functions and shortcircuited witha single test on (fboffset == 0) ? Additionally, in those tests, it looks like you are reading back and fixing up the data on the ring -- ie from uncached memory! Especially when (fboffset == 0) this is a very wasteful noop... Finally, and just being picky -- it'd be nice to keep the number of coding styles fairly minimal. It just looks a little odd to start seeing the spaces in code like 'tmp[ 0 ]', while the rest of the module is 'tmp[0]' Keith |