diff mbox

[2/4] drm/i915: Make *_crtc_mode_set work on new_config

Message ID 1412782343-28836-3-git-send-email-conselvan2@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ander Conselvan de Oliveira Oct. 8, 2014, 3:32 p.m. UTC
From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

This shouldn't change the behavior of those functions, since they are
called after the new_config is made effective and that points to the
current config. In a follow up patch, the mode set sequence will be
changed so this is called before disabling crtcs, and in that case
those functions should work on the staged config.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     |   4 +-
 drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++++--------------
 2 files changed, 101 insertions(+), 70 deletions(-)

Comments

Daniel Vetter Oct. 9, 2014, 8:28 a.m. UTC | #1
On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> This shouldn't change the behavior of those functions, since they are
> called after the new_config is made effective and that points to the
> current config. In a follow up patch, the mode set sequence will be
> changed so this is called before disabling crtcs, and in that case
> those functions should work on the staged config.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

[snip]

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3a06c3d..9d8fe8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
>   return false;
>  }
>
> +/**
> + * Returns whether any output on the specified pipe will have the specified
> + * type after a staged modeset is complete, i.e., the same as
> + * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> + * encoder->crtc.
> + */
> +static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)

s/drm_crtc/intel_crtc/ - general rule in the driver is to use the most
specific type to avoid massive amounts of upcasting everywhere.
> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_encoder *encoder;
> +
> + for_each_intel_encoder(dev, encoder)
> + if (encoder->new_crtc == to_intel_crtc(crtc) &&
> +    encoder->type == type)
> + return true;
> +
> + return false;
> +}

Just aside: With atomic we'll have the list of connectors in a table in
the atomic state structures (and no longer with pointers in the objects
itself). So this function might gain a few more parameters, but we can do
that later on easily.
-Daniel
Daniel Vetter Oct. 9, 2014, 9:11 a.m. UTC | #2
On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> This shouldn't change the behavior of those functions, since they are
> called after the new_config is made effective and that points to the
> current config. In a follow up patch, the mode set sequence will be
> changed so this is called before disabling crtcs, and in that case
> those functions should work on the staged config.
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c     |   4 +-
>  drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++++--------------
>  2 files changed, 101 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cb5367c..619723d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -792,7 +792,7 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
>        WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
>        WRPLL_DIVIDER_POST(p);
>
> - intel_crtc->config.dpll_hw_state.wrpll = val;
> + intel_crtc->new_config->dpll_hw_state.wrpll = val;
>
>   pll = intel_get_shared_dpll(intel_crtc);
>   if (pll == NULL) {
> @@ -801,7 +801,7 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
>   return false;
>   }
>
> - intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
> + intel_crtc->new_config->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
>   }
>
>   return true;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3a06c3d..9d8fe8d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
>   return false;
>  }
>
> +/**
> + * Returns whether any output on the specified pipe will have the specified
> + * type after a staged modeset is complete, i.e., the same as
> + * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> + * encoder->crtc.
> + */
> +static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)

s/drm_crtc/intel_crtc/. In general we use the most specific type
for internal functions to avoid tons of upcasting. So you might want to
throw a patch on top to convert the existing has_type for consistency.
-Daniel

> +{
> + struct drm_device *dev = crtc->dev;
> + struct intel_encoder *encoder;
> +
> + for_each_intel_encoder(dev, encoder)
> + if (encoder->new_crtc == to_intel_crtc(crtc) &&
> +    encoder->type == type)
> + return true;
> +
> + return false;
> +}
> +
>  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>   int refclk)
>  {
>   struct drm_device *dev = crtc->dev;
>   const intel_limit_t *limit;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
>   if (intel_is_dual_link_lvds(dev)) {
>   if (refclk == 100000)
>   limit = &intel_limits_ironlake_dual_lvds_100m;
> @@ -449,15 +468,15 @@ static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
>   struct drm_device *dev = crtc->dev;
>   const intel_limit_t *limit;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
>   if (intel_is_dual_link_lvds(dev))
>   limit = &intel_limits_g4x_dual_channel_lvds;
>   else
>   limit = &intel_limits_g4x_single_channel_lvds;
> - } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI) ||
> -   intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG)) {
> + } else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
> +   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
>   limit = &intel_limits_g4x_hdmi;
> - } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
> + } else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
>   limit = &intel_limits_g4x_sdvo;
>   } else /* The option is for other outputs */
>   limit = &intel_limits_i9xx_sdvo;
> @@ -475,7 +494,7 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
>   else if (IS_G4X(dev)) {
>   limit = intel_g4x_limit(crtc);
>   } else if (IS_PINEVIEW(dev)) {
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
>   limit = &intel_limits_pineview_lvds;
>   else
>   limit = &intel_limits_pineview_sdvo;
> @@ -484,14 +503,14 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
>   } else if (IS_VALLEYVIEW(dev)) {
>   limit = &intel_limits_vlv;
>   } else if (!IS_GEN2(dev)) {
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
>   limit = &intel_limits_i9xx_lvds;
>   else
>   limit = &intel_limits_i9xx_sdvo;
>   } else {
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
>   limit = &intel_limits_i8xx_lvds;
> - else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO))
> + else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
>   limit = &intel_limits_i8xx_dvo;
>   else
>   limit = &intel_limits_i8xx_dac;
> @@ -586,7 +605,7 @@ i9xx_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>   intel_clock_t clock;
>   int err = target;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
>   /*
>   * For LVDS just rely on its current settings for dual-channel.
>   * We haven't figured out how to reliably set up different
> @@ -647,7 +666,7 @@ pnv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>   intel_clock_t clock;
>   int err = target;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
>   /*
>   * For LVDS just rely on its current settings for dual-channel.
>   * We haven't figured out how to reliably set up different
> @@ -710,7 +729,7 @@ g4x_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>   int err_most = (target >> 8) + (target >> 9);
>   found = false;
>
> - if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
>   if (intel_is_dual_link_lvds(dev))
>   clock.p2 = limit->p2.p2_fast;
>   else
> @@ -5607,7 +5626,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>
>   if (IS_VALLEYVIEW(dev)) {
>   refclk = 100000;
> - } else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> + } else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
>      intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
>   refclk = dev_priv->vbt.lvds_ssc_freq;
>   DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> @@ -5758,11 +5777,11 @@ static void vlv_update_pll(struct intel_crtc *crtc)
>   if (crtc->pipe == PIPE_B)
>   dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
>   dpll |= DPLL_VCO_ENABLE;
> - crtc->config.dpll_hw_state.dpll = dpll;
> + crtc->new_config->dpll_hw_state.dpll = dpll;
>
> - dpll_md = (crtc->config.pixel_multiplier - 1)
> + dpll_md = (crtc->new_config->pixel_multiplier - 1)
>   << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> - crtc->config.dpll_hw_state.dpll_md = dpll_md;
> + crtc->new_config->dpll_hw_state.dpll_md = dpll_md;
>  }
>
>  static void vlv_prepare_pll(struct intel_crtc *crtc)
> @@ -5858,14 +5877,16 @@ static void vlv_prepare_pll(struct intel_crtc *crtc)
>
>  static void chv_update_pll(struct intel_crtc *crtc)
>  {
> - crtc->config.dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
> + crtc->new_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
>   DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
>   DPLL_VCO_ENABLE;
>   if (crtc->pipe != PIPE_A)
> - crtc->config.dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
> + crtc->new_config->dpll_hw_state.dpll |=
> + DPLL_INTEGRATED_CRI_CLK_VLV;
>
> - crtc->config.dpll_hw_state.dpll_md =
> - (crtc->config.pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> + crtc->new_config->dpll_hw_state.dpll_md =
> + (crtc->new_config->pixel_multiplier - 1)
> + << DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  }
>
>  static void chv_prepare_pll(struct intel_crtc *crtc)
> @@ -5946,29 +5967,29 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   u32 dpll;
>   bool is_sdvo;
> - struct dpll *clock = &crtc->config.dpll;
> + struct dpll *clock = &crtc->new_config->dpll;
>
>   i9xx_update_pll_dividers(crtc, reduced_clock);
>
> - is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> - intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
> + is_sdvo = intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> + intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_HDMI);
>
>   dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
> + if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS))
>   dpll |= DPLLB_MODE_LVDS;
>   else
>   dpll |= DPLLB_MODE_DAC_SERIAL;
>
>   if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) {
> - dpll |= (crtc->config.pixel_multiplier - 1)
> + dpll |= (crtc->new_config->pixel_multiplier - 1)
>   << SDVO_MULTIPLIER_SHIFT_HIRES;
>   }
>
>   if (is_sdvo)
>   dpll |= DPLL_SDVO_HIGH_SPEED;
>
> - if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
> + if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
>   dpll |= DPLL_SDVO_HIGH_SPEED;
>
>   /* compute bitmask from p1 value */
> @@ -5996,21 +6017,21 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
>   if (INTEL_INFO(dev)->gen >= 4)
>   dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>
> - if (crtc->config.sdvo_tv_clock)
> + if (crtc->new_config->sdvo_tv_clock)
>   dpll |= PLL_REF_INPUT_TVCLKINBC;
> - else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> + else if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>   intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>   dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>   else
>   dpll |= PLL_REF_INPUT_DREFCLK;
>
>   dpll |= DPLL_VCO_ENABLE;
> - crtc->config.dpll_hw_state.dpll = dpll;
> + crtc->new_config->dpll_hw_state.dpll = dpll;
>
>   if (INTEL_INFO(dev)->gen >= 4) {
> - u32 dpll_md = (crtc->config.pixel_multiplier - 1)
> + u32 dpll_md = (crtc->new_config->pixel_multiplier - 1)
>   << DPLL_MD_UDI_MULTIPLIER_SHIFT;
> - crtc->config.dpll_hw_state.dpll_md = dpll_md;
> + crtc->new_config->dpll_hw_state.dpll_md = dpll_md;
>   }
>  }
>
> @@ -6021,13 +6042,13 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>   struct drm_device *dev = crtc->base.dev;
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   u32 dpll;
> - struct dpll *clock = &crtc->config.dpll;
> + struct dpll *clock = &crtc->new_config->dpll;
>
>   i9xx_update_pll_dividers(crtc, reduced_clock);
>
>   dpll = DPLL_VGA_MODE_DIS;
>
> - if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
> + if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
>   dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>   } else {
>   if (clock->p1 == 2)
> @@ -6038,17 +6059,18 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
>   dpll |= PLL_P2_DIVIDE_BY_4;
>   }
>
> - if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
> + if (!IS_I830(dev) &&
> +    intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_DVO))
>   dpll |= DPLL_DVO_2X_MODE;
>
> - if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
> + if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>   intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>   dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>   else
>   dpll |= PLL_REF_INPUT_DREFCLK;
>
>   dpll |= DPLL_VCO_ENABLE;
> - crtc->config.dpll_hw_state.dpll = dpll;
> + crtc->new_config->dpll_hw_state.dpll = dpll;
>  }
>
>  static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> @@ -6258,7 +6280,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>   struct intel_encoder *encoder;
>   const intel_limit_t *limit;
>
> - for_each_encoder_on_crtc(dev, crtc, encoder) {
> + for_each_intel_encoder(dev, encoder) {
> + if (encoder->new_crtc != to_intel_crtc(crtc))
> + continue;
> +
>   switch (encoder->type) {
>   case INTEL_OUTPUT_LVDS:
>   is_lvds = true;
> @@ -6274,7 +6299,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>   if (is_dsi)
>   return 0;
>
> - if (!intel_crtc->config.clock_set) {
> + if (!intel_crtc->new_config->clock_set) {
>   refclk = i9xx_get_refclk(crtc, num_connectors);
>
>   /*
> @@ -6285,7 +6310,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>   */
>   limit = intel_limit(crtc, refclk);
>   ok = dev_priv->display.find_dpll(limit, crtc,
> - intel_crtc->config.port_clock,
> + intel_crtc->new_config->port_clock,
>   refclk, NULL, &clock);
>   if (!ok) {
>   DRM_ERROR("Couldn't find PLL settings for mode!\n");
> @@ -6306,11 +6331,11 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>      &reduced_clock);
>   }
>   /* Compat-code for transition, will disappear. */
> - intel_crtc->config.dpll.n = clock.n;
> - intel_crtc->config.dpll.m1 = clock.m1;
> - intel_crtc->config.dpll.m2 = clock.m2;
> - intel_crtc->config.dpll.p1 = clock.p1;
> - intel_crtc->config.dpll.p2 = clock.p2;
> + intel_crtc->new_config->dpll.n = clock.n;
> + intel_crtc->new_config->dpll.m1 = clock.m1;
> + intel_crtc->new_config->dpll.m2 = clock.m2;
> + intel_crtc->new_config->dpll.p1 = clock.p1;
> + intel_crtc->new_config->dpll.p2 = clock.p2;
>   }
>
>   if (IS_GEN2(dev)) {
> @@ -6926,7 +6951,10 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
>   int num_connectors = 0;
>   bool is_lvds = false;
>
> - for_each_encoder_on_crtc(dev, crtc, encoder) {
> + for_each_intel_encoder(dev, encoder) {
> + if (encoder->new_crtc != to_intel_crtc(crtc))
> + continue;
> +
>   switch (encoder->type) {
>   case INTEL_OUTPUT_LVDS:
>   is_lvds = true;
> @@ -7114,7 +7142,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>   const intel_limit_t *limit;
>   bool ret, is_lvds = false;
>
> - is_lvds = intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS);
> + is_lvds = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS);
>
>   refclk = ironlake_get_refclk(crtc);
>
> @@ -7125,7 +7153,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>   */
>   limit = intel_limit(crtc, refclk);
>   ret = dev_priv->display.find_dpll(limit, crtc,
> -  to_intel_crtc(crtc)->config.port_clock,
> +  to_intel_crtc(crtc)->new_config->port_clock,
>    refclk, NULL, clock);
>   if (!ret)
>   return false;
> @@ -7175,7 +7203,10 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>   int factor, num_connectors = 0;
>   bool is_lvds = false, is_sdvo = false;
>
> - for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> + for_each_intel_encoder(dev, intel_encoder) {
> + if (intel_encoder->new_crtc != to_intel_crtc(crtc))
> + continue;
> +
>   switch (intel_encoder->type) {
>   case INTEL_OUTPUT_LVDS:
>   is_lvds = true;
> @@ -7196,10 +7227,10 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>       dev_priv->vbt.lvds_ssc_freq == 100000) ||
>      (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
>   factor = 25;
> - } else if (intel_crtc->config.sdvo_tv_clock)
> + } else if (intel_crtc->new_config->sdvo_tv_clock)
>   factor = 20;
>
> - if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor))
> + if (ironlake_needs_fb_cb_tune(&intel_crtc->new_config->dpll, factor))
>   *fp |= FP_CB_TUNE;
>
>   if (fp2 && (reduced_clock->m < factor * reduced_clock->n))
> @@ -7212,20 +7243,20 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>   else
>   dpll |= DPLLB_MODE_DAC_SERIAL;
>
> - dpll |= (intel_crtc->config.pixel_multiplier - 1)
> + dpll |= (intel_crtc->new_config->pixel_multiplier - 1)
>   << PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT;
>
>   if (is_sdvo)
>   dpll |= DPLL_SDVO_HIGH_SPEED;
> - if (intel_crtc->config.has_dp_encoder)
> + if (intel_crtc->new_config->has_dp_encoder)
>   dpll |= DPLL_SDVO_HIGH_SPEED;
>
>   /* compute bitmask from p1 value */
> - dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
> + dpll |= (1 << (intel_crtc->new_config->dpll.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>   /* also FPA1 */
> - dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT;
> + dpll |= (1 << (intel_crtc->new_config->dpll.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT;
>
> - switch (intel_crtc->config.dpll.p2) {
> + switch (intel_crtc->new_config->dpll.p2) {
>   case 5:
>   dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_5;
>   break;
> @@ -7267,22 +7298,22 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
>   ok = ironlake_compute_clocks(crtc, &clock,
>       &has_reduced_clock, &reduced_clock);
> - if (!ok && !intel_crtc->config.clock_set) {
> + if (!ok && !intel_crtc->new_config->clock_set) {
>   DRM_ERROR("Couldn't find PLL settings for mode!\n");
>   return -EINVAL;
>   }
>   /* Compat-code for transition, will disappear. */
> - if (!intel_crtc->config.clock_set) {
> - intel_crtc->config.dpll.n = clock.n;
> - intel_crtc->config.dpll.m1 = clock.m1;
> - intel_crtc->config.dpll.m2 = clock.m2;
> - intel_crtc->config.dpll.p1 = clock.p1;
> - intel_crtc->config.dpll.p2 = clock.p2;
> + if (!intel_crtc->new_config->clock_set) {
> + intel_crtc->new_config->dpll.n = clock.n;
> + intel_crtc->new_config->dpll.m1 = clock.m1;
> + intel_crtc->new_config->dpll.m2 = clock.m2;
> + intel_crtc->new_config->dpll.p1 = clock.p1;
> + intel_crtc->new_config->dpll.p2 = clock.p2;
>   }
>
>   /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> - if (intel_crtc->config.has_pch_encoder) {
> - fp = i9xx_dpll_compute_fp(&intel_crtc->config.dpll);
> + if (intel_crtc->new_config->has_pch_encoder) {
> + fp = i9xx_dpll_compute_fp(&intel_crtc->new_config->dpll);
>   if (has_reduced_clock)
>   fp2 = i9xx_dpll_compute_fp(&reduced_clock);
>
> @@ -7290,12 +7321,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>       &fp, &reduced_clock,
>       has_reduced_clock ? &fp2 : NULL);
>
> - intel_crtc->config.dpll_hw_state.dpll = dpll;
> - intel_crtc->config.dpll_hw_state.fp0 = fp;
> + intel_crtc->new_config->dpll_hw_state.dpll = dpll;
> + intel_crtc->new_config->dpll_hw_state.fp0 = fp;
>   if (has_reduced_clock)
> - intel_crtc->config.dpll_hw_state.fp1 = fp2;
> + intel_crtc->new_config->dpll_hw_state.fp1 = fp2;
>   else
> - intel_crtc->config.dpll_hw_state.fp1 = fp;
> + intel_crtc->new_config->dpll_hw_state.fp1 = fp;
>
>   pll = intel_get_shared_dpll(intel_crtc);
>   if (pll == NULL) {
> --
> 1.8.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ander Conselvan de Oliveira Oct. 9, 2014, 12:18 p.m. UTC | #3
On 10/09/2014 12:11 PM, Daniel Vetter wrote:
> On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>
>> This shouldn't change the behavior of those functions, since they are
>> called after the new_config is made effective and that points to the
>> current config. In a follow up patch, the mode set sequence will be
>> changed so this is called before disabling crtcs, and in that case
>> those functions should work on the staged config.
>>
>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>> ---

[snip]

>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3a06c3d..9d8fe8d 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
>>    return false;
>>   }
>>
>> +/**
>> + * Returns whether any output on the specified pipe will have the specified
>> + * type after a staged modeset is complete, i.e., the same as
>> + * intel_pipe_has_type() but looking at encoder->new_crtc instead of
>> + * encoder->crtc.
>> + */
>> +static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)
>
> s/drm_crtc/intel_crtc/. In general we use the most specific type
> for internal functions to avoid tons of upcasting. So you might want to
> throw a patch on top to convert the existing has_type for consistency.

What about the functions in drm_i915_display_funcs (find_dpll, 
crtc_mode_set)? Are these not considered internal functions or is this 
leftover from before this rule was in place?

If I write that consistency patch it might be easier to just convert 
everything.

Ander
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
Daniel Vetter Oct. 19, 2014, 2:28 p.m. UTC | #4
On Thu, Oct 09, 2014 at 03:18:03PM +0300, Ander Conselvan de Oliveira wrote:
> On 10/09/2014 12:11 PM, Daniel Vetter wrote:
> >On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
> >>From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >>
> >>This shouldn't change the behavior of those functions, since they are
> >>called after the new_config is made effective and that points to the
> >>current config. In a follow up patch, the mode set sequence will be
> >>changed so this is called before disabling crtcs, and in that case
> >>those functions should work on the staged config.
> >>
> >>Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> >>---
> 
> [snip]
> 
> >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>index 3a06c3d..9d8fe8d 100644
> >>--- a/drivers/gpu/drm/i915/intel_display.c
> >>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
> >>   return false;
> >>  }
> >>
> >>+/**
> >>+ * Returns whether any output on the specified pipe will have the specified
> >>+ * type after a staged modeset is complete, i.e., the same as
> >>+ * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> >>+ * encoder->crtc.
> >>+ */
> >>+static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)
> >
> >s/drm_crtc/intel_crtc/. In general we use the most specific type
> >for internal functions to avoid tons of upcasting. So you might want to
> >throw a patch on top to convert the existing has_type for consistency.
> 
> What about the functions in drm_i915_display_funcs (find_dpll,
> crtc_mode_set)? Are these not considered internal functions or is this
> leftover from before this rule was in place?
> 
> If I write that consistency patch it might be easier to just convert
> everything.

leftovers, and we've been slowly converting existing code over to using
intel structures over the past 2 years. I certainly won't stop you to
clean up all that stuff, but it's a bit of work. So I'm ok if you just
have the vfunc signature use the intel_ types and then immediately convert
to a drm_ type again to avoid code churn.

Or just convert it all if you feel like it.
-Daniel
Daniel Vetter Oct. 19, 2014, 2:30 p.m. UTC | #5
On Sun, Oct 19, 2014 at 04:28:57PM +0200, Daniel Vetter wrote:
> On Thu, Oct 09, 2014 at 03:18:03PM +0300, Ander Conselvan de Oliveira wrote:
> > On 10/09/2014 12:11 PM, Daniel Vetter wrote:
> > >On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
> > >>From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > >>
> > >>This shouldn't change the behavior of those functions, since they are
> > >>called after the new_config is made effective and that points to the
> > >>current config. In a follow up patch, the mode set sequence will be
> > >>changed so this is called before disabling crtcs, and in that case
> > >>those functions should work on the staged config.
> > >>
> > >>Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > >>---
> > 
> > [snip]
> > 
> > >>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >>index 3a06c3d..9d8fe8d 100644
> > >>--- a/drivers/gpu/drm/i915/intel_display.c
> > >>+++ b/drivers/gpu/drm/i915/intel_display.c
> > >>@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
> > >>   return false;
> > >>  }
> > >>
> > >>+/**
> > >>+ * Returns whether any output on the specified pipe will have the specified
> > >>+ * type after a staged modeset is complete, i.e., the same as
> > >>+ * intel_pipe_has_type() but looking at encoder->new_crtc instead of
> > >>+ * encoder->crtc.
> > >>+ */
> > >>+static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)
> > >
> > >s/drm_crtc/intel_crtc/. In general we use the most specific type
> > >for internal functions to avoid tons of upcasting. So you might want to
> > >throw a patch on top to convert the existing has_type for consistency.
> > 
> > What about the functions in drm_i915_display_funcs (find_dpll,
> > crtc_mode_set)? Are these not considered internal functions or is this
> > leftover from before this rule was in place?
> > 
> > If I write that consistency patch it might be easier to just convert
> > everything.
> 
> leftovers, and we've been slowly converting existing code over to using
> intel structures over the past 2 years. I certainly won't stop you to
> clean up all that stuff, but it's a bit of work. So I'm ok if you just
> have the vfunc signature use the intel_ types and then immediately convert
> to a drm_ type again to avoid code churn.
> 
> Or just convert it all if you feel like it.

But if you do that please split out that rote conversion into a separate
patch for easier reviewing. Either at the start of the series (so I can
merge it right away) or at the end (only done once everything is merged)
to avoid needless rebase pain.
-Daniel
Ander Conselvan de Oliveira Oct. 20, 2014, 10:49 a.m. UTC | #6
On 10/19/2014 05:30 PM, Daniel Vetter wrote:
> On Sun, Oct 19, 2014 at 04:28:57PM +0200, Daniel Vetter wrote:
>> On Thu, Oct 09, 2014 at 03:18:03PM +0300, Ander Conselvan de Oliveira wrote:
>>> On 10/09/2014 12:11 PM, Daniel Vetter wrote:
>>>> On Wed, Oct 08, 2014 at 06:32:21PM +0300, Ander Conselvan de Oliveira wrote:
>>>>> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>>>>
>>>>> This shouldn't change the behavior of those functions, since they are
>>>>> called after the new_config is made effective and that points to the
>>>>> current config. In a follow up patch, the mode set sequence will be
>>>>> changed so this is called before disabling crtcs, and in that case
>>>>> those functions should work on the staged config.
>>>>>
>>>>> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>>>> ---
>>>
>>> [snip]
>>>
>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>>> index 3a06c3d..9d8fe8d 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>> @@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
>>>>>    return false;
>>>>>   }
>>>>>
>>>>> +/**
>>>>> + * Returns whether any output on the specified pipe will have the specified
>>>>> + * type after a staged modeset is complete, i.e., the same as
>>>>> + * intel_pipe_has_type() but looking at encoder->new_crtc instead of
>>>>> + * encoder->crtc.
>>>>> + */
>>>>> +static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)
>>>>
>>>> s/drm_crtc/intel_crtc/. In general we use the most specific type
>>>> for internal functions to avoid tons of upcasting. So you might want to
>>>> throw a patch on top to convert the existing has_type for consistency.
>>>
>>> What about the functions in drm_i915_display_funcs (find_dpll,
>>> crtc_mode_set)? Are these not considered internal functions or is this
>>> leftover from before this rule was in place?
>>>
>>> If I write that consistency patch it might be easier to just convert
>>> everything.
>>
>> leftovers, and we've been slowly converting existing code over to using
>> intel structures over the past 2 years. I certainly won't stop you to
>> clean up all that stuff, but it's a bit of work. So I'm ok if you just
>> have the vfunc signature use the intel_ types and then immediately convert
>> to a drm_ type again to avoid code churn.
>>
>> Or just convert it all if you feel like it.
>
> But if you do that please split out that rote conversion into a separate
> patch for easier reviewing. Either at the start of the series (so I can
> merge it right away) or at the end (only done once everything is merged)
> to avoid needless rebase pain.

I went down the "covert all" route. I sent the prep patches as a 
separate series now.

Thanks,
Ander
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cb5367c..619723d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -792,7 +792,7 @@  hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
 		      WRPLL_DIVIDER_REFERENCE(r2) | WRPLL_DIVIDER_FEEDBACK(n2) |
 		      WRPLL_DIVIDER_POST(p);
 
-		intel_crtc->config.dpll_hw_state.wrpll = val;
+		intel_crtc->new_config->dpll_hw_state.wrpll = val;
 
 		pll = intel_get_shared_dpll(intel_crtc);
 		if (pll == NULL) {
@@ -801,7 +801,7 @@  hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
 			return false;
 		}
 
-		intel_crtc->config.ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
+		intel_crtc->new_config->ddi_pll_sel = PORT_CLK_SEL_WRPLL(pll->id);
 	}
 
 	return true;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3a06c3d..9d8fe8d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -420,13 +420,32 @@  static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
 	return false;
 }
 
+/**
+ * Returns whether any output on the specified pipe will have the specified
+ * type after a staged modeset is complete, i.e., the same as
+ * intel_pipe_has_type() but looking at encoder->new_crtc instead of
+ * encoder->crtc.
+ */
+static bool intel_pipe_will_have_type(struct drm_crtc *crtc, int type)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_encoder *encoder;
+
+	for_each_intel_encoder(dev, encoder)
+		if (encoder->new_crtc == to_intel_crtc(crtc) &&
+		    encoder->type == type)
+			return true;
+
+	return false;
+}
+
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 						int refclk)
 {
 	struct drm_device *dev = crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev)) {
 			if (refclk == 100000)
 				limit = &intel_limits_ironlake_dual_lvds_100m;
@@ -449,15 +468,15 @@  static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	const intel_limit_t *limit;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			limit = &intel_limits_g4x_dual_channel_lvds;
 		else
 			limit = &intel_limits_g4x_single_channel_lvds;
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI) ||
-		   intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG)) {
+	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_HDMI) ||
+		   intel_pipe_will_have_type(crtc, INTEL_OUTPUT_ANALOG)) {
 		limit = &intel_limits_g4x_hdmi;
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
+	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
 	} else /* The option is for other outputs */
 		limit = &intel_limits_i9xx_sdvo;
@@ -475,7 +494,7 @@  static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
 	else if (IS_G4X(dev)) {
 		limit = intel_g4x_limit(crtc);
 	} else if (IS_PINEVIEW(dev)) {
-		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_pineview_lvds;
 		else
 			limit = &intel_limits_pineview_sdvo;
@@ -484,14 +503,14 @@  static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
 	} else if (IS_VALLEYVIEW(dev)) {
 		limit = &intel_limits_vlv;
 	} else if (!IS_GEN2(dev)) {
-		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
 		else
 			limit = &intel_limits_i9xx_sdvo;
 	} else {
-		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
+		if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i8xx_lvds;
-		else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DVO))
+		else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_DVO))
 			limit = &intel_limits_i8xx_dvo;
 		else
 			limit = &intel_limits_i8xx_dac;
@@ -586,7 +605,7 @@  i9xx_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -647,7 +666,7 @@  pnv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	intel_clock_t clock;
 	int err = target;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
 		/*
 		 * For LVDS just rely on its current settings for dual-channel.
 		 * We haven't figured out how to reliably set up different
@@ -710,7 +729,7 @@  g4x_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	int err_most = (target >> 8) + (target >> 9);
 	found = false;
 
-	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS)) {
 		if (intel_is_dual_link_lvds(dev))
 			clock.p2 = limit->p2.p2_fast;
 		else
@@ -5607,7 +5626,7 @@  static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 
 	if (IS_VALLEYVIEW(dev)) {
 		refclk = 100000;
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
+	} else if (intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
 		refclk = dev_priv->vbt.lvds_ssc_freq;
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -5758,11 +5777,11 @@  static void vlv_update_pll(struct intel_crtc *crtc)
 	if (crtc->pipe == PIPE_B)
 		dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
 	dpll |= DPLL_VCO_ENABLE;
-	crtc->config.dpll_hw_state.dpll = dpll;
+	crtc->new_config->dpll_hw_state.dpll = dpll;
 
-	dpll_md = (crtc->config.pixel_multiplier - 1)
+	dpll_md = (crtc->new_config->pixel_multiplier - 1)
 		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
-	crtc->config.dpll_hw_state.dpll_md = dpll_md;
+	crtc->new_config->dpll_hw_state.dpll_md = dpll_md;
 }
 
 static void vlv_prepare_pll(struct intel_crtc *crtc)
@@ -5858,14 +5877,16 @@  static void vlv_prepare_pll(struct intel_crtc *crtc)
 
 static void chv_update_pll(struct intel_crtc *crtc)
 {
-	crtc->config.dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
+	crtc->new_config->dpll_hw_state.dpll = DPLL_SSC_REF_CLOCK_CHV |
 		DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS |
 		DPLL_VCO_ENABLE;
 	if (crtc->pipe != PIPE_A)
-		crtc->config.dpll_hw_state.dpll |= DPLL_INTEGRATED_CRI_CLK_VLV;
+		crtc->new_config->dpll_hw_state.dpll |=
+			DPLL_INTEGRATED_CRI_CLK_VLV;
 
-	crtc->config.dpll_hw_state.dpll_md =
-		(crtc->config.pixel_multiplier - 1) << DPLL_MD_UDI_MULTIPLIER_SHIFT;
+	crtc->new_config->dpll_hw_state.dpll_md =
+		(crtc->new_config->pixel_multiplier - 1)
+		<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
 }
 
 static void chv_prepare_pll(struct intel_crtc *crtc)
@@ -5946,29 +5967,29 @@  static void i9xx_update_pll(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpll;
 	bool is_sdvo;
-	struct dpll *clock = &crtc->config.dpll;
+	struct dpll *clock = &crtc->new_config->dpll;
 
 	i9xx_update_pll_dividers(crtc, reduced_clock);
 
-	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
-		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
+	is_sdvo = intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
+		intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_HDMI);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
+	if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS))
 		dpll |= DPLLB_MODE_LVDS;
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
 
 	if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) {
-		dpll |= (crtc->config.pixel_multiplier - 1)
+		dpll |= (crtc->new_config->pixel_multiplier - 1)
 			<< SDVO_MULTIPLIER_SHIFT_HIRES;
 	}
 
 	if (is_sdvo)
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
+	if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
@@ -5996,21 +6017,21 @@  static void i9xx_update_pll(struct intel_crtc *crtc,
 	if (INTEL_INFO(dev)->gen >= 4)
 		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
 
-	if (crtc->config.sdvo_tv_clock)
+	if (crtc->new_config->sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
-	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
+	else if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
 
 	dpll |= DPLL_VCO_ENABLE;
-	crtc->config.dpll_hw_state.dpll = dpll;
+	crtc->new_config->dpll_hw_state.dpll = dpll;
 
 	if (INTEL_INFO(dev)->gen >= 4) {
-		u32 dpll_md = (crtc->config.pixel_multiplier - 1)
+		u32 dpll_md = (crtc->new_config->pixel_multiplier - 1)
 			<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
-		crtc->config.dpll_hw_state.dpll_md = dpll_md;
+		crtc->new_config->dpll_hw_state.dpll_md = dpll_md;
 	}
 }
 
@@ -6021,13 +6042,13 @@  static void i8xx_update_pll(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpll;
-	struct dpll *clock = &crtc->config.dpll;
+	struct dpll *clock = &crtc->new_config->dpll;
 
 	i9xx_update_pll_dividers(crtc, reduced_clock);
 
 	dpll = DPLL_VGA_MODE_DIS;
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
+	if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
 		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	} else {
 		if (clock->p1 == 2)
@@ -6038,17 +6059,18 @@  static void i8xx_update_pll(struct intel_crtc *crtc,
 			dpll |= PLL_P2_DIVIDE_BY_4;
 	}
 
-	if (!IS_I830(dev) && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DVO))
+	if (!IS_I830(dev) &&
+	    intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_DVO))
 		dpll |= DPLL_DVO_2X_MODE;
 
-	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
 		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
 
 	dpll |= DPLL_VCO_ENABLE;
-	crtc->config.dpll_hw_state.dpll = dpll;
+	crtc->new_config->dpll_hw_state.dpll = dpll;
 }
 
 static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
@@ -6258,7 +6280,10 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
 
-	for_each_encoder_on_crtc(dev, crtc, encoder) {
+	for_each_intel_encoder(dev, encoder) {
+		if (encoder->new_crtc != to_intel_crtc(crtc))
+			continue;
+
 		switch (encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -6274,7 +6299,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	if (is_dsi)
 		return 0;
 
-	if (!intel_crtc->config.clock_set) {
+	if (!intel_crtc->new_config->clock_set) {
 		refclk = i9xx_get_refclk(crtc, num_connectors);
 
 		/*
@@ -6285,7 +6310,7 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		 */
 		limit = intel_limit(crtc, refclk);
 		ok = dev_priv->display.find_dpll(limit, crtc,
-						 intel_crtc->config.port_clock,
+						 intel_crtc->new_config->port_clock,
 						 refclk, NULL, &clock);
 		if (!ok) {
 			DRM_ERROR("Couldn't find PLL settings for mode!\n");
@@ -6306,11 +6331,11 @@  static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 							    &reduced_clock);
 		}
 		/* Compat-code for transition, will disappear. */
-		intel_crtc->config.dpll.n = clock.n;
-		intel_crtc->config.dpll.m1 = clock.m1;
-		intel_crtc->config.dpll.m2 = clock.m2;
-		intel_crtc->config.dpll.p1 = clock.p1;
-		intel_crtc->config.dpll.p2 = clock.p2;
+		intel_crtc->new_config->dpll.n = clock.n;
+		intel_crtc->new_config->dpll.m1 = clock.m1;
+		intel_crtc->new_config->dpll.m2 = clock.m2;
+		intel_crtc->new_config->dpll.p1 = clock.p1;
+		intel_crtc->new_config->dpll.p2 = clock.p2;
 	}
 
 	if (IS_GEN2(dev)) {
@@ -6926,7 +6951,10 @@  static int ironlake_get_refclk(struct drm_crtc *crtc)
 	int num_connectors = 0;
 	bool is_lvds = false;
 
-	for_each_encoder_on_crtc(dev, crtc, encoder) {
+	for_each_intel_encoder(dev, encoder) {
+		if (encoder->new_crtc != to_intel_crtc(crtc))
+			continue;
+
 		switch (encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -7114,7 +7142,7 @@  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	const intel_limit_t *limit;
 	bool ret, is_lvds = false;
 
-	is_lvds = intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS);
+	is_lvds = intel_pipe_will_have_type(crtc, INTEL_OUTPUT_LVDS);
 
 	refclk = ironlake_get_refclk(crtc);
 
@@ -7125,7 +7153,7 @@  static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 */
 	limit = intel_limit(crtc, refclk);
 	ret = dev_priv->display.find_dpll(limit, crtc,
-					  to_intel_crtc(crtc)->config.port_clock,
+					  to_intel_crtc(crtc)->new_config->port_clock,
 					  refclk, NULL, clock);
 	if (!ret)
 		return false;
@@ -7175,7 +7203,10 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	int factor, num_connectors = 0;
 	bool is_lvds = false, is_sdvo = false;
 
-	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
+	for_each_intel_encoder(dev, intel_encoder) {
+		if (intel_encoder->new_crtc != to_intel_crtc(crtc))
+			continue;
+
 		switch (intel_encoder->type) {
 		case INTEL_OUTPUT_LVDS:
 			is_lvds = true;
@@ -7196,10 +7227,10 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		     dev_priv->vbt.lvds_ssc_freq == 100000) ||
 		    (HAS_PCH_IBX(dev) && intel_is_dual_link_lvds(dev)))
 			factor = 25;
-	} else if (intel_crtc->config.sdvo_tv_clock)
+	} else if (intel_crtc->new_config->sdvo_tv_clock)
 		factor = 20;
 
-	if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor))
+	if (ironlake_needs_fb_cb_tune(&intel_crtc->new_config->dpll, factor))
 		*fp |= FP_CB_TUNE;
 
 	if (fp2 && (reduced_clock->m < factor * reduced_clock->n))
@@ -7212,20 +7243,20 @@  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	else
 		dpll |= DPLLB_MODE_DAC_SERIAL;
 
-	dpll |= (intel_crtc->config.pixel_multiplier - 1)
+	dpll |= (intel_crtc->new_config->pixel_multiplier - 1)
 		<< PLL_REF_SDVO_HDMI_MULTIPLIER_SHIFT;
 
 	if (is_sdvo)
 		dpll |= DPLL_SDVO_HIGH_SPEED;
-	if (intel_crtc->config.has_dp_encoder)
+	if (intel_crtc->new_config->has_dp_encoder)
 		dpll |= DPLL_SDVO_HIGH_SPEED;
 
 	/* compute bitmask from p1 value */
-	dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
+	dpll |= (1 << (intel_crtc->new_config->dpll.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
 	/* also FPA1 */
-	dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT;
+	dpll |= (1 << (intel_crtc->new_config->dpll.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT;
 
-	switch (intel_crtc->config.dpll.p2) {
+	switch (intel_crtc->new_config->dpll.p2) {
 	case 5:
 		dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_5;
 		break;
@@ -7267,22 +7298,22 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	ok = ironlake_compute_clocks(crtc, &clock,
 				     &has_reduced_clock, &reduced_clock);
-	if (!ok && !intel_crtc->config.clock_set) {
+	if (!ok && !intel_crtc->new_config->clock_set) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
 		return -EINVAL;
 	}
 	/* Compat-code for transition, will disappear. */
-	if (!intel_crtc->config.clock_set) {
-		intel_crtc->config.dpll.n = clock.n;
-		intel_crtc->config.dpll.m1 = clock.m1;
-		intel_crtc->config.dpll.m2 = clock.m2;
-		intel_crtc->config.dpll.p1 = clock.p1;
-		intel_crtc->config.dpll.p2 = clock.p2;
+	if (!intel_crtc->new_config->clock_set) {
+		intel_crtc->new_config->dpll.n = clock.n;
+		intel_crtc->new_config->dpll.m1 = clock.m1;
+		intel_crtc->new_config->dpll.m2 = clock.m2;
+		intel_crtc->new_config->dpll.p1 = clock.p1;
+		intel_crtc->new_config->dpll.p2 = clock.p2;
 	}
 
 	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
-	if (intel_crtc->config.has_pch_encoder) {
-		fp = i9xx_dpll_compute_fp(&intel_crtc->config.dpll);
+	if (intel_crtc->new_config->has_pch_encoder) {
+		fp = i9xx_dpll_compute_fp(&intel_crtc->new_config->dpll);
 		if (has_reduced_clock)
 			fp2 = i9xx_dpll_compute_fp(&reduced_clock);
 
@@ -7290,12 +7321,12 @@  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 					     &fp, &reduced_clock,
 					     has_reduced_clock ? &fp2 : NULL);
 
-		intel_crtc->config.dpll_hw_state.dpll = dpll;
-		intel_crtc->config.dpll_hw_state.fp0 = fp;
+		intel_crtc->new_config->dpll_hw_state.dpll = dpll;
+		intel_crtc->new_config->dpll_hw_state.fp0 = fp;
 		if (has_reduced_clock)
-			intel_crtc->config.dpll_hw_state.fp1 = fp2;
+			intel_crtc->new_config->dpll_hw_state.fp1 = fp2;
 		else
-			intel_crtc->config.dpll_hw_state.fp1 = fp;
+			intel_crtc->new_config->dpll_hw_state.fp1 = fp;
 
 		pll = intel_get_shared_dpll(intel_crtc);
 		if (pll == NULL) {