Re: [PATCH 2/2] drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

Re: [PATCH 2/2] drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

[Daniel Vetter]

On Wed, Sep 04, 2013 at 01:30:38AM +0800, Chon Ming Lee wrote:
> For DP pll settings, there is only two golden configs.  Instead of
> running through the algorithm to determine it, hardcode the value and get it
> determine in intel_dp_set_clock.
> 
> v2: Rework on the intel_limit compiler warning. (Jani)
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>

lgtm, both patches merged. Note that checkpatch.pl was a bit unhappy about
the structure initalization layout, so I've fixed this up while applying.

I'm not too insisting on checkpatch clean code (especially the 80 char
limit for debug output often results in ugly code so is better ignored in
those cases). So please run it before submitting patches and fix up
reported issues (if it doesn't result in ugly code ofc).

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |   20 +++-----------------
>  drivers/gpu/drm/i915/intel_dp.c      |   11 ++++++++++-
>  2 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f526ea9..1a567d2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
>  		.p2_slow = 2, .p2_fast = 20 },
>  };
>  
> -static const intel_limit_t intel_limits_vlv_dp = {
> -	.dot = { .min = 25000, .max = 270000 },
> -	.vco = { .min = 4000000, .max = 6000000 },
> -	.n = { .min = 1, .max = 7 },
> -	.m = { .min = 22, .max = 450 },
> -	.m1 = { .min = 2, .max = 3 },
> -	.m2 = { .min = 11, .max = 156 },
> -	.p = { .min = 10, .max = 30 },
> -	.p1 = { .min = 1, .max = 3 },
> -	.p2 = { .dot_limit = 270000,
> -		.p2_slow = 2, .p2_fast = 20 },
> -};
> -
>  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>  						int refclk)
>  {
> @@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
>  			limit = &intel_limits_vlv_dac;
> -		else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> -			limit = &intel_limits_vlv_hdmi;
>  		else
> -			limit = &intel_limits_vlv_dp;
> +			limit = &intel_limits_vlv_hdmi;
>  	} else if (!IS_GEN2(dev)) {
>  		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i9xx_lvds;
> @@ -4890,7 +4875,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	refclk = i9xx_get_refclk(crtc, num_connectors);
>  
> -	if (!is_dsi) {
> +	if (!is_dsi && !intel_crtc->config.clock_set) {
>  		/*
>  		 * Returns a set of divisors for the desired target clock with
>  		 * the given refclk, or FALSE.  The returned values represent
> @@ -4917,6 +4902,7 @@ 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.
>  		*/
> +		limit = intel_limit(crtc, refclk);
>  		has_reduced_clock =
>  			dev_priv->display.find_dpll(limit, crtc,
>  						    dev_priv->lvds_downclock,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fd09058..76b4372 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -59,6 +59,14 @@ static const struct dp_link_dpll pch_dpll[] =
>  		{ .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }}
>  };
>  
> +static const struct dp_link_dpll vlv_dpll[] =
> +{
> +	{ DP_LINK_BW_1_62,
> +		{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 }},
> +	{ DP_LINK_BW_2_7,
> +		{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 }}
> +};
> +
>  /**
>   * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
>   * @intel_dp: DP struct
> @@ -682,7 +690,8 @@ intel_dp_set_clock(struct intel_encoder *encoder,
>  		divisor = pch_dpll;
>  		count = ARRAY_SIZE(pch_dpll);
>  	} else if (IS_VALLEYVIEW(dev)) {
> -		/* FIXME: Need to figure out optimized DP clocks for vlv. */
> +		divisor = vlv_dpll;
> +		count = ARRAY_SIZE(vlv_dpll);
>  	}
>  
>  	if (divisor && count) {
> -- 
> 1.7.7.6
> 
> _______________________________________________
> 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 1/2] drm/i915: Modify DP set clock to accomodate more eDP timings v2

[Chon Ming Lee]

eDP 1.4 supports 4-5 extra link rates in additional to current 2 link
rate.  Create a structure to store the DPLL divisor data to improve
readability.

v2: Fix the gen4_dpll/pch_dpll initialization to C99
designated initializers, and use a single loop for all platforms. (Jani and Daniel)

Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   65 ++++++++++++++++++++++-----------------
 1 files changed, 37 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2151d13..fd09058 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,6 +38,27 @@
 
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
+struct dp_link_dpll {
+	int link_bw;
+	struct dpll dpll;
+};
+
+static const struct dp_link_dpll gen4_dpll[] =
+{
+	{ DP_LINK_BW_1_62,
+		{ .p1 = 2, .p2 = 10, .n = 2, .m1 = 23, .m2 = 8 }},
+	{ DP_LINK_BW_2_7,
+		{ .p1 = 1, .p2 = 10, .n = 1, .m1 = 14, .m2 = 2 }}
+};
+
+static const struct dp_link_dpll pch_dpll[] =
+{
+	{ DP_LINK_BW_1_62,
+		{ .p1 = 2, .p2 = 10, .n = 1, .m1 = 12, .m2 = 9 }},
+	{ DP_LINK_BW_2_7,
+		{ .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }}
+};
+
 /**
  * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
  * @intel_dp: DP struct
@@ -649,42 +670,30 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 		   struct intel_crtc_config *pipe_config, int link_bw)
 {
 	struct drm_device *dev = encoder->base.dev;
+	const struct dp_link_dpll *divisor = NULL;
+	int i, count = 0;
 
 	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;
+		divisor = gen4_dpll;
+		count = ARRAY_SIZE(gen4_dpll);
 	} 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;
+		divisor = pch_dpll;
+		count = ARRAY_SIZE(pch_dpll);
 	} else if (IS_VALLEYVIEW(dev)) {
 		/* FIXME: Need to figure out optimized DP clocks for vlv. */
 	}
+
+	if (divisor && count) {
+		for (i = 0; i < count; i++) {
+			if (link_bw == divisor[i].link_bw) {
+				pipe_config->dpll = divisor[i].dpll;
+				pipe_config->clock_set = true;
+				break;
+			}
+		}
+	}
 }
 
 bool
-- 
1.7.7.6

[PATCH 2/2] drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

[Chon Ming Lee]

For DP pll settings, there is only two golden configs.  Instead of
running through the algorithm to determine it, hardcode the value and get it
determine in intel_dp_set_clock.

v2: Rework on the intel_limit compiler warning. (Jani)

Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   20 +++-----------------
 drivers/gpu/drm/i915/intel_dp.c      |   11 ++++++++++-
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f526ea9..1a567d2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 		.p2_slow = 2, .p2_fast = 20 },
 };
 
-static const intel_limit_t intel_limits_vlv_dp = {
-	.dot = { .min = 25000, .max = 270000 },
-	.vco = { .min = 4000000, .max = 6000000 },
-	.n = { .min = 1, .max = 7 },
-	.m = { .min = 22, .max = 450 },
-	.m1 = { .min = 2, .max = 3 },
-	.m2 = { .min = 11, .max = 156 },
-	.p = { .min = 10, .max = 30 },
-	.p1 = { .min = 1, .max = 3 },
-	.p2 = { .dot_limit = 270000,
-		.p2_slow = 2, .p2_fast = 20 },
-};
-
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
 						int refclk)
 {
@@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
 	} else if (IS_VALLEYVIEW(dev)) {
 		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
 			limit = &intel_limits_vlv_dac;
-		else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
-			limit = &intel_limits_vlv_hdmi;
 		else
-			limit = &intel_limits_vlv_dp;
+			limit = &intel_limits_vlv_hdmi;
 	} else if (!IS_GEN2(dev)) {
 		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
@@ -4890,7 +4875,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 
 	refclk = i9xx_get_refclk(crtc, num_connectors);
 
-	if (!is_dsi) {
+	if (!is_dsi && !intel_crtc->config.clock_set) {
 		/*
 		 * Returns a set of divisors for the desired target clock with
 		 * the given refclk, or FALSE.  The returned values represent
@@ -4917,6 +4902,7 @@ 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.
 		*/
+		limit = intel_limit(crtc, refclk);
 		has_reduced_clock =
 			dev_priv->display.find_dpll(limit, crtc,
 						    dev_priv->lvds_downclock,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fd09058..76b4372 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,6 +59,14 @@ static const struct dp_link_dpll pch_dpll[] =
 		{ .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }}
 };
 
+static const struct dp_link_dpll vlv_dpll[] =
+{
+	{ DP_LINK_BW_1_62,
+		{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 }},
+	{ DP_LINK_BW_2_7,
+		{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 }}
+};
+
 /**
  * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
  * @intel_dp: DP struct
@@ -682,7 +690,8 @@ intel_dp_set_clock(struct intel_encoder *encoder,
 		divisor = pch_dpll;
 		count = ARRAY_SIZE(pch_dpll);
 	} else if (IS_VALLEYVIEW(dev)) {
-		/* FIXME: Need to figure out optimized DP clocks for vlv. */
+		divisor = vlv_dpll;
+		count = ARRAY_SIZE(vlv_dpll);
 	}
 
 	if (divisor && count) {
-- 
1.7.7.6