[PATCH v2 10/20] drm/i915: Convert suspend/resume to atomic.

Maarten Lankhorst
2015-07-07T07:08:50Z

Instead of all the ad-hoc updating, duplicate the old state first before
reading out the hw state, then restore it.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e44dc0d6656f..db48aee7f140 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -741,7 +741,7 @@ static int i915_drm_resume(struct drm_device *dev)
spin_unlock_irq(&dev_priv->irq_lock);

drm_modeset_lock_all(dev);
- intel_modeset_setup_hw_state(dev, true);
+ intel_display_resume(dev);
drm_modeset_unlock_all(dev);

intel_dp_mst_resume(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 093d6421dddf..2a78a0ee0f97 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3278,8 +3278,7 @@ extern void intel_modeset_gem_init(struct drm_device *dev);
extern void intel_modeset_cleanup(struct drm_device *dev);
extern void intel_connector_unregister(struct intel_connector *);
extern int intel_modeset_vga_set_state(struct drm_device *dev, bool state);
-extern void intel_modeset_setup_hw_state(struct drm_device *dev,
- bool force_restore);
+extern void intel_display_resume(struct drm_device *dev);
extern void i915_redisable_vga(struct drm_device *dev);
extern void i915_redisable_vga_power_on(struct drm_device *dev);
extern bool ironlake_set_drps(struct drm_device *dev, u8 val);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc4bdb91ad4d..222d587ed4ea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -110,6 +110,7 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
int num_connectors);
static void intel_pre_disable_primary(struct drm_crtc *crtc);
+static void intel_modeset_setup_hw_state(struct drm_device *dev);

static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
{
@@ -3247,7 +3248,7 @@ void intel_finish_reset(struct drm_device *dev)
dev_priv->display.hpd_irq_setup(dev);
spin_unlock_irq(&dev_priv->irq_lock);

- intel_modeset_setup_hw_state(dev, true);
+ intel_display_resume(dev);

intel_hpd_init(dev_priv);

@@ -10239,7 +10240,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:
@@ -10257,10 +10258,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;
@@ -10279,9 +10280,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;
@@ -10292,20 +10290,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;
@@ -10373,9 +10368,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;

@@ -10421,10 +10414,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;

@@ -11827,37 +11816,6 @@ static const struct drm_crtc_helper_funcs intel_helper_funcs = {
.atomic_check = intel_crtc_atomic_check,
};

-/**
- * 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.
- */
static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
{
struct intel_connector *connector;
@@ -12297,7 +12255,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) {
@@ -12688,11 +12645,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");
}
}
@@ -12712,8 +12672,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");

@@ -12723,6 +12681,10 @@ 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
@@ -13282,11 +13244,12 @@ 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);
@@ -13299,7 +13262,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
}

connector_state->crtc = crtc;
- connector_state->best_encoder = &encoder->base;
}
}

@@ -13311,9 +13273,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
return;
}

- crtc_state->base.active = crtc_state->base.enable =
- to_intel_crtc(crtc)->new_enabled;
-
drm_mode_copy(&crtc_state->base.mode, &crtc->mode);

intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
@@ -15112,7 +15071,7 @@ void intel_modeset_init(struct drm_device *dev)
intel_fbc_disable(dev);

drm_modeset_lock_all(dev);
- intel_modeset_setup_hw_state(dev, false);
+ intel_modeset_setup_hw_state(dev);
drm_modeset_unlock_all(dev);

for_each_intel_crtc(dev, crtc) {
@@ -15500,10 +15459,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
}
}

-/* Scan out the current hw modeset state, sanitizes it and maps it into the drm
- * and i915 state tracking structures. */
-void intel_modeset_setup_hw_state(struct drm_device *dev,
- bool force_restore)
+/* Scan out the current hw modeset state,
+ * and sanitizes it to the current state
+ */
+static void
+intel_modeset_setup_hw_state(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
enum pipe pipe;
@@ -15545,25 +15505,58 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
skl_wm_get_hw_state(dev);
else if (HAS_PCH_SPLIT(dev))
ilk_wm_get_hw_state(dev);
+}

- if (force_restore) {
- i915_redisable_vga(dev);
+void intel_display_resume(struct drm_device *dev)
+{
+ struct drm_atomic_state *state = drm_atomic_state_alloc(dev);
+ struct intel_connector *conn;
+ struct intel_plane *plane;
+ struct drm_crtc *crtc;
+ 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];
+ if (!state)
+ return;

- intel_crtc_restore_mode(crtc);
- }
- } else {
- intel_modeset_update_staged_output_state(dev);
+ state->acquire_ctx = dev->mode_config.acquire_ctx;
+
+ /* preserve complete old state, including dpll */
+ intel_atomic_get_shared_dpll_state(state);
+
+ 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 err;
+
+ /* force a restore */
+ crtc_state->mode_changed = true;
+ }
+
+ for_each_intel_plane(dev, plane) {
+ ret = PTR_ERR_OR_ZERO(drm_atomic_get_plane_state(state, &plane->base));
+ if (ret)
+ goto err;
+ }
+
+ for_each_intel_connector(dev, conn) {
+ ret = PTR_ERR_OR_ZERO(drm_atomic_get_connector_state(state, &conn->base));
+ if (ret)
+ goto err;
}

- intel_modeset_check_state(dev);
+ intel_modeset_setup_hw_state(dev);
+
+ i915_redisable_vga(dev);
+ ret = intel_set_mode(state);
+ if (!ret)
+ return;
+
+err:
+ DRM_ERROR("Restoring old state failed with %i\n", ret);
+ drm_atomic_state_free(state);
}

void intel_modeset_gem_init(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c3cea178c809..97b65749f472 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 *);
@@ -550,7 +539,6 @@ struct intel_crtc {
uint32_t cursor_base;

struct intel_crtc_state *config;
- bool new_enabled;

/* reset counter value when the last flip was submitted */
unsigned int reset_counter;
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index ea85547611a5..a63d18680623 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -452,7 +452,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
*/
if (!HAS_PCH_SPLIT(dev)) {
drm_modeset_lock_all(dev);
- intel_modeset_setup_hw_state(dev, true);
+ intel_display_resume(dev);
drm_modeset_unlock_all(dev);
}

--
2.1.0