intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* A compendium of trivial patches.
@ 2010-05-27 12:18 Chris Wilson
  2010-05-27 12:18 ` [PATCH 01/11] drm/i915: Only print an message if there was an error Chris Wilson
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Hi Eric,
  just a few outstanding trivial patches for you to read on your
commute. :)
-ickle

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

* [PATCH 01/11] drm/i915: Only print an message if there was an error
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path Chris Wilson
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Only report an error if the GPU has actually detected one, otherwise we
are just hung.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_irq.c |   38 ++++++++++++++++++++++++--------------
 1 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b5dba47..2479be0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -692,24 +692,13 @@ void i915_destroy_error_state(struct drm_device *dev)
 		i915_error_state_free(dev, error);
 }
 
-/**
- * i915_handle_error - handle an error interrupt
- * @dev: drm device
- *
- * Do some basic checking of regsiter state at error interrupt time and
- * dump it to the syslog.  Also call i915_capture_error_state() to make
- * sure we get a record and make it available in debugfs.  Fire a uevent
- * so userspace knows something bad happened (should trigger collection
- * of a ring dump etc.).
- */
-static void i915_handle_error(struct drm_device *dev, bool wedged)
+static void i915_report_and_clear_eir(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 eir = I915_READ(EIR);
-	u32 pipea_stats = I915_READ(PIPEASTAT);
-	u32 pipeb_stats = I915_READ(PIPEBSTAT);
 
-	i915_capture_error_state(dev);
+	if (!eir)
+		return;
 
 	printk(KERN_ERR "render error detected, EIR: 0x%08x\n",
 	       eir);
@@ -755,6 +744,9 @@ static void i915_handle_error(struct drm_device *dev, bool wedged)
 	}
 
 	if (eir & I915_ERROR_MEMORY_REFRESH) {
+		u32 pipea_stats = I915_READ(PIPEASTAT);
+		u32 pipeb_stats = I915_READ(PIPEBSTAT);
+
 		printk(KERN_ERR "memory refresh error\n");
 		printk(KERN_ERR "PIPEASTAT: 0x%08x\n",
 		       pipea_stats);
@@ -811,6 +803,24 @@ static void i915_handle_error(struct drm_device *dev, bool wedged)
 		I915_WRITE(EMR, I915_READ(EMR) | eir);
 		I915_WRITE(IIR, I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT);
 	}
+}
+
+/**
+ * i915_handle_error - handle an error interrupt
+ * @dev: drm device
+ *
+ * Do some basic checking of regsiter state at error interrupt time and
+ * dump it to the syslog.  Also call i915_capture_error_state() to make
+ * sure we get a record and make it available in debugfs.  Fire a uevent
+ * so userspace knows something bad happened (should trigger collection
+ * of a ring dump etc.).
+ */
+static void i915_handle_error(struct drm_device *dev, bool wedged)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	i915_capture_error_state(dev);
+	i915_report_and_clear_eir(dev);
 
 	if (wedged) {
 		atomic_set(&dev_priv->mm.wedged, 1);
-- 
1.7.1

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

* [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
  2010-05-27 12:18 ` [PATCH 01/11] drm/i915: Only print an message if there was an error Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 16:03   ` Jesse Barnes
  2010-05-27 12:18 ` [PATCH 03/11] drm/i915: Avoid nesting of domain changes when setting display plane Chris Wilson
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Delay taking the mutex until we need to and ensure that we hold the
spinlock when resetting unpin_work on the error path. Also defer the
debugging print messages until after we have released the spinlock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Kristian Høgsberg <krh@bitplanet.net>
---
 drivers/gpu/drm/i915/intel_display.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cfac4dd..1845a06 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4667,8 +4667,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (work == NULL)
 		return -ENOMEM;
 
-	mutex_lock(&dev->struct_mutex);
-
 	work->event = event;
 	work->dev = crtc->dev;
 	intel_fb = to_intel_framebuffer(crtc->fb);
@@ -4678,10 +4676,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	/* We borrow the event spin lock for protecting unpin_work */
 	spin_lock_irqsave(&dev->event_lock, flags);
 	if (intel_crtc->unpin_work) {
-		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		kfree(work);
-		mutex_unlock(&dev->struct_mutex);
+
+		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
 		return -EBUSY;
 	}
 	intel_crtc->unpin_work = work;
@@ -4690,13 +4688,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	intel_fb = to_intel_framebuffer(fb);
 	obj = intel_fb->obj;
 
+	mutex_lock(&dev->struct_mutex);
 	ret = intel_pin_and_fence_fb_obj(dev, obj);
 	if (ret != 0) {
-		DRM_DEBUG_DRIVER("flip queue: %p pin & fence failed\n",
-			  to_intel_bo(obj));
-		kfree(work);
-		intel_crtc->unpin_work = NULL;
 		mutex_unlock(&dev->struct_mutex);
+
+		spin_lock_irqsave(&dev->event_lock, flags);
+		intel_crtc->unpin_work = NULL;
+		spin_unlock_irqrestore(&dev->event_lock, flags);
+
+		kfree(work);
+
+		DRM_DEBUG_DRIVER("flip queue: %p pin & fence failed\n",
+				 to_intel_bo(obj));
 		return ret;
 	}
 
-- 
1.7.1

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

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

* [PATCH 03/11] drm/i915: Avoid nesting of domain changes when setting display plane
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
  2010-05-27 12:18 ` [PATCH 01/11] drm/i915: Only print an message if there was an error Chris Wilson
  2010-05-27 12:18 ` [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object Chris Wilson
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Nesting domain changes will cause confusion when trying to interpret the
tracepoints describing the sequence of changes for the object, as well
as obscuring the order of operations for the reader of the code.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index bf70355..1c65f0b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2914,18 +2914,16 @@ i915_gem_object_set_to_display_plane(struct drm_gem_object *obj)
 			return ret;
 	}
 
+	i915_gem_object_flush_cpu_write_domain(obj);
+
 	old_write_domain = obj->write_domain;
 	old_read_domains = obj->read_domains;
 
-	obj->read_domains &= I915_GEM_DOMAIN_GTT;
-
-	i915_gem_object_flush_cpu_write_domain(obj);
-
 	/* It should now be out of any other write domains, and we can update
 	 * the domain values for our changes.
 	 */
 	BUG_ON((obj->write_domain & ~I915_GEM_DOMAIN_GTT) != 0);
-	obj->read_domains |= I915_GEM_DOMAIN_GTT;
+	obj->read_domains = I915_GEM_DOMAIN_GTT;
 	obj->write_domain = I915_GEM_DOMAIN_GTT;
 	obj_priv->dirty = 1;
 
-- 
1.7.1

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

* [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object.
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (2 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 03/11] drm/i915: Avoid nesting of domain changes when setting display plane Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 16:05   ` Jesse Barnes
  2010-05-27 12:18 ` [PATCH 05/11] drm/i915: Only print "nothing to do" debug message as required Chris Wilson
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/i915_gem.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1c65f0b..6425c2a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3307,9 +3307,13 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
 	             obj_priv->tiling_mode != I915_TILING_NONE;
 
 	/* Check fence reg constraints and rebind if necessary */
-	if (need_fence && !i915_gem_object_fence_offset_ok(obj,
-	    obj_priv->tiling_mode))
-		i915_gem_object_unbind(obj);
+	if (need_fence &&
+	    !i915_gem_object_fence_offset_ok(obj,
+					     obj_priv->tiling_mode)) {
+		ret = i915_gem_object_unbind(obj);
+		if (ret)
+			return ret;
+	}
 
 	/* Choose the GTT offset for our buffer and put it there. */
 	ret = i915_gem_object_pin(obj, (uint32_t) entry->alignment);
-- 
1.7.1

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

* [PATCH 05/11] drm/i915: Only print "nothing to do" debug message as required.
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (3 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 06/11] drm/i915: Include pitch in set_base debug statement Chris Wilson
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

If the FBC is already disabled, then we do not even attempt to disable
FBC and so there is no point emitting a debug statement at that point,
having already emitted one saying why we are disabling FBC.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1845a06..e504fdb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1248,10 +1248,11 @@ static void intel_update_fbc(struct drm_crtc *crtc,
 	return;
 
 out_disable:
-	DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
 	/* Multiple disables should be harmless */
-	if (intel_fbc_enabled(dev))
+	if (intel_fbc_enabled(dev)) {
+		DRM_DEBUG_KMS("unsupported config, disabling FBC\n");
 		intel_disable_fbc(dev);
+	}
 }
 
 static int
-- 
1.7.1

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

* [PATCH 06/11] drm/i915: Include pitch in set_base debug statement.
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (4 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 05/11] drm/i915: Only print "nothing to do" debug message as required Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 07/11] drm/i915: Rebind bo if currently bound with incorrect alignment Chris Wilson
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

Add the pitch that we about to write into the control register along
with the base, offset and coordinates that go into the other control
registers.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_display.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e504fdb..88a1ab7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1396,7 +1396,8 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	Start = obj_priv->gtt_offset;
 	Offset = y * crtc->fb->pitch + x * (crtc->fb->bits_per_pixel / 8);
 
-	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d\n", Start, Offset, x, y);
+	DRM_DEBUG_KMS("Writing base %08lX %08lX %d %d %d\n",
+		      Start, Offset, x, y, crtc->fb->pitch);
 	I915_WRITE(dspstride, crtc->fb->pitch);
 	if (IS_I965G(dev)) {
 		I915_WRITE(dspbase, Offset);
-- 
1.7.1

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

* [PATCH 07/11] drm/i915: Rebind bo if currently bound with incorrect alignment.
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (5 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 06/11] drm/i915: Include pitch in set_base debug statement Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 08/11] drm/i915: Remove spurious warning "Failure to install fence" Chris Wilson
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

Whilst pinning the buffer, check that that its current alignment
matches the requested alignment. If it does not, rebind.

This should clear up any final render errors whilst resuming,
for reference:

  Bug 27070 - [i915] Page table errors with empty ringbuffer
  https://bugs.freedesktop.org/show_bug.cgi?id=27070

  Bug 15502 -  render error detected, EIR: 0x00000010
  https://bugzilla.kernel.org/show_bug.cgi?id=15502

  Bug 13844 -  i915 error: "render error detected"
  https://bugzilla.kernel.org/show_bug.cgi?id=13844

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6425c2a..a5ca959 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4164,6 +4164,17 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment)
 	BUG_ON(obj_priv->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT);
 
 	i915_verify_inactive(dev, __FILE__, __LINE__);
+
+	if (obj_priv->gtt_space != NULL) {
+		if (alignment == 0)
+			alignment = i915_gem_get_gtt_alignment(obj);
+		if (obj_priv->gtt_offset & (alignment - 1)) {
+			ret = i915_gem_object_unbind(obj);
+			if (ret)
+				return ret;
+		}
+	}
+
 	if (obj_priv->gtt_space == NULL) {
 		ret = i915_gem_object_bind_to_gtt(obj, alignment);
 		if (ret)
-- 
1.7.1

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

* [PATCH 08/11] drm/i915: Remove spurious warning "Failure to install fence"
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (6 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 07/11] drm/i915: Rebind bo if currently bound with incorrect alignment Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 09/11] drm/i915: Check error code whilst moving buffer to GTT domain Chris Wilson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

This particular warning is harmless as we emit during the normal
pinning process where the batch buffer requires more fences than is
available without eviction. Only if we fail to evict enough fences does
this become a problem, so include the requested number of fences in the
ultimate *error* message.

v2: Remember to compile test even trial patches to remove warnings.

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

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a5ca959..b87945d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3327,9 +3327,6 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
 	if (need_fence) {
 		ret = i915_gem_object_get_fence_reg(obj);
 		if (ret != 0) {
-			if (ret != -EBUSY && ret != -ERESTARTSYS)
-				DRM_ERROR("Failure to install fence: %d\n",
-					  ret);
 			i915_gem_object_unpin(obj);
 			return ret;
 		}
@@ -3815,11 +3812,19 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		if (ret != -ENOSPC || pin_tries >= 1) {
 			if (ret != -ERESTARTSYS) {
 				unsigned long long total_size = 0;
-				for (i = 0; i < args->buffer_count; i++)
+				int num_fences = 0;
+				for (i = 0; i < args->buffer_count; i++) {
+					obj_priv = object_list[i]->driver_private;
+
 					total_size += object_list[i]->size;
-				DRM_ERROR("Failed to pin buffer %d of %d, total %llu bytes: %d\n",
+					num_fences +=
+						exec_list[i].flags & EXEC_OBJECT_NEEDS_FENCE &&
+						obj_priv->tiling_mode != I915_TILING_NONE;
+				}
+				DRM_ERROR("Failed to pin buffer %d of %d, total %llu bytes, %d fences: %d\n",
 					  pinned+1, args->buffer_count,
-					  total_size, ret);
+					  total_size, num_fences,
+					  ret);
 				DRM_ERROR("%d objects [%d pinned], "
 					  "%d object bytes [%d pinned], "
 					  "%d/%d gtt bytes\n",
-- 
1.7.1

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

* [PATCH 09/11] drm/i915: Check error code whilst moving buffer to GTT domain.
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (7 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 08/11] drm/i915: Remove spurious warning "Failure to install fence" Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 10/11] drm/i915: Reject bind_to_gtt() early if object > aperture Chris Wilson
  2010-05-27 12:18 ` [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers Chris Wilson
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

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

diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index 6f53cf7..f8c76e6 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -105,7 +105,11 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
 	}
 
 	/* Flush everything out, we'll be doing GTT only from now on */
-	i915_gem_object_set_to_gtt_domain(fbo, 1);
+	ret = i915_gem_object_set_to_gtt_domain(fbo, 1);
+	if (ret) {
+		DRM_ERROR("failed to bind fb: %d.\n", ret);
+		goto out_unpin;
+	}
 
 	info = framebuffer_alloc(0, device);
 	if (!info) {
-- 
1.7.1

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

* [PATCH 10/11] drm/i915: Reject bind_to_gtt() early if object > aperture
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (8 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 09/11] drm/i915: Check error code whilst moving buffer to GTT domain Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-27 12:18 ` [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers Chris Wilson
  10 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

If the object is bigger than the entire aperture, reject it early
before evicting everything in a vain attempt to find space.

v2: Use E2BIG as suggested by Owain G. Ainsworth.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
---
 drivers/gpu/drm/i915/i915_gem.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b87945d..f84c8e9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2648,6 +2648,14 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
 		return -EINVAL;
 	}
 
+	/* If the object is bigger than the entire aperture, reject it early
+	 * before evicting everything in a vain attempt to find space.
+	 */
+	if (obj->size > dev->gtt_total) {
+		DRM_ERROR("Attempting to bind an object larger than the aperture\n");
+		return -E2BIG;
+	}
+
  search_free:
 	free_space = drm_mm_search_free(&dev_priv->mm.gtt_space,
 					obj->size, alignment, 0);
-- 
1.7.1

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

* [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers
  2010-05-27 12:18 A compendium of trivial patches Chris Wilson
                   ` (9 preceding siblings ...)
  2010-05-27 12:18 ` [PATCH 10/11] drm/i915: Reject bind_to_gtt() early if object > aperture Chris Wilson
@ 2010-05-27 12:18 ` Chris Wilson
  2010-05-28 18:01   ` Eric Anholt
  10 siblings, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 12:18 UTC (permalink / raw)
  To: intel-gfx

The callers expect us to cleanup any partially initialised structures
before reporting the error.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f84c8e9..42866c0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4632,23 +4632,40 @@ i915_gem_init_ringbuffer(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	int ret;
+
 	dev_priv->render_ring = render_ring;
+
 	if (!I915_NEED_GFX_HWS(dev)) {
 		dev_priv->render_ring.status_page.page_addr
 			= dev_priv->status_page_dmah->vaddr;
 		memset(dev_priv->render_ring.status_page.page_addr,
 				0, PAGE_SIZE);
 	}
+
 	if (HAS_PIPE_CONTROL(dev)) {
 		ret = i915_gem_init_pipe_control(dev);
 		if (ret)
 			return ret;
 	}
+
 	ret = intel_init_ring_buffer(dev, &dev_priv->render_ring);
-	if (!ret && HAS_BSD(dev)) {
+	if (ret)
+		goto cleanup_pipe_control;
+
+	if (HAS_BSD(dev)) {
 		dev_priv->bsd_ring = bsd_ring;
 		ret = intel_init_ring_buffer(dev, &dev_priv->bsd_ring);
+		if (ret)
+			goto cleanup_render_ring;
 	}
+
+	return 0;
+
+cleanup_render_ring:
+	intel_cleanup_ring_buffer(dev, &dev_priv->render_ring);
+cleanup_pipe_control:
+	if (HAS_PIPE_CONTROL(dev))
+		i915_gem_cleanup_pipe_control(dev);
 	return ret;
 }
 
-- 
1.7.1

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

* Re: [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path
  2010-05-27 12:18 ` [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path Chris Wilson
@ 2010-05-27 16:03   ` Jesse Barnes
  0 siblings, 0 replies; 16+ messages in thread
From: Jesse Barnes @ 2010-05-27 16:03 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, 27 May 2010 13:18:13 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Delay taking the mutex until we need to and ensure that we hold the
> spinlock when resetting unpin_work on the error path. Also defer the
> debugging print messages until after we have released the spinlock.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Kristian Høgsberg <krh@bitplanet.net>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cfac4dd..1845a06 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4667,8 +4667,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	if (work == NULL)
>  		return -ENOMEM;
>  
> -	mutex_lock(&dev->struct_mutex);
> -
>  	work->event = event;
>  	work->dev = crtc->dev;
>  	intel_fb = to_intel_framebuffer(crtc->fb);
> @@ -4678,10 +4676,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	/* We borrow the event spin lock for protecting unpin_work */
>  	spin_lock_irqsave(&dev->event_lock, flags);
>  	if (intel_crtc->unpin_work) {
> -		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
>  		spin_unlock_irqrestore(&dev->event_lock, flags);
>  		kfree(work);
> -		mutex_unlock(&dev->struct_mutex);
> +
> +		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
>  		return -EBUSY;
>  	}
>  	intel_crtc->unpin_work = work;
> @@ -4690,13 +4688,19 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	intel_fb = to_intel_framebuffer(fb);
>  	obj = intel_fb->obj;
>  
> +	mutex_lock(&dev->struct_mutex);
>  	ret = intel_pin_and_fence_fb_obj(dev, obj);
>  	if (ret != 0) {
> -		DRM_DEBUG_DRIVER("flip queue: %p pin & fence failed\n",
> -			  to_intel_bo(obj));
> -		kfree(work);
> -		intel_crtc->unpin_work = NULL;
>  		mutex_unlock(&dev->struct_mutex);
> +
> +		spin_lock_irqsave(&dev->event_lock, flags);
> +		intel_crtc->unpin_work = NULL;
> +		spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +		kfree(work);
> +
> +		DRM_DEBUG_DRIVER("flip queue: %p pin & fence failed\n",
> +				 to_intel_bo(obj));
>  		return ret;
>  	}

The mutex change looks like a good cleanup, and the spin lock addition
is a good fix.  Thanks.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object.
  2010-05-27 12:18 ` [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object Chris Wilson
@ 2010-05-27 16:05   ` Jesse Barnes
  2010-05-27 16:32     ` Chris Wilson
  0 siblings, 1 reply; 16+ messages in thread
From: Jesse Barnes @ 2010-05-27 16:05 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Thu, 27 May 2010 13:18:15 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_gem.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1c65f0b..6425c2a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3307,9 +3307,13 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj,
>  	             obj_priv->tiling_mode != I915_TILING_NONE;
>  
>  	/* Check fence reg constraints and rebind if necessary */
> -	if (need_fence && !i915_gem_object_fence_offset_ok(obj,
> -	    obj_priv->tiling_mode))
> -		i915_gem_object_unbind(obj);
> +	if (need_fence &&
> +	    !i915_gem_object_fence_offset_ok(obj,
> +					     obj_priv->tiling_mode)) {
> +		ret = i915_gem_object_unbind(obj);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	/* Choose the GTT offset for our buffer and put it there. */
>  	ret = i915_gem_object_pin(obj, (uint32_t) entry->alignment);

Looks ok to me.  Out of curiosity, are you seeing errors here on unbind
with some loads?

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center

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

* Re: [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object.
  2010-05-27 16:05   ` Jesse Barnes
@ 2010-05-27 16:32     ` Chris Wilson
  0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2010-05-27 16:32 UTC (permalink / raw)
  To: Jesse Barnes; +Cc: intel-gfx

On Thu, 27 May 2010 09:05:04 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> Looks ok to me.  Out of curiosity, are you seeing errors here on unbind
> with some loads?

Since you ask, yes... But it is usually -5. ;-)
-ickle

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers
  2010-05-27 12:18 ` [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers Chris Wilson
@ 2010-05-28 18:01   ` Eric Anholt
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Anholt @ 2010-05-28 18:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx


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

On Thu, 27 May 2010 13:18:22 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The callers expect us to cleanup any partially initialised structures
> before reporting the error.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Applied this series.

[-- 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] 16+ messages in thread

end of thread, other threads:[~2010-05-28 18:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-27 12:18 A compendium of trivial patches Chris Wilson
2010-05-27 12:18 ` [PATCH 01/11] drm/i915: Only print an message if there was an error Chris Wilson
2010-05-27 12:18 ` [PATCH 02/11] drm/i915: Hold the spinlock whilst resetting unpin_work along error path Chris Wilson
2010-05-27 16:03   ` Jesse Barnes
2010-05-27 12:18 ` [PATCH 03/11] drm/i915: Avoid nesting of domain changes when setting display plane Chris Wilson
2010-05-27 12:18 ` [PATCH 04/11] drm/i915: Propagate error from unbinding an unfenceable object Chris Wilson
2010-05-27 16:05   ` Jesse Barnes
2010-05-27 16:32     ` Chris Wilson
2010-05-27 12:18 ` [PATCH 05/11] drm/i915: Only print "nothing to do" debug message as required Chris Wilson
2010-05-27 12:18 ` [PATCH 06/11] drm/i915: Include pitch in set_base debug statement Chris Wilson
2010-05-27 12:18 ` [PATCH 07/11] drm/i915: Rebind bo if currently bound with incorrect alignment Chris Wilson
2010-05-27 12:18 ` [PATCH 08/11] drm/i915: Remove spurious warning "Failure to install fence" Chris Wilson
2010-05-27 12:18 ` [PATCH 09/11] drm/i915: Check error code whilst moving buffer to GTT domain Chris Wilson
2010-05-27 12:18 ` [PATCH 10/11] drm/i915: Reject bind_to_gtt() early if object > aperture Chris Wilson
2010-05-27 12:18 ` [PATCH 11/11] drm/i915: Cleanup after failed initialization of ringbuffers Chris Wilson
2010-05-28 18:01   ` Eric Anholt

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