[Intel-gfx] [PATCH v6 7/7] drm/i915: refactor duplicate object vmap functions (reworked again)
Dave Gordon
david.s.gordon at intel.com
Mon Feb 29 21:16:12 UTC 2016
This is essentially Chris Wilson's patch of a similar name, reworked on
top of Alex Dai's recent patch:
| drm/i915: Add i915_gem_object_vmap to map GEM object to virtual space
Chris' original commentary said:
| We now have two implementations for vmapping a whole object, one for
| dma-buf and one for the ringbuffer. If we couple the vmapping into
| the obj->pages lifetime, then we can reuse an obj->vmapping for both
| and at the same time couple it into the shrinker.
|
| v2: Mark the failable kmalloc() as __GFP_NOWARN (vsyrjala)
| v3: Call unpin_vmap from the right dmabuf unmapper
v4: reimplements the same functionality, but now as wrappers round the
recently-introduced i915_gem_object_vmap_range() from Alex's patch
mentioned above.
v5: separated from two minor but unrelated changes [Tvrtko Ursulin];
this is the third and most substantial portion.
Decided not to hold onto vmappings after the pin count goes to zero.
This may reduce the benefit of Chris' scheme a bit, but does avoid
any increased risk of exhausting kernel vmap space on 32-bit kernels
[Tvrtko Ursulin]. Potentially, the vunmap() could be deferred until
the put_pages() stage if a suitable notifier were written, but we're
not doing that here. Nonetheless, the simplification of both dmabuf
and ringbuffer code makes it worthwhile in its own right.
v6: change BUG_ON() to WARN_ON(). [Tvrtko Ursulin]
Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Alex Dai <yu.dai at intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 22 +++++++++++++-----
drivers/gpu/drm/i915/i915_gem.c | 40 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 37 ++++--------------------------
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++++-----
4 files changed, 66 insertions(+), 43 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 12b0717..754441a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2172,10 +2172,7 @@ struct drm_i915_gem_object {
struct scatterlist *sg;
int last;
} get_page;
-
- /* prime dma-buf support */
- void *dma_buf_vmapping;
- int vmapping_count;
+ void *vmapping;
/** Breadcrumb of last rendering to the buffer.
* There can only be one writer, but we allow for multiple readers.
@@ -2980,7 +2977,22 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
{
BUG_ON(obj->pages_pin_count == 0);
- obj->pages_pin_count--;
+ if (--obj->pages_pin_count == 0 && obj->vmapping) {
+ /*
+ * Releasing the vmapping here may yield less benefit than
+ * if we kept it until put_pages(), but on the other hand
+ * avoids issues of exhausting kernel vmappable address
+ * space on 32-bit kernels.
+ */
+ vunmap(obj->vmapping);
+ obj->vmapping = NULL;
+ }
+}
+
+void *__must_check i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj);
+static inline void i915_gem_object_unpin_vmap(struct drm_i915_gem_object *obj)
+{
+ i915_gem_object_unpin_pages(obj);
}
void *__must_check i915_gem_object_vmap_range(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9410c72..b17423d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2235,6 +2235,12 @@ static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
ops->put_pages(obj);
obj->pages = NULL;
+ /* vmapping should have been dropped when pages_pin_count went to 0 */
+ if (WARN_ON(obj->vmapping)) {
+ vunmap(obj->vmapping);
+ obj->vmapping = NULL;
+ }
+
i915_gem_object_invalidate(obj);
return 0;
@@ -2463,6 +2469,40 @@ void *i915_gem_object_vmap_range(struct drm_i915_gem_object *obj,
return addr;
}
+/**
+ * i915_gem_object_pin_vmap - pin a GEM object and map it into kernel space
+ * @obj: the GEM object to be mapped
+ *
+ * Combines the functions of get_pages(), pin_pages() and vmap_range() on
+ * the whole object. The caller should release the mapping by calling
+ * i915_gem_object_unpin_vmap() when it is no longer required.
+ *
+ * Returns the address at which the object has been mapped, or an ERR_PTR
+ * on failure.
+ */
+void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj)
+{
+ int ret;
+
+ ret = i915_gem_object_get_pages(obj);
+ if (ret)
+ return ERR_PTR(ret);
+
+ i915_gem_object_pin_pages(obj);
+
+ if (obj->vmapping == NULL) {
+ obj->vmapping = i915_gem_object_vmap_range(obj, 0,
+ obj->base.size);
+
+ if (obj->vmapping == NULL) {
+ i915_gem_object_unpin_pages(obj);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+
+ return obj->vmapping;
+}
+
void i915_vma_move_to_active(struct i915_vma *vma,
struct drm_i915_gem_request *req)
{
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 68e21d1..adc7b5e 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -108,41 +108,17 @@ 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;
+ void *addr;
int ret;
ret = i915_mutex_lock_interruptible(dev);
if (ret)
return ERR_PTR(ret);
- if (obj->dma_buf_vmapping) {
- obj->vmapping_count++;
- goto out_unlock;
- }
-
- ret = i915_gem_object_get_pages(obj);
- if (ret)
- goto err;
-
- i915_gem_object_pin_pages(obj);
-
- ret = -ENOMEM;
-
- obj->dma_buf_vmapping = i915_gem_object_vmap_range(obj, 0,
- dma_buf->size >> PAGE_SHIFT);
-
- if (!obj->dma_buf_vmapping)
- goto err_unpin;
-
- obj->vmapping_count = 1;
-out_unlock:
+ addr = i915_gem_object_pin_vmap(obj);
mutex_unlock(&dev->struct_mutex);
- return obj->dma_buf_vmapping;
-err_unpin:
- i915_gem_object_unpin_pages(obj);
-err:
- mutex_unlock(&dev->struct_mutex);
- return ERR_PTR(ret);
+ return addr;
}
static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
@@ -151,12 +127,7 @@ static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
struct drm_device *dev = obj->base.dev;
mutex_lock(&dev->struct_mutex);
- if (--obj->vmapping_count == 0) {
- vunmap(obj->dma_buf_vmapping);
- obj->dma_buf_vmapping = NULL;
-
- i915_gem_object_unpin_pages(obj);
- }
+ i915_gem_object_unpin_vmap(obj);
mutex_unlock(&dev->struct_mutex);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 15e2d29..47f186e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2056,7 +2056,7 @@ static int init_phys_status_page(struct intel_engine_cs *ring)
void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
{
if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
- vunmap(ringbuf->virtual_start);
+ i915_gem_object_unpin_vmap(ringbuf->obj);
else
iounmap(ringbuf->virtual_start);
ringbuf->virtual_start = NULL;
@@ -2080,10 +2080,10 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
if (ret)
goto unpin;
- ringbuf->virtual_start = i915_gem_object_vmap_range(obj, 0,
- ringbuf->size >> PAGE_SHIFT);
- if (ringbuf->virtual_start == NULL) {
- ret = -ENOMEM;
+ ringbuf->virtual_start = i915_gem_object_pin_vmap(obj);
+ if (IS_ERR(ringbuf->virtual_start)) {
+ ret = PTR_ERR(ringbuf->virtual_start);
+ ringbuf->virtual_start = NULL;
goto unpin;
}
} else {
--
1.9.1
More information about the Intel-gfx
mailing list