[PATCH 1/6] drm/i915: Replace hardcoded cacheline size with macro

[PATCH 1/6] drm/i915: Replace hardcoded cacheline size with macro

[Chris Wilson]

For readibility and guess at the meaning behind the constants.

v2: Claim only the meagerest connections with reality.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 34 ++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 3d76ce135a05..9bed8764a62a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -33,6 +33,13 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
+/* Early gen2 devices have a cacheline of just 32 bytes, using 64 is overkill,
+ * but keeps the logic simple. Indeed, the whole purpose of this macro is just
+ * to give some inclination as to some of the magic values used in the various
+ * workarounds!
+ */
+#define CACHELINE_BYTES 64
+
 static inline int ring_space(struct intel_ring_buffer *ring)
 {
 	int space = (ring->head & HEAD_ADDR) - (ring->tail + I915_RING_FREE_SPACE);
@@ -175,7 +182,7 @@ gen4_render_ring_flush(struct intel_ring_buffer *ring,
 static int
 intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
 {
-	u32 scratch_addr = ring->scratch.gtt_offset + 128;
+	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	int ret;
 
 
@@ -212,7 +219,7 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
                          u32 invalidate_domains, u32 flush_domains)
 {
 	u32 flags = 0;
-	u32 scratch_addr = ring->scratch.gtt_offset + 128;
+	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	int ret;
 
 	/* Force SNB workarounds for PIPE_CONTROL flushes */
@@ -306,7 +313,7 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
 {
 	u32 flags = 0;
-	u32 scratch_addr = ring->scratch.gtt_offset + 128;
+	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	int ret;
 
 	/*
@@ -367,7 +374,7 @@ gen8_render_ring_flush(struct intel_ring_buffer *ring,
 		       u32 invalidate_domains, u32 flush_domains)
 {
 	u32 flags = 0;
-	u32 scratch_addr = ring->scratch.gtt_offset + 128;
+	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	int ret;
 
 	flags |= PIPE_CONTROL_CS_STALL;
@@ -779,7 +786,7 @@ do {									\
 static int
 pc_render_add_request(struct intel_ring_buffer *ring)
 {
-	u32 scratch_addr = ring->scratch.gtt_offset + 128;
+	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	int ret;
 
 	/* For Ironlake, MI_USER_INTERRUPT was deprecated and apparently
@@ -801,15 +808,15 @@ pc_render_add_request(struct intel_ring_buffer *ring)
 	intel_ring_emit(ring, ring->outstanding_lazy_seqno);
 	intel_ring_emit(ring, 0);
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
-	scratch_addr += 128; /* write to separate cachelines */
+	scratch_addr += 2 * CACHELINE_BYTES; /* write to separate cachelines */
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
-	scratch_addr += 128;
+	scratch_addr += 2 * CACHELINE_BYTES;
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
-	scratch_addr += 128;
+	scratch_addr += 2 * CACHELINE_BYTES;
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
-	scratch_addr += 128;
+	scratch_addr += 2 * CACHELINE_BYTES;
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
-	scratch_addr += 128;
+	scratch_addr += 2 * CACHELINE_BYTES;
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
 
 	intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE |
@@ -1418,7 +1425,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 	 */
 	ring->effective_size = ring->size;
 	if (IS_I830(ring->dev) || IS_845G(ring->dev))
-		ring->effective_size -= 128;
+		ring->effective_size -= 2 * CACHELINE_BYTES;
 
 	i915_cmd_parser_init_ring(ring);
 
@@ -1679,12 +1686,13 @@ int intel_ring_begin(struct intel_ring_buffer *ring,
 /* Align the ring tail to a cacheline boundary */
 int intel_ring_cacheline_align(struct intel_ring_buffer *ring)
 {
-	int num_dwords = (64 - (ring->tail & 63)) / sizeof(uint32_t);
+	int num_dwords = (ring->tail & (CACHELINE_BYTES - 1)) / sizeof(uint32_t);
 	int ret;
 
 	if (num_dwords == 0)
 		return 0;
 
+	num_dwords = CACHELINE_BYTES / sizeof(uint32_t) - num_dwords;
 	ret = intel_ring_begin(ring, num_dwords);
 	if (ret)
 		return ret;
@@ -2041,7 +2049,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 	ring->size = size;
 	ring->effective_size = ring->size;
 	if (IS_I830(ring->dev) || IS_845G(ring->dev))
-		ring->effective_size -= 128;
+		ring->effective_size -= 2 * CACHELINE_BYTES;
 
 	ring->virtual_start = ioremap_wc(start, size);
 	if (ring->virtual_start == NULL) {
-- 
1.9.1

[PATCH 2/6] drm/i915: Preserve ring buffers objects across resume

[Chris Wilson]

Tearing down the ring buffers across resume is overkill, risks
unnecessary failure and increases fragmentation.

After failure, since the device is still active we may end up trying to
write into the dangling iomapping and trigger an oops.

v2: stop_ringbuffers() was meant to call stop(ring) not
cleanup(ring) during resume!

Reported-by: Jae-hyeon Park <jhyeon@gmail.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=72351
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c         |  20 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 168 +++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |   1 +
 3 files changed, 105 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6370a761d137..89e736c00ddc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2383,6 +2383,11 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
 
 		i915_gem_free_request(request);
 	}
+
+	/* These may not have been flush before the reset, do so now */
+	kfree(ring->preallocated_lazy_request);
+	ring->preallocated_lazy_request = NULL;
+	ring->outstanding_lazy_seqno = 0;
 }
 
 void i915_gem_restore_fences(struct drm_device *dev)
@@ -2423,8 +2428,6 @@ void i915_gem_reset(struct drm_device *dev)
 	for_each_ring(ring, dev_priv, i)
 		i915_gem_reset_ring_cleanup(dev_priv, ring);
 
-	i915_gem_cleanup_ringbuffer(dev);
-
 	i915_gem_context_reset(dev);
 
 	i915_gem_restore_fences(dev);
@@ -4232,6 +4235,17 @@ void i915_gem_vma_destroy(struct i915_vma *vma)
 	kfree(vma);
 }
 
+static void
+i915_gem_stop_ringbuffers(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_ring_buffer *ring;
+	int i;
+
+	for_each_ring(ring, dev_priv, i)
+		intel_stop_ring_buffer(ring);
+}
+
 int
 i915_gem_suspend(struct drm_device *dev)
 {
@@ -4253,7 +4267,7 @@ i915_gem_suspend(struct drm_device *dev)
 		i915_gem_evict_everything(dev);
 
 	i915_kernel_lost_context(dev);
-	i915_gem_cleanup_ringbuffer(dev);
+	i915_gem_stop_ringbuffers(dev);
 
 	/* Hack!  Don't let anybody do execbuf while we don't control the chip.
 	 * We need to replace this with a semaphore, or something.
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9bed8764a62a..cce09f5a4426 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1301,45 +1301,39 @@ static void cleanup_status_page(struct intel_ring_buffer *ring)
 
 static int init_status_page(struct intel_ring_buffer *ring)
 {
-	struct drm_device *dev = ring->dev;
 	struct drm_i915_gem_object *obj;
-	int ret;
 
-	obj = i915_gem_alloc_object(dev, 4096);
-	if (obj == NULL) {
-		DRM_ERROR("Failed to allocate status page\n");
-		ret = -ENOMEM;
-		goto err;
-	}
+	if ((obj = ring->status_page.obj) == NULL) {
+		int ret;
 
-	ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
-	if (ret)
-		goto err_unref;
+		obj = i915_gem_alloc_object(ring->dev, 4096);
+		if (obj == NULL) {
+			DRM_ERROR("Failed to allocate status page\n");
+			return -ENOMEM;
+		}
 
-	ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
-	if (ret)
-		goto err_unref;
+		ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
+		if (ret)
+			goto err_unref;
+
+		ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
+		if (ret) {
+err_unref:
+			drm_gem_object_unreference(&obj->base);
+			return ret;
+		}
+
+		ring->status_page.obj = obj;
+	}
 
 	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(obj);
 	ring->status_page.page_addr = kmap(sg_page(obj->pages->sgl));
-	if (ring->status_page.page_addr == NULL) {
-		ret = -ENOMEM;
-		goto err_unpin;
-	}
-	ring->status_page.obj = obj;
 	memset(ring->status_page.page_addr, 0, PAGE_SIZE);
 
 	DRM_DEBUG_DRIVER("%s hws offset: 0x%08x\n",
 			ring->name, ring->status_page.gfx_addr);
 
 	return 0;
-
-err_unpin:
-	i915_gem_object_ggtt_unpin(obj);
-err_unref:
-	drm_gem_object_unreference(&obj->base);
-err:
-	return ret;
 }
 
 static int init_phys_status_page(struct intel_ring_buffer *ring)
@@ -1359,44 +1353,23 @@ static int init_phys_status_page(struct intel_ring_buffer *ring)
 	return 0;
 }
 
-static int intel_init_ring_buffer(struct drm_device *dev,
-				  struct intel_ring_buffer *ring)
+static int allocate_ring_buffer(struct intel_ring_buffer *ring)
 {
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_i915_gem_object *obj;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret;
 
-	ring->dev = dev;
-	INIT_LIST_HEAD(&ring->active_list);
-	INIT_LIST_HEAD(&ring->request_list);
-	ring->size = 32 * PAGE_SIZE;
-	memset(ring->sync_seqno, 0, sizeof(ring->sync_seqno));
-
-	init_waitqueue_head(&ring->irq_queue);
-
-	if (I915_NEED_GFX_HWS(dev)) {
-		ret = init_status_page(ring);
-		if (ret)
-			return ret;
-	} else {
-		BUG_ON(ring->id != RCS);
-		ret = init_phys_status_page(ring);
-		if (ret)
-			return ret;
-	}
+	if (ring->obj)
+		return 0;
 
 	obj = NULL;
 	if (!HAS_LLC(dev))
 		obj = i915_gem_object_create_stolen(dev, ring->size);
 	if (obj == NULL)
 		obj = i915_gem_alloc_object(dev, ring->size);
-	if (obj == NULL) {
-		DRM_ERROR("Failed to allocate ringbuffer\n");
-		ret = -ENOMEM;
-		goto err_hws;
-	}
-
-	ring->obj = obj;
+	if (obj == NULL)
+		return -ENOMEM;
 
 	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
 	if (ret)
@@ -1410,55 +1383,72 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 		ioremap_wc(dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj),
 			   ring->size);
 	if (ring->virtual_start == NULL) {
-		DRM_ERROR("Failed to map ringbuffer.\n");
 		ret = -EINVAL;
 		goto err_unpin;
 	}
 
-	ret = ring->init(ring);
-	if (ret)
-		goto err_unmap;
+	ring->obj = obj;
+	return 0;
+
+err_unpin:
+	i915_gem_object_ggtt_unpin(obj);
+err_unref:
+	drm_gem_object_unreference(&obj->base);
+	return ret;
+}
+
+static int intel_init_ring_buffer(struct drm_device *dev,
+				  struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	ring->dev = dev;
+	INIT_LIST_HEAD(&ring->active_list);
+	INIT_LIST_HEAD(&ring->request_list);
+	ring->size = 32 * PAGE_SIZE;
+	memset(ring->sync_seqno, 0, sizeof(ring->sync_seqno));
+
+	init_waitqueue_head(&ring->irq_queue);
+
+	if (I915_NEED_GFX_HWS(dev)) {
+		ret = init_status_page(ring);
+		if (ret)
+			return ret;
+	} else {
+		BUG_ON(ring->id != RCS);
+		ret = init_phys_status_page(ring);
+		if (ret)
+			return ret;
+	}
+
+	ret = allocate_ring_buffer(ring);
+	if (ret) {
+		DRM_ERROR("Failed to allocate ringbuffer %s: %d\n", ring->name, ret);
+		return ret;
+	}
 
 	/* Workaround an erratum on the i830 which causes a hang if
 	 * the TAIL pointer points to within the last 2 cachelines
 	 * of the buffer.
 	 */
 	ring->effective_size = ring->size;
-	if (IS_I830(ring->dev) || IS_845G(ring->dev))
+	if (IS_I830(dev) || IS_845G(dev))
 		ring->effective_size -= 2 * CACHELINE_BYTES;
 
 	i915_cmd_parser_init_ring(ring);
 
-	return 0;
-
-err_unmap:
-	iounmap(ring->virtual_start);
-err_unpin:
-	i915_gem_object_ggtt_unpin(obj);
-err_unref:
-	drm_gem_object_unreference(&obj->base);
-	ring->obj = NULL;
-err_hws:
-	cleanup_status_page(ring);
-	return ret;
+	return ring->init(ring);
 }
 
 void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring)
 {
-	struct drm_i915_private *dev_priv;
-	int ret;
+	struct drm_i915_private *dev_priv = to_i915(ring->dev);
 
 	if (ring->obj == NULL)
 		return;
 
-	/* Disable the ring buffer. The ring must be idle at this point */
-	dev_priv = ring->dev->dev_private;
-	ret = intel_ring_idle(ring);
-	if (ret && !i915_reset_in_progress(&dev_priv->gpu_error))
-		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
-			  ring->name, ret);
-
-	I915_WRITE_CTL(ring, 0);
+	intel_stop_ring_buffer(ring);
+	WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
 
 	iounmap(ring->virtual_start);
 
@@ -2248,3 +2238,19 @@ intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring)
 	ring->gpu_caches_dirty = false;
 	return 0;
 }
+
+void
+intel_stop_ring_buffer(struct intel_ring_buffer *ring)
+{
+	int ret;
+
+	if (ring->obj == NULL)
+		return;
+
+	ret = intel_ring_idle(ring);
+	if (ret && !i915_reset_in_progress(&to_i915(ring->dev)->gpu_error))
+		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
+			  ring->name, ret);
+
+	stop_ring(ring);
+}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 413cdc74ed53..54839165eb6d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -263,6 +263,7 @@ intel_write_status_page(struct intel_ring_buffer *ring,
 #define I915_GEM_HWS_SCRATCH_INDEX	0x30
 #define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX << MI_STORE_DWORD_INDEX_SHIFT)
 
+void intel_stop_ring_buffer(struct intel_ring_buffer *ring);
 void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring);
 
 int __must_check intel_ring_begin(struct intel_ring_buffer *ring, int n);
-- 
1.9.1

[PATCH 3/6] drm/i915: Allow the module to load even if we fail to setup rings

[Chris Wilson]

Even without enabling the ringbuffers to allow command execution, we can
still control the display engines to enable modesetting. So make the
ringbuffer initialization failure soft, and mark the GPU as wedged
instead.

v2: Only treat an EIO from ring initialisation as a soft failure, and
abort module load for any other failure, such as allocation failures.

v3: Add an *ERROR* prior to declaring the GPU wedged so that it stands
out like a sore thumb in the logs

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89e736c00ddc..adc891eb588e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4451,15 +4451,11 @@ i915_gem_init_hw(struct drm_device *dev)
 	 * the do_switch), but before enabling PPGTT. So don't move this.
 	 */
 	ret = i915_gem_context_enable(dev_priv);
-	if (ret) {
+	if (ret && ret != -EIO) {
 		DRM_ERROR("Context enable failed %d\n", ret);
-		goto err_out;
+		i915_gem_cleanup_ringbuffer(dev);
 	}
 
-	return 0;
-
-err_out:
-	i915_gem_cleanup_ringbuffer(dev);
 	return ret;
 }
 
@@ -4486,18 +4482,21 @@ int i915_gem_init(struct drm_device *dev)
 	}
 
 	ret = i915_gem_init_hw(dev);
-	mutex_unlock(&dev->struct_mutex);
-	if (ret) {
-		WARN_ON(dev_priv->mm.aliasing_ppgtt);
-		i915_gem_context_fini(dev);
-		drm_mm_takedown(&dev_priv->gtt.base.mm);
-		return ret;
+	if (ret == -EIO) {
+		/* Allow ring initialisation to fail by marking the GPU as
+		 * wedged. But we only want to do this where the GPU is angry,
+		 * for all other failure, such as an allocation failure, bail.
+		 */
+		DRM_ERROR("Failed to initialize GPU, declaring it wedged\n");
+		atomic_set_mask(I915_WEDGED, &dev_priv->gpu_error.reset_counter);
+		ret = 0;
 	}
+	mutex_unlock(&dev->struct_mutex);
 
 	/* Allow hardware batchbuffers unless told otherwise, but not for KMS. */
 	if (!drm_core_check_feature(dev, DRIVER_MODESET))
 		dev_priv->dri1.allow_batchbuffer = 1;
-	return 0;
+	return ret;
 }
 
 void
-- 
1.9.1

[PATCH 4/6] drm/i915: Mark device as wedged if we fail to resume

[Chris Wilson]

During module load, if we fail to initialise the rings, we abort the
load reporting EIO. However during resume, even though we report EIO as
we fail to reinitialize the ringbuffers, the resume continues and the
device is restored - albeit in a non-functional state. As we cannot
execute any commands on the GPU, it is effectively wedged, mark it so.

As we now preserve the ringbuffers across resume, this should prevent
UXA from falling into the trap of repeatedly sending invalid
batchbuffers and dropping all further rendering into /dev/null.

Reported-and-tested-by: Jiri Kosina <jkosina@suse.cz>
References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5d8250f7145d..629f8164a547 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -569,8 +569,10 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 		drm_mode_config_reset(dev);
 
 		mutex_lock(&dev->struct_mutex);
-
-		error = i915_gem_init_hw(dev);
+		if (i915_gem_init_hw(dev)) {
+			DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n");
+			atomic_set_mask(I915_WEDGED, &dev_priv->gpu_error.reset_counter);
+		}
 		mutex_unlock(&dev->struct_mutex);
 
 		/* We need working interrupts for modeset enabling ... */
@@ -613,7 +615,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 	mutex_unlock(&dev_priv->modeset_restore_lock);
 
 	intel_runtime_pm_put(dev_priv);
-	return error;
+	return 0;
 }
 
 static int i915_drm_thaw(struct drm_device *dev)
-- 
1.9.1

[PATCH 5/6] drm/i915: Include a little more information about why ring init fails

[Chris Wilson]

If we include the expected values for the failing ring register checks,
it makes it marginally easier to see which is the culprit.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cce09f5a4426..a8c73a0d935d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -519,12 +519,11 @@ static int init_ring_common(struct intel_ring_buffer *ring)
 		     I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj) &&
 		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
 		DRM_ERROR("%s initialization failed "
-				"ctl %08x head %08x tail %08x start %08x\n",
-				ring->name,
-				I915_READ_CTL(ring),
-				I915_READ_HEAD(ring),
-				I915_READ_TAIL(ring),
-				I915_READ_START(ring));
+			  "ctl %08x (valid? %d) head %08x tail %08x start %08x [expected %08lx]\n",
+			  ring->name,
+			  I915_READ_CTL(ring), I915_READ_CTL(ring) & RING_VALID,
+			  I915_READ_HEAD(ring), I915_READ_TAIL(ring),
+			  I915_READ_START(ring), (unsigned long)i915_gem_obj_ggtt_offset(obj));
 		ret = -EIO;
 		goto out;
 	}
-- 
1.9.1

[PATCH 6/6] drm/i915: Kick start the rings

[Chris Wilson]

On g4x, we have an issue where the register write to setup the rings do
not always take. However, it appears that the current check also passes
only by chance, a second reading of the register returns a different
broekn value - but the GPU appears to function. Based on that
observation, lets try feeding a nop into the ring and seeing if it
advances as part of our startup sanity checks.

References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a8c73a0d935d..bc52645fa8d5 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -514,10 +514,14 @@ static int init_ring_common(struct intel_ring_buffer *ring)
 			((ring->size - PAGE_SIZE) & RING_NR_PAGES)
 			| RING_VALID);
 
+	iowrite32(MI_NOOP, ring->virtual_start + 0);
+	iowrite32(MI_NOOP, ring->virtual_start + 4);
+	ring->write_tail(ring, 8);
+
 	/* If the head is still not zero, the ring is dead */
 	if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 &&
 		     I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj) &&
-		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
+		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 8, 50)) {
 		DRM_ERROR("%s initialization failed "
 			  "ctl %08x (valid? %d) head %08x tail %08x start %08x [expected %08lx]\n",
 			  ring->name,
-- 
1.9.1

Re: [PATCH 1/6] drm/i915: Replace hardcoded cacheline size with macro

[Mateo Lozano, Oscar]

> Subject: [Intel-gfx] [PATCH 1/6] drm/i915: Replace hardcoded cacheline size
> with macro
> 
> For readibility and guess at the meaning behind the constants.
> 
> v2: Claim only the meagerest connections with reality.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

Re: [PATCH 2/6] drm/i915: Preserve ring buffers objects across resume

[Mateo Lozano, Oscar]

> Subject: [Intel-gfx] [PATCH 2/6] drm/i915: Preserve ring buffers objects across
> resume
> 
> Tearing down the ring buffers across resume is overkill, risks unnecessary
> failure and increases fragmentation.
> 
> After failure, since the device is still active we may end up trying to write into
> the dangling iomapping and trigger an oops.
> 
> v2: stop_ringbuffers() was meant to call stop(ring) not
> cleanup(ring) during resume!
> 
> Reported-by: Jae-hyeon Park <jhyeon@gmail.com>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=72351
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         |  20 +++-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 168 +++++++++++++++++--------------
> -
>  drivers/gpu/drm/i915/intel_ringbuffer.h |   1 +
>  3 files changed, 105 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c index 6370a761d137..89e736c00ddc
> 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2383,6 +2383,11 @@ static void i915_gem_reset_ring_cleanup(struct
> drm_i915_private *dev_priv,
> 
>  		i915_gem_free_request(request);
>  	}
> +
> +	/* These may not have been flush before the reset, do so now */
> +	kfree(ring->preallocated_lazy_request);
> +	ring->preallocated_lazy_request = NULL;
> +	ring->outstanding_lazy_seqno = 0;
>  }
> 
>  void i915_gem_restore_fences(struct drm_device *dev) @@ -2423,8 +2428,6
> @@ void i915_gem_reset(struct drm_device *dev)
>  	for_each_ring(ring, dev_priv, i)
>  		i915_gem_reset_ring_cleanup(dev_priv, ring);
> 
> -	i915_gem_cleanup_ringbuffer(dev);
> -
>  	i915_gem_context_reset(dev);
> 
>  	i915_gem_restore_fences(dev);
> @@ -4232,6 +4235,17 @@ void i915_gem_vma_destroy(struct i915_vma
> *vma)
>  	kfree(vma);
>  }
> 
> +static void
> +i915_gem_stop_ringbuffers(struct drm_device *dev) {
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_ring_buffer *ring;
> +	int i;
> +
> +	for_each_ring(ring, dev_priv, i)
> +		intel_stop_ring_buffer(ring);
> +}
> +
>  int
>  i915_gem_suspend(struct drm_device *dev)  { @@ -4253,7 +4267,7 @@
> i915_gem_suspend(struct drm_device *dev)
>  		i915_gem_evict_everything(dev);
> 
>  	i915_kernel_lost_context(dev);
> -	i915_gem_cleanup_ringbuffer(dev);
> +	i915_gem_stop_ringbuffers(dev);
> 
>  	/* Hack!  Don't let anybody do execbuf while we don't control the chip.
>  	 * We need to replace this with a semaphore, or something.
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 9bed8764a62a..cce09f5a4426 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1301,45 +1301,39 @@ static void cleanup_status_page(struct
> intel_ring_buffer *ring)
> 
>  static int init_status_page(struct intel_ring_buffer *ring)  {
> -	struct drm_device *dev = ring->dev;
>  	struct drm_i915_gem_object *obj;
> -	int ret;
> 
> -	obj = i915_gem_alloc_object(dev, 4096);
> -	if (obj == NULL) {
> -		DRM_ERROR("Failed to allocate status page\n");
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> +	if ((obj = ring->status_page.obj) == NULL) {
> +		int ret;
> 
> -	ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
> -	if (ret)
> -		goto err_unref;
> +		obj = i915_gem_alloc_object(ring->dev, 4096);
> +		if (obj == NULL) {
> +			DRM_ERROR("Failed to allocate status page\n");
> +			return -ENOMEM;
> +		}
> 
> -	ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
> -	if (ret)
> -		goto err_unref;
> +		ret = i915_gem_object_set_cache_level(obj,
> I915_CACHE_LLC);
> +		if (ret)
> +			goto err_unref;
> +
> +		ret = i915_gem_obj_ggtt_pin(obj, 4096, 0);
> +		if (ret) {
> +err_unref:
> +			drm_gem_object_unreference(&obj->base);
> +			return ret;
> +		}
> +
> +		ring->status_page.obj = obj;
> +	}
> 
>  	ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(obj);
>  	ring->status_page.page_addr = kmap(sg_page(obj->pages->sgl));
> -	if (ring->status_page.page_addr == NULL) {
> -		ret = -ENOMEM;
> -		goto err_unpin;
> -	}
> -	ring->status_page.obj = obj;
>  	memset(ring->status_page.page_addr, 0, PAGE_SIZE);
> 
>  	DRM_DEBUG_DRIVER("%s hws offset: 0x%08x\n",
>  			ring->name, ring->status_page.gfx_addr);
> 
>  	return 0;
> -
> -err_unpin:
> -	i915_gem_object_ggtt_unpin(obj);
> -err_unref:
> -	drm_gem_object_unreference(&obj->base);
> -err:
> -	return ret;
>  }
> 
>  static int init_phys_status_page(struct intel_ring_buffer *ring) @@ -1359,44
> +1353,23 @@ static int init_phys_status_page(struct intel_ring_buffer *ring)
>  	return 0;
>  }
> 
> -static int intel_init_ring_buffer(struct drm_device *dev,
> -				  struct intel_ring_buffer *ring)
> +static int allocate_ring_buffer(struct intel_ring_buffer *ring)
>  {
> +	struct drm_device *dev = ring->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_i915_gem_object *obj;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int ret;
> 
> -	ring->dev = dev;
> -	INIT_LIST_HEAD(&ring->active_list);
> -	INIT_LIST_HEAD(&ring->request_list);
> -	ring->size = 32 * PAGE_SIZE;
> -	memset(ring->sync_seqno, 0, sizeof(ring->sync_seqno));
> -
> -	init_waitqueue_head(&ring->irq_queue);
> -
> -	if (I915_NEED_GFX_HWS(dev)) {
> -		ret = init_status_page(ring);
> -		if (ret)
> -			return ret;
> -	} else {
> -		BUG_ON(ring->id != RCS);
> -		ret = init_phys_status_page(ring);
> -		if (ret)
> -			return ret;
> -	}
> +	if (ring->obj)
> +		return 0;
> 
>  	obj = NULL;
>  	if (!HAS_LLC(dev))
>  		obj = i915_gem_object_create_stolen(dev, ring->size);
>  	if (obj == NULL)
>  		obj = i915_gem_alloc_object(dev, ring->size);
> -	if (obj == NULL) {
> -		DRM_ERROR("Failed to allocate ringbuffer\n");
> -		ret = -ENOMEM;
> -		goto err_hws;
> -	}
> -
> -	ring->obj = obj;
> +	if (obj == NULL)
> +		return -ENOMEM;
> 
>  	ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE);
>  	if (ret)
> @@ -1410,55 +1383,72 @@ static int intel_init_ring_buffer(struct drm_device
> *dev,
>  		ioremap_wc(dev_priv->gtt.mappable_base +
> i915_gem_obj_ggtt_offset(obj),
>  			   ring->size);
>  	if (ring->virtual_start == NULL) {
> -		DRM_ERROR("Failed to map ringbuffer.\n");
>  		ret = -EINVAL;
>  		goto err_unpin;
>  	}
> 
> -	ret = ring->init(ring);
> -	if (ret)
> -		goto err_unmap;
> +	ring->obj = obj;
> +	return 0;
> +
> +err_unpin:
> +	i915_gem_object_ggtt_unpin(obj);
> +err_unref:
> +	drm_gem_object_unreference(&obj->base);
> +	return ret;
> +}
> +
> +static int intel_init_ring_buffer(struct drm_device *dev,
> +				  struct intel_ring_buffer *ring)
> +{
> +	int ret;
> +
> +	ring->dev = dev;
> +	INIT_LIST_HEAD(&ring->active_list);
> +	INIT_LIST_HEAD(&ring->request_list);
> +	ring->size = 32 * PAGE_SIZE;
> +	memset(ring->sync_seqno, 0, sizeof(ring->sync_seqno));
> +
> +	init_waitqueue_head(&ring->irq_queue);
> +
> +	if (I915_NEED_GFX_HWS(dev)) {
> +		ret = init_status_page(ring);
> +		if (ret)
> +			return ret;
> +	} else {
> +		BUG_ON(ring->id != RCS);
> +		ret = init_phys_status_page(ring);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = allocate_ring_buffer(ring);
> +	if (ret) {
> +		DRM_ERROR("Failed to allocate ringbuffer %s: %d\n", ring-
> >name, ret);
> +		return ret;
> +	}
> 
>  	/* Workaround an erratum on the i830 which causes a hang if
>  	 * the TAIL pointer points to within the last 2 cachelines
>  	 * of the buffer.
>  	 */
>  	ring->effective_size = ring->size;
> -	if (IS_I830(ring->dev) || IS_845G(ring->dev))
> +	if (IS_I830(dev) || IS_845G(dev))
>  		ring->effective_size -= 2 * CACHELINE_BYTES;
> 
>  	i915_cmd_parser_init_ring(ring);
> 
> -	return 0;
> -
> -err_unmap:
> -	iounmap(ring->virtual_start);
> -err_unpin:
> -	i915_gem_object_ggtt_unpin(obj);
> -err_unref:
> -	drm_gem_object_unreference(&obj->base);
> -	ring->obj = NULL;
> -err_hws:
> -	cleanup_status_page(ring);
> -	return ret;
> +	return ring->init(ring);
>  }
> 
>  void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring)  {
> -	struct drm_i915_private *dev_priv;
> -	int ret;
> +	struct drm_i915_private *dev_priv = to_i915(ring->dev);
> 
>  	if (ring->obj == NULL)
>  		return;
> 
> -	/* Disable the ring buffer. The ring must be idle at this point */
> -	dev_priv = ring->dev->dev_private;
> -	ret = intel_ring_idle(ring);
> -	if (ret && !i915_reset_in_progress(&dev_priv->gpu_error))
> -		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
> -			  ring->name, ret);
> -
> -	I915_WRITE_CTL(ring, 0);
> +	intel_stop_ring_buffer(ring);
> +	WARN_ON((I915_READ_MODE(ring) & MODE_IDLE) == 0);
> 
>  	iounmap(ring->virtual_start);
> 
> @@ -2248,3 +2238,19 @@ intel_ring_invalidate_all_caches(struct
> intel_ring_buffer *ring)
>  	ring->gpu_caches_dirty = false;
>  	return 0;
>  }
> +
> +void
> +intel_stop_ring_buffer(struct intel_ring_buffer *ring) {
> +	int ret;
> +
> +	if (ring->obj == NULL)
> +		return;

It´s probably a good idea to use the intel_ring_initialized() macro.

Other than that:
Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

> +
> +	ret = intel_ring_idle(ring);
> +	if (ret && !i915_reset_in_progress(&to_i915(ring->dev)->gpu_error))
> +		DRM_ERROR("failed to quiesce %s whilst cleaning up: %d\n",
> +			  ring->name, ret);
> +
> +	stop_ring(ring);
> +}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 413cdc74ed53..54839165eb6d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -263,6 +263,7 @@ intel_write_status_page(struct intel_ring_buffer *ring,
>  #define I915_GEM_HWS_SCRATCH_INDEX	0x30
>  #define I915_GEM_HWS_SCRATCH_ADDR (I915_GEM_HWS_SCRATCH_INDEX
> << MI_STORE_DWORD_INDEX_SHIFT)
> 
> +void intel_stop_ring_buffer(struct intel_ring_buffer *ring);
>  void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring);
> 
>  int __must_check intel_ring_begin(struct intel_ring_buffer *ring, int n);
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/6] drm/i915: Allow the module to load even if we fail to setup rings

