Re: [PATCH v2 1/2] drm/i915: refactor i915_gem_object_pin_map()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 20/04/16 14:30, Dave Gordon wrote:
The recently-added i915_gem_object_pin_map() can be further optimised
for "small" objects. To facilitate this, and simplify the error paths
before adding the new code, this patch pulls out the "mapping" part of
the operation (involving local allocations which must be undone before
return) into its own subfunction.

The next patch will then insert the new optimisation into the middle of
the now-separated subfunction.

This reorganisation will probably not affect the generated code, as the
compiler will most likely inline it anyway, but it makes the logical
structure a bit clearer and easier to modify.

v2:
     Restructure loop-over-pages & error check (Chris Wilson)

Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c | 58 ++++++++++++++++++++++++++---------------
  1 file changed, 37 insertions(+), 21 deletions(-)

[snip]

+	for_each_sg_page(sg, &sg_iter, n_pages, 0)
+		pages[i++] = sg_page_iter_page(&sg_iter);
+
+	/* Check that we have the expected number of pages */
+	if (!WARN_ON(i != n_pages))
+		addr = vmap(pages, n_pages, 0, PAGE_KERNEL);

Well actually the shorter loop and the subsequent check didn't turn out too ugly; the only thing I don't much like is if (!WARN_ON(...)), because I think in general that code should still function if WARN() and similar macros are #defined to empty loops. But WARN_ON() has to be defined to return (the truth-value of) its parameter even if it doesn't print anything, so it's not that bad.

OTOH, while looking at this loop, I worked out a better page iterator, so I'll post that in a little while :)

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux