[Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Mon May 11 07:24:46 PDT 2015


This kills off most of the transitional helpers and uses atomic plane updates
in the modeset path to update everything.

Getting rid of the transitional plane helpers meant that planes had to be added
in the crtc check function. On modeset a connector can be moved to a different
crtc, and this is not handled correctly otherwise.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  59 ++-
 drivers/gpu/drm/i915/intel_display.c      | 655 ++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_drv.h          |   2 +-
 drivers/gpu/drm/i915/intel_sprite.c       |  80 +---
 4 files changed, 441 insertions(+), 355 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 86ba4b2c3a65..85b87e4d4b6e 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
 	struct drm_crtc *crtc = state->crtc;
-	struct intel_crtc *intel_crtc;
-	struct intel_crtc_state *crtc_state;
+	struct drm_crtc_state *crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
 
-	crtc = crtc ? crtc : plane->crtc;
-	intel_crtc = to_intel_crtc(crtc);
-
+	intel_state->visible = false;
 	/*
 	 * Both crtc and plane->crtc could be NULL if we're updating a
 	 * property while the plane is disabled.  We don't actually have
 	 * anything driver-specific we need to test in that case, so
 	 * just return success.
 	 */
-	if (!crtc)
+	if (!crtc) {
+		DRM_DEBUG_ATOMIC("Invisible: no crtc\n");
 		return 0;
+	}
+
+	crtc_state = state->state->crtc_states[drm_crtc_index(crtc)];
+	if (WARN_ON(!crtc_state))
+		return 0;
+
+	if (!crtc_state->enable) {
+		DRM_DEBUG_ATOMIC("Invisible: crtc off\n");
 
-	/* FIXME: temporary hack necessary while we still use the plane update
-	 * helper. */
-	if (state->state) {
-		crtc_state =
-			intel_atomic_get_crtc_state(state->state, intel_crtc);
-		if (IS_ERR(crtc_state))
-			return PTR_ERR(crtc_state);
-	} else {
-		crtc_state = intel_crtc->config;
+		/*
+		 * Probably allowed after converting to atomic. Right
+		 * now it probably means we have the state confused.
+		 */
+		I915_STATE_WARN_ON(plane->type == DRM_PLANE_TYPE_PRIMARY);
+		return 0;
+	}
+
+	if (!crtc_state->active) {
+		DRM_DEBUG_ATOMIC("Invisible: dpms off\n");
+		return 0;
 	}
 
 	/*
@@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
 	intel_state->clip.x1 = 0;
 	intel_state->clip.y1 = 0;
-	intel_state->clip.x2 =
-		crtc_state->base.active ? crtc_state->pipe_src_w : 0;
-	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));
-	}
+	drm_crtc_get_hv_timing(&crtc_state->mode,
+			       &intel_state->clip.x2,
+			       &intel_state->clip.y2);
 
 	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 956c9964275d..9610f76a2489 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -100,14 +100,16 @@ 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,
 	struct intel_crtc_state *crtc_state);
 static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
 			   int num_connectors);
-static void intel_crtc_enable_planes(struct drm_crtc *crtc);
-static void intel_crtc_disable_planes(struct drm_crtc *crtc);
+static void intel_pre_disable_primary(struct drm_crtc *crtc);
+static void intel_post_enable_primary(struct drm_crtc *crtc);
 
 static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
 {
@@ -2220,28 +2222,6 @@ void intel_flush_primary_plane(struct drm_i915_private *dev_priv,
 	POSTING_READ(reg);
 }
 
-/**
- * intel_enable_primary_hw_plane - enable the primary plane on a given pipe
- * @plane:  plane to be enabled
- * @crtc: crtc for the plane
- *
- * Enable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_enable_primary_hw_plane(struct drm_plane *plane,
-					  struct drm_crtc *crtc)
-{
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	/* If the pipe isn't enabled, we can't pump pixels and may hang */
-	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
-	to_intel_plane_state(plane->state)->visible = true;
-
-	dev_priv->display.update_primary_plane(crtc, plane->fb,
-					       crtc->x, crtc->y);
-}
-
 static bool need_vtd_wa(struct drm_device *dev)
 {
 #ifdef CONFIG_INTEL_IOMMU
@@ -3161,11 +3141,20 @@ intel_pipe_set_base_atomic(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane_state *plane_state =
+		to_intel_plane_state(crtc->primary->state);
+	bool was_visible = plane_state->visible;
 
-	if (dev_priv->display.disable_fbc)
+	/* Not supported right now by the helper, but lets be thorough. */
+	if (was_visible && !fb)
+		intel_pre_disable_primary(crtc);
+	else if (was_visible && dev_priv->display.disable_fbc)
 		dev_priv->display.disable_fbc(dev);
 
+	plane_state->visible = !!fb;
 	dev_priv->display.update_primary_plane(crtc, fb, x, y);
+	if (!was_visible && fb)
+		intel_post_enable_primary(crtc);
 
 	return 0;
 }
@@ -3192,16 +3181,17 @@ static void intel_update_primary_planes(struct drm_device *dev)
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
 		drm_modeset_lock(&crtc->mutex, NULL);
-		/*
-		 * FIXME: Once we have proper support for primary planes (and
-		 * disabling them without disabling the entire crtc) allow again
-		 * a NULL crtc->primary->fb.
-		 */
-		if (intel_crtc->active && crtc->primary->fb)
+
+		if (intel_crtc->active) {
+			const struct intel_plane_state *state =
+				to_intel_plane_state(crtc->primary->state);
+
 			dev_priv->display.update_primary_plane(crtc,
-							       crtc->primary->fb,
-							       crtc->x,
-							       crtc->y);
+							state->base.fb,
+							state->src.x1 >> 16,
+							state->src.y1 >> 16);
+		}
+
 		drm_modeset_unlock(&crtc->mutex);
 	}
 }
@@ -4572,20 +4562,6 @@ static void ironlake_pfit_enable(struct intel_crtc *crtc)
 	}
 }
 
-static void intel_enable_sprite_planes(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
-	struct drm_plane *plane;
-	struct intel_plane *intel_plane;
-
-	drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
-		intel_plane = to_intel_plane(plane);
-		if (intel_plane->pipe == pipe)
-			intel_plane_restore(&intel_plane->base);
-	}
-}
-
 void hsw_enable_ips(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -4815,44 +4791,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 }
 
-static void intel_crtc_enable_planes(struct drm_crtc *crtc)
-{
-	intel_enable_primary_hw_plane(crtc->primary, crtc);
-	intel_enable_sprite_planes(crtc);
-	intel_crtc_update_cursor(crtc, true);
-
-	intel_post_enable_primary(crtc);
-}
-
-static void intel_crtc_disable_planes(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_plane *intel_plane;
-	int pipe = intel_crtc->pipe;
-
-	intel_crtc_wait_for_pending_flips(crtc);
-
-	intel_pre_disable_primary(crtc);
-
-	intel_crtc_dpms_overlay_disable(intel_crtc);
-	for_each_intel_plane(dev, intel_plane) {
-		if (intel_plane->pipe == pipe) {
-			struct drm_crtc *from = intel_plane->base.crtc;
-
-			intel_plane->disable_plane(&intel_plane->base,
-						   from ?: crtc, true);
-		}
-	}
-
-	/*
-	 * FIXME: Once we grow proper nuclear flip support out of this we need
-	 * to compute the mask of flip planes precisely. For the time being
-	 * consider this a flip to a NULL plane.
-	 */
-	intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
-}
-
 static void ironlake_crtc_enable(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -11061,6 +10999,7 @@ static const 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,
 };
 
 /* Transitional helper to copy current connector/encoder state to
@@ -11426,16 +11365,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 	int i;
 	bool retry = true;
 
-	if (!check_encoder_cloning(state, to_intel_crtc(crtc))) {
-		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
-		return -EINVAL;
-	}
-
-	if (!check_digital_port_conflicts(state)) {
-		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
-		return -EINVAL;
-	}
-
 	clear_intel_crtc_state(pipe_config);
 
 	pipe_config->cpu_transcoder =
@@ -11553,9 +11482,27 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	int i;
 
 	intel_shared_dpll_commit(dev_priv);
-	drm_atomic_helper_swap_state(state->dev, state);
+
+	/*
+	 * swap crtc and connector state, plane state is already swapped in
+	 * __intel_set_mode_update_planes. Once .crtc_disable is fixed
+	 * all state should be swapped before disabling crtc's.
+	 */
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		crtc->state->state = state;
+		swap(state->crtc_states[i], crtc->state);
+		crtc->state->state = NULL;
+	}
+
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		connector->state->state = state;
+		swap(state->connector_states[i], connector->state);
+		connector->state->state = NULL;
+	}
 
 	for_each_intel_encoder(dev, intel_encoder) {
 		if (!intel_encoder->base.crtc)
@@ -12163,8 +12110,8 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	    WARN_ON(pipe_config->base.active))
 		pipe_config->base.active = false;
 
-	if (!pipe_config->base.enable)
-		return pipe_config;
+	if (!pipe_config->base.active)
+		goto done;
 
 	ret = intel_modeset_pipe_config(crtc, state, pipe_config);
 	if (ret)
@@ -12182,8 +12129,8 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	 * required changes and forcing a mode set.
 	 */
 
-	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
-
+	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]");
+done:
 	ret = drm_atomic_helper_check_planes(state->dev, state);
 	if (ret)
 		return ERR_PTR(ret);
@@ -12247,6 +12194,11 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	struct drm_device *dev = state->dev;
 	int ret;
 
+	if (!check_digital_port_conflicts(state)) {
+		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * See if the config requires any additional preparation, e.g.
 	 * to adjust global state with pipes off.  We need to do this
@@ -12267,6 +12219,112 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	return 0;
 }
 
+static void __intel_set_mode_update_planes(struct drm_device *dev,
+					   struct drm_atomic_state *state)
+{
+	int i;
+	struct drm_plane_state *old_plane_state;
+	struct drm_crtc_state *crtc_state;
+	struct drm_plane *plane;
+	struct drm_crtc *crtc;
+
+	/*
+	 * For now only swap plane state, will be replaced with a
+	 * call to drm_atomic_helper_swap_state
+	 */
+	for_each_plane_in_state(state, plane, old_plane_state, i) {
+		struct drm_plane *plane = state->planes[i];
+
+		if (!plane)
+			continue;
+
+		plane->state->state = state;
+		swap(state->plane_states[i], plane->state);
+		plane->state->state = NULL;
+	}
+
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		const struct drm_crtc_helper_funcs *funcs;
+
+		funcs = crtc->helper_private;
+
+		if (!funcs || !funcs->atomic_begin)
+			continue;
+
+		/* XXX: Hack because crtc state is not swapped */
+		crtc->state->mode_changed = crtc_state->mode_changed;
+		crtc->state->active_changed = crtc_state->active_changed;
+
+		DRM_DEBUG_ATOMIC("Calling atomic_begin on crtc %i\n", i);
+		funcs->atomic_begin(crtc);
+	}
+
+	for_each_plane_in_state(state, plane, old_plane_state, i) {
+		bool visible = to_intel_plane_state(plane->state)->visible;
+		struct intel_plane *intel_plane = to_intel_plane(plane);
+		const struct drm_plane_helper_funcs *funcs =
+			plane->helper_private;
+
+		crtc = plane->state->crtc;
+
+		/* no point in disabling if already disabled */
+		if (!to_intel_plane_state(old_plane_state)->visible &&
+		    !to_intel_plane_state(old_plane_state)->hw_enabled)
+			continue;
+
+		to_intel_plane_state(plane->state)->hw_enabled = false;
+		DRM_DEBUG_ATOMIC("Plane %i is visible: %i\n", i, visible);
+
+		if (!visible)
+			funcs->atomic_update(plane, old_plane_state);
+		else if (needs_modeset(crtc->state))
+			intel_plane->disable_plane(plane, crtc, true);
+	}
+}
+
+static void __intel_set_mode_cleanup_planes(struct drm_device *dev,
+					    struct drm_atomic_state *old_state)
+{
+	int nplanes = dev->mode_config.num_total_plane;
+	int ncrtcs = dev->mode_config.num_crtc;
+	int i;
+
+	for (i = 0; i < nplanes; i++) {
+		const struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = old_state->planes[i];
+		struct drm_plane_state *old_plane_state;
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+		old_plane_state = old_state->plane_states[i];
+
+		if (to_intel_plane_state(plane->state)->visible) {
+			DRM_DEBUG_ATOMIC("Plane %i is updated\n", i);
+			funcs->atomic_update(plane, old_plane_state);
+		} else
+			DRM_DEBUG_ATOMIC("Plane %i is left alone\n", i);
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		const struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc = old_state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (!funcs || !funcs->atomic_flush)
+			continue;
+
+		DRM_DEBUG_ATOMIC("Calling atomic_flush on crtc %i\n", i);
+		funcs->atomic_flush(crtc);
+	}
+	drm_atomic_helper_cleanup_planes(dev, old_state);
+}
+
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct intel_crtc_state *pipe_config)
 {
@@ -12275,7 +12333,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	struct drm_atomic_state *state = pipe_config->base.state;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
-	int ret = 0;
+	int ret;
 	int i;
 
 	ret = __intel_set_mode_checks(state);
@@ -12286,14 +12344,14 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	if (ret)
 		return ret;
 
+	__intel_set_mode_update_planes(dev, state);
+
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!needs_modeset(crtc_state))
 			continue;
 
-		intel_crtc_disable_planes(crtc);
+		intel_crtc_dpms_overlay_disable(to_intel_crtc(crtc));
 		dev_priv->display.crtc_disable(crtc);
-		if (!crtc_state->enable)
-			drm_plane_helper_disable(crtc->primary);
 	}
 
 	/* Only after disabling all output pipelines that will be changed can we
@@ -12305,8 +12363,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 	modeset_update_crtc_power_domains(state);
 
-	drm_atomic_helper_commit_planes(dev, state);
-
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!crtc->state->active)
@@ -12314,13 +12370,13 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 		update_scanline_offset(to_intel_crtc(crtc));
 
-		dev_priv->display.crtc_enable(crtc);
-		intel_crtc_enable_planes(crtc);
+		if (needs_modeset(crtc->state))
+			dev_priv->display.crtc_enable(crtc);
 	}
 
-	/* FIXME: add subpixel order */
+	__intel_set_mode_cleanup_planes(dev, state);
 
-	drm_atomic_helper_cleanup_planes(dev, state);
+	/* FIXME: add subpixel order */
 
 	drm_atomic_state_free(state);
 
@@ -12568,20 +12624,11 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	return 0;
 }
 
-static bool primary_plane_visible(struct drm_crtc *crtc)
-{
-	struct intel_plane_state *plane_state =
-		to_intel_plane_state(crtc->primary->state);
-
-	return plane_state->visible;
-}
-
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
 	struct drm_atomic_state *state = NULL;
 	struct intel_crtc_state *pipe_config;
-	bool primary_plane_was_visible;
 	int ret;
 
 	BUG_ON(!set);
@@ -12620,38 +12667,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
-	primary_plane_was_visible = primary_plane_visible(set->crtc);
-
 	ret = intel_set_mode_with_config(set->crtc, pipe_config);
-
-	if (ret == 0 &&
-	    pipe_config->base.enable &&
-	    pipe_config->base.planes_changed &&
-	    !needs_modeset(&pipe_config->base)) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-
-		/*
-		 * We need to make sure the primary plane is re-enabled if it
-		 * has previously been turned off.
-		 */
-		if (ret == 0 && !primary_plane_was_visible &&
-		    primary_plane_visible(set->crtc)) {
-			WARN_ON(!intel_crtc->active);
-			intel_post_enable_primary(set->crtc);
-		}
-
-		/*
-		 * In the fastboot case this may be our only check of the
-		 * state after boot.  It would be better to only do it on
-		 * the first update, but we don't have a nice way of doing that
-		 * (and really, set_config isn't used much for high freq page
-		 * flipping, so increasing its cost here shouldn't be a big
-		 * deal).
-		 */
-		if (i915.fastboot && ret == 0)
-			intel_modeset_check_state(set->crtc->dev);
-	}
-
 	if (ret) {
 		DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
 			      set->crtc->base.id, ret);
@@ -12791,6 +12807,9 @@ bool intel_wm_need_update(struct drm_plane *plane,
 	    plane->state->rotation != state->rotation)
 		return true;
 
+	if (plane->state->crtc_w != state->crtc_w)
+		return true;
+
 	return false;
 }
 
@@ -12819,6 +12838,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	unsigned frontbuffer_bits = 0;
 	int ret = 0;
 
+	if (!to_intel_plane_state(plane->state)->visible)
+		old_obj = NULL;
+
 	if (!obj)
 		return 0;
 
@@ -12915,10 +12937,8 @@ 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;
 	struct drm_framebuffer *fb = state->base.fb;
 	struct drm_rect *dest = &state->dst;
 	struct drm_rect *src = &state->src;
@@ -12926,90 +12946,46 @@ 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);
-	crtc_state = state->base.state ?
-		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
 
 	if (INTEL_INFO(dev)->gen >= 9) {
+		struct intel_crtc_state *crtc_state =
+			intel_atomic_get_crtc_state(state->base.state, intel_crtc);
+
 		min_scale = 1;
 		max_scale = skl_max_scale(intel_crtc, crtc_state);
 		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;
-		}
-
-		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 drm_plane_helper_check_update(plane, crtc, fb,
+					     src, dest, clip,
+					     min_scale, max_scale,
+					     can_position, true,
+					     &state->visible);
+}
 
-	if (INTEL_INFO(dev)->gen >= 9) {
-		ret = skl_update_scaler_users(intel_crtc, crtc_state,
-			to_intel_plane(plane), state, 0);
-		if (ret)
-			return ret;
-	}
+static void
+intel_disable_primary_plane(struct drm_plane *plane,
+			    struct drm_crtc *crtc,
+			    bool force)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	return 0;
+	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
 }
 
 static void
 intel_commit_primary_plane(struct drm_plane *plane,
-			   struct intel_plane_state *state)
+			   struct intel_plane_state *new_state)
 {
-	struct drm_crtc *crtc = state->base.crtc;
-	struct drm_framebuffer *fb = state->base.fb;
+	struct drm_crtc *crtc = new_state->base.crtc;
+	struct drm_framebuffer *fb = new_state->base.fb;
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc;
-	struct drm_rect *src = &state->src;
+	struct drm_rect *src = &new_state->src;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -13018,25 +12994,178 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	crtc->x = src->x1 >> 16;
 	crtc->y = src->y1 >> 16;
 
-	if (intel_crtc->active) {
-		if (state->visible)
-			/* FIXME: kill this fastboot hack */
-			intel_update_pipe_size(intel_crtc);
+	if (!new_state->visible ||
+	    WARN_ON(new_state->visible && !crtc->state->active)) {
+		intel_disable_primary_plane(plane, crtc, false);
+	} else {
+		/* FIXME: kill this fastboot hack */
+		intel_update_pipe_size(intel_crtc);
 
-		dev_priv->display.update_primary_plane(crtc, plane->fb,
-						       crtc->x, crtc->y);
+		dev_priv->display.update_primary_plane(crtc, fb,
+						       src->x1 >> 16,
+						       src->y1 >> 16);
 	}
 }
 
-static void
-intel_disable_primary_plane(struct drm_plane *plane,
-			    struct drm_crtc *crtc,
-			    bool force)
+/* Transitional checking here, mostly for plane updates */
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state)
 {
-	struct drm_device *dev = plane->dev;
+	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;
+	struct drm_plane *plane;
+	unsigned plane_mask;
+	int idx, ret;
+	bool mode_changed = needs_modeset(crtc_state);
+	bool is_crtc_enabled = crtc_state->active;
+	bool was_crtc_enabled = crtc->state->active;
 
-	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
+	if (!check_encoder_cloning(state, to_intel_crtc(crtc))) {
+		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
+		return -EINVAL;
+	}
+
+	memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
+	intel_crtc->atomic.update_wm = mode_changed;
+
+	idx = crtc->base.id;
+	I915_STATE_WARN(crtc->state->active != intel_crtc->active,
+		"Crtc %i mismatch between state->active(%i) and crtc->active (%i)\n",
+		idx, crtc->state->active, intel_crtc->active);
+
+	DRM_DEBUG_ATOMIC("Crtc %i was enabled %i now enabled: %i\n",
+			 idx, was_crtc_enabled, is_crtc_enabled);
+
+	plane_mask = crtc_state->plane_mask | crtc->state->plane_mask;
+	drm_for_each_plane_mask(plane, dev, plane_mask) {
+		int i = drm_plane_index(plane);
+		struct drm_plane_state *plane_state = state->plane_states[i];
+		struct intel_plane_state *old_plane_state;
+		bool turn_off, turn_on, visible, was_visible;
+		struct drm_framebuffer *fb;
+
+		if (!plane_state) {
+			const struct drm_plane_helper_funcs *funcs;
+			int ret;
+
+			if (!mode_changed || !plane->state->fb)
+				continue;
+
+			plane_state = drm_atomic_get_plane_state(state, plane);
+			if (IS_ERR(plane_state))
+				return PTR_ERR(plane_state);
+
+			funcs = plane->helper_private;
+			ret = funcs->atomic_check(plane, plane_state);
+			if (ret)
+				return ret;
+		}
+		old_plane_state = to_intel_plane_state(plane->state);
+
+		was_visible = was_crtc_enabled && (old_plane_state->visible ||
+			      old_plane_state->hw_enabled);
+		visible = to_intel_plane_state(plane_state)->visible &&
+			      is_crtc_enabled;
+
+		if (plane->state->crtc != crtc)
+			was_visible = false;
+		if (plane_state->crtc != crtc)
+			visible = false;
+
+		if (!was_visible && !visible)
+			continue;
+
+		turn_off = was_visible && (!visible || mode_changed);
+		turn_on = visible && (!was_visible || mode_changed);
+		fb = plane_state->fb;
+
+		DRM_DEBUG_ATOMIC("Crtc %i has plane %i with fb %i\n", idx,
+			plane->base.id, fb ? fb->base.id : -1);
+		DRM_DEBUG_ATOMIC("\tvisible %i -> %i, off %i, on %i, ms %i\n",
+			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, plane_state))
+			intel_crtc->atomic.update_wm = true;
+
+		if (INTEL_INFO(dev)->gen >= 9 &&
+		    plane->base.type != DRM_PLANE_TYPE_CURSOR) {
+			ret = skl_update_scaler_users(intel_crtc, pipe_config,
+					to_intel_plane(plane),
+					to_intel_plane_state(plane_state), 0);
+			if (ret)
+				return ret;
+		}
+
+		/*
+		 * 'prepare' is never called when plane is being disabled, so
+		 * we need to handle frontbuffer tracking as a special case
+		 */
+		if (old_plane_state->base.fb && !visible)
+			intel_crtc->atomic.disabled_planes |= 1 << i;
+
+		switch (plane->base.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 = true;
+			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:
+			if (visible)
+				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 << i;
+			}
+			break;
+		}
+	}
+	return 0;
 }
 
 static void intel_begin_crtc_commit(struct drm_crtc *crtc)
@@ -13087,10 +13216,13 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 	intel_runtime_pm_get(dev_priv);
 
 	/* Perform vblank evasion around commit operation */
-	if (intel_crtc->active)
+	if (intel_crtc->active && !needs_modeset(crtc->state) &&
+	    !dev_priv->power_domains.init_power_on)
 		intel_crtc->atomic.evade =
 			intel_pipe_update_start(intel_crtc,
 						&intel_crtc->atomic.start_vbl_count);
