[PATCH 5/8] drm/i915: extract set_m_n from ironlake_crtc_mode_set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 12, 2012 at 10:06:33AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> The set_m_n code was spread all over the mode_set function.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |  152 ++++++++++++++++++++--------------
>  1 file changed, 89 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b657416..8d3b9d6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4735,6 +4735,92 @@ static void ironlake_set_pipe_timings(struct intel_crtc *intel_crtc,
>  		   ((mode->hdisplay - 1) << 16) | (mode->vdisplay - 1));
>  }
>  
> +static void ironlake_set_m_n(struct drm_crtc *crtc,
> +			     struct drm_display_mode *mode,
> +			     struct drm_display_mode *adjusted_mode)
> +{
> +	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 pipe pipe = intel_crtc->pipe;
> +	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
> +	struct fdi_m_n m_n = {0};
> +	int target_clock, pixel_multiplier, lane, link_bw;
> +	bool is_dp = false, is_cpu_edp = false;
> +
> +	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> +		switch (intel_encoder->type) {
> +		case INTEL_OUTPUT_DISPLAYPORT:
> +			is_dp = true;
> +			break;
> +		case INTEL_OUTPUT_EDP:
> +			is_dp = true;
> +			if (!intel_encoder_is_pch_edp(&intel_encoder->base))
> +				is_cpu_edp = true;
> +			edp_encoder = intel_encoder;
> +			break;
> +		}
> +	}
> +
> +	/* FDI link */
> +	pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
> +	lane = 0;
> +	/* CPU eDP doesn't require FDI link, so just set DP M/N
> +	   according to current link config */
> +	if (is_cpu_edp) {
> +		intel_edp_link_config(edp_encoder, &lane, &link_bw);
> +	} else {
> +		/* FDI is a binary signal running at ~2.7GHz, encoding
> +		 * each output octet as 10 bits. The actual frequency
> +		 * is stored as a divider into a 100MHz clock, and the
> +		 * mode pixel clock is stored in units of 1KHz.
> +		 * Hence the bw of each lane in terms of the mode signal
> +		 * is:
> +		 */
> +		link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
> +	}
> +
> +	/* [e]DP over FDI requires target mode clock instead of link clock. */
> +	if (edp_encoder)
> +		target_clock = intel_edp_target_clock(edp_encoder, mode);
> +	else if (is_dp)
> +		target_clock = mode->clock;
> +	else
> +		target_clock = adjusted_mode->clock;
> +
> +	if (!lane) {
> +		/*
> +		 * Account for spread spectrum to avoid
> +		 * oversubscribing the link. Max center spread
> +		 * is 2.5%; use 5% for safety's sake.
> +		 */
> +		u32 bps = target_clock * intel_crtc->bpp * 21 / 20;
> +		lane = bps / (link_bw * 8) + 1;
> +	}
> +
> +	intel_crtc->fdi_lanes = lane;
> +
> +	if (pixel_multiplier > 1)
> +		link_bw *= pixel_multiplier;
> +
> +	ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
> +			     &m_n);
> +
> +	if (is_dp && !is_cpu_edp) {
> +		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> +	} else {
> +		/* For non-DP output, clear any trans DP clock recovery setting.*/
> +		I915_WRITE(TRANSDATA_M1(pipe), 0);
> +		I915_WRITE(TRANSDATA_N1(pipe), 0);
> +		I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> +		I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> +	}

The pch DP transcoder m/n values are imo encoder-specific state - I'm
toying around with ideas to shovel this into encoder callbacks, similarly
to the lvds port enabling. Hence ...

[pls scroll down.]

> +	I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
> +	I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
> +	I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
> +	I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);
> +}
> +
>  static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  				  struct drm_display_mode *mode,
>  				  struct drm_display_mode *adjusted_mode,
> @@ -4751,11 +4837,9 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	u32 dpll, fp = 0, fp2 = 0;
>  	bool ok, has_reduced_clock = false, is_sdvo = false;
>  	bool is_crt = false, is_lvds = false, is_tv = false, is_dp = false;
> -	struct intel_encoder *encoder, *edp_encoder = NULL;
> +	struct intel_encoder *encoder;
>  	const intel_limit_t *limit;
> -	int ret;
> -	struct fdi_m_n m_n = {0};
> -	int target_clock, pixel_multiplier, lane, link_bw, factor;
> +	int ret, factor;
>  	unsigned int pipe_bpp;
>  	bool dither;
>  	bool is_cpu_edp = false, is_pch_edp = false;
> @@ -4786,7 +4870,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  				is_pch_edp = true;
>  			else
>  				is_cpu_edp = true;
> -			edp_encoder = encoder;
>  			break;
>  		}
>  
> @@ -4828,33 +4911,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	if (is_sdvo && is_tv)
>  		i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
>  
> -
> -	/* FDI link */
> -	pixel_multiplier = intel_mode_get_pixel_multiplier(adjusted_mode);
> -	lane = 0;
> -	/* CPU eDP doesn't require FDI link, so just set DP M/N
> -	   according to current link config */
> -	if (is_cpu_edp) {
> -		intel_edp_link_config(edp_encoder, &lane, &link_bw);
> -	} else {
> -		/* FDI is a binary signal running at ~2.7GHz, encoding
> -		 * each output octet as 10 bits. The actual frequency
> -		 * is stored as a divider into a 100MHz clock, and the
> -		 * mode pixel clock is stored in units of 1KHz.
> -		 * Hence the bw of each lane in terms of the mode signal
> -		 * is:
> -		 */
> -		link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
> -	}
> -
> -	/* [e]DP over FDI requires target mode clock instead of link clock. */
> -	if (edp_encoder)
> -		target_clock = intel_edp_target_clock(edp_encoder, mode);
> -	else if (is_dp)
> -		target_clock = mode->clock;
> -	else
> -		target_clock = adjusted_mode->clock;
> -
>  	/* determine panel color depth */
>  	dither = intel_choose_pipe_bpp_dither(crtc, fb, &pipe_bpp, mode);
>  	if (is_lvds && dev_priv->lvds_dither)
> @@ -4868,23 +4924,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	}
>  	intel_crtc->bpp = pipe_bpp;
>  
> -	if (!lane) {
> -		/*
> -		 * Account for spread spectrum to avoid
> -		 * oversubscribing the link. Max center spread
> -		 * is 2.5%; use 5% for safety's sake.
> -		 */
> -		u32 bps = target_clock * intel_crtc->bpp * 21 / 20;
> -		lane = bps / (link_bw * 8) + 1;
> -	}
> -
> -	intel_crtc->fdi_lanes = lane;
> -
> -	if (pixel_multiplier > 1)
> -		link_bw *= pixel_multiplier;
> -	ironlake_compute_m_n(intel_crtc->bpp, lane, target_clock, link_bw,
> -			     &m_n);
> -
>  	fp = clock.n << 16 | clock.m1 << 8 | clock.m2;
>  	if (has_reduced_clock)
>  		fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 |
> @@ -4977,15 +5016,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	if (is_lvds)
>  		ironlake_lvds_port_enable(intel_crtc, adjusted_mode, clock.p2);
>  
> -	if (is_dp && !is_cpu_edp) {
> -		intel_dp_set_m_n(crtc, mode, adjusted_mode);
> -	} else {
> -		/* For non-DP output, clear any trans DP clock recovery setting.*/
> -		I915_WRITE(TRANSDATA_M1(pipe), 0);
> -		I915_WRITE(TRANSDATA_N1(pipe), 0);
> -		I915_WRITE(TRANSDPLINK_M1(pipe), 0);
> -		I915_WRITE(TRANSDPLINK_N1(pipe), 0);
> -	}
> +	ironlake_set_m_n(crtc, mode, adjusted_mode);

... I think we should leave this here for now ...
>  
>  	if (intel_crtc->pch_pll) {
>  		I915_WRITE(intel_crtc->pch_pll->pll_reg, dpll);
> @@ -5014,11 +5045,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	ironlake_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
> -	I915_WRITE(PIPE_DATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
> -	I915_WRITE(PIPE_DATA_N1(pipe), m_n.gmch_n);
> -	I915_WRITE(PIPE_LINK_M1(pipe), m_n.link_m);
> -	I915_WRITE(PIPE_LINK_N1(pipe), m_n.link_n);

... and instead move the new ironlake_set_m_n call to this place - that
way we don't change anything in the register write sequence, too. And
somehow changing the register write sequence bites us every time we try :(

Also, I'm hunting a strange dp dpms issue on ilk/gm45 currently and I
suspect (without any evidence). So I prefer we're extra careful around dp
state for now.

Otherwise this looks nice.
-Daniel

> -
>  	if (is_cpu_edp)
>  		ironlake_set_pll_edp(crtc, adjusted_mode->clock);
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux