All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb()
@ 2016-10-29 12:04 Chris Wilson
  2016-10-29 12:04 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw)
  To: intel-gfx

In the next patch, a few rearrangements are made to make these static.
First, we move them so the changes are not lost in the noise.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a0de5ce1a3e1..6c817215f61a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14132,6 +14132,134 @@ static int intel_atomic_check(struct drm_device *dev,
 	return calc_watermark_data(state);
 }
 
+/**
+ * intel_prepare_plane_fb - Prepare fb for usage on plane
+ * @plane: drm plane to prepare for
+ * @fb: framebuffer to prepare for presentation
+ *
+ * Prepares a framebuffer for usage on a display plane.  Generally this
+ * involves pinning the underlying object and updating the frontbuffer tracking
+ * bits.  Some older platforms need special physical address handling for
+ * cursor planes.
+ *
+ * Must be called with struct_mutex held.
+ *
+ * Returns 0 on success, negative error code on failure.
+ */
+int
+intel_prepare_plane_fb(struct drm_plane *plane,
+		       struct drm_plane_state *new_state)
+{
+	struct intel_atomic_state *intel_state =
+		to_intel_atomic_state(new_state->state);
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	struct drm_framebuffer *fb = new_state->fb;
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
+	int ret;
+
+	if (!obj && !old_obj)
+		return 0;
+
+	if (old_obj) {
+		struct drm_crtc_state *crtc_state =
+			drm_atomic_get_existing_crtc_state(new_state->state,
+							   plane->state->crtc);
+
+		/* Big Hammer, we also need to ensure that any pending
+		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
+		 * current scanout is retired before unpinning the old
+		 * framebuffer. Note that we rely on userspace rendering
+		 * into the buffer attached to the pipe they are waiting
+		 * on. If not, userspace generates a GPU hang with IPEHR
+		 * point to the MI_WAIT_FOR_EVENT.
+		 *
+		 * This should only fail upon a hung GPU, in which case we
+		 * can safely continue.
+		 */
+		if (needs_modeset(crtc_state)) {
+			ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
+							      old_obj->resv, NULL,
+							      false, 0,
+							      GFP_KERNEL);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	if (new_state->fence) { /* explicit fencing */
+		ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready,
+						    new_state->fence,
+						    I915_FENCE_TIMEOUT,
+						    GFP_KERNEL);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (!obj)
+		return 0;
+
+	if (!new_state->fence) { /* implicit fencing */
+		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
+						      obj->resv, NULL,
+						      false, I915_FENCE_TIMEOUT,
+						      GFP_KERNEL);
+		if (ret < 0)
+			return ret;
+
+		i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
+	}
+
+	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
+	    INTEL_INFO(dev)->cursor_needs_physical) {
+		int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
+		ret = i915_gem_object_attach_phys(obj, align);
+		if (ret) {
+			DRM_DEBUG_KMS("failed to attach phys object\n");
+			return ret;
+		}
+	} else {
+		struct i915_vma *vma;
+
+		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+		if (IS_ERR(vma)) {
+			DRM_DEBUG_KMS("failed to pin object\n");
+			return PTR_ERR(vma);
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * intel_cleanup_plane_fb - Cleans up an fb after plane use
+ * @plane: drm plane to clean up for
+ * @fb: old framebuffer that was on plane
+ *
+ * Cleans up a framebuffer that has just been removed from a plane.
+ *
+ * Must be called with struct_mutex held.
+ */
+void
+intel_cleanup_plane_fb(struct drm_plane *plane,
+		       struct drm_plane_state *old_state)
+{
+	struct drm_device *dev = plane->dev;
+	struct intel_plane_state *old_intel_state;
+	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
+	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
+
+	old_intel_state = to_intel_plane_state(old_state);
+
+	if (!obj && !old_obj)
+		return;
+
+	if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
+	    !INTEL_INFO(dev)->cursor_needs_physical))
+		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
+}
+
 static int intel_atomic_prepare_commit(struct drm_device *dev,
 				       struct drm_atomic_state *state)
 {
@@ -14701,134 +14829,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
 
-/**
- * intel_prepare_plane_fb - Prepare fb for usage on plane
- * @plane: drm plane to prepare for
- * @fb: framebuffer to prepare for presentation
- *
- * Prepares a framebuffer for usage on a display plane.  Generally this
- * involves pinning the underlying object and updating the frontbuffer tracking
- * bits.  Some older platforms need special physical address handling for
- * cursor planes.
- *
- * Must be called with struct_mutex held.
- *
- * Returns 0 on success, negative error code on failure.
- */
-int
-intel_prepare_plane_fb(struct drm_plane *plane,
-		       struct drm_plane_state *new_state)
-{
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(new_state->state);
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct drm_framebuffer *fb = new_state->fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
-	int ret;
-
-	if (!obj && !old_obj)
-		return 0;
-
-	if (old_obj) {
-		struct drm_crtc_state *crtc_state =
-			drm_atomic_get_existing_crtc_state(new_state->state,
-							   plane->state->crtc);
-
-		/* Big Hammer, we also need to ensure that any pending
-		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
-		 * current scanout is retired before unpinning the old
-		 * framebuffer. Note that we rely on userspace rendering
-		 * into the buffer attached to the pipe they are waiting
-		 * on. If not, userspace generates a GPU hang with IPEHR
-		 * point to the MI_WAIT_FOR_EVENT.
-		 *
-		 * This should only fail upon a hung GPU, in which case we
-		 * can safely continue.
-		 */
-		if (needs_modeset(crtc_state)) {
-			ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
-							      old_obj->resv, NULL,
-							      false, 0,
-							      GFP_KERNEL);
-			if (ret < 0)
-				return ret;
-		}
-	}
-
-	if (new_state->fence) { /* explicit fencing */
-		ret = i915_sw_fence_await_dma_fence(&intel_state->commit_ready,
-						    new_state->fence,
-						    I915_FENCE_TIMEOUT,
-						    GFP_KERNEL);
-		if (ret < 0)
-			return ret;
-	}
-
-	if (!obj)
-		return 0;
-
-	if (!new_state->fence) { /* implicit fencing */
-		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
-						      obj->resv, NULL,
-						      false, I915_FENCE_TIMEOUT,
-						      GFP_KERNEL);
-		if (ret < 0)
-			return ret;
-
-		i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
-	}
-
-	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
-	    INTEL_INFO(dev)->cursor_needs_physical) {
-		int align = IS_I830(dev_priv) ? 16 * 1024 : 256;
-		ret = i915_gem_object_attach_phys(obj, align);
-		if (ret) {
-			DRM_DEBUG_KMS("failed to attach phys object\n");
-			return ret;
-		}
-	} else {
-		struct i915_vma *vma;
-
-		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
-		if (IS_ERR(vma)) {
-			DRM_DEBUG_KMS("failed to pin object\n");
-			return PTR_ERR(vma);
-		}
-	}
-
-	return 0;
-}
-
-/**
- * intel_cleanup_plane_fb - Cleans up an fb after plane use
- * @plane: drm plane to clean up for
- * @fb: old framebuffer that was on plane
- *
- * Cleans up a framebuffer that has just been removed from a plane.
- *
- * Must be called with struct_mutex held.
- */
-void
-intel_cleanup_plane_fb(struct drm_plane *plane,
-		       struct drm_plane_state *old_state)
-{
-	struct drm_device *dev = plane->dev;
-	struct intel_plane_state *old_intel_state;
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
-	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
-
-	old_intel_state = to_intel_plane_state(old_state);
-
-	if (!obj && !old_obj)
-		return;
-
-	if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
-	    !INTEL_INFO(dev)->cursor_needs_physical))
-		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
-}
-
 int
 skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2616d92f9fee..f5975fe4553d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -782,6 +782,8 @@ struct intel_plane {
 	int max_downscale;
 	uint32_t frontbuffer_bit;
 
+	struct i915_sw_fence *last_commit;
+
 	/* Since we need to change the watermarks before/after
 	 * enabling/disabling the planes, we need to store the parameters here
 	 * as the other pieces of the struct may not reflect the values we want
-- 
2.10.1

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

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

* [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit
  2016-10-29 12:04 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
@ 2016-10-29 12:04 ` Chris Wilson
  2016-10-29 12:04 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw)
  To: intel-gfx

The generic atomic helper likes to skip a prepare_plane_fb() if it
decides that the plane->fb is unchanged. This is wrong for us for a
couple of reasons:

 - if the pipe is reconfigured (i.e. a size change) but the framebuffer
   is untouched, we still have to flush any rendering prior to the
   reconfiguration to prevent wait-for-scanline GPU hangs

 - if the framebuffer is rotated, it remains the same but has a
   different view and a different address.

Finally, even if the framebuffer is unchanged the flip/modeset should be
ordered with respect to rendering to the frontbuffer.

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

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index cb5594411bb6..4ed5b2e49691 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -192,8 +192,6 @@ static void intel_plane_atomic_update(struct drm_plane *plane,
 }
 
 const struct drm_plane_helper_funcs intel_plane_helper_funcs = {
-	.prepare_fb = intel_prepare_plane_fb,
-	.cleanup_fb = intel_cleanup_plane_fb,
 	.atomic_check = intel_plane_atomic_check,
 	.atomic_update = intel_plane_atomic_update,
 };
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6c817215f61a..69720a126c67 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14146,7 +14146,7 @@ static int intel_atomic_check(struct drm_device *dev,
  *
  * Returns 0 on success, negative error code on failure.
  */
-int
+static int
 intel_prepare_plane_fb(struct drm_plane *plane,
 		       struct drm_plane_state *new_state)
 {
@@ -14241,7 +14241,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
  *
  * Must be called with struct_mutex held.
  */
-void
+static void
 intel_cleanup_plane_fb(struct drm_plane *plane,
 		       struct drm_plane_state *old_state)
 {
@@ -14260,6 +14260,50 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
 }
 
+static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state)
+{
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	int i, j, ret;
+
+	ret = mutex_lock_interruptible(&state->dev->struct_mutex);
+	if (ret)
+		return ret;
+
+	for_each_plane_in_state(state, plane, plane_state, i) {
+		ret = intel_prepare_plane_fb(plane, plane_state);
+		if (ret)
+			break;
+	}
+
+	if (ret) {
+		for_each_plane_in_state(state, plane, plane_state, j) {
+			if (j >= i)
+				break;
+
+			intel_cleanup_plane_fb(plane, plane_state);
+		}
+	}
+
+	mutex_unlock(&state->dev->struct_mutex);
+
+	return ret;
+}
+
+static void intel_atomic_commit_cleanup_planes(struct drm_atomic_state *state)
+{
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
+	int i;
+
+	mutex_lock(&state->dev->struct_mutex);
+
+	for_each_plane_in_state(state, plane, plane_state, i)
+		intel_cleanup_plane_fb(plane, plane_state);
+
+	mutex_unlock(&state->dev->struct_mutex);
+}
+
 static int intel_atomic_prepare_commit(struct drm_device *dev,
 				       struct drm_atomic_state *state)
 {
@@ -14280,14 +14324,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 			flush_workqueue(dev_priv->wq);
 	}
 
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-
-	ret = drm_atomic_helper_prepare_planes(dev, state);
-	mutex_unlock(&dev->struct_mutex);
-
-	return ret;
+	return intel_atomic_commit_prepare_planes(state);
 }
 
 u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
@@ -14614,9 +14651,7 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	if (intel_state->modeset)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 
-	mutex_lock(&dev->struct_mutex);
-	drm_atomic_helper_cleanup_planes(dev, state);
-	mutex_unlock(&dev->struct_mutex);
+	intel_atomic_commit_cleanup_planes(state);
 
 	drm_atomic_helper_commit_cleanup_done(state);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f5975fe4553d..610f2f969a9a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1292,10 +1292,6 @@ __intel_framebuffer_create(struct drm_device *dev,
 void intel_finish_page_flip_cs(struct drm_i915_private *dev_priv, int pipe);
 void intel_finish_page_flip_mmio(struct drm_i915_private *dev_priv, int pipe);
 void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe);
-int intel_prepare_plane_fb(struct drm_plane *plane,
-			   struct drm_plane_state *new_state);
-void intel_cleanup_plane_fb(struct drm_plane *plane,
-			    struct drm_plane_state *old_state);
 int intel_plane_atomic_get_property(struct drm_plane *plane,
 				    const struct drm_plane_state *state,
 				    struct drm_property *property,
-- 
2.10.1

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

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

* [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-10-29 12:04 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
  2016-10-29 12:04 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson
@ 2016-10-29 12:04 ` Chris Wilson
  2016-10-29 12:04 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson
  2016-10-29 12:04 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw)
  To: intel-gfx

With atomic plane states we are able to track an allocation right from
preparation, during use and through to the final free after being
swapped out for a new plane. We can couple the VMA we pin for the
framebuffer (and its rotation) to this lifetime and avoid all the clumsy
lookups in between.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h           |  16 ++---
 drivers/gpu/drm/i915/intel_atomic_plane.c |   2 +
 drivers/gpu/drm/i915/intel_display.c      | 100 ++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h          |   9 ++-
 drivers/gpu/drm/i915/intel_fbc.c          |  52 ++++++----------
 drivers/gpu/drm/i915/intel_fbdev.c        |   4 +-
 drivers/gpu/drm/i915/intel_sprite.c       |   8 +--
 7 files changed, 80 insertions(+), 111 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eb89cbfe5fb9..81d2d2f603ce 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1001,6 +1001,8 @@ struct intel_fbc {
 	struct work_struct underrun_work;
 
 	struct intel_fbc_state_cache {
+		struct i915_vma *vma;
+
 		struct {
 			unsigned int mode_flags;
 			uint32_t hsw_bdw_pixel_rate;
@@ -1014,15 +1016,14 @@ struct intel_fbc {
 		} plane;
 
 		struct {
-			u64 ilk_ggtt_offset;
 			uint32_t pixel_format;
 			unsigned int stride;
-			int fence_reg;
-			unsigned int tiling_mode;
 		} fb;
 	} state_cache;
 
 	struct intel_fbc_reg_params {
+		struct i915_vma *vma;
+
 		struct {
 			enum pipe pipe;
 			enum plane plane;
@@ -1030,10 +1031,8 @@ struct intel_fbc {
 		} crtc;
 
 		struct {
-			u64 ggtt_offset;
 			uint32_t pixel_format;
 			unsigned int stride;
-			int fence_reg;
 		} fb;
 
 		int cfb_size;
@@ -3451,13 +3450,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
 	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
 }
 
-static inline unsigned long
-i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
-			    const struct i915_ggtt_view *view)
-{
-	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
-}
-
 /* i915_gem_fence.c */
 int __must_check i915_vma_get_fence(struct i915_vma *vma);
 int __must_check i915_vma_put_fence(struct i915_vma *vma);
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 4ed5b2e49691..a21ab370fe19 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 
 	__drm_atomic_helper_plane_duplicate_state(plane, state);
 
+	intel_state->vma = NULL;
+
 	return state;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 69720a126c67..9391c1dcd57f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2243,24 +2243,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 			i915_vma_pin_fence(vma);
 	}
 
+	i915_vma_get(vma);
 err:
 	intel_runtime_pm_put(dev_priv);
 	return vma;
 }
 
-void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+void intel_unpin_fb_vma(struct i915_vma *vma)
 {
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct i915_ggtt_view view;
-	struct i915_vma *vma;
-
-	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
-
-	intel_fill_fb_ggtt_view(&view, fb, rotation);
-	vma = i915_gem_object_to_ggtt(obj, &view);
+	lockdep_assert_held(&vma->vm->dev->struct_mutex);
 
 	i915_vma_unpin_fence(vma);
 	i915_gem_object_unpin_from_display_plane(vma);
+	i915_vma_put(vma);
 }
 
 static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane,
@@ -2754,7 +2749,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *c;
-	struct intel_crtc *i;
 	struct drm_i915_gem_object *obj;
 	struct drm_plane *primary = intel_crtc->base.primary;
 	struct drm_plane_state *plane_state = primary->state;
@@ -2779,20 +2773,20 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * an fb with another CRTC instead
 	 */
 	for_each_crtc(dev, c) {
-		i = to_intel_crtc(c);
+		struct intel_plane_state *state;
 
 		if (c == &intel_crtc->base)
 			continue;
 
-		if (!i->active)
+		if (!to_intel_crtc(c)->active)
 			continue;
 
-		fb = c->primary->fb;
-		if (!fb)
+		state = to_intel_plane_state(c->primary->state);
+		if (!state->vma)
 			continue;
 
-		obj = intel_fb_obj(fb);
-		if (i915_gem_object_ggtt_offset(obj, NULL) == plane_config->base) {
+		if (i915_ggtt_offset(state->vma) == plane_config->base) {
+			fb = c->primary->fb;
 			drm_framebuffer_reference(fb);
 			goto valid_fb;
 		}
@@ -2838,6 +2832,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 
 	drm_framebuffer_reference(fb);
 	primary->fb = primary->state->fb = fb;
+
+	mutex_lock(&dev->struct_mutex);
+	intel_state->vma =
+		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+	mutex_unlock(&dev->struct_mutex);
+
 	primary->crtc = primary->state->crtc = &intel_crtc->base;
 	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
 	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
@@ -3016,7 +3016,6 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_framebuffer *fb = plane_state->base.fb;
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	int plane = intel_crtc->plane;
 	u32 linear_offset;
 	u32 dspcntr;
@@ -3107,12 +3106,14 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	if (INTEL_INFO(dev)->gen >= 4) {
 		I915_WRITE(DSPSURF(plane),
-			   intel_fb_gtt_offset(fb, rotation) +
+			   intel_plane_ggtt_offset(plane_state) +
 			   intel_crtc->dspaddr_offset);
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else
-		I915_WRITE(DSPADDR(plane), i915_gem_object_ggtt_offset(obj, NULL) + linear_offset);
+		I915_WRITE(DSPADDR(plane),
+			   intel_plane_ggtt_offset(plane_state) +
+			   linear_offset);
 	POSTING_READ(reg);
 }
 
@@ -3206,7 +3207,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	I915_WRITE(DSPSURF(plane),
-		   intel_fb_gtt_offset(fb, rotation) +
+		   intel_plane_ggtt_offset(plane_state) +
 		   intel_crtc->dspaddr_offset);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
@@ -3229,23 +3230,6 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
 	}
 }
 
-u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
-			unsigned int rotation)
-{
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct i915_ggtt_view view;
-	struct i915_vma *vma;
-
-	intel_fill_fb_ggtt_view(&view, fb, rotation);
-
-	vma = i915_gem_object_to_ggtt(obj, &view);
-	if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
-		 view.type))
-		return -1;
-
-	return i915_ggtt_offset(vma);
-}
-
 static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -3446,7 +3430,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	}
 
 	I915_WRITE(PLANE_SURF(pipe, 0),
-		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
+		   intel_plane_ggtt_offset(plane_state) + surf_addr);
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
 }
@@ -11564,7 +11548,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 		flush_work(&work->mmio_work);
 
 	mutex_lock(&dev->struct_mutex);
-	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
+	intel_unpin_fb_vma(work->old_vma);
 	i915_gem_object_put(work->pending_flip_obj);
 	mutex_unlock(&dev->struct_mutex);
 
@@ -12277,8 +12261,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 	}
 
-	work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation);
-	work->gtt_offset += intel_crtc->dspaddr_offset;
+	work->old_vma = to_intel_plane_state(primary->state)->vma;
+	to_intel_plane_state(primary->state)->vma = vma;
+
+	work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset;
 	work->rotation = crtc->primary->state->rotation;
 
 	/*
@@ -12331,7 +12317,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 cleanup_request:
 	i915_add_request_no_flush(request);
 cleanup_unpin:
-	intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
+	to_intel_plane_state(primary->state)->vma = work->old_vma;
+	intel_unpin_fb_vma(vma);
 cleanup_pending:
 	atomic_dec(&intel_crtc->unpin_work_count);
 	mutex_unlock(&dev->struct_mutex);
@@ -14223,10 +14210,10 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		struct i915_vma *vma;
 
 		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
-		if (IS_ERR(vma)) {
-			DRM_DEBUG_KMS("failed to pin object\n");
-			return PTR_ERR(vma);
-		}
+		if (IS_ERR(vma))
+			ret = PTR_ERR(vma);
+
+		to_intel_plane_state(new_state)->vma = vma;
 	}
 
 	return 0;
@@ -14245,19 +14232,12 @@ static void
 intel_cleanup_plane_fb(struct drm_plane *plane,
 		       struct drm_plane_state *old_state)
 {
-	struct drm_device *dev = plane->dev;
-	struct intel_plane_state *old_intel_state;
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
-	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
-
-	old_intel_state = to_intel_plane_state(old_state);
-
-	if (!obj && !old_obj)
-		return;
+	struct i915_vma *vma;
 
-	if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
-	    !INTEL_INFO(dev)->cursor_needs_physical))
-		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
+	/* Should only called after a successful intel_prepare_plane_fb()! */
+	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
+	if (vma)
+		intel_unpin_fb_vma(vma);
 }
 
 static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state)
@@ -15192,7 +15172,7 @@ intel_update_cursor_plane(struct drm_plane *plane,
 	if (!obj)
 		addr = 0;
 	else if (!INTEL_INFO(dev)->cursor_needs_physical)
-		addr = i915_gem_object_ggtt_offset(obj, NULL);
+		addr = i915_ggtt_offset(state->vma);
 	else
 		addr = obj->phys_handle->busaddr;
 
@@ -17099,6 +17079,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
 			update_state_fb(c->primary);
 			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
 		}
+
+		to_intel_plane_state(c->primary->state)->vma = vma;
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 610f2f969a9a..af23e8665be8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -372,6 +372,7 @@ struct intel_atomic_state {
 struct intel_plane_state {
 	struct drm_plane_state base;
 	struct drm_rect clip;
+	struct i915_vma *vma;
 
 	struct {
 		u32 offset;
@@ -1058,6 +1059,7 @@ struct intel_flip_work {
 	struct work_struct mmio_work;
 
 	struct drm_crtc *crtc;
+	struct i915_vma *old_vma;
 	struct drm_framebuffer *old_fb;
 	struct drm_i915_gem_object *pending_flip_obj;
 	struct drm_pending_vblank_event *event;
@@ -1284,7 +1286,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct drm_modeset_acquire_ctx *ctx);
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
-void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
+void intel_unpin_fb_vma(struct i915_vma *vma);
 struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
@@ -1369,7 +1371,10 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
 
-u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
+static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
+{
+	return i915_ggtt_offset(state->vma);
+}
 
 u32 skl_plane_ctl_format(uint32_t pixel_format);
 u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index cbe2ebda4c40..b63d61fd2bad 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -173,7 +173,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 	if (IS_I945GM(dev_priv))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
 	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
-	fbc_ctl |= params->fb.fence_reg;
+	fbc_ctl |= params->vma->fence->id;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 }
 
@@ -193,8 +193,8 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
 	else
 		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
-		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg;
+	if (params->vma->fence) {
+		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id;
 		I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
 	} else {
 		I915_WRITE(DPFC_FENCE_YOFF, 0);
@@ -251,13 +251,14 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
+	if (params->vma->fence) {
 		dpfc_ctl |= DPFC_CTL_FENCE_EN;
 		if (IS_GEN5(dev_priv))
-			dpfc_ctl |= params->fb.fence_reg;
+			dpfc_ctl |= params->vma->fence->id;
 		if (IS_GEN6(dev_priv)) {
 			I915_WRITE(SNB_DPFC_CTL_SA,
-				   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
+				   SNB_CPU_FENCE_ENABLE |
+				   params->vma->fence->id);
 			I915_WRITE(DPFC_CPU_FENCE_OFFSET,
 				   params->crtc.fence_y_offset);
 		}
@@ -269,7 +270,8 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 	}
 
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
-	I915_WRITE(ILK_FBC_RT_BASE, params->fb.ggtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(ILK_FBC_RT_BASE,
+		   i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -319,10 +321,11 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
+	if (params->vma->fence) {
 		dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
 		I915_WRITE(SNB_DPFC_CTL_SA,
-			   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
+			   SNB_CPU_FENCE_ENABLE |
+			   params->vma->fence->id);
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);
 	} else {
 		I915_WRITE(SNB_DPFC_CTL_SA,0);
@@ -727,14 +730,6 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 	return effective_w <= max_w && effective_h <= max_h;
 }
 
-/* XXX replace me when we have VMA tracking for intel_plane_state */
-static int get_fence_id(struct drm_framebuffer *fb)
-{
-	struct i915_vma *vma = i915_gem_object_to_ggtt(intel_fb_obj(fb), NULL);
-
-	return vma && vma->fence ? vma->fence->id : I915_FENCE_REG_NONE;
-}
-
 static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 					 struct intel_crtc_state *crtc_state,
 					 struct intel_plane_state *plane_state)
@@ -743,7 +738,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	struct intel_fbc *fbc = &dev_priv->fbc;
 	struct intel_fbc_state_cache *cache = &fbc->state_cache;
 	struct drm_framebuffer *fb = plane_state->base.fb;
-	struct drm_i915_gem_object *obj;
+
+	cache->vma = NULL;
 
 	cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
@@ -758,16 +754,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	if (!cache->plane.visible)
 		return;
 
-	obj = intel_fb_obj(fb);
-
-	/* FIXME: We lack the proper locking here, so only run this on the
-	 * platforms that need. */
-	if (IS_GEN(dev_priv, 5, 6))
-		cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL);
 	cache->fb.pixel_format = fb->pixel_format;
 	cache->fb.stride = fb->pitches[0];
-	cache->fb.fence_reg = get_fence_id(fb);
-	cache->fb.tiling_mode = i915_gem_object_get_tiling(obj);
+
+	cache->vma = plane_state->vma;
 }
 
 static bool intel_fbc_can_activate(struct intel_crtc *crtc)
@@ -784,7 +774,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (!cache->plane.visible) {
+	if (!cache->vma) {
 		fbc->no_fbc_reason = "primary plane not visible";
 		return false;
 	}
@@ -807,8 +797,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	 * so have no fence associated with it) due to aperture constaints
 	 * at the time of pinning.
 	 */
-	if (cache->fb.tiling_mode != I915_TILING_X ||
-	    cache->fb.fence_reg == I915_FENCE_REG_NONE) {
+	if (!cache->vma->fence) {
 		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
 		return false;
 	}
@@ -899,17 +888,16 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	 * zero. */
 	memset(params, 0, sizeof(*params));
 
+	params->vma = cache->vma;
+
 	params->crtc.pipe = crtc->pipe;
 	params->crtc.plane = crtc->plane;
 	params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
 
 	params->fb.pixel_format = cache->fb.pixel_format;
 	params->fb.stride = cache->fb.stride;
-	params->fb.fence_reg = cache->fb.fence_reg;
 
 	params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache);
-
-	params->fb.ggtt_offset = cache->fb.ilk_ggtt_offset;
 }
 
 static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index b7098f98bb67..836e8b6128a8 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -287,7 +287,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 out_destroy_fbi:
 	drm_fb_helper_release_fbi(helper);
 out_unpin:
-	intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
+	intel_unpin_fb_vma(vma);
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -552,7 +552,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
 
 	if (ifbdev->fb) {
 		mutex_lock(&ifbdev->helper.dev->struct_mutex);
-		intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
+		intel_unpin_fb_vma(ifbdev->vma);
 		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
 
 		drm_framebuffer_remove(&ifbdev->fb->base);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 43d0350856e7..6e299dd50512 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -281,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
 	I915_WRITE(PLANE_SURF(pipe, plane),
-		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
+		   intel_plane_ggtt_offset(plane_state) + surf_addr);
 	POSTING_READ(PLANE_SURF(pipe, plane));
 }
 
@@ -470,7 +470,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w);
 	I915_WRITE(SPCNTR(pipe, plane), sprctl);
 	I915_WRITE(SPSURF(pipe, plane),
-		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ(SPSURF(pipe, plane));
 }
 
@@ -606,7 +606,7 @@ ivb_update_plane(struct drm_plane *plane,
 		I915_WRITE(SPRSCALE(pipe), sprscale);
 	I915_WRITE(SPRCTL(pipe), sprctl);
 	I915_WRITE(SPRSURF(pipe),
-		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ(SPRSURF(pipe));
 }
 
@@ -732,7 +732,7 @@ ilk_update_plane(struct drm_plane *plane,
 	I915_WRITE(DVSSCALE(pipe), dvsscale);
 	I915_WRITE(DVSCNTR(pipe), dvscntr);
 	I915_WRITE(DVSSURF(pipe),
-		   intel_fb_gtt_offset(fb, rotation) + dvssurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
 	POSTING_READ(DVSSURF(pipe));
 }
 
-- 
2.10.1

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

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

* [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb()
  2016-10-29 12:04 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
  2016-10-29 12:04 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson
  2016-10-29 12:04 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
@ 2016-10-29 12:04 ` Chris Wilson
  2016-10-29 12:04 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw)
  To: intel-gfx

Just a quick tidy now to make the next patch neater.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9391c1dcd57f..97bd5698003f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14119,6 +14119,17 @@ static int intel_atomic_check(struct drm_device *dev,
 	return calc_watermark_data(state);
 }
 
+static bool old_plane_needs_modeset(struct drm_plane *plane,
+				    struct drm_plane_state *new_state)
+{
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_existing_crtc_state(new_state->state,
+							plane->state->crtc);
+
+	return needs_modeset(crtc_state);
+}
+
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -14143,16 +14154,11 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_framebuffer *fb = new_state->fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
 	int ret;
 
-	if (!obj && !old_obj)
-		return 0;
-
-	if (old_obj) {
-		struct drm_crtc_state *crtc_state =
-			drm_atomic_get_existing_crtc_state(new_state->state,
-							   plane->state->crtc);
+	if (plane->state->fb && old_plane_needs_modeset(plane, new_state)) {
+		struct drm_i915_gem_object *old_obj =
+			intel_fb_obj(plane->state->fb);
 
 		/* Big Hammer, we also need to ensure that any pending
 		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
@@ -14165,14 +14171,12 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		 * This should only fail upon a hung GPU, in which case we
 		 * can safely continue.
 		 */
-		if (needs_modeset(crtc_state)) {
-			ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
-							      old_obj->resv, NULL,
-							      false, 0,
-							      GFP_KERNEL);
-			if (ret < 0)
-				return ret;
-		}
+		ret = i915_sw_fence_await_reservation(&intel_state->commit_ready,
+						      old_obj->resv, NULL,
+						      false, 0,
+						      GFP_KERNEL);
+		if (ret < 0)
+			return ret;
 	}
 
 	if (new_state->fence) { /* explicit fencing */
@@ -14211,7 +14215,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
 		if (IS_ERR(vma))
-			ret = PTR_ERR(vma);
+			return PTR_ERR(vma);
 
 		to_intel_plane_state(new_state)->vma = vma;
 	}
-- 
2.10.1

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

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

* [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed
  2016-10-29 12:04 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
                   ` (2 preceding siblings ...)
  2016-10-29 12:04 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson
@ 2016-10-29 12:04 ` Chris Wilson
  3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-29 12:04 UTC (permalink / raw)
  To: intel-gfx

Since an fb may have multiple VMA (due to rotations etc), we need to
wait a vblank and unpin the old VMA not if the fb itself is changed, but
if the underlying VMA is changed.

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 97bd5698003f..2cfc812df04a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12468,9 +12468,6 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
 	if (!was_visible && !visible)
 		return 0;
 
-	if (fb != old_plane_state->base.fb)
-		pipe_config->fb_changed = true;
-
 	turn_off = was_visible && (!visible || mode_changed);
 	turn_on = visible && (!was_visible || mode_changed);
 
@@ -14218,6 +14215,13 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 			return PTR_ERR(vma);
 
 		to_intel_plane_state(new_state)->vma = vma;
+		if (to_intel_plane_state(plane->state)->vma != vma) {
+			struct intel_crtc_state *crtc_state;
+
+			crtc_state = intel_atomic_get_crtc_state(new_state->state,
+								 to_intel_crtc(new_state->crtc));
+			crtc_state->fb_changed = true;
+		}
 	}
 
 	return 0;
-- 
2.10.1

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

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

* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-11-15  8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
  2016-11-15  9:52   ` Daniel Vetter
@ 2016-11-15 10:24   ` Daniel Vetter
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-11-15 10:24 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote:
> @@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  cleanup_request:
>  	i915_add_request_no_flush(request);
>  cleanup_unpin:
> -	intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
> +	to_intel_plane_state(primary->state)->vma = work->old_vma;
> +	intel_unpin_fb_vma(vma);
>  cleanup_pending:
>  	atomic_dec(&intel_crtc->unpin_work_count);
>  	mutex_unlock(&dev->struct_mutex);
> @@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  		struct i915_vma *vma;
>  
>  		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> -		if (IS_ERR(vma)) {
> -			DRM_DEBUG_KMS("failed to pin object\n");
> -			return PTR_ERR(vma);
> -		}
> +		if (IS_ERR(vma))
> +			ret = PTR_ERR(vma);

Uncessary change above that you then go ahead and partially undo in the
next patch. Please remove.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-11-15 10:02     ` Chris Wilson
@ 2016-11-15 10:13       ` Daniel Vetter
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-11-15 10:13 UTC (permalink / raw)
  To: Chris Wilson, Daniel Vetter, intel-gfx

On Tue, Nov 15, 2016 at 10:02:23AM +0000, Chris Wilson wrote:
> On Tue, Nov 15, 2016 at 10:52:00AM +0100, Daniel Vetter wrote:
> > On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote:
> > > With atomic plane states we are able to track an allocation right from
> > > preparation, during use and through to the final free after being
> > > swapped out for a new plane. We can couple the VMA we pin for the
> > > framebuffer (and its rotation) to this lifetime and avoid all the clumsy
> > > lookups in between.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > A few questions and comments below. Of course review assumes some form of
> > patch 2 lands first (but leaning towards just reverting the core one for
> > that).
> > -Daniel
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h           | 16 ++---
> > >  drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +
> > >  drivers/gpu/drm/i915/intel_display.c      | 97 +++++++++++++------------------
> > >  drivers/gpu/drm/i915/intel_drv.h          |  9 ++-
> > >  drivers/gpu/drm/i915/intel_fbc.c          | 52 +++++++----------
> > >  drivers/gpu/drm/i915/intel_fbdev.c        |  4 +-
> > >  drivers/gpu/drm/i915/intel_sprite.c       |  8 +--
> > >  7 files changed, 78 insertions(+), 110 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 4e7148a3ee8b..2bc285e2ff82 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -989,6 +989,8 @@ struct intel_fbc {
> > >  	struct work_struct underrun_work;
> > >  
> > >  	struct intel_fbc_state_cache {
> > > +		struct i915_vma *vma;
> > > +
> > >  		struct {
> > >  			unsigned int mode_flags;
> > >  			uint32_t hsw_bdw_pixel_rate;
> > > @@ -1002,15 +1004,14 @@ struct intel_fbc {
> > >  		} plane;
> > >  
> > >  		struct {
> > > -			u64 ilk_ggtt_offset;
> > >  			uint32_t pixel_format;
> > >  			unsigned int stride;
> > > -			int fence_reg;
> > > -			unsigned int tiling_mode;
> > >  		} fb;
> > >  	} state_cache;
> > >  
> > >  	struct intel_fbc_reg_params {
> > > +		struct i915_vma *vma;
> > > +
> > >  		struct {
> > >  			enum pipe pipe;
> > >  			enum plane plane;
> > > @@ -1018,10 +1019,8 @@ struct intel_fbc {
> > >  		} crtc;
> > >  
> > >  		struct {
> > > -			u64 ggtt_offset;
> > >  			uint32_t pixel_format;
> > >  			unsigned int stride;
> > > -			int fence_reg;
> > >  		} fb;
> > >  
> > >  		int cfb_size;
> > > @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
> > >  	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
> > >  }
> > >  
> > > -static inline unsigned long
> > > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
> > > -			    const struct i915_ggtt_view *view)
> > > -{
> > > -	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
> > > -}
> > > -
> > >  /* i915_gem_fence_reg.c */
> > >  int __must_check i915_vma_get_fence(struct i915_vma *vma);
> > >  int __must_check i915_vma_put_fence(struct i915_vma *vma);
> > > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > > index 89a34350a35d..5a86e0d52409 100644
> > > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
> > >  
> > >  	__drm_atomic_helper_plane_duplicate_state(plane, state);
> > >  
> > > +	intel_state->vma = NULL;
> > > +
> > >  	return state;
> > >  }
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index dca1e0b512e5..cb52116f8577 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> > >  			i915_vma_pin_fence(vma);
> > >  	}
> > >  
> > > +	i915_vma_get(vma);
> > 
> > I'm not entirely clear on why we no need a refcount for this? Vestige code
> > from when you wanted to refcount plane_state->fb like other refcounted
> > state pointers? If we restrict the lifetime to just in between
> > prepare_plane/cleanup_plane like you do I don't think this is needed ...
> 
> The lifetime is quite broad since we aquire it on takeover and it is not
> finally released until plane state destruction. It's not as simple as
> saying they only exist between two atomic commits :| Since the vma is
> being tracked outside of the parent fb (and it is independent since it
> is a view of that fb), it is safer to hold a reference.

Takeover = prepare_planes done by someone else. And assuming our current
refcounting is correct we shouldn't need this additional refcount either.

> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index fff55d93ca73..d3d26d69930a 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -372,6 +372,7 @@ struct intel_atomic_state {
> > >  struct intel_plane_state {
> > >  	struct drm_plane_state base;
> > >  	struct drm_rect clip;
> > 
> > I think a comment here about the special refcounting rules for this would
> > be good, e.g.
> > 
> > "Note that despite that vmas are refcounted using i915_vma_get() and
> > i915_vma_put() we don't refcount them as part of the state like other
> > objects. Instead their lifetime is only between prepare_planes and
> > cleanup_planes, and hence vma must be set to NULL when duplicating the
> > plane state."
> 
> That would be false :-p

What's the false part? Ignoring the takeover corner case, where we have
lots of other cases where we need to adjust refcounts and pointers and
make sure it all looks like boot-up never happened, and it matches state
that a normal atomic commit could have produced.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-11-15  9:52   ` Daniel Vetter
@ 2016-11-15 10:02     ` Chris Wilson
  2016-11-15 10:13       ` Daniel Vetter
  0 siblings, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-11-15 10:02 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Nov 15, 2016 at 10:52:00AM +0100, Daniel Vetter wrote:
> On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote:
> > With atomic plane states we are able to track an allocation right from
> > preparation, during use and through to the final free after being
> > swapped out for a new plane. We can couple the VMA we pin for the
> > framebuffer (and its rotation) to this lifetime and avoid all the clumsy
> > lookups in between.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> A few questions and comments below. Of course review assumes some form of
> patch 2 lands first (but leaning towards just reverting the core one for
> that).
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h           | 16 ++---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +
> >  drivers/gpu/drm/i915/intel_display.c      | 97 +++++++++++++------------------
> >  drivers/gpu/drm/i915/intel_drv.h          |  9 ++-
> >  drivers/gpu/drm/i915/intel_fbc.c          | 52 +++++++----------
> >  drivers/gpu/drm/i915/intel_fbdev.c        |  4 +-
> >  drivers/gpu/drm/i915/intel_sprite.c       |  8 +--
> >  7 files changed, 78 insertions(+), 110 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4e7148a3ee8b..2bc285e2ff82 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -989,6 +989,8 @@ struct intel_fbc {
> >  	struct work_struct underrun_work;
> >  
> >  	struct intel_fbc_state_cache {
> > +		struct i915_vma *vma;
> > +
> >  		struct {
> >  			unsigned int mode_flags;
> >  			uint32_t hsw_bdw_pixel_rate;
> > @@ -1002,15 +1004,14 @@ struct intel_fbc {
> >  		} plane;
> >  
> >  		struct {
> > -			u64 ilk_ggtt_offset;
> >  			uint32_t pixel_format;
> >  			unsigned int stride;
> > -			int fence_reg;
> > -			unsigned int tiling_mode;
> >  		} fb;
> >  	} state_cache;
> >  
> >  	struct intel_fbc_reg_params {
> > +		struct i915_vma *vma;
> > +
> >  		struct {
> >  			enum pipe pipe;
> >  			enum plane plane;
> > @@ -1018,10 +1019,8 @@ struct intel_fbc {
> >  		} crtc;
> >  
> >  		struct {
> > -			u64 ggtt_offset;
> >  			uint32_t pixel_format;
> >  			unsigned int stride;
> > -			int fence_reg;
> >  		} fb;
> >  
> >  		int cfb_size;
> > @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
> >  	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
> >  }
> >  
> > -static inline unsigned long
> > -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
> > -			    const struct i915_ggtt_view *view)
> > -{
> > -	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
> > -}
> > -
> >  /* i915_gem_fence_reg.c */
> >  int __must_check i915_vma_get_fence(struct i915_vma *vma);
> >  int __must_check i915_vma_put_fence(struct i915_vma *vma);
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index 89a34350a35d..5a86e0d52409 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
> >  
> >  	__drm_atomic_helper_plane_duplicate_state(plane, state);
> >  
> > +	intel_state->vma = NULL;
> > +
> >  	return state;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dca1e0b512e5..cb52116f8577 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> >  			i915_vma_pin_fence(vma);
> >  	}
> >  
> > +	i915_vma_get(vma);
> 
> I'm not entirely clear on why we no need a refcount for this? Vestige code
> from when you wanted to refcount plane_state->fb like other refcounted
> state pointers? If we restrict the lifetime to just in between
> prepare_plane/cleanup_plane like you do I don't think this is needed ...

The lifetime is quite broad since we aquire it on takeover and it is not
finally released until plane state destruction. It's not as simple as
saying they only exist between two atomic commits :| Since the vma is
being tracked outside of the parent fb (and it is independent since it
is a view of that fb), it is safer to hold a reference.

> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index fff55d93ca73..d3d26d69930a 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -372,6 +372,7 @@ struct intel_atomic_state {
> >  struct intel_plane_state {
> >  	struct drm_plane_state base;
> >  	struct drm_rect clip;
> 
> I think a comment here about the special refcounting rules for this would
> be good, e.g.
> 
> "Note that despite that vmas are refcounted using i915_vma_get() and
> i915_vma_put() we don't refcount them as part of the state like other
> objects. Instead their lifetime is only between prepare_planes and
> cleanup_planes, and hence vma must be set to NULL when duplicating the
> plane state."

That would be false :-p
-Chris

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

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

* Re: [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-11-15  8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
@ 2016-11-15  9:52   ` Daniel Vetter
  2016-11-15 10:02     ` Chris Wilson
  2016-11-15 10:24   ` Daniel Vetter
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-11-15  9:52 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Tue, Nov 15, 2016 at 08:58:15AM +0000, Chris Wilson wrote:
> With atomic plane states we are able to track an allocation right from
> preparation, during use and through to the final free after being
> swapped out for a new plane. We can couple the VMA we pin for the
> framebuffer (and its rotation) to this lifetime and avoid all the clumsy
> lookups in between.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

A few questions and comments below. Of course review assumes some form of
patch 2 lands first (but leaning towards just reverting the core one for
that).
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h           | 16 ++---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +
>  drivers/gpu/drm/i915/intel_display.c      | 97 +++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h          |  9 ++-
>  drivers/gpu/drm/i915/intel_fbc.c          | 52 +++++++----------
>  drivers/gpu/drm/i915/intel_fbdev.c        |  4 +-
>  drivers/gpu/drm/i915/intel_sprite.c       |  8 +--
>  7 files changed, 78 insertions(+), 110 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4e7148a3ee8b..2bc285e2ff82 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -989,6 +989,8 @@ struct intel_fbc {
>  	struct work_struct underrun_work;
>  
>  	struct intel_fbc_state_cache {
> +		struct i915_vma *vma;
> +
>  		struct {
>  			unsigned int mode_flags;
>  			uint32_t hsw_bdw_pixel_rate;
> @@ -1002,15 +1004,14 @@ struct intel_fbc {
>  		} plane;
>  
>  		struct {
> -			u64 ilk_ggtt_offset;
>  			uint32_t pixel_format;
>  			unsigned int stride;
> -			int fence_reg;
> -			unsigned int tiling_mode;
>  		} fb;
>  	} state_cache;
>  
>  	struct intel_fbc_reg_params {
> +		struct i915_vma *vma;
> +
>  		struct {
>  			enum pipe pipe;
>  			enum plane plane;
> @@ -1018,10 +1019,8 @@ struct intel_fbc {
>  		} crtc;
>  
>  		struct {
> -			u64 ggtt_offset;
>  			uint32_t pixel_format;
>  			unsigned int stride;
> -			int fence_reg;
>  		} fb;
>  
>  		int cfb_size;
> @@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
>  	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
>  }
>  
> -static inline unsigned long
> -i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
> -			    const struct i915_ggtt_view *view)
> -{
> -	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
> -}
> -
>  /* i915_gem_fence_reg.c */
>  int __must_check i915_vma_get_fence(struct i915_vma *vma);
>  int __must_check i915_vma_put_fence(struct i915_vma *vma);
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index 89a34350a35d..5a86e0d52409 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
>  
>  	__drm_atomic_helper_plane_duplicate_state(plane, state);
>  
> +	intel_state->vma = NULL;
> +
>  	return state;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dca1e0b512e5..cb52116f8577 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
>  			i915_vma_pin_fence(vma);
>  	}
>  
> +	i915_vma_get(vma);

I'm not entirely clear on why we no need a refcount for this? Vestige code
from when you wanted to refcount plane_state->fb like other refcounted
state pointers? If we restrict the lifetime to just in between
prepare_plane/cleanup_plane like you do I don't think this is needed ...

>  err:
>  	intel_runtime_pm_put(dev_priv);
>  	return vma;
>  }
>  
> -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> +void intel_unpin_fb_vma(struct i915_vma *vma)
>  {
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct i915_ggtt_view view;
> -	struct i915_vma *vma;
> -
> -	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
> -
> -	intel_fill_fb_ggtt_view(&view, fb, rotation);
> -	vma = i915_gem_object_to_ggtt(obj, &view);
> +	lockdep_assert_held(&vma->vm->dev->struct_mutex);
>  
>  	i915_vma_unpin_fence(vma);
>  	i915_gem_object_unpin_from_display_plane(vma);
> +	i915_vma_put(vma);
>  }
>  
>  static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane,
> @@ -2752,7 +2747,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct drm_crtc *c;
> -	struct intel_crtc *i;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_plane *primary = intel_crtc->base.primary;
>  	struct drm_plane_state *plane_state = primary->state;
> @@ -2777,20 +2771,20 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	 * an fb with another CRTC instead
>  	 */
>  	for_each_crtc(dev, c) {
> -		i = to_intel_crtc(c);
> +		struct intel_plane_state *state;
>  
>  		if (c == &intel_crtc->base)
>  			continue;
>  
> -		if (!i->active)
> +		if (!to_intel_crtc(c)->active)
>  			continue;
>  
> -		fb = c->primary->fb;
> -		if (!fb)
> +		state = to_intel_plane_state(c->primary->state);
> +		if (!state->vma)
>  			continue;
>  
> -		obj = intel_fb_obj(fb);
> -		if (i915_gem_object_ggtt_offset(obj, NULL) == plane_config->base) {
> +		if (i915_ggtt_offset(state->vma) == plane_config->base) {
> +			fb = c->primary->fb;
>  			drm_framebuffer_reference(fb);
>  			goto valid_fb;
>  		}
> @@ -2830,6 +2824,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  
>  	drm_framebuffer_reference(fb);
>  	primary->fb = primary->state->fb = fb;
> +
> +	mutex_lock(&dev->struct_mutex);
> +	intel_state->vma =
> +		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
> +	mutex_unlock(&dev->struct_mutex);
> +
>  	primary->crtc = primary->state->crtc = &intel_crtc->base;
>  	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
>  	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
> @@ -3104,13 +3104,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
>  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
>  	if (INTEL_INFO(dev)->gen >= 4) {
>  		I915_WRITE(DSPSURF(plane),
> -			   intel_fb_gtt_offset(fb, rotation) +
> +			   intel_plane_ggtt_offset(plane_state) +
>  			   intel_crtc->dspaddr_offset);
>  		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
>  		I915_WRITE(DSPLINOFF(plane), linear_offset);
>  	} else {
>  		I915_WRITE(DSPADDR(plane),
> -			   intel_fb_gtt_offset(fb, rotation) +
> +			   intel_plane_ggtt_offset(plane_state) +
>  			   intel_crtc->dspaddr_offset);
>  	}
>  	POSTING_READ(reg);
> @@ -3207,7 +3207,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
>  
>  	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
>  	I915_WRITE(DSPSURF(plane),
> -		   intel_fb_gtt_offset(fb, rotation) +
> +		   intel_plane_ggtt_offset(plane_state) +
>  		   intel_crtc->dspaddr_offset);
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
> @@ -3230,23 +3230,6 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
> -			unsigned int rotation)
> -{
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct i915_ggtt_view view;
> -	struct i915_vma *vma;
> -
> -	intel_fill_fb_ggtt_view(&view, fb, rotation);
> -
> -	vma = i915_gem_object_to_ggtt(obj, &view);
> -	if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
> -		 view.type))
> -		return -1;
> -
> -	return i915_ggtt_offset(vma);
> -}
> -
>  static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
> @@ -3447,7 +3430,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
>  	}
>  
>  	I915_WRITE(PLANE_SURF(pipe, 0),
> -		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
> +		   intel_plane_ggtt_offset(plane_state) + surf_addr);
>  
>  	POSTING_READ(PLANE_SURF(pipe, 0));
>  }
> @@ -11576,7 +11559,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  		flush_work(&work->mmio_work);
>  
>  	mutex_lock(&dev->struct_mutex);
> -	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
> +	intel_unpin_fb_vma(work->old_vma);
>  	i915_gem_object_put(work->pending_flip_obj);
>  	mutex_unlock(&dev->struct_mutex);
>  
> @@ -12286,8 +12269,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		goto cleanup_pending;
>  	}
>  
> -	work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation);
> -	work->gtt_offset += intel_crtc->dspaddr_offset;
> +	work->old_vma = to_intel_plane_state(primary->state)->vma;
> +	to_intel_plane_state(primary->state)->vma = vma;
> +
> +	work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset;
>  	work->rotation = crtc->primary->state->rotation;
>  
>  	/*
> @@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  cleanup_request:
>  	i915_add_request_no_flush(request);
>  cleanup_unpin:
> -	intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
> +	to_intel_plane_state(primary->state)->vma = work->old_vma;
> +	intel_unpin_fb_vma(vma);
>  cleanup_pending:
>  	atomic_dec(&intel_crtc->unpin_work_count);
>  	mutex_unlock(&dev->struct_mutex);
> @@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane,
>  		struct i915_vma *vma;
>  
>  		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> -		if (IS_ERR(vma)) {
> -			DRM_DEBUG_KMS("failed to pin object\n");
> -			return PTR_ERR(vma);
> -		}
> +		if (IS_ERR(vma))
> +			ret = PTR_ERR(vma);
> +
> +		to_intel_plane_state(new_state)->vma = vma;
>  	}
>  
>  	return 0;
> @@ -14256,19 +14242,12 @@ static void
>  intel_cleanup_plane_fb(struct drm_plane *plane,
>  		       struct drm_plane_state *old_state)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(plane->dev);
> -	struct intel_plane_state *old_intel_state;
> -	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
> -	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
> -
> -	old_intel_state = to_intel_plane_state(old_state);
> -
> -	if (!obj && !old_obj)
> -		return;
> +	struct i915_vma *vma;
>  
> -	if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
> -	    !INTEL_INFO(dev_priv)->cursor_needs_physical))
> -		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
> +	/* Should only called after a successful intel_prepare_plane_fb()! */
> +	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
> +	if (vma)

WARN_ON(!vma); maybe even instead of the comment?

> +		intel_unpin_fb_vma(vma);
>  }
>  
>  static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state)
> @@ -15215,7 +15194,7 @@ intel_update_cursor_plane(struct drm_plane *plane,
>  	if (!obj)
>  		addr = 0;
>  	else if (!INTEL_INFO(dev_priv)->cursor_needs_physical)
> -		addr = i915_gem_object_ggtt_offset(obj, NULL);
> +		addr = i915_ggtt_offset(state->vma);
>  	else
>  		addr = obj->phys_handle->busaddr;
>  
> @@ -17135,6 +17114,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
>  			update_state_fb(c->primary);
>  			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
>  		}
> +
> +		to_intel_plane_state(c->primary->state)->vma = vma;
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index fff55d93ca73..d3d26d69930a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -372,6 +372,7 @@ struct intel_atomic_state {
>  struct intel_plane_state {
>  	struct drm_plane_state base;
>  	struct drm_rect clip;

I think a comment here about the special refcounting rules for this would
be good, e.g.

"Note that despite that vmas are refcounted using i915_vma_get() and
i915_vma_put() we don't refcount them as part of the state like other
objects. Instead their lifetime is only between prepare_planes and
cleanup_planes, and hence vma must be set to NULL when duplicating the
plane state."

> +	struct i915_vma *vma;
>  
>  	struct {
>  		u32 offset;
> @@ -1046,6 +1047,7 @@ struct intel_flip_work {
>  	struct work_struct mmio_work;
>  
>  	struct drm_crtc *crtc;
> +	struct i915_vma *old_vma;
>  	struct drm_framebuffer *old_fb;
>  	struct drm_i915_gem_object *pending_flip_obj;
>  	struct drm_pending_vblank_event *event;
> @@ -1273,7 +1275,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
>  				    struct drm_modeset_acquire_ctx *ctx);
>  struct i915_vma *
>  intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
> -void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
> +void intel_unpin_fb_vma(struct i915_vma *vma);
>  struct drm_framebuffer *
>  __intel_framebuffer_create(struct drm_device *dev,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -1358,7 +1360,10 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
>  int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
>  
> -u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
> +static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> +{
> +	return i915_ggtt_offset(state->vma);
> +}
>  
>  u32 skl_plane_ctl_format(uint32_t pixel_format);
>  u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 62f215b12eb5..f3a1d6a5cabe 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -173,7 +173,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
>  	if (IS_I945GM(dev_priv))
>  		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
>  	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
> -	fbc_ctl |= params->fb.fence_reg;
> +	fbc_ctl |= params->vma->fence->id;
>  	I915_WRITE(FBC_CONTROL, fbc_ctl);
>  }
>  
> @@ -193,8 +193,8 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
>  	else
>  		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
>  
> -	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> -		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg;
> +	if (params->vma->fence) {
> +		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id;
>  		I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
>  	} else {
>  		I915_WRITE(DPFC_FENCE_YOFF, 0);
> @@ -251,13 +251,14 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> -	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> +	if (params->vma->fence) {
>  		dpfc_ctl |= DPFC_CTL_FENCE_EN;
>  		if (IS_GEN5(dev_priv))
> -			dpfc_ctl |= params->fb.fence_reg;
> +			dpfc_ctl |= params->vma->fence->id;
>  		if (IS_GEN6(dev_priv)) {
>  			I915_WRITE(SNB_DPFC_CTL_SA,
> -				   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
> +				   SNB_CPU_FENCE_ENABLE |
> +				   params->vma->fence->id);
>  			I915_WRITE(DPFC_CPU_FENCE_OFFSET,
>  				   params->crtc.fence_y_offset);
>  		}
> @@ -269,7 +270,8 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
>  	}
>  
>  	I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
> -	I915_WRITE(ILK_FBC_RT_BASE, params->fb.ggtt_offset | ILK_FBC_RT_VALID);
> +	I915_WRITE(ILK_FBC_RT_BASE,
> +		   i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID);
>  	/* enable it... */
>  	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
>  
> @@ -319,10 +321,11 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
>  		break;
>  	}
>  
> -	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
> +	if (params->vma->fence) {
>  		dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
>  		I915_WRITE(SNB_DPFC_CTL_SA,
> -			   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
> +			   SNB_CPU_FENCE_ENABLE |
> +			   params->vma->fence->id);
>  		I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);
>  	} else {
>  		I915_WRITE(SNB_DPFC_CTL_SA,0);
> @@ -727,14 +730,6 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
>  	return effective_w <= max_w && effective_h <= max_h;
>  }
>  
> -/* XXX replace me when we have VMA tracking for intel_plane_state */
> -static int get_fence_id(struct drm_framebuffer *fb)
> -{
> -	struct i915_vma *vma = i915_gem_object_to_ggtt(intel_fb_obj(fb), NULL);
> -
> -	return vma && vma->fence ? vma->fence->id : I915_FENCE_REG_NONE;
> -}
> -
>  static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  					 struct intel_crtc_state *crtc_state,
>  					 struct intel_plane_state *plane_state)
> @@ -743,7 +738,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  	struct intel_fbc_state_cache *cache = &fbc->state_cache;
>  	struct drm_framebuffer *fb = plane_state->base.fb;
> -	struct drm_i915_gem_object *obj;
> +
> +	cache->vma = NULL;
>  
>  	cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags;
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> @@ -758,16 +754,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
>  	if (!cache->plane.visible)
>  		return;
>  
> -	obj = intel_fb_obj(fb);
> -
> -	/* FIXME: We lack the proper locking here, so only run this on the
> -	 * platforms that need. */
> -	if (IS_GEN(dev_priv, 5, 6))
> -		cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL);
>  	cache->fb.pixel_format = fb->pixel_format;
>  	cache->fb.stride = fb->pitches[0];
> -	cache->fb.fence_reg = get_fence_id(fb);
> -	cache->fb.tiling_mode = i915_gem_object_get_tiling(obj);
> +
> +	cache->vma = plane_state->vma;
>  }
>  
>  static bool intel_fbc_can_activate(struct intel_crtc *crtc)
> @@ -784,7 +774,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  		return false;
>  	}
>  
> -	if (!cache->plane.visible) {
> +	if (!cache->vma) {
>  		fbc->no_fbc_reason = "primary plane not visible";
>  		return false;
>  	}
> @@ -807,8 +797,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
>  	 * so have no fence associated with it) due to aperture constaints
>  	 * at the time of pinning.
>  	 */
> -	if (cache->fb.tiling_mode != I915_TILING_X ||
> -	    cache->fb.fence_reg == I915_FENCE_REG_NONE) {
> +	if (!cache->vma->fence) {
>  		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
>  		return false;
>  	}
> @@ -888,17 +877,16 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
>  	 * zero. */
>  	memset(params, 0, sizeof(*params));
>  
> +	params->vma = cache->vma;
> +
>  	params->crtc.pipe = crtc->pipe;
>  	params->crtc.plane = crtc->plane;
>  	params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
>  
>  	params->fb.pixel_format = cache->fb.pixel_format;
>  	params->fb.stride = cache->fb.stride;
> -	params->fb.fence_reg = cache->fb.fence_reg;
>  
>  	params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache);
> -
> -	params->fb.ggtt_offset = cache->fb.ilk_ggtt_offset;
>  }
>  
>  static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index fc958d5ed0dc..ca29a5314f0e 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -284,7 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  out_destroy_fbi:
>  	drm_fb_helper_release_fbi(helper);
>  out_unpin:
> -	intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
> +	intel_unpin_fb_vma(vma);
>  out_unlock:
>  	mutex_unlock(&dev->struct_mutex);
>  	return ret;
> @@ -549,7 +549,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>  
>  	if (ifbdev->fb) {
>  		mutex_lock(&ifbdev->helper.dev->struct_mutex);
> -		intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
> +		intel_unpin_fb_vma(ifbdev->vma);
>  		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
>  
>  		drm_framebuffer_remove(&ifbdev->fb->base);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 8b2fc67acbba..ae8ce621e3fc 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -281,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane,
>  
>  	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
>  	I915_WRITE(PLANE_SURF(pipe, plane),
> -		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
> +		   intel_plane_ggtt_offset(plane_state) + surf_addr);
>  	POSTING_READ(PLANE_SURF(pipe, plane));
>  }
>  
> @@ -476,7 +476,7 @@ vlv_update_plane(struct drm_plane *dplane,
>  	I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w);
>  	I915_WRITE(SPCNTR(pipe, plane), sprctl);
>  	I915_WRITE(SPSURF(pipe, plane),
> -		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
> +		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
>  	POSTING_READ(SPSURF(pipe, plane));
>  }
>  
> @@ -612,7 +612,7 @@ ivb_update_plane(struct drm_plane *plane,
>  		I915_WRITE(SPRSCALE(pipe), sprscale);
>  	I915_WRITE(SPRCTL(pipe), sprctl);
>  	I915_WRITE(SPRSURF(pipe),
> -		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
> +		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
>  	POSTING_READ(SPRSURF(pipe));
>  }
>  
> @@ -739,7 +739,7 @@ ilk_update_plane(struct drm_plane *plane,
>  	I915_WRITE(DVSSCALE(pipe), dvsscale);
>  	I915_WRITE(DVSCNTR(pipe), dvscntr);
>  	I915_WRITE(DVSSURF(pipe),
> -		   intel_fb_gtt_offset(fb, rotation) + dvssurf_offset);
> +		   intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
>  	POSTING_READ(DVSSURF(pipe));
>  }
>  
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state
  2016-11-15  8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
@ 2016-11-15  8:58 ` Chris Wilson
  2016-11-15  9:52   ` Daniel Vetter
  2016-11-15 10:24   ` Daniel Vetter
  0 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2016-11-15  8:58 UTC (permalink / raw)
  To: intel-gfx

With atomic plane states we are able to track an allocation right from
preparation, during use and through to the final free after being
swapped out for a new plane. We can couple the VMA we pin for the
framebuffer (and its rotation) to this lifetime and avoid all the clumsy
lookups in between.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h           | 16 ++---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  2 +
 drivers/gpu/drm/i915/intel_display.c      | 97 +++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h          |  9 ++-
 drivers/gpu/drm/i915/intel_fbc.c          | 52 +++++++----------
 drivers/gpu/drm/i915/intel_fbdev.c        |  4 +-
 drivers/gpu/drm/i915/intel_sprite.c       |  8 +--
 7 files changed, 78 insertions(+), 110 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e7148a3ee8b..2bc285e2ff82 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -989,6 +989,8 @@ struct intel_fbc {
 	struct work_struct underrun_work;
 
 	struct intel_fbc_state_cache {
+		struct i915_vma *vma;
+
 		struct {
 			unsigned int mode_flags;
 			uint32_t hsw_bdw_pixel_rate;
@@ -1002,15 +1004,14 @@ struct intel_fbc {
 		} plane;
 
 		struct {
-			u64 ilk_ggtt_offset;
 			uint32_t pixel_format;
 			unsigned int stride;
-			int fence_reg;
-			unsigned int tiling_mode;
 		} fb;
 	} state_cache;
 
 	struct intel_fbc_reg_params {
+		struct i915_vma *vma;
+
 		struct {
 			enum pipe pipe;
 			enum plane plane;
@@ -1018,10 +1019,8 @@ struct intel_fbc {
 		} crtc;
 
 		struct {
-			u64 ggtt_offset;
 			uint32_t pixel_format;
 			unsigned int stride;
-			int fence_reg;
 		} fb;
 
 		int cfb_size;
@@ -3150,13 +3149,6 @@ i915_gem_object_to_ggtt(struct drm_i915_gem_object *obj,
 	return i915_gem_obj_to_vma(obj, &to_i915(obj->base.dev)->ggtt.base, view);
 }
 
-static inline unsigned long
-i915_gem_object_ggtt_offset(struct drm_i915_gem_object *o,
-			    const struct i915_ggtt_view *view)
-{
-	return i915_ggtt_offset(i915_gem_object_to_ggtt(o, view));
-}
-
 /* i915_gem_fence_reg.c */
 int __must_check i915_vma_get_fence(struct i915_vma *vma);
 int __must_check i915_vma_put_fence(struct i915_vma *vma);
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 89a34350a35d..5a86e0d52409 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -85,6 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 
 	__drm_atomic_helper_plane_duplicate_state(plane, state);
 
+	intel_state->vma = NULL;
+
 	return state;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dca1e0b512e5..cb52116f8577 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2241,24 +2241,19 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
 			i915_vma_pin_fence(vma);
 	}
 
+	i915_vma_get(vma);
 err:
 	intel_runtime_pm_put(dev_priv);
 	return vma;
 }
 
-void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
+void intel_unpin_fb_vma(struct i915_vma *vma)
 {
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct i915_ggtt_view view;
-	struct i915_vma *vma;
-
-	WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
-
-	intel_fill_fb_ggtt_view(&view, fb, rotation);
-	vma = i915_gem_object_to_ggtt(obj, &view);
+	lockdep_assert_held(&vma->vm->dev->struct_mutex);
 
 	i915_vma_unpin_fence(vma);
 	i915_gem_object_unpin_from_display_plane(vma);
+	i915_vma_put(vma);
 }
 
 static int intel_fb_pitch(const struct drm_framebuffer *fb, int plane,
@@ -2752,7 +2747,6 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *c;
-	struct intel_crtc *i;
 	struct drm_i915_gem_object *obj;
 	struct drm_plane *primary = intel_crtc->base.primary;
 	struct drm_plane_state *plane_state = primary->state;
@@ -2777,20 +2771,20 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	 * an fb with another CRTC instead
 	 */
 	for_each_crtc(dev, c) {
-		i = to_intel_crtc(c);
+		struct intel_plane_state *state;
 
 		if (c == &intel_crtc->base)
 			continue;
 
-		if (!i->active)
+		if (!to_intel_crtc(c)->active)
 			continue;
 
-		fb = c->primary->fb;
-		if (!fb)
+		state = to_intel_plane_state(c->primary->state);
+		if (!state->vma)
 			continue;
 
-		obj = intel_fb_obj(fb);
-		if (i915_gem_object_ggtt_offset(obj, NULL) == plane_config->base) {
+		if (i915_ggtt_offset(state->vma) == plane_config->base) {
+			fb = c->primary->fb;
 			drm_framebuffer_reference(fb);
 			goto valid_fb;
 		}
@@ -2830,6 +2824,12 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 
 	drm_framebuffer_reference(fb);
 	primary->fb = primary->state->fb = fb;
+
+	mutex_lock(&dev->struct_mutex);
+	intel_state->vma =
+		intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+	mutex_unlock(&dev->struct_mutex);
+
 	primary->crtc = primary->state->crtc = &intel_crtc->base;
 	intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary));
 	atomic_or(to_intel_plane(primary)->frontbuffer_bit,
@@ -3104,13 +3104,13 @@ static void i9xx_update_primary_plane(struct drm_plane *primary,
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	if (INTEL_INFO(dev)->gen >= 4) {
 		I915_WRITE(DSPSURF(plane),
-			   intel_fb_gtt_offset(fb, rotation) +
+			   intel_plane_ggtt_offset(plane_state) +
 			   intel_crtc->dspaddr_offset);
 		I915_WRITE(DSPTILEOFF(plane), (y << 16) | x);
 		I915_WRITE(DSPLINOFF(plane), linear_offset);
 	} else {
 		I915_WRITE(DSPADDR(plane),
-			   intel_fb_gtt_offset(fb, rotation) +
+			   intel_plane_ggtt_offset(plane_state) +
 			   intel_crtc->dspaddr_offset);
 	}
 	POSTING_READ(reg);
@@ -3207,7 +3207,7 @@ static void ironlake_update_primary_plane(struct drm_plane *primary,
 
 	I915_WRITE(DSPSTRIDE(plane), fb->pitches[0]);
 	I915_WRITE(DSPSURF(plane),
-		   intel_fb_gtt_offset(fb, rotation) +
+		   intel_plane_ggtt_offset(plane_state) +
 		   intel_crtc->dspaddr_offset);
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
 		I915_WRITE(DSPOFFSET(plane), (y << 16) | x);
@@ -3230,23 +3230,6 @@ u32 intel_fb_stride_alignment(const struct drm_i915_private *dev_priv,
 	}
 }
 
-u32 intel_fb_gtt_offset(struct drm_framebuffer *fb,
-			unsigned int rotation)
-{
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct i915_ggtt_view view;
-	struct i915_vma *vma;
-
-	intel_fill_fb_ggtt_view(&view, fb, rotation);
-
-	vma = i915_gem_object_to_ggtt(obj, &view);
-	if (WARN(!vma, "ggtt vma for display object not found! (view=%u)\n",
-		 view.type))
-		return -1;
-
-	return i915_ggtt_offset(vma);
-}
-
 static void skl_detach_scaler(struct intel_crtc *intel_crtc, int id)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
@@ -3447,7 +3430,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane,
 	}
 
 	I915_WRITE(PLANE_SURF(pipe, 0),
-		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
+		   intel_plane_ggtt_offset(plane_state) + surf_addr);
 
 	POSTING_READ(PLANE_SURF(pipe, 0));
 }
@@ -11576,7 +11559,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 		flush_work(&work->mmio_work);
 
 	mutex_lock(&dev->struct_mutex);
-	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
+	intel_unpin_fb_vma(work->old_vma);
 	i915_gem_object_put(work->pending_flip_obj);
 	mutex_unlock(&dev->struct_mutex);
 
@@ -12286,8 +12269,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		goto cleanup_pending;
 	}
 
-	work->gtt_offset = intel_fb_gtt_offset(fb, primary->state->rotation);
-	work->gtt_offset += intel_crtc->dspaddr_offset;
+	work->old_vma = to_intel_plane_state(primary->state)->vma;
+	to_intel_plane_state(primary->state)->vma = vma;
+
+	work->gtt_offset = i915_ggtt_offset(vma) + intel_crtc->dspaddr_offset;
 	work->rotation = crtc->primary->state->rotation;
 
 	/*
@@ -12340,7 +12325,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 cleanup_request:
 	i915_add_request_no_flush(request);
 cleanup_unpin:
-	intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
+	to_intel_plane_state(primary->state)->vma = work->old_vma;
+	intel_unpin_fb_vma(vma);
 cleanup_pending:
 	atomic_dec(&intel_crtc->unpin_work_count);
 	mutex_unlock(&dev->struct_mutex);
@@ -14234,10 +14220,10 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		struct i915_vma *vma;
 
 		vma = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
-		if (IS_ERR(vma)) {
-			DRM_DEBUG_KMS("failed to pin object\n");
-			return PTR_ERR(vma);
-		}
+		if (IS_ERR(vma))
+			ret = PTR_ERR(vma);
+
+		to_intel_plane_state(new_state)->vma = vma;
 	}
 
 	return 0;
@@ -14256,19 +14242,12 @@ static void
 intel_cleanup_plane_fb(struct drm_plane *plane,
 		       struct drm_plane_state *old_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(plane->dev);
-	struct intel_plane_state *old_intel_state;
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
-	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
-
-	old_intel_state = to_intel_plane_state(old_state);
-
-	if (!obj && !old_obj)
-		return;
+	struct i915_vma *vma;
 
-	if (old_obj && (plane->type != DRM_PLANE_TYPE_CURSOR ||
-	    !INTEL_INFO(dev_priv)->cursor_needs_physical))
-		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
+	/* Should only called after a successful intel_prepare_plane_fb()! */
+	vma = fetch_and_zero(&to_intel_plane_state(old_state)->vma);
+	if (vma)
+		intel_unpin_fb_vma(vma);
 }
 
 static int intel_atomic_commit_prepare_planes(struct drm_atomic_state *state)
@@ -15215,7 +15194,7 @@ intel_update_cursor_plane(struct drm_plane *plane,
 	if (!obj)
 		addr = 0;
 	else if (!INTEL_INFO(dev_priv)->cursor_needs_physical)
-		addr = i915_gem_object_ggtt_offset(obj, NULL);
+		addr = i915_ggtt_offset(state->vma);
 	else
 		addr = obj->phys_handle->busaddr;
 
@@ -17135,6 +17114,8 @@ void intel_modeset_gem_init(struct drm_device *dev)
 			update_state_fb(c->primary);
 			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
 		}
+
+		to_intel_plane_state(c->primary->state)->vma = vma;
 	}
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fff55d93ca73..d3d26d69930a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -372,6 +372,7 @@ struct intel_atomic_state {
 struct intel_plane_state {
 	struct drm_plane_state base;
 	struct drm_rect clip;
+	struct i915_vma *vma;
 
 	struct {
 		u32 offset;
@@ -1046,6 +1047,7 @@ struct intel_flip_work {
 	struct work_struct mmio_work;
 
 	struct drm_crtc *crtc;
+	struct i915_vma *old_vma;
 	struct drm_framebuffer *old_fb;
 	struct drm_i915_gem_object *pending_flip_obj;
 	struct drm_pending_vblank_event *event;
@@ -1273,7 +1275,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 				    struct drm_modeset_acquire_ctx *ctx);
 struct i915_vma *
 intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
-void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation);
+void intel_unpin_fb_vma(struct i915_vma *vma);
 struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
@@ -1358,7 +1360,10 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
 
-u32 intel_fb_gtt_offset(struct drm_framebuffer *fb, unsigned int rotation);
+static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
+{
+	return i915_ggtt_offset(state->vma);
+}
 
 u32 skl_plane_ctl_format(uint32_t pixel_format);
 u32 skl_plane_ctl_tiling(uint64_t fb_modifier);
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 62f215b12eb5..f3a1d6a5cabe 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -173,7 +173,7 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 	if (IS_I945GM(dev_priv))
 		fbc_ctl |= FBC_CTL_C3_IDLE; /* 945 needs special SR handling */
 	fbc_ctl |= (cfb_pitch & 0xff) << FBC_CTL_STRIDE_SHIFT;
-	fbc_ctl |= params->fb.fence_reg;
+	fbc_ctl |= params->vma->fence->id;
 	I915_WRITE(FBC_CONTROL, fbc_ctl);
 }
 
@@ -193,8 +193,8 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
 	else
 		dpfc_ctl |= DPFC_CTL_LIMIT_1X;
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
-		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->fb.fence_reg;
+	if (params->vma->fence) {
+		dpfc_ctl |= DPFC_CTL_FENCE_EN | params->vma->fence->id;
 		I915_WRITE(DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
 	} else {
 		I915_WRITE(DPFC_FENCE_YOFF, 0);
@@ -251,13 +251,14 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
+	if (params->vma->fence) {
 		dpfc_ctl |= DPFC_CTL_FENCE_EN;
 		if (IS_GEN5(dev_priv))
-			dpfc_ctl |= params->fb.fence_reg;
+			dpfc_ctl |= params->vma->fence->id;
 		if (IS_GEN6(dev_priv)) {
 			I915_WRITE(SNB_DPFC_CTL_SA,
-				   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
+				   SNB_CPU_FENCE_ENABLE |
+				   params->vma->fence->id);
 			I915_WRITE(DPFC_CPU_FENCE_OFFSET,
 				   params->crtc.fence_y_offset);
 		}
@@ -269,7 +270,8 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 	}
 
 	I915_WRITE(ILK_DPFC_FENCE_YOFF, params->crtc.fence_y_offset);
-	I915_WRITE(ILK_FBC_RT_BASE, params->fb.ggtt_offset | ILK_FBC_RT_VALID);
+	I915_WRITE(ILK_FBC_RT_BASE,
+		   i915_ggtt_offset(params->vma) | ILK_FBC_RT_VALID);
 	/* enable it... */
 	I915_WRITE(ILK_DPFC_CONTROL, dpfc_ctl | DPFC_CTL_EN);
 
@@ -319,10 +321,11 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 		break;
 	}
 
-	if (params->fb.fence_reg != I915_FENCE_REG_NONE) {
+	if (params->vma->fence) {
 		dpfc_ctl |= IVB_DPFC_CTL_FENCE_EN;
 		I915_WRITE(SNB_DPFC_CTL_SA,
-			   SNB_CPU_FENCE_ENABLE | params->fb.fence_reg);
+			   SNB_CPU_FENCE_ENABLE |
+			   params->vma->fence->id);
 		I915_WRITE(DPFC_CPU_FENCE_OFFSET, params->crtc.fence_y_offset);
 	} else {
 		I915_WRITE(SNB_DPFC_CTL_SA,0);
@@ -727,14 +730,6 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 	return effective_w <= max_w && effective_h <= max_h;
 }
 
-/* XXX replace me when we have VMA tracking for intel_plane_state */
-static int get_fence_id(struct drm_framebuffer *fb)
-{
-	struct i915_vma *vma = i915_gem_object_to_ggtt(intel_fb_obj(fb), NULL);
-
-	return vma && vma->fence ? vma->fence->id : I915_FENCE_REG_NONE;
-}
-
 static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 					 struct intel_crtc_state *crtc_state,
 					 struct intel_plane_state *plane_state)
@@ -743,7 +738,8 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	struct intel_fbc *fbc = &dev_priv->fbc;
 	struct intel_fbc_state_cache *cache = &fbc->state_cache;
 	struct drm_framebuffer *fb = plane_state->base.fb;
-	struct drm_i915_gem_object *obj;
+
+	cache->vma = NULL;
 
 	cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
@@ -758,16 +754,10 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
 	if (!cache->plane.visible)
 		return;
 
-	obj = intel_fb_obj(fb);
-
-	/* FIXME: We lack the proper locking here, so only run this on the
-	 * platforms that need. */
-	if (IS_GEN(dev_priv, 5, 6))
-		cache->fb.ilk_ggtt_offset = i915_gem_object_ggtt_offset(obj, NULL);
 	cache->fb.pixel_format = fb->pixel_format;
 	cache->fb.stride = fb->pitches[0];
-	cache->fb.fence_reg = get_fence_id(fb);
-	cache->fb.tiling_mode = i915_gem_object_get_tiling(obj);
+
+	cache->vma = plane_state->vma;
 }
 
 static bool intel_fbc_can_activate(struct intel_crtc *crtc)
@@ -784,7 +774,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 		return false;
 	}
 
-	if (!cache->plane.visible) {
+	if (!cache->vma) {
 		fbc->no_fbc_reason = "primary plane not visible";
 		return false;
 	}
@@ -807,8 +797,7 @@ static bool intel_fbc_can_activate(struct intel_crtc *crtc)
 	 * so have no fence associated with it) due to aperture constaints
 	 * at the time of pinning.
 	 */
-	if (cache->fb.tiling_mode != I915_TILING_X ||
-	    cache->fb.fence_reg == I915_FENCE_REG_NONE) {
+	if (!cache->vma->fence) {
 		fbc->no_fbc_reason = "framebuffer not tiled or fenced";
 		return false;
 	}
@@ -888,17 +877,16 @@ static void intel_fbc_get_reg_params(struct intel_crtc *crtc,
 	 * zero. */
 	memset(params, 0, sizeof(*params));
 
+	params->vma = cache->vma;
+
 	params->crtc.pipe = crtc->pipe;
 	params->crtc.plane = crtc->plane;
 	params->crtc.fence_y_offset = get_crtc_fence_y_offset(crtc);
 
 	params->fb.pixel_format = cache->fb.pixel_format;
 	params->fb.stride = cache->fb.stride;
-	params->fb.fence_reg = cache->fb.fence_reg;
 
 	params->cfb_size = intel_fbc_calculate_cfb_size(dev_priv, cache);
-
-	params->fb.ggtt_offset = cache->fb.ilk_ggtt_offset;
 }
 
 static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index fc958d5ed0dc..ca29a5314f0e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -284,7 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 out_destroy_fbi:
 	drm_fb_helper_release_fbi(helper);
 out_unpin:
-	intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
+	intel_unpin_fb_vma(vma);
 out_unlock:
 	mutex_unlock(&dev->struct_mutex);
 	return ret;
@@ -549,7 +549,7 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
 
 	if (ifbdev->fb) {
 		mutex_lock(&ifbdev->helper.dev->struct_mutex);
-		intel_unpin_fb_obj(&ifbdev->fb->base, DRM_ROTATE_0);
+		intel_unpin_fb_vma(ifbdev->vma);
 		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
 
 		drm_framebuffer_remove(&ifbdev->fb->base);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 8b2fc67acbba..ae8ce621e3fc 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -281,7 +281,7 @@ skl_update_plane(struct drm_plane *drm_plane,
 
 	I915_WRITE(PLANE_CTL(pipe, plane), plane_ctl);
 	I915_WRITE(PLANE_SURF(pipe, plane),
-		   intel_fb_gtt_offset(fb, rotation) + surf_addr);
+		   intel_plane_ggtt_offset(plane_state) + surf_addr);
 	POSTING_READ(PLANE_SURF(pipe, plane));
 }
 
@@ -476,7 +476,7 @@ vlv_update_plane(struct drm_plane *dplane,
 	I915_WRITE(SPSIZE(pipe, plane), (crtc_h << 16) | crtc_w);
 	I915_WRITE(SPCNTR(pipe, plane), sprctl);
 	I915_WRITE(SPSURF(pipe, plane),
-		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ(SPSURF(pipe, plane));
 }
 
@@ -612,7 +612,7 @@ ivb_update_plane(struct drm_plane *plane,
 		I915_WRITE(SPRSCALE(pipe), sprscale);
 	I915_WRITE(SPRCTL(pipe), sprctl);
 	I915_WRITE(SPRSURF(pipe),
-		   intel_fb_gtt_offset(fb, rotation) + sprsurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + sprsurf_offset);
 	POSTING_READ(SPRSURF(pipe));
 }
 
@@ -739,7 +739,7 @@ ilk_update_plane(struct drm_plane *plane,
 	I915_WRITE(DVSSCALE(pipe), dvsscale);
 	I915_WRITE(DVSCNTR(pipe), dvscntr);
 	I915_WRITE(DVSSURF(pipe),
-		   intel_fb_gtt_offset(fb, rotation) + dvssurf_offset);
+		   intel_plane_ggtt_offset(plane_state) + dvssurf_offset);
 	POSTING_READ(DVSSURF(pipe));
 }
 
-- 
2.10.2

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

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

end of thread, other threads:[~2016-11-15 10:24 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-29 12:04 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
2016-10-29 12:04 ` [PATCH 2/5] drm/i915: Always prepare planes at the start of an atomic commit Chris Wilson
2016-10-29 12:04 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
2016-10-29 12:04 ` [PATCH 4/5] drm/i915: Quick spring clean of intel_prepare_plane_fb() Chris Wilson
2016-10-29 12:04 ` [PATCH 5/5] drm/i915: Set crtc_state->fb_changed whenever a VMA is changed Chris Wilson
2016-11-15  8:58 [PATCH 1/5] drm/i915: Move intel_prepare_plane_fb() and intel_cleanup_plane_fb() Chris Wilson
2016-11-15  8:58 ` [PATCH 3/5] drm/i915: Track pinned vma in intel_plane_state Chris Wilson
2016-11-15  9:52   ` Daniel Vetter
2016-11-15 10:02     ` Chris Wilson
2016-11-15 10:13       ` Daniel Vetter
2016-11-15 10:24   ` 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.