diff mbox

[8/8] drm/i915: move dp clock computations to encoder->compute_config

Message ID 1364463723-30252-9-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 28, 2013, 9:42 a.m. UTC
With the exception of hsw, which has dedicated DP clocks which run at
the fixed frequency already, and vlv, which doesn't have optmized
pre-defined dp clock parameters (yet).

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 97 +-----------------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 45 +++++++++++++++++
 2 files changed, 46 insertions(+), 96 deletions(-)

Comments

Jesse Barnes April 2, 2013, 9:20 p.m. UTC | #1
On Thu, 28 Mar 2013 10:42:03 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> With the exception of hsw, which has dedicated DP clocks which run at
> the fixed frequency already, and vlv, which doesn't have optmized
> pre-defined dp clock parameters (yet).
> 

Nice.  I think we should do this for common HDMI modes too.  We have
some extra clock manipulation regs we can use to tune things, so having
fixed dividers for 720p and 1080p along with the tuning params should
give us better behavior than what we have today.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Daniel Vetter April 2, 2013, 9:26 p.m. UTC | #2
On Tue, Apr 2, 2013 at 11:20 PM, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:

> On Thu, 28 Mar 2013 10:42:03 +0100
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > With the exception of hsw, which has dedicated DP clocks which run at
> > the fixed frequency already, and vlv, which doesn't have optmized
> > pre-defined dp clock parameters (yet).
> >
>
> Nice.  I think we should do this for common HDMI modes too.  We have
> some extra clock manipulation regs we can use to tune things, so having
> fixed dividers for 720p and 1080p along with the tuning params should
> give us better behavior than what we have today.


Imo we still have a few lower-hanging fruit before we need to start doing
clock fine-tuning in hdmi-land: Atm we don't really bother with supporting
the 1001/1000 modified clocks in the CEA spec at all ... Once we have the
support code for that, adding fine-tuned clocks for those modes starts to
make sense, so that we really hit them spot-on.
-Daniel
Daniel Vetter April 5, 2013, 6:51 p.m. UTC | #3
On Tue, Apr 02, 2013 at 11:26:06PM +0200, Daniel Vetter wrote:
> On Tue, Apr 2, 2013 at 11:20 PM, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:
> 
> > On Thu, 28 Mar 2013 10:42:03 +0100
> > Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > > With the exception of hsw, which has dedicated DP clocks which run at
> > > the fixed frequency already, and vlv, which doesn't have optmized
> > > pre-defined dp clock parameters (yet).
> > >
> >
> > Nice.  I think we should do this for common HDMI modes too.  We have
> > some extra clock manipulation regs we can use to tune things, so having
> > fixed dividers for 720p and 1080p along with the tuning params should
> > give us better behavior than what we have today.
> 
> 
> Imo we still have a few lower-hanging fruit before we need to start doing
> clock fine-tuning in hdmi-land: Atm we don't really bother with supporting
> the 1001/1000 modified clocks in the CEA spec at all ... Once we have the
> support code for that, adding fine-tuned clocks for those modes starts to
> make sense, so that we really hit them spot-on.

I've dropped this patch here again for now, since the previous patch is
horribly broken: It completely missed to transform the ironlake dpll
functions over to use pipe_config->dpll instead of the intel_clock_t
computed. Thanks to Paulo for dissecting this embarassement.

/me hides in shame

Cheers, Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5319133..f1cabf3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -114,15 +114,6 @@  intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 			intel_clock_t *best_clock);
 
 static bool
-intel_find_pll_g4x_dp(const intel_limit_t *, struct drm_crtc *crtc,
-		      int target, int refclk, intel_clock_t *match_clock,
-		      intel_clock_t *best_clock);
-static bool
-intel_find_pll_ironlake_dp(const intel_limit_t *, 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);
@@ -254,20 +245,6 @@  static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
 	.find_pll = intel_g4x_find_best_PLL,
 };
 
-static const intel_limit_t intel_limits_g4x_display_port = {
-	.dot = { .min = 161670, .max = 227000 },
-	.vco = { .min = 1750000, .max = 3500000},
-	.n = { .min = 1, .max = 2 },
-	.m = { .min = 97, .max = 108 },
-	.m1 = { .min = 0x10, .max = 0x12 },
-	.m2 = { .min = 0x05, .max = 0x06 },
-	.p = { .min = 10, .max = 20 },
-	.p1 = { .min = 1, .max = 2},
-	.p2 = { .dot_limit = 0,
-		.p2_slow = 10, .p2_fast = 10 },
-	.find_pll = intel_find_pll_g4x_dp,
-};
-
 static const intel_limit_t intel_limits_pineview_sdvo = {
 	.dot = { .min = 20000, .max = 400000},
 	.vco = { .min = 1700000, .max = 3500000 },
@@ -374,20 +351,6 @@  static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 	.find_pll = intel_g4x_find_best_PLL,
 };
 
-static const intel_limit_t intel_limits_ironlake_display_port = {
-	.dot = { .min = 25000, .max = 350000 },
-	.vco = { .min = 1760000, .max = 3510000},
-	.n = { .min = 1, .max = 2 },
-	.m = { .min = 81, .max = 90 },
-	.m1 = { .min = 12, .max = 22 },
-	.m2 = { .min = 5, .max = 9 },
-	.p = { .min = 10, .max = 20 },
-	.p1 = { .min = 1, .max = 2},
-	.p2 = { .dot_limit = 0,
-		.p2_slow = 10, .p2_fast = 10 },
-	.find_pll = intel_find_pll_ironlake_dp,
-};
-
 static const intel_limit_t intel_limits_vlv_dac = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 4000000, .max = 6000000 },
@@ -497,10 +460,7 @@  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 			else
 				limit = &intel_limits_ironlake_single_lvds;
 		}
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
-		   intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
-		limit = &intel_limits_ironlake_display_port;
-	else
+	} else
 		limit = &intel_limits_ironlake_dac;
 
 	return limit;
@@ -521,8 +481,6 @@  static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
 		limit = &intel_limits_g4x_hdmi;
 	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
 		limit = &intel_limits_g4x_sdvo;
-	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
-		limit = &intel_limits_g4x_display_port;
 	} else /* The option is for other outputs */
 		limit = &intel_limits_i9xx_sdvo;
 
@@ -765,59 +723,6 @@  intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
 }
 
 static bool
-intel_find_pll_ironlake_dp(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;
-
-	if (target < 200000) {
-		clock.n = 1;
-		clock.p1 = 2;
-		clock.p2 = 10;
-		clock.m1 = 12;
-		clock.m2 = 9;
-	} else {
-		clock.n = 2;
-		clock.p1 = 1;
-		clock.p2 = 10;
-		clock.m1 = 14;
-		clock.m2 = 8;
-	}
-	intel_clock(dev, refclk, &clock);
-	memcpy(best_clock, &clock, sizeof(intel_clock_t));
-	return true;
-}
-
-/* DisplayPort has only two frequencies, 162MHz and 270MHz */
-static bool
-intel_find_pll_g4x_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
-		      int target, int refclk, intel_clock_t *match_clock,
-		      intel_clock_t *best_clock)
-{
-	intel_clock_t clock;
-	if (target < 200000) {
-		clock.p1 = 2;
-		clock.p2 = 10;
-		clock.n = 2;
-		clock.m1 = 23;
-		clock.m2 = 8;
-	} else {
-		clock.p1 = 1;
-		clock.p2 = 10;
-		clock.n = 1;
-		clock.m1 = 14;
-		clock.m2 = 2;
-	}
-	clock.m = 5 * (clock.m1 + 2) + (clock.m2 + 2);
-	clock.p = (clock.p1 * clock.p2);
-	clock.dot = 96000 * clock.m / (clock.n + 2) / clock.p;
-	clock.vco = 0;
-	memcpy(best_clock, &clock, sizeof(intel_clock_t));
-	return true;
-}
-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)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b1bf00b..78006a7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -670,6 +670,49 @@  intel_dp_i2c_init(struct intel_dp *intel_dp,
 	return ret;
 }
 
+static void
+intel_dp_set_clock(struct intel_encoder *encoder,
+		   struct intel_crtc_config *pipe_config, int link_bw)
+{
+	struct drm_device *dev = encoder->base.dev;
+
+	if (IS_G4X(dev)) {
+		if (link_bw == DP_LINK_BW_1_62) {
+			pipe_config->dpll.p1 = 2;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.n = 2;
+			pipe_config->dpll.m1 = 23;
+			pipe_config->dpll.m2 = 8;
+		} else {
+			pipe_config->dpll.p1 = 1;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.n = 1;
+			pipe_config->dpll.m1 = 14;
+			pipe_config->dpll.m2 = 2;
+		}
+		pipe_config->clock_set = true;
+	} else if (IS_HASWELL(dev)) {
+		/* Haswell has special-purpose DP DDI clocks. */
+	} else if (HAS_PCH_SPLIT(dev)) {
+		if (link_bw == DP_LINK_BW_1_62) {
+			pipe_config->dpll.n = 1;
+			pipe_config->dpll.p1 = 2;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.m1 = 12;
+			pipe_config->dpll.m2 = 9;
+		} else {
+			pipe_config->dpll.n = 2;
+			pipe_config->dpll.p1 = 1;
+			pipe_config->dpll.p2 = 10;
+			pipe_config->dpll.m1 = 14;
+			pipe_config->dpll.m2 = 8;
+		}
+		pipe_config->clock_set = true;
+	} else if (IS_VALLEYVIEW(dev)) {
+		/* FIXME: Need to figure out optimized DP clocks for vlv. */
+	}
+}
+
 bool
 intel_dp_compute_config(struct intel_encoder *encoder,
 			struct intel_crtc_config *pipe_config)
@@ -765,6 +808,8 @@  found:
 			       target_clock, adjusted_mode->clock,
 			       &pipe_config->dp_m_n);
 
+	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
+
 	return true;
 }