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

[Dave Gordon <david.s.gordon@intel.com>]

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@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx