[Intel-gfx] [PATCH RFC 1/5] drm/i915: Get rid of intel_crtc_disable and related code.
Ander Conselvan De Oliveira
conselvan2 at gmail.com
Fri Apr 24 01:46:46 PDT 2015
On Wed, 2015-04-22 at 13:24 +0200, maarten.lankhorst at linux.intel.com
wrote:
> From: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>
> 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.
This is a bit confusing since until this patch the .off() code is
actually called from intel_crtc_disable, which calls
intel_put_shared_dpll(). I think what you intended to say here is that
intel_shared_dpll_commit() has the same effect as put_shared_dpll(), so
it is safe to remove that call, at which point the .off() hook is not
necessary anymore.
> Move all the updates
> done in intel_crtc_disable to intel_modeset_update_state, one less special case
> to worry about.
Maybe split this to a separate patch?
Ander
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 1 -
> drivers/gpu/drm/i915/intel_display.c | 100 +++++------------------------------
> drivers/gpu/drm/i915/intel_drv.h | 1 -
> 3 files changed, 14 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 61b756bdbaad..9e62926e71f0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -572,7 +572,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 35fde239c200..92d54dd30d7e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4123,27 +4123,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)
> {
> @@ -5089,13 +5068,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;
> @@ -5722,10 +5694,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)
> {
> @@ -5775,34 +5743,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);
> @@ -11219,7 +11159,6 @@ 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);
>
> @@ -11227,15 +11166,12 @@ intel_modeset_update_state(struct drm_atomic_state *state)
> 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);
> @@ -11250,14 +11186,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;
>
> @@ -11268,7 +11202,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;
> }
>
> }
> @@ -11946,12 +11881,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
> @@ -13615,7 +13548,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)) {
> @@ -13626,7 +13558,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)) {
> @@ -13637,7 +13568,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)) {
> @@ -13647,7 +13577,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 {
> @@ -13657,7 +13586,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 70b70e9be167..fb89f5f3498c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1078,7 +1078,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);
More information about the Intel-gfx
mailing list