On Wed, 2007-10-17 at 11:32 +0200, Thomas Hellström wrote: > Dave Airlie wrote: > >> DRM_BO_HINT_DONT_FENCE is implied, and use that instead of the set pin > >> interface. We can perhaps rename it to drmBOSetStatus or something more > >> suitable. > >> > >> This will get rid of the user-space unfenced list access (which I > >> believe was the main motivation behind the set pin interface?) while > >> keeping the currently heavily used (at least in Poulsbo) functionality > >> to move out NO_EVICT scanout buffers to local memory before unpinning > >> them, (to avoid VRAM and TT fragmentation, as DRI clients may still > >> reference those buffers, so they won't get destroyed before a new one is > >> allocated).
Yes, the unfenced list thing and the associated DRM_BO_HINTS required was part of the motivation for set_pin. But it also made sense from the X Server's perspective: we want to allocate buffers, and then there are times where we want them pinned because we're scanning out from them, but that pinning isn't associated with doing rendering to/from them that requires fencing as part of being involved in the command stream. And it avoided the issue with the previous validate-based interface where if you didn't sync the flags with what the X Server had updated, you might mistakenly pin/unpin a buffer. Can you clarify the operation being done where you move scanout buffers before unpinning them? That seems contradictory to me -- how are you scanning out while the object is being moved, and how are you considering it pinned during that time? Potentially related to your concerns, dave brought up an issue with the current set_pin implementation. Take the framebuffer resize sequence I'd proposed before: Turn off CRTCs Unpin old framebuffer Allocate new framebuffer Copy from old to new Free old Pin new Turn it back on. We'll have chosen an offset for new at "Copy old from new", and so during pin we'll just pin it right in place without moving it to avoid fragmentation. This sucks. It was a problem with the old interface as well, but it should be fixable by replacing the current poorly-suited validate call in the set_pin implementation with something that chooses the best available offset rather than just anything that matches memory types, and moves it into place before pinning. > >> It would also allow us to specify where we want to pin buffers. If we > >> remove the memory flag specification from drmBOCreate there's no other > >> way to do that, except running the buffer through a superioctl which > >> isn't very nice. Yeah, not making bo_set_pin include a pin location was an oversight that I didn't end up correcting before push. > >> Also it would make it much easier to unbreak i915 zone rendering and > >> derived work. > >> > >> If we can agree on this, I'll come up with a patch. > >> > > > > Have you had a chance to look at this I can probably spend some time on > > this to get the interface finalised.. > > > > Dave. > > > Hi, Dave, > > So, I did some quick work on this with the result in the > drm-ttm-finalize branch. > Basically what's done is to revert the setPin interface, and replace > drmBOValidate with drmBOSetStatus. > drmBOSetStatus is a single buffer interface with the same functionality > as previously drmBOValidate but with the exception that > it implies DRM_BO_HINT_DONT_FENCE, > NO_MOVE buffers have been blocked pending a proper implementation, and > optional tiling info has been added. > The OP linked interface is gone, but the arguments are kept in drm.h for > now, for driver specific IOCTLS. > > Looking at the buffer object create interface, I think it's a better > idea to remove the need for locking and specify the memory flags rather > than to assume a buffer in system memory before first validation. > Consider a driver that wants to put a texture buffer in VRAM. > It does createbuffer, copies the texture in, and then it gets validated > as part of the superioctl. If we don't specify the VRAM flag at buffer > creation, DRM will go ahead and create a ttm, allocate a lot of pages > and then at validate do a copy and free all pages again. If we specify > the VRAM flag, the buffer will on creation just reserve a vram area and > point any mapped pages to it. > Buffer creation and destruction in fixed memory areas was quite fast > previously, since no cache flushes were needed, > and I'd like to keep it that way. If we don't care or know how buffers > are created, we just specify DRM_BO_FLAG_MEM_LOCAL, which will give the > modified behavior currently present in master. But we need to remove > the need for locking in the buffer manager. I think we decided at XDS that, since you need to have the pages available during evict, allocating them up front at object create and failing then is a way better thing to be doing. But yes, if we can handle the locking issues, being able to write directly into VRAM (or even uncached TTM memory) on your first map of a buffer that you're going to just hand off for rendering is nice. However, I think putting this "Could you place this here so we can go faster?" information into create is the wrong place. My example for this would be a screen capture app that's scraping the screen, doing some accelerated operations on a sequence of buffers to encode it to video, then at some point in the pipeline doing software operations on some buffers because it can't be done in hardware any more. At that point of mapping those objects you want to migrate to cached local memory for your repeated sampling. If we only allow memory location hinting* for mapping at object create, we can't fix this case, while if we move location hinting to map, we can do both this and your VRAM case. I believe GL also has some hints that applications can pass at object map time that might be relevant to our buffer management, though I haven't reviewed that in some time. And mesa's VBO-based software tnl could use it for a nice speedup on i915. * I don't care whether it's strict or just a hint. > Also the unfenced list cleaning could probably be removed. It should be > considered a bug to leave something on the unfenced list outside of the > super-ioctl. Absolutely. -- Eric Anholt [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED]
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
-- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