+	else
+		intel_crtc->atomic.evade = false;
 }
 
 static void intel_finish_crtc_commit(struct drm_crtc *crtc)
@@ -13099,6 +13231,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_plane *p;
+	unsigned plane_mask;
 
 	if (intel_crtc->atomic.evade)
 		intel_pipe_update_end(intel_crtc,
@@ -13120,12 +13253,13 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
 	if (intel_crtc->atomic.post_enable_primary)
 		intel_post_enable_primary(crtc);
 
-	drm_for_each_legacy_plane(p, &dev->mode_config.plane_list)
-		if (intel_crtc->atomic.update_sprite_watermarks & drm_plane_index(p))
-			intel_update_sprite_watermarks(p, crtc, 0, 0, 0,
-						       false, false);
+	plane_mask = intel_crtc->atomic.update_sprite_watermarks;
+	drm_for_each_plane_mask(p, dev, plane_mask)
+		intel_update_sprite_watermarks(p, crtc, 0, 0, 0, false, false);
 
 	memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
+	crtc->state->mode_changed = false;
+	crtc->state->active_changed = false;
 }
 
 /**
@@ -13238,13 +13372,9 @@ intel_check_cursor_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct intel_crtc *intel_crtc;
 	unsigned stride;
 	int ret;
 
-	crtc = crtc ? crtc : plane->crtc;
-	intel_crtc = to_intel_crtc(crtc);
-
 	ret = drm_plane_helper_check_update(plane, crtc, fb,
 					    src, dest, clip,
 					    DRM_PLANE_HELPER_NO_SCALING,
@@ -13256,7 +13386,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)) {
@@ -13273,19 +13403,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
@@ -13306,20 +13427,26 @@ intel_disable_cursor_plane(struct drm_plane *plane,
 
 static void
 intel_commit_cursor_plane(struct drm_plane *plane,
-			  struct intel_plane_state *state)
+			  struct intel_plane_state *new_state)
 {
-	struct drm_crtc *crtc = state->base.crtc;
+	struct drm_crtc *crtc = new_state->base.crtc;
 	struct drm_device *dev = plane->dev;
 	struct intel_crtc *intel_crtc;
-	struct drm_i915_gem_object *obj = intel_fb_obj(state->base.fb);
+	struct drm_i915_gem_object *obj = intel_fb_obj(new_state->base.fb);
 	uint32_t addr;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
 
-	plane->fb = state->base.fb;
-	crtc->cursor_x = state->base.crtc_x;
-	crtc->cursor_y = state->base.crtc_y;
+	plane->fb = new_state->base.fb;
+	crtc->cursor_x = new_state->base.crtc_x;
+	crtc->cursor_y = new_state->base.crtc_y;
+
+	if (!new_state->visible ||
+	    WARN_ON(new_state->visible && !crtc->state->active)) {
+		intel_disable_cursor_plane(plane, crtc, false);
+		return;
+	}
 
 	if (intel_crtc->cursor_bo == obj)
 		goto update;
@@ -13333,10 +13460,9 @@ intel_commit_cursor_plane(struct drm_plane *plane,
 
 	intel_crtc->cursor_addr = addr;
 	intel_crtc->cursor_bo = obj;
-update:
 
-	if (intel_crtc->active)
-		intel_crtc_update_cursor(crtc, state->visible);
+update:
+	intel_crtc_update_cursor(crtc, new_state->visible);
 }
 
 static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
@@ -14695,7 +14821,14 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev,
 		crtc->base.enabled = crtc->active;
 
 		plane_state = to_intel_plane_state(primary->state);
-		plane_state->visible = primary_get_hw_state(crtc);
+		plane_state->hw_enabled = plane_state->visible =
+			primary_get_hw_state(crtc);
+		if (plane_state->visible)
+			crtc->base.state->plane_mask |=
+				1 << drm_plane_index(primary);
+		else
+			crtc->base.state->plane_mask &=
+				~(1 << drm_plane_index(primary));
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1e892098eea2..9ef89c91aa5c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -235,7 +235,7 @@ struct intel_plane_state {
 	struct drm_rect src;
 	struct drm_rect dst;
 	struct drm_rect clip;
-	bool visible;
+	bool visible, hw_enabled;
 
 	/*
 	 * scaler_id
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 497e7953ad4d..28291ab0993f 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -759,7 +759,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct intel_crtc *intel_crtc = to_intel_crtc(state->base.crtc);
-	struct intel_crtc_state *crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
@@ -771,16 +770,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	int hscale, vscale;
 	int max_scale, min_scale;
 	int pixel_size;
-	int ret;
-
-	intel_crtc = intel_crtc ? intel_crtc : to_intel_crtc(plane->crtc);
-	crtc_state = state->base.state ?
-		intel_atomic_get_crtc_state(state->base.state, intel_crtc) : NULL;
 
-	if (!fb) {
-		state->visible = false;
-		goto finish;
-	}
+	if (!fb)
+		return 0;
 
 	/* Don't modify another pipe's plane */
 	if (intel_plane->pipe != intel_crtc->pipe) {
@@ -803,6 +795,9 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	min_scale = intel_plane->can_scale ? 1 : (1 << 16);
 
 	if (INTEL_INFO(dev)->gen >= 9) {
+		struct intel_crtc_state *crtc_state =
+			intel_atomic_get_crtc_state(state->base.state, intel_crtc);
+
 		min_scale = 1;
 		max_scale = skl_max_scale(intel_crtc, crtc_state);
 	}
@@ -816,7 +811,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
 	BUG_ON(vscale < 0);
 
-	state->visible =  drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
+	state->visible = drm_rect_clip_scaled(src, dst, clip, hscale, vscale);
 
 	crtc_x = dst->x1;
 	crtc_y = dst->y1;
@@ -921,36 +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_users(intel_crtc, crtc_state, intel_plane,
-			state, 0);
-		if (ret)
-			return ret;
-	}
-
 	return 0;
 }
 
@@ -959,7 +924,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
 	struct drm_crtc *crtc = state->base.crtc;
-	struct intel_crtc *intel_crtc;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_framebuffer *fb = state->base.fb;
 	int crtc_x, crtc_y;
@@ -967,26 +931,22 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 	uint32_t src_x, src_y, src_w, src_h;
 
 	crtc = crtc ? crtc : plane->crtc;
-	intel_crtc = to_intel_crtc(crtc);
-
 	plane->fb = fb;
 
-	if (intel_crtc->active) {
-		if (state->visible) {
-			crtc_x = state->dst.x1;
-			crtc_y = state->dst.y1;
-			crtc_w = drm_rect_width(&state->dst);
-			crtc_h = drm_rect_height(&state->dst);
-			src_x = state->src.x1 >> 16;
-			src_y = state->src.y1 >> 16;
-			src_w = drm_rect_width(&state->src) >> 16;
-			src_h = drm_rect_height(&state->src) >> 16;
-			intel_plane->update_plane(plane, crtc, fb,
-						  crtc_x, crtc_y, crtc_w, crtc_h,
-						  src_x, src_y, src_w, src_h);
-		} else {
-			intel_plane->disable_plane(plane, crtc, false);
-		}
+	if (state->visible && crtc->state->active) {
+		crtc_x = state->dst.x1;
+		crtc_y = state->dst.y1;
+		crtc_w = drm_rect_width(&state->dst);
+		crtc_h = drm_rect_height(&state->dst);
+		src_x = state->src.x1 >> 16;
+		src_y = state->src.y1 >> 16;
+		src_w = drm_rect_width(&state->src) >> 16;
+		src_h = drm_rect_height(&state->src) >> 16;
+		intel_plane->update_plane(plane, crtc, fb,
+					  crtc_x, crtc_y, crtc_w, crtc_h,
+					  src_x, src_y, src_w, src_h);
+	} else {
+		intel_plane->disable_plane(plane, crtc, false);
 	}
 }
 
-- 
2.1.0



More information about the Intel-gfx mailing list