[Mateo Lozano, Oscar]

> Subject: [Intel-gfx] [PATCH 3/6] drm/i915: Allow the module to load even if we
> fail to setup rings
> 
> Even without enabling the ringbuffers to allow command execution, we can still
> control the display engines to enable modesetting. So make the ringbuffer
> initialization failure soft, and mark the GPU as wedged instead.
> 
> v2: Only treat an EIO from ring initialisation as a soft failure, and abort module
> load for any other failure, such as allocation failures.
> 
> v3: Add an *ERROR* prior to declaring the GPU wedged so that it stands out
> like a sore thumb in the logs
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

Re: [PATCH 4/6] drm/i915: Mark device as wedged if we fail to resume

[Mateo Lozano, Oscar]

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> Chris Wilson
> Sent: Wednesday, April 09, 2014 9:20 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 4/6] drm/i915: Mark device as wedged if we fail to
> resume
> 
> During module load, if we fail to initialise the rings, we abort the load reporting
> EIO. However during resume, even though we report EIO as we fail to
> reinitialize the ringbuffers, the resume continues and the device is restored -
> albeit in a non-functional state. As we cannot execute any commands on the
> GPU, it is effectively wedged, mark it so.
> 
> As we now preserve the ringbuffers across resume, this should prevent UXA
> from falling into the trap of repeatedly sending invalid batchbuffers and
> dropping all further rendering into /dev/null.
> 
> Reported-and-tested-by: Jiri Kosina <jkosina@suse.cz>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index 5d8250f7145d..629f8164a547 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -569,8 +569,10 @@ static int __i915_drm_thaw(struct drm_device *dev,
> bool restore_gtt_mappings)
>  		drm_mode_config_reset(dev);
> 
>  		mutex_lock(&dev->struct_mutex);
> -
> -		error = i915_gem_init_hw(dev);
> +		if (i915_gem_init_hw(dev)) {
> +			DRM_ERROR("failed to re-initialize GPU, declaring
> wedged!\n");

The "int error = 0;" is not needed anymore. Other than that:

Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

> +			atomic_set_mask(I915_WEDGED, &dev_priv-
> >gpu_error.reset_counter);
> +		}
>  		mutex_unlock(&dev->struct_mutex);
> 
>  		/* We need working interrupts for modeset enabling ... */ @@
> -613,7 +615,7 @@ static int __i915_drm_thaw(struct drm_device *dev, bool
> restore_gtt_mappings)
>  	mutex_unlock(&dev_priv->modeset_restore_lock);
> 
>  	intel_runtime_pm_put(dev_priv);
> -	return error;
> +	return 0;
>  }
> 
>  static int i915_drm_thaw(struct drm_device *dev)
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915: Include a little more information about why ring init fails

[Mateo Lozano, Oscar]

> Subject: [Intel-gfx] [PATCH 5/6] drm/i915: Include a little more information
> about why ring init fails
> 
> If we include the expected values for the failing ring register checks, it makes it
> marginally easier to see which is the culprit.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

Re: [PATCH 6/6] drm/i915: Kick start the rings

[Mateo Lozano, Oscar]

> Subject: [Intel-gfx] [PATCH 6/6] drm/i915: Kick start the rings
> 
> On g4x, we have an issue where the register write to setup the rings do not
> always take. However, it appears that the current check also passes only by
> chance, a second reading of the register returns a different broekn value - but
> the GPU appears to function. Based on that observation, lets try feeding a nop
> into the ring and seeing if it advances as part of our startup sanity checks.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index a8c73a0d935d..bc52645fa8d5 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -514,10 +514,14 @@ static int init_ring_common(struct intel_ring_buffer
> *ring)
>  			((ring->size - PAGE_SIZE) & RING_NR_PAGES)
>  			| RING_VALID);
> 
> +	iowrite32(MI_NOOP, ring->virtual_start + 0);
> +	iowrite32(MI_NOOP, ring->virtual_start + 4);
> +	ring->write_tail(ring, 8);

Maybe add a comment in the code about why we are doing this? (otherwise, it looks a bit magical)

>  	/* If the head is still not zero, the ring is dead */
>  	if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 &&
>  		     I915_READ_START(ring) == i915_gem_obj_ggtt_offset(obj)
> &&
> -		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) {
> +		     (I915_READ_HEAD(ring) & HEAD_ADDR) == 8, 50)) {
>  		DRM_ERROR("%s initialization failed "
>  			  "ctl %08x (valid? %d) head %08x tail %08x start %08x
> [expected %08lx]\n",
>  			  ring->name,
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915: Kick start the rings

[Daniel Vetter]

On Tue, Apr 22, 2014 at 01:06:45PM +0000, Mateo Lozano, Oscar wrote:
> > Subject: [Intel-gfx] [PATCH 6/6] drm/i915: Kick start the rings
> > 
> > On g4x, we have an issue where the register write to setup the rings do not
> > always take. However, it appears that the current check also passes only by
> > chance, a second reading of the register returns a different broekn value - but
> > the GPU appears to function. Based on that observation, lets try feeding a nop
> > into the ring and seeing if it advances as part of our startup sanity checks.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index a8c73a0d935d..bc52645fa8d5 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -514,10 +514,14 @@ static int init_ring_common(struct intel_ring_buffer
> > *ring)
> >  			((ring->size - PAGE_SIZE) & RING_NR_PAGES)
> >  			| RING_VALID);
> > 
> > +	iowrite32(MI_NOOP, ring->virtual_start + 0);
> > +	iowrite32(MI_NOOP, ring->virtual_start + 4);
> > +	ring->write_tail(ring, 8);
> 
> Maybe add a comment in the code about why we are doing this? (otherwise, it looks a bit magical)

Yeah, this is a bit too much magic. Also it looks like it doesn't really
help the bug report with his gm45, so I think I'll punt on this one for
now.

All other patches merged with the 2 small suggestions from Oscar applied
while merging. Thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch