diff mbox

[v2,1/2] drm/i915: Add i915_gem_object_vmap to map GEM object to virtual space

Message ID 1455820298-5463-2-git-send-email-yu.dai@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

yu.dai@intel.com Feb. 18, 2016, 6:31 p.m. UTC
From: Alex Dai <yu.dai@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)

Signed-off-by: Alex Dai <yu.dai@intel.com>
Cc: Dave Gordon <david.s.gordon@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Alex Dai <yu.dai@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(-)

Comments

Chris Wilson Feb. 18, 2016, 9:05 p.m. UTC | #1
On Thu, Feb 18, 2016 at 10:31:37AM -0800, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@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)

Who owns the pages? vmap doesn't increase the page refcount nor
mapcount, so it is the callers responsibility to keep the pages alive
for the duration of the vmapping.

I suggested i915_gem_object_pin_vmap/unpin_vmap for that reason and that
also provides the foundation for undoing one of the more substantial
performance regressions from vmap_batch().
-Chris
yu.dai@intel.com Feb. 18, 2016, 9:30 p.m. UTC | #2
On 02/18/2016 01:05 PM, Chris Wilson wrote:
> On Thu, Feb 18, 2016 at 10:31:37AM -0800, yu.dai@intel.com wrote:
> > From: Alex Dai <yu.dai@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)
>
> Who owns the pages? vmap doesn't increase the page refcount nor
> mapcount, so it is the callers responsibility to keep the pages alive
> for the duration of the vmapping.
>
> I suggested i915_gem_object_pin_vmap/unpin_vmap for that reason and that
> also provides the foundation for undoing one of the more substantial
> performance regressions from vmap_batch().
>
>

OK, found it at 050/190 of your patch series. That is a huge list of 
patches. :-) The code I put here does not change (at least tries to 
keep) the current code logic or driver behavior. I am not opposed to 
using i915_gem_object_pin_vmap/unpin_vmap at all. I will now just keep 
eyes on that patch.

Alex
Tvrtko Ursulin Feb. 19, 2016, 11:07 a.m. UTC | #3
On 18/02/16 18:31, yu.dai@intel.com wrote:
> From: Alex Dai <yu.dai@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@intel.com>

Regards,

Tvrtko


