Message ID | 1412782343-28836-3-git-send-email-conselvan2@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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
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
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.
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
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
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 --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) {