drm/i915: push commit_output_state past the crtc/encoder preparing

With this change we can (finally!) rip out a few of the temporary hacks
and clean up a few other things:
- Kill intel_crtc_prepare_encoders, now unused.
- Kill the hacks in the crtc_disable/enable functions to always call the
  encoder callbacks, we now always call the crtc functions with the right
  encoder -> crtc links.
- Also push down the crtc->enable, encoder and connector dpms state
  updates. Unfortunately we can't add a WARN in the crtc_disable
  callbacks to ensure that the crtc is always still enabled when
  disabling an output pipe - the crtc sanitizer of the hw readout path
  can hit this when it needs to disable an active pipe without any
  enabled outputs.
- Only call crtc->disable if the pipe is already enabled - again avoids
  running afoul of the new WARN.

v2: Copy&paste our own version of crtc_in_use, too.

v3: We need to update the dpms an encoder->connectors_active states,
too.

v4: I've forgotten to kill the unconditional encoder->disable calls in
the crtc_disable functions.

v5: Rip out leftover debug printk.

v6: Properly clear intel_encoder->connectors_active. This wasn't
properly cleared when disabling an encoder because it was no longer on
the new connector list, but the crtc was still enabled (i.e. switching
the encoder of an active crtc). Reported by Jani Nikula.

v7: Don't clobber the encoder->connectors_active state of untouched
encoders. Since X likes to first disable all outputs with dpms off
before setting a new framebuffer, this hit a few warnings. Reported by
Paulo Zanoni.

v8: Kill the now stale comment warning that intel_crtc->active is not
always updated at the right times.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

2 files changed, 70 insertions, 41 deletions

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e3d3abff670c..2a393cbe6173 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3221,10 +3221,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	WARN_ON(!crtc->enabled);
 
-	/* XXX: For compatability with the crtc helper code, call the encoder's
-	 * enable function unconditionally for now. */
 	if (intel_crtc->active)
-		goto encoders;
+		return;
 
 	intel_crtc->active = true;
 	intel_update_watermarks(dev);
@@ -3272,7 +3270,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	intel_crtc_update_cursor(crtc, true);
 
-encoders:
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
 
@@ -3290,14 +3287,13 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	int plane = intel_crtc->plane;
 	u32 reg, temp;
 
-	/* XXX: For compatability with the crtc helper code, call the encoder's
-	 * disable function unconditionally for now. */
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->disable(encoder);
 
 	if (!intel_crtc->active)
 		return;
 
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		encoder->disable(encoder);
+
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
 	intel_crtc_update_cursor(crtc, false);
@@ -3399,10 +3395,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	WARN_ON(!crtc->enabled);
 
-	/* XXX: For compatability with the crtc helper code, call the encoder's
-	 * enable function unconditionally for now. */
 	if (intel_crtc->active)
-		goto encoders;
+		return;
 
 	intel_crtc->active = true;
 	intel_update_watermarks(dev);
@@ -3418,7 +3412,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_dpms_overlay(intel_crtc, true);
 	intel_crtc_update_cursor(crtc, true);
 
-encoders:
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
 }
@@ -3432,14 +3425,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
 
-	/* XXX: For compatability with the crtc helper code, call the encoder's
-	 * disable function unconditionally for now. */
-	for_each_encoder_on_crtc(dev, crtc, encoder)
-		encoder->disable(encoder);
 
 	if (!intel_crtc->active)
 		return;
 
+	for_each_encoder_on_crtc(dev, crtc, encoder)
+		encoder->disable(encoder);
+
 	/* Give the overlay scaler a chance to disable if it's on this pipe */
 	intel_crtc_wait_for_pending_flips(crtc);
 	drm_vblank_off(dev, pipe);
@@ -6631,18 +6623,6 @@ static bool intel_encoder_crtc_ok(struct drm_encoder *encoder,
 	return false;
 }
 
-static void
-intel_crtc_prepare_encoders(struct drm_device *dev)
-{
-	struct intel_encoder *encoder;
-
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
-		/* Disable unused encoders */
-		if (encoder->base.crtc == NULL)
-			encoder->disable(encoder);
-	}
-}
-
 /**
  * intel_modeset_update_staged_output_state
  *
@@ -6822,6 +6802,60 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
 	*prepare_pipes &= ~(*disable_pipes);
 }
 
+static bool intel_crtc_in_use(struct drm_crtc *crtc)
+{
+	struct drm_encoder *encoder;
+	struct drm_device *dev = crtc->dev;
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head)
+		if (encoder->crtc == crtc)
+			return true;
+
+	return false;
+}
+
+static void
+intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
+{
+	struct intel_encoder *intel_encoder;
+	struct intel_crtc *intel_crtc;
+	struct drm_connector *connector;
+
+	list_for_each_entry(intel_encoder, &dev->mode_config.encoder_list,
+			    base.head) {
+		if (!intel_encoder->base.crtc)
+			continue;
+
+		intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
+
+		if (prepare_pipes & (1 << intel_crtc->pipe))
+			intel_encoder->connectors_active = false;
+	}
+
+	intel_modeset_commit_output_state(dev);
+
+	/* Update computed state. */
+	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
+			    base.head) {
+		intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base);
+	}
+
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		if (!connector->encoder || !connector->encoder->crtc)
+			continue;
+
+		intel_crtc = to_intel_crtc(connector->encoder->crtc);
+
+		if (prepare_pipes & (1 << intel_crtc->pipe)) {
+			connector->dpms = DRM_MODE_DPMS_ON;
+
+			intel_encoder = to_intel_encoder(connector->encoder);
+			intel_encoder->connectors_active = true;
+		}
+	}
+
+}
+
 #define for_each_intel_crtc_masked(dev, mask, intel_crtc) \
 	list_for_each_entry((intel_crtc), \
 			    &(dev)->mode_config.crtc_list, \
@@ -6850,12 +6884,6 @@ bool intel_set_mode(struct drm_crtc *crtc,
 	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
 		intel_crtc_disable(&intel_crtc->base);
 
-	intel_modeset_commit_output_state(dev);
-
-	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list,
-			    base.head)
-		intel_crtc->base.enabled = drm_helper_crtc_in_use(crtc);
-
 	saved_hwmode = crtc->hwmode;
 	saved_mode = crtc->mode;
 
@@ -6870,12 +6898,12 @@ bool intel_set_mode(struct drm_crtc *crtc,
 		if (IS_ERR(adjusted_mode)) {
 			return false;
 		}
-
-		intel_crtc_prepare_encoders(dev);
 	}
 
-	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc)
-		dev_priv->display.crtc_disable(&intel_crtc->base);
+	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
+		if (intel_crtc->base.enabled)
+			dev_priv->display.crtc_disable(&intel_crtc->base);
+	}
 
 	if (modeset_pipes) {
 		crtc->mode = *mode;
@@ -6883,6 +6911,10 @@ bool intel_set_mode(struct drm_crtc *crtc,
 		crtc->y = y;
 	}
 
+	/* Only after disabling all output pipelines that will be changed can we
+	 * update the the output configuration. */
+	intel_modeset_update_state(dev, prepare_pipes);
+
 	/* Set up the DPLL and any encoders state that needs to adjust or depend
 	 * on the DPLL.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5137f9bef13e..d03e77752157 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -184,9 +184,6 @@ struct intel_crtc {
 	 * Whether the crtc and the connected output pipeline is active. Implies
 	 * that crtc->enabled is set, i.e. the current mode configuration has
 	 * some outputs connected to this crtc.
-	 *
-	 * Atm crtc->enabled is unconditionally updated _before_ the hw state is
-	 * changed, hence we can only check this when enabling the crtc.
 	 */
 	bool active;
 	bool primary_disabled; /* is the crtc obscured by a plane? */