public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v99 1/4] drm/i915: Map the ringbuffer using WB on LLC machines
@ 2015-10-21  9:21 Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chris Wilson @ 2015-10-21  9:21 UTC (permalink / raw)
  To: daniel.vetter; +Cc: intel-gfx

If we have llc coherency, we can write directly into the ringbuffer
using ordinary cached writes rather than forcing WC access.

v2: An important consequence is that we can forgo the mappable request
for WB ringbuffers, allowing for many more simultaneous contexts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 70 ++++++++++++++++++++++++++-------
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index d608be46ea6e..f81ec7785fac 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1966,11 +1966,35 @@ static int init_phys_status_page(struct intel_engine_cs *ring)
 
 void intel_unpin_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
 {
-	iounmap(ringbuf->virtual_start);
+	if (HAS_LLC(ringbuf->obj->base.dev) && !ringbuf->obj->stolen)
+		vunmap(ringbuf->virtual_start);
+	else
+		iounmap(ringbuf->virtual_start);
 	ringbuf->virtual_start = NULL;
 	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)
 {
@@ -1978,21 +2002,39 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 	struct drm_i915_gem_object *obj = ringbuf->obj;
 	int ret;
 
-	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
-	if (ret)
-		return ret;
+	if (HAS_LLC(dev_priv) && !obj->stolen) {
+		ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, 0);
+		if (ret)
+			return ret;
 
-	ret = i915_gem_object_set_to_gtt_domain(obj, true);
-	if (ret) {
-		i915_gem_object_ggtt_unpin(obj);
-		return ret;
-	}
+		ret = i915_gem_object_set_to_cpu_domain(obj, true);
+		if (ret) {
+			i915_gem_object_ggtt_unpin(obj);
+			return ret;
+		}
+
+		ringbuf->virtual_start = vmap_obj(obj);
+		if (ringbuf->virtual_start == NULL) {
+			i915_gem_object_ggtt_unpin(obj);
+			return -ENOMEM;
+		}
+	} else {
+		ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
+		if (ret)
+			return ret;
 
-	ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base +
-			i915_gem_obj_ggtt_offset(obj), ringbuf->size);
-	if (ringbuf->virtual_start == NULL) {
-		i915_gem_object_ggtt_unpin(obj);
-		return -EINVAL;
+		ret = i915_gem_object_set_to_gtt_domain(obj, true);
+		if (ret) {
+			i915_gem_object_ggtt_unpin(obj);
+			return ret;
+		}
+
+		ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base +
+						    i915_gem_obj_ggtt_offset(obj), ringbuf->size);
+		if (ringbuf->virtual_start == NULL) {
+			i915_gem_object_ggtt_unpin(obj);
+			return -EINVAL;
+		}
 	}
 
 	return 0;
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions
  2015-10-21  9:21 [PATCH v99 1/4] drm/i915: Map the ringbuffer using WB on LLC machines Chris Wilson
@ 2015-10-21  9:21 ` Chris Wilson
  2015-10-21  9:44   ` Daniel Vetter
  2015-10-21  9:21 ` [PATCH v99 3/4] drm,i915: Introduce drm_malloc_gfp() Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory Chris Wilson
  2 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2015-10-21  9:21 UTC (permalink / raw)
  To: daniel.vetter; +Cc: intel-gfx

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)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         | 12 ++++---
 drivers/gpu/drm/i915/i915_gem.c         | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_dmabuf.c  | 55 +++++----------------------------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 53 ++++++++++---------------------
 4 files changed, 72 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4093eedfd664..343a0a723d2c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2089,10 +2089,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.
@@ -2840,12 +2837,19 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
 	BUG_ON(obj->pages == NULL);
 	obj->pages_pin_count++;
 }
+
 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--;
 }
 
+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);
+}
+
 int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
 int i915_gem_object_sync(struct drm_i915_gem_object *obj,
 			 struct intel_engine_cs *to,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 297d6bbbe0f2..872712c4b0ac 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2187,6 +2187,11 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
 	ops->put_pages(obj);
 	obj->pages = NULL;
 
+	if (obj->vmapping) {
+		vunmap(obj->vmapping);
+		obj->vmapping = NULL;
+	}
+
 	i915_gem_object_invalidate(obj);
 
 	return 0;
@@ -2353,6 +2358,42 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
 	return 0;
 }
 
+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) {
+		struct sg_page_iter sg_iter;
+		struct page **pages;
+		int n;
+
+		n = obj->base.size >> PAGE_SHIFT;
+		pages = kmalloc(n*sizeof(*pages), GFP_TEMPORARY | __GFP_NOWARN);
+		if (pages == NULL)
+			pages = drm_malloc_ab(n, sizeof(*pages));
+		if (pages != NULL) {
+			n = 0;
+			for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
+				pages[n++] = sg_page_iter_page(&sg_iter);
+
+			obj->vmapping = vmap(pages, n, 0, PAGE_KERNEL);
+			drm_free_large(pages);
+		}
+		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 e9c2bfd85b52..5c4163d3845a 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -95,14 +95,12 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
 {
 	struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
 
-	mutex_lock(&obj->base.dev->struct_mutex);
-
 	dma_unmap_sg(attachment->dev, sg->sgl, sg->nents, dir);
 	sg_free_table(sg);
 	kfree(sg);
 
-	i915_gem_object_unpin_pages(obj);
-
+	mutex_lock(&obj->base.dev->struct_mutex);
+	i915_gem_object_unpin_vmap(obj);
 	mutex_unlock(&obj->base.dev->struct_mutex);
 }
 
@@ -110,51 +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;
-	struct sg_page_iter sg_iter;
-	struct page **pages;
-	int ret, i;
+	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;
-
-	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);
-
-	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)
@@ -163,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_pages(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 f81ec7785fac..49aa52440db2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1967,34 +1967,12 @@ 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;
 	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)
 {
@@ -2008,15 +1986,14 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 			return ret;
 
 		ret = i915_gem_object_set_to_cpu_domain(obj, true);
-		if (ret) {
-			i915_gem_object_ggtt_unpin(obj);
-			return ret;
-		}
+		if (ret)
+			goto unpin;
 
-		ringbuf->virtual_start = vmap_obj(obj);
-		if (ringbuf->virtual_start == NULL) {
-			i915_gem_object_ggtt_unpin(obj);
-			return -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 {
 		ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
@@ -2024,20 +2001,22 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
 			return ret;
 
 		ret = i915_gem_object_set_to_gtt_domain(obj, true);
-		if (ret) {
-			i915_gem_object_ggtt_unpin(obj);
-			return ret;
-		}
+		if (ret)
+			goto unpin;
 
 		ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base +
 						    i915_gem_obj_ggtt_offset(obj), ringbuf->size);
 		if (ringbuf->virtual_start == NULL) {
-			i915_gem_object_ggtt_unpin(obj);
-			return -EINVAL;
+			ret = -ENOMEM;
+			goto unpin;
 		}
 	}
 
 	return 0;
+
+unpin:
+	i915_gem_object_ggtt_unpin(obj);
+	return ret;
 }
 
 void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v99 3/4] drm,i915: Introduce drm_malloc_gfp()
  2015-10-21  9:21 [PATCH v99 1/4] drm/i915: Map the ringbuffer using WB on LLC machines Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions Chris Wilson
@ 2015-10-21  9:21 ` Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory Chris Wilson
  2 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2015-10-21  9:21 UTC (permalink / raw)
  To: daniel.vetter; +Cc: intel-gfx, dri-devel

I have instances where I want to use drm_malloc_ab() but with a custom
gfp mask. And with those, where I want a temporary allocation, I want to
try a high-order kmalloc() before using a vmalloc().

So refactor my usage into drm_malloc_gfp().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c            |  4 +---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 +++-----
 drivers/gpu/drm/i915/i915_gem_gtt.c        |  5 +++--
 drivers/gpu/drm/i915/i915_gem_userptr.c    | 15 ++++-----------
 include/drm/drm_mem_util.h                 | 19 +++++++++++++++++++
 5 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 872712c4b0ac..1801664c445d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2374,9 +2374,7 @@ void *i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj)
 		int n;
 
 		n = obj->base.size >> PAGE_SHIFT;
-		pages = kmalloc(n*sizeof(*pages), GFP_TEMPORARY | __GFP_NOWARN);
-		if (pages == NULL)
-			pages = drm_malloc_ab(n, sizeof(*pages));
+		pages = drm_malloc_gfp(n, sizeof(*pages), GFP_TEMPORARY);
 		if (pages != NULL) {
 			n = 0;
 			for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c35c9dc526e7..91fb7417efc0 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1730,11 +1730,9 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	exec2_list = kmalloc(sizeof(*exec2_list)*args->buffer_count,
-			     GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
-	if (exec2_list == NULL)
-		exec2_list = drm_malloc_ab(sizeof(*exec2_list),
-					   args->buffer_count);
+	exec2_list = drm_malloc_gfp(sizeof(*exec2_list),
+				    args->buffer_count,
+				    GFP_TEMPORARY);
 	if (exec2_list == NULL) {
 		DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
 			  args->buffer_count);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 6e8c86d829d2..7820e8983136 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3221,8 +3221,9 @@ intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
 	int ret = -ENOMEM;
 
 	/* Allocate a temporary list of source pages for random access. */
-	page_addr_list = drm_malloc_ab(obj->base.size / PAGE_SIZE,
-				       sizeof(dma_addr_t));
+	page_addr_list = drm_malloc_gfp(obj->base.size / PAGE_SIZE,
+					sizeof(dma_addr_t),
+					GFP_TEMPORARY);
 	if (!page_addr_list)
 		return ERR_PTR(ret);
 
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index afd4c2c4cc04..885605c38e7c 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -575,10 +575,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct *_work)
 	ret = -ENOMEM;
 	pinned = 0;
 
-	pvec = kmalloc(npages*sizeof(struct page *),
-		       GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
-	if (pvec == NULL)
-		pvec = drm_malloc_ab(npages, sizeof(struct page *));
+	pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
 	if (pvec != NULL) {
 		struct mm_struct *mm = obj->userptr.mm->mm;
 
@@ -715,14 +712,10 @@ i915_gem_userptr_get_pages(struct drm_i915_gem_object *obj)
 	pvec = NULL;
 	pinned = 0;
 	if (obj->userptr.mm->mm == current->mm) {
-		pvec = kmalloc(num_pages*sizeof(struct page *),
-			       GFP_TEMPORARY | __GFP_NOWARN | __GFP_NORETRY);
+		pvec = drm_malloc_gfp(num_pages, sizeof(struct page *), GFP_TEMPORARY);
 		if (pvec == NULL) {
-			pvec = drm_malloc_ab(num_pages, sizeof(struct page *));
-			if (pvec == NULL) {
-				__i915_gem_userptr_set_active(obj, false);
-				return -ENOMEM;
-			}
+			__i915_gem_userptr_set_active(obj, false);
+			return -ENOMEM;
 		}
 
 		pinned = __get_user_pages_fast(obj->userptr.ptr, num_pages,
diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
index e42495ad8136..741ce75a72b4 100644
--- a/include/drm/drm_mem_util.h
+++ b/include/drm/drm_mem_util.h
@@ -54,6 +54,25 @@ static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
 			 GFP_KERNEL | __GFP_HIGHMEM, PAGE_KERNEL);
 }
 
+static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
+{
+	if (size != 0 && nmemb > SIZE_MAX / size)
+		return NULL;
+
+	if (size * nmemb <= PAGE_SIZE)
+	    return kmalloc(nmemb * size, gfp);
+
+	if (gfp & __GFP_RECLAIMABLE) {
+		void *ptr = kmalloc(nmemb * size,
+				    gfp | __GFP_NOWARN | __GFP_NORETRY);
+		if (ptr)
+			return ptr;
+	}
+
+	return __vmalloc(size * nmemb,
+			 gfp | __GFP_HIGHMEM, PAGE_KERNEL);
+}
+
 static __inline void drm_free_large(void *ptr)
 {
 	kvfree(ptr);
-- 
2.6.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory
  2015-10-21  9:21 [PATCH v99 1/4] drm/i915: Map the ringbuffer using WB on LLC machines Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions Chris Wilson
  2015-10-21  9:21 ` [PATCH v99 3/4] drm,i915: Introduce drm_malloc_gfp() Chris Wilson
@ 2015-10-21  9:21 ` Chris Wilson
  2015-10-30 13:19   ` Mika Kuoppala
  2 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2015-10-21  9:21 UTC (permalink / raw)
  To: daniel.vetter; +Cc: intel-gfx

Ringbuffers are now being written to either through LLC or WC paths, so
treating them as simply iomem is no longer adequate. However, for the
older !llc hardware, the hardware is documentated as treating the TAIL
register update as serialising, so we can relax the barriers when filling
the rings (but even if it were not, it is still an uncached register write
and so serialising anyway.).

For simplicity, let's ignore the iomem annotation.

v2: Remove iomem from ringbuffer->virtual_address

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        |  7 +------
 drivers/gpu/drm/i915/intel_lrc.h        |  6 +++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  7 +------
 drivers/gpu/drm/i915/intel_ringbuffer.h | 19 +++++++++++++------
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d38746c5370d..10020505be75 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -713,13 +713,8 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
 
 static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
 {
-	uint32_t __iomem *virt;
 	int rem = ringbuf->size - ringbuf->tail;
-
-	virt = ringbuf->virtual_start + ringbuf->tail;
-	rem /= 4;
-	while (rem--)
-		iowrite32(MI_NOOP, virt++);
+	memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
 
 	ringbuf->tail = 0;
 	intel_ring_update_space(ringbuf);
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 64f89f9982a2..8b56fb934763 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -53,8 +53,9 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
  */
 static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
 {
-	ringbuf->tail &= ringbuf->size - 1;
+	intel_ringbuffer_advance(ringbuf);
 }
+
 /**
  * intel_logical_ring_emit() - write a DWORD to the ringbuffer.
  * @ringbuf: Ringbuffer to write to.
@@ -63,8 +64,7 @@ static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
 static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
 					   u32 data)
 {
-	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
-	ringbuf->tail += 4;
+	intel_ringbuffer_emit(ringbuf, data);
 }
 
 /* Logical Ring Contexts */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 49aa52440db2..0eaaab92dea0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2180,13 +2180,8 @@ static int ring_wait_for_space(struct intel_engine_cs *ring, int n)
 
 static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
 {
-	uint32_t __iomem *virt;
 	int rem = ringbuf->size - ringbuf->tail;
-
-	virt = ringbuf->virtual_start + ringbuf->tail;
-	rem /= 4;
-	while (rem--)
-		iowrite32(MI_NOOP, virt++);
+	memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
 
 	ringbuf->tail = 0;
 	intel_ring_update_space(ringbuf);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 2e85fda94963..10f80df5f121 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -97,7 +97,7 @@ struct intel_ring_hangcheck {
 
 struct intel_ringbuffer {
 	struct drm_i915_gem_object *obj;
-	void __iomem *virtual_start;
+	void *virtual_start;
 
 	struct intel_engine_cs *ring;
 
@@ -427,17 +427,24 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
 
 int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
 int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
+static inline void intel_ringbuffer_emit(struct intel_ringbuffer *rb,
+					 u32 data)
+{
+	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
+	rb->tail += 4;
+}
+static inline void intel_ringbuffer_advance(struct intel_ringbuffer *rb)
+{
+	rb->tail &= rb->size - 1;
+}
 static inline void intel_ring_emit(struct intel_engine_cs *ring,
 				   u32 data)
 {
-	struct intel_ringbuffer *ringbuf = ring->buffer;
-	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
-	ringbuf->tail += 4;
+	intel_ringbuffer_emit(ring->buffer, data);
 }
 static inline void intel_ring_advance(struct intel_engine_cs *ring)
 {
-	struct intel_ringbuffer *ringbuf = ring->buffer;
-	ringbuf->tail &= ringbuf->size - 1;
+	intel_ringbuffer_advance(ring->buffer);
 }
 int __intel_ring_space(int head, int tail, int size);
 void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
-- 
2.6.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions
  2015-10-21  9:21 ` [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions Chris Wilson
@ 2015-10-21  9:44   ` Daniel Vetter
  2015-10-21 10:13     ` Chris Wilson
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2015-10-21  9:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: daniel.vetter, intel-gfx

On Wed, Oct 21, 2015 at 10:21:19AM +0100, Chris Wilson wrote:
> 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)
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 12 ++++---
>  drivers/gpu/drm/i915/i915_gem.c         | 41 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c  | 55 +++++----------------------------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 53 ++++++++++---------------------
>  4 files changed, 72 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4093eedfd664..343a0a723d2c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2089,10 +2089,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.
> @@ -2840,12 +2837,19 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
>  	BUG_ON(obj->pages == NULL);
>  	obj->pages_pin_count++;
>  }
> +
>  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--;
>  }
>  
> +void *__must_check i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj);

erm, that should be in a header.

> +static inline void i915_gem_object_unpin_vmap(struct drm_i915_gem_object *obj)
> +{
> +	i915_gem_object_unpin_pages(obj);
> +}

And this one maybe right next to it. Also, kerneldoc is missing for both
of them.

With that polish applied this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Please push the entire pile once Dave Airlie has acked the drm part for
drm_malloc_ab_gfp.
-Daniel


> +
>  int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
>  int i915_gem_object_sync(struct drm_i915_gem_object *obj,
>  			 struct intel_engine_cs *to,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 297d6bbbe0f2..872712c4b0ac 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2187,6 +2187,11 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
>  	ops->put_pages(obj);
>  	obj->pages = NULL;
>  
> +	if (obj->vmapping) {
> +		vunmap(obj->vmapping);
> +		obj->vmapping = NULL;
> +	}
> +
>  	i915_gem_object_invalidate(obj);
>  
>  	return 0;
> @@ -2353,6 +2358,42 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
>  	return 0;
>  }
>  
> +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) {
> +		struct sg_page_iter sg_iter;
> +		struct page **pages;
> +		int n;
> +
> +		n = obj->base.size >> PAGE_SHIFT;
> +		pages = kmalloc(n*sizeof(*pages), GFP_TEMPORARY | __GFP_NOWARN);
> +		if (pages == NULL)
> +			pages = drm_malloc_ab(n, sizeof(*pages));
> +		if (pages != NULL) {
> +			n = 0;
> +			for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0)
> +				pages[n++] = sg_page_iter_page(&sg_iter);
> +
> +			obj->vmapping = vmap(pages, n, 0, PAGE_KERNEL);
> +			drm_free_large(pages);
> +		}
> +		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 e9c2bfd85b52..5c4163d3845a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -95,14 +95,12 @@ static void i915_gem_unmap_dma_buf(struct dma_buf_attachment *attachment,
>  {
>  	struct drm_i915_gem_object *obj = dma_buf_to_obj(attachment->dmabuf);
>  
> -	mutex_lock(&obj->base.dev->struct_mutex);
> -
>  	dma_unmap_sg(attachment->dev, sg->sgl, sg->nents, dir);
>  	sg_free_table(sg);
>  	kfree(sg);
>  
> -	i915_gem_object_unpin_pages(obj);
> -
> +	mutex_lock(&obj->base.dev->struct_mutex);
> +	i915_gem_object_unpin_vmap(obj);
>  	mutex_unlock(&obj->base.dev->struct_mutex);
>  }
>  
> @@ -110,51 +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;
> -	struct sg_page_iter sg_iter;
> -	struct page **pages;
> -	int ret, i;
> +	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;
> -
> -	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);
> -
> -	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)
> @@ -163,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_pages(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 f81ec7785fac..49aa52440db2 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1967,34 +1967,12 @@ 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;
>  	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)
>  {
> @@ -2008,15 +1986,14 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
>  			return ret;
>  
>  		ret = i915_gem_object_set_to_cpu_domain(obj, true);
> -		if (ret) {
> -			i915_gem_object_ggtt_unpin(obj);
> -			return ret;
> -		}
> +		if (ret)
> +			goto unpin;
>  
> -		ringbuf->virtual_start = vmap_obj(obj);
> -		if (ringbuf->virtual_start == NULL) {
> -			i915_gem_object_ggtt_unpin(obj);
> -			return -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 {
>  		ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
> @@ -2024,20 +2001,22 @@ int intel_pin_and_map_ringbuffer_obj(struct drm_device *dev,
>  			return ret;
>  
>  		ret = i915_gem_object_set_to_gtt_domain(obj, true);
> -		if (ret) {
> -			i915_gem_object_ggtt_unpin(obj);
> -			return ret;
> -		}
> +		if (ret)
> +			goto unpin;
>  
>  		ringbuf->virtual_start = ioremap_wc(dev_priv->gtt.mappable_base +
>  						    i915_gem_obj_ggtt_offset(obj), ringbuf->size);
>  		if (ringbuf->virtual_start == NULL) {
> -			i915_gem_object_ggtt_unpin(obj);
> -			return -EINVAL;
> +			ret = -ENOMEM;
> +			goto unpin;
>  		}
>  	}
>  
>  	return 0;
> +
> +unpin:
> +	i915_gem_object_ggtt_unpin(obj);
> +	return ret;
>  }
>  
>  void intel_destroy_ringbuffer_obj(struct intel_ringbuffer *ringbuf)
> -- 
> 2.6.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions
  2015-10-21  9:44   ` Daniel Vetter
@ 2015-10-21 10:13     ` Chris Wilson
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2015-10-21 10:13 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: daniel.vetter, intel-gfx

On Wed, Oct 21, 2015 at 11:44:02AM +0200, Daniel Vetter wrote:
> On Wed, Oct 21, 2015 at 10:21:19AM +0100, Chris Wilson wrote:
> > 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)
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         | 12 ++++---
> >  drivers/gpu/drm/i915/i915_gem.c         | 41 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_gem_dmabuf.c  | 55 +++++----------------------------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 53 ++++++++++---------------------
> >  4 files changed, 72 insertions(+), 89 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4093eedfd664..343a0a723d2c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2089,10 +2089,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.
> > @@ -2840,12 +2837,19 @@ static inline void i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
> >  	BUG_ON(obj->pages == NULL);
> >  	obj->pages_pin_count++;
> >  }
> > +
> >  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--;
> >  }
> >  
> > +void *__must_check i915_gem_object_pin_vmap(struct drm_i915_gem_object *obj);
> 
> erm, that should be in a header.

It is.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory
  2015-10-21  9:21 ` [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory Chris Wilson
@ 2015-10-30 13:19   ` Mika Kuoppala
  0 siblings, 0 replies; 7+ messages in thread
From: Mika Kuoppala @ 2015-10-30 13:19 UTC (permalink / raw)
  To: Chris Wilson, daniel.vetter; +Cc: intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Ringbuffers are now being written to either through LLC or WC paths, so
> treating them as simply iomem is no longer adequate. However, for the
> older !llc hardware, the hardware is documentated as treating the TAIL
> register update as serialising, so we can relax the barriers when filling
> the rings (but even if it were not, it is still an uncached register write
> and so serialising anyway.).
>
> For simplicity, let's ignore the iomem annotation.
>
> v2: Remove iomem from ringbuffer->virtual_address
>

iomem annotation is for sparse. i915_irq.c
still uses ioread32 thus mixing the address spaces.

I see this has already r-b but something like this
as a followup would make sparse quiet:

diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index 6e0a568..b5e9383 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2831,7 +2831,7 @@ semaphore_waits_for(struct intel_engine_cs *ring,
u32 *seqno)
        head &= ring->buffer->size - 1;
 
                /* This here seems to blow up */
-                  cmd = ioread32(ring->buffer->virtual_start + head);
+                      cmd = intel_ringbuffer_read(ring->buffer, head);
                           if (cmd == ipehr)
                                   break;
 
@@ -2841,11 +2841,11 @@ semaphore_waits_for(struct intel_engine_cs
*ring, u32 *seqno)
       if (!i)
          return NULL;
 
-       *seqno = ioread32(ring->buffer->virtual_start + head + 4) + 1;
+       *seqno = intel_ringbuffer_read(ring->buffer, head + 4) + 1;
        if (INTEL_INFO(ring->dev)->gen >= 8) {
-          offset = ioread32(ring->buffer->virtual_start + head + 12);
+                 offset = intel_ringbuffer_read(ring->buffer, head +
12);
                offset <<= 32;
-                      offset = ioread32(ring->buffer->virtual_start +
head + 8);
+      offset = intel_ringbuffer_read(ring->buffer, head + 8);
       }
       return semaphore_wait_to_signaller_ring(ring, ipehr, offset);
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ea6f8a7..48fdfc3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1999,7 +1999,7 @@ void intel_unpin_ringbuffer_obj(struct
intel_ringbuffer *ringbuf)
                 if (HAS_LLC(ringbuf->obj->base.dev) &&
                 !ringbuf->obj->stolen)
                        vunmap(ringbuf->virtual_start);
                        else
-                               iounmap(ringbuf->virtual_start);
+                                       iounmap((void __iomem
*)ringbuf->virtual_start);
        ringbuf->virtual_start = NULL;
        i915_gem_object_ggtt_unpin(ringbuf->obj);
 }
