On 19/02/16 11:07, Tvrtko Ursulin wrote:
On 18/02/16 18:31, yu.dai@xxxxxxxxx wrote:From: Alex Dai <yu.dai@xxxxxxxxx> There are several places inside driver where a GEM object is mapped to kernel virtual space. The mapping is either done for the whole object or certain page range of it. This patch introduces a function i915_gem_object_vmap to do such job. v2: Use obj->pages->nents for iteration within i915_gem_object_vmap; break when it finishes all desired pages. The caller need to pass in actual page number. (Tvrtko Ursulin)Look OK to me. Just one more thing, it would be good to add a WARN_ON and bail out if pages are not pinned. Just because the function is now public and that is a bit stronger than kerneldoc. With that added: Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Have to retract this due thing I noticed in Dave's repost of this patch, below:
Regards, TvrtkoSigned-off-by: Alex Dai <yu.dai@xxxxxxxxx> Cc: Dave Gordon <david.s.gordon@xxxxxxxxx> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx> --- drivers/gpu/drm/i915/i915_cmd_parser.c | 28 +------------------- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_gem.c | 47 +++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_dmabuf.c | 16 +++-------- drivers/gpu/drm/i915/intel_ringbuffer.c | 24 ++--------------- 5 files changed, 56 insertions(+), 62 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 814d894..915e8c1 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -863,37 +863,11 @@ find_reg(const struct drm_i915_reg_descriptor *table, static u32 *vmap_batch(struct drm_i915_gem_object *obj, unsigned start, unsigned len) { - int i; - void *addr = NULL; - struct sg_page_iter sg_iter; int first_page = start >> PAGE_SHIFT; int last_page = (len + start + 4095) >> PAGE_SHIFT; int npages = last_page - first_page; - struct page **pages; - - pages = drm_malloc_ab(npages, sizeof(*pages)); - if (pages == NULL) { - DRM_DEBUG_DRIVER("Failed to get space for pages\n"); - goto finish; - } - - i = 0; - for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, first_page) { - pages[i++] = sg_page_iter_page(&sg_iter); - if (i == npages) - break; - } - - addr = vmap(pages, i, 0, PAGE_KERNEL); - if (addr == NULL) { - DRM_DEBUG_DRIVER("Failed to vmap pages\n"); - goto finish; - } -finish: - if (pages) - drm_free_large(pages); - return (u32*)addr; + return (u32*)i915_gem_object_vmap(obj, first_page, npages); } /* Returns a vmap'd pointer to dest_obj, which the caller must unmap */ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6644c2e..5b00a6a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2899,6 +2899,9 @@ struct drm_i915_gem_object *i915_gem_object_create_from_data( struct drm_device *dev, const void *data, size_t size); void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); +void *i915_gem_object_vmap(struct drm_i915_gem_object *obj, + unsigned int first, + unsigned int npages); /* Flags used by pin/bind&friends. */ #define PIN_MAPPABLE (1<<0) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index f68f346..4bc0ce7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5356,3 +5356,50 @@ fail: drm_gem_object_unreference(&obj->base); return ERR_PTR(ret); } + +/** + * i915_gem_object_vmap - map a GEM obj into kernel virtual space + * @obj: the GEM obj to be mapped + * @first: index of the first page where mapping starts + * @npages: how many pages to be mapped, starting from first page + * + * Map a given page range of GEM obj into kernel virtual space. The caller must + * make sure the associated pages are gathered and pinned before calling this + * function. vunmap should be called after use. + * + * NULL will be returned if fails. + */ +void *i915_gem_object_vmap(struct drm_i915_gem_object *obj, + unsigned int first, + unsigned int npages) +{ + struct sg_page_iter sg_iter; + struct page **pages; + void *addr; + int i; + + if (first + npages > obj->pages->nents) { + DRM_DEBUG_DRIVER("Invalid page count\n");
nents can be equal or less than number of pages in an object so this can fail the attempt incorrectly. Should check against obj->base.size >> PAGE_SHIFT instead.
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
