[PATCH 13/13] drm/i915: clear up the fdi/dp set_m_n confusion

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

 



On Wed, Mar 27, 2013 at 12:45:02AM +0100, Daniel Vetter wrote:
> There's a rather decent confusion going on around transcoder m_n
> values. So let's clarify:
> - All dp encoders need this, either on the pch transcoder if it's a
>   pch port, or on the cpu transcoder/pipe if it's a cpu port.
> - fdi links need to have the right m_n values for the fdi link set in
>   the cpu transcoder.
> 
> To handle the pch vs transcoder stuff a bit better, extract transcoder
> set_m_n helpers. To make them simpler, set intel_crtc->cpu_transcoder
> als in ironlake_crtc_mode_set, so that gen5+ (where the cpu m_n
> registers are all at the same offset) can use it.
> 
> Haswell modeset is decently confused about dp vs. edp vs. fdi. dp vs.
> edp works exactly the same as dp (since there's no pch dp any more),
> so use that as a check. And only set up the fdi m_n values if we
> really have a pch encoder present (which means we have a VGA encoder).
> 
> On ilk+ we've called ironlake_set_m_n both for cpu_edp and for pch
> encoders. Now that dp_set_m_n handles all dp links (thanks to the
> pch encoder check), we can ditch the cpu_edp stuff from the
> fdi_set_m_n function.
> 
> Since the dp_m_n values are not readily available, we need to
> carefully coax the edp values out of the encoder. Hence we can't (yet)
> kill this superflous complexity.
> 
> v2: Rebase on top of the ivb fdi B/C check patch - we need to properly
> clear intel_crtc->fdi_lane, otherwise those checks will misfire.
> 
> v3: Rebased on top of a s/IS_HASWELL/HAS_DDI/ patch from Paulo Zanoni.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Oops, that patch is already part of the next pile of pipe_config stuff.
Please disregard (for now ...).
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c | 86 +++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_dp.c      | 30 ++-----------
>  drivers/gpu/drm/i915/intel_drv.h     |  8 ++++
>  3 files changed, 67 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8ab7520..b076665 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5297,15 +5297,47 @@ int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
>  	return bps / (link_bw * 8) + 1;
>  }
>  
> -static void ironlake_set_m_n(struct drm_crtc *crtc)
> +void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
> +				  struct intel_link_m_n *m_n)
>  {
> -	struct drm_device *dev = crtc->dev;
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe = crtc->pipe;
> +
> +	I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> +	I915_WRITE(TRANSDATA_N1(pipe), m_n->gmch_n);
> +	I915_WRITE(TRANSDPLINK_M1(pipe), m_n->link_m);
> +	I915_WRITE(TRANSDPLINK_N1(pipe), m_n->link_n);
> +}
> +
> +void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> +				  struct intel_link_m_n *m_n)
> +{
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int pipe = crtc->pipe;
> +	enum transcoder transcoder = crtc->cpu_transcoder;
> +
> +	if (INTEL_INFO(dev)->gen >= 5) {
> +		I915_WRITE(PIPE_DATA_M1(transcoder), TU_SIZE(m_n->tu) | m_n->gmch_m);
> +		I915_WRITE(PIPE_DATA_N1(transcoder), m_n->gmch_n);
> +		I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
> +		I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
> +	} else {
> +		I915_WRITE(PIPE_GMCH_DATA_M(pipe), TU_SIZE(m_n->tu) | m_n->gmch_m);
> +		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n->gmch_n);
> +		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n->link_m);
> +		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n->link_n);
> +	}
> +}
> +
> +static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_display_mode *adjusted_mode =
>  		&intel_crtc->config.adjusted_mode;
>  	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> -	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	struct intel_encoder *intel_encoder, *edp_encoder = NULL;
>  	struct intel_link_m_n m_n = {0};
>  	int target_clock, lane, link_bw;
> @@ -5325,22 +5357,14 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
>  		}
>  	}
>  
> -	/* FDI link */
> -	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;
> -	}
> +	/* 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)
> @@ -5350,9 +5374,8 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
>  	else
>  		target_clock = adjusted_mode->clock;
>  
> -	if (!lane)
> -		lane = ironlake_get_lanes_required(target_clock, link_bw,
> -						   intel_crtc->config.pipe_bpp);
> +	lane = ironlake_get_lanes_required(target_clock, link_bw,
> +					   intel_crtc->config.pipe_bpp);
>  
>  	intel_crtc->fdi_lanes = lane;
>  
> @@ -5361,10 +5384,7 @@ static void ironlake_set_m_n(struct drm_crtc *crtc)
>  	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane, target_clock,
>  			       link_bw, &m_n);
>  
> -	I915_WRITE(PIPE_DATA_M1(cpu_transcoder), TU_SIZE(m_n.tu) | m_n.gmch_m);
> -	I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
> -	I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
> -	I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
> +	intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
>  }
>  
>  static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> @@ -5511,6 +5531,8 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
>  	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
>  
> +	intel_crtc->cpu_transcoder = pipe;
> +
>  	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
>  				     &has_reduced_clock, &reduced_clock);
>  	if (!ok) {
> @@ -5549,7 +5571,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  	} else
>  		intel_put_pch_pll(intel_crtc);
>  
> -	if (is_dp && !is_cpu_edp)
> +	if (is_dp)
>  		intel_dp_set_m_n(crtc, mode, adjusted_mode);
>  
>  	for_each_encoder_on_crtc(dev, crtc, encoder)
> @@ -5585,7 +5607,9 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	/* Note, this also computes intel_crtc->fdi_lanes which is used below in
>  	 * ironlake_check_fdi_lanes. */
> -	ironlake_set_m_n(crtc);
> +	intel_crtc->fdi_lanes = 0;
> +	if (intel_crtc->config.has_pch_encoder)
> +		ironlake_fdi_set_m_n(crtc);
>  
>  	fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>  
> @@ -5697,15 +5721,15 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
>  	DRM_DEBUG_KMS("Mode for pipe %d:\n", pipe);
>  	drm_mode_debug_printmodeline(mode);
>  
> -	if (is_dp && !is_cpu_edp)
> +	if (is_dp)
>  		intel_dp_set_m_n(crtc, mode, adjusted_mode);
>  
>  	intel_crtc->lowfreq_avail = false;
>  
>  	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
>  
> -	if (!is_dp || is_cpu_edp)
> -		ironlake_set_m_n(crtc);
> +	if (intel_crtc->config.has_pch_encoder)
> +		ironlake_fdi_set_m_n(crtc);
>  
>  	haswell_set_pipeconf(crtc, adjusted_mode, dither);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 251aa6b..6b8a279 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -766,12 +766,9 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  	struct drm_device *dev = crtc->dev;
>  	struct intel_encoder *intel_encoder;
>  	struct intel_dp *intel_dp;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int lane_count = 4;
>  	struct intel_link_m_n m_n;
> -	int pipe = intel_crtc->pipe;
> -	enum transcoder cpu_transcoder = intel_crtc->cpu_transcoder;
>  	int target_clock;
>  
>  	/*
> @@ -805,29 +802,10 @@ intel_dp_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode,
>  	intel_link_compute_m_n(intel_crtc->config.pipe_bpp, lane_count,
>  			       target_clock, adjusted_mode->clock, &m_n);
>  
> -	if (HAS_DDI(dev)) {
> -		I915_WRITE(PIPE_DATA_M1(cpu_transcoder),
> -			   TU_SIZE(m_n.tu) | m_n.gmch_m);
> -		I915_WRITE(PIPE_DATA_N1(cpu_transcoder), m_n.gmch_n);
> -		I915_WRITE(PIPE_LINK_M1(cpu_transcoder), m_n.link_m);
> -		I915_WRITE(PIPE_LINK_N1(cpu_transcoder), m_n.link_n);
> -	} else if (HAS_PCH_SPLIT(dev)) {
> -		I915_WRITE(TRANSDATA_M1(pipe), TU_SIZE(m_n.tu) | m_n.gmch_m);
> -		I915_WRITE(TRANSDATA_N1(pipe), m_n.gmch_n);
> -		I915_WRITE(TRANSDPLINK_M1(pipe), m_n.link_m);
> -		I915_WRITE(TRANSDPLINK_N1(pipe), m_n.link_n);
> -	} else if (IS_VALLEYVIEW(dev)) {
> -		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);
> -	} else {
> -		I915_WRITE(PIPE_GMCH_DATA_M(pipe),
> -			   TU_SIZE(m_n.tu) | m_n.gmch_m);
> -		I915_WRITE(PIPE_GMCH_DATA_N(pipe), m_n.gmch_n);
> -		I915_WRITE(PIPE_DP_LINK_M(pipe), m_n.link_m);
> -		I915_WRITE(PIPE_DP_LINK_N(pipe), m_n.link_n);
> -	}
> +	if (intel_crtc->config.has_pch_encoder)
> +		intel_pch_transcoder_set_m_n(intel_crtc, &m_n);
> +	else
> +		intel_cpu_transcoder_set_m_n(intel_crtc, &m_n);
>  }
>  
>  void intel_dp_init_link_config(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5c7b04b..3a9b7be 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -192,8 +192,12 @@ struct intel_crtc_config {
>  	 */
>  	bool limited_color_range;
>  
> +	/* DP has a bunch of special case unfortunately, so mark the pipe
> +	 * accordingly. */
> +	bool has_dp_encoder;
>  	bool dither;
>  	int pipe_bpp;
> +	struct intel_link_m_n dp_m_n;
>  
>  	/* Used by SDVO (and if we ever fix it, HDMI). */
>  	unsigned pixel_multiplier;
> @@ -641,6 +645,10 @@ extern void intel_init_clock_gating(struct drm_device *dev);
>  extern void intel_write_eld(struct drm_encoder *encoder,
>  			    struct drm_display_mode *mode);
>  extern void intel_cpt_verify_modeset(struct drm_device *dev, int pipe);
> +extern void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
> +					 struct intel_link_m_n *m_n);
> +extern void intel_pch_transcoder_set_m_n(struct intel_crtc *crtc,
> +					 struct intel_link_m_n *m_n);
>  extern void intel_prepare_ddi(struct drm_device *dev);
>  extern void hsw_fdi_link_train(struct drm_crtc *crtc);
>  extern void intel_ddi_init(struct drm_device *dev, enum port port);
> -- 
> 1.7.11.7
> 

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