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

Daniel Vetter daniel at ffwll.ch
Tue May 12 03:11:36 PDT 2015


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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list