[Intel-gfx] [PATCH v4 19/27] drm/i915: Read hw state into an atomic state struct, v2.
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Mon Jun 1 03:50:03 PDT 2015
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
To make this work we load the new hardware state into the
atomic_state, then swap it with the sw state.
This lets us change the force restore path in setup_hw_state()
to use a single call to intel_mode_set() to restore all the
previous state.
As a nice bonus this kills off encoder->new_encoder,
connector->new_enabled and crtc->new_enabled. They were used only
to restore the state after a modeset.
Changes since v1:
- Make sure all possible planes are added with their crtc set,
so they will be turned off on first modeset.
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at 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, 243 insertions(+), 152 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 83078763ba14..17bf9e80c557 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;
}
-static void
+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 a134d83b15ca..816f509b577a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10060,7 +10060,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
retry:
ret = drm_modeset_lock(&config->connection_mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
/*
* Algorithm gets a little messy:
@@ -10078,10 +10078,10 @@ retry:
ret = drm_modeset_lock(&crtc->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
old->dpms_mode = connector->dpms;
old->load_detect_temp = false;
@@ -10100,9 +10100,6 @@ 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;
@@ -10113,20 +10110,17 @@ retry:
*/
if (!crtc) {
DRM_DEBUG_KMS("no pipe available for load-detect\n");
- goto fail_unlock;
+ goto fail;
}
ret = drm_modeset_lock(&crtc->mutex, ctx);
if (ret)
- goto fail_unlock;
+ goto fail;
ret = drm_modeset_lock(&crtc->primary->mutex, ctx);
if (ret)
- goto fail_unlock;
- intel_encoder->new_crtc = to_intel_crtc(crtc);
- to_intel_connector(connector)->new_encoder = intel_encoder;
+ goto fail;
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;
@@ -10194,9 +10188,7 @@ retry:
intel_wait_for_vblank(dev, intel_crtc->pipe);
return true;
- fail:
- intel_crtc->new_enabled = crtc->state->enable;
-fail_unlock:
+fail:
drm_atomic_state_free(state);
state = NULL;
@@ -10242,10 +10234,6 @@ 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;
@@ -11392,33 +11380,6 @@ 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.
@@ -11949,7 +11910,6 @@ 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) {
@@ -12253,11 +12213,14 @@ 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(&connector->new_encoder->base != connector->base.encoder,
+ I915_STATE_WARN(state->best_encoder != encoder,
"connector's staged encoder doesn't match current encoder\n");
}
}
@@ -12277,8 +12240,6 @@ 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");
@@ -12288,6 +12249,9 @@ 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
@@ -12713,11 +12677,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->new_crtc->base != crtc)
+ if (encoder->base.crtc != crtc)
continue;
for_each_intel_connector(dev, connector) {
- if (connector->new_encoder != encoder)
+ if (connector->base.state->best_encoder != &encoder->base)
continue;
connector_state = drm_atomic_get_connector_state(state, &connector->base);
@@ -12730,14 +12694,10 @@ 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",
@@ -12746,9 +12706,6 @@ 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);
}
@@ -14823,6 +14780,7 @@ 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;
@@ -14986,99 +14944,225 @@ 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.enabled)
return false;
return I915_READ(DSPCNTR(crtc->plane)) & DISPLAY_PLANE_ENABLE;
}
-static void intel_modeset_readout_hw_state(struct drm_device *dev)
+static int readout_hw_crtc_state(struct drm_atomic_state *state,
+ struct intel_crtc *crtc)
{
- 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;
+ 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;
- for_each_intel_crtc(dev, crtc) {
- struct drm_plane *primary = crtc->base.primary;
- struct intel_plane_state *plane_state;
+ crtc_state = intel_atomic_get_crtc_state(state, crtc);
+ if (IS_ERR(crtc_state))
+ return PTR_ERR(crtc_state);
- memset(crtc->config, 0, sizeof(*crtc->config));
- crtc->config->base.crtc = &crtc->base;
+ ret = drm_atomic_add_affected_planes(state, &crtc->base);
+ if (ret)
+ return ret;
- crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
+ memset(crtc_state, 0, sizeof(*crtc_state));
+ crtc_state->base.crtc = &crtc->base;
+ crtc_state->base.state = state;
- crtc->active = dev_priv->display.get_pipe_config(crtc,
- crtc->config);
+ crtc_state->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
- crtc->base.state->enable = crtc->active;
- crtc->base.state->active = crtc->active;
- crtc->base.enabled = crtc->active;
+ crtc_state->base.enable = crtc_state->base.active =
+ crtc->base.enabled = dev_priv->display.get_pipe_config(crtc, crtc_state);
- plane_state = to_intel_plane_state(primary->state);
- plane_state->visible = primary_get_hw_state(crtc);
+ /* update transitional state */
+ crtc->active = crtc_state->base.active;
+ crtc->config = crtc_state;
- DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
- crtc->base.base.id,
- crtc->active ? "enabled" : "disabled");
- }
+ drm_plane_state = drm_atomic_get_plane_state(state, primary);
+ if (IS_ERR(drm_plane_state))
+ return PTR_ERR(drm_plane_state);
+
+ plane_state = to_intel_plane_state(drm_plane_state);
+ plane_state->visible = primary_get_hw_state(crtc);
+ 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));
+
+ DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
+ crtc->base.base.id,
+ crtc_state->base.active ? "enabled" : "disabled");
+
+ return 0;
+}
+
+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;
+
+ 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,
- &pll->config.hw_state);
+ &shared_dpll[i].hw_state);
+
pll->active = 0;
- pll->config.crtc_mask = 0;
- for_each_intel_crtc(dev, crtc) {
- if (crtc->active && intel_crtc_to_shared_dpll(crtc) == pll) {
+ 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->active++;
- pll->config.crtc_mask |= 1 << crtc->pipe;
+ shared_dpll[i].crtc_mask |=
+ 1 << crtc->pipe;
}
}
DRM_DEBUG_KMS("%s hw state readout: crtc_mask 0x%08x, on %i\n",
- pll->name, pll->config.crtc_mask, pll->on);
+ pll->name, shared_dpll[i].crtc_mask,
+ pll->on);
- if (pll->config.crtc_mask)
+ if (shared_dpll[i].crtc_mask)
intel_display_power_get(dev_priv, POWER_DOMAIN_PLLS);
}
- for_each_intel_encoder(dev, encoder) {
- pipe = 0;
+ return 0;
+}
- if (encoder->get_hw_state(encoder, &pipe)) {
- 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;
- }
+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;
- encoder->connectors_active = false;
- DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
- encoder->base.base.id,
- encoder->base.name,
- encoder->base.crtc ? "enabled" : "disabled",
- pipe_name(pipe));
- }
+ 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->encoder->connectors_active = true;
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;
+ } else {
+ encoder->base.crtc = NULL;
+ }
+
+ DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n",
+ encoder->base.base.id,
+ encoder->base.name,
+ encoder->base.crtc ? "enabled" : "disabled",
+ 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;
+ }
+
+ 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
@@ -15087,37 +15171,57 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
bool force_restore)
{
struct drm_i915_private *dev_priv = dev->dev_private;
- enum pipe pipe;
- struct intel_crtc *crtc;
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
struct intel_encoder *encoder;
+ struct drm_atomic_state *state;
+ struct intel_shared_dpll_config shared_dplls[I915_NUM_PLLS];
int i;
- intel_modeset_readout_hw_state(dev);
-
- /*
- * 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);
- }
+ 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);
+
+ /* 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);
+
/* HW state is read out, now we need to sanitize this mess. */
for_each_intel_encoder(dev, encoder) {
intel_sanitize_encoder(encoder);
}
- 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,
+ 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),
"[setup_hw_state]");
}
@@ -15141,20 +15245,17 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
ilk_wm_get_hw_state(dev);
if (force_restore) {
- i915_redisable_vga(dev);
+ int ret;
- /*
- * 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];
+ i915_redisable_vga(dev);
- intel_crtc_restore_mode(crtc);
+ ret = intel_set_mode(state);
+ if (ret) {
+ DRM_ERROR("Failed to restore previous mode\n");
+ drm_atomic_state_free(state);
}
} else {
- intel_modeset_update_staged_output_state(dev);
+ drm_atomic_state_free(state);
}
intel_modeset_check_state(dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ddae28ac4e6f..01830532b33e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -130,11 +130,6 @@ 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;
@@ -195,12 +190,6 @@ 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 *);
@@ -535,7 +524,6 @@ 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;
@@ -1417,6 +1405,8 @@ 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
More information about the Intel-gfx
mailing list