Bruce, others. This patch series raises a number of questions.
1) It seems to do a lot more than just fixing a hang issue. See below. 2) There are code cleanups mixed with other stuff, which makes the patches difficult to read. Can you supply the code cleanups as separate patches? As for the additional functionality: a) You define AGP headers 3) and 4). I can't find a description of them in any documentation I have. Are they described somewhere in your open-source code? What do they do? b) You define a separate command submission mechanism for video. Why is that needed? c) You define a double-buffered AGP operation mode instead of the ring-buffer mode? Why is that needed? d) You define a via authmagic IOCTL that seems to do nothing? What is the purpose of this ioctl? e) Why is the INIT_JUDGE ioctl needed? The init ioctl should only be called by the DRM master and surely it must be able to keep track of that itself? f) From what I can see, the AGP buffers submitted though the video command submission mechanism are never checked by the command verifier? g) The GET_INFO ioctl should probably be replaced with a GETPARAM ioctl, (see how other drivers have done this). h) The suspend / resume functionality seems to duplicate a lot of stuff done in the X server driver. For example the 3D engine initialization is done there IIRC. More seriously, it seems like most of these additions are there to support a mode where an (unauthorized) drm client prepare double command buffers for submission directly in AGP memory, without any command checking and has access to the register map, and given the insecure nature of the unichrome, that should never be allowed unless perhaps in an "embedded" mode which should never be turned on by default. Currently, unichrome security is enforced in the following way: 1) No user-space processes except the DRM master may access the register maps. 2) All other user-space processes including 3d clients and video players *MUST* write to registers through DRM. 3) All AGP command buffers must be checked by the command verifier. If you want to add functionality, please use one patch per functionality item if possible. If there is a fix for a hang, plase supply that fix in a single patch. Thanks, Thomas ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july -- _______________________________________________ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