> Signed-off-by: Alex Dai <yu.dai@intel.com>
> Cc: Dave Gordon <david.s.gordon@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Alex Dai <yu.dai@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");
> +		return NULL;
> +	}
> +
> +	pages = drm_malloc_ab(npages, sizeof(*pages));
> +	if (pages == NULL) {
> +		DRM_DEBUG_DRIVER("Failed to get space for pages\n");
> +		return NULL;
> +	}
> +
> +	i = 0;
> +	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, first) {
> +		pages[i++] = sg_page_iter_page(&sg_iter);
> +		if (i == npages)
> +			break;
> +	}
> +
> +	addr = vmap(pages, npages, 0, PAGE_KERNEL);
> +	if (addr == NULL)
> +		DRM_DEBUG_DRIVER("Failed to vmap pages\n");
> +	drm_free_large(pages);
> +
> +	return addr;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index 1f3eef6..6133036 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -110,9 +110,7 @@ static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
>   {
>   	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>   	struct drm_device *dev = obj->base.dev;
> -	struct sg_page_iter sg_iter;
> -	struct page **pages;
> -	int ret, i;
> +	int ret;
>
>   	ret = i915_mutex_lock_interruptible(dev);
>   	if (ret)
> @@ -131,16 +129,8 @@ static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
>
>   	ret = -ENOMEM;
>
> -	pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
> -	if (pages == NULL)
> -		goto err_unpin;
> -
> -	i = 0;
> -	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
> -		pages[i++] = sg_page_iter_page(&sg_iter);
> -
> -	obj->dma_buf_vmapping = vmap(pages, i, 0, PAGE_KERNEL);
> -	drm_free_large(pages);
> +	obj->dma_buf_vmapping = i915_gem_object_vmap(obj, 0,
> +			dma_buf->size >> PAGE_SHIFT);
>
>   	if (!obj->dma_buf_vmapping)
>   		goto err_unpin;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 45ce45a..93666e9 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2064,27 +2064,6 @@ void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
>   	i915_gem_object_ggtt_unpin(ringbuf->obj);
>   }
>
> -static u32 *vmap_obj(struct drm_i915_gem_object *obj)
> -{
> -	struct sg_page_iter sg_iter;
> -	struct page **pages;
> -	void *addr;
> -	int i;
> -
> -	pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
> -	if (pages == NULL)
> -		return NULL;
> -
> -	i = 0;
> -	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
> -		pages[i++] = sg_page_iter_page(&sg_iter);
> -
> -	addr = vmap(pages, i, 0, PAGE_KERNEL);
> -	drm_free_large(pages);
> -
> -	return addr;
> -}
> -
>   int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
>   				     struct intel_ringbuffer *ringbuf)
>   {
> @@ -2103,7 +2082,8 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
>   			return ret;
>   		}
>
> -		ringbuf->virtual_start = vmap_obj(obj);
> +		ringbuf->virtual_start = i915_gem_object_vmap(obj, 0,
> +				ringbuf->size >> PAGE_SHIFT);
>   		if (ringbuf->virtual_start == NULL) {
>   			i915_gem_object_ggtt_unpin(obj);
>   			return -ENOMEM;
>
Tvrtko Ursulin Feb. 29, 2016, 12:03 p.m. UTC | #4
On 19/02/16 11:07, Tvrtko Ursulin wrote:
>
> On 18/02/16 18:31, yu.dai@intel.com wrote:
>> From: Alex Dai <yu.dai@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@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@intel.com>
>> Cc: Dave Gordon <david.s.gordon@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Alex Dai <yu.dai@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
diff mbox

Patch

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");
+		return NULL;
+	}
+
+	pages = drm_malloc_ab(npages, sizeof(*pages));
+	if (pages == NULL) {
+		DRM_DEBUG_DRIVER("Failed to get space for pages\n");
+		return NULL;
+	}
+
+	i = 0;
+	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, first) {
+		pages[i++] = sg_page_iter_page(&sg_iter);
+		if (i == npages)
+			break;
+	}
+
+	addr = vmap(pages, npages, 0, PAGE_KERNEL);
+	if (addr == NULL)
+		DRM_DEBUG_DRIVER("Failed to vmap pages\n");
+	drm_free_large(pages);
+
+	return addr;
+}
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 1f3eef6..6133036 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -110,9 +110,7 @@  static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
 {
 	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
 	struct drm_device *dev = obj->base.dev;
-	struct sg_page_iter sg_iter;
-	struct page **pages;
-	int ret, i;
+	int ret;
 
 	ret = i915_mutex_lock_interruptible(dev);
 	if (ret)
@@ -131,16 +129,8 @@  static void *i915_gem_dmabuf_vmap(struct dma_buf *dma_buf)
 
 	ret = -ENOMEM;
 
-	pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
-	if (pages == NULL)
-		goto err_unpin;
-
-	i = 0;
-	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
-		pages[i++] = sg_page_iter_page(&sg_iter);
-
-	obj->dma_buf_vmapping = vmap(pages, i, 0, PAGE_KERNEL);
-	drm_free_large(pages);
+	obj->dma_buf_vmapping = i915_gem_object_vmap(obj, 0,
+			dma_buf->size >> PAGE_SHIFT);
 
 	if (!obj->dma_buf_vmapping)
 		goto err_unpin;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 45ce45a..93666e9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2064,27 +2064,6 @@  void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 	i915_gem_object_ggtt_unpin(ringbuf->obj);
 }
 
-static u32 *vmap_obj(struct drm_i915_gem_object *obj)
-{
-	struct sg_page_iter sg_iter;
-	struct page **pages;
-	void *addr;
-	int i;
-
-	pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages));
-	if (pages == NULL)
-		return NULL;
-
-	i = 0;
-	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
-		pages[i++] = sg_page_iter_page(&sg_iter);
-
-	addr = vmap(pages, i, 0, PAGE_KERNEL);
-	drm_free_large(pages);
-
-	return addr;
-}
-
 int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 				     struct intel_ringbuffer *ringbuf)
 {
@@ -2103,7 +2082,8 @@  int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 			return ret;
 		}
 
-		ringbuf->virtual_start = vmap_obj(obj);
+		ringbuf->virtual_start = i915_gem_object_vmap(obj, 0,
+				ringbuf->size >> PAGE_SHIFT);
 		if (ringbuf->virtual_start == NULL) {
 			i915_gem_object_ggtt_unpin(obj);
 			return -ENOMEM;