diff mbox

[v2,12/27] drm/i915: Split plane updates of crtc->atomic into a helper, v2.

Message ID 1433422077-14907-13-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst June 4, 2015, 12:47 p.m. UTC
This makes it easier to verify that no changes are done when
calling this from crtc instead.

Changes since v1:
 - Make intel_wm_need_update static and always check it.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  21 +--
 drivers/gpu/drm/i915/intel_display.c      | 275 ++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h          |   8 +-
 drivers/gpu/drm/i915/intel_sprite.c       |  32 +---
 4 files changed, 176 insertions(+), 160 deletions(-)

Comments

Matt Roper June 11, 2015, 1:35 a.m. UTC | #1
On Thu, Jun 04, 2015 at 02:47:42PM +0200, Maarten Lankhorst wrote:
> This makes it easier to verify that no changes are done when
> calling this from crtc instead.
> 
> Changes since v1:
>  - Make intel_wm_need_update static and always check it.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Do we even need crtc->atomic anymore?  When I first added that, I
expected it to just be a temporary dumping ground until we had
crtc_state's tracked and swapped properly (which we do now).  Can we
just migrate these fields into the state structure instead?


<snip>
> +int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
> +				    struct drm_plane_state *plane_state)
> +{
> +	struct drm_crtc *crtc = crtc_state->crtc;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct drm_plane *plane = plane_state->plane;
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_plane_state *old_plane_state =
> +		to_intel_plane_state(plane->state);
> +	int idx = intel_crtc->base.base.id, ret;
> +	int i = drm_plane_index(plane);
> +	bool mode_changed = needs_modeset(crtc_state);
> +	bool was_crtc_enabled = crtc->state->active;
> +	bool is_crtc_enabled = crtc_state->active;
> +
> +	bool turn_off, turn_on, visible, was_visible;
> +	struct drm_framebuffer *fb = plane_state->fb;
> +
> +	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
> +	    plane->type != DRM_PLANE_TYPE_CURSOR) {
> +		ret = skl_update_scaler_plane(
> +			to_intel_crtc_state(crtc_state),
> +			to_intel_plane_state(plane_state));
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/*
> +	 * Disabling a plane is always okay; we just need to update
> +	 * fb tracking in a special way since cleanup_fb() won't
> +	 * get called by the plane helpers.
> +	 */
> +	if (old_plane_state->base.fb && !fb)
> +		intel_crtc->atomic.disabled_planes |= 1 << i;
> +
> +	/* don't run rest during modeset yet */
> +	if (!intel_crtc->active || mode_changed)
> +		return 0;
> +
> +	was_visible = old_plane_state->visible;
> +	visible = to_intel_plane_state(plane_state)->visible;
> +
> +	if (!was_crtc_enabled && WARN_ON(was_visible))
> +		was_visible = false;
> +
> +	if (!is_crtc_enabled && WARN_ON(visible))
> +		visible = false;
> +
> +	if (!was_visible && !visible)
> +		return 0;
> +
> +	turn_off = was_visible && (!visible || mode_changed);
> +	turn_on = visible && (!was_visible || mode_changed);
> +
> +	DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
> +			 plane->base.id, fb ? fb->base.id : -1);
> +
> +	DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
> +			 plane->base.id, was_visible, visible,
> +			 turn_off, turn_on, mode_changed);
> +
> +	if (intel_wm_need_update(plane, plane_state))
> +		intel_crtc->atomic.update_wm = true;

Hmm, I realize this is the way the code already worked, but this is only
going to trigger update_watermarks rather than update_sprite_watermarks,
which on some platforms could make us update watermarks with stale
sprite values.  I think the only reason we get away with this today is
because we actually perform sprite watermark updates in the low-level
plane update functions (which is bad since we're under vblank
evasion...).  I had some patches that fixed that oversight as part of my
watermark RFC series, but they haven't gone in; I probably need to
extract the fix from the rest of the RFC.

Not sure if you want to worry about it as part of your work here or not
since this doesn't leave us any worse off than we already are today;
just figured I'd mention it so we don't forget about it.


Matt
Maarten Lankhorst June 11, 2015, 3:44 a.m. UTC | #2
Op 11-06-15 om 03:35 schreef Matt Roper:
> On Thu, Jun 04, 2015 at 02:47:42PM +0200, Maarten Lankhorst wrote:
>> This makes it easier to verify that no changes are done when
>> calling this from crtc instead.
>>
>> Changes since v1:
>>  - Make intel_wm_need_update static and always check it.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Do we even need crtc->atomic anymore?  When I first added that, I
> expected it to just be a temporary dumping ground until we had
> crtc_state's tracked and swapped properly (which we do now).  Can we
> just migrate these fields into the state structure instead?
This is temporarily, I need full conversion to atomic to kill off crtc->atomic.

