From 41da1f5d492b4bfef04419feb12ee9b0ea0517cb Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:45 +0200 Subject: [PATCH 01/33] drm/i915: get rid of put_shared_dpll Now that the pll updates are staged the put_shared_dpll function consists only of checks that are done in check_shared_dpll_state after a modeset too. The changes to pll->config are overwritten by intel_shared_dpll_commit, so this entire function is a noop. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 34 +++------------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 3 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 58846b58b676..9d271172dddd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4199,27 +4199,6 @@ static void lpt_pch_enable(struct drm_crtc *crtc) lpt_enable_pch_transcoder(dev_priv, cpu_transcoder); } -void intel_put_shared_dpll(struct intel_crtc *crtc) -{ - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); - - if (pll == NULL) - return; - - if (!(pll->config.crtc_mask & (1 << crtc->pipe))) { - WARN(1, "bad %s crtc mask\n", pll->name); - return; - } - - pll->config.crtc_mask &= ~(1 << crtc->pipe); - if (pll->config.crtc_mask == 0) { - WARN_ON(pll->on); - WARN_ON(pll->active); - } - - crtc->config->shared_dpll = DPLL_ID_PRIVATE; -} - struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state) { @@ -5206,13 +5185,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) intel_disable_shared_dpll(intel_crtc); } -static void ironlake_crtc_off(struct drm_crtc *crtc) -{ - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - intel_put_shared_dpll(intel_crtc); -} - - static void i9xx_pfit_enable(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; @@ -14770,7 +14742,7 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; + dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = skylake_update_primary_plane; } else if (HAS_DDI(dev)) { @@ -14781,7 +14753,7 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; + dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (HAS_PCH_SPLIT(dev)) { @@ -14792,7 +14764,7 @@ static void intel_init_display(struct drm_device *dev) ironlake_crtc_compute_clock; dev_priv->display.crtc_enable = ironlake_crtc_enable; dev_priv->display.crtc_disable = ironlake_crtc_disable; - dev_priv->display.off = ironlake_crtc_off; + dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (IS_VALLEYVIEW(dev)) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 5c9d6217f371..4635c83e6dc2 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1083,7 +1083,6 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv, #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false) struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, struct intel_crtc_state *state); -void intel_put_shared_dpll(struct intel_crtc *crtc); void vlv_force_pll_on(struct drm_device *dev, enum pipe pipe, const struct dpll *dpll); From 69024de8ba9ee28bbb2c0ba2f813d37a7d0be80a Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:46 +0200 Subject: [PATCH 02/33] drm/i915: get rid of intel_crtc_disable and related code, v3 Now that the dpll updates are (mostly) atomic, the .off() code is a noop, and intel_crtc_disable does mostly the same as intel_modeset_update_state. Move all logic for connectors_active and setting dpms to that function. Changes since v1: - Move drm_atomic_helper_swap_state up. Changes since v2: - Split out intel_put_shared_dpll removal. Changes since v3: - Rebase on top of latest drm-intel. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_display.c | 93 +++++++--------------------- 2 files changed, 23 insertions(+), 71 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 46722f85ef76..03ae50ee4737 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -631,7 +631,6 @@ struct drm_i915_display_funcs { struct intel_crtc_state *crtc_state); void (*crtc_enable)(struct drm_crtc *crtc); void (*crtc_disable)(struct drm_crtc *crtc); - void (*off)(struct drm_crtc *crtc); void (*audio_codec_enable)(struct drm_connector *connector, struct intel_encoder *encoder, struct drm_display_mode *mode); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9d271172dddd..490581081df7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6278,10 +6278,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) mutex_unlock(&dev->struct_mutex); } -static void i9xx_crtc_off(struct drm_crtc *crtc) -{ -} - /* Master function to enable/disable CRTC and corresponding power wells */ void intel_crtc_control(struct drm_crtc *crtc, bool enable) { @@ -6331,34 +6327,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) crtc->state->active = enable; } -static void intel_crtc_disable(struct drm_crtc *crtc) -{ - struct drm_device *dev = crtc->dev; - struct drm_connector *connector; - struct drm_i915_private *dev_priv = dev->dev_private; - - /* crtc should still be enabled when we disable it. */ - WARN_ON(!crtc->state->enable); - - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); - dev_priv->display.off(crtc); - - drm_plane_helper_disable(crtc->primary); - - /* Update computed state. */ - list_for_each_entry(connector, &dev->mode_config.connector_list, head) { - if (!connector->encoder || !connector->encoder->crtc) - continue; - - if (connector->encoder->crtc != crtc) - continue; - - connector->dpms = DRM_MODE_DPMS_OFF; - to_intel_encoder(connector->encoder)->connectors_active = false; - } -} - void intel_encoder_destroy(struct drm_encoder *encoder) { struct intel_encoder *intel_encoder = to_intel_encoder(encoder); @@ -12272,26 +12240,22 @@ intel_modeset_update_state(struct drm_atomic_state *state) struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; struct drm_connector *connector; - int i; intel_shared_dpll_commit(dev_priv); + drm_atomic_helper_swap_state(state->dev, state); for_each_intel_encoder(dev, intel_encoder) { if (!intel_encoder->base.crtc) continue; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc != intel_encoder->base.crtc) - continue; + crtc = intel_encoder->base.crtc; + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + if (!crtc_state || !needs_modeset(crtc->state)) + continue; - if (crtc_state->enable && needs_modeset(crtc_state)) - intel_encoder->connectors_active = false; - - break; - } + intel_encoder->connectors_active = false; } - drm_atomic_helper_swap_state(state->dev, state); intel_modeset_fixup_state(state); /* Double check state. */ @@ -12303,27 +12267,23 @@ intel_modeset_update_state(struct drm_atomic_state *state) if (!connector->encoder || !connector->encoder->crtc) continue; - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (crtc != connector->encoder->crtc) - continue; + crtc = connector->encoder->crtc; + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + if (!crtc_state || !needs_modeset(crtc->state)) + continue; - if (crtc->state->enable && needs_modeset(crtc->state)) { - struct drm_property *dpms_property = - dev->mode_config.dpms_property; + if (crtc->state->enable) { + struct drm_property *dpms_property = + dev->mode_config.dpms_property; - connector->dpms = DRM_MODE_DPMS_ON; - drm_object_property_set_value(&connector->base, - dpms_property, - DRM_MODE_DPMS_ON); + connector->dpms = DRM_MODE_DPMS_ON; + drm_object_property_set_value(&connector->base, dpms_property, DRM_MODE_DPMS_ON); - intel_encoder = to_intel_encoder(connector->encoder); - intel_encoder->connectors_active = true; - } - - break; - } + intel_encoder = to_intel_encoder(connector->encoder); + intel_encoder->connectors_active = true; + } else + connector->dpms = DRM_MODE_DPMS_OFF; } - } static bool intel_fuzzy_clock_check(int clock1, int clock2) @@ -13001,12 +12961,10 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, if (!needs_modeset(crtc_state)) continue; - if (!crtc_state->enable) { - intel_crtc_disable(crtc); - } else if (crtc->state->enable) { - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); - } + intel_crtc_disable_planes(crtc); + dev_priv->display.crtc_disable(crtc); + if (!crtc_state->enable) + drm_plane_helper_disable(crtc->primary); } /* crtc->mode is already used by the ->mode_set callbacks, hence we need @@ -14742,7 +14700,6 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = skylake_update_primary_plane; } else if (HAS_DDI(dev)) { @@ -14753,7 +14710,6 @@ static void intel_init_display(struct drm_device *dev) haswell_crtc_compute_clock; dev_priv->display.crtc_enable = haswell_crtc_enable; dev_priv->display.crtc_disable = haswell_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (HAS_PCH_SPLIT(dev)) { @@ -14764,7 +14720,6 @@ static void intel_init_display(struct drm_device *dev) ironlake_crtc_compute_clock; dev_priv->display.crtc_enable = ironlake_crtc_enable; dev_priv->display.crtc_disable = ironlake_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (IS_VALLEYVIEW(dev)) { @@ -14774,7 +14729,6 @@ static void intel_init_display(struct drm_device *dev) dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock; dev_priv->display.crtc_enable = valleyview_crtc_enable; dev_priv->display.crtc_disable = i9xx_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = i9xx_update_primary_plane; } else { @@ -14784,7 +14738,6 @@ static void intel_init_display(struct drm_device *dev) dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock; dev_priv->display.crtc_enable = i9xx_crtc_enable; dev_priv->display.crtc_disable = i9xx_crtc_disable; - dev_priv->display.off = i9xx_crtc_off; dev_priv->display.update_primary_plane = i9xx_update_primary_plane; } From 6b72d486245265676df9866734bca1b39252e480 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:47 +0200 Subject: [PATCH 03/33] drm/i915: add intel_display_suspend, v2 This is a function used to disable all crtc's. This makes it clearer to distinguish between when mode needs to be preserved and when it can be trashed. Changes since v1: - Copy power changes from intel_crtc_control. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 4 +-- drivers/gpu/drm/i915/intel_display.c | 38 ++++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a051a0241883..78ef0bb53c36 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -601,7 +601,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv); static int i915_drm_suspend(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_crtc *crtc; pci_power_t opregion_target_state; int error; @@ -632,8 +631,7 @@ static int i915_drm_suspend(struct drm_device *dev) * for _thaw. Also, power gate the CRTC power wells. */ drm_modeset_lock_all(dev); - for_each_crtc(dev, crtc) - intel_crtc_control(crtc, false); + intel_display_suspend(dev); drm_modeset_unlock_all(dev); intel_dp_mst_suspend(dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 490581081df7..da7c886026da 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3190,9 +3190,6 @@ void intel_crtc_reset(struct intel_crtc *crtc) void intel_prepare_reset(struct drm_device *dev) { - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *crtc; - /* no reset support for gen2 */ if (IS_GEN2(dev)) return; @@ -3207,13 +3204,7 @@ void intel_prepare_reset(struct drm_device *dev) * Disabling the crtcs gracefully seems nicer. Also the * g33 docs say we should at least disable all the planes. */ - for_each_intel_crtc(dev, crtc) { - if (!crtc->active) - continue; - - intel_crtc_disable_planes(&crtc->base); - dev_priv->display.crtc_disable(&crtc->base); - } + intel_display_suspend(dev); } void intel_finish_reset(struct drm_device *dev) @@ -6278,6 +6269,33 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) mutex_unlock(&dev->struct_mutex); } +/* + * turn all crtc's off, but do not adjust state + * This has to be paired with a call to intel_modeset_setup_hw_state. + */ +void intel_display_suspend(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_crtc *crtc; + + for_each_crtc(dev, crtc) { + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum intel_display_power_domain domain; + unsigned long domains; + + if (!intel_crtc->active) + continue; + + intel_crtc_disable_planes(crtc); + dev_priv->display.crtc_disable(crtc); + + 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; + } +} + /* Master function to enable/disable CRTC and corresponding power wells */ void intel_crtc_control(struct drm_crtc *crtc, bool enable) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4635c83e6dc2..31afeb05ea90 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -993,6 +993,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); +void intel_display_suspend(struct drm_device *dev); void intel_crtc_control(struct drm_crtc *crtc, bool enable); void intel_crtc_reset(struct intel_crtc *crtc); void intel_crtc_update_dpms(struct drm_crtc *crtc); From 1b5092592824d1c91d6e48d820b6047f6ba323ce Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:48 +0200 Subject: [PATCH 04/33] drm/i915: use intel_crtc_control everywhere, v3. Having a single path for everything makes it a lot easier to keep crtc_state->active in sync with intel_crtc->active. A crtc cannot be changed to active when not enabled, because it means no mode is set and no connectors are connected. This should also make intel_crtc->active match crtc_state->active. Changes since v1: - Reworded commit message, there's no intel_crtc_toggle. Changes since v2: - Change some callers of intel_crtc_control to intel_display_suspend. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_debugfs.c | 18 ++++++++-- drivers/gpu/drm/i915/intel_display.c | 54 +++++++++++----------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 47d9854434c5..aac252ca0bda 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3630,12 +3630,18 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev) */ if (crtc->config->cpu_transcoder == TRANSCODER_EDP && !crtc->config->pch_pfit.enabled) { + bool active = crtc->active; + + if (active) + intel_crtc_control(&crtc->base, false); + crtc->config->pch_pfit.force_thru = true; intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); - intel_crtc_reset(crtc); + if (active) + intel_crtc_control(&crtc->base, true); } drm_modeset_unlock_all(dev); } @@ -3654,12 +3660,18 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev) * routing. */ if (crtc->config->pch_pfit.force_thru) { - crtc->config->pch_pfit.force_thru = false; + bool active = crtc->active; - intel_crtc_reset(crtc); + if (active) + intel_crtc_control(&crtc->base, false); + + crtc->config->pch_pfit.force_thru = false; intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); + + if (active) + intel_crtc_control(&crtc->base, true); } drm_modeset_unlock_all(dev); } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index da7c886026da..aab75d3b7d20 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3175,19 +3175,6 @@ static void intel_update_primary_planes(struct drm_device *dev) } } -void intel_crtc_reset(struct intel_crtc *crtc) -{ - struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); - - if (!crtc->active) - return; - - intel_crtc_disable_planes(&crtc->base); - dev_priv->display.crtc_disable(&crtc->base); - dev_priv->display.crtc_enable(&crtc->base); - intel_crtc_enable_planes(&crtc->base); -} - void intel_prepare_reset(struct drm_device *dev) { /* no reset support for gen2 */ @@ -3199,7 +3186,6 @@ void intel_prepare_reset(struct drm_device *dev) return; drm_modeset_lock_all(dev); - /* * Disabling the crtcs gracefully seems nicer. Also the * g33 docs say we should at least disable all the planes. @@ -6305,26 +6291,29 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable) enum intel_display_power_domain domain; unsigned long domains; + if (enable == intel_crtc->active) + return; + + if (enable && !crtc->state->enable) + return; + + crtc->state->active = enable; if (enable) { - if (!intel_crtc->active) { - domains = get_crtc_power_domains(crtc); - for_each_power_domain(domain, domains) - intel_display_power_get(dev_priv, domain); - intel_crtc->enabled_power_domains = domains; + domains = get_crtc_power_domains(crtc); + for_each_power_domain(domain, domains) + intel_display_power_get(dev_priv, domain); + intel_crtc->enabled_power_domains = domains; - dev_priv->display.crtc_enable(crtc); - intel_crtc_enable_planes(crtc); - } + dev_priv->display.crtc_enable(crtc); + intel_crtc_enable_planes(crtc); } else { - if (intel_crtc->active) { - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); + intel_crtc_disable_planes(crtc); + dev_priv->display.crtc_disable(crtc); - 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; - } + 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; } } @@ -6341,8 +6330,6 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) enable |= intel_encoder->connectors_active; intel_crtc_control(crtc, enable); - - crtc->state->active = enable; } void intel_encoder_destroy(struct drm_encoder *encoder) @@ -15240,8 +15227,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) plane = crtc->plane; to_intel_plane_state(crtc->base.primary->state)->visible = true; crtc->plane = !plane; - intel_crtc_disable_planes(&crtc->base); - dev_priv->display.crtc_disable(&crtc->base); + intel_crtc_control(&crtc->base, false); crtc->plane = plane; /* ... and break all links. */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 31afeb05ea90..ea1351f87f0a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -995,7 +995,6 @@ void intel_mark_idle(struct drm_device *dev); void intel_crtc_restore_mode(struct drm_crtc *crtc); void intel_display_suspend(struct drm_device *dev); void intel_crtc_control(struct drm_crtc *crtc, bool enable); -void intel_crtc_reset(struct intel_crtc *crtc); void intel_crtc_update_dpms(struct drm_crtc *crtc); void intel_encoder_destroy(struct drm_encoder *encoder); int intel_connector_init(struct intel_connector *); From 3cb480bcb3397a1cfdc04115adcdb33393fde4f9 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:49 +0200 Subject: [PATCH 05/33] drm/i915: Use drm_atomic_helper_update_legacy_modeset_state, v2. Now that the helper is exported there's no need to duplicate this code any more. Changes since v1: - move intel_modeset_update_staged_output_state call to the right place. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 61 ++-------------------------- 1 file changed, 4 insertions(+), 57 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aab75d3b7d20..c1042ea50136 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11737,43 +11737,6 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev) } } -/* Fixup legacy state after an atomic state swap. - */ -static void intel_modeset_fixup_state(struct drm_atomic_state *state) -{ - struct intel_crtc *crtc; - struct intel_encoder *encoder; - struct intel_connector *connector; - - for_each_intel_connector(state->dev, connector) { - connector->base.encoder = connector->base.state->best_encoder; - if (connector->base.encoder) - connector->base.encoder->crtc = - connector->base.state->crtc; - } - - /* Update crtc of disabled encoders */ - for_each_intel_encoder(state->dev, encoder) { - int num_connectors = 0; - - for_each_intel_connector(state->dev, connector) - if (connector->base.encoder == &encoder->base) - num_connectors++; - - if (num_connectors == 0) - encoder->base.crtc = NULL; - } - - for_each_intel_crtc(state->dev, crtc) { - crtc->base.enabled = crtc->base.state->enable; - crtc->config = to_intel_crtc_state(crtc->base.state); - } - - /* Copy the new configuration to the staged state, to keep the few - * pieces of code that haven't been converted yet happy */ - intel_modeset_update_staged_output_state(state->dev); -} - static void connected_sink_compute_bpp(struct intel_connector *connector, struct intel_crtc_state *pipe_config) @@ -12261,11 +12224,14 @@ intel_modeset_update_state(struct drm_atomic_state *state) intel_encoder->connectors_active = false; } - intel_modeset_fixup_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) { WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc)); + + to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); } list_for_each_entry(connector, &dev->mode_config.connector_list, head) { @@ -12972,25 +12938,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc, drm_plane_helper_disable(crtc->primary); } - /* crtc->mode is already used by the ->mode_set callbacks, hence we need - * to set it here already despite that we pass it down the callchain. - * - * Note we'll need to fix this up when we start tracking multiple - * pipes; here we assume a single modeset_pipe and only track the - * single crtc and mode. - */ - if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) { - modeset_crtc->mode = pipe_config->base.mode; - - /* - * Calculate and store various constants which - * are later needed by vblank and swap-completion - * timestamping. They are derived from true hwmode. - */ - drm_calc_timestamping_constants(modeset_crtc, - &pipe_config->base.adjusted_mode); - } - /* Only after disabling all output pipelines that will be changed can we * update the the output configuration. */ intel_modeset_update_state(state); From c72d969b23253ee8196e4190ec2c15e7cf607372 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 1 Jun 2015 12:49:50 +0200 Subject: [PATCH 06/33] drm/i915: Make __intel_set_mode() take only atomic state as argument With the use of drm_atomic_helper_update_legacy_modeset_state the last user of modeset_crtc is removed from this function. Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c1042ea50136..7116217ca25b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12909,12 +12909,10 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) return 0; } -static int __intel_set_mode(struct drm_crtc *modeset_crtc, - struct intel_crtc_state *pipe_config) +static int __intel_set_mode(struct drm_atomic_state *state) { - struct drm_device *dev = modeset_crtc->dev; + struct drm_device *dev = state->dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_atomic_state *state = pipe_config->base.state; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; int ret = 0; @@ -12974,7 +12972,7 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc, { int ret; - ret = __intel_set_mode(crtc, pipe_config); + ret = __intel_set_mode(pipe_config->base.state); if (ret == 0) intel_modeset_check_state(crtc->dev); From cdba954e426761cdfe08ce4c9909cd97ce254b9c Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 1 Jun 2015 12:49:51 +0200 Subject: [PATCH 07/33] drm/i915: Set mode_changed for audio in intel_modeset_pipe_config() A follow up patch will make intel_modeset_compute_config() deal with multiple crtcs, so move crtc specific stuff into the lower level crtc specific function. Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 51 +++++++++++++++------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7116217ca25b..eb40f9fc9b44 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -431,6 +431,12 @@ static void vlv_clock(int refclk, intel_clock_t *clock) clock->dot = DIV_ROUND_CLOSEST(clock->vco, clock->p); } +static bool +needs_modeset(struct drm_crtc_state *state) +{ + return state->mode_changed || state->active_changed; +} + /** * Returns whether any output on the specified pipe is of the specified type */ @@ -12085,6 +12091,15 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, return -EINVAL; } + /* + * XXX: Add all connectors to make the crtc state match the encoders. + */ + if (!needs_modeset(&pipe_config->base)) { + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + return ret; + } + clear_intel_crtc_state(pipe_config); pipe_config->cpu_transcoder = @@ -12176,6 +12191,18 @@ encoder_retry: DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n", base_bpp, pipe_config->pipe_bpp, pipe_config->dither); + /* Check if we need to force a modeset */ + if (pipe_config->has_audio != + to_intel_crtc_state(crtc->state)->has_audio) + pipe_config->base.mode_changed = true; + + /* + * Note we have an issue here with infoframes: current code + * only updates them on the full mode set path per hw + * requirements. So here we should be checking for any + * required changes and forcing a mode set. + */ + return 0; fail: return ret; @@ -12193,12 +12220,6 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc) return false; } -static bool -needs_modeset(struct drm_crtc_state *state) -{ - return state->mode_changed || state->active_changed; -} - static void intel_modeset_update_state(struct drm_atomic_state *state) { @@ -12785,10 +12806,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config; int ret = 0; - ret = drm_atomic_add_affected_connectors(state, crtc); - if (ret) - return ERR_PTR(ret); - ret = drm_atomic_helper_check_modeset(state->dev, state); if (ret) return ERR_PTR(ret); @@ -12810,19 +12827,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc, if (ret) return ERR_PTR(ret); - /* Check things that can only be changed through modeset */ - if (pipe_config->has_audio != - to_intel_crtc(crtc)->config->has_audio) - pipe_config->base.mode_changed = true; - - /* - * Note we have an issue here with infoframes: current code - * only updates them on the full mode set path per hw - * requirements. So here we should be checking for any - * required changes and forcing a mode set. - */ - - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); + intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]"); ret = drm_atomic_helper_check_planes(state->dev, state); if (ret) From 53d9f4e99de001374eb06195609cc0451f31a318 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:52 +0200 Subject: [PATCH 08/33] drm/i915: Use crtc_state->active instead of crtc_state->enable crtc_state->enable means a crtc is configured, but it may be turned off for dpms. Until the commit "use intel_crtc_control everywhere" crtc_state->active was not updated on crtc off, but now crtc_state->active should be used for tracking whether a crtc is scanning out or not. A few commits from now dpms will be handled by calling intel_set_mode with a different value for crtc_state->active, which causes a crtc to turn on or off. At this point crtc->active should mirror crtc_state->active, so some paranoia from the crtc_disable functions can be removed. intel_set_mode_setup_plls still checks for ->enable, because all resources that are needed have to be calculated, else dpms changes may not succeed. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++-------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index eb52a039d628..dadd586f0527 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, return -EINVAL; } - if (!crtc->state->enable) { + if (!crtc->state->active) { DRM_DEBUG_KMS("crtc %d is disabled\n", pipe); return -EBUSY; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index eb40f9fc9b44..4648fa5ab55a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4629,7 +4629,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->enable || !intel_crtc->active) + if (!crtc->state->active) return; if (HAS_GMCH_DISPLAY(dev_priv->dev)) { @@ -4845,9 +4845,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) struct intel_encoder *encoder; int pipe = intel_crtc->pipe; - WARN_ON(!crtc->state->enable); - - if (intel_crtc->active) + if (WARN_ON(intel_crtc->active)) return; if (intel_crtc->config->has_pch_encoder) @@ -4951,9 +4949,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) struct intel_encoder *encoder; int pipe = intel_crtc->pipe; - WARN_ON(!crtc->state->enable); - - if (intel_crtc->active) + if (WARN_ON(intel_crtc->active)) return; if (intel_crtc_to_shared_dpll(intel_crtc)) @@ -5055,7 +5051,7 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; u32 reg, temp; - if (!intel_crtc->active) + if (WARN_ON(!intel_crtc->active)) return; for_each_encoder_on_crtc(dev, crtc, encoder) @@ -5118,7 +5114,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) struct intel_encoder *encoder; enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder; - if (!intel_crtc->active) + if (WARN_ON(!intel_crtc->active)) return; for_each_encoder_on_crtc(dev, crtc, encoder) { @@ -5978,7 +5974,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) /* add all active pipes to the state */ for_each_crtc(state->dev, crtc) { - if (!crtc->state->enable) + if (!crtc->state->active) continue; crtc_state = drm_atomic_get_crtc_state(state, crtc); @@ -5988,7 +5984,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) /* disable/enable all currently active pipes while we change cdclk */ for_each_crtc_in_state(state, crtc, crtc_state, i) - if (crtc_state->enable) + if (crtc_state->active) crtc_state->mode_changed = true; return 0; @@ -6076,9 +6072,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; bool is_dsi; - WARN_ON(!crtc->state->enable); - - if (intel_crtc->active) + if (WARN_ON(intel_crtc->active)) return; is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI); @@ -6154,9 +6148,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) struct intel_encoder *encoder; int pipe = intel_crtc->pipe; - WARN_ON(!crtc->state->enable); - - if (intel_crtc->active) + if (WARN_ON(intel_crtc->active)) return; i9xx_set_pll_dividers(intel_crtc); @@ -6216,7 +6208,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) struct intel_encoder *encoder; int pipe = intel_crtc->pipe; - if (!intel_crtc->active) + if (WARN_ON(!intel_crtc->active)) return; /* @@ -12264,7 +12256,7 @@ intel_modeset_update_state(struct drm_atomic_state *state) if (!crtc_state || !needs_modeset(crtc->state)) continue; - if (crtc->state->enable) { + if (crtc->state->active) { struct drm_property *dpms_property = dev->mode_config.dpms_property; @@ -12679,6 +12671,10 @@ check_crtc_state(struct drm_device *dev) "crtc active state doesn't match with hw state " "(expected %i, found %i)\n", crtc->active, active); + I915_STATE_WARN(crtc->active != crtc->base.state->active, + "transitional active state does not match atomic hw state " + "(expected %i, found %i)\n", crtc->base.state->active, crtc->active); + if (active && !intel_pipe_config_compare(dev, crtc->config, &pipe_config)) { I915_STATE_WARN(1, "pipe state doesn't match!\n"); @@ -12820,6 +12816,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc, if (IS_ERR(pipe_config)) return pipe_config; + if (!pipe_config->base.enable && + WARN_ON(pipe_config->base.active)) + pipe_config->base.active = false; + if (!pipe_config->base.enable) return pipe_config; @@ -12932,7 +12932,7 @@ static int __intel_set_mode(struct drm_atomic_state *state) return ret; for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!needs_modeset(crtc_state)) + if (!needs_modeset(crtc_state) || !crtc->state->active) continue; intel_crtc_disable_planes(crtc); @@ -12954,7 +12954,7 @@ static int __intel_set_mode(struct drm_atomic_state *state) /* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!needs_modeset(crtc->state) || !crtc->state->enable) + if (!needs_modeset(crtc->state) || !crtc->state->active) continue; update_scanline_offset(to_intel_crtc(crtc)); @@ -15215,7 +15215,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->enable) { + if (crtc->active != crtc->base.state->active) { struct intel_encoder *encoder; /* This can happen either due to bugs in the get_hw_state From 85a96e7a4213de094acc63fd433dcf766e91c782 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:53 +0200 Subject: [PATCH 09/33] drm/i915: Make sure all planes and connectors are added on modeset. Add missing calls to drm_atomic_add_affected_*. This is needed to convert to atomic planes. When converting to atomic all planes are needed on modeset. For good measure make sure all connectors are added too. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4648fa5ab55a..2b5b829a98ad 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5959,7 +5959,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; int max_pixclk = intel_mode_max_pixclk(state->dev, state); - int cdclk, i; + int cdclk, ret = 0; if (max_pixclk < 0) return max_pixclk; @@ -5974,20 +5974,25 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) /* add all active pipes to the state */ for_each_crtc(state->dev, crtc) { - if (!crtc->state->active) - continue; - crtc_state = drm_atomic_get_crtc_state(state, crtc); if (IS_ERR(crtc_state)) return PTR_ERR(crtc_state); + + if (!crtc_state->active || needs_modeset(crtc_state)) + continue; + + crtc_state->mode_changed = true; + + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + break; + + ret = drm_atomic_add_affected_planes(state, crtc); + if (ret) + break; } - /* disable/enable all currently active pipes while we change cdclk */ - for_each_crtc_in_state(state, crtc, crtc_state, i) - if (crtc_state->active) - crtc_state->mode_changed = true; - - return 0; + return ret; } static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv) @@ -12185,8 +12190,10 @@ encoder_retry: /* Check if we need to force a modeset */ if (pipe_config->has_audio != - to_intel_crtc_state(crtc->state)->has_audio) + to_intel_crtc_state(crtc->state)->has_audio) { pipe_config->base.mode_changed = true; + ret = drm_atomic_add_affected_planes(state, crtc); + } /* * Note we have an issue here with infoframes: current code @@ -12194,8 +12201,6 @@ encoder_retry: * requirements. So here we should be checking for any * required changes and forcing a mode set. */ - - return 0; fail: return ret; } From 36750f284b3a4f19b304fda1bb7d6e9e1275ea8d Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:54 +0200 Subject: [PATCH 10/33] drm/i915: update plane state during init Atomic planes updates rely on having a accurate plane_mask. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 2b5b829a98ad..91a0f7ca7ed2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2632,9 +2632,9 @@ valid_fb: dev_priv->preserve_bios_swizzle = true; primary->fb = fb; - primary->state->crtc = &intel_crtc->base; - primary->crtc = &intel_crtc->base; + primary->crtc = primary->state->crtc = &intel_crtc->base; update_state_fb(primary); + intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary)); obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe); } @@ -15563,7 +15563,9 @@ void intel_modeset_gem_init(struct drm_device *dev) to_intel_crtc(c)->pipe); drm_framebuffer_unreference(c->primary->fb); c->primary->fb = NULL; + c->primary->crtc = c->primary->state->crtc = NULL; update_state_fb(c->primary); + c->state->plane_mask &= ~(1 << drm_plane_index(c->primary)); } } From 8a8f7f44a1b704c482f77a0d31cbcdf1af062263 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:55 +0200 Subject: [PATCH 11/33] drm/i915: do not wait for vblank when crtc is off This can happen when turning off a sprite plane. Because the crtc state is not yet always swapped correctly and transitional helpers are used the crtc state cannot be relied on. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 91a0f7ca7ed2..e0edff9d034d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13768,7 +13768,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc) intel_runtime_pm_put(dev_priv); - if (intel_crtc->atomic.wait_vblank) + if (intel_crtc->atomic.wait_vblank && intel_crtc->active) intel_wait_for_vblank(dev, intel_crtc->pipe); intel_frontbuffer_flip(dev, intel_crtc->atomic.fb_bits); From fb9d6cf8c29bfcb0b3c602f7ded87f128d730382 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:56 +0200 Subject: [PATCH 12/33] drm/i915: calculate primary visibility changes instead of calling from set_config This should be much cleaner, with the same effects. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 46 +++++----------------------- 1 file changed, 7 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e0edff9d034d..52759c03c9e8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13225,20 +13225,11 @@ intel_modeset_stage_output_state(struct drm_device *dev, return 0; } -static bool primary_plane_visible(struct drm_crtc *crtc) -{ - struct intel_plane_state *plane_state = - to_intel_plane_state(crtc->primary->state); - - return plane_state->visible; -} - static int intel_crtc_set_config(struct drm_mode_set *set) { struct drm_device *dev; struct drm_atomic_state *state = NULL; struct intel_crtc_state *pipe_config; - bool primary_plane_was_visible; int ret; BUG_ON(!set); @@ -13277,38 +13268,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set) intel_update_pipe_size(to_intel_crtc(set->crtc)); - primary_plane_was_visible = primary_plane_visible(set->crtc); - ret = intel_set_mode_with_config(set->crtc, pipe_config); - if (ret == 0 && - pipe_config->base.enable && - pipe_config->base.planes_changed && - !needs_modeset(&pipe_config->base)) { - struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); - - /* - * We need to make sure the primary plane is re-enabled if it - * has previously been turned off. - */ - if (ret == 0 && !primary_plane_was_visible && - primary_plane_visible(set->crtc)) { - WARN_ON(!intel_crtc->active); - intel_post_enable_primary(set->crtc); - } - - /* - * In the fastboot case this may be our only check of the - * state after boot. It would be better to only do it on - * the first update, but we don't have a nice way of doing that - * (and really, set_config isn't used much for high freq page - * flipping, so increasing its cost here shouldn't be a big - * deal). - */ - if (i915.fastboot && ret == 0) - intel_modeset_check_state(set->crtc->dev); - } - if (ret) { DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n", set->crtc->base.id, ret); @@ -13641,8 +13602,15 @@ intel_check_primary_plane(struct drm_plane *plane, */ if (IS_BROADWELL(dev)) intel_crtc->atomic.wait_vblank = true; + + if (crtc_state && !needs_modeset(&crtc_state->base)) + intel_crtc->atomic.post_enable_primary = true; } + if (!state->visible && old_state->visible && + crtc_state && !needs_modeset(&crtc_state->base)) + intel_crtc->atomic.pre_disable_primary = true; + intel_crtc->atomic.fb_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe); From 568c634a2af62e07ed248a6e7fe9770173f9d9b2 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 1 Jun 2015 12:49:57 +0200 Subject: [PATCH 13/33] drm/i915: Support modeset across multiple pipes Compute new pipe_configs for all crtcs in the atomic state. The commit part of the mode set (__intel_set_mode()) is already enabled to support multiple pipes, the only thing missing was calculating a new pipe_config for every crtc. Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 108 +++++++++++---------------- 1 file changed, 45 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 52759c03c9e8..e566515e27ca 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -86,8 +86,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, static void ironlake_pch_clock_get(struct intel_crtc *crtc, struct intel_crtc_state *pipe_config); -static int intel_set_mode(struct drm_crtc *crtc, - struct drm_atomic_state *state); +static int intel_set_mode(struct drm_atomic_state *state); static int intel_framebuffer_init(struct drm_device *dev, struct intel_framebuffer *ifb, struct drm_mode_fb_cmd2 *mode_cmd, @@ -10482,7 +10481,7 @@ retry: drm_mode_copy(&crtc_state->base.mode, mode); - if (intel_set_mode(crtc, state)) { + if (intel_set_mode(state)) { DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n"); if (old->release_fb) old->release_fb->funcs->destroy(old->release_fb); @@ -10556,7 +10555,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, if (ret) goto fail; - ret = intel_set_mode(crtc, state); + ret = intel_set_mode(state); if (ret) goto fail; @@ -12068,9 +12067,10 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state) static int intel_modeset_pipe_config(struct drm_crtc *crtc, - struct drm_atomic_state *state, - struct intel_crtc_state *pipe_config) + struct drm_atomic_state *state) { + struct drm_crtc_state *crtc_state; + struct intel_crtc_state *pipe_config; struct intel_encoder *encoder; struct drm_connector *connector; struct drm_connector_state *connector_state; @@ -12088,6 +12088,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, return -EINVAL; } + crtc_state = drm_atomic_get_existing_crtc_state(state, crtc); + if (WARN_ON(!crtc_state)) + return -EINVAL; + + pipe_config = to_intel_crtc_state(crtc_state); + /* * XXX: Add all connectors to make the crtc state match the encoders. */ @@ -12800,45 +12806,35 @@ static void update_scanline_offset(struct intel_crtc *crtc) crtc->scanline_offset = 1; } -static struct intel_crtc_state * -intel_modeset_compute_config(struct drm_crtc *crtc, - struct drm_atomic_state *state) +static int +intel_modeset_compute_config(struct drm_atomic_state *state) { - struct intel_crtc_state *pipe_config; - int ret = 0; + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret, i; ret = drm_atomic_helper_check_modeset(state->dev, state); if (ret) - return ERR_PTR(ret); + return ret; - /* - * Note this needs changes when we start tracking multiple modes - * and crtcs. At that point we'll need to compute the whole config - * (i.e. one pipe_config for each crtc) rather than just the one - * for this crtc. - */ - pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc)); - if (IS_ERR(pipe_config)) - return pipe_config; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (!crtc_state->enable && + WARN_ON(crtc_state->active)) + crtc_state->active = false; - if (!pipe_config->base.enable && - WARN_ON(pipe_config->base.active)) - pipe_config->base.active = false; + if (!crtc_state->enable) + continue; - if (!pipe_config->base.enable) - return pipe_config; + ret = intel_modeset_pipe_config(crtc, state); + if (ret) + return ret; - ret = intel_modeset_pipe_config(crtc, state, pipe_config); - if (ret) - return ERR_PTR(ret); + intel_dump_pipe_config(to_intel_crtc(crtc), + to_intel_crtc_state(crtc_state), + "[modeset]"); + } - intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config, "[modeset]"); - - ret = drm_atomic_helper_check_planes(state->dev, state); - if (ret) - return ERR_PTR(ret); - - return pipe_config; + return drm_atomic_helper_check_planes(state->dev, state); } static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) @@ -12977,37 +12973,27 @@ static int __intel_set_mode(struct drm_atomic_state *state) return 0; } -static int intel_set_mode_with_config(struct drm_crtc *crtc, - struct intel_crtc_state *pipe_config) +static int intel_set_mode_checked(struct drm_atomic_state *state) { + struct drm_device *dev = state->dev; int ret; - ret = __intel_set_mode(pipe_config->base.state); - + ret = __intel_set_mode(state); if (ret == 0) - intel_modeset_check_state(crtc->dev); + intel_modeset_check_state(dev); return ret; } -static int intel_set_mode(struct drm_crtc *crtc, - struct drm_atomic_state *state) +static int intel_set_mode(struct drm_atomic_state *state) { - struct intel_crtc_state *pipe_config; - int ret = 0; + int ret; - pipe_config = intel_modeset_compute_config(crtc, state); - if (IS_ERR(pipe_config)) { - ret = PTR_ERR(pipe_config); - goto out; - } - - ret = intel_set_mode_with_config(crtc, pipe_config); + ret = intel_modeset_compute_config(state); if (ret) - goto out; + return ret; -out: - return ret; + return intel_set_mode_checked(state); } void intel_crtc_restore_mode(struct drm_crtc *crtc) @@ -13079,7 +13065,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc) intel_modeset_setup_plane_state(state, crtc, &crtc->mode, crtc->primary->fb, crtc->x, crtc->y); - ret = intel_set_mode(crtc, state); + ret = intel_set_mode(state); if (ret) drm_atomic_state_free(state); } @@ -13229,7 +13215,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set) { struct drm_device *dev; struct drm_atomic_state *state = NULL; - struct intel_crtc_state *pipe_config; int ret; BUG_ON(!set); @@ -13260,16 +13245,13 @@ static int intel_crtc_set_config(struct drm_mode_set *set) if (ret) goto out; - pipe_config = intel_modeset_compute_config(set->crtc, state); - if (IS_ERR(pipe_config)) { - ret = PTR_ERR(pipe_config); + ret = intel_modeset_compute_config(state); + if (ret) goto out; - } intel_update_pipe_size(to_intel_crtc(set->crtc)); - ret = intel_set_mode_with_config(set->crtc, pipe_config); - + ret = intel_set_mode_checked(state); if (ret) { DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n", set->crtc->base.id, ret); From 880fa62648bf193eb17193a9eae00a52c7844ea7 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:49:58 +0200 Subject: [PATCH 14/33] drm/i915: Zap call to drm_plane_helper_disable, v2. The primary plane can still be configured when crtc is off, furthermore this is also a noop now that affected planes are added on modesets. Changes since v1: - Move commit so no frontbuffer_bits warnings are generated. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e566515e27ca..dcd256f47fe5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12938,8 +12938,6 @@ static int __intel_set_mode(struct drm_atomic_state *state) intel_crtc_disable_planes(crtc); dev_priv->display.crtc_disable(crtc); - if (!crtc_state->enable) - drm_plane_helper_disable(crtc->primary); } /* Only after disabling all output pipelines that will be changed can we From de419ab6b774facc14b2fa71e3d8642027924c86 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Thu, 4 Jun 2015 10:21:28 +0200 Subject: [PATCH 15/33] drm/i915: Use global atomic state for staged pll, config, v3. Now that we can subclass drm_atomic_state we can also use it to keep track of all the pll settings. atomic_state is a better place to hold all shared state than keeping pll->new_config everywhere. Changes since v1: - Assert connection_mutex is held. Changes since v2: - Fix swapped arguments to kzalloc for intel_atomic_state_alloc. (Jani Nikula) Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_atomic.c | 51 ++++++++++++ drivers/gpu/drm/i915/intel_display.c | 113 ++++++++------------------- drivers/gpu/drm/i915/intel_drv.h | 13 +++ 4 files changed, 98 insertions(+), 80 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 03ae50ee4737..611fbd86c1cc 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -376,7 +376,6 @@ struct intel_shared_dpll_config { struct intel_shared_dpll { struct intel_shared_dpll_config config; - struct intel_shared_dpll_config *new_config; int active; /* count of number of active CRTCs (i.e. DPMS on) */ bool on; /* is the PLL actually active? Disabled during modeset */ diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 7ed8033aae60..45af3cc9d04c 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -421,3 +421,54 @@ int intel_atomic_setup_scalers(struct drm_device *dev, return 0; } + +static void +intel_atomic_duplicate_dpll_state(struct drm_i915_private *dev_priv, + struct intel_shared_dpll_config *shared_dpll) +{ + enum intel_dpll_id i; + + /* Copy shared dpll state */ + for (i = 0; i < dev_priv->num_shared_dpll; i++) { + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i]; + + shared_dpll[i] = pll->config; + } +} + +struct intel_shared_dpll_config * +intel_atomic_get_shared_dpll_state(struct drm_atomic_state *s) +{ + struct intel_atomic_state *state = to_intel_atomic_state(s); + + WARN_ON(!drm_modeset_is_locked(&s->dev->mode_config.connection_mutex)); + + if (!state->dpll_set) { + state->dpll_set = true; + + intel_atomic_duplicate_dpll_state(to_i915(s->dev), + state->shared_dpll); + } + + return state->shared_dpll; +} + +struct drm_atomic_state * +intel_atomic_state_alloc(struct drm_device *dev) +{ + struct intel_atomic_state *state = kzalloc(sizeof(*state), GFP_KERNEL); + + if (!state || drm_atomic_state_init(dev, &state->base) < 0) { + kfree(state); + return NULL; + } + + return &state->base; +} + +void intel_atomic_state_clear(struct drm_atomic_state *s) +{ + struct intel_atomic_state *state = to_intel_atomic_state(s); + drm_atomic_state_default_clear(&state->base); + state->dpll_set = false; +} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dcd256f47fe5..aec398cd2289 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4186,8 +4186,11 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, { struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; struct intel_shared_dpll *pll; + struct intel_shared_dpll_config *shared_dpll; enum intel_dpll_id i; + shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state); + if (HAS_PCH_IBX(dev_priv->dev)) { /* Ironlake PCH has a fixed PLL->PCH pipe mapping. */ i = (enum intel_dpll_id) crtc->pipe; @@ -4196,7 +4199,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", crtc->base.base.id, pll->name); - WARN_ON(pll->new_config->crtc_mask); + WARN_ON(shared_dpll[i].crtc_mask); goto found; } @@ -4216,7 +4219,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, pll = &dev_priv->shared_dplls[i]; DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", crtc->base.base.id, pll->name); - WARN_ON(pll->new_config->crtc_mask); + WARN_ON(shared_dpll[i].crtc_mask); goto found; } @@ -4225,15 +4228,15 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, pll = &dev_priv->shared_dplls[i]; /* Only want to check enabled timings first */ - if (pll->new_config->crtc_mask == 0) + if (shared_dpll[i].crtc_mask == 0) continue; if (memcmp(&crtc_state->dpll_hw_state, - &pll->new_config->hw_state, - sizeof(pll->new_config->hw_state)) == 0) { + &shared_dpll[i].hw_state, + sizeof(crtc_state->dpll_hw_state)) == 0) { DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n", crtc->base.base.id, pll->name, - pll->new_config->crtc_mask, + shared_dpll[i].crtc_mask, pll->active); goto found; } @@ -4242,7 +4245,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, /* Ok no matching timings, maybe there's a free one? */ for (i = 0; i < dev_priv->num_shared_dpll; i++) { pll = &dev_priv->shared_dplls[i]; - if (pll->new_config->crtc_mask == 0) { + if (shared_dpll[i].crtc_mask == 0) { DRM_DEBUG_KMS("CRTC:%d allocated %s\n", crtc->base.base.id, pll->name); goto found; @@ -4252,83 +4255,33 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc, return NULL; found: - if (pll->new_config->crtc_mask == 0) - pll->new_config->hw_state = crtc_state->dpll_hw_state; + if (shared_dpll[i].crtc_mask == 0) + shared_dpll[i].hw_state = + crtc_state->dpll_hw_state; crtc_state->shared_dpll = i; DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name, pipe_name(crtc->pipe)); - pll->new_config->crtc_mask |= 1 << crtc->pipe; + shared_dpll[i].crtc_mask |= 1 << crtc->pipe; return pll; } -/** - * intel_shared_dpll_start_config - start a new PLL staged config - * @dev_priv: DRM device - * @clear_pipes: mask of pipes that will have their PLLs freed - * - * Starts a new PLL staged config, copying the current config but - * releasing the references of pipes specified in clear_pipes. - */ -static int intel_shared_dpll_start_config(struct drm_i915_private *dev_priv, - unsigned clear_pipes) +static void intel_shared_dpll_commit(struct drm_atomic_state *state) { + struct drm_i915_private *dev_priv = to_i915(state->dev); + struct intel_shared_dpll_config *shared_dpll; struct intel_shared_dpll *pll; enum intel_dpll_id i; + if (!to_intel_atomic_state(state)->dpll_set) + return; + + shared_dpll = to_intel_atomic_state(state)->shared_dpll; for (i = 0; i < dev_priv->num_shared_dpll; i++) { pll = &dev_priv->shared_dplls[i]; - - pll->new_config = kmemdup(&pll->config, sizeof pll->config, - GFP_KERNEL); - if (!pll->new_config) - goto cleanup; - - pll->new_config->crtc_mask &= ~clear_pipes; - } - - return 0; - -cleanup: - while (--i >= 0) { - pll = &dev_priv->shared_dplls[i]; - kfree(pll->new_config); - pll->new_config = NULL; - } - - return -ENOMEM; -} - -static void intel_shared_dpll_commit(struct drm_i915_private *dev_priv) -{ - struct intel_shared_dpll *pll; - enum intel_dpll_id i; - - for (i = 0; i < dev_priv->num_shared_dpll; i++) { - pll = &dev_priv->shared_dplls[i]; - - WARN_ON(pll->new_config == &pll->config); - - pll->config = *pll->new_config; - kfree(pll->new_config); - pll->new_config = NULL; - } -} - -static void intel_shared_dpll_abort_config(struct drm_i915_private *dev_priv) -{ - struct intel_shared_dpll *pll; - enum intel_dpll_id i; - - for (i = 0; i < dev_priv->num_shared_dpll; i++) { - pll = &dev_priv->shared_dplls[i]; - - WARN_ON(pll->new_config == &pll->config); - - kfree(pll->new_config); - pll->new_config = NULL; + pll->config = shared_dpll[i]; } } @@ -12227,13 +12180,12 @@ static void intel_modeset_update_state(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; - struct drm_i915_private *dev_priv = dev->dev_private; struct intel_encoder *intel_encoder; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; struct drm_connector *connector; - intel_shared_dpll_commit(dev_priv); + intel_shared_dpll_commit(state); drm_atomic_helper_swap_state(state->dev, state); for_each_intel_encoder(dev, intel_encoder) { @@ -12862,9 +12814,13 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) } } - ret = intel_shared_dpll_start_config(dev_priv, clear_pipes); - if (ret) - goto done; + if (clear_pipes) { + struct intel_shared_dpll_config *shared_dpll = + intel_atomic_get_shared_dpll_state(state); + + for (i = 0; i < dev_priv->num_shared_dpll; i++) + shared_dpll[i].crtc_mask &= ~clear_pipes; + } for_each_crtc_in_state(state, crtc, crtc_state, i) { if (!needs_modeset(crtc_state) || !crtc_state->enable) @@ -12875,13 +12831,10 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) ret = dev_priv->display.crtc_compute_clock(intel_crtc, intel_crtc_state); - if (ret) { - intel_shared_dpll_abort_config(dev_priv); - goto done; - } + if (ret) + return ret; } -done: return ret; } @@ -14582,6 +14535,8 @@ static const struct drm_mode_config_funcs intel_mode_funcs = { .output_poll_changed = intel_fbdev_output_poll_changed, .atomic_check = intel_atomic_check, .atomic_commit = intel_atomic_commit, + .atomic_state_alloc = intel_atomic_state_alloc, + .atomic_state_clear = intel_atomic_state_clear, }; /* Set up chip specific display functions */ diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ea1351f87f0a..e15039800cf0 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -241,6 +241,13 @@ typedef struct dpll { int p; } intel_clock_t; +struct intel_atomic_state { + struct drm_atomic_state base; + + bool dpll_set; + struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS]; +}; + struct intel_plane_state { struct drm_plane_state base; struct drm_rect src; @@ -628,6 +635,7 @@ struct cxsr_latency { unsigned long cursor_hpll_disable; }; +#define to_intel_atomic_state(x) container_of(x, struct intel_atomic_state, base) #define to_intel_crtc(x) container_of(x, struct intel_crtc, base) #define to_intel_crtc_state(x) container_of(x, struct intel_crtc_state, base) #define to_intel_connector(x) container_of(x, struct intel_connector, base) @@ -1404,6 +1412,11 @@ int intel_connector_atomic_get_property(struct drm_connector *connector, struct drm_crtc_state *intel_crtc_duplicate_state(struct drm_crtc *crtc); void intel_crtc_destroy_state(struct drm_crtc *crtc, struct drm_crtc_state *state); +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); + static inline struct intel_crtc_state * intel_atomic_get_crtc_state(struct drm_atomic_state *state, struct intel_crtc *crtc) From 61c054983271426f8d31ef9e52eda249b123a7df Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:00 +0200 Subject: [PATCH 16/33] drm/i915: Use drm_atomic_helper_swap_state in intel_atomic_commit. And update crtc->config to point to the new state. There is no point in swapping only part of the state when the rest of the state should be untouched. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_atomic.c | 50 +++++++++-------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 45af3cc9d04c..d0b901ddb357 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -129,6 +129,8 @@ int intel_atomic_commit(struct drm_device *dev, struct drm_atomic_state *state, bool async) { + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; int ret; int i; @@ -142,48 +144,26 @@ int intel_atomic_commit(struct drm_device *dev, return ret; /* Point of no return */ + drm_atomic_helper_swap_state(dev, state); - /* - * FIXME: The proper sequence here will eventually be: - * - * drm_atomic_helper_swap_state(dev, state) - * drm_atomic_helper_commit_modeset_disables(dev, state); - * drm_atomic_helper_commit_planes(dev, state); - * drm_atomic_helper_commit_modeset_enables(dev, state); - * drm_atomic_helper_wait_for_vblanks(dev, state); - * drm_atomic_helper_cleanup_planes(dev, state); - * drm_atomic_state_free(state); - * - * once we have full atomic modeset. For now, just manually update - * plane states to avoid clobbering good states with dummy states - * while nuclear pageflipping. - */ - for (i = 0; i < dev->mode_config.num_total_plane; i++) { - struct drm_plane *plane = state->planes[i]; - - if (!plane) - continue; - - plane->state->state = state; - swap(state->plane_states[i], plane->state); - plane->state->state = NULL; - } - - /* swap crtc_scaler_state */ - for (i = 0; i < dev->mode_config.num_crtc; i++) { - struct drm_crtc *crtc = state->crtcs[i]; - if (!crtc) { - continue; - } - - to_intel_crtc(crtc)->config->scaler_state = - to_intel_crtc_state(state->crtc_states[i])->scaler_state; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); if (INTEL_INFO(dev)->gen >= 9) skl_detach_scalers(to_intel_crtc(crtc)); } + /* + * FIXME: The proper sequence here will eventually be: + * + * drm_atomic_helper_commit_modeset_disables(dev, state); + * drm_atomic_helper_commit_planes(dev, state); + * drm_atomic_helper_commit_modeset_enables(dev, state); + * + * once we have full atomic modeset. + */ drm_atomic_helper_commit_planes(dev, state); + drm_atomic_helper_wait_for_vblanks(dev, state); drm_atomic_helper_cleanup_planes(dev, state); drm_atomic_state_free(state); From 5ac1c4bcf073ad897c4510931518275d9e393dc7 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:01 +0200 Subject: [PATCH 17/33] drm/i915: Swap planes on each crtc separately, v2. Repeated calls to begin_crtc_commit can cause warnings like this: [ 169.127746] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616 [ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip [ 169.127840] 3 locks held by kms_flip/1947: [ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at: [] __drm_modeset_lock_all+0x9c/0x130 [ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: [] __drm_modeset_lock_all+0xad/0x130 [ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: [] drm_modeset_lock+0x38/0x110 [ 169.127879] irq event stamp: 665690 [ 169.127882] hardirqs last enabled at (665689): [] _raw_spin_unlock_irqrestore+0x55/0x70 [ 169.127889] hardirqs last disabled at (665690): [] intel_pipe_update_start+0x113/0x5c0 [i915] [ 169.127936] softirqs last enabled at (665470): [] __do_softirq+0x236/0x650 [ 169.127942] softirqs last disabled at (665465): [] irq_exit+0xc5/0xd0 [ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+ #4039 [ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 [ 169.127957] ffff8800c49036f0 ffff8800cde5fa28 ffffffff817f6907 0000000080000001 [ 169.127964] 0000000000000000 ffff8800cde5fa58 ffffffff810aebed 0000000000000046 [ 169.127970] ffffffff81c5d518 0000000000000268 0000000000000000 ffff8800cde5fa88 [ 169.127981] Call Trace: [ 169.127992] [] dump_stack+0x4f/0x7b [ 169.128001] [] ___might_sleep+0x16d/0x270 [ 169.128008] [] __might_sleep+0x48/0x90 [ 169.128017] [] mutex_lock_nested+0x29/0x410 [ 169.128073] [] ? vgpu_write64+0x220/0x220 [i915] [ 169.128138] [] ? ironlake_update_primary_plane+0x2ff/0x410 [i915] [ 169.128198] [] intel_frontbuffer_flush+0x25/0x70 [i915] [ 169.128253] [] intel_finish_crtc_commit+0x4c/0x180 [i915] [ 169.128279] [] drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper] [ 169.128338] [] __intel_set_mode+0x684/0x830 [i915] [ 169.128378] [] intel_crtc_set_config+0x49a/0x620 [i915] [ 169.128385] [] ? mutex_unlock+0x9/0x10 [ 169.128391] [] drm_mode_set_config_internal+0x69/0x120 [ 169.128398] [] ? might_fault+0x57/0xb0 [ 169.128403] [] drm_mode_setcrtc+0x253/0x620 [ 169.128409] [] drm_ioctl+0x1a0/0x6a0 [ 169.128415] [] ? get_parent_ip+0x11/0x50 [ 169.128424] [] do_vfs_ioctl+0x2f8/0x530 [ 169.128429] [] ? trace_hardirqs_on+0xd/0x10 [ 169.128435] [] ? selinux_file_ioctl+0x56/0x100 [ 169.128439] [] SyS_ioctl+0x81/0xa0 [ 169.128445] [] system_call_fastpath+0x12/0x6f Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc. The problem here was that the drm_atomic_helper_commit_planes() helper we were using was basically designed to do begin_crtc_commit(crtc #1) begin_crtc_commit(crtc #2) ... commit all planes finish_crtc_commit(crtc #1) finish_crtc_commit(crtc #2) The problem here is that since our hardware relies on vblank evasion, our CRTC 'begin' function waits until we're out of the danger zone in which register writes might wind up straddling the vblank, then disables interrupts; our 'finish' function re-enables interrupts after the registers have been written. The expectation is that the operations between 'begin' and 'end' must be performed without sleeping (since interrupts are disabled) and should happen as quickly as possible. By clumping all of the 'begin' calls together, we introducing a couple problems: * Subsequent 'begin' invocations might sleep (which is illegal) * The first 'begin' ensured that we were far enough from the vblank that we could write our registers safely and ensure they all fell within the same frame. Adding extra delay waiting for subsequent CRTC's wasn't accounted for and could put us back into the 'danger zone' for CRTC #1. This commit solves the problem by using a new helper that allows an order of operations like: for each crtc { begin_crtc_commit(crtc) // sleep (maybe), then disable interrupts commit planes for this specific CRTC end_crtc_commit(crtc) // reenable interrupts } so that sleeps will only be performed while interrupts are enabled and we can be sure that registers for a CRTC will be written immediately once we know we're in the safe zone. The crtc->config->base.crtc update may seem unrelated, but the helper will use it to obtain the crtc for the state. Without the update it will dereference NULL and crash. Changes since v1: - Use Matt Roper's commit message. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_atomic.c | 13 +++---------- drivers/gpu/drm/i915/intel_display.c | 5 +++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index d0b901ddb357..4df6d2d7a9c8 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -151,18 +151,11 @@ int intel_atomic_commit(struct drm_device *dev, if (INTEL_INFO(dev)->gen >= 9) skl_detach_scalers(to_intel_crtc(crtc)); + + drm_atomic_helper_commit_planes_on_crtc(crtc_state); } - /* - * FIXME: The proper sequence here will eventually be: - * - * drm_atomic_helper_commit_modeset_disables(dev, state); - * drm_atomic_helper_commit_planes(dev, state); - * drm_atomic_helper_commit_modeset_enables(dev, state); - * - * once we have full atomic modeset. - */ - drm_atomic_helper_commit_planes(dev, state); + /* FIXME: This function should eventually call __intel_set_mode when needed */ drm_atomic_helper_wait_for_vblanks(dev, state); drm_atomic_helper_cleanup_planes(dev, state); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index aec398cd2289..b56641b6de70 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12902,10 +12902,10 @@ static int __intel_set_mode(struct drm_atomic_state *state) modeset_update_crtc_power_domains(state); - drm_atomic_helper_commit_planes(dev, state); - /* Now enable the clocks, plane, pipe, and connectors that we set up. */ for_each_crtc_in_state(state, crtc, crtc_state, i) { + drm_atomic_helper_commit_planes_on_crtc(crtc_state); + if (!needs_modeset(crtc->state) || !crtc->state->active) continue; @@ -15267,6 +15267,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) struct intel_plane_state *plane_state; memset(crtc->config, 0, sizeof(*crtc->config)); + crtc->config->base.crtc = &crtc->base; crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; From c347a6768df15c7145ac16bf4b4f7c5fc2be1179 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 1 Jun 2015 12:50:02 +0200 Subject: [PATCH 18/33] drm/i915: Move cdclk and pll setup to intel_modeset_compute_config(), v2. It makes more sense there, since these are computation steps that can fail. Changes since v1: - Rename __intel_set_mode_checks to intel_modeset_checks (Matt Roper) - Move intel_modeset_checks to before check_planes, so it won't have to be moved later. Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 74 +++++++++++++--------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b56641b6de70..12d6da8f0465 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12758,38 +12758,7 @@ static void update_scanline_offset(struct intel_crtc *crtc) crtc->scanline_offset = 1; } -static int -intel_modeset_compute_config(struct drm_atomic_state *state) -{ - struct drm_crtc *crtc; - struct drm_crtc_state *crtc_state; - int ret, i; - - ret = drm_atomic_helper_check_modeset(state->dev, state); - if (ret) - return ret; - - for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!crtc_state->enable && - WARN_ON(crtc_state->active)) - crtc_state->active = false; - - if (!crtc_state->enable) - continue; - - ret = intel_modeset_pipe_config(crtc, state); - if (ret) - return ret; - - intel_dump_pipe_config(to_intel_crtc(crtc), - to_intel_crtc_state(crtc_state), - "[modeset]"); - } - - return drm_atomic_helper_check_planes(state->dev, state); -} - -static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) +static int intel_modeset_setup_plls(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; struct drm_i915_private *dev_priv = to_i915(dev); @@ -12839,7 +12808,7 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state) } /* Code that should eventually be part of atomic_check() */ -static int __intel_set_mode_checks(struct drm_atomic_state *state) +static int intel_modeset_checks(struct drm_atomic_state *state) { struct drm_device *dev = state->dev; int ret; @@ -12861,11 +12830,42 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state) return ret; } - ret = __intel_set_mode_setup_plls(state); + return intel_modeset_setup_plls(state); +} + +static int +intel_modeset_compute_config(struct drm_atomic_state *state) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + int ret, i; + + ret = drm_atomic_helper_check_modeset(state->dev, state); if (ret) return ret; - return 0; + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (!crtc_state->enable && + WARN_ON(crtc_state->active)) + crtc_state->active = false; + + if (!crtc_state->enable) + continue; + + ret = intel_modeset_pipe_config(crtc, state); + if (ret) + return ret; + + intel_dump_pipe_config(to_intel_crtc(crtc), + to_intel_crtc_state(crtc_state), + "[modeset]"); + } + + ret = intel_modeset_checks(state); + if (ret) + return ret; + + return drm_atomic_helper_check_planes(state->dev, state); } static int __intel_set_mode(struct drm_atomic_state *state) @@ -12877,10 +12877,6 @@ static int __intel_set_mode(struct drm_atomic_state *state) int ret = 0; int i; - ret = __intel_set_mode_checks(state); - if (ret < 0) - return ret; - ret = drm_atomic_helper_prepare_planes(dev, state); if (ret) return ret; From 37ade41794e914103b8db417e480afd20dcea971 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 1 Jun 2015 12:50:03 +0200 Subject: [PATCH 19/33] drm/i915: Read hw state into an atomic state struct, v2. 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 Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_atomic.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 389 +++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 14 +- 3 files changed, 248 insertions(+), 157 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 4df6d2d7a9c8..e47e00e5b130 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 12d6da8f0465..c04bc80d1a65 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10312,7 +10312,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: @@ -10330,10 +10330,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; @@ -10352,9 +10352,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; @@ -10365,20 +10362,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; @@ -10446,9 +10440,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; @@ -10494,10 +10486,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; @@ -11644,33 +11632,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. @@ -12201,7 +12162,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) { @@ -12505,11 +12465,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"); } } @@ -12529,8 +12492,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"); @@ -12540,6 +12501,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 @@ -12969,11 +12933,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); @@ -12986,14 +12950,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", @@ -13002,9 +12962,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); } @@ -15080,6 +15037,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; @@ -15243,78 +15201,182 @@ 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 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); + + /* update transitional state */ + crtc->active = crtc_state->base.active; + crtc->config = crtc_state; + + 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_encoder *encoder; - struct intel_connector *connector; + struct intel_crtc_state *crtc_state; int i; - for_each_intel_crtc(dev, crtc) { - struct drm_plane *primary = crtc->base.primary; - struct intel_plane_state *plane_state; - - memset(crtc->config, 0, sizeof(*crtc->config)); - crtc->config->base.crtc = &crtc->base; - - crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; - - 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; - - plane_state = to_intel_plane_state(primary->state); - plane_state->visible = primary_get_hw_state(crtc); - - 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, - &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); } + 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)) { - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); - encoder->base.crtc = &crtc->base; - encoder->get_config(encoder, crtc->config); + 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; } - encoder->connectors_active = false; DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe %c\n", encoder->base.base.id, encoder->base.name, @@ -15322,20 +15384,42 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) pipe_name(pipe)); } - 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"); + 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 @@ -15344,37 +15428,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]"); } @@ -15398,20 +15502,17 @@ 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); - /* - * 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); + 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 e15039800cf0..4aa10a5198f6 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; @@ -1416,6 +1404,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, From 5da76e94c4b8d40465a907fc3a151051e8021cdc Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:04 +0200 Subject: [PATCH 20/33] drm/i915: Implement intel_crtc_control using atomic state, v4 Assume the callers lock everything with drm_modeset_lock_all. This change had to be done after converting suspend/resume to use atomic_state so the atomic state is preserved, otherwise all transitional state is erased. Now all callers of .crtc_enable and .crtc_disable go through atomic modeset! :-D Changes since v1: - Only check for crtc_state->active in valleyview_modeset_global_pipes. - Only check for crtc_state->active in modeset_update_crtc_power_domains. Changes since v2: - Rework on top of the changed patch order. Changes since v3: - Rename intel_crtc_toggle in description to *_control - Change return value to int. - Do not add plane state, should be done implicitly already. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 53 +++++++++++++++++----------- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c04bc80d1a65..048a86857d0d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6238,38 +6238,49 @@ void intel_display_suspend(struct drm_device *dev) } /* Master function to enable/disable CRTC and corresponding power wells */ -void intel_crtc_control(struct drm_crtc *crtc, bool enable) +int intel_crtc_control(struct drm_crtc *crtc, bool enable) { struct drm_device *dev = crtc->dev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_mode_config *config = &dev->mode_config; + struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum intel_display_power_domain domain; - unsigned long domains; + struct intel_crtc_state *pipe_config; + struct drm_atomic_state *state; + int ret; if (enable == intel_crtc->active) - return; + return 0; if (enable && !crtc->state->enable) - return; + return 0; - crtc->state->active = enable; - if (enable) { - domains = get_crtc_power_domains(crtc); - for_each_power_domain(domain, domains) - intel_display_power_get(dev_priv, domain); - intel_crtc->enabled_power_domains = domains; + /* this function should be called with drm_modeset_lock_all for now */ + if (WARN_ON(!ctx)) + return -EIO; + lockdep_assert_held(&ctx->ww_ctx); - dev_priv->display.crtc_enable(crtc); - intel_crtc_enable_planes(crtc); - } else { - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); + state = drm_atomic_state_alloc(dev); + if (WARN_ON(!state)) + return -ENOMEM; - 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; + state->acquire_ctx = ctx; + state->allow_modeset = true; + + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); + if (IS_ERR(pipe_config)) { + ret = PTR_ERR(pipe_config); + goto err; } + pipe_config->base.active = enable; + + ret = intel_set_mode(state); + if (!ret) + return ret; + +err: + DRM_ERROR("Updating crtc active failed with %i\n", ret); + drm_atomic_state_free(state); + return ret; } /** diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4aa10a5198f6..7f8fa7f026aa 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -990,7 +990,7 @@ 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); void intel_display_suspend(struct drm_device *dev); -void intel_crtc_control(struct drm_crtc *crtc, bool enable); +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); int intel_connector_init(struct intel_connector *); From 06ea0b0897db906c5616f660a34b54d92f7d09cf Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:05 +0200 Subject: [PATCH 21/33] drm/i915: Make intel_display_suspend atomic, v2. Calculate all state using a normal transition, but afterwards fudge crtc->state->active back to its old value. This should still allow state restore in setup_hw_state to work properly. Calling intel_set_mode will cause intel_display_set_init_power to be called, make sure init_power gets set again afterwards. Changes since v1: - Fix to compile with v2 of the patch that adds intel_display_suspend. - Add intel_display_set_init_power. - Set return value to int to allow error checking. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 3 ++ drivers/gpu/drm/i915/intel_display.c | 57 +++++++++++++++++++++------- drivers/gpu/drm/i915/intel_drv.h | 2 +- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 78ef0bb53c36..d3632c56fdf7 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -634,6 +634,9 @@ 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 048a86857d0d..547d6db2a6ee 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6214,27 +6214,58 @@ 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. */ -void intel_display_suspend(struct drm_device *dev) +int intel_display_suspend(struct drm_device *dev) { - struct drm_i915_private *dev_priv = to_i915(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_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 intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum intel_display_power_domain domain; - unsigned long domains; + struct drm_crtc_state *crtc_state = + drm_atomic_get_crtc_state(state, crtc); - if (!intel_crtc->active) + ret = PTR_ERR_OR_ZERO(crtc_state); + if (ret) + goto free; + + if (!crtc_state->active) continue; - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); - - 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; + 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; + + return ret; + } + } + +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 7f8fa7f026aa..6d9c771747f9 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -989,7 +989,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); -void intel_display_suspend(struct drm_device *dev); +int 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); From 1c5e19f8f124b2ff442756e79d9a05c2b9494a28 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:06 +0200 Subject: [PATCH 22/33] drm/i915: move swap state to the right place This is a preparation for passing crtc state to the helpers. When converting all users of crtc->config to use the old or new state it's easier to find regressions when swap_state is done first. If crtc->config is swapped at the same place as swap_state bugs will never be found. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 547d6db2a6ee..d4d7708b58a4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12189,7 +12189,6 @@ intel_modeset_update_state(struct drm_atomic_state *state) struct drm_connector *connector; intel_shared_dpll_commit(state); - drm_atomic_helper_swap_state(state->dev, state); for_each_intel_encoder(dev, intel_encoder) { if (!intel_encoder->base.crtc) @@ -12887,8 +12886,10 @@ static int __intel_set_mode(struct drm_atomic_state *state) if (ret) return ret; + drm_atomic_helper_swap_state(dev, state); + for_each_crtc_in_state(state, crtc, crtc_state, i) { - if (!needs_modeset(crtc_state) || !crtc->state->active) + if (!needs_modeset(crtc->state) || !crtc_state->active) continue; intel_crtc_disable_planes(crtc); From fc467a221a01c5ec369a3969fcafea077c3677b4 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:07 +0200 Subject: [PATCH 23/33] drm/i915: Use crtc->hwmode for vblanks, v2. intel_crtc->config will be removed eventually, so use crtc->hwmode. drm_atomic_helper_update_legacy_modeset_state updates hwmode, but crtc->active will eventually be gone too. Set dotclock to zero to indicate the crtc is inactive. Changes since v1: - With the hwmode update in drm*update_legacy_modeset_state removed, intel_modeset_update_state has to assign it instead. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_irq.c | 13 ++++++------- drivers/gpu/drm/i915/intel_display.c | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dadd586f0527..56db9e747464 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -564,8 +564,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe) u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal; struct intel_crtc *intel_crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); - const struct drm_display_mode *mode = - &intel_crtc->config->base.adjusted_mode; + const struct drm_display_mode *mode = &intel_crtc->base.hwmode; htotal = mode->crtc_htotal; hsync_start = mode->crtc_hsync_start; @@ -620,7 +619,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - const struct drm_display_mode *mode = &crtc->config->base.adjusted_mode; + const struct drm_display_mode *mode = &crtc->base.hwmode; enum pipe pipe = crtc->pipe; int position, vtotal; @@ -647,14 +646,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - const struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode; + const struct drm_display_mode *mode = &intel_crtc->base.hwmode; int position; int vbl_start, vbl_end, hsync_start, htotal, vtotal; bool in_vbl = true; int ret = 0; unsigned long irqflags; - if (!intel_crtc->active) { + if (WARN_ON(!mode->crtc_clock)) { DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled " "pipe %c\n", pipe_name(pipe)); return 0; @@ -796,7 +795,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, return -EINVAL; } - if (!crtc->state->active) { + if (!crtc->hwmode.crtc_clock) { DRM_DEBUG_KMS("crtc %d is disabled\n", pipe); return -EBUSY; } @@ -805,7 +804,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe, return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, vblank_time, flags, crtc, - &to_intel_crtc(crtc)->config->base.adjusted_mode); + &crtc->hwmode); } static bool intel_hpd_irq_event(struct drm_device *dev, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index d4d7708b58a4..7e2bbd7e6210 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12209,6 +12209,12 @@ intel_modeset_update_state(struct drm_atomic_state *state) WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc)); to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state); + + /* Update hwmode for vblank functions */ + if (crtc->state->active) + crtc->hwmode = crtc->state->adjusted_mode; + else + crtc->hwmode.crtc_clock = 0; } list_for_each_entry(connector, &dev->mode_config.connector_list, head) { From f77076c91d563a07c6519b80e234b4e962306b67 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:08 +0200 Subject: [PATCH 24/33] drm/i915: Remove use of crtc->config from i915_debugfs.c crtc->config is updated to always contain to the active crtc_state and only differs from crtc_state during crtc_disable. It will eventually be removed, so start with some low hanging fruit. For crtc->active the situation is the same; it will be removed eventually. Instead use crtc->state->active. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_debugfs.c | 42 ++++++++++++++++++----------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index aac252ca0bda..0029c25bf72a 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2779,13 +2779,16 @@ static int i915_display_info(struct seq_file *m, void *unused) seq_printf(m, "---------\n"); for_each_intel_crtc(dev, crtc) { bool active; + struct intel_crtc_state *pipe_config; int x, y; + pipe_config = to_intel_crtc_state(crtc->base.state); + seq_printf(m, "CRTC %d: pipe: %c, active=%s (size=%dx%d)\n", crtc->base.base.id, pipe_name(crtc->pipe), - yesno(crtc->active), crtc->config->pipe_src_w, - crtc->config->pipe_src_h); - if (crtc->active) { + yesno(pipe_config->base.active), + pipe_config->pipe_src_w, pipe_config->pipe_src_h); + if (pipe_config->base.active) { intel_crtc_info(m, crtc); active = cursor_position(dev, crtc->pipe, &x, &y); @@ -3026,7 +3029,7 @@ static void drrs_status_per_crtc(struct seq_file *m, seq_puts(m, "\n\n"); - if (intel_crtc->config->has_drrs) { + if (to_intel_crtc_state(intel_crtc->base.state)->has_drrs) { struct intel_panel *panel; mutex_lock(&drrs->mutex); @@ -3078,7 +3081,7 @@ static int i915_drrs_status(struct seq_file *m, void *unused) for_each_intel_crtc(dev, intel_crtc) { drm_modeset_lock(&intel_crtc->base.mutex, NULL); - if (intel_crtc->active) { + if (intel_crtc->base.state->active) { active_crtc_cnt++; seq_printf(m, "\nCRTC %d: ", active_crtc_cnt); @@ -3620,22 +3623,27 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]); + struct intel_crtc_state *pipe_config; drm_modeset_lock_all(dev); + pipe_config = to_intel_crtc_state(crtc->base.state); + /* * If we use the eDP transcoder we need to make sure that we don't * bypass the pfit, since otherwise the pipe CRC source won't work. Only * relevant on hsw with pipe A when using the always-on power well * routing. */ - if (crtc->config->cpu_transcoder == TRANSCODER_EDP && - !crtc->config->pch_pfit.enabled) { - bool active = crtc->active; + if (pipe_config->cpu_transcoder == TRANSCODER_EDP && + !pipe_config->pch_pfit.enabled) { + bool active = pipe_config->base.active; - if (active) + if (active) { intel_crtc_control(&crtc->base, false); + pipe_config = to_intel_crtc_state(crtc->base.state); + } - crtc->config->pch_pfit.force_thru = true; + pipe_config->pch_pfit.force_thru = true; intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); @@ -3651,6 +3659,7 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_A]); + struct intel_crtc_state *pipe_config; drm_modeset_lock_all(dev); /* @@ -3659,13 +3668,16 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev) * relevant on hsw with pipe A when using the always-on power well * routing. */ - if (crtc->config->pch_pfit.force_thru) { - bool active = crtc->active; + pipe_config = to_intel_crtc_state(crtc->base.state); + if (pipe_config->pch_pfit.force_thru) { + bool active = pipe_config->base.active; - if (active) + if (active) { intel_crtc_control(&crtc->base, false); + pipe_config = to_intel_crtc_state(crtc->base.state); + } - crtc->config->pch_pfit.force_thru = false; + pipe_config->pch_pfit.force_thru = false; intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A)); @@ -3787,7 +3799,7 @@ static int pipe_crc_set_source(struct drm_device *dev, enum pipe pipe, pipe_name(pipe)); drm_modeset_lock(&crtc->base.mutex, NULL); - if (crtc->active) + if (crtc->base.state->active) intel_wait_for_vblank(dev, pipe); drm_modeset_unlock(&crtc->base.mutex); From 99d736a2ce431dbbcf96ee9d26bd41ca2c2284a1 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:09 +0200 Subject: [PATCH 25/33] drm/i915: Calculate haswell plane workaround, v5. This needs to be done last after all modesets have been calculated. A modeset first disables all crtc's, so any crtc that undergoes a modeset counts as inactive. If no modeset's done, or > 1 crtc's stay w/a doesn't apply. Apply workaround on the first crtc if 1 crtc stays active. Apply workaround on the second crtc if no crtc was active. Changes since v1: - Use intel_crtc->atomic as a place to put hsw_workaround_pipe. - Make sure quirk only applies to haswell. - Use first loop to iterate over newly enabled crtc's only. This increases readability. Changes since v2: - Move hsw_workaround_pipe back to crtc_state. Changes since v3: - Return errors from haswell_mode_set_planes_workaround. Changes since v4: - Clean up commit message. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 113 +++++++++++++++++++-------- drivers/gpu/drm/i915/intel_drv.h | 3 + 2 files changed, 84 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7e2bbd7e6210..cd9a7545b0d3 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4864,42 +4864,15 @@ static bool hsw_crtc_supports_ips(struct intel_crtc *crtc) return HAS_IPS(crtc->base.dev) && crtc->pipe == PIPE_A; } -/* - * This implements the workaround described in the "notes" section of the mode - * set sequence documentation. When going from no pipes or single pipe to - * multiple pipes, and planes are enabled after the pipe, we need to wait at - * least 2 vblanks on the first pipe before enabling planes on the second pipe. - */ -static void haswell_mode_set_planes_workaround(struct intel_crtc *crtc) -{ - struct drm_device *dev = crtc->base.dev; - struct intel_crtc *crtc_it, *other_active_crtc = NULL; - - /* We want to get the other_active_crtc only if there's only 1 other - * active crtc. */ - for_each_intel_crtc(dev, crtc_it) { - if (!crtc_it->active || crtc_it == crtc) - continue; - - if (other_active_crtc) - return; - - other_active_crtc = crtc_it; - } - if (!other_active_crtc) - return; - - intel_wait_for_vblank(dev, other_active_crtc->pipe); - intel_wait_for_vblank(dev, other_active_crtc->pipe); -} - static void haswell_crtc_enable(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); struct intel_encoder *encoder; - int pipe = intel_crtc->pipe; + int pipe = intel_crtc->pipe, hsw_workaround_pipe; + struct intel_crtc_state *pipe_config = + to_intel_crtc_state(crtc->state); if (WARN_ON(intel_crtc->active)) return; @@ -4976,7 +4949,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) /* If we change the relative order between pipe/planes enabling, we need * to change the workaround. */ - haswell_mode_set_planes_workaround(intel_crtc); + hsw_workaround_pipe = pipe_config->hsw_workaround_pipe; + if (IS_HASWELL(dev) && hsw_workaround_pipe != INVALID_PIPE) { + intel_wait_for_vblank(dev, hsw_workaround_pipe); + intel_wait_for_vblank(dev, hsw_workaround_pipe); + } } static void ironlake_pfit_disable(struct intel_crtc *crtc) @@ -12818,6 +12795,71 @@ static int intel_modeset_setup_plls(struct drm_atomic_state *state) return ret; } +/* + * This implements the workaround described in the "notes" section of the mode + * set sequence documentation. When going from no pipes or single pipe to + * multiple pipes, and planes are enabled after the pipe, we need to wait at + * least 2 vblanks on the first pipe before enabling planes on the second pipe. + */ +static int haswell_mode_set_planes_workaround(struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state; + struct intel_crtc *intel_crtc; + struct drm_crtc *crtc; + struct intel_crtc_state *first_crtc_state = NULL; + struct intel_crtc_state *other_crtc_state = NULL; + enum pipe first_pipe = INVALID_PIPE, enabled_pipe = INVALID_PIPE; + int i; + + /* look at all crtc's that are going to be enabled in during modeset */ + for_each_crtc_in_state(state, crtc, crtc_state, i) { + intel_crtc = to_intel_crtc(crtc); + + if (!crtc_state->active || !needs_modeset(crtc_state)) + continue; + + if (first_crtc_state) { + other_crtc_state = to_intel_crtc_state(crtc_state); + break; + } else { + first_crtc_state = to_intel_crtc_state(crtc_state); + first_pipe = intel_crtc->pipe; + } + } + + /* No workaround needed? */ + if (!first_crtc_state) + return 0; + + /* w/a possibly needed, check how many crtc's are already enabled. */ + for_each_intel_crtc(state->dev, intel_crtc) { + struct intel_crtc_state *pipe_config; + + pipe_config = intel_atomic_get_crtc_state(state, intel_crtc); + if (IS_ERR(pipe_config)) + return PTR_ERR(pipe_config); + + pipe_config->hsw_workaround_pipe = INVALID_PIPE; + + if (!pipe_config->base.active || + needs_modeset(&pipe_config->base)) + continue; + + /* 2 or more enabled crtcs means no need for w/a */ + if (enabled_pipe != INVALID_PIPE) + return 0; + + enabled_pipe = intel_crtc->pipe; + } + + if (enabled_pipe != INVALID_PIPE) + first_crtc_state->hsw_workaround_pipe = enabled_pipe; + else if (other_crtc_state) + other_crtc_state->hsw_workaround_pipe = first_pipe; + + return 0; +} + /* Code that should eventually be part of atomic_check() */ static int intel_modeset_checks(struct drm_atomic_state *state) { @@ -12841,7 +12883,14 @@ static int intel_modeset_checks(struct drm_atomic_state *state) return ret; } - return intel_modeset_setup_plls(state); + ret = intel_modeset_setup_plls(state); + if (ret) + return ret; + + if (IS_HASWELL(dev)) + ret = haswell_mode_set_planes_workaround(state); + + return ret; } static int diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6d9c771747f9..5312160e7c95 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -443,6 +443,9 @@ struct intel_crtc_state { int pbn; struct intel_crtc_scaler_state scaler_state; + + /* w/a for waiting 2 vblanks during crtc enable */ + enum pipe hsw_workaround_pipe; }; struct intel_pipe_wm { From 3538b9dffd8344cd40413018bcd7dc7b2bc1e21d Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:10 +0200 Subject: [PATCH 26/33] drm/i915: Use atomic state for calculating DVO_2X_MODE on i830. This is a small behavioral change because it leaves DVO_2X_MODE set between crtc_disable and crtc_enable. This is probably harmless though and if not should be fixed by calculating 2x mode before enable/disable pll. This is needed because intel_crtc->active will be removed eventually. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cd9a7545b0d3..28fc3efa95ed 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1698,7 +1698,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; @@ -1779,7 +1779,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), From 5c2db1882ab32546c46318d9feb017673c072717 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 1 Jun 2015 12:50:11 +0200 Subject: [PATCH 27/33] drm/i915: use calculated state for vblank evasion crtc->active will be gone eventually, and this check should be just as good. Signed-off-by: Maarten Lankhorst Reviewed-by: Matt Roper Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 28fc3efa95ed..a232dc9f51f6 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13658,6 +13658,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc) struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_crtc_state *crtc_state = intel_crtc->base.state; struct intel_plane *intel_plane; struct drm_plane *p; unsigned fb_bits = 0; @@ -13701,7 +13702,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) + if (crtc_state->active && !needs_modeset(crtc_state)) intel_crtc->atomic.evade = intel_pipe_update_start(intel_crtc, &intel_crtc->atomic.start_vbl_count); From 9716c691ce06b043d3e75c8ff93704cb40c52265 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 10 Jun 2015 10:24:19 +0200 Subject: [PATCH 28/33] Revert "drm/i915: Make intel_display_suspend atomic, v2." 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 Acked-by: Ander Conselvan de Oliveira Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 3 -- drivers/gpu/drm/i915/intel_display.c | 57 +++++++--------------------- drivers/gpu/drm/i915/intel_drv.h | 2 +- 3 files changed, 14 insertions(+), 48 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 a232dc9f51f6..cc7fad9905b9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6191,58 +6191,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); + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + enum intel_display_power_domain domain; + unsigned long domains; - ret = PTR_ERR_OR_ZERO(crtc_state); - if (ret) - goto free; - - if (!crtc_state->active) + if (!intel_crtc->active) continue; - crtc_state->active = false; - crtc_mask |= 1 << drm_crtc_index(crtc); + intel_crtc_disable_planes(crtc); + dev_priv->display.crtc_disable(crtc); + + 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; } - - 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; - - return ret; - } - } - -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 5312160e7c95..9ca683ae2dec 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); From f721790560be9551c9e7f1644e04960b3ac44d06 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 10 Jun 2015 10:24:20 +0200 Subject: [PATCH 29/33] Revert "drm/i915: Read hw state into an atomic state struct, v2." 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 Acked-by: Ander Conselvan de Oliveira Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_atomic.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 389 ++++++++++----------------- drivers/gpu/drm/i915/intel_drv.h | 14 +- 3 files changed, 157 insertions(+), 248 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 cc7fad9905b9..5cc2263db199 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10300,7 +10300,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: @@ -10318,10 +10318,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; @@ -10340,6 +10340,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; @@ -10350,17 +10353,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; @@ -10428,7 +10434,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; @@ -10474,6 +10482,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; @@ -11620,6 +11632,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. @@ -12149,6 +12188,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) { @@ -12458,14 +12498,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"); } } @@ -12485,6 +12522,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"); @@ -12494,9 +12533,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 @@ -13000,11 +13036,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); @@ -13017,10 +13053,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", @@ -13029,6 +13069,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); } @@ -15105,7 +15148,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; @@ -15269,182 +15311,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); - - /* update transitional state */ - crtc->active = crtc_state->base.active; - crtc->config = crtc_state; - - 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 drm_i915_private *dev_priv = dev->dev_private; + enum pipe pipe; struct intel_crtc *crtc; - struct intel_crtc_state *crtc_state; + struct intel_encoder *encoder; + struct intel_connector *connector; int i; - shared_dpll = intel_atomic_get_shared_dpll_state(state); + for_each_intel_crtc(dev, crtc) { + struct drm_plane *primary = crtc->base.primary; + struct intel_plane_state *plane_state; + + memset(crtc->config, 0, sizeof(*crtc->config)); + crtc->config->base.crtc = &crtc->base; + + crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE; + + 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; + + plane_state = to_intel_plane_state(primary->state); + plane_state->visible = primary_get_hw_state(crtc); + + DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n", + crtc->base.base.id, + crtc->active ? "enabled" : "disabled"); + } + 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, @@ -15452,42 +15390,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 @@ -15496,57 +15412,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; + 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); + } } - 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_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]"); } @@ -15570,17 +15466,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 9ca683ae2dec..b28029a1c8f2 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; @@ -1407,8 +1419,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, From b17d48e27d35bc890ff205c9663b58803798b63b Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Fri, 12 Jun 2015 11:15:39 +0200 Subject: [PATCH 30/33] drm/i915: Do not use atomic modesets in hw readout. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should fix fallout caused by making intel_crtc_control and update_dpms atomic, which became a problem after reverting the atomic hw readout patch. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929 Reported-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 75 ++++++++++++---------------- 1 file changed, 32 insertions(+), 43 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5cc2263db199..7abaffeda7ce 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6187,31 +6187,35 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) mutex_unlock(&dev->struct_mutex); } +static void intel_crtc_disable_noatomic(struct drm_crtc *crtc) +{ + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->dev); + enum intel_display_power_domain domain; + unsigned long domains; + + if (!intel_crtc->active) + return; + + intel_crtc_disable_planes(crtc); + dev_priv->display.crtc_disable(crtc); + + 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; +} + /* * turn all crtc's off, but do not adjust state * This has to be paired with a call to intel_modeset_setup_hw_state. */ void intel_display_suspend(struct drm_device *dev) { - struct drm_i915_private *dev_priv = to_i915(dev); struct drm_crtc *crtc; - for_each_crtc(dev, crtc) { - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum intel_display_power_domain domain; - unsigned long domains; - - if (!intel_crtc->active) - continue; - - intel_crtc_disable_planes(crtc); - dev_priv->display.crtc_disable(crtc); - - 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; - } + for_each_crtc(dev, crtc) + intel_crtc_disable_noatomic(crtc); } /* Master function to enable/disable CRTC and corresponding power wells */ @@ -15120,7 +15124,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_encoder *encoder; u32 reg; + bool enable; /* Clear any frame start delays used for debugging left by the BIOS */ reg = PIPECONF(crtc->config->cpu_transcoder); @@ -15137,7 +15143,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) * disable the crtc (and hence change the state) if it is wrong. Note * that gen4+ has a fixed plane -> pipe mapping. */ if (INTEL_INFO(dev)->gen < 4 && !intel_check_plane_mapping(crtc)) { - struct intel_connector *connector; bool plane; DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n", @@ -15149,29 +15154,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) plane = crtc->plane; to_intel_plane_state(crtc->base.primary->state)->visible = true; crtc->plane = !plane; - intel_crtc_control(&crtc->base, false); + intel_crtc_disable_noatomic(&crtc->base); crtc->plane = plane; - - /* ... and break all links. */ - for_each_intel_connector(dev, connector) { - if (connector->encoder->base.crtc != &crtc->base) - continue; - - connector->base.dpms = DRM_MODE_DPMS_OFF; - connector->base.encoder = NULL; - } - /* multiple connectors may have the same encoder: - * handle them and break crtc link separately */ - for_each_intel_connector(dev, connector) - if (connector->encoder->base.crtc == &crtc->base) { - connector->encoder->base.crtc = NULL; - connector->encoder->connectors_active = false; - } - - WARN_ON(crtc->active); - crtc->base.state->enable = false; - crtc->base.state->active = false; - crtc->base.enabled = false; } if (dev_priv->quirks & QUIRK_PIPEA_FORCE && @@ -15185,13 +15169,18 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) /* Adjust the state of the output pipe according to whether we * have active connectors/encoders. */ - intel_crtc_update_dpms(&crtc->base); + enable = false; + for_each_encoder_on_crtc(dev, &crtc->base, encoder) + enable |= encoder->connectors_active; + + if (!enable) + intel_crtc_disable_noatomic(&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 + * functions or because of calls to intel_crtc_disable_noatomic, + * 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, From 02e0efb5b40b42f06668ba38f39654c57feaacdb Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Fri, 12 Jun 2015 11:15:40 +0200 Subject: [PATCH 31/33] drm/i915: get rid of intel_plane_restore in intel_crtc_page_flip Use a full atomic call instead. intel_crtc_page_flip will still have to live until async updates are allowed. This doesn't seem to be a regression from the convert to atomic, part 3 patch. During GPU reset it fixes the following warning: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 752 at drivers/gpu/drm/drm_crtc.c:5337 drm_mode_page_flip_ioctl+0x27b/0x360() Modules linked in: i915 CPU: 0 PID: 752 Comm: Xorg Not tainted 4.1.0-rc7-patser+ #4090 Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015 ffffffff81c90866 ffff8800d87c3ca8 ffffffff817f7d87 0000000080000001 0000000000000000 ffff8800d87c3ce8 ffffffff81084955 ffff880000000000 ffff8800d87c3dc0 ffff8800d93d1208 0000000000000000 ffff8800b7d1f3e0 Call Trace: [] dump_stack+0x4f/0x7b [] warn_slowpath_common+0x85/0xc0 [] warn_slowpath_null+0x15/0x20 [] drm_mode_page_flip_ioctl+0x27b/0x360 [] drm_ioctl+0x1a0/0x6a0 [] ? get_parent_ip+0x11/0x50 [] ? avc_has_perm+0x20/0x280 [] ? get_parent_ip+0x11/0x50 [] do_vfs_ioctl+0x2f8/0x530 [] ? expand_files+0x261/0x270 [] ? selinux_file_ioctl+0x56/0x100 [] SyS_ioctl+0x81/0xa0 [] system_call_fastpath+0x12/0x6f ---[ end trace 9ce834560085bd64 ]--- Signed-off-by: Maarten Lankhorst Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 29 +++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 7abaffeda7ce..cdf6549c8e74 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11618,8 +11618,35 @@ free_work: kfree(work); if (ret == -EIO) { + struct drm_atomic_state *state; + struct drm_plane_state *plane_state; + out_hang: - ret = intel_plane_restore(primary); + state = drm_atomic_state_alloc(dev); + if (!state) + return -ENOMEM; + state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc); + +retry: + plane_state = drm_atomic_get_plane_state(state, primary); + ret = PTR_ERR_OR_ZERO(plane_state); + if (!ret) { + drm_atomic_set_fb_for_plane(plane_state, fb); + + ret = drm_atomic_set_crtc_for_plane(plane_state, crtc); + if (!ret) + ret = drm_atomic_commit(state); + } + + if (ret == -EDEADLK) { + drm_modeset_backoff(state->acquire_ctx); + drm_atomic_state_clear(state); + goto retry; + } + + if (ret) + drm_atomic_state_free(state); + if (ret == 0 && event) { spin_lock_irq(&dev->event_lock); drm_send_vblank_event(dev, pipe, event); From b8b7fadec3c797d7babb3c7fec484971e1604978 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Fri, 12 Jun 2015 11:15:41 +0200 Subject: [PATCH 32/33] drm/i915: Set hwmode during readout. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was introduced after converting hw readout to atomic, so it should have been part of the revert too. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90929 Reported-by: Ville Syrjälä Tested-by: Ville Syrjälä Signed-off-by: Maarten Lankhorst Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cdf6549c8e74..14ccf49b9067 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15357,6 +15357,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) crtc->base.state->enable = crtc->active; crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.hwmode = crtc->config->base.adjusted_mode; plane_state = to_intel_plane_state(primary->state); plane_state->visible = primary_get_hw_state(crtc); From c0165304e10f317672e20f2b40770d74c51e287f Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Fri, 12 Jun 2015 11:15:42 +0200 Subject: [PATCH 33/33] drm/i915: Only enable cursor if it can be enabled. The cursor should only be enabled if it's visible. This fixes igt/kms_cursor_crc, which may otherwise produce the following warning: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 3425 at drivers/gpu/drm/i915/intel_display.c:9995 intel_crtc_update_cursor+0x14c/0x4d0 [i915]() Missing switch case (0) in i9xx_update_cursor Modules linked in: i915 CPU: 0 PID: 3425 Comm: kms_cursor_crc Tainted: G W 4.1.0-rc7-patser+ #4079 Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 ) 04/15/2014 ffffffffc01aad10 ffff8800b083faa8 ffffffff817f7827 0000000080000001 ffff8800b083faf8 ffff8800b083fae8 ffffffff81084955 ffff8800b083fad8 ffff8800c4931148 0000000001200000 ffff8800c48b0000 0000000000000000 Call Trace: [] dump_stack+0x4f/0x7b [] warn_slowpath_common+0x85/0xc0 [] warn_slowpath_fmt+0x41/0x50 [] intel_crtc_update_cursor+0x14c/0x4d0 [i915] [] __intel_set_mode+0x6c4/0x750 [i915] [] intel_crtc_set_config+0x473/0x5c0 [i915] [] drm_mode_set_config_internal+0x69/0x120 [] drm_mode_setcrtc+0x189/0x540 [] drm_ioctl+0x1a0/0x6a0 [] ? get_parent_ip+0x11/0x50 [] do_vfs_ioctl+0x2f8/0x530 [] ? trace_hardirqs_on+0xd/0x10 [] ? selinux_file_ioctl+0x56/0x100 [] SyS_ioctl+0x81/0xa0 [] system_call_fastpath+0x12/0x6f ---[ end trace abf0f71163290a96 ]--- Signed-off-by: Maarten Lankhorst Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_display.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 14ccf49b9067..afe91a8f7e36 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4748,7 +4748,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) intel_enable_primary_hw_plane(crtc->primary, crtc); intel_enable_sprite_planes(crtc); - intel_crtc_update_cursor(crtc, true); + if (to_intel_plane_state(crtc->cursor->state)->visible) + intel_crtc_update_cursor(crtc, true); intel_post_enable_primary(crtc);