On ke, 2016-04-20 at 14:30 +0100, 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(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6ce2c31..5344b6d 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2396,6 +2396,42 @@ static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
> return 0;
> }
>
> +/* The 'mapping' part of i915_gem_object_pin_map() below */
> +static void *i915_gem_object_map(const struct drm_i915_gem_object *obj)
> +{
> + unsigned long n_pages = obj->base.size >> PAGE_SHIFT;
> + struct scatterlist *sg = obj->pages->sgl;
> + struct sg_page_iter sg_iter;
> + struct page **pages;
> + unsigned long i = 0;
> + void *addr = NULL;
> +
> + /* A single page can always be kmapped */
> + if (n_pages == 1)
> + return kmap(sg_page(sg));
> +
> + pages = drm_malloc_gfp(n_pages, sizeof(*pages), GFP_TEMPORARY);
> + if (pages == NULL) {
> + DRM_DEBUG_DRIVER("Failed to get space for pages\n");
> + return NULL;
> + }
> +
> + 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);
> +
> + if (addr == NULL)
> + DRM_DEBUG_DRIVER("Failed to vmap pages\n");
> +
This kind of construct is used elsewhere, too.
if (WARN_ON(i != n_pages)) {
DRM_DEBUG_DRIVER("Failed to vmap pages\n");
goto out;
}
addr = vmap(pages, n_pages, 0, PAGE_KERNEL);
out:
> + drm_free_large(pages);
> +
> + return addr;
> +}
> +
> +/* get, pin, and map the pages of the object into kernel space */
> void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj)
> {
> int ret;
> @@ -2409,27 +2445,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj)
> i915_gem_object_pin_pages(obj);
>
> if (obj->mapping == NULL) {
> - struct page **pages;
> -
> - pages = NULL;
> - if (obj->base.size == PAGE_SIZE)
> - obj->mapping = kmap(sg_page(obj->pages->sgl));
> - else
> - pages = drm_malloc_gfp(obj->base.size >> PAGE_SHIFT,
> - sizeof(*pages),
> - GFP_TEMPORARY);
> - if (pages != NULL) {
> - struct sg_page_iter sg_iter;
> - int n;
> -
> - n = 0;
> - for_each_sg_page(obj->pages->sgl, &sg_iter,
> - obj->pages->nents, 0)
> - pages[n++] = sg_page_iter_page(&sg_iter);
> -
> - obj->mapping = vmap(pages, n, 0, PAGE_KERNEL);
> - drm_free_large(pages);
> - }
> + obj->mapping = i915_gem_object_map(obj);
> if (obj->mapping == NULL) {
> i915_gem_object_unpin_pages(obj);
> return ERR_PTR(-ENOMEM);
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
