[PATCH v2 8/8] drm/i915: Move atomic crtc update checking to the check crtc function.

[Maarten Lankhorst <maarten.lankhorst@linux.intel.com>]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Changes since v1:
 - Clear intel_crtc->atomic at the start of the check function (paranoia).
 
 drivers/gpu/drm/i915/intel_atomic_plane.c |  18 +--
 drivers/gpu/drm/i915/intel_display.c      | 196 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_sprite.c       |  25 +---
 3 files changed, 134 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index a27ee8cbb627..4b639b54583d 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -123,8 +123,10 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	 * anything driver-specific we need to test in that case, so
 	 * just return success.
 	 */
-	if (!crtc)
+	if (!crtc || !intel_crtc->active || !state->fb) {
+		intel_state->visible = 0;
 		return 0;
+	}
 
 	/*
 	 * The original src/dest coordinates are stored in state->base, but
@@ -148,20 +150,6 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.y2 =
 		intel_crtc->active ? intel_crtc->config->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)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61a29f142c70..4965743053b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -101,6 +101,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state);
 static void intel_begin_crtc_commit(struct drm_crtc *crtc);
 static void intel_finish_crtc_commit(struct drm_crtc *crtc);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
@@ -10656,6 +10658,7 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.load_lut = intel_crtc_load_lut,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
+	.atomic_check = intel_atomic_check_crtc,
 };
 
 /**
@@ -12783,6 +12786,9 @@ bool intel_wm_need_update(struct drm_plane *plane,
 	    plane->state->rotation != state->rotation)
 		return true;
 
+	if (plane->type == DRM_PLANE_TYPE_CURSOR)
+		return plane->state->crtc_w != state->crtc_w;
+
 	return false;
 }
 
@@ -12877,7 +12883,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 drm_framebuffer *fb = state->base.fb;
@@ -12885,7 +12890,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
 	bool can_position = false;
-	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -12893,58 +12897,12 @@ intel_check_primary_plane(struct drm_plane *plane,
 	if (INTEL_INFO(dev)->gen >= 9)
 		can_position = true;
 
-	ret = drm_plane_helper_check_update(plane, crtc, fb,
-					    src, dest, clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    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;
-		}
-
-		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;
-	}
-
-	return 0;
+	return drm_plane_helper_check_update(plane, crtc, fb,
+					     src, dest, clip,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     can_position, true,
+					     &state->visible);
 }
 
 static void
@@ -12986,6 +12944,121 @@ intel_disable_primary_plane(struct drm_plane *plane,
 	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
 }
 
+/* Transitional checking here, mostly for plane updates */
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state)
+{
+	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned plane_mask;
+	int i, nplanes = dev->mode_config.num_total_plane;
+	bool mode_changed = crtc_state->mode_changed;
+
+	if (!crtc_state->planes_changed)
+		return 0;
+
+	if (!intel_crtc->active)
+		return 0;
+
+	memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
+	intel_crtc->atomic.update_wm = mode_changed;
+
+	DRM_DEBUG_ATOMIC("Crtc %i was enabled %i now enabled: %i\n", drm_crtc_index(crtc), crtc->state->enable, crtc_state->enable);
+
+	plane_mask = crtc_state->plane_mask | crtc->state->plane_mask;
+	for (i = 0; i < nplanes; i++) {
+		struct intel_plane_state *plane_state, *old_plane_state;
+		struct intel_plane *plane = to_intel_plane(state->planes[i]);
+		bool turn_off, turn_on, visible, was_visible;
+
+		if (!plane)
+			continue;
+
+		plane_state = to_intel_plane_state(state->plane_states[i]);
+		old_plane_state = to_intel_plane_state(plane->base.state);
+
+		was_visible = old_plane_state->visible && crtc->state->enable;
+		visible = plane_state->visible && crtc_state->enable;
+
+		turn_off = was_visible && (!visible || mode_changed);
+		turn_on = visible && (!was_visible || mode_changed);
+
+		DRM_DEBUG_ATOMIC("Plane %i with fb %p and crtc %p, was_visible %i, visible %i, turn_off %i, turn_on %i, modeset: %i\n",
+				 i, plane_state->base.fb, plane_state->base.crtc, was_visible, visible, turn_off, turn_on, mode_changed);
+
+		/* plane being turned off as part of modeset or changes? */
+		if (intel_wm_need_update(&plane->base, &plane_state->base))
+			intel_crtc->atomic.update_wm = true;
+
+		if (old_plane_state->base.fb && !plane_state->base.fb)
+		/*
+		 * '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->base));
+
+		switch (plane->base.type) {
+		case DRM_PLANE_TYPE_PRIMARY:
+			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->base.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 = true;
+			break;
+		case DRM_PLANE_TYPE_CURSOR:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+			break;
+		case DRM_PLANE_TYPE_OVERLAY:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+			if (turn_off) {
+				intel_crtc->atomic.wait_vblank = true;
+				intel_crtc->atomic.update_sprite_watermarks |=
+					(1 << drm_plane_index(&plane->base));
+			}
+			break;
+		}
+	}
+	return 0;
+}
+
 static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -13200,7 +13273,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 	/* 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)) {
@@ -13217,19 +13290,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_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7419e04b113f..9cf35c33bc61 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -815,7 +815,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 */
@@ -952,29 +952,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));
-		}
-	}
-
 	return 0;
 }
 
-- 
2.1.0


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