On Wed, 19 Feb 2003, [iso-8859-15] José Fonseca wrote: > 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.
At some point, I'm planning on trying out oprofile with the new branch. Last time I looked, a lot of time seemed to be spent in freelist_get waiting for free buffers, but I think that was also before copy/verify. > > > > 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. I'll be interested to see what you come up with. How much of the setup in atidri.c were you thinking of moving to the kernel? We'd still need to allow for XF86Config options dealing with the AGP aperture and buffer region size(s), as well as passing a handles back to the Xserver for the AGP aperture (to setup the AGP registers) and the AGP texture map (although maybe the Mesa client could just get it with a get_param ioctl). If the Xserver does it, it needs to leave space for the private buffers in AGP when calculating the region sizes/offsets. > > 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? I think that would be fine, but I think that might mean that the extras package will be required too. BTW, what's the status of the trunk snaphots? Are we waiting for 4.3.0 to start those again? -- Leif Delgass http://www.retinalburn.net ------------------------------------------------------- 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
