[34/42] drm/i915: get rid of crtc->config in intel_display.c, part 1
diff mbox

Message ID 1431354318-11995-35-git-send-email-maarten.lankhorst@linux.intel.com
State New
Headers show

Commit Message

Maarten Lankhorst May 11, 2015, 2:25 p.m. UTC
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@linux.intel.com>
---
 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(-)

Comments

Daniel Vetter May 12, 2015, 10:11 a.m. UTC | #1
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@linux.intel.com>
> ---
>  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?
>  
> -	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?

>  	}
>  
>  	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?
-Daniel

>  
> @@ -14798,7 +14798,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev,
>  		pll->active = 0;
>  		pll->config.crtc_mask = 0;
>  		for_each_intel_crtc(dev, crtc) {
> -			if (crtc->base.state->active && intel_crtc_to_shared_dpll(crtc) == pll) {
> +			if (crtc->base.state->active &&
> +			    i == to_intel_crtc_state(crtc->base.state)->shared_dpll) {
>  				pll->active++;
>  				pll->config.crtc_mask |= 1 << crtc->pipe;
>  			}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1a0b0cd857c3..7979a2f6838c 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1058,10 +1058,8 @@ bool intel_wm_need_update(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
>  
>  /* shared dpll functions */
> -struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
>  void assert_shared_dpll(struct drm_i915_private *dev_priv,
> -			struct intel_shared_dpll *pll,
> -			bool state);
> +			enum intel_dpll_id, bool state);
>  #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
>  #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
>  struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 8aaaa24144f0..52788ed6f775 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -149,7 +149,7 @@ static void intel_pre_enable_lvds(struct intel_encoder *encoder)
>  	if (HAS_PCH_SPLIT(dev)) {
>  		assert_fdi_rx_pll_disabled(dev_priv, pipe);
>  		assert_shared_dpll_disabled(dev_priv,
> -					    intel_crtc_to_shared_dpll(crtc));
> +					    pipe_config->shared_dpll);
>  	} else {
>  		assert_pll_disabled(dev_priv, pipe);
>  	}
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Maarten Lankhorst May 12, 2015, 2:13 p.m. UTC | #2
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@linux.intel.com>
>> ---
>>  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
Daniel Vetter May 12, 2015, 5:01 p.m. UTC | #3
On Tue, May 12, 2015 at 04:13:54PM +0200, Maarten Lankhorst wrote:
> 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@linux.intel.com>
> >> ---
> >>  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.

Yeah that makes sense where we need to pass in the right pipe_config. But
in the _enable functions just using to_intel_crtc_state(crtc->state) is
ok, and just doing that would cut down on the overall diff. Same idea as
not passing pipe_config around explicitly for other functions only called
from _enable hooks (and other places where crtc->state is the right
state).

I'm ok with open-coding the dpll lookup in compute/disable/get_config functions.

> 
> >>  
> >> -	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.

i9xx_update_primary_plane doesn't have any shared dpll code, which means
this s/crtc->config/pipe_config replacement should probably be in a
different patch.

> >>  	}
> >>  
> >>  	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.

If you do, separate patch please. Yes I'm asking that a lot, but I'm also
super-scared of all these changes here ;-)
-Daniel

Patch
diff mbox

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;
 
 	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];
 
-	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;
 	}
 
 	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);
 	}
 
@@ -14798,7 +14798,8 @@  static void intel_modeset_readout_hw_state(struct drm_device *dev,
 		pll->active = 0;
 		pll->config.crtc_mask = 0;
 		for_each_intel_crtc(dev, crtc) {
-			if (crtc->base.state->active && intel_crtc_to_shared_dpll(crtc) == pll) {
+			if (crtc->base.state->active &&
+			    i == to_intel_crtc_state(crtc->base.state)->shared_dpll) {
 				pll->active++;
 				pll->config.crtc_mask |= 1 << crtc->pipe;
 			}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a0b0cd857c3..7979a2f6838c 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1058,10 +1058,8 @@  bool intel_wm_need_update(struct drm_plane *plane,
 			  struct drm_plane_state *state);
 
 /* shared dpll functions */
-struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc);
 void assert_shared_dpll(struct drm_i915_private *dev_priv,
-			struct intel_shared_dpll *pll,
-			bool state);
+			enum intel_dpll_id, bool state);
 #define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
 #define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
 struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 8aaaa24144f0..52788ed6f775 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -149,7 +149,7 @@  static void intel_pre_enable_lvds(struct intel_encoder *encoder)
 	if (HAS_PCH_SPLIT(dev)) {
 		assert_fdi_rx_pll_disabled(dev_priv, pipe);
 		assert_shared_dpll_disabled(dev_priv,
-					    intel_crtc_to_shared_dpll(crtc));
+					    pipe_config->shared_dpll);
 	} else {
 		assert_pll_disabled(dev_priv, pipe);
 	}