[Intel-gfx] [PATCH 7/8] drm/i915: create pipe_config->dpll for clock state

Jesse Barnes jbarnes at virtuousgeek.org
Tue Apr 2 23:14:23 CEST 2013


On Thu, 28 Mar 2013 10:42:02 +0100
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:

> Clock computations and handling are highly encoder specific, both in
> the optimal clock selection and also in which clocks to use and when
> sharing of clocks is possible.
> 
> So the best place to do this is somewhere in the encoders, with a
> generic fallback for those encoders without special needs. To facility
> this, add a pipe_config->clocks_set boolean.
> 
> This patch here is only prep work, it simply sets the computed clock
> values in pipe_config->dpll, and uses that data in the hw clock
> setting functions.
> 
> Haswell code isn't touched, simply because Haswell clocks work much
> different and need their own infrastructure (with probably a
> Haswell-specific config->ddi_clock substruct).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 155 +++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  12 +++
>  2 files changed, 95 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c9e873e..5319133 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>  	return refclk;
>  }
>  
> -static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
> -				      intel_clock_t *clock)
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
>  {
> +	unsigned dotclock = crtc->config.adjusted_mode.clock;
> +	struct dpll *clock = &crtc->config.dpll;
> +
>  	/* SDVO TV has fixed PLL values depend on its clock range,
>  	   this mirrors vbios setting. */
> -	if (adjusted_mode->clock >= 100000
> -	    && adjusted_mode->clock < 140500) {
> +	if (dotclock >= 100000 && dotclock < 140500) {
>  		clock->p1 = 2;
>  		clock->p2 = 10;
>  		clock->n = 3;
>  		clock->m1 = 16;
>  		clock->m2 = 8;
> -	} else if (adjusted_mode->clock >= 140500
> -		   && adjusted_mode->clock <= 200000) {
> +	} else if (dotclock >= 140500 && dotclock <= 200000) {
>  		clock->p1 = 1;
>  		clock->p2 = 10;
>  		clock->n = 6;
>  		clock->m1 = 12;
>  		clock->m2 = 8;
>  	}
> +
> +	crtc->config.clock_set = true;
>  }
>  
> -static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
> -				     intel_clock_t *clock,
> +static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>  				     intel_clock_t *reduced_clock)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 fp, fp2 = 0;
> +	struct dpll *clock = &crtc->config.dpll;
>  
>  	if (IS_PINEVIEW(dev)) {
>  		fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2;
> @@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
>  
>  	I915_WRITE(FP0(pipe), fp);
>  
> -	intel_crtc->lowfreq_avail = false;
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	crtc->lowfreq_avail = false;
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  	    reduced_clock && i915_powersave) {
>  		I915_WRITE(FP1(pipe), fp2);
> -		intel_crtc->lowfreq_avail = true;
> +		crtc->lowfreq_avail = true;
>  	} else {
>  		I915_WRITE(FP1(pipe), fp);
>  	}
> @@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
>  		intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
>  }
>  
> -static void vlv_update_pll(struct drm_crtc *crtc,
> -			   intel_clock_t *clock, intel_clock_t *reduced_clock,
> -			   int num_connectors)
> +static void vlv_update_pll(struct intel_crtc *crtc)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll, mdiv, pdiv;
>  	u32 bestn, bestm1, bestm2, bestp1, bestp2;
>  	bool is_sdvo;
> @@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>  	mutex_lock(&dev_priv->dpio_lock);
>  
> -	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  	dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> @@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	I915_WRITE(DPLL(pipe), dpll);
>  	POSTING_READ(DPLL(pipe));
>  
> -	bestn = clock->n;
> -	bestm1 = clock->m1;
> -	bestm2 = clock->m2;
> -	bestp1 = clock->p1;
> -	bestp2 = clock->p2;
> +	bestn = crtc->config.dpll.n;
> +	bestm1 = crtc->config.dpll.m1;
> +	bestm2 = crtc->config.dpll.m2;
> +	bestp1 = crtc->config.dpll.p1;
> +	bestp2 = crtc->config.dpll.p2;
>  
>  	/*
>  	 * In Valleyview PLL and program lane counter registers are exposed
> @@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>  	intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>  
> -	if (intel_crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(intel_crtc);
> +	if (crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	temp = 0;
>  	if (is_sdvo) {
>  		temp = 0;
> -		if (intel_crtc->config.pixel_multiplier > 1) {
> -			temp = (intel_crtc->config.pixel_multiplier - 1)
> +		if (crtc->config.pixel_multiplier > 1) {
> +			temp = (crtc->config.pixel_multiplier - 1)
>  				<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  		}
>  	}
> @@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL_MD(pipe));
>  
>  	/* Now program lane control registers */
> -	if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> -			|| intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> -	{
> +	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
> +	   || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
>  		temp = 0x1000C4;
>  		if(pipe == 1)
>  			temp |= (1 << 21);
>  		intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
>  	}
> -	if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> -	{
> +
> +	if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
>  		temp = 0x1000C4;
>  		if(pipe == 1)
>  			temp |= (1 << 21);
> @@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  	mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> -static void i9xx_update_pll(struct drm_crtc *crtc,
> -			    intel_clock_t *clock, intel_clock_t *reduced_clock,
> +static void i9xx_update_pll(struct intel_crtc *crtc,
> +			    intel_clock_t *reduced_clock,
>  			    int num_connectors)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll;
>  	bool is_sdvo;
> +	struct dpll *clock = &crtc->config.dpll;
>  
> -	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +	i9xx_update_pll_dividers(crtc, reduced_clock);
>  
> -	is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -		intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +	is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +		intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
>  		dpll |= DPLLB_MODE_LVDS;
>  	else
>  		dpll |= DPLLB_MODE_DAC_SERIAL;
>  
>  	if (is_sdvo) {
> -		if ((intel_crtc->config.pixel_multiplier > 1) &&
> +		if ((crtc->config.pixel_multiplier > 1) &&
>  		    (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
> -			dpll |= (intel_crtc->config.pixel_multiplier - 1)
> +			dpll |= (crtc->config.pixel_multiplier - 1)
>  				<< SDVO_MULTIPLIER_SHIFT_HIRES;
>  		}
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  	}
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
>  		dpll |= DPLL_DVO_HIGH_SPEED;
>  
>  	/* compute bitmask from p1 value */
> @@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>  
> -	if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +	if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
> -	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>  		/* XXX: just matching BIOS for now */
>  		/*	dpll |= PLL_REF_INPUT_TVCLKINBC; */
>  		dpll |= 3;
> -	else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL(pipe));
>  	udelay(150);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> -	if (intel_crtc->config.has_dp_encoder)
> -		intel_dp_set_m_n(intel_crtc);
> +	if (crtc->config.has_dp_encoder)
> +		intel_dp_set_m_n(crtc);
>  
>  	I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  		u32 temp = 0;
>  		if (is_sdvo) {
>  			temp = 0;
> -			if (intel_crtc->config.pixel_multiplier > 1) {
> -				temp = (intel_crtc->config.pixel_multiplier - 1)
> +			if (crtc->config.pixel_multiplier > 1) {
> +				temp = (crtc->config.pixel_multiplier - 1)
>  					<< DPLL_MD_UDI_MULTIPLIER_SHIFT;
>  			}
>  		}
> @@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>  	}
>  }
>  
> -static void i8xx_update_pll(struct drm_crtc *crtc,
> +static void i8xx_update_pll(struct intel_crtc *crtc,
>  			    struct drm_display_mode *adjusted_mode,
> -			    intel_clock_t *clock, intel_clock_t *reduced_clock,
> +			    intel_clock_t *reduced_clock,
>  			    int num_connectors)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_encoder *encoder;
> -	int pipe = intel_crtc->pipe;
> +	int pipe = crtc->pipe;
>  	u32 dpll;
> +	struct dpll *clock = &crtc->config.dpll;
>  
> -	i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +	i9xx_update_pll_dividers(crtc, reduced_clock);
>  
>  	dpll = DPLL_VGA_MODE_DIS;
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
>  		dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>  	} else {
>  		if (clock->p1 == 2)
> @@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>  			dpll |= PLL_P2_DIVIDE_BY_4;
>  	}
>  
> -	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>  		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
> @@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>  	POSTING_READ(DPLL(pipe));
>  	udelay(150);
>  
> -	for_each_encoder_on_crtc(dev, crtc, encoder)
> +	for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>  		if (encoder->pre_pll_enable)
>  			encoder->pre_pll_enable(encoder);
>  
> @@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  						    &clock,
>  						    &reduced_clock);
>  	}
> +	/* Compat-code for transition, will disappear. */
> +	if (!intel_crtc->config.clock_set) {
> +		intel_crtc->config.dpll.n = clock.n;
> +		intel_crtc->config.dpll.m1 = clock.m1;
> +		intel_crtc->config.dpll.m2 = clock.m2;
> +		intel_crtc->config.dpll.p1 = clock.p1;
> +		intel_crtc->config.dpll.p2 = clock.p2;
> +	}
>  
>  	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> +		i9xx_adjust_sdvo_tv_clock(intel_crtc);
>  
>  	if (IS_GEN2(dev))
> -		i8xx_update_pll(crtc, adjusted_mode, &clock,
> +		i8xx_update_pll(intel_crtc, adjusted_mode,
>  				has_reduced_clock ? &reduced_clock : NULL,
>  				num_connectors);
>  	else if (IS_VALLEYVIEW(dev))
> -		vlv_update_pll(crtc, &clock,
> -				has_reduced_clock ? &reduced_clock : NULL,
> -				num_connectors);
> +		vlv_update_pll(intel_crtc);
>  	else
> -		i9xx_update_pll(crtc, &clock,
> +		i9xx_update_pll(intel_crtc,
>  				has_reduced_clock ? &reduced_clock : NULL,
>  				num_connectors);
>  
> @@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
>  	}
>  
>  	if (is_sdvo && is_tv)
> -		i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> +		i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
>  
>  	return true;
>  }
> @@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  		DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  		return -EINVAL;
>  	}
> +	/* Compat-code for transition, will disappear. */
> +	if (!intel_crtc->config.clock_set) {
> +		intel_crtc->config.dpll.n = clock.n;
> +		intel_crtc->config.dpll.m1 = clock.m1;
> +		intel_crtc->config.dpll.m2 = clock.m2;
> +		intel_crtc->config.dpll.p1 = clock.p1;
> +		intel_crtc->config.dpll.p2 = clock.p2;
> +	}
>  
>  	/* Ensure that the cursor is valid for the new mode before changing... */
>  	intel_crtc_update_cursor(crtc, true);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2a86a12..d27885a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -196,6 +196,18 @@ struct intel_crtc_config {
>  	 * accordingly. */
>  	bool has_dp_encoder;
>  	bool dither;
> +
> +	/* Controls for the clock computation, to override various stages. */
> +	bool clock_set;
> +
> +	/* Settings for the intel dpll used on pretty much everything but
> +	 * haswell. */
> +	struct dpll {
> +		unsigned n;
> +		unsigned m1, m2;
> +		unsigned p1, p2;
> +	} dpll;
> +
>  	int pipe_bpp;
>  	struct intel_link_m_n dp_m_n;
>  	/**

This one's hard to review since you mixed in a drm_crtc->intel_crtc
function arg change.

I'd rather have that split out, but it looks ok.

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center



More information about the Intel-gfx mailing list