[Intel-gfx] [PATCH] drm/i915: store adjusted dotclock in adjusted_mode->clock

Daniel Vetter daniel.vetter at ffwll.ch
Fri May 31 21:59:49 CEST 2013


... not the port clock. This allows us to kill the funny semantics
around pixel_target_clock.

Since the dpll code still needs the real port clock, add a new
port_clock field to the pipe configuration. Handling the default case
for that one is a bit tricky, since encoders might not consistently
overwrite it when retrying the crtc/encoder bw arbitrage step in the
compute config stage. Hence we need to always clear port_clock and
update it again if the encoder hasn't put in something more specific.
This can't be done in one step since the encoder might want to adjust
the mode first.

I was a bit on the fence whether I should subsume the pixel multiplier
handling into the port_clock, too. But then I decided against this
since it's on an abstract level still the dotclock of the adjusted
mode, and only our hw makes it a bit special due to the separate pixel
mulitplier setting (which requires that the dpll runs at the
non-multiplied dotclock).

So after this patch the adjusted_mode accurately describes the mode we
feed into the port, after the panel fitter and pixel multiplier (or
line doubling, if we ever bother with that) have done their job.
Since the fdi link is between the pfit and the pixel multiplier steps
we need to be careful with calculating the fdi link config.

The ironlake cpu edp port is the only encoder interested in the
portclock (outside of compute_config), refactor the flow a bit to
avoid duplicated code. I've broken this part in an early version of
the patch, hence why I've changed the code a bit more than strictly
required.

v2: Fix up ilk cpu pll handling.

v3: Introduce an fdi_dotclock variable in ironlake_fdi_compute_config
to make it clearer that we transmit the adjusted_mode without the
pixel multiplier taken into account. The old code multiplied the the
available link bw with the pixel multiplier, which results in the same
fdi configuration, but is much more confusing.

v4: Rebase on top of Imre's is_cpu_edp removal.

v5: Rebase on top of Paulo's haswell watermark fixes, which introduce
a new place which looked at the pixel_clock and so needed conversion.

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org> (v3)
Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  3 ++-
 drivers/gpu/drm/i915/intel_display.c | 37 ++++++++++++++------------
 drivers/gpu/drm/i915/intel_dp.c      | 50 +++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_drv.h     | 13 +++++-----
 drivers/gpu/drm/i915/intel_hdmi.c    |  4 +--
 drivers/gpu/drm/i915/intel_pm.c      |  5 +---
 6 files changed, 49 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9649df8..486c46b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -624,7 +624,7 @@ intel_ddi_calculate_wrpll(int clock /* in Hz */,
 		      clock, *p_out, *n2_out, *r2_out);
 }
 
-bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock)
+bool intel_ddi_pll_mode_set(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *intel_encoder = intel_ddi_get_crtc_encoder(crtc);
@@ -634,6 +634,7 @@ bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock)
 	int type = intel_encoder->type;
 	enum pipe pipe = intel_crtc->pipe;
 	uint32_t reg, val;
+	int clock = intel_crtc->config.port_clock;
 
 	/* TODO: reuse PLLs when possible (compare values) */
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c3722d..2772b61 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3974,7 +3974,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
-	int target_clock, lane, link_bw;
+	int lane, link_bw, fdi_dotclock;
 	bool setup_ok, needs_recompute = false;
 
 retry:
@@ -3987,19 +3987,15 @@ retry:
 	 */
 	link_bw = intel_fdi_link_freq(dev) * MHz(100)/KHz(1)/10;
 
-	if (pipe_config->pixel_target_clock)
-		target_clock = pipe_config->pixel_target_clock;
-	else
-		target_clock = adjusted_mode->clock;
-
-	lane = ironlake_get_lanes_required(target_clock, link_bw,
+	lane = ironlake_get_lanes_required(adjusted_mode->clock, link_bw,
 					   pipe_config->pipe_bpp);
 
 	pipe_config->fdi_lanes = lane;
 
+	fdi_dotclock = adjusted_mode->clock;
 	if (pipe_config->pixel_multiplier > 1)
-		link_bw *= pipe_config->pixel_multiplier;
-	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
+		fdi_dotclock /= pipe_config->pixel_multiplier;
+	intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
 			       link_bw, &pipe_config->fdi_m_n);
 
 	setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
@@ -4340,8 +4336,6 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_display_mode *adjusted_mode =
-		&crtc->config.adjusted_mode;
 	struct intel_encoder *encoder;
 	int pipe = crtc->pipe;
 	u32 dpll, mdiv;
@@ -4394,7 +4388,7 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 	vlv_dpio_write(dev_priv, DPIO_DIV(pipe), mdiv);
 
 	/* Set HBR and RBR LPF coefficients */
-	if (adjusted_mode->clock == 162000 ||
+	if (crtc->config.port_clock == 162000 ||
 	    intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI))
 		vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
 				 0x005f0021);
@@ -4839,7 +4833,8 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
 	limit = intel_limit(crtc, refclk);
-	ok = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock,
+	ok = dev_priv->display.find_dpll(limit, crtc,
+					 intel_crtc->config.port_clock,
 					 refclk, NULL, &clock);
 	if (!ok) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
@@ -5447,7 +5442,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
 }
 
 static bool ironlake_compute_clocks(struct drm_crtc *crtc,
-				    struct drm_display_mode *adjusted_mode,
 				    intel_clock_t *clock,
 				    bool *has_reduced_clock,
 				    intel_clock_t *reduced_clock)
@@ -5475,7 +5469,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
 	limit = intel_limit(crtc, refclk);
-	ret = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock,
+	ret = dev_priv->display.find_dpll(limit, crtc,
+					  to_intel_crtc(crtc)->config.port_clock,
 					  refclk, NULL, clock);
 	if (!ret)
 		return false;
@@ -5675,7 +5670,7 @@ 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));
 
-	ok = ironlake_compute_clocks(crtc, adjusted_mode, &clock,
+	ok = ironlake_compute_clocks(crtc, &clock,
 				     &has_reduced_clock, &reduced_clock);
 	if (!ok) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
@@ -5878,7 +5873,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 	WARN(num_connectors != 1, "%d connectors attached to pipe %c\n",
 	     num_connectors, pipe_name(pipe));
 
-	if (!intel_ddi_pll_mode_set(crtc, adjusted_mode->clock))
+	if (!intel_ddi_pll_mode_set(crtc))
 		return -EINVAL;
 
 	/* Ensure that the cursor is valid for the new mode before changing... */
@@ -7769,6 +7764,9 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		goto fail;
 
 encoder_retry:
+	/* Ensure the port clock default is reset when retrying. */
+	pipe_config->port_clock = 0;
+
 	/* Pass our mode to the connectors and the CRTC to give them a chance to
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
@@ -7797,6 +7795,11 @@ encoder_retry:
 		}
 	}
 
+	/* Set default port clock if not overwritten by the encoder. Needs to be
+	 * done afterwards in case the encoder adjusts the mode. */
+	if (!pipe_config->port_clock)
+		pipe_config->port_clock = pipe_config->adjusted_mode.clock;
+
 	ret = intel_crtc_compute_config(crtc, pipe_config);
 	if (ret < 0) {
 		DRM_DEBUG_KMS("CRTC fixup failed\n");
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 91a31b3..c92eeeb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -677,7 +677,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	int max_clock = intel_dp_max_link_bw(intel_dp) == DP_LINK_BW_2_7 ? 1 : 0;
 	int bpp, mode_rate;
 	static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
-	int target_clock, link_avail, link_clock;
+	int link_avail, link_clock;
 
 	if (HAS_PCH_SPLIT(dev) && !HAS_DDI(dev) && port != PORT_A)
 		pipe_config->has_pch_encoder = true;
@@ -694,8 +694,6 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 			intel_pch_panel_fitting(intel_crtc, pipe_config,
 						intel_connector->panel.fitting_mode);
 	}
-	/* We need to take the panel's fixed mode into account. */
-	target_clock = adjusted_mode->clock;
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		return false;
@@ -711,7 +709,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		bpp = min_t(int, bpp, dev_priv->vbt.edp_bpp);
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
-		mode_rate = intel_dp_link_required(target_clock, bpp);
+		mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp);
 
 		for (clock = 0; clock <= max_clock; clock++) {
 			for (lane_count = 1; lane_count <= max_lane_count; lane_count <<= 1) {
@@ -746,18 +744,17 @@ found:
 
 	intel_dp->link_bw = bws[clock];
 	intel_dp->lane_count = lane_count;
-	adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
 	pipe_config->pipe_bpp = bpp;
-	pipe_config->pixel_target_clock = target_clock;
+	pipe_config->port_clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
 
 	DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
 		      intel_dp->link_bw, intel_dp->lane_count,
-		      adjusted_mode->clock, bpp);
+		      pipe_config->port_clock, bpp);
 	DRM_DEBUG_KMS("DP link bw required %i available %i\n",
 		      mode_rate, link_avail);
 
 	intel_link_compute_m_n(bpp, lane_count,
-			       target_clock, adjusted_mode->clock,
+			       adjusted_mode->clock, pipe_config->port_clock,
 			       &pipe_config->dp_m_n);
 
 	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
@@ -780,24 +777,28 @@ void intel_dp_init_link_config(struct intel_dp *intel_dp)
 	}
 }
 
-static void ironlake_set_pll_edp(struct drm_crtc *crtc, int clock)
+static void ironlake_set_pll_cpu_edp(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = crtc->dev;
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct intel_crtc *crtc = to_intel_crtc(dig_port->base.base.crtc);
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 dpa_ctl;
 
-	DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", clock);
+	DRM_DEBUG_KMS("eDP PLL enable for clock %d\n", crtc->config.port_clock);
 	dpa_ctl = I915_READ(DP_A);
 	dpa_ctl &= ~DP_PLL_FREQ_MASK;
 
-	if (clock < 200000) {
+	if (crtc->config.port_clock == 162000) {
 		/* For a long time we've carried around a ILK-DevA w/a for the
 		 * 160MHz clock. If we're really unlucky, it's still required.
 		 */
 		DRM_DEBUG_KMS("160MHz cpu eDP clock, might need ilk devA w/a\n");
 		dpa_ctl |= DP_PLL_FREQ_160MHZ;
+		intel_dp->DP |= DP_PLL_FREQ_160MHZ;
 	} else {
 		dpa_ctl |= DP_PLL_FREQ_270MHZ;
+		intel_dp->DP |= DP_PLL_FREQ_270MHZ;
 	}
 
 	I915_WRITE(DP_A, dpa_ctl);
@@ -814,8 +815,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	enum port port = dp_to_dig_port(intel_dp)->port;
-	struct drm_crtc *crtc = encoder->crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc *crtc = to_intel_crtc(encoder->crtc);
 
 	/*
 	 * There are four kinds of DP registers:
@@ -845,7 +845,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 
 	if (intel_dp->has_audio) {
 		DRM_DEBUG_DRIVER("Enabling DP audio on pipe %c\n",
-				 pipe_name(intel_crtc->pipe));
+				 pipe_name(crtc->pipe));
 		intel_dp->DP |= DP_AUDIO_OUTPUT_ENABLE;
 		intel_write_eld(encoder, adjusted_mode);
 	}
@@ -864,13 +864,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN)
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
-		intel_dp->DP |= intel_crtc->pipe << 29;
-
-		/* don't miss out required setting for eDP */
-		if (adjusted_mode->clock < 200000)
-			intel_dp->DP |= DP_PLL_FREQ_160MHZ;
-		else
-			intel_dp->DP |= DP_PLL_FREQ_270MHZ;
+		intel_dp->DP |= crtc->pipe << 29;
 	} else if (!HAS_PCH_CPT(dev) || port == PORT_A) {
 		if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
 			intel_dp->DP |= intel_dp->color_range;
@@ -884,22 +878,14 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 		if (intel_dp->link_configuration[1] & DP_LANE_COUNT_ENHANCED_FRAME_EN)
 			intel_dp->DP |= DP_ENHANCED_FRAMING;
 
-		if (intel_crtc->pipe == 1)
+		if (crtc->pipe == 1)
 			intel_dp->DP |= DP_PIPEB_SELECT;
-
-		if (port == PORT_A && !IS_VALLEYVIEW(dev)) {
-			/* don't miss out required setting for eDP */
-			if (adjusted_mode->clock < 200000)
-				intel_dp->DP |= DP_PLL_FREQ_160MHZ;
-			else
-				intel_dp->DP |= DP_PLL_FREQ_270MHZ;
-		}
 	} else {
 		intel_dp->DP |= DP_LINK_TRAIN_OFF_CPT;
 	}
 
 	if (port == PORT_A && !IS_VALLEYVIEW(dev))
-		ironlake_set_pll_edp(crtc, adjusted_mode->clock);
+		ironlake_set_pll_cpu_edp(intel_dp);
 }
 
 #define IDLE_ON_MASK		(PP_ON | 0 	  | PP_SEQUENCE_MASK | 0                     | PP_SEQUENCE_STATE_MASK)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index fdf6303..afda71f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -243,12 +243,13 @@ struct intel_crtc_config {
 
 	int pipe_bpp;
 	struct intel_link_m_n dp_m_n;
-	/**
-	 * This is currently used by DP and HDMI encoders since those can have a
-	 * target pixel clock != the port link clock (which is currently stored
-	 * in adjusted_mode->clock).
+
+	/*
+	 * Frequence the dpll for the port should run at. Differs from the
+	 * adjusted dotclock e.g. for DP or 12bpc hdmi mode.
 	 */
-	int pixel_target_clock;
+	int port_clock;
+
 	/* Used by SDVO (and if we ever fix it, HDMI). */
 	unsigned pixel_multiplier;
 
@@ -786,7 +787,7 @@ extern void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
 extern void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc);
 extern void intel_ddi_disable_pipe_clock(struct intel_crtc *intel_crtc);
 extern void intel_ddi_setup_hw_pll_state(struct drm_device *dev);
-extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc, int clock);
+extern bool intel_ddi_pll_mode_set(struct drm_crtc *crtc);
 extern void intel_ddi_put_crtc_pll(struct drm_crtc *crtc);
 extern void intel_ddi_set_pipe_settings(struct drm_crtc *crtc);
 extern void intel_ddi_prepare_link_retrain(struct drm_encoder *encoder);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8062a92..bc12518 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -835,9 +835,7 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 		desired_bpp = 12*3;
 
 		/* Need to adjust the port link by 1.5x for 12bpc. */
-		adjusted_mode->clock = clock_12bpc;
-		pipe_config->pixel_target_clock =
-			pipe_config->requested_mode.clock;
+		pipe_config->port_clock = clock_12bpc;
 	} else {
 		DRM_DEBUG_KMS("picking bpc to 8 for HDMI output\n");
 		desired_bpp = 8*3;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 49a1887..4126fb1 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2078,10 +2078,7 @@ static uint32_t hsw_wm_get_pixel_rate(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	uint32_t pixel_rate, pfit_size;
 
-	if (intel_crtc->config.pixel_target_clock)
-		pixel_rate = intel_crtc->config.pixel_target_clock;
-	else
-		pixel_rate = intel_crtc->config.adjusted_mode.clock;
+	pixel_rate = intel_crtc->config.adjusted_mode.clock;
 
 	/* We only use IF-ID interlacing. If we ever use PF-ID we'll need to
 	 * adjust the pixel_rate here. */
-- 
1.7.11.7




More information about the Intel-gfx mailing list