On 13/10/2016 10:20, Chris Wilson wrote:
On Thu, Oct 13, 2016 at 10:03:58AM +0100, Tvrtko Ursulin wrote:From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> In order to reuse the same logic in several places in the driver, extract the logic which adds pages to the sg list and does the potential coalescing, into separate functions. Code wanting to build the sg table needs to do the following: 1. Call i915_sg_create to create the state object for a given maximum number of pages. 2. Iterate using i915_sg_for_each_page 3. Call i915_sg_add_page to add all the pages in order 4. On success call i915_sg_complete, or on failure i915_sg_abort In this patch only i915_gem_object_get_pages_gtt gets converted to use the new functions.Yup, much happier. Though can we just allocate the state on the stack, it is the same as what we are previously using anyway. 40 bytes.
Thought about this also and had a slight preference not to use stack, since we got to have kmalloc and error handling in i915_sg_create anyway.
I've also thought about doing the sg_trim, just never actually tried... Just not convinced that recreating the compact sg is better than copying the already compact sg (for dmabuf, partial).
Agreed already on dmabuf. Partials yes, I suppose that could be implemented more efficiently. You think it is worth special casing it rather than reuse the common API? It would have to handle a start in the middle of the sg entry so it wouldn't be a straightforward copy.
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
