[08/13] drm/i915: Pin after setting to the display plane

Commit Message

Chris Wilson April 14, 2011, 9:03 a.m. UTC

A few operations we do in order to move the object into the display
plane it is important for future safety to forbid whilst pinned. As a
result, we want to pin afterwards. At the moment, setting to the display
plane of an unbound object is simply to bind it, so
set_to_display_plane() becomes a no-op.

Signed-off-by: Chris Wilson <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem.c      |    7 +++----
 drivers/gpu/drm/i915/intel_display.c |   20 ++++++++++----------
 drivers/gpu/drm/i915/intel_overlay.c |    8 ++++----
 3 files changed, 17 insertions(+), 18 deletions(-)

Comments

Daniel Vetter April 14, 2011, 5:34 p.m. UTC | #1

On Thu, Apr 14, 2011 at 10:03:42AM +0100, Chris Wilson wrote:
> A few operations we do in order to move the object into the display
> plane it is important for future safety to forbid whilst pinned. As a
> result, we want to pin afterwards. At the moment, setting to the display
> plane of an unbound object is simply to bind it, so
> set_to_display_plane() becomes a no-op.

After the movement all three code-paths suffer from
	if (ret)
		goto foo_unpin;
before anything is actually pinned. With that fixed, it's

Reviewed-by: Daniel Vetter <[e-mail]>

Chris Wilson April 14, 2011, 9:31 p.m. UTC | #2

On Thu, 14 Apr 2011 19:34:17 +0200, Daniel Vetter <[e-mail]> wrote:
> After the movement all three code-paths suffer from
> 	if (ret)
> 		goto foo_unpin;
> before anything is actually pinned. With that fixed, it's

My only defense was that was about the third or fifth variation that I
tried in a couple of hours with lots of painful rebasing to get the patch
flow right...

I know for one iteration I had remembered to update the error paths.

Oh well,
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b92510c..3f1181b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3105,15 +3105,14 @@  i915_gem_object_set_to_display_plane(struct drm_i915_gem_object *obj,
 	uint32_t old_read_domains;
 	int ret;
 
-	/* Not valid to be called on unbound objects. */
-	if (obj->gtt_space == NULL)
-		return -EINVAL;
+	/* If the object is currently unbound, this is a no-op. */
+	if (obj->gtt_space)
+		return 0;
 
 	ret = i915_gem_object_flush_gpu_write_domain(obj);
 	if (ret)
 		return ret;
 
-
 	/* Currently, we are always called from an non-interruptible context. */
 	if (pipelined != obj->ring) {
 		ret = i915_gem_object_wait_rendering(obj);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 56741c6..811b6e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1807,14 +1807,14 @@  intel_pin_and_fence_fb_obj(struct drm_device *dev,
 	}
 
 	dev_priv->mm.interruptible = false;
-	ret = i915_gem_object_pin(obj, alignment, true);
-	if (ret)
-		goto err_interruptible;
-
 	ret = i915_gem_object_set_to_display_plane(obj, pipelined);
 	if (ret)
 		goto err_unpin;
 
+	ret = i915_gem_object_pin(obj, alignment, true);
+	if (ret)
+		goto err_interruptible;
+
 	/* Install a fence for tiled scan-out. Pre-i965 always needs a
 	 * fence, whereas 965+ only requires a fence if using
 	 * framebuffer compression.  For simplicity, we always install
@@ -5354,12 +5354,6 @@  static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 			goto fail_locked;
 		}
 
-		ret = i915_gem_object_pin(obj, PAGE_SIZE, true);
-		if (ret) {
-			DRM_ERROR("failed to pin cursor bo\n");
-			goto fail_locked;
-		}
-
 		ret = i915_gem_object_set_to_display_plane(obj, NULL);
 		if (ret) {
 			DRM_ERROR("failed to move cursor bo into the GTT\n");
@@ -5372,6 +5366,12 @@  static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 			goto fail_unpin;
 		}
 
+		ret = i915_gem_object_pin(obj, PAGE_SIZE, true);
+		if (ret) {
+			DRM_ERROR("failed to pin cursor bo\n");
+			goto fail_locked;
+		}
+
 		addr = obj->gtt_offset;
 	} else {
 		int align = IS_I830(dev) ? 16 * 1024 : 256;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index e0903c5..b4ae58d 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -773,10 +773,6 @@  static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	if (ret != 0)
 		return ret;
 
-	ret = i915_gem_object_pin(new_bo, PAGE_SIZE, true);
-	if (ret != 0)
-		return ret;
-
 	ret = i915_gem_object_set_to_display_plane(new_bo, NULL);
 	if (ret != 0)
 		goto out_unpin;
@@ -785,6 +781,10 @@  static int intel_overlay_do_put_image(struct intel_overlay *overlay,
 	if (ret)
 		goto out_unpin;
 
+	ret = i915_gem_object_pin(new_bo, PAGE_SIZE, true);
+	if (ret != 0)
+		return ret;
+
 	if (!overlay->active) {
 		regs = intel_overlay_map_regs(overlay);
 		if (!regs) {