After that I have 2 update functions, pre_plane_update and post_plane_update.

Any required changes could then be calculated from plane_state->visible, old_plane_state->visible and crtc enabled/disabled.
> <snip>
>> +int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>> +				    struct drm_plane_state *plane_state)
>> +{
>> +	struct drm_crtc *crtc = crtc_state->crtc;
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +	struct drm_plane *plane = plane_state->plane;
>> +	struct drm_device *dev = crtc->dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct intel_plane_state *old_plane_state =
>> +		to_intel_plane_state(plane->state);
>> +	int idx = intel_crtc->base.base.id, ret;
>> +	int i = drm_plane_index(plane);
>> +	bool mode_changed = needs_modeset(crtc_state);
>> +	bool was_crtc_enabled = crtc->state->active;
>> +	bool is_crtc_enabled = crtc_state->active;
>> +
>> +	bool turn_off, turn_on, visible, was_visible;
>> +	struct drm_framebuffer *fb = plane_state->fb;
>> +
>> +	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
>> +	    plane->type != DRM_PLANE_TYPE_CURSOR) {
>> +		ret = skl_update_scaler_plane(
>> +			to_intel_crtc_state(crtc_state),
>> +			to_intel_plane_state(plane_state));
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	/*
>> +	 * Disabling a plane is always okay; we just need to update
>> +	 * fb tracking in a special way since cleanup_fb() won't
>> +	 * get called by the plane helpers.
>> +	 */
>> +	if (old_plane_state->base.fb && !fb)
>> +		intel_crtc->atomic.disabled_planes |= 1 << i;
>> +
>> +	/* don't run rest during modeset yet */
>> +	if (!intel_crtc->active || mode_changed)
>> +		return 0;
>> +
>> +	was_visible = old_plane_state->visible;
>> +	visible = to_intel_plane_state(plane_state)->visible;
>> +
>> +	if (!was_crtc_enabled && WARN_ON(was_visible))
>> +		was_visible = false;
>> +
>> +	if (!is_crtc_enabled && WARN_ON(visible))
>> +		visible = false;
>> +
>> +	if (!was_visible && !visible)
>> +		return 0;
>> +
>> +	turn_off = was_visible && (!visible || mode_changed);
>> +	turn_on = visible && (!was_visible || mode_changed);
>> +
>> +	DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
>> +			 plane->base.id, fb ? fb->base.id : -1);
>> +
>> +	DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
>> +			 plane->base.id, was_visible, visible,
>> +			 turn_off, turn_on, mode_changed);
>> +
>> +	if (intel_wm_need_update(plane, plane_state))
>> +		intel_crtc->atomic.update_wm = true;
> Hmm, I realize this is the way the code already worked, but this is only
> going to trigger update_watermarks rather than update_sprite_watermarks,
> which on some platforms could make us update watermarks with stale
> sprite values.  I think the only reason we get away with this today is
> because we actually perform sprite watermark updates in the low-level
> plane update functions (which is bad since we're under vblank
> evasion...).  I had some patches that fixed that oversight as part of my
> watermark RFC series, but they haven't gone in; I probably need to
> extract the fix from the rest of the RFC.
>
> Not sure if you want to worry about it as part of your work here or not
> since this doesn't leave us any worse off than we already are today;
> just figured I'd mention it so we don't forget about it.
>
Yeah, I only wanted to preserve current behavior in this commit, so it becomes more clear why it happens.

~Maarten
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 86ba4b2c3a65..aa2128369a0a 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -114,6 +114,7 @@  static int intel_plane_atomic_check(struct drm_plane *plane,
 	struct intel_crtc_state *crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
+	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -160,20 +161,6 @@  static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.y2 =
 		crtc_state->base.active ? crtc_state->pipe_src_h : 0;
 
-	/*
-	 * Disabling a plane is always okay; we just need to update
-	 * fb tracking in a special way since cleanup_fb() won't
-	 * get called by the plane helpers.
-	 */
-	if (state->fb == NULL && plane->state->fb != NULL) {
-		/*
-		 * 'prepare' is never called when plane is being disabled, so
-		 * we need to handle frontbuffer tracking as a special case
-		 */
-		intel_crtc->atomic.disabled_planes |=
-			(1 << drm_plane_index(plane));
-	}
-
 	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
@@ -198,7 +185,11 @@  static int intel_plane_atomic_check(struct drm_plane *plane,
 		}
 	}
 
