| Message ID | 1431354318-11995-34-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 11, 2015 at 04:25:09PM +0200, Maarten Lankhorst wrote: > 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. Imo this should be split in the remaing changes to get rid of ->active and then the actual removal. > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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)) { The super-correct fix would be to pass the old_state around and use those connectors. Probably not worth the trouble. > 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) { Misplace hunk? > /* 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; > - } > - } I don't think you can nuke this. Essentially we have two bits here: - is the pipe enabled (bit31 in pipeconf)? - is any encoder using this pipe and enabled? The later is computed/updated in intel_crtc_update_dpms. I'm not sure where exactly, but something in these pile of changes might have broken this. If not I think it should be extracted into a separate patch together with the other changes (to intel_crtc_control maybe?) to make it easier to follow why this code isn't needed any more. At least silently removing code with scary comments without explaining why it's dead now (and without moving the comments to the new places) isn't good. -Daniel > - > - 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 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, May 11, 2015 at 4:25 PM, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote: > @@ -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); Missed one: Why do you remove this? Imo that's a fairly crucial consistency check. -Daniel
Op 12-05-15 om 12:03 schreef Daniel Vetter: > On Mon, May 11, 2015 at 4:25 PM, Maarten Lankhorst > <maarten.lankhorst@linux.intel.com> wrote: >> @@ -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); > > Missed one: Why do you remove this? Imo that's a fairly crucial > consistency check. > -Daniel It's not removed, but crtc->active is the same as crtc->base.state->active now. The check still works as intended. :-)
On Tue, May 12, 2015 at 04:07:14PM +0200, Maarten Lankhorst wrote: > Op 12-05-15 om 12:03 schreef Daniel Vetter: > > On Mon, May 11, 2015 at 4:25 PM, Maarten Lankhorst > > <maarten.lankhorst@linux.intel.com> wrote: > >> @@ -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); > > > > Missed one: Why do you remove this? Imo that's a fairly crucial > > consistency check. > > -Daniel > It's not removed, but crtc->active is the same as crtc->base.state->active now. The check still works as intended. :-) Oh there's a confusion (maybe from ealier patches?). You derive enabled_crtcs from state->active, but it should be derived from state->enable. That's at least the case in current -nightly. And active_crtcs should be derived from state->active ofc (currently looking at intel_crtc->active). If I'm piecing the patches together the active_crtcs case gets removed and the enabled_crtcs is mixing things up after your series. We definitely need both of them still I think. -Daniel
Hi, On 12 May 2015 at 17:57, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, May 12, 2015 at 04:07:14PM +0200, Maarten Lankhorst wrote: >> Op 12-05-15 om 12:03 schreef Daniel Vetter: >> > On Mon, May 11, 2015 at 4:25 PM, Maarten Lankhorst >> > <maarten.lankhorst@linux.intel.com> wrote: >> >> @@ -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); >> > >> > Missed one: Why do you remove this? Imo that's a fairly crucial >> > consistency check. >> > -Daniel >> It's not removed, but crtc->active is the same as crtc->base.state->active now. The check still works as intended. :-) > > Oh there's a confusion (maybe from ealier patches?). You derive > enabled_crtcs from state->active, but it should be derived from > state->enable. That's at least the case in current -nightly. > > And active_crtcs should be derived from state->active ofc (currently > looking at intel_crtc->active). If I'm piecing the patches together the > active_crtcs case gets removed and the enabled_crtcs is mixing things up > after your series. We definitely need both of them still I think. I'll freely admit to getting lost in the maze, but is it that crtc->base.state->active / enabled_crtcs is testing the to-be-enabled set (pending state), and crtc->active / active_crtcs is testing the was-previously-enabled set (old state)? If so, these checks are indeed different, but it's kind of hard to tell. And we'd need to capture the entire previous state to pass in. Maybe those would be better as two separate checks; one before we swap the state, and one after? Cheers, Daniel
On Tue, May 12, 2015 at 06:01:40PM +0100, Daniel Stone wrote: > Hi, > > On 12 May 2015 at 17:57, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Tue, May 12, 2015 at 04:07:14PM +0200, Maarten Lankhorst wrote: > >> Op 12-05-15 om 12:03 schreef Daniel Vetter: > >> > On Mon, May 11, 2015 at 4:25 PM, Maarten Lankhorst > >> > <maarten.lankhorst@linux.intel.com> wrote: > >> >> @@ -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); > >> > > >> > Missed one: Why do you remove this? Imo that's a fairly crucial > >> > consistency check. > >> > -Daniel > >> It's not removed, but crtc->active is the same as crtc->base.state->active now. The check still works as intended. :-) > > > > Oh there's a confusion (maybe from ealier patches?). You derive > > enabled_crtcs from state->active, but it should be derived from > > state->enable. That's at least the case in current -nightly. > > > > And active_crtcs should be derived from state->active ofc (currently > > looking at intel_crtc->active). If I'm piecing the patches together the > > active_crtcs case gets removed and the enabled_crtcs is mixing things up > > after your series. We definitely need both of them still I think. > > I'll freely admit to getting lost in the maze, but is it that > crtc->base.state->active / enabled_crtcs is testing the to-be-enabled > set (pending state), and crtc->active / active_crtcs is testing the > was-previously-enabled set (old state)? If so, these checks are indeed > different, but it's kind of hard to tell. And we'd need to capture the > entire previous state to pass in. Maybe those would be better as two > separate checks; one before we swap the state, and one after? This is state config cross-checks done at the end, so never looks at transitions but only at snapshots. enabled = logically enabled, i.e. resources are reserved to make sure dpms on will succeed active = hw actually running And active always implies enabled. We have that distinction both on the crtc and by necessity also on the dplls used by them. But somehow that got a bit mixed up in Maarten's series here. Current -nightly still keeps them nicely separate. -Daniel
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;
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@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(-)