[PATCH 69/81] drm/i915: push commit_output_state past the crtc/encoder preparing

Daniel Vetter
2012-07-11T15:36:44Z

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.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_display.c | 88 +++++++++++++++++++---------------
1 files changed, 50 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2815dc9..45db3e7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3190,10 +3190,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);
@@ -3241,7 +3239,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);

@@ -3259,14 +3256,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);
@@ -3387,10 +3383,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);
@@ -3406,7 +3400,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);
}
@@ -3420,14 +3413,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
*
@@ -6815,6 +6795,18 @@ 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;
+}
+
#define for_each_intel_crtc_masked(dev, mask, intel_crtc) \
list_for_each_entry((intel_crtc), \
&(dev)->mode_config.crtc_list, \
@@ -6832,6 +6824,7 @@ bool intel_crtc_set_mode(struct drm_crtc *crtc,
struct drm_encoder_helper_funcs *encoder_funcs;
struct drm_encoder *encoder;
struct intel_crtc *intel_crtc;
+ struct drm_connector *connector;
unsigned disable_pipes, prepare_pipes, modeset_pipes;
bool ret = true;

@@ -6844,12 +6837,6 @@ bool intel_crtc_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;

@@ -6864,12 +6851,12 @@ bool intel_crtc_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;
@@ -6877,6 +6864,31 @@ bool intel_crtc_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_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;
+
+ to_intel_encoder(connector->encoder)->connectors_active = true;
+
+ printk("adjusting dpms for connector %i to %i\n", connector->base.id,
+ connector->dpms);
+ }
+
/* Set up the DPLL and any encoders state that needs to adjust or depend
* on the DPLL.
*/
--
1.7.7.6