[PATCH 0/9] pll limit fixes and a few other things

[PATCH 0/9] pll limit fixes and a few other things

[Daniel Vetter]

Hi all,

So pipe config conversion progresses neatly, and this pile here contains a few
more prep work things:
- fix up the pll limits (or at least what the current believe about the correct
  values is ...)
- unconfuse the meaning of adjusted_mode->clock, a side effect of this is that
  it makes the hsw wm patch I've rejected from Paulo obsolete (or at least it
  should do so).
- and on top a tiny cleanup and a paranoid WARN I've promised Mika

Comments&flames as usual highly welcome!

Cheers, Daniel

Daniel Vetter (9):
  drm/i915: split out intel_pnv_find_best_PLL
  drm/i915: move find_pll callback to dev_priv->display
  drm/i915: fixup i8xx pll limits
  drm/i915: fixup g4x pll limits
  drm/i915: pnv dpll doesn't use m1!
  drm/i915: fix ibx/cpt/ppt dpll limits
  drm/i915: store adjust dotclock in adjustede_mode->clock
  drm/i915: Drop some no longer required mode/adjusted_mode parameters
  drm/i915: check for strange pfit pipe assignemnt on ivb/hsw

 drivers/gpu/drm/i915/i915_drv.h      |   7 +
 drivers/gpu/drm/i915/intel_ddi.c     |   3 +-
 drivers/gpu/drm/i915/intel_display.c | 313 ++++++++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dp.c      |  50 ++----
 drivers/gpu/drm/i915/intel_drv.h     |  13 +-
 drivers/gpu/drm/i915/intel_hdmi.c    |   4 +-
 6 files changed, 196 insertions(+), 194 deletions(-)

-- 
1.7.11.7

[PATCH 1/9] drm/i915: split out intel_pnv_find_best_PLL

[Daniel Vetter]

Pineview is just different.

Also split out i9xx_clock from intel_clock and drop the now redundant
struct device * parameter.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 92 ++++++++++++++++++++++++++++++------
 1 file changed, 77 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4196155..dc09c95 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -97,10 +97,13 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 		    int target, int refclk, intel_clock_t *match_clock,
 		    intel_clock_t *best_clock);
 static bool
+intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
+			int target, int refclk, intel_clock_t *match_clock,
+			intel_clock_t *best_clock);
+static bool
 intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 			int target, int refclk, intel_clock_t *match_clock,
 			intel_clock_t *best_clock);
-
 static bool
 intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
 			int target, int refclk, intel_clock_t *match_clock,
@@ -246,7 +249,7 @@ static const intel_limit_t intel_limits_pineview_sdvo = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 200000,
 		.p2_slow = 10, .p2_fast = 5 },
-	.find_pll = intel_find_best_PLL,
+	.find_pll = intel_pnv_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_pineview_lvds = {
@@ -260,7 +263,7 @@ static const intel_limit_t intel_limits_pineview_lvds = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 112000,
 		.p2_slow = 14, .p2_fast = 14 },
-	.find_pll = intel_find_best_PLL,
+	.find_pll = intel_pnv_find_best_PLL,
 };
 
 /* Ironlake / Sandybridge
@@ -512,12 +515,8 @@ static uint32_t i9xx_dpll_compute_m(struct dpll *dpll)
 	return 5 * (dpll->m1 + 2) + (dpll->m2 + 2);
 }
 
-static void intel_clock(struct drm_device *dev, int refclk, intel_clock_t *clock)
+static void i9xx_clock(int refclk, intel_clock_t *clock)
 {
-	if (IS_PINEVIEW(dev)) {
-		pineview_clock(refclk, clock);
-		return;
-	}
 	clock->m = i9xx_dpll_compute_m(clock);
 	clock->p = clock->p1 * clock->p2;
 	clock->vco = refclk * clock->m / (clock->n + 2);
@@ -578,7 +577,68 @@ static bool
 intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 		    int target, int refclk, intel_clock_t *match_clock,
 		    intel_clock_t *best_clock)
+{
+	struct drm_device *dev = crtc->dev;
+	intel_clock_t clock;
+	int err = target;
+
+	if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
+		/*
+		 * For LVDS just rely on its current settings for dual-channel.
+		 * We haven't figured out how to reliably set up different
+		 * single/dual channel state, if we even can.
+		 */
+		if (intel_is_dual_link_lvds(dev))
+			clock.p2 = limit->p2.p2_fast;
+		else
+			clock.p2 = limit->p2.p2_slow;
+	} else {
+		if (target < limit->p2.dot_limit)
+			clock.p2 = limit->p2.p2_slow;
+		else
+			clock.p2 = limit->p2.p2_fast;
+	}
+
+	memset(best_clock, 0, sizeof(*best_clock));
+
+	for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max;
+	     clock.m1++) {
+		for (clock.m2 = limit->m2.min;
+		     clock.m2 <= limit->m2.max; clock.m2++) {
+			/* m1 is always 0 in Pineview */
+			if (clock.m2 >= clock.m1 && !IS_PINEVIEW(dev))
+				break;
+			for (clock.n = limit->n.min;
+			     clock.n <= limit->n.max; clock.n++) {
+				for (clock.p1 = limit->p1.min;
+					clock.p1 <= limit->p1.max; clock.p1++) {
+					int this_err;
 
+					i9xx_clock(refclk, &clock);
+					if (!intel_PLL_is_valid(dev, limit,
+								&clock))
+						continue;
+					if (match_clock &&
+					    clock.p != match_clock->p)
+						continue;
+
+					this_err = abs(clock.dot - target);
+					if (this_err < err) {
+						*best_clock = clock;
+						err = this_err;
+					}
+				}
+			}
+		}
+	}
+
+	return (err != target);
+}
+
+static bool
+intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
+			int target, int refclk, intel_clock_t *match_clock,
+			intel_clock_t *best_clock)
 {
 	struct drm_device *dev = crtc->dev;
 	intel_clock_t clock;
@@ -616,7 +676,7 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 					clock.p1 <= limit->p1.max; clock.p1++) {
 					int this_err;
 
-					intel_clock(dev, refclk, &clock);
+					pineview_clock(refclk, &clock);
 					if (!intel_PLL_is_valid(dev, limit,
 								&clock))
 						continue;
@@ -675,7 +735,7 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 				     clock.p1 >= limit->p1.min; clock.p1--) {
 					int this_err;
 
-					intel_clock(dev, refclk, &clock);
+					i9xx_clock(refclk, &clock);
 					if (!intel_PLL_is_valid(dev, limit,
 								&clock))
 						continue;
@@ -6962,8 +7022,10 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
 			return 0;
 		}
 
-		/* XXX: Handle the 100Mhz refclk */
-		intel_clock(dev, 96000, &clock);
+		if (IS_PINEVIEW(dev))
+			pineview_clock(96000, &clock);
+		else
+			i9xx_clock(96000, &clock);
 	} else {
 		bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
 
@@ -6975,9 +7037,9 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
 			if ((dpll & PLL_REF_INPUT_MASK) ==
 			    PLLB_REF_INPUT_SPREADSPECTRUMIN) {
 				/* XXX: might not be 66MHz */
-				intel_clock(dev, 66000, &clock);
+				i9xx_clock(66000, &clock);
 			} else
-				intel_clock(dev, 48000, &clock);
+				i9xx_clock(48000, &clock);
 		} else {
 			if (dpll & PLL_P1_DIVIDE_BY_TWO)
 				clock.p1 = 2;
@@ -6990,7 +7052,7 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc)
 			else
 				clock.p2 = 2;
 
-			intel_clock(dev, 48000, &clock);
+			i9xx_clock(48000, &clock);
 		}
 	}
 
-- 
1.7.11.7

[PATCH 2/9] drm/i915: move find_pll callback to dev_priv->display

[Daniel Vetter]

Now that the DP madness is cleared out, this is all only per-platform.
So move it out from the intel clock limits structure.

While at it drop the intel prefix on the static functions, call the
vtable entry find_dpll (since it's for the display pll) and rip out
the now unnecessary forward declarations.

Note that the parameters of ->find_dpll are still unchanged, but they
eventually need to be moved over to just take in a pipe configuration.
But currently a lot of things are still missing from the pipe
configuration (reflock, output-specific dpll limits and preferences,
downclocked dotclock). So this will happen in a later step.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.h      |   7 +++
 drivers/gpu/drm/i915/intel_display.c | 106 ++++++++++-------------------------
 2 files changed, 38 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 14817de..b1ddc6b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -306,6 +306,8 @@ struct drm_i915_error_state {
 
 struct intel_crtc_config;
 struct intel_crtc;
+struct intel_limit;
+struct dpll;
 
 struct drm_i915_display_funcs {
 	bool (*fbc_enabled)(struct drm_device *dev);
@@ -313,6 +315,11 @@ struct drm_i915_display_funcs {
 	void (*disable_fbc)(struct drm_device *dev);
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
+	bool (*find_dpll)(const struct intel_limit *limit,
+			  struct drm_crtc *crtc,
+			  int target, int refclk,
+			  struct dpll *match_clock,
+			  struct dpll *best_clock);
 	void (*update_wm)(struct drm_device *dev);
 	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
 				 uint32_t sprite_width, int pixel_size);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc09c95..959c2ae 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -59,24 +59,6 @@ typedef struct intel_limit intel_limit_t;
 struct intel_limit {
 	intel_range_t   dot, vco, n, m, m1, m2, p, p1;
 	intel_p2_t	    p2;
-	/**
-	 * find_pll() - Find the best values for the PLL
-	 * @limit: limits for the PLL
-	 * @crtc: current CRTC
-	 * @target: target frequency in kHz
-	 * @refclk: reference clock frequency in kHz
-	 * @match_clock: if provided, @best_clock P divider must
-	 *               match the P divider from @match_clock
-	 *               used for LVDS downclocking
-	 * @best_clock: best PLL values found
-	 *
-	 * Returns true on success, false on failure.
-	 */
-	bool (*find_pll)(const intel_limit_t *limit,
-			 struct drm_crtc *crtc,
-			 int target, int refclk,
-			 intel_clock_t *match_clock,
-			 intel_clock_t *best_clock);
 };
 
 /* FDI */
@@ -92,23 +74,6 @@ intel_pch_rawclk(struct drm_device *dev)
 	return I915_READ(PCH_RAWCLK_FREQ) & RAWCLK_FREQ_MASK;
 }
 
-static bool
-intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
-		    int target, int refclk, intel_clock_t *match_clock,
-		    intel_clock_t *best_clock);
-static bool
-intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock);
-static bool
-intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock);
-static bool
-intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock);
-
 static inline u32 /* units of 100MHz */
 intel_fdi_link_freq(struct drm_device *dev)
 {
@@ -130,7 +95,6 @@ static const intel_limit_t intel_limits_i8xx_dvo = {
 	.p1 = { .min = 2, .max = 33 },
 	.p2 = { .dot_limit = 165000,
 		.p2_slow = 4, .p2_fast = 2 },
-	.find_pll = intel_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_i8xx_lvds = {
@@ -144,7 +108,6 @@ static const intel_limit_t intel_limits_i8xx_lvds = {
 	.p1 = { .min = 1, .max = 6 },
 	.p2 = { .dot_limit = 165000,
 		.p2_slow = 14, .p2_fast = 7 },
-	.find_pll = intel_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_i9xx_sdvo = {
@@ -158,7 +121,6 @@ static const intel_limit_t intel_limits_i9xx_sdvo = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 200000,
 		.p2_slow = 10, .p2_fast = 5 },
-	.find_pll = intel_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_i9xx_lvds = {
@@ -172,7 +134,6 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 112000,
 		.p2_slow = 14, .p2_fast = 7 },
-	.find_pll = intel_find_best_PLL,
 };
 
 
@@ -189,7 +150,6 @@ static const intel_limit_t intel_limits_g4x_sdvo = {
 		.p2_slow = 10,
 		.p2_fast = 10
 	},
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_g4x_hdmi = {
@@ -203,7 +163,6 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
 	.p1 = { .min = 1, .max = 8},
 	.p2 = { .dot_limit = 165000,
 		.p2_slow = 10, .p2_fast = 5 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
@@ -218,7 +177,6 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
 	.p2 = { .dot_limit = 0,
 		.p2_slow = 14, .p2_fast = 14
 	},
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
@@ -233,7 +191,6 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
 	.p2 = { .dot_limit = 0,
 		.p2_slow = 7, .p2_fast = 7
 	},
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_pineview_sdvo = {
@@ -249,7 +206,6 @@ static const intel_limit_t intel_limits_pineview_sdvo = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 200000,
 		.p2_slow = 10, .p2_fast = 5 },
-	.find_pll = intel_pnv_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_pineview_lvds = {
@@ -263,7 +219,6 @@ static const intel_limit_t intel_limits_pineview_lvds = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 112000,
 		.p2_slow = 14, .p2_fast = 14 },
-	.find_pll = intel_pnv_find_best_PLL,
 };
 
 /* Ironlake / Sandybridge
@@ -282,7 +237,6 @@ static const intel_limit_t intel_limits_ironlake_dac = {
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 225000,
 		.p2_slow = 10, .p2_fast = 5 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_ironlake_single_lvds = {
@@ -296,7 +250,6 @@ static const intel_limit_t intel_limits_ironlake_single_lvds = {
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
 		.p2_slow = 14, .p2_fast = 14 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_ironlake_dual_lvds = {
@@ -310,7 +263,6 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds = {
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
 		.p2_slow = 7, .p2_fast = 7 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 /* LVDS 100mhz refclk limits. */
@@ -325,7 +277,6 @@ static const intel_limit_t intel_limits_ironlake_single_lvds_100m = {
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
 		.p2_slow = 14, .p2_fast = 14 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
@@ -339,7 +290,6 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 	.p1 = { .min = 2, .max = 6 },
 	.p2 = { .dot_limit = 225000,
 		.p2_slow = 7, .p2_fast = 7 },
-	.find_pll = intel_g4x_find_best_PLL,
 };
 
 static const intel_limit_t intel_limits_vlv_dac = {
@@ -353,7 +303,6 @@ static const intel_limit_t intel_limits_vlv_dac = {
 	.p1 = { .min = 1, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
-	.find_pll = intel_vlv_find_best_pll,
 };
 
 static const intel_limit_t intel_limits_vlv_hdmi = {
@@ -367,7 +316,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
-	.find_pll = intel_vlv_find_best_pll,
 };
 
 static const intel_limit_t intel_limits_vlv_dp = {
@@ -381,7 +329,6 @@ static const intel_limit_t intel_limits_vlv_dp = {
 	.p1 = { .min = 1, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
-	.find_pll = intel_vlv_find_best_pll,
 };
 
 u32 intel_dpio_read(struct drm_i915_private *dev_priv, int reg)
@@ -574,7 +521,7 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
 }
 
 static bool
-intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
+i9xx_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 		    int target, int refclk, intel_clock_t *match_clock,
 		    intel_clock_t *best_clock)
 {
@@ -636,9 +583,9 @@ intel_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 }
 
 static bool
-intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock)
+pnv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
+		   int target, int refclk, intel_clock_t *match_clock,
+		   intel_clock_t *best_clock)
 {
 	struct drm_device *dev = crtc->dev;
 	intel_clock_t clock;
@@ -698,9 +645,9 @@ intel_pnv_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 }
 
 static bool
-intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock)
+g4x_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
+		   int target, int refclk, intel_clock_t *match_clock,
+		   intel_clock_t *best_clock)
 {
 	struct drm_device *dev = crtc->dev;
 	intel_clock_t clock;
@@ -755,9 +702,9 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 }
 
 static bool
-intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
-			int target, int refclk, intel_clock_t *match_clock,
-			intel_clock_t *best_clock)
+vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
+		   int target, int refclk, intel_clock_t *match_clock,
+		   intel_clock_t *best_clock)
 {
 	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
 	u32 m, n, fastclk;
@@ -4968,8 +4915,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 = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, NULL,
-			     &clock);
+	ok = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock,
+					 refclk, NULL, &clock);
 	if (!ok) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
 		return -EINVAL;
@@ -4985,10 +4932,10 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		 * by using the FP0/FP1. In such case we will disable the LVDS
 		 * downclock feature.
 		*/
-		has_reduced_clock = limit->find_pll(limit, crtc,
+		has_reduced_clock =
+			dev_priv->display.find_dpll(limit, crtc,
 						    dev_priv->lvds_downclock,
-						    refclk,
-						    &clock,
+						    refclk, &clock,
 						    &reduced_clock);
 	}
 	/* Compat-code for transition, will disappear. */
@@ -5605,8 +5552,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 = limit->find_pll(limit, crtc, adjusted_mode->clock, refclk, NULL,
-			      clock);
+	ret = dev_priv->display.find_dpll(limit, crtc, adjusted_mode->clock,
+					  refclk, NULL, clock);
 	if (!ret)
 		return false;
 
@@ -5617,11 +5564,11 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 		 * by using the FP0/FP1. In such case we will disable the LVDS
 		 * downclock feature.
 		*/
-		*has_reduced_clock = limit->find_pll(limit, crtc,
-						     dev_priv->lvds_downclock,
-						     refclk,
-						     clock,
-						     reduced_clock);
+		*has_reduced_clock =
+			dev_priv->display.find_dpll(limit, crtc,
+						    dev_priv->lvds_downclock,
+						    refclk, clock,
+						    reduced_clock);
 	}
 
 	return true;
@@ -9074,6 +9021,15 @@ static void intel_init_display(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	if (HAS_PCH_SPLIT(dev) || IS_G4X(dev))
+		dev_priv->display.find_dpll = g4x_find_best_dpll;
+	else if (IS_VALLEYVIEW(dev))
+		dev_priv->display.find_dpll = vlv_find_best_dpll;
+	else if (IS_PINEVIEW(dev))
+		dev_priv->display.find_dpll = pnv_find_best_dpll;
+	else
+		dev_priv->display.find_dpll = i9xx_find_best_dpll;
+
 	if (HAS_DDI(dev)) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
-- 
1.7.11.7

[PATCH 3/9] drm/i915: fixup i8xx pll limits

[Daniel Vetter]

So apparently the pll limits in the docs are the real values, but the
formula actually seems to consistenly use register values. See

commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2
Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Date:   Wed Feb 13 22:20:22 2013 +0100

    drm/i915: Set i9xx sdvo clock limits according to specifications

On gen2 the situation is a bit more messy: We reuse the code for
m1/m2/n computation and register frobbing from gen3, so those limits
need to be in register values (and so need to substract 2 from the doc
limits).

But gen2 also stores p1/p2 with 2/1 substracted in the register! For
those though i8xx_update_pll does the adjustments, but the clock
computation code assumes it works like on gen3. So for divisor limits
we must _not_ adjust them to the register values.

v2: Extract the common i8xx m/n limits into a single define. This was
motivated by the mess for the g4x limits where they've all been off in
different directions, apparently to fix specific bugs ...

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 959c2ae..ea8eb0c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -84,13 +84,16 @@ intel_fdi_link_freq(struct drm_device *dev)
 		return 27;
 }
 
+#define I8XX_DPLL_M_N_LIMITS \
+	.n = { .min = 1, .max = 14 }, \
+	.m = { .min = 96, .max = 140 }, \
+	.m1 = { .min = 16, .max = 24 }, \
+	.m2 = { .min = 4, .max = 14 },
+
 static const intel_limit_t intel_limits_i8xx_dvo = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 930000, .max = 1400000 },
-	.n = { .min = 3, .max = 16 },
-	.m = { .min = 96, .max = 140 },
-	.m1 = { .min = 18, .max = 26 },
-	.m2 = { .min = 6, .max = 16 },
+	I8XX_DPLL_M_N_LIMITS
 	.p = { .min = 4, .max = 128 },
 	.p1 = { .min = 2, .max = 33 },
 	.p2 = { .dot_limit = 165000,
@@ -100,10 +103,7 @@ static const intel_limit_t intel_limits_i8xx_dvo = {
 static const intel_limit_t intel_limits_i8xx_lvds = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 930000, .max = 1400000 },
-	.n = { .min = 3, .max = 16 },
-	.m = { .min = 96, .max = 140 },
-	.m1 = { .min = 18, .max = 26 },
-	.m2 = { .min = 6, .max = 16 },
+	I8XX_DPLL_M_N_LIMITS
 	.p = { .min = 4, .max = 128 },
 	.p1 = { .min = 1, .max = 6 },
 	.p2 = { .dot_limit = 165000,
-- 
1.7.11.7

Re: [PATCH 3/9] drm/i915: fixup i8xx pll limits

[Damien Lespiau]

On Tue, May 21, 2013 at 09:54:53PM +0200, Daniel Vetter wrote:
> So apparently the pll limits in the docs are the real values, but the
> formula actually seems to consistenly use register values. See
> 
> commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2
> Author: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Date:   Wed Feb 13 22:20:22 2013 +0100
> 
>     drm/i915: Set i9xx sdvo clock limits according to specifications
> 
> On gen2 the situation is a bit more messy: We reuse the code for
> m1/m2/n computation and register frobbing from gen3, so those limits
> need to be in register values (and so need to substract 2 from the doc
> limits).
> 
> But gen2 also stores p1/p2 with 2/1 substracted in the register! For
> those though i8xx_update_pll does the adjustments, but the clock
> computation code assumes it works like on gen3. So for divisor limits
> we must _not_ adjust them to the register values.

I think this deserves a comment for the next one looking at that code.

Also the documented computations for p are:
  gen2: p = (p1 + 2) * 2^(p2 + 1)
  gen3: p = p1 * p2

which lead me to believe we weren't computing the dot clock correctly on
gen2 (but in fact it's fine as p1/p2 are never expressed as register
values, which are not a direct deduction (say on gen 3, p2 = 10 is coded
with 0, p2 = 5 is coded by 1))

So IIUC, we have n, m1, m2 that we express in the "register domain"
while m, p1, p2, p are in the "clock formula domain", for lack of a
better wording. And so the limits follow that.

Otherwise, the patch looks good:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> 
> v2: Extract the common i8xx m/n limits into a single define. This was
> motivated by the mess for the g4x limits where they've all been off in
> different directions, apparently to fix specific bugs ...
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 959c2ae..ea8eb0c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -84,13 +84,16 @@ intel_fdi_link_freq(struct drm_device *dev)
>  		return 27;
>  }
>  
> +#define I8XX_DPLL_M_N_LIMITS \
> +	.n = { .min = 1, .max = 14 }, \
> +	.m = { .min = 96, .max = 140 }, \
> +	.m1 = { .min = 16, .max = 24 }, \
> +	.m2 = { .min = 4, .max = 14 },
> +
>  static const intel_limit_t intel_limits_i8xx_dvo = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 930000, .max = 1400000 },
> -	.n = { .min = 3, .max = 16 },
> -	.m = { .min = 96, .max = 140 },
> -	.m1 = { .min = 18, .max = 26 },
> -	.m2 = { .min = 6, .max = 16 },
> +	I8XX_DPLL_M_N_LIMITS
>  	.p = { .min = 4, .max = 128 },
>  	.p1 = { .min = 2, .max = 33 },
>  	.p2 = { .dot_limit = 165000,
> @@ -100,10 +103,7 @@ static const intel_limit_t intel_limits_i8xx_dvo = {
>  static const intel_limit_t intel_limits_i8xx_lvds = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 930000, .max = 1400000 },
> -	.n = { .min = 3, .max = 16 },
> -	.m = { .min = 96, .max = 140 },
> -	.m1 = { .min = 18, .max = 26 },
> -	.m2 = { .min = 6, .max = 16 },
> +	I8XX_DPLL_M_N_LIMITS
>  	.p = { .min = 4, .max = 128 },
>  	.p1 = { .min = 1, .max = 6 },
>  	.p2 = { .dot_limit = 165000,
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/9] drm/i915: fixup g4x pll limits

[Daniel Vetter]

Again the same confusion that our code expects m1/m2 in register values.
This time around with the added fun that many of the existing values
have been all off by a bit in different directions. Hence extract a
common #define.

Note that n limits differ between lvds and other outputs. Strangely they've
all been correct already.

v2: Rebased on top of the DP pll rework, which makes it even more
obvious that we can do this ...

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ea8eb0c..cb54131 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
 		.p2_slow = 14, .p2_fast = 7 },
 };
 
+#define G4X_DPLL_M_LIMITS \
+	.m = { .min = 104, .max = 138 },\
+	.m1 = { .min = 15, .max = 21 },\
+	.m2 = { .min = 3, .max = 11 },
 
 static const intel_limit_t intel_limits_g4x_sdvo = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 1750000, .max = 3500000},
 	.n = { .min = 1, .max = 4 },
-	.m = { .min = 104, .max = 138 },
-	.m1 = { .min = 17, .max = 23 },
-	.m2 = { .min = 5, .max = 11 },
+	G4X_DPLL_M_LIMITS
 	.p = { .min = 10, .max = 30 },
 	.p1 = { .min = 1, .max = 3},
 	.p2 = { .dot_limit = 270000,
@@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
 	.dot = { .min = 22000, .max = 400000 },
 	.vco = { .min = 1750000, .max = 3500000},
 	.n = { .min = 1, .max = 4 },
-	.m = { .min = 104, .max = 138 },
-	.m1 = { .min = 16, .max = 23 },
-	.m2 = { .min = 5, .max = 11 },
+	G4X_DPLL_M_LIMITS
 	.p = { .min = 5, .max = 80 },
 	.p1 = { .min = 1, .max = 8},
 	.p2 = { .dot_limit = 165000,
@@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
 	.dot = { .min = 20000, .max = 115000 },
 	.vco = { .min = 1750000, .max = 3500000 },
 	.n = { .min = 1, .max = 3 },
-	.m = { .min = 104, .max = 138 },
-	.m1 = { .min = 17, .max = 23 },
-	.m2 = { .min = 5, .max = 11 },
+	G4X_DPLL_M_LIMITS
 	.p = { .min = 28, .max = 112 },
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 0,
@@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
 	.dot = { .min = 80000, .max = 224000 },
 	.vco = { .min = 1750000, .max = 3500000 },
 	.n = { .min = 1, .max = 3 },
-	.m = { .min = 104, .max = 138 },
-	.m1 = { .min = 17, .max = 23 },
-	.m2 = { .min = 5, .max = 11 },
+	G4X_DPLL_M_LIMITS
 	.p = { .min = 14, .max = 42 },
 	.p1 = { .min = 2, .max = 6 },
 	.p2 = { .dot_limit = 0,
-- 
1.7.11.7

Re: [PATCH 4/9] drm/i915: fixup g4x pll limits

[Damien Lespiau]

On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote:
> Again the same confusion that our code expects m1/m2 in register values.
> This time around with the added fun that many of the existing values
> have been all off by a bit in different directions. Hence extract a
> common #define.
> 
> Note that n limits differ between lvds and other outputs. Strangely they've
> all been correct already.
> 
> v2: Rebased on top of the DP pll rework, which makes it even more
> obvious that we can do this ...

I'm a bit confused by this one:

  - I don't seem to find the special LVDS limits for n
  - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake
    and cantiga. The docs single out Cantiga and we seem to need a
    different set of limits for that platform (compared to the rest of
    gen4) platforms?
  - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6
    (cantiga)
  - m doesn't seem to match what I have, it seems like it could be the
    cantiga values (except that m is in "formula space" so we don't
    substract 2)
  - m1 and m2 seem to match what I have for cantiga, are we supposed to
    have those limits for eaglelake as well?

Well, the only conclusion is that I must be reading the wrong docs, or?

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ea8eb0c..cb54131 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
>  		.p2_slow = 14, .p2_fast = 7 },
>  };
>  
> +#define G4X_DPLL_M_LIMITS \
> +	.m = { .min = 104, .max = 138 },\
> +	.m1 = { .min = 15, .max = 21 },\
> +	.m2 = { .min = 3, .max = 11 },
>  
>  static const intel_limit_t intel_limits_g4x_sdvo = {
>  	.dot = { .min = 25000, .max = 270000 },
>  	.vco = { .min = 1750000, .max = 3500000},
>  	.n = { .min = 1, .max = 4 },
> -	.m = { .min = 104, .max = 138 },
> -	.m1 = { .min = 17, .max = 23 },
> -	.m2 = { .min = 5, .max = 11 },
> +	G4X_DPLL_M_LIMITS
>  	.p = { .min = 10, .max = 30 },
>  	.p1 = { .min = 1, .max = 3},
>  	.p2 = { .dot_limit = 270000,
> @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
>  	.dot = { .min = 22000, .max = 400000 },
>  	.vco = { .min = 1750000, .max = 3500000},
>  	.n = { .min = 1, .max = 4 },
> -	.m = { .min = 104, .max = 138 },
> -	.m1 = { .min = 16, .max = 23 },
> -	.m2 = { .min = 5, .max = 11 },
> +	G4X_DPLL_M_LIMITS
>  	.p = { .min = 5, .max = 80 },
>  	.p1 = { .min = 1, .max = 8},
>  	.p2 = { .dot_limit = 165000,
> @@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
>  	.dot = { .min = 20000, .max = 115000 },
>  	.vco = { .min = 1750000, .max = 3500000 },
>  	.n = { .min = 1, .max = 3 },
> -	.m = { .min = 104, .max = 138 },
> -	.m1 = { .min = 17, .max = 23 },
> -	.m2 = { .min = 5, .max = 11 },
> +	G4X_DPLL_M_LIMITS
>  	.p = { .min = 28, .max = 112 },
>  	.p1 = { .min = 2, .max = 8 },
>  	.p2 = { .dot_limit = 0,
> @@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
>  	.dot = { .min = 80000, .max = 224000 },
>  	.vco = { .min = 1750000, .max = 3500000 },
>  	.n = { .min = 1, .max = 3 },
> -	.m = { .min = 104, .max = 138 },
> -	.m1 = { .min = 17, .max = 23 },
> -	.m2 = { .min = 5, .max = 11 },
> +	G4X_DPLL_M_LIMITS
>  	.p = { .min = 14, .max = 42 },
>  	.p1 = { .min = 2, .max = 6 },
>  	.p2 = { .dot_limit = 0,
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/9] drm/i915: fixup g4x pll limits

[Ville Syrjälä]

On Wed, Jun 12, 2013 at 12:07:39AM +0100, Damien Lespiau wrote:
> On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote:
> > Again the same confusion that our code expects m1/m2 in register values.
> > This time around with the added fun that many of the existing values
> > have been all off by a bit in different directions. Hence extract a
> > common #define.
> > 
> > Note that n limits differ between lvds and other outputs. Strangely they've
> > all been correct already.
> > 
> > v2: Rebased on top of the DP pll rework, which makes it even more
> > obvious that we can do this ...
> 
> I'm a bit confused by this one:
> 
>   - I don't seem to find the special LVDS limits for n

Me neither.

>   - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake
>     and cantiga. The docs single out Cantiga and we seem to need a
>     different set of limits for that platform (compared to the rest of
>     gen4) platforms?

I've noticed that the docs often mention just CTG leaving out ELK, even
though the stuff it's describing applies to both.

>   - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6
>     (cantiga)

I see 3-8 and 4-6.

>   - m doesn't seem to match what I have, it seems like it could be the
>     cantiga values (except that m is in "formula space" so we don't
>     substract 2)

105-140 is what the spec tells me, so a bit off either way.

There's more inconsistency w/ the p/p1 limits too, and the single
channel lvds dotclock limit doesn't agree w/ the spec either.

>   - m1 and m2 seem to match what I have for cantiga, are we supposed to
>     have those limits for eaglelake as well?
> 
> Well, the only conclusion is that I must be reading the wrong docs, or?
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ea8eb0c..cb54131 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
> >  		.p2_slow = 14, .p2_fast = 7 },
> >  };
> >  
> > +#define G4X_DPLL_M_LIMITS \
> > +	.m = { .min = 104, .max = 138 },\
> > +	.m1 = { .min = 15, .max = 21 },\
> > +	.m2 = { .min = 3, .max = 11 },
> >  
> >  static const intel_limit_t intel_limits_g4x_sdvo = {
> >  	.dot = { .min = 25000, .max = 270000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 10, .max = 30 },
> >  	.p1 = { .min = 1, .max = 3},
> >  	.p2 = { .dot_limit = 270000,
> > @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
> >  	.dot = { .min = 22000, .max = 400000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 16, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 5, .max = 80 },
> >  	.p1 = { .min = 1, .max = 8},
> >  	.p2 = { .dot_limit = 165000,
> > @@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
> >  	.dot = { .min = 20000, .max = 115000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 28, .max = 112 },
> >  	.p1 = { .min = 2, .max = 8 },
> >  	.p2 = { .dot_limit = 0,
> > @@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
> >  	.dot = { .min = 80000, .max = 224000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 14, .max = 42 },
> >  	.p1 = { .min = 2, .max = 6 },
> >  	.p2 = { .dot_limit = 0,
> > -- 
> > 1.7.11.7
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 4/9] drm/i915: fixup g4x pll limits

[Daniel Vetter]

On Wed, Jun 12, 2013 at 12:07:39AM +0100, Damien Lespiau wrote:
> On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote:
> > Again the same confusion that our code expects m1/m2 in register values.
> > This time around with the added fun that many of the existing values
> > have been all off by a bit in different directions. Hence extract a
> > common #define.
> > 
> > Note that n limits differ between lvds and other outputs. Strangely they've
> > all been correct already.
> > 
> > v2: Rebased on top of the DP pll rework, which makes it even more
> > obvious that we can do this ...
> 
> I'm a bit confused by this one:
> 
>   - I don't seem to find the special LVDS limits for n
>   - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake
>     and cantiga. The docs single out Cantiga and we seem to need a
>     different set of limits for that platform (compared to the rest of
>     gen4) platforms?
>   - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6
>     (cantiga)

I've looked at the mess around n values and decided to run the other
direction. My understanding is that the min/max in the xml are just the
limits of the table and not the hw (i.e. with the given clock ranges and
precision all the most precise clock settings happen to only use that
range).

For the actual values they seem to agree with my docs here with the
exception of TV-out. I have no idea what's going on there really, but at
least this patch here shouldn't break stuff. Otoh native TV-out seems to
have it's own pll and also pipe timing registers, so maybe it simply
doesn't matter.

>   - m doesn't seem to match what I have, it seems like it could be the
>     cantiga values (except that m is in "formula space" so we don't
>     substract 2)

Yeah, m is in formula space, since we compare that limit with what
i9xx_dpll_compute_m does.

>   - m1 and m2 seem to match what I have for cantiga, are we supposed to
>     have those limits for eaglelake as well?

Afaik eaglelake as the desktop version of cantiga has all the same stuff
(minus a few mobile-only power saving features). The spreadsheet I have
here is even called eaglelake_cantiga_something.xls.

> Well, the only conclusion is that I must be reading the wrong docs, or?

I guess I didn't really help :(
-Daniel

> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++------------
> >  1 file changed, 8 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index ea8eb0c..cb54131 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = {
> >  		.p2_slow = 14, .p2_fast = 7 },
> >  };
> >  
> > +#define G4X_DPLL_M_LIMITS \
> > +	.m = { .min = 104, .max = 138 },\
> > +	.m1 = { .min = 15, .max = 21 },\
> > +	.m2 = { .min = 3, .max = 11 },
> >  
> >  static const intel_limit_t intel_limits_g4x_sdvo = {
> >  	.dot = { .min = 25000, .max = 270000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 10, .max = 30 },
> >  	.p1 = { .min = 1, .max = 3},
> >  	.p2 = { .dot_limit = 270000,
> > @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = {
> >  	.dot = { .min = 22000, .max = 400000 },
> >  	.vco = { .min = 1750000, .max = 3500000},
> >  	.n = { .min = 1, .max = 4 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 16, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 5, .max = 80 },
> >  	.p1 = { .min = 1, .max = 8},
> >  	.p2 = { .dot_limit = 165000,
> > @@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = {
> >  	.dot = { .min = 20000, .max = 115000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 28, .max = 112 },
> >  	.p1 = { .min = 2, .max = 8 },
> >  	.p2 = { .dot_limit = 0,
> > @@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
> >  	.dot = { .min = 80000, .max = 224000 },
> >  	.vco = { .min = 1750000, .max = 3500000 },
> >  	.n = { .min = 1, .max = 3 },
> > -	.m = { .min = 104, .max = 138 },
> > -	.m1 = { .min = 17, .max = 23 },
> > -	.m2 = { .min = 5, .max = 11 },
> > +	G4X_DPLL_M_LIMITS
> >  	.p = { .min = 14, .max = 42 },
> >  	.p1 = { .min = 2, .max = 6 },
> >  	.p2 = { .dot_limit = 0,
> > -- 
> > 1.7.11.7
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@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

Re: [PATCH 4/9] drm/i915: fixup g4x pll limits

[Damien Lespiau]

On Wed, Jun 12, 2013 at 09:39:54AM +0200, Daniel Vetter wrote:
> Afaik eaglelake as the desktop version of cantiga has all the same stuff
> (minus a few mobile-only power saving features). The spreadsheet I have
> here is even called eaglelake_cantiga_something.xls.

Ah right, so let's believe this spreadsheet, it's more convincing that
the pure hw limits. With that:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

[PATCH 5/9] drm/i915: pnv dpll doesn't use m1!

[Daniel Vetter]

So don't try to store it in the DPLL_FP register.

Otherwise it looks like the limits for pineview are correct: It has
it's own clock computation code, which doesn't use an offset for n
divisors, and the register value based m limits look sane enough.

v2: Rebase on top of the pineview clock refactor and fixup up the
commit message: It's m1 pnv doens't care about, not m2!

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb54131..520e340 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4300,7 +4300,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
 
 static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
 {
-	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
+	return (1 << dpll->n) << 16 | dpll->m2;
 }
 
 static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
-- 
1.7.11.7

Re: [PATCH 5/9] drm/i915: pnv dpll doesn't use m1!

[Damien Lespiau]

On Tue, May 21, 2013 at 09:54:55PM +0200, Daniel Vetter wrote:
> So don't try to store it in the DPLL_FP register.
> 
> Otherwise it looks like the limits for pineview are correct: It has
> it's own clock computation code, which doesn't use an offset for n
> divisors, and the register value based m limits look sane enough.

  - n can vary between 2 and 6, but we declare the 3-6 as limits.
  - p1 seems to be able to go up to 9
  - the m upper limit seems a bit big, but the docs are a bit shy on
    that values for pnv.

Otherwise, the change itself seems good:

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

> 
> v2: Rebase on top of the pineview clock refactor and fixup up the
> commit message: It's m1 pnv doens't care about, not m2!
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index cb54131..520e340 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4300,7 +4300,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
>  
>  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
>  {
> -	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
> +	return (1 << dpll->n) << 16 | dpll->m2;
>  }
>  
>  static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/9] drm/i915: pnv dpll doesn't use m1!

[Daniel Vetter]

On Wed, Jun 12, 2013 at 12:38:40AM +0100, Damien Lespiau wrote:
> On Tue, May 21, 2013 at 09:54:55PM +0200, Daniel Vetter wrote:
> > So don't try to store it in the DPLL_FP register.
> > 
> > Otherwise it looks like the limits for pineview are correct: It has
> > it's own clock computation code, which doesn't use an offset for n
> > divisors, and the register value based m limits look sane enough.
> 
>   - n can vary between 2 and 6, but we declare the 3-6 as limits.
>   - p1 seems to be able to go up to 9
>   - the m upper limit seems a bit big, but the docs are a bit shy on
>     that values for pnv.
> 
> Otherwise, the change itself seems good:

Quoted in the commit message and merged the patch.

> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Thanks for the review, Daniel

> 
> > 
> > v2: Rebase on top of the pineview clock refactor and fixup up the
> > commit message: It's m1 pnv doens't care about, not m2!
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index cb54131..520e340 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -4300,7 +4300,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors)
> >  
> >  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
> >  {
> > -	return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2;
> > +	return (1 << dpll->n) << 16 | dpll->m2;
> >  }
> >  
> >  static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll)
> > -- 
> > 1.7.11.7
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@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

[PATCH 6/9] drm/i915: fix ibx/cpt/ppt dpll limits

[Daniel Vetter]

Now this was broken in pretty fundamental ways:
- M1/M2 have been consistently off by 2 and used doc values instead of
  the two less registers values our code expects.
- M/N limits often were too small by seemingly arbitrary amounts. I
  suspect this started to work around issues due to the wrong M1/M2
  limits.

Rectify this all and consolidate the limits a bit with a #define where
the docs say that they should be equal.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++-------------------------
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 520e340..80698ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -217,18 +217,16 @@ static const intel_limit_t intel_limits_pineview_lvds = {
 		.p2_slow = 14, .p2_fast = 14 },
 };
 
-/* Ironlake / Sandybridge
- *
- * We calculate clock using (register_value + 2) for N/M1/M2, so here
- * the range value for them is (actual_value - 2).
- */
+#define ILK_DPLL_M_N_LIMITS \
+	.n = { .min = 1, .max = 6 }, \
+	.m = { .min = 79, .max = 127 }, \
+	.m1 = { .min = 10, .max = 20 }, \
+	.m2 = { .min = 3, .max = 7 }, \
+
 static const intel_limit_t intel_limits_ironlake_dac = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 1760000, .max = 3510000 },
-	.n = { .min = 1, .max = 5 },
-	.m = { .min = 79, .max = 127 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	ILK_DPLL_M_N_LIMITS
 	.p = { .min = 5, .max = 80 },
 	.p1 = { .min = 1, .max = 8 },
 	.p2 = { .dot_limit = 225000,
@@ -238,10 +236,7 @@ static const intel_limit_t intel_limits_ironlake_dac = {
 static const intel_limit_t intel_limits_ironlake_single_lvds = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 1760000, .max = 3510000 },
-	.n = { .min = 1, .max = 3 },
-	.m = { .min = 79, .max = 118 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	ILK_DPLL_M_N_LIMITS
 	.p = { .min = 28, .max = 112 },
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
@@ -251,10 +246,7 @@ static const intel_limit_t intel_limits_ironlake_single_lvds = {
 static const intel_limit_t intel_limits_ironlake_dual_lvds = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 1760000, .max = 3510000 },
-	.n = { .min = 1, .max = 3 },
-	.m = { .min = 79, .max = 127 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	ILK_DPLL_M_N_LIMITS
 	.p = { .min = 14, .max = 56 },
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
@@ -265,10 +257,7 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds = {
 static const intel_limit_t intel_limits_ironlake_single_lvds_100m = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 1760000, .max = 3510000 },
-	.n = { .min = 1, .max = 2 },
-	.m = { .min = 79, .max = 126 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	ILK_DPLL_M_N_LIMITS
 	.p = { .min = 28, .max = 112 },
 	.p1 = { .min = 2, .max = 8 },
 	.p2 = { .dot_limit = 225000,
@@ -278,10 +267,7 @@ static const intel_limit_t intel_limits_ironlake_single_lvds_100m = {
 static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 	.dot = { .min = 25000, .max = 350000 },
 	.vco = { .min = 1760000, .max = 3510000 },
-	.n = { .min = 1, .max = 3 },
-	.m = { .min = 79, .max = 126 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
+	ILK_DPLL_M_N_LIMITS
 	.p = { .min = 14, .max = 42 },
 	.p1 = { .min = 2, .max = 6 },
 	.p2 = { .dot_limit = 225000,
-- 
1.7.11.7

Re: [PATCH 6/9] drm/i915: fix ibx/cpt/ppt dpll limits

[Damien Lespiau]

On Tue, May 21, 2013 at 09:54:56PM +0200, Daniel Vetter wrote:
> Now this was broken in pretty fundamental ways:
> - M1/M2 have been consistently off by 2 and used doc values instead of
>   the two less registers values our code expects.
> - M/N limits often were too small by seemingly arbitrary amounts. I
>   suspect this started to work around issues due to the wrong M1/M2
>   limits.
> 
> Rectify this all and consolidate the limits a bit with a #define where
> the docs say that they should be equal.

So this time I'm reading ibexpeak_xxx.xls

> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++-------------------------
>  1 file changed, 11 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 520e340..80698ac 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -217,18 +217,16 @@ static const intel_limit_t intel_limits_pineview_lvds = {
>  		.p2_slow = 14, .p2_fast = 14 },
>  };
>  
> -/* Ironlake / Sandybridge
> - *
> - * We calculate clock using (register_value + 2) for N/M1/M2, so here
> - * the range value for them is (actual_value - 2).
> - */
> +#define ILK_DPLL_M_N_LIMITS \
> +	.n = { .min = 1, .max = 6 }, \
> +	.m = { .min = 79, .max = 127 }, \
> +	.m1 = { .min = 10, .max = 20 }, \
> +	.m2 = { .min = 3, .max = 7 }, \
> +
>  static const intel_limit_t intel_limits_ironlake_dac = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 1760000, .max = 3510000 },
> -	.n = { .min = 1, .max = 5 },
> -	.m = { .min = 79, .max = 127 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> +	ILK_DPLL_M_N_LIMITS
>  	.p = { .min = 5, .max = 80 },
>  	.p1 = { .min = 1, .max = 8 },
>  	.p2 = { .dot_limit = 225000,

In ibexpeak_xxxx.xsl I have n between 3 and 7 (so 1,5 seemed correct)
for DAC. It also restrics M between 79 and 118.

> @@ -238,10 +236,7 @@ static const intel_limit_t intel_limits_ironlake_dac = {
>  static const intel_limit_t intel_limits_ironlake_single_lvds = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 1760000, .max = 3510000 },
> -	.n = { .min = 1, .max = 3 },
> -	.m = { .min = 79, .max = 118 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> +	ILK_DPLL_M_N_LIMITS
>  	.p = { .min = 28, .max = 112 },
>  	.p1 = { .min = 2, .max = 8 },
>  	.p2 = { .dot_limit = 225000,
> @@ -251,10 +246,7 @@ static const intel_limit_t intel_limits_ironlake_single_lvds = {
>  static const intel_limit_t intel_limits_ironlake_dual_lvds = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 1760000, .max = 3510000 },
> -	.n = { .min = 1, .max = 3 },
> -	.m = { .min = 79, .max = 127 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> +	ILK_DPLL_M_N_LIMITS
>  	.p = { .min = 14, .max = 56 },
>  	.p1 = { .min = 2, .max = 8 },
>  	.p2 = { .dot_limit = 225000,

For single channel LVDS with ref clock at 120MHz I have n between 3 and
5 (so 1,3 seemed correct). M is restricted to 79,118 here as well

> @@ -265,10 +257,7 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds = {
>  static const intel_limit_t intel_limits_ironlake_single_lvds_100m = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 1760000, .max = 3510000 },
> -	.n = { .min = 1, .max = 2 },
> -	.m = { .min = 79, .max = 126 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> +	ILK_DPLL_M_N_LIMITS
>  	.p = { .min = 28, .max = 112 },
>  	.p1 = { .min = 2, .max = 8 },
>  	.p2 = { .dot_limit = 225000,

For dual channel LVDS, I have n between 3 and 5, so 1,3? M between
79,127

> @@ -278,10 +267,7 @@ static const intel_limit_t intel_limits_ironlake_single_lvds_100m = {
>  static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
>  	.dot = { .min = 25000, .max = 350000 },
>  	.vco = { .min = 1760000, .max = 3510000 },
> -	.n = { .min = 1, .max = 3 },
> -	.m = { .min = 79, .max = 126 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> +	ILK_DPLL_M_N_LIMITS
>  	.p = { .min = 14, .max = 42 },
>  	.p1 = { .min = 2, .max = 6 },
>  	.p2 = { .dot_limit = 225000,

And there n between 3,4 so (1,2), M was indeed between 79 and 126

-- 
Damien

[PATCH 7/9] drm/i915: store adjust dotclock in adjustede_mode->clock

[Daniel Vetter]

... 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 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, if we ever bother with that) multiplier 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.

Signed-off-by: Daniel Vetter <daniel.vetter@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 +--
 5 files changed, 48 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e668521..809dfd6 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 80698ac..fb481d9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4044,7 +4044,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:
@@ -4057,19 +4057,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,
@@ -4398,8 +4394,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;
@@ -4452,7 +4446,7 @@ static void vlv_update_pll(struct intel_crtc *crtc)
 	intel_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))
 		intel_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe),
 				 0x005f0021);
@@ -4897,7 +4891,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");
@@ -5506,7 +5501,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)
@@ -5534,7 +5528,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;
@@ -5736,7 +5731,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_crtc->config.cpu_transcoder = pipe;
 
-	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");
@@ -5966,7 +5961,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
 
-	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... */
@@ -7789,6 +7784,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.
@@ -7817,6 +7815,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 7c5eb2f..b1dfb0f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -691,7 +691,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) && !is_cpu_edp(intel_dp))
 		pipe_config->has_pch_encoder = true;
@@ -708,8 +708,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;
@@ -725,7 +723,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) {
@@ -760,18 +758,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);
@@ -794,24 +791,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);
@@ -827,8 +828,7 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-	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:
@@ -858,7 +858,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);
 	}
@@ -877,13 +877,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) || is_cpu_edp(intel_dp)) {
 		if (!HAS_PCH_SPLIT(dev) && !IS_VALLEYVIEW(dev))
 			intel_dp->DP |= intel_dp->color_range;
@@ -897,22 +891,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 (is_cpu_edp(intel_dp) && !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 (is_cpu_edp(intel_dp) && !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 a435ce1..9d5ab4a 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;
 
@@ -778,7 +779,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 18f8ce0..60b0d1d 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;
-- 
1.7.11.7

Re: [PATCH 7/9] drm/i915: store adjust dotclock in adjustede_mode->clock

[Jesse Barnes]

On Tue, 21 May 2013 21:54:57 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

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

Nitpick, this seems like unrelated code motion?  It's probably more
sensible to do it here though, so no objection.

I'm sure I missed something on a GM45 DP docked config or a VLV
all-in-one with DP->LVDS bridge, but overall it looks ok.

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

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] drm/i915: store adjusted dotclock in adjustede_mode->clock

[Daniel Vetter]

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

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

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@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 +--
 5 files changed, 48 insertions(+), 59 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 88369db..ab190cb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3945,7 +3945,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:
@@ -3958,19 +3958,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,
@@ -4299,8 +4295,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;
@@ -4353,7 +4347,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);
@@ -4798,7 +4792,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");
@@ -5409,7 +5404,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)
@@ -5437,7 +5431,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;
@@ -5637,7 +5632,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");
@@ -5862,7 +5857,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	WARN_ON(I915_READ(DSPCNTR(plane)) & DISPLAY_PLANE_ENABLE);
 
-	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... */
@@ -7706,6 +7701,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.
@@ -7734,6 +7732,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 bb4c50b..3db4b14 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;
 
@@ -784,7 +785,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;
-- 
1.7.11.7

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

[Daniel Vetter]

... 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@virtuousgeek.org> (v3)
Signed-off-by: Daniel Vetter <daniel.vetter@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

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

[Paulo Zanoni]

2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>:
> ... 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@virtuousgeek.org> (v3)
> Signed-off-by: Daniel Vetter <daniel.vetter@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);

This seems to change how we set m_n values, which is not the goal of
the patch. Can't we try to do this in a separate patch? I've spent a
lot of time in the last months bisecting display code, and the big
patches were the ones I spent most time debugging...


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

I really think that all the code that moves these lines above belong
to a separate patch. This patch is already too big and if we bisect
eDP regressions to it we'll have a lot to debug. It looks correct, but
it's better to be safe, especially considering all those complicated
gen+port+pch checks inside the intel_dp_mode_set.


>         }
>
>         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
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

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

[Paulo Zanoni]

2013/5/31 Paulo Zanoni <przanoni@gmail.com>:
> 2013/5/31 Daniel Vetter <daniel.vetter@ffwll.ch>:
>> ... 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@virtuousgeek.org> (v3)
>> Signed-off-by: Daniel Vetter <daniel.vetter@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);
>
> This seems to change how we set m_n values, which is not the goal of
> the patch. Can't we try to do this in a separate patch? I've spent a
> lot of time in the last months bisecting display code, and the big
> patches were the ones I spent most time debugging...
>
>
>>
>>         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;
>
> I really think that all the code that moves these lines above belong
> to a separate patch. This patch is already too big and if we bisect
> eDP regressions to it we'll have a lot to debug. It looks correct, but
> it's better to be safe, especially considering all those complicated
> gen+port+pch checks inside the intel_dp_mode_set.

Also, I can't find a way to apply this patch to dinq. The patch alone
doesn't apply. It used to be the 7th patch of the PLL series, but that
series also needs some rebasing.

>
>
>>         }
>>
>>         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
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni



-- 
Paulo Zanoni

[PATCH 8/9] drm/i915: Drop some no longer required mode/adjusted_mode parameters

[Daniel Vetter]

We can get at this easily through intel_crtc->config now.

v2: Drop more stuff gcc spotted.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fb481d9..25fa8cd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4619,7 +4619,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 }
 
 static void i8xx_update_pll(struct intel_crtc *crtc,
-			    struct drm_display_mode *adjusted_mode,
 			    intel_clock_t *reduced_clock,
 			    int num_connectors)
 {
@@ -4674,14 +4673,15 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 	I915_WRITE(DPLL(pipe), dpll);
 }
 
-static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
-				   struct drm_display_mode *mode,
-				   struct drm_display_mode *adjusted_mode)
+static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config.adjusted_mode;
+	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
 
 	/* We need to be careful not to changed the adjusted mode, for otherwise
@@ -4859,8 +4859,6 @@ static int i9xx_crtc_mode_set(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 drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
@@ -4925,7 +4923,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	}
 
 	if (IS_GEN2(dev))
-		i8xx_update_pll(intel_crtc, adjusted_mode,
+		i8xx_update_pll(intel_crtc,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 	else if (IS_VALLEYVIEW(dev))
@@ -4948,7 +4946,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
 	drm_mode_debug_printmodeline(mode);
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	/* pipesrc and dspsize control the size that is scaled from,
 	 * which should always be the user's requested size.
@@ -5805,7 +5803,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		}
 	}
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
@@ -5975,7 +5973,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_crtc->lowfreq_avail = false;
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
-- 
1.7.11.7

[PATCH] drm/i915: Drop some no longer required mode/adjusted_mode parameters

[Daniel Vetter]

We can get at this easily through intel_crtc->config now.

v2: Drop more stuff gcc spotted.

v3: Drop even more stuff gcc spotted.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1213f7c..ae2823c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4526,7 +4526,6 @@ static void i9xx_update_pll(struct intel_crtc *crtc,
 }
 
 static void i8xx_update_pll(struct intel_crtc *crtc,
-			    struct drm_display_mode *adjusted_mode,
 			    intel_clock_t *reduced_clock,
 			    int num_connectors)
 {
@@ -4581,14 +4580,15 @@ static void i8xx_update_pll(struct intel_crtc *crtc,
 	I915_WRITE(DPLL(pipe), dpll);
 }
 
-static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
-				   struct drm_display_mode *mode,
-				   struct drm_display_mode *adjusted_mode)
+static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder;
+	struct drm_display_mode *adjusted_mode =
+		&intel_crtc->config.adjusted_mode;
+	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	uint32_t vsyncshift, crtc_vtotal, crtc_vblank_end;
 
 	/* We need to be careful not to changed the adjusted mode, for otherwise
@@ -4766,8 +4766,6 @@ static int i9xx_crtc_mode_set(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 drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
@@ -4832,7 +4830,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	}
 
 	if (IS_GEN2(dev))
-		i8xx_update_pll(intel_crtc, adjusted_mode,
+		i8xx_update_pll(intel_crtc,
 				has_reduced_clock ? &reduced_clock : NULL,
 				num_connectors);
 	else if (IS_VALLEYVIEW(dev))
@@ -4855,7 +4853,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
 	drm_mode_debug_printmodeline(mode);
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	/* pipesrc and dspsize control the size that is scaled from,
 	 * which should always be the user's requested size.
@@ -5610,8 +5608,6 @@ static int ironlake_crtc_mode_set(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 drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
@@ -5712,7 +5708,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 		}
 	}
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
@@ -5828,8 +5824,6 @@ static int haswell_crtc_mode_set(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 drm_display_mode *adjusted_mode =
-		&intel_crtc->config.adjusted_mode;
 	struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
 	int pipe = intel_crtc->pipe;
 	int plane = intel_crtc->plane;
@@ -5880,7 +5874,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
 
 	intel_crtc->lowfreq_avail = false;
 
-	intel_set_pipe_timings(intel_crtc, mode, adjusted_mode);
+	intel_set_pipe_timings(intel_crtc);
 
 	if (intel_crtc->config.has_pch_encoder) {
 		intel_cpu_transcoder_set_m_n(intel_crtc,
-- 
1.7.11.7

Re: [PATCH] drm/i915: Drop some no longer required mode/adjusted_mode parameters

[Damien Lespiau]

On Sun, May 26, 2013 at 05:48:15PM +0200, Daniel Vetter wrote:
> We can get at this easily through intel_crtc->config now.
> 
> v2: Drop more stuff gcc spotted.
> 
> v3: Drop even more stuff gcc spotted.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

[PATCH 9/9] drm/i915: check for strange pfit pipe assignemnt on ivb/hsw

[Daniel Vetter]

Panel fitters on ivb/hsw are not created equal since not all of them
support the new high-quality upscaling mode. To offset this the hw
allows us to freely assign the pfits to pipes.

Since our code currently doesn't support this we might fall over when
taking over firmware state. So check for this case and WARN about it.
We can then improve the code once we've hit this in the wild. Or once
we decide to support the improved upscale modes, though that requires
global arbitrage of modeset resources across crtcs.

Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 25fa8cd..73b6c9f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5856,6 +5856,14 @@ static void ironlake_get_pfit_config(struct intel_crtc *crtc,
 	if (tmp & PF_ENABLE) {
 		pipe_config->pch_pfit.pos = I915_READ(PF_WIN_POS(crtc->pipe));
 		pipe_config->pch_pfit.size = I915_READ(PF_WIN_SZ(crtc->pipe));
+
+		/* We currently do not free assignements of panel fitters on
+		 * ivb/hsw (since we don't use the higher upscaling modes which
+		 * differentiates them) so just WARN about this case for now. */
+		if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
+			WARN_ON((tmp & PF_PIPE_SEL_MASK_IVB) !=
+				PF_PIPE_SEL_IVB(crtc->pipe));
+		}
 	}
 }
 
-- 
1.7.11.7

Re: [PATCH 9/9] drm/i915: check for strange pfit pipe assignemnt on ivb/hsw

[Damien Lespiau]

On Tue, May 21, 2013 at 09:54:59PM +0200, Daniel Vetter wrote:
> Panel fitters on ivb/hsw are not created equal since not all of them
> support the new high-quality upscaling mode. To offset this the hw
> allows us to freely assign the pfits to pipes.
> 
> Since our code currently doesn't support this we might fall over when
> taking over firmware state. So check for this case and WARN about it.
> We can then improve the code once we've hit this in the wild. Or once
> we decide to support the improved upscale modes, though that requires
> global arbitrage of modeset resources across crtcs.
> 
> Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

This is already in with a IS_GEN7()

-- 
Damien