[PATCH 0/2] Revert atomic hw readout.

[PATCH 0/2] Revert atomic hw readout.

[Maarten Lankhorst]

Seems this is causing too many issues, revert temporarily until the issues are fixed.

This might break some of the changes I made using atomic state:
  Can someone test on haswell with multiple crtcs,
  to see if the haswell plane workaround change still works?
  "drm/i915: Calculate haswell plane workaround, v5."

  And i830 with DVO? For DVO_2X_MODE.
  "drm/i915: Use atomic state for calculating DVO_2X_MODE on i830."

Maarten Lankhorst (2):
  Revert "drm/i915: Make intel_display_suspend atomic, v2."
  Revert "drm/i915: Read hw state into an atomic state struct, v2."

 drivers/gpu/drm/i915/i915_drv.c      |   3 -
 drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 434 ++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_drv.h     |  16 +-
 4 files changed, 165 insertions(+), 290 deletions(-)

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

[PATCH 1/2] Revert "drm/i915: Make intel_display_suspend atomic, v2."

[Maarten Lankhorst]

This reverts commit 490f400db5d886fc28566af69b02f6497f31be4b.

We're not ready yet to make it atomic, we calculate some state in
advance, but without atomic plane support atomic the hw readout will
fail.

It's required to revert this commit to revert the atomic hw
state readout patch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90868
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90861

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  3 --
 drivers/gpu/drm/i915/intel_display.c | 55 ++++++++----------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 3 files changed, 13 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d3632c56fdf7..78ef0bb53c36 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -634,9 +634,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 	intel_display_suspend(dev);
 	drm_modeset_unlock_all(dev);
 
-	/* suspending displays will unsets init power */
-	intel_display_set_init_power(dev_priv, true);
-
 	intel_dp_mst_suspend(dev);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c38c297f6a41..350eee22b9e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6177,58 +6177,27 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
  * turn all crtc's off, but do not adjust state
  * This has to be paired with a call to intel_modeset_setup_hw_state.
  */
-int intel_display_suspend(struct drm_device *dev)
+void intel_display_suspend(struct drm_device *dev)
 {
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
-	struct drm_atomic_state *state;
+	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *crtc;
-	unsigned crtc_mask = 0;
-	int ret = 0;
-
-	if (WARN_ON(!ctx))
-		return 0;
-
-	lockdep_assert_held(&ctx->ww_ctx);
-	state = drm_atomic_state_alloc(dev);
-	if (WARN_ON(!state))
-		return -ENOMEM;
-
-	state->acquire_ctx = ctx;
-	state->allow_modeset = true;
 
 	for_each_crtc(dev, crtc) {
-		struct drm_crtc_state *crtc_state =
-			drm_atomic_get_crtc_state(state, crtc);
-
-		ret = PTR_ERR_OR_ZERO(crtc_state);
-		if (ret)
-			goto free;
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		enum intel_display_power_domain domain;
+		unsigned long domains;
 
-		if (!crtc_state->active)
+		if (!intel_crtc->active)
 			continue;
 
-		crtc_state->active = false;
-		crtc_mask |= 1 << drm_crtc_index(crtc);
-	}
-
-	if (crtc_mask) {
-		ret = intel_set_mode(state);
-
-		if (!ret) {
-			for_each_crtc(dev, crtc)
-				if (crtc_mask & (1 << drm_crtc_index(crtc)))
-					crtc->state->active = true;
+		intel_crtc_disable_planes(crtc);
+		dev_priv->display.crtc_disable(crtc);
 
-			return ret;
-		}
+		domains = intel_crtc->enabled_power_domains;
+		for_each_power_domain(domain, domains)
+			intel_display_power_put(dev_priv, domain);
+		intel_crtc->enabled_power_domains = 0;
 	}
-
-free:
-	if (ret)
-		DRM_ERROR("Suspending crtc's failed with %i\n", ret);
-	drm_atomic_state_free(state);
-	return ret;
 }
 
 /* Master function to enable/disable CRTC and corresponding power wells */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb3a30c1e7cc..58bd0edba2a4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -992,7 +992,7 @@ int intel_pch_rawclk(struct drm_device *dev);
 void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
-int intel_display_suspend(struct drm_device *dev);
+void intel_display_suspend(struct drm_device *dev);
 int intel_crtc_control(struct drm_crtc *crtc, bool enable);
 void intel_crtc_update_dpms(struct drm_crtc *crtc);
 void intel_encoder_destroy(struct drm_encoder *encoder);
-- 
2.1.0

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

[PATCH 2/2] Revert "drm/i915: Read hw state into an atomic state struct, v2."

[Maarten Lankhorst]

This reverts commit 3bae26eb2991c00670df377cf6c3bc2b0577e82a.

Seems it introduces regressions for 3 different reasons, oh boy..

In bug #90868 as I can see the atomic state will be restored on
resume without the planes being set up properly. Because plane
setup here requires the atomic state, we'll have to settle
for committing atomic planes first.

In bug #90861 the failure appears to affect mostly DP devices,
and happens because reading out the atomic state prevents a modeset
on boot, which would require better hw state readout.

In bug #90874 it's shown that cdclk should be part of the atomic
state, so only performing a single modeset during resume excarbated
the issue.

It's better to fix those issues first, and then commit this patch,
so do that temporarily.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90868
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90861
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90874
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 379 +++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h     |  14 +-
 3 files changed, 152 insertions(+), 243 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index e47e00e5b130..4df6d2d7a9c8 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -395,7 +395,7 @@ int intel_atomic_setup_scalers(struct drm_device *dev,
 	return 0;
 }
 
-void
+static void
 intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv,
 				  struct intel_shared_dpll_config *shared_dpll)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 350eee22b9e2..e5c2dd72952b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10288,7 +10288,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 retry:
 	ret = drm_modeset_lock(&config->connection_mutex, ctx);
 	if (ret)
-		goto fail;
+		goto fail_unlock;
 
 	/*
 	 * Algorithm gets a little messy:
@@ -10306,10 +10306,10 @@ retry:
 
 		ret = drm_modeset_lock(&crtc->mutex, ctx);
 		if (ret)
-			goto fail;
+			goto fail_unlock;
 		ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
 		if (ret)
-			goto fail;
+			goto fail_unlock;
 
 		old->dpms_mode = connector->dpms;
 		old->load_detect_temp = false;
@@ -10328,6 +10328,9 @@ retry:
 			continue;
 		if (possible_crtc->state->enable)
 			continue;
+		/* This can occur when applying the pipe A quirk on resume. */
+		if (to_intel_crtc(possible_crtc)->new_enabled)
+			continue;
 
 		crtc = possible_crtc;
 		break;
@@ -10338,17 +10341,20 @@ retry:
 	 */
 	if (!crtc) {
 		DRM_DEBUG_KMS("no pipe available for load-detect\n");
-		goto fail;
+		goto fail_unlock;
 	}
 
 	ret = drm_modeset_lock(&crtc->mutex, ctx);
 	if (ret)
-		goto fail;
+		goto fail_unlock;
 	ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
 	if (ret)
-		goto fail;
+		goto fail_unlock;
+	intel_encoder->new_crtc = to_intel_crtc(crtc);
+	to_intel_connector(connector)->new_encoder = intel_encoder;
 
 	intel_crtc = to_intel_crtc(crtc);
+	intel_crtc->new_enabled = true;
 	old->dpms_mode = connector->dpms;
 	old->load_detect_temp = true;
 	old->release_fb = NULL;
@@ -10416,7 +10422,9 @@ retry:
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 	return true;
 
-fail:
+ fail:
+	intel_crtc->new_enabled = crtc->state->enable;
+fail_unlock:
 	drm_atomic_state_free(state);
 	state = NULL;
 
@@ -10462,6 +10470,10 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (IS_ERR(crtc_state))
 			goto fail;
 
+		to_intel_connector(connector)->new_encoder = NULL;
+		intel_encoder->new_crtc = NULL;
+		intel_crtc->new_enabled = false;
+
 		connector_state->best_encoder = NULL;
 		connector_state->crtc = NULL;
 
@@ -11608,6 +11620,33 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.atomic_flush = intel_finish_crtc_commit,
 };
 
+/**
+ * intel_modeset_update_staged_output_state
+ *
+ * Updates the staged output configuration state, e.g. after we've read out the
+ * current hw state.
+ */
+static void intel_modeset_update_staged_output_state(struct drm_device *dev)
+{
+	struct intel_crtc *crtc;
+	struct intel_encoder *encoder;
+	struct intel_connector *connector;
+
+	for_each_intel_connector(dev, connector) {
+		connector->new_encoder =
+			to_intel_encoder(connector->base.encoder);
+	}
+
+	for_each_intel_encoder(dev, encoder) {
+		encoder->new_crtc =
+			to_intel_crtc(encoder->base.crtc);
+	}
+
+	for_each_intel_crtc(dev, crtc) {
+		crtc->new_enabled = crtc->base.state->enable;
+	}
+}
+
 /* Transitional helper to copy current connector/encoder state to
  * connector->state. This is needed so that code that is partially
  * converted to atomic does the right thing.
@@ -12137,6 +12176,7 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 	}
 
 	drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
+	intel_modeset_update_staged_output_state(state->dev);
 
 	/* Double check state. */
 	for_each_crtc(dev, crtc) {
@@ -12446,14 +12486,11 @@ check_connector_state(struct drm_device *dev)
 	struct intel_connector *connector;
 
 	for_each_intel_connector(dev, connector) {
-		struct drm_encoder *encoder = connector->base.encoder;
-		struct drm_connector_state *state = connector->base.state;
-
 		/* This also checks the encoder/connector hw state with the
 		 * ->get_hw_state callbacks. */
 		intel_connector_check_state(connector);
 
-		I915_STATE_WARN(state->best_encoder != encoder,
+		I915_STATE_WARN(&connector->new_encoder->base != connector->base.encoder,
 		     "connector's staged encoder doesn't match current encoder\n");
 	}
 }
@@ -12473,6 +12510,8 @@ check_encoder_state(struct drm_device *dev)
 			      encoder->base.base.id,
 			      encoder->base.name);
 
+		I915_STATE_WARN(&encoder->new_crtc->base != encoder->base.crtc,
+		     "encoder's stage crtc doesn't match current crtc\n");
 		I915_STATE_WARN(encoder->connectors_active && !encoder->base.crtc,
 		     "encoder's active_connectors set, but no crtc\n");
 
@@ -12482,9 +12521,6 @@ check_encoder_state(struct drm_device *dev)
 			enabled = true;
 			if (connector->base.dpms != DRM_MODE_DPMS_OFF)
 				active = true;
-
-			I915_STATE_WARN(connector->base.state->crtc != encoder->base.crtc,
-			     "encoder's stage crtc doesn't match current crtc\n");
 		}
 		/*
 		 * for MST connectors if we unplug the connector is gone
@@ -12988,11 +13024,11 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	 * need to copy the staged config to the atomic state, otherwise the
 	 * mode set will just reapply the state the HW is already in. */
 	for_each_intel_encoder(dev, encoder) {
-		if (encoder->base.crtc != crtc)
+		if (&encoder->new_crtc->base != crtc)
 			continue;
 
 		for_each_intel_connector(dev, connector) {
-			if (connector->base.state->best_encoder != &encoder->base)
+			if (connector->new_encoder != encoder)
 				continue;
 
 			connector_state = drm_atomic_get_connector_state(state, &connector->base);
@@ -13005,10 +13041,14 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 			}
 
 			connector_state->crtc = crtc;
+			connector_state->best_encoder = &encoder->base;
 		}
 	}
 
 	for_each_intel_crtc(dev, intel_crtc) {
+		if (intel_crtc->new_enabled == intel_crtc->base.enabled)
+			continue;
+
 		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
 		if (IS_ERR(crtc_state)) {
 			DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
@@ -13017,6 +13057,9 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 			continue;
 		}
 
+		crtc_state->base.active = crtc_state->base.enable =
+			intel_crtc->new_enabled;
+
 		if (&intel_crtc->base == crtc)
 			drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
 	}
@@ -15093,7 +15136,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		 * ...  */
 		plane = crtc->plane;
 		to_intel_plane_state(crtc->base.primary->state)->visible = true;
-		crtc->base.primary->crtc = &crtc->base;
 		crtc->plane = !plane;
 		intel_crtc_control(&crtc->base, false);
 		crtc->plane = plane;
@@ -15257,182 +15299,78 @@ static bool primary_get_hw_state(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 
-	if (!crtc->base.enabled)
+	if (!crtc->active)
 		return false;
 
 	return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
 }
 
-static int readout_hw_crtc_state(struct drm_atomic_state *state,
-				 struct intel_crtc *crtc)
+static void intel_modeset_readout_hw_state(struct drm_device *dev)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_crtc_state *crtc_state;
-	struct drm_plane *primary = crtc->base.primary;
-	struct drm_plane_state *drm_plane_state;
-	struct intel_plane_state *plane_state;
-	int ret;
-
-	crtc_state = intel_atomic_get_crtc_state(state, crtc);
-	if (IS_ERR(crtc_state))
-		return PTR_ERR(crtc_state);
-
-	ret = drm_atomic_add_affected_planes(state, &crtc->base);
-	if (ret)
-		return ret;
-
-	memset(crtc_state, 0, sizeof(*crtc_state));
-	crtc_state->base.crtc = &crtc->base;
-	crtc_state->base.state = state;
-
-	crtc_state->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
-
-	crtc_state->base.enable = crtc_state->base.active =
-	crtc->base.enabled = dev_priv->display.get_pipe_config(crtc, crtc_state);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe;
+	struct intel_crtc *crtc;
+	struct intel_encoder *encoder;
+	struct intel_connector *connector;
+	int i;
 
-	/* update transitional state */
-	crtc->active = crtc_state->base.active;
-	crtc->config = crtc_state;
+	for_each_intel_crtc(dev, crtc) {
+		struct drm_plane *primary = crtc->base.primary;
+		struct intel_plane_state *plane_state;
 
-	drm_plane_state = drm_atomic_get_plane_state(state, primary);
-	if (IS_ERR(drm_plane_state))
-		return PTR_ERR(drm_plane_state);
+		memset(crtc->config, 0, sizeof(*crtc->config));
+		crtc->config->base.crtc = &crtc->base;
 
-	plane_state = to_intel_plane_state(drm_plane_state);
-	plane_state->visible = primary_get_hw_state(crtc);
+		crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
 
-	if (plane_state->visible) {
-		primary->crtc = &crtc->base;
-		crtc_state->base.plane_mask |= 1 << drm_plane_index(primary);
-	} else
-		crtc_state->base.plane_mask &= ~(1 << drm_plane_index(primary));
+		crtc->active = dev_priv->display.get_pipe_config(crtc,
+								 crtc->config);
 
-	DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
-		      crtc->base.base.id,
-		      crtc_state->base.active ? "enabled" : "disabled");
+		crtc->base.state->enable = crtc->active;
+		crtc->base.state->active = crtc->active;
+		crtc->base.enabled = crtc->active;
 
-	return 0;
-}
+		plane_state = to_intel_plane_state(primary->state);
+		plane_state->visible = primary_get_hw_state(crtc);
 
-static int readout_hw_pll_state(struct drm_atomic_state *state)
-{
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_shared_dpll_config *shared_dpll;
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
-	int i;
+		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
+			      crtc->base.base.id,
+			      crtc->active ? "enabled" : "disabled");
+	}
 
-	shared_dpll = intel_atomic_get_shared_dpll_state(state);
 	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
 		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
 
 		pll->on = pll->get_hw_state(dev_priv, pll,
-					    &shared_dpll[i].hw_state);
-
+					    &pll->config.hw_state);
 		pll->active = 0;
-		shared_dpll[i].crtc_mask = 0;
-
-		for_each_intel_crtc(state->dev, crtc) {
-			crtc_state = intel_atomic_get_crtc_state(state, crtc);
-			if (IS_ERR(crtc_state))
-				return PTR_ERR(crtc_state);
-
-			if (crtc_state->base.active &&
-			    crtc_state->shared_dpll == i) {
+		pll->config.crtc_mask = 0;
+		for_each_intel_crtc(dev, crtc) {
+			if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
 				pll->active++;
-				shared_dpll[i].crtc_mask |=
-					1 << crtc->pipe;
+				pll->config.crtc_mask |= 1 << crtc->pipe;
 			}
 		}
 
 		DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
-			      pll->name, shared_dpll[i].crtc_mask,
-			      pll->on);
+			      pll->name, pll->config.crtc_mask, pll->on);
 
-		if (shared_dpll[i].crtc_mask)
+		if (pll->config.crtc_mask)
 			intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
 	}
 
-	return 0;
-}
-
-static struct drm_connector_state *
-get_connector_state_for_encoder(struct drm_atomic_state *state,
-				struct intel_encoder *encoder)
-{
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int i;
-
-	for_each_connector_in_state(state, connector, connector_state, i)
-		if (connector_state->best_encoder == &encoder->base)
-			return connector_state;
-
-	return NULL;
-}
-
-static int readout_hw_connector_encoder_state(struct drm_atomic_state *state)
-{
-	struct drm_device *dev = state->dev;
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_crtc *crtc;
-	struct drm_crtc_state *drm_crtc_state;
-	struct intel_crtc_state *crtc_state;
-	struct intel_encoder *encoder;
-	struct intel_connector *connector;
-	struct drm_connector_state *connector_state;
-	enum pipe pipe;
-
-	for_each_intel_connector(dev, connector) {
-		connector_state =
-			drm_atomic_get_connector_state(state, &connector->base);
-		if (IS_ERR(connector_state))
-			return PTR_ERR(connector_state);
-
-		if (connector->get_hw_state(connector)) {
-			connector->base.dpms = DRM_MODE_DPMS_ON;
-			connector->base.encoder = &connector->encoder->base;
-		} else {
-			connector->base.dpms = DRM_MODE_DPMS_OFF;
-			connector->base.encoder = NULL;
-		}
-
-		/* We'll update the crtc field when reading encoder state */
-		connector_state->crtc = NULL;
-
-		connector_state->best_encoder = connector->base.encoder;
-
-		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] hw state readout: %s\n",
-			      connector->base.base.id,
-			      connector->base.name,
-			      connector->base.encoder ? "enabled" : "disabled");
-	}
-
 	for_each_intel_encoder(dev, encoder) {
 		pipe = 0;
 
-		connector_state =
-			get_connector_state_for_encoder(state, encoder);
-
-		encoder->connectors_active = !!connector_state;
-
 		if (encoder->get_hw_state(encoder, &pipe)) {
-			encoder->base.crtc =
-				dev_priv->pipe_to_crtc_mapping[pipe];
-			crtc = to_intel_crtc(encoder->base.crtc);
-
-			drm_crtc_state =
-				state->crtc_states[drm_crtc_index(&crtc->base)];
-			crtc_state = to_intel_crtc_state(drm_crtc_state);
-
-			encoder->get_config(encoder, crtc_state);
-
-			if (connector_state)
-				connector_state->crtc = &crtc->base;
+			crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+			encoder->base.crtc = &crtc->base;
+			encoder->get_config(encoder, crtc->config);
 		} else {
 			encoder->base.crtc = NULL;
 		}
 
+		encoder->connectors_active = false;
 		DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
 			      encoder->base.base.id,
 			      encoder->base.name,
@@ -15440,42 +15378,20 @@ static int readout_hw_connector_encoder_state(struct drm_atomic_state *state)
 			      pipe_name(pipe));
 	}
 
-	return 0;
-}
-
-static struct drm_atomic_state *
-intel_modeset_readout_hw_state(struct drm_device *dev)
-{
-	struct intel_crtc *crtc;
-	int ret = 0;
-
-	struct drm_atomic_state *state;
-
-	state = drm_atomic_state_alloc(dev);
-	if (!state)
-		return ERR_PTR(-ENOMEM);
-
-	state->acquire_ctx = dev->mode_config.acquire_ctx;
-
-	for_each_intel_crtc(dev, crtc) {
-		ret = readout_hw_crtc_state(state, crtc);
-		if (ret)
-			goto err_free;
+	for_each_intel_connector(dev, connector) {
+		if (connector->get_hw_state(connector)) {
+			connector->base.dpms = DRM_MODE_DPMS_ON;
+			connector->encoder->connectors_active = true;
+			connector->base.encoder = &connector->encoder->base;
+		} else {
+			connector->base.dpms = DRM_MODE_DPMS_OFF;
+			connector->base.encoder = NULL;
+		}
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] hw state readout: %s\n",
+			      connector->base.base.id,
+			      connector->base.name,
+			      connector->base.encoder ? "enabled" : "disabled");
 	}
-
-	ret = readout_hw_pll_state(state);
-	if (ret)
-		goto err_free;
-
-	ret = readout_hw_connector_encoder_state(state);
-	if (ret)
-		goto err_free;
-
-	return state;
-
-err_free:
-	drm_atomic_state_free(state);
-	return ERR_PTR(ret);
 }
 
 /* Scan out the current hw modeset state, sanitizes it and maps it into the drm
@@ -15484,57 +15400,37 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 				  bool force_restore)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	enum pipe pipe;
+	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
-	struct drm_atomic_state *state;
-	struct intel_shared_dpll_config shared_dplls[I915_NUM_PLLS];
 	int i;
 
-	state = intel_modeset_readout_hw_state(dev);
-	if (IS_ERR(state)) {
-		DRM_ERROR("Failed to read out hw state\n");
-		return;
-	}
-
-	drm_atomic_helper_swap_state(dev, state);
+	intel_modeset_readout_hw_state(dev);
 
-	/* swap sw/hw dpll state */
-	intel_atomic_duplicate_dpll_state(dev_priv, shared_dplls);
-	intel_shared_dpll_commit(state);
-	memcpy(to_intel_atomic_state(state)->shared_dpll,
-	       shared_dplls, sizeof(*shared_dplls) * dev_priv->num_shared_dpll);
+	/*
+	 * Now that we have the config, copy it to each CRTC struct
+	 * Note that this could go away if we move to using crtc_config
+	 * checking everywhere.
+	 */
+	for_each_intel_crtc(dev, crtc) {
+		if (crtc->active && i915.fastboot) {
+			intel_mode_from_pipe_config(&crtc->base.mode,
+						    crtc->config);
+			DRM_DEBUG_KMS("[CRTC:%d] found active mode: ",
+				      crtc->base.base.id);
+			drm_mode_debug_printmodeline(&crtc->base.mode);
+		}
+	}
 
 	/* HW state is read out, now we need to sanitize this mess. */
 	for_each_intel_encoder(dev, encoder) {
 		intel_sanitize_encoder(encoder);
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-		/* prevent unnneeded restores with force_restore */
-		crtc_state->active_changed =
-		crtc_state->mode_changed =
-		crtc_state->planes_changed = false;
-
-		if (crtc->enabled) {
-			intel_mode_from_pipe_config(&crtc->state->mode,
-				to_intel_crtc_state(crtc->state));
-
-			drm_mode_copy(&crtc->mode, &crtc->state->mode);
-			drm_mode_copy(&crtc->hwmode,
-				      &crtc->state->adjusted_mode);
-		}
-
-		intel_sanitize_crtc(intel_crtc);
-
-		/*
-		 * sanitize_crtc may have forced an update of crtc->state,
-		 * so reload in intel_dump_pipe_config
-		 */
-		intel_dump_pipe_config(intel_crtc,
-				       to_intel_crtc_state(crtc->state),
+	for_each_pipe(dev_priv, pipe) {
+		crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+		intel_sanitize_crtc(crtc);
+		intel_dump_pipe_config(crtc, crtc->config,
 				       "[setup_hw_state]");
 	}
 
@@ -15558,17 +15454,20 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
 		ilk_wm_get_hw_state(dev);
 
 	if (force_restore) {
-		int ret;
-
 		i915_redisable_vga(dev);
 
-		ret = intel_set_mode(state);
-		if (ret) {
-			DRM_ERROR("Failed to restore previous mode\n");
-			drm_atomic_state_free(state);
+		/*
+		 * We need to use raw interfaces for restoring state to avoid
+		 * checking (bogus) intermediate states.
+		 */
+		for_each_pipe(dev_priv, pipe) {
+			struct drm_crtc *crtc =
+				dev_priv->pipe_to_crtc_mapping[pipe];
+
+			intel_crtc_restore_mode(crtc);
 		}
 	} else {
-		drm_atomic_state_free(state);
+		intel_modeset_update_staged_output_state(dev);
 	}
 
 	intel_modeset_check_state(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 58bd0edba2a4..d87b85f16127 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -130,6 +130,11 @@ struct intel_fbdev {
 
 struct intel_encoder {
 	struct drm_encoder base;
+	/*
+	 * The new crtc this encoder will be driven from. Only differs from
+	 * base->crtc while a modeset is in progress.
+	 */
+	struct intel_crtc *new_crtc;
 
 	enum intel_output_type type;
 	unsigned int cloneable;
@@ -190,6 +195,12 @@ struct intel_connector {
 	 */
 	struct intel_encoder *encoder;
 
+	/*
+	 * The new encoder this connector will be driven. Only differs from
+	 * encoder while a modeset is in progress.
+	 */
+	struct intel_encoder *new_encoder;
+
 	/* Reads out the current hw, returning true if the connector is enabled
 	 * and active (i.e. dpms ON state). */
 	bool (*get_hw_state)(struct intel_connector *);
@@ -527,6 +538,7 @@ struct intel_crtc {
 
 	struct intel_initial_plane_config plane_config;
 	struct intel_crtc_state *config;
+	bool new_enabled;
 
 	/* reset counter value when the last flip was submitted */
 	unsigned int reset_counter;
@@ -1408,8 +1420,6 @@ struct drm_atomic_state *intel_atomic_state_alloc(struct drm_device *dev);
 void intel_atomic_state_clear(struct drm_atomic_state *);
 struct intel_shared_dpll_config *
 intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s);
-void intel_atomic_duplicate_dpll_state(struct drm_i915_private *,
-				       struct intel_shared_dpll_config *);
 
 static inline struct intel_crtc_state *
 intel_atomic_get_crtc_state(struct drm_atomic_state *state,
-- 
2.1.0

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

Re: [PATCH 0/2] Revert atomic hw readout.

[Ander Conselvan De Oliveira]

For both patches,

Acked-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

On Wed, 2015-06-10 at 10:24 +0200, Maarten Lankhorst wrote:
> Seems this is causing too many issues, revert temporarily until the issues are fixed.
> 
> This might break some of the changes I made using atomic state:
>   Can someone test on haswell with multiple crtcs,
>   to see if the haswell plane workaround change still works?
>   "drm/i915: Calculate haswell plane workaround, v5."
> 
>   And i830 with DVO? For DVO_2X_MODE.
>   "drm/i915: Use atomic state for calculating DVO_2X_MODE on i830."
> 
> Maarten Lankhorst (2):
>   Revert "drm/i915: Make intel_display_suspend atomic, v2."
>   Revert "drm/i915: Read hw state into an atomic state struct, v2."
> 
>  drivers/gpu/drm/i915/i915_drv.c      |   3 -
>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
>  drivers/gpu/drm/i915/intel_display.c | 434 ++++++++++++-----------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  16 +-
>  4 files changed, 165 insertions(+), 290 deletions(-)
> 


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

Re: [PATCH 0/2] Revert atomic hw readout.

[Jani Nikula]

On Wed, 10 Jun 2015, Ander Conselvan De Oliveira <conselvan2@gmail.com> wrote:
> For both patches,
>
> Acked-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>

And both pushed to drm-intel-next-queued, thanks for the patches and
ack.

BR,
Jani.


>
> On Wed, 2015-06-10 at 10:24 +0200, Maarten Lankhorst wrote:
>> Seems this is causing too many issues, revert temporarily until the issues are fixed.
>> 
>> This might break some of the changes I made using atomic state:
>>   Can someone test on haswell with multiple crtcs,
>>   to see if the haswell plane workaround change still works?
>>   "drm/i915: Calculate haswell plane workaround, v5."
>> 
>>   And i830 with DVO? For DVO_2X_MODE.
>>   "drm/i915: Use atomic state for calculating DVO_2X_MODE on i830."
>> 
>> Maarten Lankhorst (2):
>>   Revert "drm/i915: Make intel_display_suspend atomic, v2."
>>   Revert "drm/i915: Read hw state into an atomic state struct, v2."
>> 
>>  drivers/gpu/drm/i915/i915_drv.c      |   3 -
>>  drivers/gpu/drm/i915/intel_atomic.c  |   2 +-
>>  drivers/gpu/drm/i915/intel_display.c | 434 ++++++++++++-----------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |  16 +-
>>  4 files changed, 165 insertions(+), 290 deletions(-)
>> 
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx