[Intel-gfx] [PATCH 33/42] drm/i915: remove crtc->active tracking completely

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


Small behavioral change: DPLL_DVO_2X_MODE may stay enabled during modeset
for I830 if new state requires it, instead of being disabled and enabled
again.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 140 ++++++++++-------------------------
 drivers/gpu/drm/i915/intel_drv.h     |   6 --
 2 files changed, 40 insertions(+), 106 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7a79659dca00..004067bd0b6c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1680,7 +1680,7 @@ static int intel_num_dvo_pipes(struct drm_device *dev)
 	int count = 0;
 
 	for_each_intel_crtc(dev, crtc)
-		count += crtc->active &&
+		count += crtc->base.state->active &&
 			intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO);
 
 	return count;
@@ -1703,7 +1703,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc, struct intel_crtc_state *pi
 		assert_panel_unlocked(dev_priv, crtc->pipe);
 
 	/* Enable DVO 2x clock on both PLLs if necessary */
-	if (IS_I830(dev) && intel_num_dvo_pipes(dev) > 0) {
+	if (IS_I830(dev) && intel_num_dvo_pipes(dev)) {
 		/*
 		 * It appears to be important that we don't enable this
 		 * for the current pipe before otherwise configuring the
@@ -1762,7 +1762,7 @@ static void i9xx_disable_pll(struct intel_crtc *crtc,
 	/* Disable DVO 2x clock on both PLLs if necessary */
 	if (IS_I830(dev) &&
 	    intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO) &&
-	    intel_num_dvo_pipes(dev) == 1) {
+	    !intel_num_dvo_pipes(dev)) {
 		I915_WRITE(DPLL(PIPE_B),
 			   I915_READ(DPLL(PIPE_B)) & ~DPLL_DVO_2X_MODE);
 		I915_WRITE(DPLL(PIPE_A),
@@ -2583,7 +2583,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 		if (c == &intel_crtc->base)
 			continue;
 
-		if (!i->active)
+		if (!c->state->active)
 			continue;
 
 		fb = c->primary->fb;
@@ -3167,11 +3167,9 @@ static void intel_update_primary_planes(struct drm_device *dev)
 	struct drm_crtc *crtc;
 
 	for_each_crtc(dev, crtc) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
 		drm_modeset_lock(&crtc->mutex, NULL);
 
-		if (intel_crtc->active) {
+		if (crtc->state->active) {
 			const struct intel_plane_state *state =
 				to_intel_plane_state(crtc->primary->state);
 
@@ -4619,7 +4617,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
 	bool reenable_ips = false;
 
 	/* The clocks have to be on to load the palette. */
-	if (!crtc->state->active || !intel_crtc->active)
+	if (!crtc->state->active)
 		return;
 
 	if (HAS_GMCH_DISPLAY(dev_priv->dev)) {
@@ -4778,10 +4776,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe;
 	struct intel_crtc_state *pipe_config;
 
-	WARN_ON(!crtc->state->enable);
+	WARN_ON(!crtc->state->active);
 
-	if (WARN_ON(intel_crtc->active))
-		return;
 	pipe_config = to_intel_crtc_state(crtc->state);
 
 	if (pipe_config->has_pch_encoder)
@@ -4799,8 +4795,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	ironlake_set_pipeconf(crtc);
 
-	intel_crtc->active = true;
-
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 	intel_set_pch_fifo_underrun_reporting(dev_priv, pipe, true);
 
@@ -4808,7 +4802,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_enable)
 			encoder->pre_enable(encoder);
 
-	if (intel_crtc->config->has_pch_encoder) {
+	if (pipe_config->has_pch_encoder) {
 		/* Note: FDI PLL enabling _must_ be done before we enable the
 		 * cpu pipes, hence this is separate from all the other fdi/pch
 		 * enabling. */
@@ -4859,9 +4853,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	WARN_ON(!crtc->state->enable);
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	pipe_config = to_intel_crtc_state(crtc->state);
 	if (intel_crtc_to_shared_dpll(intel_crtc))
 		intel_enable_shared_dpll(intel_crtc);
@@ -4885,8 +4876,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_set_pipe_csc(crtc);
 
-	intel_crtc->active = true;
-
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		if (encoder->pre_enable)
@@ -5743,8 +5732,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	WARN_ON(!crtc->state->active);
 
-	if (WARN_ON(intel_crtc->active))
-		return;
 	pipe_config = to_intel_crtc_state(crtc->state);
 
 	is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
@@ -5770,8 +5757,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	intel_crtc->active = true;
-
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
@@ -5824,9 +5809,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 	pipe_config = to_intel_crtc_state(crtc->state);
 	WARN_ON(!pipe_config->base.active);
 
-	if (WARN_ON(intel_crtc->active))
-		return;
-
 	i9xx_set_pll_dividers(intel_crtc);
 
 	if (intel_crtc->config->has_dp_encoder)
@@ -5836,8 +5818,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_set_pipeconf(intel_crtc);
 
-	intel_crtc->active = true;
-
 	if (!IS_GEN2(dev))
 		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 
@@ -5930,7 +5910,7 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)
 	struct drm_atomic_state *state;
 	int ret;
 
-	if (enable == intel_crtc->active)
+	if (enable == crtc->state->active)
 		return;
 
 	if (enable && !crtc->state->enable)
@@ -6045,9 +6025,9 @@ static void intel_connector_check_state(struct intel_connector *connector)
 
 			crtc = encoder->base.crtc;
 
-			I915_STATE_WARN(!crtc->state->active,
+			I915_STATE_WARN(!crtc->state->enable,
 					"crtc not enabled\n");
-			I915_STATE_WARN(!to_intel_crtc(crtc)->active, "crtc not active\n");
+			I915_STATE_WARN(!crtc->state->active, "crtc not active\n");
 			I915_STATE_WARN(pipe != to_intel_crtc(crtc)->pipe,
 			     "encoder active on the wrong pipe\n");
 		}
@@ -8798,7 +8778,7 @@ static void assert_can_disable_lcpll(struct drm_i915_private *dev_priv)
 	struct intel_crtc *crtc;
 
 	for_each_intel_crtc(dev, crtc)
-		I915_STATE_WARN(crtc->active, "CRTC for pipe %c enabled\n",
+		I915_STATE_WARN(crtc->base.state->active, "CRTC for pipe %c enabled\n",
 		     pipe_name(crtc->pipe));
 
 	I915_STATE_WARN(I915_READ(HSW_PWR_WELL_DRIVER), "Power well on\n");
@@ -11738,7 +11718,7 @@ static void check_wm_state(struct drm_device *dev)
 		struct skl_ddb_entry *hw_entry, *sw_entry;
 		const enum pipe pipe = intel_crtc->pipe;
 
-		if (!intel_crtc->active)
+		if (!intel_crtc->base.state->active)
 			continue;
 
 		/* planes */
@@ -11871,9 +11851,6 @@ check_crtc_state(struct drm_device *dev)
 		DRM_DEBUG_KMS("[CRTC:%d]\n",
 			      crtc->base.base.id);
 
-		I915_STATE_WARN(crtc->active && !crtc->base.state->enable,
-		     "active crtc, but not enabled in sw tracking\n");
-
 		for_each_intel_encoder(dev, encoder) {
 			if (encoder->base.crtc != &crtc->base)
 				continue;
@@ -11882,9 +11859,9 @@ check_crtc_state(struct drm_device *dev)
 				active = true;
 		}
 
-		I915_STATE_WARN(active != crtc->active,
+		I915_STATE_WARN(active != crtc->base.state->active,
 		     "crtc's computed active state doesn't match tracked active state "
-		     "(expected %i, found %i)\n", active, crtc->active);
+		     "(expected %i, found %i)\n", active, crtc->base.state->active);
 		I915_STATE_WARN(enabled != crtc->base.state->enable,
 		     "crtc's computed enabled state doesn't match tracked enabled state "
 		     "(expected %i, found %i)\n", enabled,
@@ -11896,7 +11873,7 @@ check_crtc_state(struct drm_device *dev)
 		/* hw state is inconsistent with the pipe quirk */
 		if ((crtc->pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) ||
 		    (crtc->pipe == PIPE_B && dev_priv->quirks & QUIRK_PIPEB_FORCE))
-			active = crtc->active;
+			active = crtc->base.state->active;
 
 		for_each_intel_encoder(dev, encoder) {
 			enum pipe pipe;
@@ -11906,9 +11883,9 @@ check_crtc_state(struct drm_device *dev)
 				encoder->get_config(encoder, &pipe_config);
 		}
 
-		I915_STATE_WARN(crtc->active != active,
+		I915_STATE_WARN(crtc->base.state->active != active,
 		     "crtc active state doesn't match with hw state "
-		     "(expected %i, found %i)\n", crtc->active, active);
+		     "(expected %i, found %i)\n", crtc->base.state->active, active);
 
 		if (active &&
 		    !intel_pipe_config_compare(dev, crtc->config, &pipe_config)) {
@@ -11931,7 +11908,7 @@ check_shared_dpll_state(struct drm_device *dev)
 
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
-		int enabled_crtcs = 0, active_crtcs = 0;
+		int active_crtcs = 0;
 		bool active;
 
 		memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
@@ -11953,16 +11930,14 @@ check_shared_dpll_state(struct drm_device *dev)
 
 		for_each_intel_crtc(dev, crtc) {
 			if (crtc->base.state->active && intel_crtc_to_shared_dpll(crtc) == pll)
-				enabled_crtcs++;
-			if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll)
 				active_crtcs++;
 		}
 		I915_STATE_WARN(pll->active != active_crtcs,
 		     "pll active crtcs mismatch (expected %i, found %i)\n",
 		     pll->active, active_crtcs);
-		I915_STATE_WARN(hweight32(pll->config.crtc_mask) != enabled_crtcs,
+		I915_STATE_WARN(hweight32(pll->config.crtc_mask) != active_crtcs,
 		     "pll enabled crtcs mismatch (expected %i, found %i)\n",
-		     hweight32(pll->config.crtc_mask), enabled_crtcs);
+		     hweight32(pll->config.crtc_mask), active_crtcs);
 
 		I915_STATE_WARN(pll->on && memcmp(&pll->config.hw_state, &dpll_hw_state,
 				       sizeof(dpll_hw_state)),
@@ -12264,10 +12239,10 @@ static void __intel_set_mode_update_planes(struct drm_device *dev,
 		to_intel_plane_state(plane->state)->hw_enabled = false;
 		DRM_DEBUG_ATOMIC("Plane %i is visible: %i\n", i, visible);
 
-		if (!visible)
+		if (crtc && needs_modeset(crtc->state))
+			intel_plane->disable_plane(plane, crtc, !visible);
+		else if (!visible)
 			funcs->atomic_update(plane, old_plane_state);
-		else if (needs_modeset(crtc->state))
-			intel_plane->disable_plane(plane, crtc, true);
 	}
 }
 
@@ -12349,8 +12324,6 @@ static int __intel_set_mode(struct drm_atomic_state *state)
 		intel_crtc_dpms_overlay_disable(to_intel_crtc(crtc));
 		dev_priv->display.crtc_disable(crtc, pipe_config);
 
-		intel_crtc->active = false;
-
 		if (intel_crtc_to_shared_dpll(intel_crtc))
 			intel_disable_shared_dpll(intel_crtc);
 	}
@@ -13035,11 +13008,9 @@ static int intel_atomic_check_crtc(struct drm_crtc *crtc,
 	memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
 	intel_crtc->atomic.update_wm = mode_changed;
 	intel_crtc->atomic.disable_fbc = mode_changed;
+	intel_crtc->atomic.evade = !mode_changed && was_crtc_enabled;
 
 	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);
@@ -13229,7 +13200,7 @@ 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 && !needs_modeset(crtc->state) &&
+	if (intel_crtc->atomic.evade &&
 	    !dev_priv->power_domains.init_power_on)
 		intel_crtc->atomic.evade =
 			intel_pipe_update_start(intel_crtc,
@@ -14537,7 +14508,7 @@ void intel_modeset_init(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 
 	for_each_intel_crtc(dev, crtc) {
-		if (!crtc->active)
+		if (!crtc->base.state->active)
 			continue;
 
 		/*
@@ -14615,7 +14586,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 
 	/* restore vblank interrupts to correct state */
 	drm_crtc_vblank_reset(&crtc->base);
-	if (crtc->active) {
+	if (crtc->base.state->active) {
 		update_scanline_offset(crtc);
 		drm_crtc_vblank_on(&crtc->base);
 	}
@@ -14655,14 +14626,14 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 				connector->encoder->connectors_active = false;
 			}
 
-		WARN_ON(crtc->active);
+		WARN_ON(crtc->base.state->active);
 		crtc->base.state->enable = false;
 		crtc->base.state->active = false;
 		crtc->base.enabled = false;
 	}
 
 	if (dev_priv->quirks & QUIRK_PIPEA_FORCE &&
-	    crtc->pipe == PIPE_A && !crtc->active) {
+	    crtc->pipe == PIPE_A && !crtc->base.state->active) {
 		/* BIOS forgot to enable pipe A, this mostly happens after
 		 * resume. Force-enable the pipe to fix this, the update_dpms
 		 * call below we restore the pipe to the right state, but leave
@@ -14674,35 +14645,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 	 * have active connectors/encoders. */
 	intel_crtc_update_dpms(&crtc->base);
 
-	if (crtc->active != crtc->base.state->active) {
-		struct intel_encoder *encoder;
-
-		/* This can happen either due to bugs in the get_hw_state
-		 * functions or because the pipe is force-enabled due to the
-		 * pipe A quirk. */
-		DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n",
-			      crtc->base.base.id,
-			      crtc->base.state->enable ? "enabled" : "disabled",
-			      crtc->active ? "enabled" : "disabled");
-
-		crtc->base.state->enable = crtc->active;
-		crtc->base.state->active = crtc->active;
-		crtc->base.enabled = crtc->active;
-
-		/* Because we only establish the connector -> encoder ->
-		 * crtc links if something is active, this means the
-		 * crtc is now deactivated. Break the links. connector
-		 * -> encoder links are only establish when things are
-		 *  actually up, hence no need to break them. */
-		WARN_ON(crtc->active);
-
-		for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
-			WARN_ON(encoder->connectors_active);
-			encoder->base.crtc = NULL;
-		}
-	}
-
-	if (crtc->active || HAS_GMCH_DISPLAY(dev)) {
+	if (crtc->base.state->active || HAS_GMCH_DISPLAY(dev)) {
 		/*
 		 * We start out with underrun reporting disabled to avoid races.
 		 * For correct bookkeeping mark this on active crtcs.
@@ -14730,7 +14673,7 @@ static void intel_sanitize_encoder(struct intel_encoder *encoder)
 	/* We need to check both for a crtc link (meaning that the
 	 * encoder is active and trying to read from a pipe) and the
 	 * pipe itself being active. */
-	bool has_active_crtc = crtc && to_intel_crtc(crtc)->active;
+	bool has_active_crtc = crtc && crtc->state->active;
 
 	if (encoder->connectors_active && !has_active_crtc) {
 		DRM_DEBUG_KMS("[ENCODER:%d:%s] has active connectors but no active pipe!\n",
@@ -14800,7 +14743,7 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 
-	if (!crtc->active)
+	if (!crtc->base.state->active)
 		return false;
 
 	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
@@ -14828,12 +14771,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev,
 		if (crtc->base.state->active)
 			*crtc_mask |= drm_crtc_index(&crtc->base);
 
-		crtc->active = dev_priv->display.get_pipe_config(crtc,
-								 crtc->config);
-
-		crtc->base.state->enable = crtc->active;
-		crtc->base.state->active = crtc->active;
-		crtc->base.enabled = crtc->active;
+		crtc->base.enabled = crtc->base.state->enable =
+		crtc->base.state->active =
+			dev_priv->display.get_pipe_config(crtc, crtc->config);
 
 		plane_state = to_intel_plane_state(primary->state);
 		plane_state->hw_enabled = plane_state->visible =
@@ -14847,7 +14787,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev,
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,
-			      crtc->active ? "enabled" : "disabled");
+			      crtc->base.state->active ? "enabled" : "disabled");
 	}
 
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
@@ -14858,7 +14798,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev,
 		pll->active = 0;
 		pll->config.crtc_mask = 0;
 		for_each_intel_crtc(dev, crtc) {
-			if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
+			if (crtc->base.state->active && intel_crtc_to_shared_dpll(crtc) == pll) {
 				pll->active++;
 				pll->config.crtc_mask |= 1 << crtc->pipe;
 			}
@@ -14927,9 +14867,9 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 	 * checking everywhere.
 	 */
 	for_each_intel_crtc(dev, crtc) {
-		if (crtc->active && i915.fastboot) {
+		if (crtc->base.state->active && i915.fastboot) {
 			intel_mode_from_pipe_config(&crtc->base.mode,
-						    crtc->config);
+				to_intel_crtc_state(crtc->base.state));
 			DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
 				      crtc->base.base.id);
 			drm_mode_debug_printmodeline(&crtc->base.mode);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 6c2ba5dbcd79..1a0b0cd857c3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -492,12 +492,6 @@ struct intel_crtc {
 	enum pipe pipe;
 	enum plane plane;
 	u8 lut_r[256], lut_g[256], lut_b[256];
-	/*
-	 * 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.
-	 */
-	bool active;
 	unsigned long enabled_power_domains;
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
-- 
2.1.0



More information about the Intel-gfx mailing list