-	return intel_plane->check_plane(plane, intel_state);
+	ret = intel_plane->check_plane(plane, intel_state);
+	if (ret || !state->state)
+		return ret;
+
+	return intel_plane_atomic_calc_changes(&crtc_state->base, state);
 }
 
 static void intel_plane_atomic_update(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 310b98226d82..517a1bd302d5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4388,18 +4388,18 @@  int skl_update_scaler_crtc(struct intel_crtc_state *state, int force_detach)
  * skl_update_scaler_plane - Stages update to scaler state for a given plane.
  *
  * @state: crtc's scaler state
- * @intel_plane: affected plane
  * @plane_state: atomic plane state to update
  *
  * Return
  *     0 - scaler_usage updated successfully
  *    error - requested scaling cannot be supported or other error condition
  */
-int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
-			    struct intel_plane *intel_plane,
-			    struct intel_plane_state *plane_state)
+static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
+				   struct intel_plane_state *plane_state)
 {
 
+	struct intel_plane *intel_plane =
+		to_intel_plane(plane_state->base.plane);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	int ret;
 
@@ -11425,6 +11425,161 @@  out_hang:
 	return ret;
 }
 
+
+/**
+ * intel_wm_need_update - Check whether watermarks need updating
+ * @plane: drm plane
+ * @state: new plane state
+ *
+ * Check current plane state versus the new one to determine whether
+ * watermarks need to be recalculated.
+ *
+ * Returns true or false.
+ */
+static bool intel_wm_need_update(struct drm_plane *plane,
+				 struct drm_plane_state *state)
+{
+	/* Update watermarks on tiling changes. */
+	if (!plane->state->fb || !state->fb ||
+	    plane->state->fb->modifier[0] != state->fb->modifier[0] ||
+	    plane->state->rotation != state->rotation)
+		return true;
+
+	if (plane->state->crtc_w != state->crtc_w)
+		return true;
+
+	return false;
+}
+
+int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
+				    struct drm_plane_state *plane_state)
+{
+	struct drm_crtc *crtc = crtc_state->crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_plane *plane = plane_state->plane;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane_state *old_plane_state =
+		to_intel_plane_state(plane->state);
+	int idx = intel_crtc->base.base.id, ret;
+	int i = drm_plane_index(plane);
+	bool mode_changed = needs_modeset(crtc_state);
+	bool was_crtc_enabled = crtc->state->active;
+	bool is_crtc_enabled = crtc_state->active;
+
+	bool turn_off, turn_on, visible, was_visible;
+	struct drm_framebuffer *fb = plane_state->fb;
+
+	if (crtc_state && INTEL_INFO(dev)->gen >= 9 &&
+	    plane->type != DRM_PLANE_TYPE_CURSOR) {
+		ret = skl_update_scaler_plane(
+			to_intel_crtc_state(crtc_state),
+			to_intel_plane_state(plane_state));
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * Disabling a plane is always okay; we just need to update
+	 * fb tracking in a special way since cleanup_fb() won't
+	 * get called by the plane helpers.
+	 */
+	if (old_plane_state->base.fb && !fb)
+		intel_crtc->atomic.disabled_planes |= 1 << i;
+
+	/* don't run rest during modeset yet */
+	if (!intel_crtc->active || mode_changed)
+		return 0;
+
+	was_visible = old_plane_state->visible;
+	visible = to_intel_plane_state(plane_state)->visible;
+
+	if (!was_crtc_enabled && WARN_ON(was_visible))
+		was_visible = false;
+
+	if (!is_crtc_enabled && WARN_ON(visible))
+		visible = false;
+
+	if (!was_visible && !visible)
+		return 0;
+
+	turn_off = was_visible && (!visible || mode_changed);
+	turn_on = visible && (!was_visible || mode_changed);
+
+	DRM_DEBUG_ATOMIC("[CRTC:%i] has [PLANE:%i] with fb %i\n", idx,
+			 plane->base.id, fb ? fb->base.id : -1);
+
+	DRM_DEBUG_ATOMIC("[PLANE:%i] visible %i -> %i, off %i, on %i, ms %i\n",
+			 plane->base.id, was_visible, visible,
+			 turn_off, turn_on, mode_changed);
+
+	if (intel_wm_need_update(plane, plane_state))
+		intel_crtc->atomic.update_wm = true;
+
+	switch (plane->type) {
+	case DRM_PLANE_TYPE_PRIMARY:
+		if (visible)
+			intel_crtc->atomic.fb_bits |=
+			    INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+		intel_crtc->atomic.wait_for_flips = true;
+		intel_crtc->atomic.pre_disable_primary = turn_off;
+		intel_crtc->atomic.post_enable_primary = turn_on;
+
+		if (turn_off)
+			intel_crtc->atomic.disable_fbc = true;
+
+		/*
+		 * FBC does not work on some platforms for rotated
+		 * planes, so disable it when rotation is not 0 and
+		 * update it when rotation is set back to 0.
+		 *
+		 * FIXME: This is redundant with the fbc update done in
+		 * the primary plane enable function except that that
+		 * one is done too late. We eventually need to unify
+		 * this.
+		 */
+
+		if (visible &&
+		    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+		    dev_priv->fbc.crtc == intel_crtc &&
+		    plane_state->rotation != BIT(DRM_ROTATE_0))
+			intel_crtc->atomic.disable_fbc = true;
+
+		/*
+		 * BDW signals flip done immediately if the plane
+		 * is disabled, even if the plane enable is already
+		 * armed to occur at the next vblank :(
+		 */
+		if (turn_on && IS_BROADWELL(dev))
+			intel_crtc->atomic.wait_vblank = true;
+
+		intel_crtc->atomic.update_fbc |= visible || mode_changed;
+		break;
+	case DRM_PLANE_TYPE_CURSOR:
+		if (visible)
+			intel_crtc->atomic.fb_bits |=
+			    INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+		break;
+	case DRM_PLANE_TYPE_OVERLAY:
+		/*
+		 * 'prepare' is never called when plane is being disabled, so
+		 * we need to handle frontbuffer tracking as a special case
+		 */
+		if (visible)
+			intel_crtc->atomic.fb_bits |=
+			    INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+		if (turn_off && is_crtc_enabled) {
+			intel_crtc->atomic.wait_vblank = true;
+			intel_crtc->atomic.update_sprite_watermarks |=
+				1 << i;
+		}
+		break;
+	}
+	return 0;
+}
+
 static bool encoders_cloneable(const struct intel_encoder *a,
 			       const struct intel_encoder *b)
 {
@@ -13118,28 +13273,6 @@  static void intel_shared_dpll_init(struct drm_device *dev)
 }
 
 /**
- * intel_wm_need_update - Check whether watermarks need updating
- * @plane: drm plane
- * @state: new plane state
- *
- * Check current plane state versus the new one to determine whether
- * watermarks need to be recalculated.
- *
- * Returns true or false.
- */
-bool intel_wm_need_update(struct drm_plane *plane,
-			  struct drm_plane_state *state)
-{
-	/* Update watermarks on tiling changes. */
-	if (!plane->state->fb || !state->fb ||
-	    plane->state->fb->modifier[0] != state->fb->modifier[0] ||
-	    plane->state->rotation != state->rotation)
-		return true;
-
-	return false;
-}
-
-/**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
  * @fb: framebuffer to prepare for presentation
@@ -13260,7 +13393,6 @@  intel_check_primary_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
 	struct intel_crtc_state *crtc_state;
@@ -13271,7 +13403,6 @@  intel_check_primary_plane(struct drm_plane *plane,
 	bool can_position = false;
 	int max_scale = DRM_PLANE_HELPER_NO_SCALING;
 	int min_scale = DRM_PLANE_HELPER_NO_SCALING;
-	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -13287,73 +13418,11 @@  intel_check_primary_plane(struct drm_plane *plane,
 		can_position = true;
 	}
 
-	ret = drm_plane_helper_check_update(plane, crtc, fb,
-					    src, dest, clip,
-					    min_scale,
-					    max_scale,
-					    can_position, true,
-					    &state->visible);
-	if (ret)
-		return ret;
-
-	if (intel_crtc->active) {
-		struct intel_plane_state *old_state =
-			to_intel_plane_state(plane->state);
-
-		intel_crtc->atomic.wait_for_flips = true;
-
-		/*
-		 * FBC does not work on some platforms for rotated
-		 * planes, so disable it when rotation is not 0 and
-		 * update it when rotation is set back to 0.
-		 *
-		 * FIXME: This is redundant with the fbc update done in
-		 * the primary plane enable function except that that
-		 * one is done too late. We eventually need to unify
-		 * this.
-		 */
-		if (state->visible &&
-		    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
-		    dev_priv->fbc.crtc == intel_crtc &&
-		    state->base.rotation != BIT(DRM_ROTATE_0)) {
-			intel_crtc->atomic.disable_fbc = true;
-		}
-
-		if (state->visible && !old_state->visible) {
-			/*
-			 * BDW signals flip done immediately if the plane
-			 * is disabled, even if the plane enable is already
-			 * armed to occur at the next vblank :(
-			 */
-			if (IS_BROADWELL(dev))
-				intel_crtc->atomic.wait_vblank = true;
-
-			if (crtc_state && !needs_modeset(&crtc_state->base))
-				intel_crtc->atomic.post_enable_primary = true;
-		}
-
-		if (!state->visible && old_state->visible &&
-		    crtc_state && !needs_modeset(&crtc_state->base))
-			intel_crtc->atomic.pre_disable_primary = true;
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
-
-		intel_crtc->atomic.update_fbc = true;
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-	}
-
-	if (INTEL_INFO(dev)->gen >= 9) {
-		ret = skl_update_scaler_plane(crtc_state,
-					      to_intel_plane(plane),
-					      state);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
+	return drm_plane_helper_check_update(plane, crtc, fb,
+					     src, dest, clip,
+					     min_scale, max_scale,
+					     can_position, true,
+					     &state->visible);
 }
 
 static void
@@ -13613,10 +13682,9 @@  intel_check_cursor_plane(struct drm_plane *plane,
 	if (ret)
 		return ret;
 
-
 	/* if we want to turn off the cursor ignore width and height */
 	if (!obj)
-		goto finish;
+		return 0;
 
 	/* Check for which cursor types we support */
 	if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
@@ -13633,19 +13701,10 @@  intel_check_cursor_plane(struct drm_plane *plane,
 
 	if (fb->modifier[0] != DRM_FORMAT_MOD_NONE) {
 		DRM_DEBUG_KMS("cursor cannot be tiled\n");
-		ret = -EINVAL;
-	}
-
-finish:
-	if (intel_crtc->active) {
-		if (plane->state->crtc_w != state->base.crtc_w)
-			intel_crtc->atomic.update_wm = true;
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+		return -EINVAL;
 	}
 
-	return ret;
+	return 0;
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e116a5f6346e..802f3e99c849 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1055,6 +1055,8 @@  int intel_plane_atomic_set_property(struct drm_plane *plane,
 				    struct drm_plane_state *state,
 				    struct drm_property *property,
 				    uint64_t val);
+int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
+				    struct drm_plane_state *plane_state);
 
 unsigned int
 intel_tile_height(struct drm_device *dev, uint32_t pixel_format,
@@ -1069,9 +1071,6 @@  intel_rotation_90_or_270(unsigned int rotation)
 void intel_create_rotation_property(struct drm_device *dev,
 					struct intel_plane *plane);
 
-bool intel_wm_need_update(struct drm_plane *plane,
-			  struct drm_plane_state *state);
-
 /* shared dpll functions */
 struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
@@ -1136,9 +1135,6 @@  void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
 void intel_modeset_preclose(struct drm_device *dev, struct drm_file *file);
 void skl_detach_scalers(struct intel_crtc *intel_crtc);
-int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
-			    struct intel_plane *intel_plane,
-			    struct intel_plane_state *plane_state);
 
 int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state, int force_detach);
 int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index fe95f25f019a..f5921b652b90 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -756,7 +756,6 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	int max_scale, min_scale;
 	bool can_scale;
 	int pixel_size;
-	int ret;
 
 	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
 	crtc_state = state->base.state ?
@@ -764,7 +763,7 @@  intel_check_sprite_plane(struct drm_plane *plane,
 
 	if (!fb) {
 		state->visible = false;
-		goto finish;
+		return 0;
 	}
 
 	/* Don't modify another pipe's plane */
@@ -917,35 +916,6 @@  intel_check_sprite_plane(struct drm_plane *plane,
 	dst->y1 = crtc_y;
 	dst->y2 = crtc_y + crtc_h;
 
-finish:
-	/*
-	 * If the sprite is completely covering the primary plane,
-	 * we can disable the primary and save power.
-	 */
-	if (intel_crtc->active) {
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-
-		if (!state->visible) {
-			/*
-			 * Avoid underruns when disabling the sprite.
-			 * FIXME remove once watermark updates are done properly.
-			 */
-			intel_crtc->atomic.wait_vblank = true;
-			intel_crtc->atomic.update_sprite_watermarks |=
-				(1 << drm_plane_index(plane));
-		}
-	}
-
-	if (INTEL_INFO(dev)->gen >= 9) {
-		ret = skl_update_scaler_plane(crtc_state, intel_plane, state);
-		if (ret)
-			return ret;
-	}
-
 	return 0;
 }