[Intel-gfx] [PATCH v2 1/2] drm/i915: Add i915_gem_object_vmap to map GEM object to virtual space
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Feb 29 12:03:35 UTC 2016
On 19/02/16 11:07, Tvrtko Ursulin wrote:
>
> On 18/02/16 18:31, yu.dai at intel.com wrote:
>> From: Alex Dai <yu.dai at intel.com>
>>
>> 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 at intel.com>
Have to retract this due thing I noticed in Dave's repost of this patch,
below:
> Regards,
>
> Tvrtko
>
>
>> Signed-off-by: Alex Dai <yu.dai at intel.com>
>> Cc: Dave Gordon <david.s.gordon at intel.com>
>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Alex Dai <yu.dai at intel.com>
>> ---
>> 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
More information about the Intel-gfx
mailing list