All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 08/11] drm/i915: enable ppgtt
@ 2011-11-28 21:24 Daniel Vetter
  2011-11-28 21:24 ` [PATCH 09/11] drm/i915: split out dma mapping from global gtt bind/unbind functions Daniel Vetter
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 21:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

v2: Don't try to enable ppgtt on pre-snb.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 ++
 drivers/gpu/drm/i915/i915_drv.h |    1 +
 drivers/gpu/drm/i915/i915_gem.c |   39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f12b43e..c7cf881 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -698,6 +698,8 @@ int i915_reset(struct drm_device *dev, u8 flags)
 		if (HAS_BLT(dev))
 		    dev_priv->ring[BCS].init(&dev_priv->ring[BCS]);
 
+		i915_gem_init_ppgtt(dev);
+
 		mutex_unlock(&dev->struct_mutex);
 		drm_irq_uninstall(dev);
 		drm_mode_config_reset(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5db3b84..0d228d2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1228,6 +1228,7 @@ int __must_check i915_gem_object_set_domain(struct drm_i915_gem_object *obj,
 int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
 int __must_check i915_gem_init_hw(struct drm_device *dev);
 void i915_gem_init_swizzling(struct drm_device *dev);
+void i915_gem_init_ppgtt(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 void i915_gem_do_init(struct drm_device *dev,
 		      unsigned long start,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 593aa60..e1d7852 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3772,6 +3772,43 @@ void i915_gem_init_swizzling(struct drm_device *dev)
 				 DISP_TILE_SURFACE_SWIZZLING);
 
 }
+
+void i915_gem_init_ppgtt(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	uint32_t pd_offset;
+	struct intel_ring_buffer *ring;
+	int i;
+
+	if (!HAS_ALIASING_PPGTT(dev))
+		return;
+
+	pd_offset = dev_priv->mm.aliasing_ppgtt->pd_offset;
+	pd_offset /= 64; /* in cachelines, */
+	pd_offset <<= 16;
+
+	if (INTEL_INFO(dev)->gen == 6) {
+		uint32_t ecochk = I915_READ(GAM_ECOCHK);
+		I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
+				       ECOCHK_PPGTT_CACHE64B);
+		I915_WRITE(GFX_MODE, GFX_MODE_ENABLE(GFX_PPGTT_ENABLE));
+	} else if (INTEL_INFO(dev)->gen >= 7) {
+		I915_WRITE(GAM_ECOCHK, ECOCHK_PPGTT_CACHE64B);
+		/* GFX_MODE is per-ring on gen7+ */
+	}
+
+	for (i = 0; i < I915_NUM_RINGS; i++) {
+		ring = &dev_priv->ring[i];
+
+		if (INTEL_INFO(dev)->gen >= 7)
+			I915_WRITE(RING_MODE_GEN7(ring),
+				   GFX_MODE_ENABLE(GFX_PPGTT_ENABLE));
+
+		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
+		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
+	}
+}
+
 int
 i915_gem_init_hw(struct drm_device *dev)
 {
@@ -3798,6 +3835,8 @@ i915_gem_init_hw(struct drm_device *dev)
 
 	dev_priv->next_seqno = 1;
 
+	i915_gem_init_ppgtt(dev);
+
 	return 0;
 
 cleanup_bsd_ring:
-- 
1.7.7.3

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

* [PATCH 09/11] drm/i915: split out dma mapping from global gtt bind/unbind functions
  2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
@ 2011-11-28 21:24 ` Daniel Vetter
  2011-11-28 21:24 ` [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 21:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Note that there's a functional change buried in this patch wrt the ilk
dmar workaround: We now only idle the gpu while tearing down the dmar
mappings, not while clearing the gtt. Keeping the current semantics
would have made for some really ugly code and afaik the issue is only
with the dmar unmapping that needs a fully idle gpu.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     |    5 ++-
 drivers/gpu/drm/i915/i915_gem.c     |    6 +++-
 drivers/gpu/drm/i915/i915_gem_gtt.c |   45 +++++++++++++---------------------
 3 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0d228d2..51de703 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1276,10 +1276,11 @@ void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
 			      struct drm_i915_gem_object *obj);
 
 void i915_gem_restore_gtt_mappings(struct drm_device *dev);
-int __must_check i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj);
-void i915_gem_gtt_rebind_object(struct drm_i915_gem_object *obj,
+int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj);
+void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 				enum i915_cache_level cache_level);
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
+void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
 
 /* i915_gem_evict.c */
 int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e1d7852..234eae8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2138,6 +2138,7 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
 		obj->has_aliasing_ppgtt_mapping = 0;
 	}
+	i915_gem_gtt_finish_object(obj);
 
 	i915_gem_object_put_pages_gtt(obj);
 
@@ -2780,7 +2781,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 		return ret;
 	}
 
-	ret = i915_gem_gtt_bind_object(obj);
+	ret = i915_gem_gtt_prepare_object(obj);
 	if (ret) {
 		i915_gem_object_put_pages_gtt(obj);
 		drm_mm_put_block(obj->gtt_space);
@@ -2791,6 +2792,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 		goto search_free;
 	}
+	i915_gem_gtt_bind_object(obj, obj->cache_level);
 
 	list_add_tail(&obj->gtt_list, &dev_priv->mm.gtt_list);
 	list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
@@ -2984,7 +2986,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				return ret;
 		}
 
-		i915_gem_gtt_rebind_object(obj, cache_level);
+		i915_gem_gtt_bind_object(obj, cache_level);
 		if (obj->has_aliasing_ppgtt_mapping)
 			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
 					       obj, cache_level);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 061ae12..bfa500d 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -349,42 +349,28 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev)
 
 	list_for_each_entry(obj, &dev_priv->mm.gtt_list, gtt_list) {
 		i915_gem_clflush_object(obj);
-		i915_gem_gtt_rebind_object(obj, obj->cache_level);
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
 	}
 
 	intel_gtt_chipset_flush();
 }
 
-int i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj)
+int i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned int agp_type = cache_level_to_agp_type(dev, obj->cache_level);
-	int ret;
 
-	if (dev_priv->mm.gtt->needs_dmar) {
-		ret = intel_gtt_map_memory(obj->pages,
-					   obj->base.size >> PAGE_SHIFT,
-					   &obj->sg_list,
-					   &obj->num_sg);
-		if (ret != 0)
-			return ret;
-
-		intel_gtt_insert_sg_entries(obj->sg_list,
-					    obj->num_sg,
-					    obj->gtt_space->start >> PAGE_SHIFT,
-					    agp_type);
-	} else
-		intel_gtt_insert_pages(obj->gtt_space->start >> PAGE_SHIFT,
-				       obj->base.size >> PAGE_SHIFT,
-				       obj->pages,
-				       agp_type);
-
-	return 0;
+	if (dev_priv->mm.gtt->needs_dmar)
+		return intel_gtt_map_memory(obj->pages,
+					    obj->base.size >> PAGE_SHIFT,
+					    &obj->sg_list,
+					    &obj->num_sg);
+	else
+		return 0;
 }
 
-void i915_gem_gtt_rebind_object(struct drm_i915_gem_object *obj,
-				enum i915_cache_level cache_level)
+void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
+			      enum i915_cache_level cache_level)
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -406,15 +392,18 @@ void i915_gem_gtt_rebind_object(struct drm_i915_gem_object *obj,
 
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
 {
+	intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
+			      obj->base.size >> PAGE_SHIFT);
+}
+
+void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
+{
 	struct drm_device *dev = obj->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	bool interruptible;
 
 	interruptible = do_idling(dev_priv);
 
-	intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
-			      obj->base.size >> PAGE_SHIFT);
-
 	if (obj->sg_list) {
 		intel_gtt_unmap_memory(obj->sg_list, obj->num_sg);
 		obj->sg_list = NULL;
-- 
1.7.7.3

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

* [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed
  2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
  2011-11-28 21:24 ` [PATCH 09/11] drm/i915: split out dma mapping from global gtt bind/unbind functions Daniel Vetter
@ 2011-11-28 21:24 ` Daniel Vetter
  2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
  2011-11-29  1:55   ` [PATCH 10/11] " Eric Anholt
  2011-11-28 21:24 ` [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace Daniel Vetter
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 21:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

And track the existence of such a binding similar to the aliasing
ppgtt case. Speeds up binding/unbinding in the common case where we
only need a ppgtt binding (which is accessed in a cpu coherent fashion
by the gpu) and no gloabl gtt binding (which needs uc writes for the
ptes).

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h            |    1 +
 drivers/gpu/drm/i915/i915_gem.c            |   16 +++++++++++++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   10 ++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c        |    4 ++++
 4 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 51de703..462dd11 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -857,6 +857,7 @@ struct drm_i915_gem_object {
 	unsigned int cache_level:2;
 
 	unsigned int has_aliasing_ppgtt_mapping:1;
+	unsigned int has_global_gtt_mapping:1;
 
 	struct page **pages;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 234eae8..ee907d8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1224,6 +1224,9 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			goto unlock;
 	}
 
+	if (!obj->has_global_gtt_mapping)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->tiling_mode == I915_TILING_NONE)
 		ret = i915_gem_object_put_fence(obj);
 	else
@@ -2133,7 +2136,8 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 
 	trace_i915_gem_object_unbind(obj);
 
-	i915_gem_gtt_unbind_object(obj);
+	if (obj->has_global_gtt_mapping)
+		i915_gem_gtt_unbind_object(obj);
 	if (obj->has_aliasing_ppgtt_mapping) {
 		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
 		obj->has_aliasing_ppgtt_mapping = 0;
@@ -2792,7 +2796,9 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 		goto search_free;
 	}
-	i915_gem_gtt_bind_object(obj, obj->cache_level);
+
+	if (!dev_priv->mm.aliasing_ppgtt)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
 
 	list_add_tail(&obj->gtt_list, &dev_priv->mm.gtt_list);
 	list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
@@ -2986,7 +2992,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				return ret;
 		}
 
-		i915_gem_gtt_bind_object(obj, cache_level);
+		if (obj->has_global_gtt_mapping)
+			i915_gem_gtt_bind_object(obj, cache_level);
 		if (obj->has_aliasing_ppgtt_mapping)
 			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
 					       obj, cache_level);
@@ -3369,6 +3376,9 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 			return ret;
 	}
 
+	if (!obj->has_global_gtt_mapping && map_and_fenceable)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->pin_count++ == 0) {
 		if (!obj->active)
 			list_move_tail(&obj->mm_list,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 9c81cda..c7a681a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -381,6 +381,16 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 		io_mapping_unmap_atomic(reloc_page);
 	}
 
+	/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
+	 * pipe_control writes because the gpu doesn't properly redirect them
+	 * through the ppgtt for non_secure batchbuffers. */
+	if (unlikely(IS_GEN6(dev) &&
+	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
+	    !obj->has_global_gtt_mapping)) {
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+		obj->has_global_gtt_mapping = 1;
+	}
+
 	/* and update the user's relocation entry */
 	reloc->presumed_offset = target_offset;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bfa500d..f437d4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -388,12 +388,16 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 				       obj->base.size >> PAGE_SHIFT,
 				       obj->pages,
 				       agp_type);
+
+	obj->has_global_gtt_mapping = 1;
 }
 
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
 {
 	intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
 			      obj->base.size >> PAGE_SHIFT);
+
+	obj->has_global_gtt_mapping = 0;
 }
 
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
-- 
1.7.7.3

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

* [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace
  2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
  2011-11-28 21:24 ` [PATCH 09/11] drm/i915: split out dma mapping from global gtt bind/unbind functions Daniel Vetter
  2011-11-28 21:24 ` [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
@ 2011-11-28 21:24 ` Daniel Vetter
  2011-12-06 17:39   ` Ben Widawsky
  2011-11-29 12:47 ` [PATCH 08/11] drm/i915: enable ppgtt Chris Wilson
  2011-11-30  0:05 ` Ben Widawsky
  4 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 21:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

On Sanybridge a few MI read/write commands only work when ppgtt is
enabled. Userspace therefore needs to be able to check whether ppgtt
is enabled. For added hilarity, you need to reset the "use global GTT"
bit on both snb/ivb when ppgtt is enabled, otherwise it won't work.
Despite what bspec says about automatically using ppgtt ...

Luckily PIPE_CONTROL (the only write cmd current userspace uses) is
not affected by all this, as tested by tests/gem_pipe_control_store_loop.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_dma.c |    3 +++
 include/drm/i915_drm.h          |    1 +
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 9d9a92c..c3ab856 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -781,6 +781,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_RELAXED_DELTA:
 		value = 1;
 		break;
+	case I915_PARAM_HAS_ALIASING_PPGTT:
+		value = dev_priv->mm.aliasing_ppgtt ? 1 : 0;
+		break;
 	default:
 		DRM_DEBUG_DRIVER("Unknown parameter %d\n",
 				 param->param);
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 28c0d11..ebc8439 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -291,6 +291,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_COHERENT_RINGS	 13
 #define I915_PARAM_HAS_EXEC_CONSTANTS	 14
 #define I915_PARAM_HAS_RELAXED_DELTA	 15
+#define I915_PARAM_HAS_ALIASING_PPGTT	 16
 
 typedef struct drm_i915_getparam {
 	int param;
-- 
1.7.7.3

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

* [PATCH 1/3] drm/i915: bind objects to the global gtt only when needed
  2011-11-28 21:24 ` [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
@ 2011-11-28 22:32   ` Daniel Vetter
  2011-11-28 22:32     ` [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt Daniel Vetter
                       ` (2 more replies)
  2011-11-29  1:55   ` [PATCH 10/11] " Eric Anholt
  1 sibling, 3 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

And track the existence of such a binding similar to the aliasing
ppgtt case. Speeds up binding/unbinding in the common case where we
only need a ppgtt binding (which is accessed in a cpu coherent fashion
by the gpu) and no gloabl gtt binding (which needs uc writes for the
ptes).

This patch just puts the required tracking in place.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_gem.c     |   12 ++++++++++--
 drivers/gpu/drm/i915/i915_gem_gtt.c |    4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e026299..8b5c016 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -857,6 +857,7 @@ struct drm_i915_gem_object {
 	unsigned int cache_level:2;
 
 	unsigned int has_aliasing_ppgtt_mapping:1;
+	unsigned int has_global_gtt_mapping:1;
 
 	struct page **pages;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 234eae8..88e5c1b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1224,6 +1224,9 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			goto unlock;
 	}
 
+	if (!obj->has_global_gtt_mapping)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->tiling_mode == I915_TILING_NONE)
 		ret = i915_gem_object_put_fence(obj);
 	else
@@ -2133,7 +2136,8 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 
 	trace_i915_gem_object_unbind(obj);
 
-	i915_gem_gtt_unbind_object(obj);
+	if (obj->has_global_gtt_mapping)
+		i915_gem_gtt_unbind_object(obj);
 	if (obj->has_aliasing_ppgtt_mapping) {
 		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
 		obj->has_aliasing_ppgtt_mapping = 0;
@@ -2986,7 +2990,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				return ret;
 		}
 
-		i915_gem_gtt_bind_object(obj, cache_level);
+		if (obj->has_global_gtt_mapping)
+			i915_gem_gtt_bind_object(obj, cache_level);
 		if (obj->has_aliasing_ppgtt_mapping)
 			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
 					       obj, cache_level);
@@ -3369,6 +3374,9 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 			return ret;
 	}
 
+	if (!obj->has_global_gtt_mapping && map_and_fenceable)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->pin_count++ == 0) {
 		if (!obj->active)
 			list_move_tail(&obj->mm_list,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bfa500d..f437d4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -388,12 +388,16 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 				       obj->base.size >> PAGE_SHIFT,
 				       obj->pages,
 				       agp_type);
+
+	obj->has_global_gtt_mapping = 1;
 }
 
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
 {
 	intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
 			      obj->base.size >> PAGE_SHIFT);
+
+	obj->has_global_gtt_mapping = 0;
 }
 
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
-- 
1.7.7.3

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

* [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt
  2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
@ 2011-11-28 22:32     ` Daniel Vetter
  2011-12-02 22:07       ` [PATCH] " Daniel Vetter
  2011-11-28 22:32     ` [PATCH 3/3] drm/i915: enable lazy global-gtt binding Daniel Vetter
  2011-11-29 11:27     ` [PATCH] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
  2 siblings, 1 reply; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

PIPE_CONTROL on snb needs global gtt mappings in place to workaround a
hw gotcha. No other commands need such a workaround. Luckily we can
detect a PIPE_CONTROL commands easily because they have a write_domain
= I915_GEM_DOMAIN_INSTRUCTION (and nothing else has that).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 9c81cda..c7a681a 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -381,6 +381,16 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 		io_mapping_unmap_atomic(reloc_page);
 	}
 
+	/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
+	 * pipe_control writes because the gpu doesn't properly redirect them
+	 * through the ppgtt for non_secure batchbuffers. */
+	if (unlikely(IS_GEN6(dev) &&
+	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
+	    !obj->has_global_gtt_mapping)) {
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+		obj->has_global_gtt_mapping = 1;
+	}
+
 	/* and update the user's relocation entry */
 	reloc->presumed_offset = target_offset;
 
-- 
1.7.7.3

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

* [PATCH 3/3] drm/i915: enable lazy global-gtt binding
  2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
  2011-11-28 22:32     ` [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt Daniel Vetter
@ 2011-11-28 22:32     ` Daniel Vetter
  2011-11-29 11:27     ` [PATCH] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
  2 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-28 22:32 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

Now that everything is in place, only bind to the global gtt
when actually required. Patch split-up suggested by Chris Wilson.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 88e5c1b..ee907d8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2796,7 +2796,9 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj,
 
 		goto search_free;
 	}
-	i915_gem_gtt_bind_object(obj, obj->cache_level);
+
+	if (!dev_priv->mm.aliasing_ppgtt)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
 
 	list_add_tail(&obj->gtt_list, &dev_priv->mm.gtt_list);
 	list_add_tail(&obj->mm_list, &dev_priv->mm.inactive_list);
-- 
1.7.7.3

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

* Re: [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed
  2011-11-28 21:24 ` [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
  2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
@ 2011-11-29  1:55   ` Eric Anholt
  2011-12-02 22:12     ` Daniel Vetter
  1 sibling, 1 reply; 17+ messages in thread
From: Eric Anholt @ 2011-11-29  1:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter


[-- Attachment #1.1: Type: text/plain, Size: 737 bytes --]

On Mon, 28 Nov 2011 22:24:54 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> And track the existence of such a binding similar to the aliasing
> ppgtt case. Speeds up binding/unbinding in the common case where we
> only need a ppgtt binding (which is accessed in a cpu coherent fashion
> by the gpu) and no gloabl gtt binding (which needs uc writes for the
> ptes).

For one month from mid-june to mid-july, Mesa master was setting
I915_GEM_DOMAIN_GTT in the workaround pipe control.  That was changed to
be _INSTRUCTION, because _GTT caused kernel oopses, and query objects
had already been doing _INSTRUCTION since forever.  There are no uses of
MI_STOREs in the driver.

I'd say the minor ABI change potential in here is safe.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* [PATCH] drm/i915: bind objects to the global gtt only when needed
  2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
  2011-11-28 22:32     ` [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt Daniel Vetter
  2011-11-28 22:32     ` [PATCH 3/3] drm/i915: enable lazy global-gtt binding Daniel Vetter
@ 2011-11-29 11:27     ` Daniel Vetter
  2 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-11-29 11:27 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

And track the existence of such a binding similar to the aliasing
ppgtt case. Speeds up binding/unbinding in the common case where we
only need a ppgtt binding (which is accessed in a cpu coherent fashion
by the gpu) and no gloabl gtt binding (which needs uc writes for the
ptes).

This patch just puts the required tracking in place.

v2: Check that global gtt mappings exist in the error_state capture
code (they should because we pin all relevant things as mappable).
Suggested by Chris Wilson.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_gem.c     |   12 ++++++++++--
 drivers/gpu/drm/i915/i915_gem_gtt.c |    4 ++++
 drivers/gpu/drm/i915/i915_irq.c     |    3 +++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e026299..8b5c016 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -857,6 +857,7 @@ struct drm_i915_gem_object {
 	unsigned int cache_level:2;
 
 	unsigned int has_aliasing_ppgtt_mapping:1;
+	unsigned int has_global_gtt_mapping:1;
 
 	struct page **pages;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 234eae8..88e5c1b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1224,6 +1224,9 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 			goto unlock;
 	}
 
+	if (!obj->has_global_gtt_mapping)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->tiling_mode == I915_TILING_NONE)
 		ret = i915_gem_object_put_fence(obj);
 	else
@@ -2133,7 +2136,8 @@ i915_gem_object_unbind(struct drm_i915_gem_object *obj)
 
 	trace_i915_gem_object_unbind(obj);
 
-	i915_gem_gtt_unbind_object(obj);
+	if (obj->has_global_gtt_mapping)
+		i915_gem_gtt_unbind_object(obj);
 	if (obj->has_aliasing_ppgtt_mapping) {
 		i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj);
 		obj->has_aliasing_ppgtt_mapping = 0;
@@ -2986,7 +2990,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
 				return ret;
 		}
 
-		i915_gem_gtt_bind_object(obj, cache_level);
+		if (obj->has_global_gtt_mapping)
+			i915_gem_gtt_bind_object(obj, cache_level);
 		if (obj->has_aliasing_ppgtt_mapping)
 			i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt,
 					       obj, cache_level);
@@ -3369,6 +3374,9 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj,
 			return ret;
 	}
 
+	if (!obj->has_global_gtt_mapping && map_and_fenceable)
+		i915_gem_gtt_bind_object(obj, obj->cache_level);
+
 	if (obj->pin_count++ == 0) {
 		if (!obj->active)
 			list_move_tail(&obj->mm_list,
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index bfa500d..f437d4b 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -388,12 +388,16 @@ void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
 				       obj->base.size >> PAGE_SHIFT,
 				       obj->pages,
 				       agp_type);
+
+	obj->has_global_gtt_mapping = 1;
 }
 
 void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj)
 {
 	intel_gtt_clear_range(obj->gtt_space->start >> PAGE_SHIFT,
 			      obj->base.size >> PAGE_SHIFT);
+
+	obj->has_global_gtt_mapping = 0;
 }
 
 void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 65dc543..20f6e5a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -711,6 +711,9 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
 	if (src == NULL || src->pages == NULL || !src->map_and_fenceable)
 		return NULL;
 
+	if (!src->has_global_gtt_mapping)
+		return NULL;
+
 	page_count = src->base.size / PAGE_SIZE;
 
 	dst = kmalloc(sizeof(*dst) + page_count * sizeof(u32 *), GFP_ATOMIC);
-- 
1.7.7.3

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

* Re: [PATCH 08/11] drm/i915: enable ppgtt
  2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
                   ` (2 preceding siblings ...)
  2011-11-28 21:24 ` [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace Daniel Vetter
@ 2011-11-29 12:47 ` Chris Wilson
  2011-11-30  0:05 ` Ben Widawsky
  4 siblings, 0 replies; 17+ messages in thread
From: Chris Wilson @ 2011-11-29 12:47 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

On Mon, 28 Nov 2011 22:24:52 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> +void i915_gem_init_ppgtt(struct drm_device *dev)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	uint32_t pd_offset;
> +	struct intel_ring_buffer *ring;
> +	int i;
> +
> +	if (!HAS_ALIASING_PPGTT(dev))
> +		return;

This wants to be

  if (dev_priv->mm.aliasing_ppgtt == NULL)
    return;

instead. Slightly safer against the clumsy ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 08/11] drm/i915: enable ppgtt
  2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
                   ` (3 preceding siblings ...)
  2011-11-29 12:47 ` [PATCH 08/11] drm/i915: enable ppgtt Chris Wilson
@ 2011-11-30  0:05 ` Ben Widawsky
  2011-12-02 22:19   ` Daniel Vetter
  4 siblings, 1 reply; 17+ messages in thread
From: Ben Widawsky @ 2011-11-30  0:05 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Nov 28, 2011 at 10:24:52PM +0100, Daniel Vetter wrote:
> v2: Don't try to enable ppgtt on pre-snb.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |    2 ++
>  drivers/gpu/drm/i915/i915_drv.h |    1 +
>  drivers/gpu/drm/i915/i915_gem.c |   39 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f12b43e..c7cf881 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -698,6 +698,8 @@ int i915_reset(struct drm_device *dev, u8 flags)
>  		if (HAS_BLT(dev))
>  		    dev_priv->ring[BCS].init(&dev_priv->ring[BCS]);
>  
> +		i915_gem_init_ppgtt(dev);
> +
>  		mutex_unlock(&dev->struct_mutex);
>  		drm_irq_uninstall(dev);
>  		drm_mode_config_reset(dev);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5db3b84..0d228d2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1228,6 +1228,7 @@ int __must_check i915_gem_object_set_domain(struct drm_i915_gem_object *obj,
>  int __must_check i915_gem_object_finish_gpu(struct drm_i915_gem_object *obj);
>  int __must_check i915_gem_init_hw(struct drm_device *dev);
>  void i915_gem_init_swizzling(struct drm_device *dev);
> +void i915_gem_init_ppgtt(struct drm_device *dev);
>  void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
>  void i915_gem_do_init(struct drm_device *dev,
>  		      unsigned long start,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 593aa60..e1d7852 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3772,6 +3772,43 @@ void i915_gem_init_swizzling(struct drm_device *dev)
>  				 DISP_TILE_SURFACE_SWIZZLING);
>  
>  }
> +
> +void i915_gem_init_ppgtt(struct drm_device *dev)
> +{
> +	drm_i915_private_t *dev_priv = dev->dev_private;
> +	uint32_t pd_offset;
> +	struct intel_ring_buffer *ring;
> +	int i;
> +
> +	if (!HAS_ALIASING_PPGTT(dev))
> +		return;
> +
> +	pd_offset = dev_priv->mm.aliasing_ppgtt->pd_offset;
> +	pd_offset /= 64; /* in cachelines, */
> +	pd_offset <<= 16;
> +
> +	if (INTEL_INFO(dev)->gen == 6) {
> +		uint32_t ecochk = I915_READ(GAM_ECOCHK);
> +		I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
> +				       ECOCHK_PPGTT_CACHE64B);
> +		I915_WRITE(GFX_MODE, GFX_MODE_ENABLE(GFX_PPGTT_ENABLE));
> +	} else if (INTEL_INFO(dev)->gen >= 7) {
> +		I915_WRITE(GAM_ECOCHK, ECOCHK_PPGTT_CACHE64B);
> +		/* GFX_MODE is per-ring on gen7+ */
> +	}
> +
> +	for (i = 0; i < I915_NUM_RINGS; i++) {
> +		ring = &dev_priv->ring[i];
> +
> +		if (INTEL_INFO(dev)->gen >= 7)
> +			I915_WRITE(RING_MODE_GEN7(ring),
> +				   GFX_MODE_ENABLE(GFX_PPGTT_ENABLE));
> +
> +		I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G);
> +		I915_WRITE(RING_PP_DIR_BASE(ring), pd_offset);
> +	}
> +}
> +
>  int
>  i915_gem_init_hw(struct drm_device *dev)
>  {
> @@ -3798,6 +3835,8 @@ i915_gem_init_hw(struct drm_device *dev)
>  
>  	dev_priv->next_seqno = 1;
>  
> +	i915_gem_init_ppgtt(dev);
> +
>  	return 0;
>  
>  cleanup_bsd_ring:
Maybe through some DRM_INFOs in here for later debugging to see if we
failed in setting up the page tables?

Also, does this work across resume and reset?

Ben

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

* [PATCH] drm/i915: implement SNB workaround for lazy global gtt
  2011-11-28 22:32     ` [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt Daniel Vetter
@ 2011-12-02 22:07       ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-12-02 22:07 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

PIPE_CONTROL on snb needs global gtt mappings in place to workaround a
hw gotcha. No other commands need such a workaround. Luckily we can
detect a PIPE_CONTROL commands easily because they have a write_domain
= I915_GEM_DOMAIN_INSTRUCTION (and nothing else has that).

v2: Binding the target of such a reloc into the global gtt actually
works instead of binding the source, which is rather pointless ...

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 962def5..1e710ae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -278,6 +278,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 {
 	struct drm_device *dev = obj->base.dev;
 	struct drm_gem_object *target_obj;
+	struct drm_i915_gem_object *target_i915_obj;
 	uint32_t target_offset;
 	int ret = -EINVAL;
 
@@ -286,7 +287,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 	if (unlikely(target_obj == NULL))
 		return -ENOENT;
 
-	target_offset = to_intel_bo(target_obj)->gtt_offset;
+	target_i915_obj = to_intel_bo(target_obj);
+	target_offset = target_i915_obj->gtt_offset;
 
 	/* The target buffer should have appeared before us in the
 	 * exec_object list, so it should have a GTT space bound by now.
@@ -388,6 +390,17 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
 		io_mapping_unmap_atomic(reloc_page);
 	}
 
+	/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
+	 * pipe_control writes because the gpu doesn't properly redirect them
+	 * through the ppgtt for non_secure batchbuffers. */
+	if (unlikely(IS_GEN6(dev) &&
+	    reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION &&
+	    !target_i915_obj->has_global_gtt_mapping)) {
+		i915_gem_gtt_bind_object(target_i915_obj,
+					 target_i915_obj->cache_level);
+		target_i915_obj->has_global_gtt_mapping = 1;
+	}
+
 	/* and update the user's relocation entry */
 	reloc->presumed_offset = target_offset;
 
-- 
1.7.6.4

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

* Re: [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed
  2011-11-29  1:55   ` [PATCH 10/11] " Eric Anholt
@ 2011-12-02 22:12     ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-12-02 22:12 UTC (permalink / raw)
  To: Eric Anholt; +Cc: Daniel Vetter, intel-gfx

On Mon, Nov 28, 2011 at 05:55:58PM -0800, Eric Anholt wrote:
> On Mon, 28 Nov 2011 22:24:54 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > And track the existence of such a binding similar to the aliasing
> > ppgtt case. Speeds up binding/unbinding in the common case where we
> > only need a ppgtt binding (which is accessed in a cpu coherent fashion
> > by the gpu) and no gloabl gtt binding (which needs uc writes for the
> > ptes).
> 
> For one month from mid-june to mid-july, Mesa master was setting
> I915_GEM_DOMAIN_GTT in the workaround pipe control.  That was changed to
> be _INSTRUCTION, because _GTT caused kernel oopses, and query objects
> had already been doing _INSTRUCTION since forever.  There are no uses of
> MI_STOREs in the driver.
> 
> I'd say the minor ABI change potential in here is safe.

Can I interpret this as an Acked-by for the (fixed up and actually tested)
patch?
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* Re: [PATCH 08/11] drm/i915: enable ppgtt
  2011-11-30  0:05 ` Ben Widawsky
@ 2011-12-02 22:19   ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-12-02 22:19 UTC (permalink / raw)
  To: Daniel Vetter, intel-gfx

Sorry, missed your mail.

On Tue, Nov 29, 2011 at 04:05:53PM -0800, Ben Widawsky wrote:
> On Mon, Nov 28, 2011 at 10:24:52PM +0100, Daniel Vetter wrote:
> Maybe through some DRM_INFOs in here for later debugging to see if we
> failed in setting up the page tables?

Tha's kinda what the debugfs file is for, to check whether the registers
have the right values in an easy way. Caveat though, there are some errata
on snb that you can't read back stuff from the render ring registers. And
experience shows that if you frob these too often, they start to read
garbage (but writes still work as they should, strangely).

> Also, does this work across resume and reset?

i915_gem_init_hw is called for both initial setup and resum, and the reset
code is a few hunks up. Reset tested with my gpu hangman.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

* Re: [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace
  2011-11-28 21:24 ` [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace Daniel Vetter
@ 2011-12-06 17:39   ` Ben Widawsky
  2011-12-06 18:31     ` Chris Wilson
  0 siblings, 1 reply; 17+ messages in thread
From: Ben Widawsky @ 2011-12-06 17:39 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Mon, Nov 28, 2011 at 10:24:55PM +0100, Daniel Vetter wrote:
> On Sanybridge a few MI read/write commands only work when ppgtt is
> enabled. Userspace therefore needs to be able to check whether ppgtt
> is enabled. For added hilarity, you need to reset the "use global GTT"
> bit on both snb/ivb when ppgtt is enabled, otherwise it won't work.
> Despite what bspec says about automatically using ppgtt ...
> 
> Luckily PIPE_CONTROL (the only write cmd current userspace uses) is
> not affected by all this, as tested by tests/gem_pipe_control_store_loop.

Since this is all SNB only, and we have no good benchmarks to show
performance gains, can we not just enable this for IVB+?

> 
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

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

* Re: [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace
  2011-12-06 17:39   ` Ben Widawsky
@ 2011-12-06 18:31     ` Chris Wilson
  2011-12-06 19:59       ` Daniel Vetter
  0 siblings, 1 reply; 17+ messages in thread
From: Chris Wilson @ 2011-12-06 18:31 UTC (permalink / raw)
  To: Ben Widawsky, Daniel Vetter; +Cc: intel-gfx

On Tue, 6 Dec 2011 09:39:39 -0800, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Mon, Nov 28, 2011 at 10:24:55PM +0100, Daniel Vetter wrote:
> > On Sanybridge a few MI read/write commands only work when ppgtt is
> > enabled. Userspace therefore needs to be able to check whether ppgtt
> > is enabled. For added hilarity, you need to reset the "use global GTT"
> > bit on both snb/ivb when ppgtt is enabled, otherwise it won't work.
> > Despite what bspec says about automatically using ppgtt ...
> > 
> > Luckily PIPE_CONTROL (the only write cmd current userspace uses) is
> > not affected by all this, as tested by tests/gem_pipe_control_store_loop.
> 
> Since this is all SNB only, and we have no good benchmarks to show
> performance gains, can we not just enable this for IVB+?

Don't we have the same basic problem with IVB, that we need to adjust
the commands in the batchbuffer depending on whether HAS_ALIASING_PPGTT
is true? Or do you mean that we should just assume that IVB uses the
ppgtt and so kernel 3.3+, which will be around March...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace
  2011-12-06 18:31     ` Chris Wilson
@ 2011-12-06 19:59       ` Daniel Vetter
  0 siblings, 0 replies; 17+ messages in thread
From: Daniel Vetter @ 2011-12-06 19:59 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, Ben Widawsky, intel-gfx

On Tue, Dec 06, 2011 at 06:31:14PM +0000, Chris Wilson wrote:
> On Tue, 6 Dec 2011 09:39:39 -0800, Ben Widawsky <ben@bwidawsk.net> wrote:
> > On Mon, Nov 28, 2011 at 10:24:55PM +0100, Daniel Vetter wrote:
> > > On Sanybridge a few MI read/write commands only work when ppgtt is
> > > enabled. Userspace therefore needs to be able to check whether ppgtt
> > > is enabled. For added hilarity, you need to reset the "use global GTT"
> > > bit on both snb/ivb when ppgtt is enabled, otherwise it won't work.
> > > Despite what bspec says about automatically using ppgtt ...
> > >
> > > Luckily PIPE_CONTROL (the only write cmd current userspace uses) is
> > > not affected by all this, as tested by tests/gem_pipe_control_store_loop.
> >
> > Since this is all SNB only, and we have no good benchmarks to show
> > performance gains, can we not just enable this for IVB+?
>
> Don't we have the same basic problem with IVB, that we need to adjust
> the commands in the batchbuffer depending on whether HAS_ALIASING_PPGTT
> is true? Or do you mean that we should just assume that IVB uses the
> ppgtt and so kernel 3.3+, which will be around March...

Clarified this a bit with Ben on irc: We need to change the MI_STORE/LOAD
commands on both snb and ivb depending upon whether ppgtt is enabled or
not. Luckily no current userspace needs this (otherwise it'd be an obvious
no-go), but might come in handy for certain features - currently only a
bunch of crazy i-g-t tests that hammer the cpu with gpu interrupts from
different rings and try to test whether that works correctly.

Another story is PIPE_CONTROL where ppgtt is transparent like for all
other render commands (i.e. hw transparently switches to ppgtt access if
the buffer is marked non-secure), safe for the fact that we need the w/a
in place for snb.

I think ppgtt also makes sense on snb (even though there's not much
speedup to be had there):
- it's what the windows driver uses
- it seems to improve stability for the vt-d+semaphore issues (not
  everywhere though).
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

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

end of thread, other threads:[~2011-12-06 19:59 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-28 21:24 [PATCH 08/11] drm/i915: enable ppgtt Daniel Vetter
2011-11-28 21:24 ` [PATCH 09/11] drm/i915: split out dma mapping from global gtt bind/unbind functions Daniel Vetter
2011-11-28 21:24 ` [PATCH 10/11] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
2011-11-28 22:32   ` [PATCH 1/3] " Daniel Vetter
2011-11-28 22:32     ` [PATCH 2/3] drm/i915: implement SNB workaround for lazy global gtt Daniel Vetter
2011-12-02 22:07       ` [PATCH] " Daniel Vetter
2011-11-28 22:32     ` [PATCH 3/3] drm/i915: enable lazy global-gtt binding Daniel Vetter
2011-11-29 11:27     ` [PATCH] drm/i915: bind objects to the global gtt only when needed Daniel Vetter
2011-11-29  1:55   ` [PATCH 10/11] " Eric Anholt
2011-12-02 22:12     ` Daniel Vetter
2011-11-28 21:24 ` [PATCH 11/11] drm/i915: add HAS_ALIASING_PPGTT parameter for userspace Daniel Vetter
2011-12-06 17:39   ` Ben Widawsky
2011-12-06 18:31     ` Chris Wilson
2011-12-06 19:59       ` Daniel Vetter
2011-11-29 12:47 ` [PATCH 08/11] drm/i915: enable ppgtt Chris Wilson
2011-11-30  0:05 ` Ben Widawsky
2011-12-02 22:19   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.