Op 12-05-15 om 12:11 schreef Daniel Vetter: > On Mon, May 11, 2015 at 04:25:10PM +0200, Maarten Lankhorst wrote: >> Removed some occurences, roughly based on where the errors of >> removing crtc->config occured. Because it's used a lot in this >> file the changes are done in passes. >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_display.c | 205 ++++++++++++++++++----------------- >> drivers/gpu/drm/i915/intel_drv.h | 4 +- >> drivers/gpu/drm/i915/intel_lvds.c | 2 +- >> 3 files changed, 105 insertions(+), 106 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index 004067bd0b6c..fb2ecb65aaaa 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -1141,29 +1141,20 @@ static void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state) >> #define assert_dsi_pll_enabled(d) assert_dsi_pll(d, true) >> #define assert_dsi_pll_disabled(d) assert_dsi_pll(d, false) >> >> -struct intel_shared_dpll * >> -intel_crtc_to_shared_dpll(struct intel_crtc *crtc) >> -{ >> - struct drm_i915_private *dev_priv = crtc->base.dev->dev_private; >> - >> - if (crtc->config->shared_dpll < 0) >> - return NULL; >> - >> - return &dev_priv->shared_dplls[crtc->config->shared_dpll]; >> -} >> - >> /* For ILK+ */ >> void assert_shared_dpll(struct drm_i915_private *dev_priv, >> - struct intel_shared_dpll *pll, >> + enum intel_dpll_id shared_dpll, >> bool state) >> { >> - bool cur_state; >> struct intel_dpll_hw_state hw_state; >> + struct intel_shared_dpll *pll; >> + bool cur_state; >> >> - if (WARN (!pll, >> + if (WARN(shared_dpll < 0, >> "asserting DPLL %s with no DPLL\n", state_string(state))) >> return; >> >> + pll = &dev_priv->shared_dplls[shared_dpll]; >> cur_state = pll->get_hw_state(dev_priv, pll, &hw_state); >> I915_STATE_WARN(cur_state != state, >> "%s assertion failure (expected %s, current %s)\n", >> @@ -1691,7 +1682,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc, struct intel_crtc_state *pi >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> int reg = DPLL(crtc->pipe); >> - u32 dpll = crtc->config->dpll_hw_state.dpll; >> + u32 dpll = pipe_config->dpll_hw_state.dpll; > Lots of your patches are sprinkled with unrelated crtc->config -> > pipe_config conversions. Or just plain rolling out of a local variable > instead of accessing some other pointer. That makes it fairly hard to > review them for a given topic (like shared dpll here) since it's never > clear whether it really includes everything, and what's the reason for all > the other hunks. > >> >> assert_pipe_disabled(dev_priv, pipe_config->cpu_transcoder, crtc->pipe); >> >> @@ -1721,7 +1712,7 @@ static void i9xx_enable_pll(struct intel_crtc *crtc, struct intel_crtc_state *pi >> >> if (INTEL_INFO(dev)->gen >= 4) { >> I915_WRITE(DPLL_MD(crtc->pipe), >> - crtc->config->dpll_hw_state.dpll_md); >> + pipe_config->dpll_hw_state.dpll_md); >> } else { >> /* The pixel multiplier can only be updated once the >> * DPLL is enabled and the clocks are stable. >> @@ -1859,20 +1850,19 @@ void vlv_wait_port_ready(struct drm_i915_private *dev_priv, >> port_name(dport->port), I915_READ(dpll_reg) & port_mask, expected_mask); >> } >> >> -static void intel_prepare_shared_dpll(struct intel_crtc *crtc) >> +static void intel_prepare_shared_dpll(struct drm_i915_private *dev_priv, >> + enum intel_dpll_id shared_dpll) >> { >> - struct drm_device *dev = crtc->base.dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); >> + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[shared_dpll]; > Why change anything here? This is called from enable hooks, so can keep on > accessing intel_crtc->state ... or not? I got rid of intel_crtc_to_shared_dpll, because it only required pipe_config->shared_dpll I felt it could be passed as argument instead. >> >> - if (WARN_ON(pll == NULL)) >> + if (WARN_ON(shared_dpll < 0)) >> return; >> >> WARN_ON(!pll->config.crtc_mask); >> if (pll->active == 0) { >> DRM_DEBUG_DRIVER("setting up %s\n", pll->name); >> WARN_ON(pll->on); >> - assert_shared_dpll_disabled(dev_priv, pll); >> + assert_shared_dpll_disabled(dev_priv, shared_dpll); >> >> pll->mode_set(dev_priv, pll); >> } >> @@ -1886,25 +1876,23 @@ static void intel_prepare_shared_dpll(struct intel_crtc *crtc) >> * The PCH PLL needs to be enabled before the PCH transcoder, since it >> * drives the transcoder clock. >> */ >> -static void intel_enable_shared_dpll(struct intel_crtc *crtc) >> +static void intel_enable_shared_dpll(struct drm_i915_private *dev_priv, >> + enum intel_dpll_id shared_dpll) >> { >> - struct drm_device *dev = crtc->base.dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); >> + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[shared_dpll]; >> >> - if (WARN_ON(pll == NULL)) >> + if (WARN_ON(shared_dpll < 0)) >> return; >> >> if (WARN_ON(pll->config.crtc_mask == 0)) >> return; >> >> - DRM_DEBUG_KMS("enable %s (active %d, on? %d) for crtc %d\n", >> - pll->name, pll->active, pll->on, >> - crtc->base.base.id); >> + DRM_DEBUG_KMS("enable %s (active %d, on? %d)\n", >> + pll->name, pll->active, pll->on); >> >> if (pll->active++) { >> WARN_ON(!pll->on); >> - assert_shared_dpll_enabled(dev_priv, pll); >> + assert_shared_dpll_enabled(dev_priv, shared_dpll); >> return; >> } >> WARN_ON(pll->on); >> @@ -1916,30 +1904,29 @@ static void intel_enable_shared_dpll(struct intel_crtc *crtc) >> pll->on = true; >> } >> >> -static void intel_disable_shared_dpll(struct intel_crtc *crtc) >> +static void intel_disable_shared_dpll(struct drm_i915_private *dev_priv, >> + enum intel_dpll_id shared_dpll) >> { >> - struct drm_device *dev = crtc->base.dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc); >> + struct intel_shared_dpll *pll = &dev_priv->shared_dplls[shared_dpll]; >> + >> + if (shared_dpll < 0) >> + return; >> >> /* PCH only available on ILK+ */ >> - BUG_ON(INTEL_INFO(dev)->gen < 5); >> - if (WARN_ON(pll == NULL)) >> - return; >> + BUG_ON(dev_priv->info.gen < 5); >> >> if (WARN_ON(pll->config.crtc_mask == 0)) >> return; >> >> - DRM_DEBUG_KMS("disable %s (active %d, on? %d) for crtc %d\n", >> - pll->name, pll->active, pll->on, >> - crtc->base.base.id); >> + DRM_DEBUG_KMS("disable %s (active %d, on? %d)\n", >> + pll->name, pll->active, pll->on); >> >> if (WARN_ON(pll->active == 0)) { >> - assert_shared_dpll_disabled(dev_priv, pll); >> + assert_shared_dpll_disabled(dev_priv, shared_dpll); >> return; >> } >> >> - assert_shared_dpll_enabled(dev_priv, pll); >> + assert_shared_dpll_enabled(dev_priv, shared_dpll); >> WARN_ON(!pll->on); >> if (--pll->active) >> return; >> @@ -1964,8 +1951,7 @@ static void ironlake_enable_pch_transcoder(struct drm_i915_private *dev_priv, >> BUG_ON(!HAS_PCH_SPLIT(dev)); >> >> /* Make sure PCH DPLL is enabled */ >> - assert_shared_dpll_enabled(dev_priv, >> - intel_crtc_to_shared_dpll(intel_crtc)); >> + assert_shared_dpll_enabled(dev_priv, pipe_config->shared_dpll); >> >> /* FDI must be feeding us bits for PCH ports */ >> assert_fdi_tx_enabled(dev_priv, pipe_config->cpu_transcoder, pipe); >> @@ -2126,7 +2112,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc) >> else >> assert_pll_enabled(dev_priv, pipe); >> else { >> - if (crtc->config->has_pch_encoder) { >> + if (pipe_config->has_pch_encoder) { >> /* if driving the PCH, we need FDI enabled */ >> assert_fdi_rx_pll_enabled(dev_priv, pch_transcoder); >> assert_fdi_tx_pll_enabled(dev_priv, >> @@ -2622,6 +2608,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, >> bool visible = to_intel_plane_state(primary->state)->visible; >> struct drm_i915_gem_object *obj; >> int plane = intel_crtc->plane; >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->state); >> unsigned long linear_offset; >> u32 dspcntr; >> u32 reg = DSPCNTR(plane); >> @@ -2655,13 +2643,13 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, >> * which should always be the user's requested size. >> */ >> I915_WRITE(DSPSIZE(plane), >> - ((intel_crtc->config->pipe_src_h - 1) << 16) | >> - (intel_crtc->config->pipe_src_w - 1)); >> + ((pipe_config->pipe_src_h - 1) << 16) | >> + (pipe_config->pipe_src_w - 1)); >> I915_WRITE(DSPPOS(plane), 0); >> } else if (IS_CHERRYVIEW(dev) && plane == PLANE_B) { >> I915_WRITE(PRIMSIZE(plane), >> - ((intel_crtc->config->pipe_src_h - 1) << 16) | >> - (intel_crtc->config->pipe_src_w - 1)); >> + ((pipe_config->pipe_src_h - 1) << 16) | >> + (pipe_config->pipe_src_w - 1)); >> I915_WRITE(PRIMPOS(plane), 0); >> I915_WRITE(PRIMCNSTALPHA(plane), 0); >> } >> @@ -2719,14 +2707,14 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc, >> if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) { >> dspcntr |= DISPPLANE_ROTATE_180; >> >> - x += (intel_crtc->config->pipe_src_w - 1); >> - y += (intel_crtc->config->pipe_src_h - 1); >> + x += (pipe_config->pipe_src_w - 1); >> + y += (pipe_config->pipe_src_h - 1); >> >> /* Finding the last pixel of the last line of the display >> data and adding to linear_offset*/ >> linear_offset += >> - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] + >> - (intel_crtc->config->pipe_src_w - 1) * pixel_size; >> + (pipe_config->pipe_src_h - 1) * fb->pitches[0] + >> + (pipe_config->pipe_src_w - 1) * pixel_size; > Unrelated changes above, or do I miss something? What's unrelated? But yeah I mostly split it up by fixing things up until it started compiling again. I should split the display stuff up further. >> } >> >> I915_WRITE(reg, dspcntr); >> @@ -2818,17 +2806,20 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc, >> fb->pitches[0]); >> linear_offset -= intel_crtc->dspaddr_offset; >> if (crtc->primary->state->rotation == BIT(DRM_ROTATE_180)) { >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->state); >> + >> dspcntr |= DISPPLANE_ROTATE_180; >> >> if (!IS_HASWELL(dev) && !IS_BROADWELL(dev)) { >> - x += (intel_crtc->config->pipe_src_w - 1); >> - y += (intel_crtc->config->pipe_src_h - 1); >> + x += (pipe_config->pipe_src_w - 1); >> + y += (pipe_config->pipe_src_h - 1); >> >> /* Finding the last pixel of the last line of the display >> data and adding to linear_offset*/ >> linear_offset += >> - (intel_crtc->config->pipe_src_h - 1) * fb->pitches[0] + >> - (intel_crtc->config->pipe_src_w - 1) * pixel_size; >> + (pipe_config->pipe_src_h - 1) * fb->pitches[0] + >> + (pipe_config->pipe_src_w - 1) * pixel_size; >> } >> } >> >> @@ -2901,12 +2892,13 @@ void skl_detach_scalers(struct intel_crtc *intel_crtc) >> struct intel_crtc_scaler_state *scaler_state; >> int i; >> >> - if (!intel_crtc || !intel_crtc->config) >> + if (!intel_crtc) >> return; >> >> dev = intel_crtc->base.dev; >> dev_priv = dev->dev_private; >> - scaler_state = &intel_crtc->config->scaler_state; >> + scaler_state = >> + &to_intel_crtc_state(intel_crtc->base.state)->scaler_state; >> >> /* loop through and disable scalers that aren't in use */ >> for (i = 0; i < intel_crtc->num_scalers; i++) { >> @@ -3298,6 +3290,8 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) >> struct drm_device *dev = crtc->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> const struct drm_display_mode *adjusted_mode; >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->base.state); >> >> if (!i915.fastboot) >> return; >> @@ -3316,20 +3310,20 @@ static void intel_update_pipe_size(struct intel_crtc *crtc) >> * then update the pipesrc and pfit state, even on the flip path. >> */ >> >> - adjusted_mode = &crtc->config->base.adjusted_mode; >> + adjusted_mode = &pipe_config->base.adjusted_mode; >> >> I915_WRITE(PIPESRC(crtc->pipe), >> ((adjusted_mode->crtc_hdisplay - 1) << 16) | >> (adjusted_mode->crtc_vdisplay - 1)); >> - if (!crtc->config->pch_pfit.enabled && >> + if (!pipe_config->pch_pfit.enabled && >> (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) || >> intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) { >> I915_WRITE(PF_CTL(crtc->pipe), 0); >> I915_WRITE(PF_WIN_POS(crtc->pipe), 0); >> I915_WRITE(PF_WIN_SZ(crtc->pipe), 0); >> } >> - crtc->config->pipe_src_w = adjusted_mode->crtc_hdisplay; >> - crtc->config->pipe_src_h = adjusted_mode->crtc_vdisplay; >> + pipe_config->pipe_src_w = adjusted_mode->crtc_hdisplay; >> + pipe_config->pipe_src_h = adjusted_mode->crtc_vdisplay; >> } >> >> static void intel_fdi_normal_train(struct drm_crtc *crtc) >> @@ -3379,6 +3373,8 @@ static void ironlake_fdi_link_train(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); >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->state); >> int pipe = intel_crtc->pipe; >> u32 reg, temp, tries; >> >> @@ -3396,7 +3392,7 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc) >> reg = FDI_TX_CTL(pipe); >> temp = I915_READ(reg); >> temp &= ~FDI_DP_PORT_WIDTH_MASK; >> - temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes); >> + temp |= FDI_DP_PORT_WIDTH(pipe_config->fdi_lanes); >> temp &= ~FDI_LINK_TRAIN_NONE; >> temp |= FDI_LINK_TRAIN_PATTERN_1; >> I915_WRITE(reg, temp | FDI_TX_ENABLE); >> @@ -3476,6 +3472,8 @@ static void gen6_fdi_link_train(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); >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->state); >> int pipe = intel_crtc->pipe; >> u32 reg, temp, i, retry; >> >> @@ -3494,7 +3492,7 @@ static void gen6_fdi_link_train(struct drm_crtc *crtc) >> reg = FDI_TX_CTL(pipe); >> temp = I915_READ(reg); >> temp &= ~FDI_DP_PORT_WIDTH_MASK; >> - temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes); >> + temp |= FDI_DP_PORT_WIDTH(pipe_config->fdi_lanes); >> temp &= ~FDI_LINK_TRAIN_NONE; >> temp |= FDI_LINK_TRAIN_PATTERN_1; >> temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK; >> @@ -3608,6 +3606,8 @@ static void ivb_manual_fdi_link_train(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); >> + struct intel_crtc_state *pipe_config = >> + to_intel_crtc_state(crtc->state); >> int pipe = intel_crtc->pipe; >> u32 reg, temp, i, j; >> >> @@ -3645,7 +3645,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc) >> reg = FDI_TX_CTL(pipe); >> temp = I915_READ(reg); >> temp &= ~FDI_DP_PORT_WIDTH_MASK; >> - temp |= FDI_DP_PORT_WIDTH(intel_crtc->config->fdi_lanes); >> + temp |= FDI_DP_PORT_WIDTH(pipe_config->fdi_lanes); >> temp |= FDI_LINK_TRAIN_PATTERN_1_IVB; >> temp &= ~FDI_LINK_TRAIN_VOL_EMP_MASK; >> temp |= snb_b_fdi_train_param[j/2]; >> @@ -3914,11 +3914,10 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) >> } >> >> /* Program iCLKIP clock to the desired frequency */ >> -static void lpt_program_iclkip(struct drm_crtc *crtc) >> +static void lpt_program_iclkip(struct drm_i915_private *dev_priv, >> + struct intel_crtc_state *pipe_config) >> { >> - struct drm_device *dev = crtc->dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - int clock = to_intel_crtc(crtc)->config->base.adjusted_mode.crtc_clock; >> + int clock = pipe_config->base.adjusted_mode.crtc_clock; >> u32 divsel, phaseinc, auxdiv, phasedir = 0; >> u32 temp; >> >> @@ -4002,13 +4001,10 @@ static void lpt_program_iclkip(struct drm_crtc *crtc) >> mutex_unlock(&dev_priv->dpio_lock); >> } >> >> -static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, >> +static void ironlake_pch_transcoder_set_timings(struct drm_i915_private *dev_priv, >> + enum transcoder cpu_transcoder, >> enum pipe pch_transcoder) >> { >> - struct drm_device *dev = crtc->base.dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - enum transcoder cpu_transcoder = crtc->config->cpu_transcoder; >> - >> I915_WRITE(PCH_TRANS_HTOTAL(pch_transcoder), >> I915_READ(HTOTAL(cpu_transcoder))); >> I915_WRITE(PCH_TRANS_HBLANK(pch_transcoder), >> @@ -4026,7 +4022,8 @@ static void ironlake_pch_transcoder_set_timings(struct intel_crtc *crtc, >> I915_READ(VSYNCSHIFT(cpu_transcoder))); >> } >> >> -static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) >> +static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, >> + bool enable) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> uint32_t temp; >> @@ -4047,15 +4044,15 @@ static void cpt_set_fdi_bc_bifurcation(struct drm_device *dev, bool enable) >> POSTING_READ(SOUTH_CHICKEN1); >> } >> >> -static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) >> +static void ivybridge_update_fdi_bc_bifurcation(struct drm_device *dev, >> + struct intel_crtc *intel_crtc, >> + struct intel_crtc_state *pipe_config) >> { >> - struct drm_device *dev = intel_crtc->base.dev; >> - >> switch (intel_crtc->pipe) { >> case PIPE_A: >> break; >> case PIPE_B: >> - if (intel_crtc->config->fdi_lanes > 2) >> + if (pipe_config->fdi_lanes > 2) >> cpt_set_fdi_bc_bifurcation(dev, false); >> else >> cpt_set_fdi_bc_bifurcation(dev, true); >> @@ -4078,7 +4075,8 @@ static void ivybridge_update_fdi_bc_bifurcation(struct intel_crtc *intel_crtc) >> * - DP transcoding bits >> * - transcoder >> */ >> -static void ironlake_pch_enable(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config) >> +static void ironlake_pch_enable(struct drm_crtc *crtc, >> + struct intel_crtc_state *pipe_config) >> { >> struct drm_device *dev = crtc->dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> @@ -4089,7 +4087,8 @@ static void ironlake_pch_enable(struct drm_crtc *crtc, struct intel_crtc_state * >> assert_pch_transcoder_disabled(dev_priv, pipe); >> >> if (IS_IVYBRIDGE(dev)) >> - ivybridge_update_fdi_bc_bifurcation(intel_crtc); >> + ivybridge_update_fdi_bc_bifurcation(dev, intel_crtc, >> + pipe_config); >> >> /* Write the TU size bits before fdi link training, so that error >> * detection works. */ >> @@ -4121,11 +4120,12 @@ static void ironlake_pch_enable(struct drm_crtc *crtc, struct intel_crtc_state * >> * Note that enable_shared_dpll tries to do the right thing, but >> * get_shared_dpll unconditionally resets the pll - we need that to have >> * the right LVDS enable sequence. */ >> - intel_enable_shared_dpll(intel_crtc); >> + intel_enable_shared_dpll(dev_priv, pipe_config->shared_dpll); >> >> /* set transcoder timing, panel must allow it */ >> assert_panel_unlocked(dev_priv, pipe); >> - ironlake_pch_transcoder_set_timings(intel_crtc, pipe); >> + ironlake_pch_transcoder_set_timings(dev_priv, >> + pipe_config->cpu_transcoder, pipe); >> >> intel_fdi_normal_train(crtc); >> >> @@ -4166,19 +4166,17 @@ static void ironlake_pch_enable(struct drm_crtc *crtc, struct intel_crtc_state * >> ironlake_enable_pch_transcoder(dev_priv, pipe, pipe_config); >> } >> >> -static void lpt_pch_enable(struct drm_crtc *crtc) >> +static void lpt_pch_enable(struct drm_i915_private *dev_priv, >> + struct intel_crtc_state *pipe_config) >> { >> - struct drm_device *dev = crtc->dev; >> - struct drm_i915_private *dev_priv = dev->dev_private; >> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> - enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder; >> + enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; >> >> assert_pch_transcoder_disabled(dev_priv, TRANSCODER_A); >> >> - lpt_program_iclkip(crtc); >> + lpt_program_iclkip(dev_priv, pipe_config); >> >> /* Set transcoder timing. */ >> - ironlake_pch_transcoder_set_timings(intel_crtc, PIPE_A); >> + ironlake_pch_transcoder_set_timings(dev_priv, cpu_transcoder, PIPE_A); >> >> lpt_enable_pch_transcoder(dev_priv, cpu_transcoder); >> } >> @@ -4781,7 +4779,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) >> pipe_config = to_intel_crtc_state(crtc->state); >> >> if (pipe_config->has_pch_encoder) >> - intel_prepare_shared_dpll(intel_crtc); >> + intel_prepare_shared_dpll(dev_priv, >> + pipe_config->shared_dpll); >> >> if (pipe_config->has_dp_encoder) >> intel_dp_set_m_n(intel_crtc, M1_N1); >> @@ -4854,8 +4853,8 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) >> WARN_ON(!crtc->state->enable); >> >> pipe_config = to_intel_crtc_state(crtc->state); >> - if (intel_crtc_to_shared_dpll(intel_crtc)) >> - intel_enable_shared_dpll(intel_crtc); >> + if (pipe_config->shared_dpll >= 0) >> + intel_enable_shared_dpll(dev_priv, pipe_config->shared_dpll); >> >> if (pipe_config->has_dp_encoder) >> intel_dp_set_m_n(intel_crtc, M1_N1); >> @@ -4910,7 +4909,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) >> intel_enable_pipe(intel_crtc); >> >> if (intel_crtc->config->has_pch_encoder) >> - lpt_pch_enable(crtc); >> + lpt_pch_enable(dev_priv, pipe_config); >> >> if (intel_crtc->config->dp_encoder_is_mst) >> intel_ddi_set_vc_payload_alloc(crtc, >> @@ -11929,7 +11928,8 @@ check_shared_dpll_state(struct drm_device *dev) >> pll->on, active); >> >> for_each_intel_crtc(dev, crtc) { >> - if (crtc->base.state->active && intel_crtc_to_shared_dpll(crtc) == pll) >> + if (crtc->base.state->active && >> + to_intel_crtc_state(crtc->base.state)->shared_dpll == i) >> active_crtcs++; >> } >> I915_STATE_WARN(pll->active != active_crtcs, >> @@ -12311,7 +12311,6 @@ static int __intel_set_mode(struct drm_atomic_state *state) >> __intel_set_mode_update_planes(dev, state); >> >> for_each_crtc_in_state(state, crtc, old_crtc_state, i) { >> - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); >> struct intel_crtc_state *pipe_config; >> >> if (!needs_modeset(crtc->state)) >> @@ -12324,8 +12323,7 @@ static int __intel_set_mode(struct drm_atomic_state *state) >> intel_crtc_dpms_overlay_disable(to_intel_crtc(crtc)); >> dev_priv->display.crtc_disable(crtc, pipe_config); >> >> - if (intel_crtc_to_shared_dpll(intel_crtc)) >> - intel_disable_shared_dpll(intel_crtc); >> + intel_disable_shared_dpll(dev_priv, pipe_config->shared_dpll); >> } >> >> /* Only after disabling all output pipelines that will be changed can we >> @@ -12719,7 +12717,9 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv, >> >> /* Make sure no transcoder isn't still depending on us. */ >> for_each_intel_crtc(dev, crtc) { >> - if (intel_crtc_to_shared_dpll(crtc) == pll) >> + int i = pll - dev_priv->shared_dplls; >> + >> + if (to_intel_crtc_state(crtc->base.state)->shared_dpll == i) >> assert_pch_transcoder_disabled(dev_priv, crtc->pipe); >> } > This is called with the with the new config already put in place, but it > should check whether any of the _old_ pipe configs that used the dpll are > really all shut down. This won't misfire since if we shut it down to use > it in some other configuration then those pipes should be ofc off too. > Otoh we do check before enabling the pipe that the dpll is running, hence > I think this is redundant and maybe we could remove > it right away? Removing sounds fine. ~Maarten _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx