Stage shared dpll configs

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

---
 drivers/gpu/drm/i915/intel_ddi.c     |   4 +-
 drivers/gpu/drm/i915/intel_display.c | 167 +++++++++++++++++++++--------------
 2 files changed, 101 insertions(+), 70 deletions(-)

drivers/gpu/drm/i915/intel_ddi.c +2 -2

index cb5367c..619723d 100644
@@ -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;

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644

drivers/gpu/drm/i915/intel_display.c +92 -69

+{
+ 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 10 lines · Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

---

Quoted 8 lines

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644
@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
   return false;
  }
 

drivers/gpu/drm/i915/intel_display.c static bool intel_pipe_has_type(struct drm_crtc *crtc, int type) · line 429

Quoted 7 lines

drivers/gpu/drm/i915/intel_display.c +7 -0

+/**
+ * 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.

Quoted 11 lines · Daniel Vetter

Quoted 10 lines · Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

---

[snip]

Quoted 8 lines

Quoted 8 lines

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644
@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
  return false;
 }
 

Quoted 11 lines

drivers/gpu/drm/i915/intel_display.c static bool intel_pipe_has_type(struct drm_crtc *crtc, int type) · line 429

Quoted 7 lines

drivers/gpu/drm/i915/intel_display.c +7 -0

+/**
+ * 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.

Quoted 35 lines · Ander Conselvan de Oliveira

Quoted 11 lines · Daniel Vetter

Quoted 10 lines · Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

---

[snip]

Quoted 8 lines

Quoted 8 lines

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644
@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
  return false;
 }
 

Quoted 11 lines

drivers/gpu/drm/i915/intel_display.c static bool intel_pipe_has_type(struct drm_crtc *crtc, int type) · line 429

Quoted 7 lines

drivers/gpu/drm/i915/intel_display.c +7 -0

+/**
+ * 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.

Quoted 6 lines

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.

Quoted 49 lines · Daniel Vetter

Quoted 35 lines · Ander Conselvan de Oliveira

Quoted 11 lines · Daniel Vetter

Quoted 10 lines · Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira <[email protected]>

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 <[email protected]>

---

[snip]

Quoted 8 lines

Quoted 8 lines

drivers/gpu/drm/i915/intel_display.c +0 -0

index 3a06c3d..9d8fe8d 100644
@@ -420,13 +420,32 @@ static bool intel_pipe_has_type(struct drm_crtc *crtc, int type)
   return false;
  }
 

Quoted 11 lines

drivers/gpu/drm/i915/intel_display.c static bool intel_pipe_has_type(struct drm_crtc *crtc, int type) · line 429

Quoted 7 lines

drivers/gpu/drm/i915/intel_display.c +7 -0

+/**
+ * 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.

Quoted 6 lines

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.

Quoted 1 line

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.

From: Ander Conselvan de Oliveira <[email protected]>

It is possible for a mode set to fail if there aren't shared DPLLS that match the new configuration requirement or other errors in clock computation. Since that step was executed after disabling crtcs, in the failure case the hardware configuration is changed and needs to be restored. Doing those things early allows the mode set to fail before actually touching the hardware.

Signed-off-by Ander Conselvan de Oliveira <[email protected]>

---
 drivers/gpu/drm/i915/i915_drv.h      |   7 +++
 drivers/gpu/drm/i915/intel_ddi.c     |   2 -
 drivers/gpu/drm/i915/intel_display.c | 113 ++++++++++++++++++++++++++---------
 3 files changed, 92 insertions(+), 30 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +7 -0

index cb6df07..0bb3b54 100644
@@ -245,6 +245,12 @@ struct intel_shared_dpll {
  bool (*get_hw_state)(struct drm_i915_private *dev_priv,
      struct intel_shared_dpll *pll,
      struct intel_dpll_hw_state *hw_state);
+
+ /* Holds the new configuration of the shared DPLLS during mode set */
+ struct {
+ unsigned crtc_mask;
+ struct intel_dpll_hw_state hw_state;
+ } staged_config;
 };
 
 /* Used by dp and fdi links */

@@ -476,6 +482,7 @@ struct drm_i915_display_funcs {
  struct intel_crtc_config *);
  void (*get_plane_config)(struct intel_crtc *,
  struct intel_plane_config *);
