On Tue, Feb 18, 2003 at 06:30:40PM -0500, Leif Delgass wrote: > I also made some other changes to the copy/verify: > > I added a check to the ioctl handler (mach64_dma_vertex) to check that the > vertex buffer has an integral number of dwords (in addition to the check > against MACH64_BUFFER_SIZE). I also changed copy_and_verify_from_user to > return an error code instead of the number of bytes copied. I don't see > any reason to dispatch a buffer unless all checks succeed (in fact it can > potentially cause lockups if we submit a partial buffer), so it's either > all or nothing. If it succeeds then we just copy buf->used to the private > buffer struct. This also allows us to return a more meaningful error > code, e.g. EFAULT for failed verify_area/copy_from_user, EINVAL for bad > command counts or buffer size, and EACCES for disallowed register commands > in the buffer. It also lets us get rid of the 'copied' local var and a > couple of adds inside the loop.
At that time I coded that way as an aid to debug that code. Now I don't see any problem in letting that go. > A couple of other minor tweaks I made > were copying the byte length parameter to a local var and declaring the > function as inline. It also now will generate an error if there is a > register command without at least one data dword following it (in addition > to checking for a command count that overflows buf->used). That check is > now the new loop condition (n > 1 instead of n (!=0) ), so it doesn't add > anything inside the loop. There is one extra conditional in the loop now > to check the return of copy_from_user. All of this may or may not have > much of a performance effect, but it is a pretty critical code path, I > think. My impression is that the effect of the copy/verify is noticeable in slower machines but neglectable in faster. But I guess that the hit is probably more related to the processor cache than its actual speed. > > > 3. We still need to work out the wrapper/alternative to > > > pci_alloc_consistent() and friends. > > > > I'm still reading Ian texmem-2 proposal and looking to the source code > > to get a hold of this. > It's now official: I'm coding on this. I'm adding the _ioctl suffix to most IOCTLs (e.g., agp_alloc -> agp_alloc_ioctl) to leave to the agp_alloc for internal DRM usage, and writing a set of pci_* to wrap the pci_*_consistent and pci_pool_* API in the Linux. Nothing of this will break binary compatability and will allow one to [optionally] setup all main DMA buffers from within DRM_IOCTL_DMA IOCTL instead of the X server. Of course that I would like to pursue this idea in Mach64 driver. > Something I have in my old tree that I haven't merged to the new branch > yet is setting up the ring in AGP when available. That will get rid of > the use of pci_alloc_consistent for AGP cards. However we still can't do > private buffers in AGP without multiple buffer lists. However, it might > be enough to allow porting/testing on *BSD to continue with the PCI mode > setup in mach64_dma.c disabled (with the current temporary code that > uses client buffers instead of secure buffers). BTW, is it OK to enable snapshots from the 0-0-6 branch instead? José Fonseca __________________________________________________ Do You Yahoo!? Everything you'll ever need on one web page from News and Sport to Email and Music Charts http://uk.my.yahoo.com ------------------------------------------------------- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge _______________________________________________ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