@@ -2059,7 +2059,7 @@ int intel_pin_and_map_ringbuffer_obj(struct
drm_device *dev,
                        return ret;
                               }
 
-               ringbuf->virtual_start =
                ioremap_wc(dev_priv->gtt.mappable_base +
+
                ringbuf->virtual_start = (void __force
                *)ioremap_wc(dev_priv->gtt.mappable_base +
                                                                                            i915_gem_obj_ggtt_offset(obj),
                ringbuf->size);
                        if (ringbuf->virtual_start == NULL) {
                                                   i915_gem_object_ggtt_unpin(obj);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
                b/drivers/gpu/drm/i915/intel_ringbuffer.h
index a15f6c2..335e632 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -448,6 +448,13 @@ static inline void intel_ringbuffer_advance(struct
                intel_ringbuffer *rb)
                rb->tail &= rb->size - 1;
 }
 
+static inline u32 intel_ringbuffer_read(const struct intel_ringbuffer
*rb,
+                                       const u32 offset)
+{
+       WARN_ON(offset >= rb->size);
+       return *(uint32_t *)(rb->virtual_start + offset);
+}
+
 static inline void intel_ring_emit(struct intel_engine_cs *ring,
                                              u32 data)
 {

-Mika

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        |  7 +------
>  drivers/gpu/drm/i915/intel_lrc.h        |  6 +++---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  7 +------
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 19 +++++++++++++------
>  4 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d38746c5370d..10020505be75 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -713,13 +713,8 @@ intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request)
>  
>  static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
>  {
> -	uint32_t __iomem *virt;
>  	int rem = ringbuf->size - ringbuf->tail;
> -
> -	virt = ringbuf->virtual_start + ringbuf->tail;
> -	rem /= 4;
> -	while (rem--)
> -		iowrite32(MI_NOOP, virt++);
> +	memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
>  
>  	ringbuf->tail = 0;
>  	intel_ring_update_space(ringbuf);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index 64f89f9982a2..8b56fb934763 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -53,8 +53,9 @@ int logical_ring_flush_all_caches(struct drm_i915_gem_request *req);
>   */
>  static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
>  {
> -	ringbuf->tail &= ringbuf->size - 1;
> +	intel_ringbuffer_advance(ringbuf);
>  }
> +
>  /**
>   * intel_logical_ring_emit() - write a DWORD to the ringbuffer.
>   * @ringbuf: Ringbuffer to write to.
> @@ -63,8 +64,7 @@ static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf)
>  static inline void intel_logical_ring_emit(struct intel_ringbuffer *ringbuf,
>  					   u32 data)
>  {
> -	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
> -	ringbuf->tail += 4;
> +	intel_ringbuffer_emit(ringbuf, data);
>  }
>  
>  /* Logical Ring Contexts */
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 49aa52440db2..0eaaab92dea0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2180,13 +2180,8 @@ static int ring_wait_for_space(struct intel_engine_cs *ring, int n)
>  
>  static void __wrap_ring_buffer(struct intel_ringbuffer *ringbuf)
>  {
> -	uint32_t __iomem *virt;
>  	int rem = ringbuf->size - ringbuf->tail;
> -
> -	virt = ringbuf->virtual_start + ringbuf->tail;
> -	rem /= 4;
> -	while (rem--)
> -		iowrite32(MI_NOOP, virt++);
> +	memset(ringbuf->virtual_start + ringbuf->tail, 0, rem);
>  
>  	ringbuf->tail = 0;
>  	intel_ring_update_space(ringbuf);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 2e85fda94963..10f80df5f121 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -97,7 +97,7 @@ struct intel_ring_hangcheck {
>  
>  struct intel_ringbuffer {
>  	struct drm_i915_gem_object *obj;
> -	void __iomem *virtual_start;
> +	void *virtual_start;
>  
>  	struct intel_engine_cs *ring;
>  
> @@ -427,17 +427,24 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request);
>  
>  int __must_check intel_ring_begin(struct drm_i915_gem_request *req, int n);
>  int __must_check intel_ring_cacheline_align(struct drm_i915_gem_request *req);
> +static inline void intel_ringbuffer_emit(struct intel_ringbuffer *rb,
> +					 u32 data)
> +{
> +	*(uint32_t *)(rb->virtual_start + rb->tail) = data;
> +	rb->tail += 4;
> +}
> +static inline void intel_ringbuffer_advance(struct intel_ringbuffer *rb)
> +{
> +	rb->tail &= rb->size - 1;
> +}
>  static inline void intel_ring_emit(struct intel_engine_cs *ring,
>  				   u32 data)
>  {
> -	struct intel_ringbuffer *ringbuf = ring->buffer;
> -	iowrite32(data, ringbuf->virtual_start + ringbuf->tail);
> -	ringbuf->tail += 4;
> +	intel_ringbuffer_emit(ring->buffer, data);
>  }
>  static inline void intel_ring_advance(struct intel_engine_cs *ring)
>  {
> -	struct intel_ringbuffer *ringbuf = ring->buffer;
> -	ringbuf->tail &= ringbuf->size - 1;
> +	intel_ringbuffer_advance(ring->buffer);
>  }
>  int __intel_ring_space(int head, int tail, int size);
>  void intel_ring_update_space(struct intel_ringbuffer *ringbuf);
> -- 
> 2.6.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-10-30 13:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-21  9:21 [PATCH v99 1/4] drm/i915: Map the ringbuffer using WB on LLC machines Chris Wilson
2015-10-21  9:21 ` [PATCH v99 2/4] drm/i915: Refactor duplicate object vmap functions Chris Wilson
2015-10-21  9:44   ` Daniel Vetter
2015-10-21 10:13     ` Chris Wilson
2015-10-21  9:21 ` [PATCH v99 3/4] drm,i915: Introduce drm_malloc_gfp() Chris Wilson
2015-10-21  9:21 ` [PATCH v99 4/4] drm/i915: Treat ringbuffer writes as write to normal memory Chris Wilson
2015-10-30 13:19   ` Mika Kuoppala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox