| Message ID | 1431354318-11995-3-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 11, 2015 at 04:24:38PM +0200, Maarten Lankhorst wrote: > Now that the dpll updates are (mostly) atomic, the .off() code is no longer used, > and there are no more callers for intel_put_shared_dpll. Move all the updates > done in intel_crtc_disable to intel_modeset_update_state, one less special case > to worry about. The commit message is a bit innaccurate: We do still have callers of put_shared_dpll, but that work is superseeded by intel_shared_dpll_start_config. Hopefully. Because of that I think this should be two patches: First one to nuke put_shared_dpll, second one to inline crtc_disable. One comment below. > > Changes since v1: > - Move drm_atomic_helper_swap_state up. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_display.c | 106 +++++++---------------------------- > drivers/gpu/drm/i915/intel_drv.h | 1 - > 3 files changed, 19 insertions(+), 89 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 6b3142a99171..a6d3ab94ec15 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -574,7 +574,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 c5543f0bcda9..af96d686aae2 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4228,27 +4228,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); > - } We're dropping the edge-detection warnings here. But I think the code in check_shared_dpll_state should be paranoid enough to keep us covered. Worth mentioning in the commit message though. -Daniel > - > - 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) > { > @@ -5224,13 +5203,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; > @@ -6062,10 +6034,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) > { > @@ -6115,34 +6083,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); > @@ -11614,26 +11554,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) > - break; > - > - if (crtc != intel_encoder->base.crtc) > + crtc = intel_encoder->base.crtc; > + crtc_state = state->crtc_states[drm_crtc_index(crtc)]; > + if (!crtc_state || !needs_modeset(crtc->state)) > continue; > > - if (crtc_state->enable && needs_modeset(crtc_state)) > - intel_encoder->connectors_active = false; > + intel_encoder->connectors_active = false; > } > > - drm_atomic_helper_swap_state(state->dev, state); > intel_modeset_fixup_state(state); > > /* Double check state. */ > @@ -11645,14 +11581,12 @@ 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) > - break; > - > - if (crtc != connector->encoder->crtc) > + crtc = connector->encoder->crtc; > + crtc_state = state->crtc_states[drm_crtc_index(crtc)]; > + if (!crtc_state || !needs_modeset(crtc->state)) > continue; > > - if (crtc->state->enable && needs_modeset(crtc->state)) { > + if (crtc->state->enable) { > struct drm_property *dpms_property = > dev->mode_config.dpms_property; > > @@ -11663,7 +11597,8 @@ intel_modeset_update_state(struct drm_atomic_state *state) > > intel_encoder = to_intel_encoder(connector->encoder); > intel_encoder->connectors_active = true; > - } > + } else > + connector->dpms = DRM_MODE_DPMS_OFF; > } > > } > @@ -12214,6 +12149,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; > > @@ -12341,12 +12280,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 > @@ -14064,7 +14001,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 = ironlake_crtc_off; > dev_priv->display.update_primary_plane = > skylake_update_primary_plane; > } else if (HAS_DDI(dev)) { > @@ -14075,7 +14011,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 = ironlake_crtc_off; > dev_priv->display.update_primary_plane = > ironlake_update_primary_plane; > } else if (HAS_PCH_SPLIT(dev)) { > @@ -14086,7 +14021,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 = ironlake_crtc_off; > dev_priv->display.update_primary_plane = > ironlake_update_primary_plane; > } else if (IS_VALLEYVIEW(dev)) { > @@ -14096,7 +14030,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 { > @@ -14106,7 +14039,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; > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 29262228f264..4c690403942c 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1082,7 +1082,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); > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 6b3142a99171..a6d3ab94ec15 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -574,7 +574,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 c5543f0bcda9..af96d686aae2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4228,27 +4228,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) { @@ -5224,13 +5203,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; @@ -6062,10 +6034,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) { @@ -6115,34 +6083,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); @@ -11614,26 +11554,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) - break; - - if (crtc != intel_encoder->base.crtc) + crtc = intel_encoder->base.crtc; + crtc_state = state->crtc_states[drm_crtc_index(crtc)]; + if (!crtc_state || !needs_modeset(crtc->state)) continue; - if (crtc_state->enable && needs_modeset(crtc_state)) - intel_encoder->connectors_active = false; + intel_encoder->connectors_active = false; } - drm_atomic_helper_swap_state(state->dev, state); intel_modeset_fixup_state(state); /* Double check state. */ @@ -11645,14 +11581,12 @@ 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) - break; - - if (crtc != connector->encoder->crtc) + crtc = connector->encoder->crtc; + crtc_state = state->crtc_states[drm_crtc_index(crtc)]; + if (!crtc_state || !needs_modeset(crtc->state)) continue; - if (crtc->state->enable && needs_modeset(crtc->state)) { + if (crtc->state->enable) { struct drm_property *dpms_property = dev->mode_config.dpms_property; @@ -11663,7 +11597,8 @@ intel_modeset_update_state(struct drm_atomic_state *state) intel_encoder = to_intel_encoder(connector->encoder); intel_encoder->connectors_active = true; - } + } else + connector->dpms = DRM_MODE_DPMS_OFF; } } @@ -12214,6 +12149,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; @@ -12341,12 +12280,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 @@ -14064,7 +14001,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 = ironlake_crtc_off; dev_priv->display.update_primary_plane = skylake_update_primary_plane; } else if (HAS_DDI(dev)) { @@ -14075,7 +14011,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 = ironlake_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (HAS_PCH_SPLIT(dev)) { @@ -14086,7 +14021,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 = ironlake_crtc_off; dev_priv->display.update_primary_plane = ironlake_update_primary_plane; } else if (IS_VALLEYVIEW(dev)) { @@ -14096,7 +14030,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 { @@ -14106,7 +14039,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; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 29262228f264..4c690403942c 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1082,7 +1082,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);
Now that the dpll updates are (mostly) atomic, the .off() code is no longer used, and there are no more callers for intel_put_shared_dpll. Move all the updates done in intel_crtc_disable to intel_modeset_update_state, one less special case to worry about. Changes since v1: - Move drm_atomic_helper_swap_state up. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 - drivers/gpu/drm/i915/intel_display.c | 106 +++++++---------------------------- drivers/gpu/drm/i915/intel_drv.h | 1 - 3 files changed, 19 insertions(+), 89 deletions(-)