+ int (*crtc_compute_clock)(struct drm_crtc *crtc);
  int (*crtc_mode_set)(struct drm_crtc *crtc,
      int x, int y,
      struct drm_framebuffer *old_fb);

drivers/gpu/drm/i915/intel_ddi.c +0 -2

index 619723d..fc6f988 100644
@@ -821,8 +821,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
  struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
  int clock = intel_crtc->config.port_clock;
 
- intel_put_shared_dpll(intel_crtc);
-
  return hsw_ddi_pll_select(intel_crtc, intel_encoder, clock);
 }
 

drivers/gpu/drm/i915/intel_display.c +85 -29

index 46df2db..4a8aff7 100644
@@ -3852,15 +3852,9 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
 struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
 {
  struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ struct intel_shared_dpll *pll;
  enum intel_dpll_id i;
 
- if (pll) {
- DRM_DEBUG_KMS("CRTC:%d dropping existing %s\n",
-      crtc->base.base.id, pll->name);
- intel_put_shared_dpll(crtc);
- }
-
  if (HAS_PCH_IBX(dev_priv->dev)) {
  /* Ironlake PCH has a fixed PLL->PCH pipe mapping. */
  i = (enum intel_dpll_id) crtc->pipe;

@@ -3869,7 +3863,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
       crtc->base.base.id, pll->name);
 
- WARN_ON(pll->crtc_mask);
+ WARN_ON(pll->staged_config.crtc_mask);
 
  goto found;
  }

@@ -3878,16 +3872,16 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  pll = &dev_priv->shared_dplls[i];
 
  /* Only want to check enabled timings first */
- if (pll->crtc_mask == 0)
+ if (pll->staged_config.crtc_mask == 0)
  continue;
 
- if (memcmp(&crtc->config.dpll_hw_state, &pll->hw_state,
-   sizeof(pll->hw_state)) == 0) {
- DRM_DEBUG_KMS("CRTC:%d sharing existing %s "
-      "(crtc_mask 0x%08x, active %d)\n",
+ if (memcmp(&crtc->new_config->dpll_hw_state,
+   &pll->staged_config.hw_state,
+   sizeof(pll->staged_config.hw_state)) == 0) {
+ DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n",
       crtc->base.base.id, pll->name,
-      pll->crtc_mask, pll->active);
-
+      pll->staged_config.crtc_mask,
+      pll->active);
  goto found;
  }
  }

@@ -3895,7 +3889,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  /* Ok no matching timings, maybe there's a free one? */
  for (i = 0; i < dev_priv->num_shared_dpll; i++) {
  pll = &dev_priv->shared_dplls[i];
- if (pll->crtc_mask == 0) {
+ if (pll->staged_config.crtc_mask == 0) {
  DRM_DEBUG_KMS("CRTC:%d allocated %s\n",
       crtc->base.base.id, pll->name);
  goto found;

@@ -3905,18 +3899,53 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  return NULL;
 
 found:
- if (pll->crtc_mask == 0)
- pll->hw_state = crtc->config.dpll_hw_state;
+ if (pll->staged_config.crtc_mask == 0)
+ pll->staged_config.hw_state = crtc->new_config->dpll_hw_state;
 
- crtc->config.shared_dpll = i;
+ crtc->new_config->shared_dpll = i;
  DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
  pipe_name(crtc->pipe));
 
- pll->crtc_mask |= 1 << crtc->pipe;
+ pll->staged_config.crtc_mask |= 1 << crtc->pipe;
 
  return pll;
 }
 
+/**
+ * intel_shared_dpll_start_config - start a new PLL staged config
+ * @dev_priv: DRM device
+ * @clear_pipes: mask of pipes that will have their PLLs freed
+ *
+ * Starts a new PLL staged config, copying the current config but
+ * releasing the references of pipes specified in clear_pipes.
+ */
+static void intel_shared_dpll_start_config(struct drm_i915_private *dev_priv,
+   unsigned clear_pipes)
+{
+ struct intel_shared_dpll *pll;
+ enum intel_dpll_id i;
+
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ pll = &dev_priv->shared_dplls[i];
+
+ pll->staged_config.crtc_mask = pll->crtc_mask & ~clear_pipes;
+ pll->staged_config.hw_state = pll->hw_state;
+ }
+}
+
+static void intel_shared_dpll_commit(struct drm_i915_private *dev_priv)
+{
+ struct intel_shared_dpll *pll;
+ enum intel_dpll_id i;
+
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ pll = &dev_priv->shared_dplls[i];
+
+ pll->hw_state = pll->staged_config.hw_state;
+ pll->crtc_mask = pll->staged_config.crtc_mask;
+ }
+}
+
 static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 {
  struct drm_i915_private *dev_priv = dev->dev_private;

@@ -6272,6 +6301,11 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
       int x, int y,
       struct drm_framebuffer *fb)
 {
+ return 0;
+}
+
+static int i9xx_crtc_compute_clock(struct drm_crtc *crtc)
+{
  struct drm_device *dev = crtc->dev;
  struct drm_i915_private *dev_priv = dev->dev_private;
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

@@ -7281,9 +7315,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
  return dpll | DPLL_VCO_ENABLE;
 }
 
-static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
-  int x, int y,
-  struct drm_framebuffer *fb)
+static int ironlake_crtc_compute_clock(struct drm_crtc *crtc)
 {
  struct drm_device *dev = crtc->dev;
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

@@ -7336,8 +7368,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
  pipe_name(intel_crtc->pipe));
  return -EINVAL;
  }
- } else
- intel_put_shared_dpll(intel_crtc);
+ }
 
  if (is_lvds && has_reduced_clock && i915.powersave)
  intel_crtc->lowfreq_avail = true;

@@ -7347,6 +7378,13 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
  return 0;
 }
 
+static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
+  int x, int y,
+  struct drm_framebuffer *fb)
+{
+ return 0;
+}
+
 static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
  struct intel_link_m_n *m_n)
 {

@@ -7836,9 +7874,7 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
  modeset_update_crtc_power_domains(dev);
 }
 
-static int haswell_crtc_mode_set(struct drm_crtc *crtc,
- int x, int y,
- struct drm_framebuffer *fb)
+static int haswell_crtc_compute_clock(struct drm_crtc *crtc)
 {
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 

@@ -7850,6 +7886,13 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
  return 0;
 }
 
+static int haswell_crtc_mode_set(struct drm_crtc *crtc,
+ int x, int y,
+ struct drm_framebuffer *fb)
+{
+ return 0;
+}
+
 static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
  enum port port,
  struct intel_crtc_config *pipe_config)

@@ -10988,6 +11031,14 @@ static int __intel_set_mode(struct drm_crtc *crtc,
  prepare_pipes &= ~disable_pipes;
  }
 
+ intel_shared_dpll_start_config(dev_priv, modeset_pipes | disable_pipes);
+
+ for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
+ ret = dev_priv->display.crtc_compute_clock(&intel_crtc->base);
+ if (ret)
+ goto done;
+ }
+
  for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
  intel_crtc_disable(&intel_crtc->base);
 

@@ -11015,6 +11066,8 @@ static int __intel_set_mode(struct drm_crtc *crtc,
  &pipe_config->adjusted_mode);
  }
 
+ intel_shared_dpll_commit(dev_priv);
+
  /* Only after disabling all output pipelines that will be changed can we
  * update the the output configuration. */
  intel_modeset_update_state(dev, prepare_pipes);

@@ -12580,6 +12633,7 @@ static void intel_init_display(struct drm_device *dev)
  if (HAS_DDI(dev)) {
  dev_priv->display.get_pipe_config = haswell_get_pipe_config;
  dev_priv->display.get_plane_config = ironlake_get_plane_config;
+ dev_priv->display.crtc_compute_clock = haswell_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
  dev_priv->display.crtc_enable = haswell_crtc_enable;
  dev_priv->display.crtc_disable = haswell_crtc_disable;

@@ -12593,6 +12647,7 @@ static void intel_init_display(struct drm_device *dev)
  } else if (HAS_PCH_SPLIT(dev)) {
  dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
  dev_priv->display.get_plane_config = ironlake_get_plane_config;
+ dev_priv->display.crtc_compute_clock = ironlake_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
  dev_priv->display.crtc_enable = ironlake_crtc_enable;
  dev_priv->display.crtc_disable = ironlake_crtc_disable;

@@ -12602,6 +12657,7 @@ static void intel_init_display(struct drm_device *dev)
  } else if (IS_VALLEYVIEW(dev)) {
  dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
  dev_priv->display.get_plane_config = i9xx_get_plane_config;
+ dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
  dev_priv->display.crtc_enable = valleyview_crtc_enable;
  dev_priv->display.crtc_disable = i9xx_crtc_disable;

@@ -12611,6 +12667,7 @@ static void intel_init_display(struct drm_device *dev)
  } else {
  dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
  dev_priv->display.get_plane_config = i9xx_get_plane_config;
+ dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
  dev_priv->display.crtc_enable = i9xx_crtc_enable;
  dev_priv->display.crtc_disable = i9xx_crtc_disable;
--

1.8.3.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Quoted 9 lines · Ander Conselvan de Oliveira

From: Ander Conselvan de Oliveira <[email protected]>

It is possible for a mode set to fail if there aren't shared DPLLS that match the new configuration requirement or other errors in clock computation. Since that step was executed after disabling crtcs, in the failure case the hardware configuration is changed and needs to be restored. Doing those things early allows the mode set to fail before actually touching the hardware.

Quoted 1 line

Signed-off-by Ander Conselvan de Oliveira <[email protected]>

Looks nifty and gets us to the shiny new world without too much fuzz. I think for paranoia it would be good to split this patch here per platform (i.e. i9xx, ilk+ and hsw+). And if we make the crtc_mode_set hook optional we can also get rid of all the dummy functions here. And remove the caller (yay!) at the end of the series.

Longer-term we might still want to shuffle quite a few of the encoder-specific pll computation code into encoder->compute_config. But that can be done as leisure if someone gets bored ;-)

Cheers, Daniel

Quoted 300 lines

---
 drivers/gpu/drm/i915/i915_drv.h      |   7 +++
 drivers/gpu/drm/i915/intel_ddi.c     |   2 -
 drivers/gpu/drm/i915/intel_display.c | 113 ++++++++++++++++++++++++++---------
 3 files changed, 92 insertions(+), 30 deletions(-)

drivers/gpu/drm/i915/i915_drv.h +7 -0

index cb6df07..0bb3b54 100644
@@ -245,6 +245,12 @@ struct intel_shared_dpll {
  bool (*get_hw_state)(struct drm_i915_private *dev_priv,
      struct intel_shared_dpll *pll,
      struct intel_dpll_hw_state *hw_state);
+
+ /* Holds the new configuration of the shared DPLLS during mode set */
+ struct {
+ unsigned crtc_mask;
+ struct intel_dpll_hw_state hw_state;
+ } staged_config;
 };
 
 /* Used by dp and fdi links */

@@ -476,6 +482,7 @@ struct drm_i915_display_funcs {
  struct intel_crtc_config *);
  void (*get_plane_config)(struct intel_crtc *,
  struct intel_plane_config *);
+ int (*crtc_compute_clock)(struct drm_crtc *crtc);
  int (*crtc_mode_set)(struct drm_crtc *crtc,
      int x, int y,
      struct drm_framebuffer *old_fb);

drivers/gpu/drm/i915/intel_ddi.c +0 -2

index 619723d..fc6f988 100644
@@ -821,8 +821,6 @@ bool intel_ddi_pll_select(struct intel_crtc *intel_crtc)
  struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
  int clock = intel_crtc->config.port_clock;
 
- intel_put_shared_dpll(intel_crtc);
-
  return hsw_ddi_pll_select(intel_crtc, intel_encoder, clock);
 }
 

drivers/gpu/drm/i915/intel_display.c +85 -29

index 46df2db..4a8aff7 100644
@@ -3852,15 +3852,9 @@ void intel_put_shared_dpll(struct intel_crtc *crtc)
 struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
 {
  struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
- struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
+ struct intel_shared_dpll *pll;
  enum intel_dpll_id i;
 
- if (pll) {
- DRM_DEBUG_KMS("CRTC:%d dropping existing %s\n",
-      crtc->base.base.id, pll->name);
- intel_put_shared_dpll(crtc);
- }
-
  if (HAS_PCH_IBX(dev_priv->dev)) {
  /* Ironlake PCH has a fixed PLL->PCH pipe mapping. */
  i = (enum intel_dpll_id) crtc->pipe;

@@ -3869,7 +3863,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n",
       crtc->base.base.id, pll->name);
 
- WARN_ON(pll->crtc_mask);
+ WARN_ON(pll->staged_config.crtc_mask);
 
  goto found;
  }

@@ -3878,16 +3872,16 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  pll = &dev_priv->shared_dplls[i];
 
  /* Only want to check enabled timings first */
- if (pll->crtc_mask == 0)
+ if (pll->staged_config.crtc_mask == 0)
  continue;
 
- if (memcmp(&crtc->config.dpll_hw_state, &pll->hw_state,
-   sizeof(pll->hw_state)) == 0) {
- DRM_DEBUG_KMS("CRTC:%d sharing existing %s "
-      "(crtc_mask 0x%08x, active %d)\n",
+ if (memcmp(&crtc->new_config->dpll_hw_state,
+   &pll->staged_config.hw_state,
+   sizeof(pll->staged_config.hw_state)) == 0) {
+ DRM_DEBUG_KMS("CRTC:%d sharing existing %s (crtc mask 0x%08x, ative %d)\n",
       crtc->base.base.id, pll->name,
-      pll->crtc_mask, pll->active);
-
+      pll->staged_config.crtc_mask,
+      pll->active);
  goto found;
  }
  }

@@ -3895,7 +3889,7 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  /* Ok no matching timings, maybe there's a free one? */
  for (i = 0; i < dev_priv->num_shared_dpll; i++) {
  pll = &dev_priv->shared_dplls[i];
- if (pll->crtc_mask == 0) {
+ if (pll->staged_config.crtc_mask == 0) {
  DRM_DEBUG_KMS("CRTC:%d allocated %s\n",
       crtc->base.base.id, pll->name);
  goto found;

@@ -3905,18 +3899,53 @@ struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc)
  return NULL;
 
 found:
- if (pll->crtc_mask == 0)
- pll->hw_state = crtc->config.dpll_hw_state;
+ if (pll->staged_config.crtc_mask == 0)
+ pll->staged_config.hw_state = crtc->new_config->dpll_hw_state;
 
- crtc->config.shared_dpll = i;
+ crtc->new_config->shared_dpll = i;
  DRM_DEBUG_DRIVER("using %s for pipe %c\n", pll->name,
  pipe_name(crtc->pipe));
 
- pll->crtc_mask |= 1 << crtc->pipe;
+ pll->staged_config.crtc_mask |= 1 << crtc->pipe;
 
  return pll;
 }
 
+/**
+ * intel_shared_dpll_start_config - start a new PLL staged config
+ * @dev_priv: DRM device
+ * @clear_pipes: mask of pipes that will have their PLLs freed
+ *
+ * Starts a new PLL staged config, copying the current config but
+ * releasing the references of pipes specified in clear_pipes.
+ */
+static void intel_shared_dpll_start_config(struct drm_i915_private *dev_priv,
+   unsigned clear_pipes)
+{
+ struct intel_shared_dpll *pll;
+ enum intel_dpll_id i;
+
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ pll = &dev_priv->shared_dplls[i];
+
+ pll->staged_config.crtc_mask = pll->crtc_mask & ~clear_pipes;
+ pll->staged_config.hw_state = pll->hw_state;
+ }
+}
+
+static void intel_shared_dpll_commit(struct drm_i915_private *dev_priv)
+{
+ struct intel_shared_dpll *pll;
+ enum intel_dpll_id i;
+
+ for (i = 0; i < dev_priv->num_shared_dpll; i++) {
+ pll = &dev_priv->shared_dplls[i];
+
+ pll->hw_state = pll->staged_config.hw_state;
+ pll->crtc_mask = pll->staged_config.crtc_mask;
+ }
+}
+
 static void cpt_verify_modeset(struct drm_device *dev, int pipe)
 {
  struct drm_i915_private *dev_priv = dev->dev_private;

@@ -6272,6 +6301,11 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
       int x, int y,
       struct drm_framebuffer *fb)
 {
+ return 0;
+}
+
+static int i9xx_crtc_compute_clock(struct drm_crtc *crtc)
+{
  struct drm_device *dev = crtc->dev;
  struct drm_i915_private *dev_priv = dev->dev_private;
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

@@ -7281,9 +7315,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
  return dpll | DPLL_VCO_ENABLE;
 }
 
-static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
-  int x, int y,
-  struct drm_framebuffer *fb)
+static int ironlake_crtc_compute_clock(struct drm_crtc *crtc)
 {
  struct drm_device *dev = crtc->dev;
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);

@@ -7336,8 +7368,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
  pipe_name(intel_crtc->pipe));
  return -EINVAL;
  }
- } else
- intel_put_shared_dpll(intel_crtc);
+ }
 
  if (is_lvds && has_reduced_clock && i915.powersave)
  intel_crtc->lowfreq_avail = true;

@@ -7347,6 +7378,13 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
  return 0;
 }
 
+static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
+  int x, int y,
+  struct drm_framebuffer *fb)
+{
+ return 0;
+}
+
 static void intel_pch_transcoder_get_m_n(struct intel_crtc *crtc,
  struct intel_link_m_n *m_n)
 {

@@ -7836,9 +7874,7 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
  modeset_update_crtc_power_domains(dev);
 }
 
-static int haswell_crtc_mode_set(struct drm_crtc *crtc,
- int x, int y,
- struct drm_framebuffer *fb)
+static int haswell_crtc_compute_clock(struct drm_crtc *crtc)
 {
  struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 

@@ -7850,6 +7886,13 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
  return 0;
 }
 
+static int haswell_crtc_mode_set(struct drm_crtc *crtc,
+ int x, int y,
+ struct drm_framebuffer *fb)
+{
+ return 0;
+}
+
 static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
  enum port port,
  struct intel_crtc_config *pipe_config)

@@ -10988,6 +11031,14 @@ static int __intel_set_mode(struct drm_crtc *crtc,
  prepare_pipes &= ~disable_pipes;
  }
 
+ intel_shared_dpll_start_config(dev_priv, modeset_pipes | disable_pipes);
+
+ for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
+ ret = dev_priv->display.crtc_compute_clock(&intel_crtc->base);
+ if (ret)
+ goto done;
+ }
+
  for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
  intel_crtc_disable(&intel_crtc->base);
 

@@ -11015,6 +11066,8 @@ static int __intel_set_mode(struct drm_crtc *crtc,
  &pipe_config->adjusted_mode);
  }
 
+ intel_shared_dpll_commit(dev_priv);
+
  /* Only after disabling all output pipelines that will be changed can we
  * update the the output configuration. */
  intel_modeset_update_state(dev, prepare_pipes);

@@ -12580,6 +12633,7 @@ static void intel_init_display(struct drm_device *dev)
  if (HAS_DDI(dev)) {
  dev_priv->display.get_pipe_config = haswell_get_pipe_config;
  dev_priv->display.get_plane_config = ironlake_get_plane_config;
+ dev_priv->display.crtc_compute_clock = haswell_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
  dev_priv->display.crtc_enable = haswell_crtc_enable;
  dev_priv->display.crtc_disable = haswell_crtc_disable;

@@ -12593,6 +12647,7 @@ static void intel_init_display(struct drm_device *dev)
  } else if (HAS_PCH_SPLIT(dev)) {
  dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
  dev_priv->display.get_plane_config = ironlake_get_plane_config;
+ dev_priv->display.crtc_compute_clock = ironlake_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
  dev_priv->display.crtc_enable = ironlake_crtc_enable;
  dev_priv->display.crtc_disable = ironlake_crtc_disable;

@@ -12602,6 +12657,7 @@ static void intel_init_display(struct drm_device *dev)
  } else if (IS_VALLEYVIEW(dev)) {
  dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
  dev_priv->display.get_plane_config = i9xx_get_plane_config;
+ dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
  dev_priv->display.crtc_enable = valleyview_crtc_enable;
  dev_priv->display.crtc_disable = i9xx_crtc_disable;

@@ -12611,6 +12667,7 @@ static void intel_init_display(struct drm_device *dev)
  } else {
  dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
  dev_priv->display.get_plane_config = i9xx_get_plane_config;
+ dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
  dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
  dev_priv->display.crtc_enable = i9xx_crtc_enable;
  dev_priv->display.crtc_disable = i9xx_crtc_disable;
--

1.8.3.2

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Signature · 7 lines

Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx