All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/15] Clean up ironlake clock computation code
@ 2016-03-21 16:00 Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code Ander Conselvan de Oliveira
                   ` (16 more replies)
  0 siblings, 17 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Hi,

I updated this patch series to also clean up the GMCH pll code. Most of
the ironlake patches have been reviewed by Maarten already. The first
one was extended to remove the num_connector checks from the GMCH code.

Ville, does this address your concern that ILK and GMCH code should be
similar?

Thanks,
Ander

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ander Conselvan de Oliveira (15):
  drm/i915: Remove checks for cloned config with LVDS in dpll code
  drm/i915: Merge ironlake_get_refclk() into its only caller
  drm/i915: Fold intel_ironlake_limit() into clock computation function
  drm/i915: Call g4x_find_best_dpll() directly from ILK+ code
  drm/i915: Simplify ironlake reduced clock logic a bit
  drm/i915: Don't calculate a new clock in ILK+ code if it is already
    set
  drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock()
  drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case
  drm/i915: Pass crtc_state->dpll directly to ->find_dpll()
  drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
  drm/i915: Merge ironlake_compute_clocks() and
    ironlake_crtc_compute_clock()
  drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks
  drm/i915: Split gen2_crtc_compute_clock()
  drm/i915: Split g4x_crtc_compute_clock()
  drm/i915: Split PNV version of crtc_compute_clock()

 drivers/gpu/drm/i915/i915_drv.h      |  18 --
 drivers/gpu/drm/i915/intel_display.c | 610 ++++++++++++++++++-----------------
 2 files changed, 315 insertions(+), 313 deletions(-)

-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 10:11   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 02/15] drm/i915: Merge ironlake_get_refclk() into its only caller Ander Conselvan de Oliveira
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

LVDS is not cloneable, so the check is unnecessary. Removing it makes
the code neater.

v2: Remove checks from GMCH code too, not only ILK+. (Ville)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 70 +++++++++---------------------------
 1 file changed, 16 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 28ead66..710fd9a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -113,8 +113,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
-static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
-			   int num_connectors);
+static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(struct intel_crtc *crtc);
 static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
 static void ironlake_pfit_enable(struct intel_crtc *crtc);
@@ -1076,7 +1075,7 @@ chv_find_best_dpll(const intel_limit_t *limit,
 bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
 			intel_clock_t *best_clock)
 {
-	int refclk = i9xx_get_refclk(crtc_state, 0);
+	int refclk = i9xx_get_refclk(crtc_state);
 
 	return chv_find_best_dpll(intel_limit(crtc_state, refclk), crtc_state,
 				  target_clock, refclk, NULL, best_clock);
@@ -7113,8 +7112,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
-			   int num_connectors)
+static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7125,7 +7123,7 @@ static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) {
 		refclk = 100000;
 	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
+	    intel_panel_use_ssc(dev_priv)) {
 		refclk = dev_priv->vbt.lvds_ssc_freq;
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
 	} else if (!IS_GEN2(dev)) {
@@ -7566,8 +7564,7 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
 
 static void i9xx_compute_dpll(struct intel_crtc *crtc,
 			      struct intel_crtc_state *crtc_state,
-			      intel_clock_t *reduced_clock,
-			      int num_connectors)
+			      intel_clock_t *reduced_clock)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7626,7 +7623,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 	if (crtc_state->sdvo_tv_clock)
 		dpll |= PLL_REF_INPUT_TVCLKINBC;
 	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
+		 intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
@@ -7643,8 +7640,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
 
 static void i8xx_compute_dpll(struct intel_crtc *crtc,
 			      struct intel_crtc_state *crtc_state,
-			      intel_clock_t *reduced_clock,
-			      int num_connectors)
+			      intel_clock_t *reduced_clock)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7670,7 +7666,7 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
 		dpll |= DPLL_DVO_2X_MODE;
 
 	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
+	    intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
@@ -7898,14 +7894,10 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk, num_connectors = 0;
+	int refclk;
 	intel_clock_t clock;
 	bool ok;
 	const intel_limit_t *limit;
-	struct drm_atomic_state *state = crtc_state->base.state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	int i;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
@@ -7913,13 +7905,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	if (crtc_state->has_dsi_encoder)
 		return 0;
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc == &crtc->base)
-			num_connectors++;
-	}
-
 	if (!crtc_state->clock_set) {
-		refclk = i9xx_get_refclk(crtc_state, num_connectors);
+		refclk = i9xx_get_refclk(crtc_state);
 
 		/*
 		 * Returns a set of divisors for the desired target clock with
@@ -7945,15 +7932,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	}
 
 	if (IS_GEN2(dev)) {
-		i8xx_compute_dpll(crtc, crtc_state, NULL,
-				  num_connectors);
+		i8xx_compute_dpll(crtc, crtc_state, NULL);
 	} else if (IS_CHERRYVIEW(dev)) {
 		chv_compute_dpll(crtc, crtc_state);
 	} else if (IS_VALLEYVIEW(dev)) {
 		vlv_compute_dpll(crtc, crtc_state);
 	} else {
-		i9xx_compute_dpll(crtc, crtc_state, NULL,
-				  num_connectors);
+		i9xx_compute_dpll(crtc, crtc_state, NULL);
 	}
 
 	return 0;
@@ -8639,30 +8624,9 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_atomic_state *state = crtc_state->base.state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	struct intel_encoder *encoder;
-	int num_connectors = 0, i;
-	bool is_lvds = false;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != crtc_state->base.crtc)
-			continue;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-
-		switch (encoder->type) {
-		case INTEL_OUTPUT_LVDS:
-			is_lvds = true;
-			break;
-		default:
-			break;
-		}
-		num_connectors++;
-	}
 
-	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    intel_panel_use_ssc(dev_priv)) {
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
 			      dev_priv->vbt.lvds_ssc_freq);
 		return dev_priv->vbt.lvds_ssc_freq;
@@ -8896,7 +8860,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	uint32_t dpll;
-	int factor, num_connectors = 0, i;
+	int factor, i;
 	bool is_lvds = false, is_sdvo = false;
 
 	for_each_connector_in_state(state, connector, connector_state, i) {
@@ -8916,8 +8880,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		default:
 			break;
 		}
-
-		num_connectors++;
 	}
 
 	/* Enable autotuning of the PLL clock (if permissible) */
@@ -8971,7 +8933,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 		break;
 	}
 
-	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
+	if (is_lvds && intel_panel_use_ssc(dev_priv))
 		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 02/15] drm/i915: Merge ironlake_get_refclk() into its only caller
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 03/15] drm/i915: Fold intel_ironlake_limit() into clock computation function Ander Conselvan de Oliveira
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

A previous patch made ironlake_get_refclk() very simple, so merge
it into its only caller.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 710fd9a..47cd1f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8620,21 +8620,6 @@ void intel_init_pch_refclk(struct drm_device *dev)
 		lpt_init_pch_refclk(dev);
 }
 
-static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv)) {
-		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
-			      dev_priv->vbt.lvds_ssc_freq);
-		return dev_priv->vbt.lvds_ssc_freq;
-	}
-
-	return 120000;
-}
-
 static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
@@ -8814,7 +8799,14 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	const intel_limit_t *limit;
 	bool ret;
 
-	refclk = ironlake_get_refclk(crtc_state);
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    intel_panel_use_ssc(dev_priv)) {
+		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
+			      dev_priv->vbt.lvds_ssc_freq);
+		refclk = dev_priv->vbt.lvds_ssc_freq;
+	} else {
+		refclk = 120000;
+	}
 
 	/*
 	 * Returns a set of divisors for the desired target clock with the given
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 03/15] drm/i915: Fold intel_ironlake_limit() into clock computation function
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 02/15] drm/i915: Merge ironlake_get_refclk() into its only caller Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 04/15] drm/i915: Call g4x_find_best_dpll() directly from ILK+ code Ander Conselvan de Oliveira
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The funcion intel_ironlake_limit() is only called by the crtc compute
clock path. By merging it into ironlake_compute_clocks(), the code gets
clearer, since there's no more if-ladders to follow.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 56 +++++++++++++++---------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 47cd1f0..459b188 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -568,30 +568,6 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
 }
 
 static const intel_limit_t *
-intel_ironlake_limit(struct intel_crtc_state *crtc_state, int refclk)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	const intel_limit_t *limit;
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
-		if (intel_is_dual_link_lvds(dev)) {
-			if (refclk == 100000)
-				limit = &intel_limits_ironlake_dual_lvds_100m;
-			else
-				limit = &intel_limits_ironlake_dual_lvds;
-		} else {
-			if (refclk == 100000)
-				limit = &intel_limits_ironlake_single_lvds_100m;
-			else
-				limit = &intel_limits_ironlake_single_lvds;
-		}
-	} else
-		limit = &intel_limits_ironlake_dac;
-
-	return limit;
-}
-
-static const intel_limit_t *
 intel_g4x_limit(struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
@@ -621,8 +597,8 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 
 	if (IS_BROXTON(dev))
 		limit = &intel_limits_bxt;
-	else if (HAS_PCH_SPLIT(dev))
-		limit = intel_ironlake_limit(crtc_state, refclk);
+	else if (WARN_ON(HAS_PCH_SPLIT(dev)))
+		limit = NULL;
 	else if (IS_G4X(dev)) {
 		limit = intel_g4x_limit(crtc_state);
 	} else if (IS_PINEVIEW(dev)) {
@@ -8799,13 +8775,28 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	const intel_limit_t *limit;
 	bool ret;
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv)) {
-		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
-			      dev_priv->vbt.lvds_ssc_freq);
-		refclk = dev_priv->vbt.lvds_ssc_freq;
+	refclk = 120000;
+
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
+				      dev_priv->vbt.lvds_ssc_freq);
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+		}
+
+		if (intel_is_dual_link_lvds(dev)) {
+			if (refclk == 100000)
+				limit = &intel_limits_ironlake_dual_lvds_100m;
+			else
+				limit = &intel_limits_ironlake_dual_lvds;
+		} else {
+			if (refclk == 100000)
+				limit = &intel_limits_ironlake_single_lvds_100m;
+			else
+				limit = &intel_limits_ironlake_single_lvds;
+		}
 	} else {
-		refclk = 120000;
+		limit = &intel_limits_ironlake_dac;
 	}
 
 	/*
@@ -8813,7 +8804,6 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 * refclk, or FALSE.  The returned values represent the clock equation:
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
-	limit = intel_limit(crtc_state, refclk);
 	ret = dev_priv->display.find_dpll(limit, crtc_state,
 					  crtc_state->port_clock,
 					  refclk, NULL, clock);
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 04/15] drm/i915: Call g4x_find_best_dpll() directly from ILK+ code
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (2 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 03/15] drm/i915: Fold intel_ironlake_limit() into clock computation function Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 05/15] drm/i915: Simplify ironlake reduced clock logic a bit Ander Conselvan de Oliveira
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The call to dev_priv->display.find_dpll() is already in platform
specific code, so avoid the extra detour.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 459b188..003f1d4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8804,9 +8804,8 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc,
 	 * refclk, or FALSE.  The returned values represent the clock equation:
 	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
 	 */
-	ret = dev_priv->display.find_dpll(limit, crtc_state,
-					  crtc_state->port_clock,
-					  refclk, NULL, clock);
+	ret = g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				 refclk, NULL, clock);
 	if (!ret)
 		return false;
 
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 05/15] drm/i915: Simplify ironlake reduced clock logic a bit
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (3 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 04/15] drm/i915: Call g4x_find_best_dpll() directly from ILK+ code Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 06/15] drm/i915: Don't calculate a new clock in ILK+ code if it is already set Ander Conselvan de Oliveira
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Check has_reduced_clock only once when setting dpll_hw_state, making the
code slightly more readable.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 003f1d4..a6faa96 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8960,6 +8960,8 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 		fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
 		if (has_reduced_clock)
 			fp2 = i9xx_dpll_compute_fp(&reduced_clock);
+		else
+			fp2 = fp;
 
 		dpll = ironlake_compute_dpll(crtc, crtc_state,
 					     &fp, &reduced_clock,
@@ -8967,10 +8969,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 
 		crtc_state->dpll_hw_state.dpll = dpll;
 		crtc_state->dpll_hw_state.fp0 = fp;
-		if (has_reduced_clock)
-			crtc_state->dpll_hw_state.fp1 = fp2;
-		else
-			crtc_state->dpll_hw_state.fp1 = fp;
+		crtc_state->dpll_hw_state.fp1 = fp2;
 
 		pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
 		if (pll == NULL) {
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 06/15] drm/i915: Don't calculate a new clock in ILK+ code if it is already set
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (4 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 05/15] drm/i915: Simplify ironlake reduced clock logic a bit Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 07/15] drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Remove the clock calculation from ironlake_crtc_compute_clock() when the
encoder compute_config() already set one. The value was just thrown away
in that case.

Note that the previously set clock is not validated against the limits
anymore. That is ok since the fixed clocks from DP and SDVO are within
the supported range, so the call to ironlake_compute_clocks() would
never fail in that case.

v2: Add note about not checking fixed clocks agains limits. (Maarten)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a6faa96..f4bef9e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8928,7 +8928,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock, reduced_clock;
 	u32 dpll = 0, fp = 0, fp2 = 0;
-	bool ok, has_reduced_clock = false;
+	bool has_reduced_clock = false;
 	bool is_lvds = false;
 	struct intel_shared_dpll *pll;
 
@@ -8940,14 +8940,15 @@ static int ironlake_crtc_compute_clock(struct intel_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->base, crtc_state, &clock,
-				     &has_reduced_clock, &reduced_clock);
-	if (!ok && !crtc_state->clock_set) {
-		DRM_ERROR("Couldn't find PLL settings for mode!\n");
-		return -EINVAL;
-	}
-	/* Compat-code for transition, will disappear. */
 	if (!crtc_state->clock_set) {
+		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
+					     &has_reduced_clock,
+					     &reduced_clock)) {
+			DRM_ERROR("Couldn't find PLL settings for mode!\n");
+			return -EINVAL;
+		}
+
+		/* Compat-code for transition, will disappear. */
 		crtc_state->dpll.n = clock.n;
 		crtc_state->dpll.m1 = clock.m1;
 		crtc_state->dpll.m2 = clock.m2;
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 07/15] drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (5 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 06/15] drm/i915: Don't calculate a new clock in ILK+ code if it is already set Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 08/15] drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case Ander Conselvan de Oliveira
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The checks were added in commit 5dc5298bb3e5 ("drm/i915: add proper
CPU/PCH checks to crtc_mode_set functions") in a time when there was
doubts on what PCHs would be supported by HSW. There are similar checks
for PCH type in intel_detect_pch() and the function pointers are
initialized based on platform/pch information, so the removed WARN can't
ever be reached.

v2: Rebase without patch that drops lvds downclock code. (Ville)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f4bef9e..c6fc22f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8925,7 +8925,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 				       struct intel_crtc_state *crtc_state)
 {
-	struct drm_device *dev = crtc->base.dev;
 	intel_clock_t clock, reduced_clock;
 	u32 dpll = 0, fp = 0, fp2 = 0;
 	bool has_reduced_clock = false;
@@ -8937,9 +8936,6 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 
 	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
 
-	WARN(!(HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)),
-	     "Unexpected PCH type %d\n", INTEL_PCH_TYPE(dev));
-
 	if (!crtc_state->clock_set) {
 		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
 					     &has_reduced_clock,
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 08/15] drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (6 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 07/15] drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-21 16:00 ` [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll() Ander Conselvan de Oliveira
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

None of the code in ironlake_crtc_compute_clock() is relevant for CPU
eDP. The CPU eDP PLL is turned on and off in ironlake_edp_pll_{on,off}
from the DP code and that doesn't depend on the crtc_state->dpll values,
so just return early in that case.

v2: Rebase without patch that drops lvds downclock code. (Ville)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 47 ++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c6fc22f..0af1e7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8928,13 +8928,16 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 	intel_clock_t clock, reduced_clock;
 	u32 dpll = 0, fp = 0, fp2 = 0;
 	bool has_reduced_clock = false;
-	bool is_lvds = false;
 	struct intel_shared_dpll *pll;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	is_lvds = intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS);
+	crtc->lowfreq_avail = false;
+
+	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
+	if (!crtc_state->has_pch_encoder)
+		return 0;
 
 	if (!crtc_state->clock_set) {
 		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
@@ -8952,34 +8955,30 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 		crtc_state->dpll.p2 = clock.p2;
 	}
 
-	/* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
-	if (crtc_state->has_pch_encoder) {
-		fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
-		if (has_reduced_clock)
-			fp2 = i9xx_dpll_compute_fp(&reduced_clock);
-		else
-			fp2 = fp;
+	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
+	if (has_reduced_clock)
+		fp2 = i9xx_dpll_compute_fp(&reduced_clock);
+	else
+		fp2 = fp;
 
-		dpll = ironlake_compute_dpll(crtc, crtc_state,
-					     &fp, &reduced_clock,
-					     has_reduced_clock ? &fp2 : NULL);
+	dpll = ironlake_compute_dpll(crtc, crtc_state,
+				     &fp, &reduced_clock,
+				     has_reduced_clock ? &fp2 : NULL);
 
-		crtc_state->dpll_hw_state.dpll = dpll;
-		crtc_state->dpll_hw_state.fp0 = fp;
-		crtc_state->dpll_hw_state.fp1 = fp2;
+	crtc_state->dpll_hw_state.dpll = dpll;
+	crtc_state->dpll_hw_state.fp0 = fp;
+	crtc_state->dpll_hw_state.fp1 = fp2;
 
-		pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
-		if (pll == NULL) {
-			DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
-					 pipe_name(crtc->pipe));
-			return -EINVAL;
-		}
+	pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
+	if (pll == NULL) {
+		DRM_DEBUG_DRIVER("failed to find PLL for pipe %c\n",
+				 pipe_name(crtc->pipe));
+		return -EINVAL;
 	}
 
-	if (is_lvds && has_reduced_clock)
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    has_reduced_clock)
 		crtc->lowfreq_avail = true;
-	else
-		crtc->lowfreq_avail = false;
 
 	return 0;
 }
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (7 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 08/15] drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 10:18   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll() Ander Conselvan de Oliveira
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

When calculating clocks, just pass a pointer to crtc_state->dpll
directly to the find_dpll() hook. Back when this was introduced in
commit f47709a9502f3 ("drm/i915: create pipe_config->dpll for clock
state") there was no staged crtc config or atomic crtc state, so it was
possible to overwrite the current configuration on error. That hasn't
been the case for a while now, so finally make it "disappear".

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0af1e7d..0b6eabf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7871,7 +7871,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int refclk;
-	intel_clock_t clock;
 	bool ok;
 	const intel_limit_t *limit;
 
@@ -7893,18 +7892,12 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		limit = intel_limit(crtc_state, refclk);
 		ok = dev_priv->display.find_dpll(limit, crtc_state,
 						 crtc_state->port_clock,
-						 refclk, NULL, &clock);
+						 refclk, NULL,
+						 &crtc_state->dpll);
 		if (!ok) {
 			DRM_ERROR("Couldn't find PLL settings for mode!\n");
 			return -EINVAL;
 		}
-
-		/* Compat-code for transition, will disappear. */
-		crtc_state->dpll.n = clock.n;
-		crtc_state->dpll.m1 = clock.m1;
-		crtc_state->dpll.m2 = clock.m2;
-		crtc_state->dpll.p1 = clock.p1;
-		crtc_state->dpll.p2 = clock.p2;
 	}
 
 	if (IS_GEN2(dev)) {
@@ -8925,7 +8918,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 				       struct intel_crtc_state *crtc_state)
 {
-	intel_clock_t clock, reduced_clock;
+	intel_clock_t reduced_clock;
 	u32 dpll = 0, fp = 0, fp2 = 0;
 	bool has_reduced_clock = false;
 	struct intel_shared_dpll *pll;
@@ -8939,20 +8932,13 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 	if (!crtc_state->has_pch_encoder)
 		return 0;
 
-	if (!crtc_state->clock_set) {
-		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
-					     &has_reduced_clock,
-					     &reduced_clock)) {
-			DRM_ERROR("Couldn't find PLL settings for mode!\n");
-			return -EINVAL;
-		}
-
-		/* Compat-code for transition, will disappear. */
-		crtc_state->dpll.n = clock.n;
-		crtc_state->dpll.m1 = clock.m1;
-		crtc_state->dpll.m2 = clock.m2;
-		crtc_state->dpll.p1 = clock.p1;
-		crtc_state->dpll.p2 = clock.p2;
+	if (!crtc_state->clock_set &&
+	    !ironlake_compute_clocks(&crtc->base, crtc_state,
+				     &crtc_state->dpll,
+				     &has_reduced_clock,
+				     &reduced_clock)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
 	}
 
 	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (8 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 12:49   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Follow what is done in i8xx_compute_dpll() and i9xx_compute_dpll() and
move the lower level details of setting crtc_state->dpll_hw_state into
the _compute_dpll() function.
---
 drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b6eabf..b6541a0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8821,10 +8821,9 @@ static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
 	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
 }
 
-static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
-				      struct intel_crtc_state *crtc_state,
-				      u32 *fp,
-				      intel_clock_t *reduced_clock, u32 *fp2)
+static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
+				  struct intel_crtc_state *crtc_state,
+				  intel_clock_t *reduced_clock)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
 	struct drm_device *dev = crtc->dev;
@@ -8833,7 +8832,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
-	uint32_t dpll;
+	u32 dpll, fp, fp2;
 	int factor, i;
 	bool is_lvds = false, is_sdvo = false;
 
@@ -8866,11 +8865,19 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	} else if (crtc_state->sdvo_tv_clock)
 		factor = 20;
 
+	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
+
 	if (ironlake_needs_fb_cb_tune(&crtc_state->dpll, factor))
-		*fp |= FP_CB_TUNE;
+		fp |= FP_CB_TUNE;
+
+	if (reduced_clock) {
+		fp2 = i9xx_dpll_compute_fp(reduced_clock);
 
-	if (fp2 && (reduced_clock->m < factor * reduced_clock->n))
-		*fp2 |= FP_CB_TUNE;
+		if (reduced_clock->m < factor * reduced_clock->n)
+			fp2 |= FP_CB_TUNE;
+	} else {
+		fp2 = fp;
+	}
 
 	dpll = 0;
 
@@ -8912,14 +8919,17 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	else
 		dpll |= PLL_REF_INPUT_DREFCLK;
 
-	return dpll | DPLL_VCO_ENABLE;
+	dpll |= DPLL_VCO_ENABLE;
+
+	crtc_state->dpll_hw_state.dpll = dpll;
+	crtc_state->dpll_hw_state.fp0 = fp;
+	crtc_state->dpll_hw_state.fp1 = fp2;
 }
 
 static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 				       struct intel_crtc_state *crtc_state)
 {
 	intel_clock_t reduced_clock;
-	u32 dpll = 0, fp = 0, fp2 = 0;
 	bool has_reduced_clock = false;
 	struct intel_shared_dpll *pll;
 
@@ -8941,19 +8951,8 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
-	if (has_reduced_clock)
-		fp2 = i9xx_dpll_compute_fp(&reduced_clock);
-	else
-		fp2 = fp;
-
-	dpll = ironlake_compute_dpll(crtc, crtc_state,
-				     &fp, &reduced_clock,
-				     has_reduced_clock ? &fp2 : NULL);
-
-	crtc_state->dpll_hw_state.dpll = dpll;
-	crtc_state->dpll_hw_state.fp0 = fp;
-	crtc_state->dpll_hw_state.fp1 = fp2;
+	ironlake_compute_dpll(crtc, crtc_state,
+			      has_reduced_clock ? &reduced_clock : NULL);
 
 	pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
 	if (pll == NULL) {
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (9 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 12:51   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks Ander Conselvan de Oliveira
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Merge ironlake_compute_clocks() into ironlake_crtc_compute_clock() so
the clock computation logic is all in one place. The resulting function
is still quite simple. Follow up patches will make the similar code for
GMCH platforms look similar.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 86 ++++++++++++++----------------------
 1 file changed, 33 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b6541a0..13b509e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -848,6 +848,11 @@ pnv_find_best_dpll(const intel_limit_t *limit,
 	return (err != target);
 }
 
+/*
+ * Returns a set of divisors for the desired target clock with the given
+ * refclk, or FALSE.  The returned values represent the clock equation:
+ * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ */
 static bool
 g4x_find_best_dpll(const intel_limit_t *limit,
 		   struct intel_crtc_state *crtc_state,
@@ -8756,55 +8761,6 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
 	}
 }
 
-static bool ironlake_compute_clocks(struct drm_crtc *crtc,
-				    struct intel_crtc_state *crtc_state,
-				    intel_clock_t *clock,
-				    bool *has_reduced_clock,
-				    intel_clock_t *reduced_clock)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk;
-	const intel_limit_t *limit;
-	bool ret;
-
-	refclk = 120000;
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
-		if (intel_panel_use_ssc(dev_priv)) {
-			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
-				      dev_priv->vbt.lvds_ssc_freq);
-			refclk = dev_priv->vbt.lvds_ssc_freq;
-		}
-
-		if (intel_is_dual_link_lvds(dev)) {
-			if (refclk == 100000)
-				limit = &intel_limits_ironlake_dual_lvds_100m;
-			else
-				limit = &intel_limits_ironlake_dual_lvds;
-		} else {
-			if (refclk == 100000)
-				limit = &intel_limits_ironlake_single_lvds_100m;
-			else
-				limit = &intel_limits_ironlake_single_lvds;
-		}
-	} else {
-		limit = &intel_limits_ironlake_dac;
-	}
-
-	/*
-	 * Returns a set of divisors for the desired target clock with the given
-	 * refclk, or FALSE.  The returned values represent the clock equation:
-	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
-	 */
-	ret = g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
-				 refclk, NULL, clock);
-	if (!ret)
-		return false;
-
-	return true;
-}
-
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
 {
 	/*
@@ -8929,9 +8885,13 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 				       struct intel_crtc_state *crtc_state)
 {
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	intel_clock_t reduced_clock;
 	bool has_reduced_clock = false;
 	struct intel_shared_dpll *pll;
+	const intel_limit_t *limit;
+	int refclk = 120000;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
@@ -8942,11 +8902,31 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
 	if (!crtc_state->has_pch_encoder)
 		return 0;
 
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
+				      dev_priv->vbt.lvds_ssc_freq);
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+		}
+
+		if (intel_is_dual_link_lvds(dev)) {
+			if (refclk == 100000)
+				limit = &intel_limits_ironlake_dual_lvds_100m;
+			else
+				limit = &intel_limits_ironlake_dual_lvds;
+		} else {
+			if (refclk == 100000)
+				limit = &intel_limits_ironlake_single_lvds_100m;
+			else
+				limit = &intel_limits_ironlake_single_lvds;
+		}
+	} else {
+		limit = &intel_limits_ironlake_dac;
+	}
+
 	if (!crtc_state->clock_set &&
-	    !ironlake_compute_clocks(&crtc->base, crtc_state,
-				     &crtc_state->dpll,
-				     &has_reduced_clock,
-				     &reduced_clock)) {
+	    !g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				refclk, NULL, &crtc_state->dpll)) {
 		DRM_ERROR("Couldn't find PLL settings for mode!\n");
 		return -EINVAL;
 	}
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (10 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 10:26   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock() Ander Conselvan de Oliveira
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

In order for VLV and CHV to use i9xx_crtc_compute_clocks(), a number
of if ladders is necessary: one for setting the find_dpll() hook, one
for choosing the limits struct, one for choosing the right compute dpll
function and one for initializing the crtc_compute_clock() hook.

By extracting a platform specific implementation for each platform, the
number of if-ladders is reduced to one.

While at it also clean up bxt_find_best_dpll() which depends on some of
the CHV code.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 102 ++++++++++++++++++++++++++---------
 1 file changed, 78 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 13b509e..dca5b15 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -113,7 +113,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
-static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(struct intel_crtc *crtc);
 static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
 static void ironlake_pfit_enable(struct intel_crtc *crtc);
@@ -595,21 +594,17 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
-	if (IS_BROXTON(dev))
-		limit = &intel_limits_bxt;
-	else if (WARN_ON(HAS_PCH_SPLIT(dev)))
+	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
+	    HAS_PCH_SPLIT(dev))
 		limit = NULL;
-	else if (IS_G4X(dev)) {
+
+	if (IS_G4X(dev)) {
 		limit = intel_g4x_limit(crtc_state);
 	} else if (IS_PINEVIEW(dev)) {
 		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_pineview_lvds;
 		else
 			limit = &intel_limits_pineview_sdvo;
-	} else if (IS_CHERRYVIEW(dev)) {
-		limit = &intel_limits_chv;
-	} else if (IS_VALLEYVIEW(dev)) {
-		limit = &intel_limits_vlv;
 	} else if (!IS_GEN2(dev)) {
 		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
@@ -623,6 +618,9 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 		else
 			limit = &intel_limits_i8xx_dac;
 	}
+
+	WARN_ON(limit == NULL);
+
 	return limit;
 }
 
@@ -941,6 +939,11 @@ static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
 	return *error_ppm + 10 < best_error_ppm;
 }
 
+/*
+ * Returns a set of divisors for the desired target clock with the given
+ * refclk, or FALSE.  The returned values represent the clock equation:
+ * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ */
 static bool
 vlv_find_best_dpll(const intel_limit_t *limit,
 		   struct intel_crtc_state *crtc_state,
@@ -995,6 +998,11 @@ vlv_find_best_dpll(const intel_limit_t *limit,
 	return found;
 }
 
+/*
+ * Returns a set of divisors for the desired target clock with the given
+ * refclk, or FALSE.  The returned values represent the clock equation:
+ * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ */
 static bool
 chv_find_best_dpll(const intel_limit_t *limit,
 		   struct intel_crtc_state *crtc_state,
@@ -1056,9 +1064,10 @@ chv_find_best_dpll(const intel_limit_t *limit,
 bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
 			intel_clock_t *best_clock)
 {
-	int refclk = i9xx_get_refclk(crtc_state);
+	int refclk = 100000;
+	const intel_limit_t *limit = &intel_limits_bxt;
 
-	return chv_find_best_dpll(intel_limit(crtc_state, refclk), crtc_state,
+	return chv_find_best_dpll(limit, crtc_state,
 				  target_clock, refclk, NULL, best_clock);
 }
 
@@ -7101,9 +7110,7 @@ static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
 
 	WARN_ON(!crtc_state->base.state);
 
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) {
-		refclk = 100000;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
 	    intel_panel_use_ssc(dev_priv)) {
 		refclk = dev_priv->vbt.lvds_ssc_freq;
 		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
@@ -7907,10 +7914,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 
 	if (IS_GEN2(dev)) {
 		i8xx_compute_dpll(crtc, crtc_state, NULL);
-	} else if (IS_CHERRYVIEW(dev)) {
-		chv_compute_dpll(crtc, crtc_state);
-	} else if (IS_VALLEYVIEW(dev)) {
-		vlv_compute_dpll(crtc, crtc_state);
 	} else {
 		i9xx_compute_dpll(crtc, crtc_state, NULL);
 	}
@@ -7918,6 +7921,54 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	return 0;
 }
 
+static int chv_crtc_compute_clock(struct intel_crtc *crtc,
+				  struct intel_crtc_state *crtc_state)
+{
+	int refclk = 100000;
+	const intel_limit_t *limit = &intel_limits_chv;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (crtc_state->has_dsi_encoder)
+		return 0;
+
+	if (!crtc_state->clock_set &&
+	    !chv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	chv_compute_dpll(crtc, crtc_state);
+
+	return 0;
+}
+
+static int vlv_crtc_compute_clock(struct intel_crtc *crtc,
+				  struct intel_crtc_state *crtc_state)
+{
+	int refclk = 100000;
+	const intel_limit_t *limit = &intel_limits_vlv;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (crtc_state->has_dsi_encoder)
+		return 0;
+
+	if (!crtc_state->clock_set &&
+	    !vlv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	vlv_compute_dpll(crtc, crtc_state);
+
+	return 0;
+}
+
 static void i9xx_get_pfit_config(struct intel_crtc *crtc,
 				 struct intel_crtc_state *pipe_config)
 {
@@ -14849,10 +14900,6 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 {
 	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
 		dev_priv->display.find_dpll = g4x_find_best_dpll;
-	else if (IS_CHERRYVIEW(dev_priv))
-		dev_priv->display.find_dpll = chv_find_best_dpll;
-	else if (IS_VALLEYVIEW(dev_priv))
-		dev_priv->display.find_dpll = vlv_find_best_dpll;
 	else if (IS_PINEVIEW(dev_priv))
 		dev_priv->display.find_dpll = pnv_find_best_dpll;
 	else
@@ -14882,11 +14929,18 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 			ironlake_crtc_compute_clock;
 		dev_priv->display.crtc_enable = ironlake_crtc_enable;
 		dev_priv->display.crtc_disable = ironlake_crtc_disable;
-	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
+	} else if (IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			i9xx_get_initial_plane_config;
-		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
+		dev_priv->display.crtc_compute_clock = chv_crtc_compute_clock;
+		dev_priv->display.crtc_enable = valleyview_crtc_enable;
+		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+	} else if (IS_VALLEYVIEW(dev_priv)) {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_initial_plane_config =
+			i9xx_get_initial_plane_config;
+		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	} else {
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (11 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 10:24   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock() Ander Conselvan de Oliveira
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Split a GEN2 specific version from i9xx_crtc_compute_clock(). With this
there is no need for i9xx_get_refclk() anymore, and the differences
between platforms become more obvious.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 91 +++++++++++++++++++++---------------
 1 file changed, 53 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dca5b15..245d6c6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -595,7 +595,7 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 	const intel_limit_t *limit;
 
 	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
-	    HAS_PCH_SPLIT(dev))
+	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
 		limit = NULL;
 
 	if (IS_G4X(dev)) {
@@ -610,13 +610,6 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 			limit = &intel_limits_i9xx_lvds;
 		else
 			limit = &intel_limits_i9xx_sdvo;
-	} else {
-		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
-			limit = &intel_limits_i8xx_lvds;
-		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
-			limit = &intel_limits_i8xx_dvo;
-		else
-			limit = &intel_limits_i8xx_dac;
 	}
 
 	WARN_ON(limit == NULL);
@@ -7102,27 +7095,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk;
-
-	WARN_ON(!crtc_state->base.state);
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv)) {
-		refclk = dev_priv->vbt.lvds_ssc_freq;
-		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
-	} else if (!IS_GEN2(dev)) {
-		refclk = 96000;
-	} else {
-		refclk = 48000;
-	}
-
-	return refclk;
-}
-
 static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
 {
 	return (1 << dpll->n) << 16 | dpll->m2;
@@ -7877,14 +7849,50 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 	POSTING_READ(PIPECONF(intel_crtc->pipe));
 }
 
+static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
+				   struct intel_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	const intel_limit_t *limit;
+	int refclk = 48000;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		}
+
+		limit = &intel_limits_i8xx_lvds;
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO)) {
+		limit = &intel_limits_i8xx_dvo;
+	} else {
+		limit = &intel_limits_i8xx_dac;
+	}
+
+	if (!crtc_state->clock_set &&
+	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				 refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	i8xx_compute_dpll(crtc, crtc_state, NULL);
+
+	return 0;
+}
+
 static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 				   struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk;
 	bool ok;
 	const intel_limit_t *limit;
+	int refclk = 96000;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
@@ -7892,9 +7900,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	if (crtc_state->has_dsi_encoder)
 		return 0;
 
-	if (!crtc_state->clock_set) {
-		refclk = i9xx_get_refclk(crtc_state);
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    intel_panel_use_ssc(dev_priv)) {
+		refclk = dev_priv->vbt.lvds_ssc_freq;
+		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+	}
 
+	if (!crtc_state->clock_set) {
 		/*
 		 * Returns a set of divisors for the desired target clock with
 		 * the given refclk, or FALSE.  The returned values represent
@@ -7912,11 +7924,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		}
 	}
 
-	if (IS_GEN2(dev)) {
-		i8xx_compute_dpll(crtc, crtc_state, NULL);
-	} else {
-		i9xx_compute_dpll(crtc, crtc_state, NULL);
-	}
+	i9xx_compute_dpll(crtc, crtc_state, NULL);
 
 	return 0;
 }
@@ -14943,13 +14951,20 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
-	} else {
+	} else if (!IS_GEN2(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			i9xx_get_initial_plane_config;
 		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+	} else {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_initial_plane_config =
+			i9xx_get_initial_plane_config;
+		dev_priv->display.crtc_compute_clock = gen2_crtc_compute_clock;
+		dev_priv->display.crtc_enable = i9xx_crtc_enable;
+		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	}
 
 	/* Returns the core display clock speed */
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (12 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 12:52   ` Ville Syrjälä
  2016-03-21 16:00 ` [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock() Ander Conselvan de Oliveira
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Split a G4X specific version from i9xx_crtc_compute_clock(). With this
the differences between platforms become more obvious.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 82 +++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 245d6c6..552fd4c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -567,40 +567,16 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
 }
 
 static const intel_limit_t *
-intel_g4x_limit(struct intel_crtc_state *crtc_state)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	const intel_limit_t *limit;
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
-		if (intel_is_dual_link_lvds(dev))
-			limit = &intel_limits_g4x_dual_channel_lvds;
-		else
-			limit = &intel_limits_g4x_single_channel_lvds;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
-		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
-		limit = &intel_limits_g4x_hdmi;
-	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
-		limit = &intel_limits_g4x_sdvo;
-	} else /* The option is for other outputs */
-		limit = &intel_limits_i9xx_sdvo;
-
-	return limit;
-}
-
-static const intel_limit_t *
 intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 {
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	const intel_limit_t *limit;
 
 	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
-	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
+	    HAS_PCH_SPLIT(dev) || IS_G4X(dev) || IS_GEN2(dev))
 		limit = NULL;
 
-	if (IS_G4X(dev)) {
-		limit = intel_g4x_limit(crtc_state);
-	} else if (IS_PINEVIEW(dev)) {
+	if (IS_PINEVIEW(dev)) {
 		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_pineview_lvds;
 		else
@@ -7885,6 +7861,49 @@ static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
 	return 0;
 }
 
+static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
+				  struct intel_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	const intel_limit_t *limit;
+	int refclk = 96000;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		}
+
+		if (intel_is_dual_link_lvds(dev))
+			limit = &intel_limits_g4x_dual_channel_lvds;
+		else
+			limit = &intel_limits_g4x_single_channel_lvds;
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
+		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
+		limit = &intel_limits_g4x_hdmi;
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
+		limit = &intel_limits_g4x_sdvo;
+	} else {
+		/* The option is for other outputs */
+		limit = &intel_limits_i9xx_sdvo;
+	}
+
+	if (!crtc_state->clock_set &&
+	    !g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	i9xx_compute_dpll(crtc, crtc_state, NULL);
+
+	return 0;
+}
+
 static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 				   struct intel_crtc_state *crtc_state)
 {
@@ -14906,9 +14925,7 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
  */
 void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 {
-	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
-		dev_priv->display.find_dpll = g4x_find_best_dpll;
-	else if (IS_PINEVIEW(dev_priv))
+	if (IS_PINEVIEW(dev_priv))
 		dev_priv->display.find_dpll = pnv_find_best_dpll;
 	else
 		dev_priv->display.find_dpll = i9xx_find_best_dpll;
@@ -14951,6 +14968,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+	} else if (IS_G4X(dev_priv)) {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_initial_plane_config =
+			i9xx_get_initial_plane_config;
+		dev_priv->display.crtc_compute_clock = g4x_crtc_compute_clock;
+		dev_priv->display.crtc_enable = i9xx_crtc_enable;
+		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	} else if (!IS_GEN2(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock()
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (13 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-21 16:00 ` Ander Conselvan de Oliveira
  2016-03-22 12:55   ` Ville Syrjälä
  2016-03-22  9:33 ` ✗ Fi.CI.BAT: warning for Clean up ironlake clock computation code (rev3) Patchwork
  2016-03-22 14:32 ` ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4) Patchwork
  16 siblings, 1 reply; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-21 16:00 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Split a pnv_crtc_compute_clock(), so the differences between platforms
become more obvious.

With this, there are no more users of intel_limit() or the ->find_dpll()
hook, so get rid of them.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  18 -----
 drivers/gpu/drm/i915/intel_display.c | 134 +++++++++++++++++++++--------------
 2 files changed, 79 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index efca534..83ee8d7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -575,24 +575,6 @@ struct dpll;
 struct drm_i915_display_funcs {
 	int (*get_display_clock_speed)(struct drm_device *dev);
 	int (*get_fifo_size)(struct drm_device *dev, int plane);
-	/**
-	 * find_dpll() - 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_dpll)(const struct intel_limit *limit,
-			  struct intel_crtc_state *crtc_state,
-			  int target, int refclk,
-			  struct dpll *match_clock,
-			  struct dpll *best_clock);
 	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
 	int (*compute_intermediate_wm)(struct drm_device *dev,
 				       struct intel_crtc *intel_crtc,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 552fd4c..4d7c29a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -566,33 +566,6 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
 	return false;
 }
 
-static const intel_limit_t *
-intel_limit(struct intel_crtc_state *crtc_state, int refclk)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	const intel_limit_t *limit;
-
-	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
-	    HAS_PCH_SPLIT(dev) || IS_G4X(dev) || IS_GEN2(dev))
-		limit = NULL;
-
-	if (IS_PINEVIEW(dev)) {
-		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
-			limit = &intel_limits_pineview_lvds;
-		else
-			limit = &intel_limits_pineview_sdvo;
-	} else if (!IS_GEN2(dev)) {
-		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
-			limit = &intel_limits_i9xx_lvds;
-		else
-			limit = &intel_limits_i9xx_sdvo;
-	}
-
-	WARN_ON(limit == NULL);
-
-	return limit;
-}
-
 /*
  * Platform specific helpers to calculate the port PLL loopback- (clock.m),
  * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
@@ -723,6 +696,16 @@ i9xx_select_p2_div(const intel_limit_t *limit,
 	}
 }
 
+/*
+ * Returns a set of divisors for the desired target clock with the given
+ * refclk, or FALSE.  The returned values represent the clock equation:
+ * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ *
+ * Target and reference clocks are specified in kHz.
+ *
+ * If match_clock is provided, then best_clock P divider must match the P
+ * divider from @match_clock used for LVDS downclocking.
+ */
 static bool
 i9xx_find_best_dpll(const intel_limit_t *limit,
 		    struct intel_crtc_state *crtc_state,
@@ -770,6 +753,16 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
 	return (err != target);
 }
 
+/*
+ * Returns a set of divisors for the desired target clock with the given
+ * refclk, or FALSE.  The returned values represent the clock equation:
+ * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ *
+ * Target and reference clocks are specified in kHz.
+ *
+ * If match_clock is provided, then best_clock P divider must match the P
+ * divider from @match_clock used for LVDS downclocking.
+ */
 static bool
 pnv_find_best_dpll(const intel_limit_t *limit,
 		   struct intel_crtc_state *crtc_state,
@@ -819,6 +812,11 @@ pnv_find_best_dpll(const intel_limit_t *limit,
  * Returns a set of divisors for the desired target clock with the given
  * refclk, or FALSE.  The returned values represent the clock equation:
  * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
+ *
+ * Target and reference clocks are specified in kHz.
+ *
+ * If match_clock is provided, then best_clock P divider must match the P
+ * divider from @match_clock used for LVDS downclocking.
  */
 static bool
 g4x_find_best_dpll(const intel_limit_t *limit,
@@ -7904,43 +7902,67 @@ static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
 	return 0;
 }
 
+static int pnv_crtc_compute_clock(struct intel_crtc *crtc,
+				  struct intel_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	const intel_limit_t *limit;
+	int refclk = 96000;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		}
+
+		limit = &intel_limits_pineview_lvds;
+	} else {
+		limit = &intel_limits_pineview_sdvo;
+	}
+
+	if (!crtc_state->clock_set &&
+	    !pnv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	i9xx_compute_dpll(crtc, crtc_state, NULL);
+
+	return 0;
+}
+
 static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 				   struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	bool ok;
 	const intel_limit_t *limit;
 	int refclk = 96000;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	if (crtc_state->has_dsi_encoder)
-		return 0;
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		}
 
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv)) {
-		refclk = dev_priv->vbt.lvds_ssc_freq;
-		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		limit = &intel_limits_i9xx_lvds;
+	} else {
+		limit = &intel_limits_i9xx_sdvo;
 	}
 
-	if (!crtc_state->clock_set) {
-		/*
-		 * Returns a set of divisors for the desired target clock with
-		 * the given refclk, or FALSE.  The returned values represent
-		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
-		 * 2) / p1 / p2.
-		 */
-		limit = intel_limit(crtc_state, refclk);
-		ok = dev_priv->display.find_dpll(limit, crtc_state,
-						 crtc_state->port_clock,
-						 refclk, NULL,
-						 &crtc_state->dpll);
-		if (!ok) {
-			DRM_ERROR("Couldn't find PLL settings for mode!\n");
-			return -EINVAL;
-		}
+	if (!crtc_state->clock_set &&
+	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				 refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
 	}
 
 	i9xx_compute_dpll(crtc, crtc_state, NULL);
@@ -14925,11 +14947,6 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
  */
 void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 {
-	if (IS_PINEVIEW(dev_priv))
-		dev_priv->display.find_dpll = pnv_find_best_dpll;
-	else
-		dev_priv->display.find_dpll = i9xx_find_best_dpll;
-
 	if (INTEL_INFO(dev_priv)->gen >= 9) {
 		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
@@ -14975,6 +14992,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.crtc_compute_clock = g4x_crtc_compute_clock;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+	} else if (IS_PINEVIEW(dev_priv)) {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_initial_plane_config =
+			i9xx_get_initial_plane_config;
+		dev_priv->display.crtc_compute_clock = pnv_crtc_compute_clock;
+		dev_priv->display.crtc_enable = i9xx_crtc_enable;
+		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	} else if (!IS_GEN2(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✗ Fi.CI.BAT: warning for Clean up ironlake clock computation code (rev3)
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (14 preceding siblings ...)
  2016-03-21 16:00 ` [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-22  9:33 ` Patchwork
  2016-03-22 14:32 ` ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4) Patchwork
  16 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2016-03-22  9:33 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Clean up ironlake clock computation code (rev3)
URL   : https://patchwork.freedesktop.org/series/4367/
State : warning

== Summary ==

Series 4367v3 Clean up ironlake clock computation code
http://patchwork.freedesktop.org/api/1.0/series/4367/revisions/3/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (bdw-ultra)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (hsw-brixbox)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (hsw-brixbox)
                dmesg-warn -> PASS       (bdw-ultra)
Test kms_force_connector_basic:
        Subgroup force-load-detect:
                pass       -> SKIP       (ivb-t430s)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> DMESG-FAIL (snb-x220t)
                pass       -> DMESG-WARN (snb-dellxps)
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-x220t)

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:170  dwarn:1   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:192  pass:154  dwarn:1   dfail:0   fail:0   skip:37 
byt-nuc          total:192  pass:156  dwarn:1   dfail:0   fail:0   skip:35 
hsw-brixbox      total:192  pass:168  dwarn:2   dfail:0   fail:0   skip:22 
ilk-hp8440p      total:192  pass:129  dwarn:0   dfail:0   fail:0   skip:63 
ivb-t430s        total:192  pass:166  dwarn:0   dfail:0   fail:0   skip:26 
skl-i5k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
snb-dellxps      total:192  pass:156  dwarn:2   dfail:0   fail:0   skip:34 
snb-x220t        total:192  pass:158  dwarn:0   dfail:1   fail:0   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1667/

4b39223f6e3bef4dfa678f7239dcd87c38e20e96 drm-intel-nightly: 2016y-03m-21d-18h-43m-18s UTC integration manifest
5ffa03a3263f594e48a709f4f8db4e8a5223a788 drm/i915: Split PNV version of crtc_compute_clock()
6c10bd62fa294892c0974add54f3fbc9a2a87df0 drm/i915: Split g4x_crtc_compute_clock()
6ce10b173d2d185e47fa27a9b8615231e3ac7576 drm/i915: Split gen2_crtc_compute_clock()
8a930124289ebf7e0648ec68113219d6d0b56aee drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks
786c93f0215646da7970c0459581d9223e1c015f drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock()
75d5037c5dcf01adde98bde86198c26edb64e8dd drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
ec3e829edfebec88434cd04ae15497ac2f329026 drm/i915: Pass crtc_state->dpll directly to ->find_dpll()
a2a5d14a29776ec42b32cb2eb63de60c861b74e0 drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case
a73d99e0a247cc05e8df6a4282c11dffe82113b6 drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock()
0f7f126c302c5ea7b734f15283c721ec3a122190 drm/i915: Don't calculate a new clock in ILK+ code if it is already set
fc49c5eddcee69015304d050a16553331a61bba5 drm/i915: Simplify ironlake reduced clock logic a bit
66345371c0bf3388e69ef7cb7e9c4aa404f1a3ff drm/i915: Call g4x_find_best_dpll() directly from ILK+ code
665cb1ae40b53054383f3f27436571fa57017b97 drm/i915: Fold intel_ironlake_limit() into clock computation function
18537e1f57f96846a852c557f6b2940456b07c4a drm/i915: Merge ironlake_get_refclk() into its only caller
0533b5f21cccf343bf37e492f0a4a71a1c637541 drm/i915: Remove checks for cloned config with LVDS in dpll code

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code
  2016-03-21 16:00 ` [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code Ander Conselvan de Oliveira
@ 2016-03-22 10:11   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 10:11 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:02PM +0200, Ander Conselvan de Oliveira wrote:
> LVDS is not cloneable, so the check is unnecessary. Removing it makes
> the code neater.
> 
> v2: Remove checks from GMCH code too, not only ILK+. (Ville)
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 70 +++++++++---------------------------
>  1 file changed, 16 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 28ead66..710fd9a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -113,8 +113,7 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state);
> -static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> -			   int num_connectors);
> +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state);
>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
>  static void ironlake_pfit_enable(struct intel_crtc *crtc);
> @@ -1076,7 +1075,7 @@ chv_find_best_dpll(const intel_limit_t *limit,
>  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
>  			intel_clock_t *best_clock)
>  {
> -	int refclk = i9xx_get_refclk(crtc_state, 0);
> +	int refclk = i9xx_get_refclk(crtc_state);
>  
>  	return chv_find_best_dpll(intel_limit(crtc_state, refclk), crtc_state,
>  				  target_clock, refclk, NULL, best_clock);
> @@ -7113,8 +7112,7 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> -static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
> -			   int num_connectors)
> +static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7125,7 +7123,7 @@ static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
>  	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) {
>  		refclk = 100000;
>  	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -	    intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
> +	    intel_panel_use_ssc(dev_priv)) {
>  		refclk = dev_priv->vbt.lvds_ssc_freq;
>  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
>  	} else if (!IS_GEN2(dev)) {
> @@ -7566,8 +7564,7 @@ void vlv_force_pll_off(struct drm_device *dev, enum pipe pipe)
>  
>  static void i9xx_compute_dpll(struct intel_crtc *crtc,
>  			      struct intel_crtc_state *crtc_state,
> -			      intel_clock_t *reduced_clock,
> -			      int num_connectors)
> +			      intel_clock_t *reduced_clock)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7626,7 +7623,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
>  	if (crtc_state->sdvo_tv_clock)
>  		dpll |= PLL_REF_INPUT_TVCLKINBC;
>  	else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> +		 intel_panel_use_ssc(dev_priv))
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
> @@ -7643,8 +7640,7 @@ static void i9xx_compute_dpll(struct intel_crtc *crtc,
>  
>  static void i8xx_compute_dpll(struct intel_crtc *crtc,
>  			      struct intel_crtc_state *crtc_state,
> -			      intel_clock_t *reduced_clock,
> -			      int num_connectors)
> +			      intel_clock_t *reduced_clock)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7670,7 +7666,7 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
>  		dpll |= DPLL_DVO_2X_MODE;
>  
>  	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -		 intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> +	    intel_panel_use_ssc(dev_priv))
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
> @@ -7898,14 +7894,10 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int refclk, num_connectors = 0;
> +	int refclk;
>  	intel_clock_t clock;
>  	bool ok;
>  	const intel_limit_t *limit;
> -	struct drm_atomic_state *state = crtc_state->base.state;
> -	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> -	int i;
>  
>  	memset(&crtc_state->dpll_hw_state, 0,
>  	       sizeof(crtc_state->dpll_hw_state));
> @@ -7913,13 +7905,8 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	if (crtc_state->has_dsi_encoder)
>  		return 0;
>  
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> -		if (connector_state->crtc == &crtc->base)
> -			num_connectors++;
> -	}
> -
>  	if (!crtc_state->clock_set) {
> -		refclk = i9xx_get_refclk(crtc_state, num_connectors);
> +		refclk = i9xx_get_refclk(crtc_state);
>  
>  		/*
>  		 * Returns a set of divisors for the desired target clock with
> @@ -7945,15 +7932,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	}
>  
>  	if (IS_GEN2(dev)) {
> -		i8xx_compute_dpll(crtc, crtc_state, NULL,
> -				  num_connectors);
> +		i8xx_compute_dpll(crtc, crtc_state, NULL);
>  	} else if (IS_CHERRYVIEW(dev)) {
>  		chv_compute_dpll(crtc, crtc_state);
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		vlv_compute_dpll(crtc, crtc_state);
>  	} else {
> -		i9xx_compute_dpll(crtc, crtc_state, NULL,
> -				  num_connectors);
> +		i9xx_compute_dpll(crtc, crtc_state, NULL);
>  	}
>  
>  	return 0;
> @@ -8639,30 +8624,9 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_atomic_state *state = crtc_state->base.state;
> -	struct drm_connector *connector;
> -	struct drm_connector_state *connector_state;
> -	struct intel_encoder *encoder;
> -	int num_connectors = 0, i;
> -	bool is_lvds = false;
> -
> -	for_each_connector_in_state(state, connector, connector_state, i) {
> -		if (connector_state->crtc != crtc_state->base.crtc)
> -			continue;
> -
> -		encoder = to_intel_encoder(connector_state->best_encoder);
> -
> -		switch (encoder->type) {
> -		case INTEL_OUTPUT_LVDS:
> -			is_lvds = true;
> -			break;
> -		default:
> -			break;
> -		}
> -		num_connectors++;
> -	}
>  
> -	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2) {
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> +	    intel_panel_use_ssc(dev_priv)) {
>  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
>  			      dev_priv->vbt.lvds_ssc_freq);
>  		return dev_priv->vbt.lvds_ssc_freq;
> @@ -8896,7 +8860,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
>  	uint32_t dpll;
> -	int factor, num_connectors = 0, i;
> +	int factor, i;
>  	bool is_lvds = false, is_sdvo = false;
>  
>  	for_each_connector_in_state(state, connector, connector_state, i) {
> @@ -8916,8 +8880,6 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		default:
>  			break;
>  		}
> -
> -		num_connectors++;
>  	}
>  
>  	/* Enable autotuning of the PLL clock (if permissible) */
> @@ -8971,7 +8933,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  		break;
>  	}
>  
> -	if (is_lvds && intel_panel_use_ssc(dev_priv) && num_connectors < 2)
> +	if (is_lvds && intel_panel_use_ssc(dev_priv))
>  		dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll()
  2016-03-21 16:00 ` [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll() Ander Conselvan de Oliveira
@ 2016-03-22 10:18   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 10:18 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:10PM +0200, Ander Conselvan de Oliveira wrote:
> When calculating clocks, just pass a pointer to crtc_state->dpll
> directly to the find_dpll() hook. Back when this was introduced in
> commit f47709a9502f3 ("drm/i915: create pipe_config->dpll for clock
> state") there was no staged crtc config or atomic crtc state, so it was
> possible to overwrite the current configuration on error. That hasn't
> been the case for a while now, so finally make it "disappear".
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

We'll populate the derived valeus in the crtc_state->dpll now too, but
IIRC the shared dpll code and state checks only use the dpll_hw_state
so this should totally OK.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 34 ++++++++++------------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0af1e7d..0b6eabf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7871,7 +7871,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int refclk;
> -	intel_clock_t clock;
>  	bool ok;
>  	const intel_limit_t *limit;
>  
> @@ -7893,18 +7892,12 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		limit = intel_limit(crtc_state, refclk);
>  		ok = dev_priv->display.find_dpll(limit, crtc_state,
>  						 crtc_state->port_clock,
> -						 refclk, NULL, &clock);
> +						 refclk, NULL,
> +						 &crtc_state->dpll);
>  		if (!ok) {
>  			DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  			return -EINVAL;
>  		}
> -
> -		/* Compat-code for transition, will disappear. */
> -		crtc_state->dpll.n = clock.n;
> -		crtc_state->dpll.m1 = clock.m1;
> -		crtc_state->dpll.m2 = clock.m2;
> -		crtc_state->dpll.p1 = clock.p1;
> -		crtc_state->dpll.p2 = clock.p2;
>  	}
>  
>  	if (IS_GEN2(dev)) {
> @@ -8925,7 +8918,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *crtc_state)
>  {
> -	intel_clock_t clock, reduced_clock;
> +	intel_clock_t reduced_clock;
>  	u32 dpll = 0, fp = 0, fp2 = 0;
>  	bool has_reduced_clock = false;
>  	struct intel_shared_dpll *pll;
> @@ -8939,20 +8932,13 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  	if (!crtc_state->has_pch_encoder)
>  		return 0;
>  
> -	if (!crtc_state->clock_set) {
> -		if (!ironlake_compute_clocks(&crtc->base, crtc_state, &clock,
> -					     &has_reduced_clock,
> -					     &reduced_clock)) {
> -			DRM_ERROR("Couldn't find PLL settings for mode!\n");
> -			return -EINVAL;
> -		}
> -
> -		/* Compat-code for transition, will disappear. */
> -		crtc_state->dpll.n = clock.n;
> -		crtc_state->dpll.m1 = clock.m1;
> -		crtc_state->dpll.m2 = clock.m2;
> -		crtc_state->dpll.p1 = clock.p1;
> -		crtc_state->dpll.p2 = clock.p2;
> +	if (!crtc_state->clock_set &&
> +	    !ironlake_compute_clocks(&crtc->base, crtc_state,
> +				     &crtc_state->dpll,
> +				     &has_reduced_clock,
> +				     &reduced_clock)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
>  	}
>  
>  	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock()
  2016-03-21 16:00 ` [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-22 10:24   ` Ville Syrjälä
  2016-03-22 11:11     ` Daniel Vetter
  0 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 10:24 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:14PM +0200, Ander Conselvan de Oliveira wrote:
> Split a GEN2 specific version from i9xx_crtc_compute_clock(). With this
> there is no need for i9xx_get_refclk() anymore, and the differences
> between platforms become more obvious.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Let's call it i8xx shall we. That's the more typical convention in the
modeset code.

Otherwise looks OK, so with that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 91 +++++++++++++++++++++---------------
>  1 file changed, 53 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dca5b15..245d6c6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -595,7 +595,7 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  	const intel_limit_t *limit;
>  
>  	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
> -	    HAS_PCH_SPLIT(dev))
> +	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
>  		limit = NULL;
>  
>  	if (IS_G4X(dev)) {
> @@ -610,13 +610,6 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  			limit = &intel_limits_i9xx_lvds;
>  		else
>  			limit = &intel_limits_i9xx_sdvo;
> -	} else {
> -		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
> -			limit = &intel_limits_i8xx_lvds;
> -		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
> -			limit = &intel_limits_i8xx_dvo;
> -		else
> -			limit = &intel_limits_i8xx_dac;
>  	}
>  
>  	WARN_ON(limit == NULL);
> @@ -7102,27 +7095,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
>  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
>  }
>  
> -static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
> -{
> -	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int refclk;
> -
> -	WARN_ON(!crtc_state->base.state);
> -
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -	    intel_panel_use_ssc(dev_priv)) {
> -		refclk = dev_priv->vbt.lvds_ssc_freq;
> -		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> -	} else if (!IS_GEN2(dev)) {
> -		refclk = 96000;
> -	} else {
> -		refclk = 48000;
> -	}
> -
> -	return refclk;
> -}
> -
>  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
>  {
>  	return (1 << dpll->n) << 16 | dpll->m2;
> @@ -7877,14 +7849,50 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  	POSTING_READ(PIPECONF(intel_crtc->pipe));
>  }
>  
> +static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	const intel_limit_t *limit;
> +	int refclk = 48000;
> +
> +	memset(&crtc_state->dpll_hw_state, 0,
> +	       sizeof(crtc_state->dpll_hw_state));
> +
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> +		if (intel_panel_use_ssc(dev_priv)) {
> +			refclk = dev_priv->vbt.lvds_ssc_freq;
> +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +		}
> +
> +		limit = &intel_limits_i8xx_lvds;
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO)) {
> +		limit = &intel_limits_i8xx_dvo;
> +	} else {
> +		limit = &intel_limits_i8xx_dac;
> +	}
> +
> +	if (!crtc_state->clock_set &&
> +	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				 refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
> +	}
> +
> +	i8xx_compute_dpll(crtc, crtc_state, NULL);
> +
> +	return 0;
> +}
> +
>  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int refclk;
>  	bool ok;
>  	const intel_limit_t *limit;
> +	int refclk = 96000;
>  
>  	memset(&crtc_state->dpll_hw_state, 0,
>  	       sizeof(crtc_state->dpll_hw_state));
> @@ -7892,9 +7900,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	if (crtc_state->has_dsi_encoder)
>  		return 0;
>  
> -	if (!crtc_state->clock_set) {
> -		refclk = i9xx_get_refclk(crtc_state);
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> +	    intel_panel_use_ssc(dev_priv)) {
> +		refclk = dev_priv->vbt.lvds_ssc_freq;
> +		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +	}
>  
> +	if (!crtc_state->clock_set) {
>  		/*
>  		 * Returns a set of divisors for the desired target clock with
>  		 * the given refclk, or FALSE.  The returned values represent
> @@ -7912,11 +7924,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  		}
>  	}
>  
> -	if (IS_GEN2(dev)) {
> -		i8xx_compute_dpll(crtc, crtc_state, NULL);
> -	} else {
> -		i9xx_compute_dpll(crtc, crtc_state, NULL);
> -	}
> +	i9xx_compute_dpll(crtc, crtc_state, NULL);
>  
>  	return 0;
>  }
> @@ -14943,13 +14951,20 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> -	} else {
> +	} else if (!IS_GEN2(dev_priv)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
>  			i9xx_get_initial_plane_config;
>  		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> +	} else {
> +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> +		dev_priv->display.get_initial_plane_config =
> +			i9xx_get_initial_plane_config;
> +		dev_priv->display.crtc_compute_clock = gen2_crtc_compute_clock;
> +		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  	}
>  
>  	/* Returns the core display clock speed */
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks
  2016-03-21 16:00 ` [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks Ander Conselvan de Oliveira
@ 2016-03-22 10:26   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 10:26 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:13PM +0200, Ander Conselvan de Oliveira wrote:
> In order for VLV and CHV to use i9xx_crtc_compute_clocks(), a number
> of if ladders is necessary: one for setting the find_dpll() hook, one
> for choosing the limits struct, one for choosing the right compute dpll
> function and one for initializing the crtc_compute_clock() hook.
> 
> By extracting a platform specific implementation for each platform, the
> number of if-ladders is reduced to one.
> 
> While at it also clean up bxt_find_best_dpll() which depends on some of
> the CHV code.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 102 ++++++++++++++++++++++++++---------
>  1 file changed, 78 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 13b509e..dca5b15 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -113,7 +113,6 @@ static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state);
> -static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state);
>  static void skylake_pfit_enable(struct intel_crtc *crtc);
>  static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
>  static void ironlake_pfit_enable(struct intel_crtc *crtc);
> @@ -595,21 +594,17 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
>  
> -	if (IS_BROXTON(dev))
> -		limit = &intel_limits_bxt;
> -	else if (WARN_ON(HAS_PCH_SPLIT(dev)))
> +	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
> +	    HAS_PCH_SPLIT(dev))
>  		limit = NULL;
> -	else if (IS_G4X(dev)) {
> +
> +	if (IS_G4X(dev)) {
>  		limit = intel_g4x_limit(crtc_state);
>  	} else if (IS_PINEVIEW(dev)) {
>  		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_pineview_lvds;
>  		else
>  			limit = &intel_limits_pineview_sdvo;
> -	} else if (IS_CHERRYVIEW(dev)) {
> -		limit = &intel_limits_chv;
> -	} else if (IS_VALLEYVIEW(dev)) {
> -		limit = &intel_limits_vlv;
>  	} else if (!IS_GEN2(dev)) {
>  		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i9xx_lvds;
> @@ -623,6 +618,9 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  		else
>  			limit = &intel_limits_i8xx_dac;
>  	}
> +
> +	WARN_ON(limit == NULL);
> +
>  	return limit;
>  }
>  
> @@ -941,6 +939,11 @@ static bool vlv_PLL_is_optimal(struct drm_device *dev, int target_freq,
>  	return *error_ppm + 10 < best_error_ppm;
>  }
>  
> +/*
> + * Returns a set of divisors for the desired target clock with the given
> + * refclk, or FALSE.  The returned values represent the clock equation:
> + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + */
>  static bool
>  vlv_find_best_dpll(const intel_limit_t *limit,
>  		   struct intel_crtc_state *crtc_state,
> @@ -995,6 +998,11 @@ vlv_find_best_dpll(const intel_limit_t *limit,
>  	return found;
>  }
>  
> +/*
> + * Returns a set of divisors for the desired target clock with the given
> + * refclk, or FALSE.  The returned values represent the clock equation:
> + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + */
>  static bool
>  chv_find_best_dpll(const intel_limit_t *limit,
>  		   struct intel_crtc_state *crtc_state,
> @@ -1056,9 +1064,10 @@ chv_find_best_dpll(const intel_limit_t *limit,
>  bool bxt_find_best_dpll(struct intel_crtc_state *crtc_state, int target_clock,
>  			intel_clock_t *best_clock)
>  {
> -	int refclk = i9xx_get_refclk(crtc_state);
> +	int refclk = 100000;
> +	const intel_limit_t *limit = &intel_limits_bxt;
>  
> -	return chv_find_best_dpll(intel_limit(crtc_state, refclk), crtc_state,
> +	return chv_find_best_dpll(limit, crtc_state,
>  				  target_clock, refclk, NULL, best_clock);
>  }
>  
> @@ -7101,9 +7110,7 @@ static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
>  
>  	WARN_ON(!crtc_state->base.state);
>  
> -	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || IS_BROXTON(dev)) {
> -		refclk = 100000;
> -	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
>  	    intel_panel_use_ssc(dev_priv)) {
>  		refclk = dev_priv->vbt.lvds_ssc_freq;
>  		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> @@ -7907,10 +7914,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  
>  	if (IS_GEN2(dev)) {
>  		i8xx_compute_dpll(crtc, crtc_state, NULL);
> -	} else if (IS_CHERRYVIEW(dev)) {
> -		chv_compute_dpll(crtc, crtc_state);
> -	} else if (IS_VALLEYVIEW(dev)) {
> -		vlv_compute_dpll(crtc, crtc_state);
>  	} else {
>  		i9xx_compute_dpll(crtc, crtc_state, NULL);
>  	}
> @@ -7918,6 +7921,54 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  	return 0;
>  }
>  
> +static int chv_crtc_compute_clock(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state)
> +{
> +	int refclk = 100000;
> +	const intel_limit_t *limit = &intel_limits_chv;
> +
> +	memset(&crtc_state->dpll_hw_state, 0,
> +	       sizeof(crtc_state->dpll_hw_state));
> +
> +	if (crtc_state->has_dsi_encoder)
> +		return 0;
> +
> +	if (!crtc_state->clock_set &&
> +	    !chv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
> +	}
> +
> +	chv_compute_dpll(crtc, crtc_state);
> +
> +	return 0;
> +}
> +
> +static int vlv_crtc_compute_clock(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state)
> +{
> +	int refclk = 100000;
> +	const intel_limit_t *limit = &intel_limits_vlv;
> +
> +	memset(&crtc_state->dpll_hw_state, 0,
> +	       sizeof(crtc_state->dpll_hw_state));
> +
> +	if (crtc_state->has_dsi_encoder)
> +		return 0;
> +
> +	if (!crtc_state->clock_set &&
> +	    !vlv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
> +	}
> +
> +	vlv_compute_dpll(crtc, crtc_state);
> +
> +	return 0;
> +}
> +
>  static void i9xx_get_pfit_config(struct intel_crtc *crtc,
>  				 struct intel_crtc_state *pipe_config)
>  {
> @@ -14849,10 +14900,6 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
>  		dev_priv->display.find_dpll = g4x_find_best_dpll;
> -	else if (IS_CHERRYVIEW(dev_priv))
> -		dev_priv->display.find_dpll = chv_find_best_dpll;
> -	else if (IS_VALLEYVIEW(dev_priv))
> -		dev_priv->display.find_dpll = vlv_find_best_dpll;
>  	else if (IS_PINEVIEW(dev_priv))
>  		dev_priv->display.find_dpll = pnv_find_best_dpll;
>  	else
> @@ -14882,11 +14929,18 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  			ironlake_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = ironlake_crtc_enable;
>  		dev_priv->display.crtc_disable = ironlake_crtc_disable;
> -	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> +	} else if (IS_CHERRYVIEW(dev_priv)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
>  			i9xx_get_initial_plane_config;
> -		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
> +		dev_priv->display.crtc_compute_clock = chv_crtc_compute_clock;
> +		dev_priv->display.crtc_enable = valleyview_crtc_enable;
> +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> +	} else if (IS_VALLEYVIEW(dev_priv)) {
> +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> +		dev_priv->display.get_initial_plane_config =
> +			i9xx_get_initial_plane_config;
> +		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  	} else {
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock()
  2016-03-22 10:24   ` Ville Syrjälä
@ 2016-03-22 11:11     ` Daniel Vetter
  2016-03-22 13:35       ` [PATCH v2 13/15] drm/i915: Split i8xx_crtc_compute_clock() Ander Conselvan de Oliveira
  0 siblings, 1 reply; 30+ messages in thread
From: Daniel Vetter @ 2016-03-22 11:11 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Ander Conselvan de Oliveira, intel-gfx

On Tue, Mar 22, 2016 at 12:24:27PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 21, 2016 at 06:00:14PM +0200, Ander Conselvan de Oliveira wrote:
> > Split a GEN2 specific version from i9xx_crtc_compute_clock(). With this
> > there is no need for i9xx_get_refclk() anymore, and the differences
> > between platforms become more obvious.
> > 
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> 
> Let's call it i8xx shall we. That's the more typical convention in the
> modeset code.

Yeah, wanted to drop the same bikeshed - genX is generally what we use for
render side stuff, whereas product names are more used for modeset, simply
because that tends to be the splits along which IP blocks are reused.
-Daniel

> 
> Otherwise looks OK, so with that
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 91 +++++++++++++++++++++---------------
> >  1 file changed, 53 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index dca5b15..245d6c6 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -595,7 +595,7 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
> >  	const intel_limit_t *limit;
> >  
> >  	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
> > -	    HAS_PCH_SPLIT(dev))
> > +	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
> >  		limit = NULL;
> >  
> >  	if (IS_G4X(dev)) {
> > @@ -610,13 +610,6 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
> >  			limit = &intel_limits_i9xx_lvds;
> >  		else
> >  			limit = &intel_limits_i9xx_sdvo;
> > -	} else {
> > -		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
> > -			limit = &intel_limits_i8xx_lvds;
> > -		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
> > -			limit = &intel_limits_i8xx_dvo;
> > -		else
> > -			limit = &intel_limits_i8xx_dac;
> >  	}
> >  
> >  	WARN_ON(limit == NULL);
> > @@ -7102,27 +7095,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
> >  		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
> >  }
> >  
> > -static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
> > -{
> > -	struct drm_device *dev = crtc_state->base.crtc->dev;
> > -	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	int refclk;
> > -
> > -	WARN_ON(!crtc_state->base.state);
> > -
> > -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > -	    intel_panel_use_ssc(dev_priv)) {
> > -		refclk = dev_priv->vbt.lvds_ssc_freq;
> > -		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> > -	} else if (!IS_GEN2(dev)) {
> > -		refclk = 96000;
> > -	} else {
> > -		refclk = 48000;
> > -	}
> > -
> > -	return refclk;
> > -}
> > -
> >  static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
> >  {
> >  	return (1 << dpll->n) << 16 | dpll->m2;
> > @@ -7877,14 +7849,50 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
> >  	POSTING_READ(PIPECONF(intel_crtc->pipe));
> >  }
> >  
> > +static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
> > +				   struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_device *dev = crtc->base.dev;
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	const intel_limit_t *limit;
> > +	int refclk = 48000;
> > +
> > +	memset(&crtc_state->dpll_hw_state, 0,
> > +	       sizeof(crtc_state->dpll_hw_state));
> > +
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> > +		if (intel_panel_use_ssc(dev_priv)) {
> > +			refclk = dev_priv->vbt.lvds_ssc_freq;
> > +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> > +		}
> > +
> > +		limit = &intel_limits_i8xx_lvds;
> > +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO)) {
> > +		limit = &intel_limits_i8xx_dvo;
> > +	} else {
> > +		limit = &intel_limits_i8xx_dac;
> > +	}
> > +
> > +	if (!crtc_state->clock_set &&
> > +	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> > +				 refclk, NULL, &crtc_state->dpll)) {
> > +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	i8xx_compute_dpll(crtc, crtc_state, NULL);
> > +
> > +	return 0;
> > +}
> > +
> >  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> >  				   struct intel_crtc_state *crtc_state)
> >  {
> >  	struct drm_device *dev = crtc->base.dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > -	int refclk;
> >  	bool ok;
> >  	const intel_limit_t *limit;
> > +	int refclk = 96000;
> >  
> >  	memset(&crtc_state->dpll_hw_state, 0,
> >  	       sizeof(crtc_state->dpll_hw_state));
> > @@ -7892,9 +7900,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> >  	if (crtc_state->has_dsi_encoder)
> >  		return 0;
> >  
> > -	if (!crtc_state->clock_set) {
> > -		refclk = i9xx_get_refclk(crtc_state);
> > +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> > +	    intel_panel_use_ssc(dev_priv)) {
> > +		refclk = dev_priv->vbt.lvds_ssc_freq;
> > +		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> > +	}
> >  
> > +	if (!crtc_state->clock_set) {
> >  		/*
> >  		 * Returns a set of divisors for the desired target clock with
> >  		 * the given refclk, or FALSE.  The returned values represent
> > @@ -7912,11 +7924,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
> >  		}
> >  	}
> >  
> > -	if (IS_GEN2(dev)) {
> > -		i8xx_compute_dpll(crtc, crtc_state, NULL);
> > -	} else {
> > -		i9xx_compute_dpll(crtc, crtc_state, NULL);
> > -	}
> > +	i9xx_compute_dpll(crtc, crtc_state, NULL);
> >  
> >  	return 0;
> >  }
> > @@ -14943,13 +14951,20 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
> >  		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
> >  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
> >  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> > -	} else {
> > +	} else if (!IS_GEN2(dev_priv)) {
> >  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> >  		dev_priv->display.get_initial_plane_config =
> >  			i9xx_get_initial_plane_config;
> >  		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
> >  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> >  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> > +	} else {
> > +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> > +		dev_priv->display.get_initial_plane_config =
> > +			i9xx_get_initial_plane_config;
> > +		dev_priv->display.crtc_compute_clock = gen2_crtc_compute_clock;
> > +		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> > +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> >  	}
> >  
> >  	/* Returns the core display clock speed */
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
  2016-03-21 16:00 ` [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll() Ander Conselvan de Oliveira
@ 2016-03-22 12:49   ` Ville Syrjälä
  2016-03-22 13:22     ` Ander Conselvan De Oliveira
  0 siblings, 1 reply; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 12:49 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:11PM +0200, Ander Conselvan de Oliveira wrote:
> Follow what is done in i8xx_compute_dpll() and i9xx_compute_dpll() and
> move the lower level details of setting crtc_state->dpll_hw_state into
> the _compute_dpll() function.

Missing sob.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b6eabf..b6541a0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8821,10 +8821,9 @@ static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor)
>  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
>  }
>  
> -static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> -				      struct intel_crtc_state *crtc_state,
> -				      u32 *fp,
> -				      intel_clock_t *reduced_clock, u32 *fp2)
> +static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> +				  struct intel_crtc_state *crtc_state,
> +				  intel_clock_t *reduced_clock)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
>  	struct drm_device *dev = crtc->dev;
> @@ -8833,7 +8832,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	struct drm_connector *connector;
>  	struct drm_connector_state *connector_state;
>  	struct intel_encoder *encoder;
> -	uint32_t dpll;
> +	u32 dpll, fp, fp2;
>  	int factor, i;
>  	bool is_lvds = false, is_sdvo = false;
>  
> @@ -8866,11 +8865,19 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	} else if (crtc_state->sdvo_tv_clock)
>  		factor = 20;
>  
> +	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> +
>  	if (ironlake_needs_fb_cb_tune(&crtc_state->dpll, factor))
> -		*fp |= FP_CB_TUNE;
> +		fp |= FP_CB_TUNE;
> +
> +	if (reduced_clock) {
> +		fp2 = i9xx_dpll_compute_fp(reduced_clock);
>  
> -	if (fp2 && (reduced_clock->m < factor * reduced_clock->n))
> -		*fp2 |= FP_CB_TUNE;
> +		if (reduced_clock->m < factor * reduced_clock->n)
> +			fp2 |= FP_CB_TUNE;
> +	} else {
> +		fp2 = fp;
> +	}
>  
>  	dpll = 0;
>  
> @@ -8912,14 +8919,17 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  	else
>  		dpll |= PLL_REF_INPUT_DREFCLK;
>  
> -	return dpll | DPLL_VCO_ENABLE;
> +	dpll |= DPLL_VCO_ENABLE;
> +
> +	crtc_state->dpll_hw_state.dpll = dpll;
> +	crtc_state->dpll_hw_state.fp0 = fp;
> +	crtc_state->dpll_hw_state.fp1 = fp2;
>  }
>  
>  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *crtc_state)
>  {
>  	intel_clock_t reduced_clock;
> -	u32 dpll = 0, fp = 0, fp2 = 0;
>  	bool has_reduced_clock = false;
>  	struct intel_shared_dpll *pll;
>  
> @@ -8941,19 +8951,8 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> -	if (has_reduced_clock)
> -		fp2 = i9xx_dpll_compute_fp(&reduced_clock);
> -	else
> -		fp2 = fp;
> -
> -	dpll = ironlake_compute_dpll(crtc, crtc_state,
> -				     &fp, &reduced_clock,
> -				     has_reduced_clock ? &fp2 : NULL);
> -
> -	crtc_state->dpll_hw_state.dpll = dpll;
> -	crtc_state->dpll_hw_state.fp0 = fp;
> -	crtc_state->dpll_hw_state.fp1 = fp2;
> +	ironlake_compute_dpll(crtc, crtc_state,
> +			      has_reduced_clock ? &reduced_clock : NULL);
>  
>  	pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
>  	if (pll == NULL) {
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock()
  2016-03-21 16:00 ` [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-22 12:51   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 12:51 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:12PM +0200, Ander Conselvan de Oliveira wrote:
> Merge ironlake_compute_clocks() into ironlake_crtc_compute_clock() so
> the clock computation logic is all in one place. The resulting function
> is still quite simple. Follow up patches will make the similar code for
> GMCH platforms look similar.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 86 ++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b6541a0..13b509e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -848,6 +848,11 @@ pnv_find_best_dpll(const intel_limit_t *limit,
>  	return (err != target);
>  }
>  
> +/*
> + * Returns a set of divisors for the desired target clock with the given
> + * refclk, or FALSE.  The returned values represent the clock equation:
> + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + */
>  static bool
>  g4x_find_best_dpll(const intel_limit_t *limit,
>  		   struct intel_crtc_state *crtc_state,
> @@ -8756,55 +8761,6 @@ static void haswell_set_pipemisc(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static bool ironlake_compute_clocks(struct drm_crtc *crtc,
> -				    struct intel_crtc_state *crtc_state,
> -				    intel_clock_t *clock,
> -				    bool *has_reduced_clock,
> -				    intel_clock_t *reduced_clock)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	int refclk;
> -	const intel_limit_t *limit;
> -	bool ret;
> -
> -	refclk = 120000;
> -
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> -		if (intel_panel_use_ssc(dev_priv)) {
> -			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> -				      dev_priv->vbt.lvds_ssc_freq);
> -			refclk = dev_priv->vbt.lvds_ssc_freq;
> -		}
> -
> -		if (intel_is_dual_link_lvds(dev)) {
> -			if (refclk == 100000)
> -				limit = &intel_limits_ironlake_dual_lvds_100m;
> -			else
> -				limit = &intel_limits_ironlake_dual_lvds;
> -		} else {
> -			if (refclk == 100000)
> -				limit = &intel_limits_ironlake_single_lvds_100m;
> -			else
> -				limit = &intel_limits_ironlake_single_lvds;
> -		}
> -	} else {
> -		limit = &intel_limits_ironlake_dac;
> -	}
> -
> -	/*
> -	 * Returns a set of divisors for the desired target clock with the given
> -	 * refclk, or FALSE.  The returned values represent the clock equation:
> -	 * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> -	 */
> -	ret = g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> -				 refclk, NULL, clock);
> -	if (!ret)
> -		return false;
> -
> -	return true;
> -}
> -
>  int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp)
>  {
>  	/*
> @@ -8929,9 +8885,13 @@ static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
>  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *crtc_state)
>  {
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	intel_clock_t reduced_clock;
>  	bool has_reduced_clock = false;
>  	struct intel_shared_dpll *pll;
> +	const intel_limit_t *limit;
> +	int refclk = 120000;
>  
>  	memset(&crtc_state->dpll_hw_state, 0,
>  	       sizeof(crtc_state->dpll_hw_state));
> @@ -8942,11 +8902,31 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
>  	if (!crtc_state->has_pch_encoder)
>  		return 0;
>  
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> +		if (intel_panel_use_ssc(dev_priv)) {
> +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n",
> +				      dev_priv->vbt.lvds_ssc_freq);
> +			refclk = dev_priv->vbt.lvds_ssc_freq;
> +		}
> +
> +		if (intel_is_dual_link_lvds(dev)) {
> +			if (refclk == 100000)
> +				limit = &intel_limits_ironlake_dual_lvds_100m;
> +			else
> +				limit = &intel_limits_ironlake_dual_lvds;
> +		} else {
> +			if (refclk == 100000)
> +				limit = &intel_limits_ironlake_single_lvds_100m;
> +			else
> +				limit = &intel_limits_ironlake_single_lvds;
> +		}
> +	} else {
> +		limit = &intel_limits_ironlake_dac;
> +	}
> +
>  	if (!crtc_state->clock_set &&
> -	    !ironlake_compute_clocks(&crtc->base, crtc_state,
> -				     &crtc_state->dpll,
> -				     &has_reduced_clock,
> -				     &reduced_clock)) {
> +	    !g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				refclk, NULL, &crtc_state->dpll)) {
>  		DRM_ERROR("Couldn't find PLL settings for mode!\n");
>  		return -EINVAL;
>  	}
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock()
  2016-03-21 16:00 ` [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-22 12:52   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 12:52 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:15PM +0200, Ander Conselvan de Oliveira wrote:
> Split a G4X specific version from i9xx_crtc_compute_clock(). With this
> the differences between platforms become more obvious.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 82 +++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 245d6c6..552fd4c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -567,40 +567,16 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
>  }
>  
>  static const intel_limit_t *
> -intel_g4x_limit(struct intel_crtc_state *crtc_state)
> -{
> -	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	const intel_limit_t *limit;
> -
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> -		if (intel_is_dual_link_lvds(dev))
> -			limit = &intel_limits_g4x_dual_channel_lvds;
> -		else
> -			limit = &intel_limits_g4x_single_channel_lvds;
> -	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
> -		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> -		limit = &intel_limits_g4x_hdmi;
> -	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
> -		limit = &intel_limits_g4x_sdvo;
> -	} else /* The option is for other outputs */
> -		limit = &intel_limits_i9xx_sdvo;
> -
> -	return limit;
> -}
> -
> -static const intel_limit_t *
>  intel_limit(struct intel_crtc_state *crtc_state, int refclk)
>  {
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	const intel_limit_t *limit;
>  
>  	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
> -	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
> +	    HAS_PCH_SPLIT(dev) || IS_G4X(dev) || IS_GEN2(dev))
>  		limit = NULL;
>  
> -	if (IS_G4X(dev)) {
> -		limit = intel_g4x_limit(crtc_state);
> -	} else if (IS_PINEVIEW(dev)) {
> +	if (IS_PINEVIEW(dev)) {
>  		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_pineview_lvds;
>  		else
> @@ -7885,6 +7861,49 @@ static int gen2_crtc_compute_clock(struct intel_crtc *crtc,
>  	return 0;
>  }
>  
> +static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	const intel_limit_t *limit;
> +	int refclk = 96000;
> +
> +	memset(&crtc_state->dpll_hw_state, 0,
> +	       sizeof(crtc_state->dpll_hw_state));
> +
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> +		if (intel_panel_use_ssc(dev_priv)) {
> +			refclk = dev_priv->vbt.lvds_ssc_freq;
> +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +		}
> +
> +		if (intel_is_dual_link_lvds(dev))
> +			limit = &intel_limits_g4x_dual_channel_lvds;
> +		else
> +			limit = &intel_limits_g4x_single_channel_lvds;
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_HDMI) ||
> +		   intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
> +		limit = &intel_limits_g4x_hdmi;
> +	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_SDVO)) {
> +		limit = &intel_limits_g4x_sdvo;
> +	} else {
> +		/* The option is for other outputs */
> +		limit = &intel_limits_i9xx_sdvo;
> +	}
> +
> +	if (!crtc_state->clock_set &&
> +	    !g4x_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
> +	}
> +
> +	i9xx_compute_dpll(crtc, crtc_state, NULL);
> +
> +	return 0;
> +}
> +
>  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *crtc_state)
>  {
> @@ -14906,9 +14925,7 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
>   */
>  void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  {
> -	if (HAS_PCH_SPLIT(dev_priv) || IS_G4X(dev_priv))
> -		dev_priv->display.find_dpll = g4x_find_best_dpll;
> -	else if (IS_PINEVIEW(dev_priv))
> +	if (IS_PINEVIEW(dev_priv))
>  		dev_priv->display.find_dpll = pnv_find_best_dpll;
>  	else
>  		dev_priv->display.find_dpll = i9xx_find_best_dpll;
> @@ -14951,6 +14968,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = valleyview_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> +	} else if (IS_G4X(dev_priv)) {
> +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> +		dev_priv->display.get_initial_plane_config =
> +			i9xx_get_initial_plane_config;
> +		dev_priv->display.crtc_compute_clock = g4x_crtc_compute_clock;
> +		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  	} else if (!IS_GEN2(dev_priv)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock()
  2016-03-21 16:00 ` [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock() Ander Conselvan de Oliveira
@ 2016-03-22 12:55   ` Ville Syrjälä
  0 siblings, 0 replies; 30+ messages in thread
From: Ville Syrjälä @ 2016-03-22 12:55 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Mon, Mar 21, 2016 at 06:00:16PM +0200, Ander Conselvan de Oliveira wrote:
> Split a pnv_crtc_compute_clock(), so the differences between platforms
> become more obvious.
> 
> With this, there are no more users of intel_limit() or the ->find_dpll()
> hook, so get rid of them.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  18 -----
>  drivers/gpu/drm/i915/intel_display.c | 134 +++++++++++++++++++++--------------
>  2 files changed, 79 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index efca534..83ee8d7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -575,24 +575,6 @@ struct dpll;
>  struct drm_i915_display_funcs {
>  	int (*get_display_clock_speed)(struct drm_device *dev);
>  	int (*get_fifo_size)(struct drm_device *dev, int plane);
> -	/**
> -	 * find_dpll() - 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_dpll)(const struct intel_limit *limit,
> -			  struct intel_crtc_state *crtc_state,
> -			  int target, int refclk,
> -			  struct dpll *match_clock,
> -			  struct dpll *best_clock);
>  	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
>  	int (*compute_intermediate_wm)(struct drm_device *dev,
>  				       struct intel_crtc *intel_crtc,
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 552fd4c..4d7c29a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -566,33 +566,6 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
>  	return false;
>  }
>  
> -static const intel_limit_t *
> -intel_limit(struct intel_crtc_state *crtc_state, int refclk)
> -{
> -	struct drm_device *dev = crtc_state->base.crtc->dev;
> -	const intel_limit_t *limit;
> -
> -	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
> -	    HAS_PCH_SPLIT(dev) || IS_G4X(dev) || IS_GEN2(dev))
> -		limit = NULL;
> -
> -	if (IS_PINEVIEW(dev)) {
> -		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
> -			limit = &intel_limits_pineview_lvds;
> -		else
> -			limit = &intel_limits_pineview_sdvo;
> -	} else if (!IS_GEN2(dev)) {
> -		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
> -			limit = &intel_limits_i9xx_lvds;
> -		else
> -			limit = &intel_limits_i9xx_sdvo;
> -	}
> -
> -	WARN_ON(limit == NULL);
> -
> -	return limit;
> -}
> -
>  /*
>   * Platform specific helpers to calculate the port PLL loopback- (clock.m),
>   * and post-divider (clock.p) values, pre- (clock.vco) and post-divided fast
> @@ -723,6 +696,16 @@ i9xx_select_p2_div(const intel_limit_t *limit,
>  	}
>  }
>  
> +/*
> + * Returns a set of divisors for the desired target clock with the given
> + * refclk, or FALSE.  The returned values represent the clock equation:
> + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + *
> + * Target and reference clocks are specified in kHz.
> + *
> + * If match_clock is provided, then best_clock P divider must match the P
> + * divider from @match_clock used for LVDS downclocking.
> + */
>  static bool
>  i9xx_find_best_dpll(const intel_limit_t *limit,
>  		    struct intel_crtc_state *crtc_state,
> @@ -770,6 +753,16 @@ i9xx_find_best_dpll(const intel_limit_t *limit,
>  	return (err != target);
>  }
>  
> +/*
> + * Returns a set of divisors for the desired target clock with the given
> + * refclk, or FALSE.  The returned values represent the clock equation:
> + * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + *
> + * Target and reference clocks are specified in kHz.
> + *
> + * If match_clock is provided, then best_clock P divider must match the P
> + * divider from @match_clock used for LVDS downclocking.
> + */
>  static bool
>  pnv_find_best_dpll(const intel_limit_t *limit,
>  		   struct intel_crtc_state *crtc_state,
> @@ -819,6 +812,11 @@ pnv_find_best_dpll(const intel_limit_t *limit,
>   * Returns a set of divisors for the desired target clock with the given
>   * refclk, or FALSE.  The returned values represent the clock equation:
>   * reflck * (5 * (m1 + 2) + (m2 + 2)) / (n + 2) / p1 / p2.
> + *
> + * Target and reference clocks are specified in kHz.
> + *
> + * If match_clock is provided, then best_clock P divider must match the P
> + * divider from @match_clock used for LVDS downclocking.
>   */
>  static bool
>  g4x_find_best_dpll(const intel_limit_t *limit,
> @@ -7904,43 +7902,67 @@ static int g4x_crtc_compute_clock(struct intel_crtc *crtc,
>  	return 0;
>  }
>  
> +static int pnv_crtc_compute_clock(struct intel_crtc *crtc,
> +				  struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	const intel_limit_t *limit;
> +	int refclk = 96000;
> +
> +	memset(&crtc_state->dpll_hw_state, 0,
> +	       sizeof(crtc_state->dpll_hw_state));
> +
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> +		if (intel_panel_use_ssc(dev_priv)) {
> +			refclk = dev_priv->vbt.lvds_ssc_freq;
> +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +		}
> +
> +		limit = &intel_limits_pineview_lvds;
> +	} else {
> +		limit = &intel_limits_pineview_sdvo;
> +	}
> +
> +	if (!crtc_state->clock_set &&
> +	    !pnv_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
> +	}
> +
> +	i9xx_compute_dpll(crtc, crtc_state, NULL);
> +
> +	return 0;
> +}
> +
>  static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
>  				   struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	bool ok;
>  	const intel_limit_t *limit;
>  	int refclk = 96000;
>  
>  	memset(&crtc_state->dpll_hw_state, 0,
>  	       sizeof(crtc_state->dpll_hw_state));
>  
> -	if (crtc_state->has_dsi_encoder)
> -		return 0;
> +	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
> +		if (intel_panel_use_ssc(dev_priv)) {
> +			refclk = dev_priv->vbt.lvds_ssc_freq;
> +			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +		}
>  
> -	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
> -	    intel_panel_use_ssc(dev_priv)) {
> -		refclk = dev_priv->vbt.lvds_ssc_freq;
> -		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
> +		limit = &intel_limits_i9xx_lvds;
> +	} else {
> +		limit = &intel_limits_i9xx_sdvo;
>  	}
>  
> -	if (!crtc_state->clock_set) {
> -		/*
> -		 * Returns a set of divisors for the desired target clock with
> -		 * the given refclk, or FALSE.  The returned values represent
> -		 * the clock equation: reflck * (5 * (m1 + 2) + (m2 + 2)) / (n +
> -		 * 2) / p1 / p2.
> -		 */
> -		limit = intel_limit(crtc_state, refclk);
> -		ok = dev_priv->display.find_dpll(limit, crtc_state,
> -						 crtc_state->port_clock,
> -						 refclk, NULL,
> -						 &crtc_state->dpll);
> -		if (!ok) {
> -			DRM_ERROR("Couldn't find PLL settings for mode!\n");
> -			return -EINVAL;
> -		}
> +	if (!crtc_state->clock_set &&
> +	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
> +				 refclk, NULL, &crtc_state->dpll)) {
> +		DRM_ERROR("Couldn't find PLL settings for mode!\n");
> +		return -EINVAL;
>  	}
>  
>  	i9xx_compute_dpll(crtc, crtc_state, NULL);
> @@ -14925,11 +14947,6 @@ static const struct drm_mode_config_funcs intel_mode_funcs = {
>   */
>  void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  {
> -	if (IS_PINEVIEW(dev_priv))
> -		dev_priv->display.find_dpll = pnv_find_best_dpll;
> -	else
> -		dev_priv->display.find_dpll = i9xx_find_best_dpll;
> -
>  	if (INTEL_INFO(dev_priv)->gen >= 9) {
>  		dev_priv->display.get_pipe_config = haswell_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> @@ -14975,6 +14992,13 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.crtc_compute_clock = g4x_crtc_compute_clock;
>  		dev_priv->display.crtc_enable = i9xx_crtc_enable;
>  		dev_priv->display.crtc_disable = i9xx_crtc_disable;
> +	} else if (IS_PINEVIEW(dev_priv)) {
> +		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> +		dev_priv->display.get_initial_plane_config =
> +			i9xx_get_initial_plane_config;
> +		dev_priv->display.crtc_compute_clock = pnv_crtc_compute_clock;
> +		dev_priv->display.crtc_enable = i9xx_crtc_enable;
> +		dev_priv->display.crtc_disable = i9xx_crtc_disable;
>  	} else if (!IS_GEN2(dev_priv)) {
>  		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
>  		dev_priv->display.get_initial_plane_config =
> -- 
> 2.4.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
  2016-03-22 12:49   ` Ville Syrjälä
@ 2016-03-22 13:22     ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-03-22 13:22 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 2016-03-22 at 14:49 +0200, Ville Syrjälä wrote:
> On Mon, Mar 21, 2016 at 06:00:11PM +0200, Ander Conselvan de Oliveira wrote:
> > Follow what is done in i8xx_compute_dpll() and i9xx_compute_dpll() and
> > move the lower level details of setting crtc_state->dpll_hw_state into
> > the _compute_dpll() function.
> 
> Missing sob.

Oops,

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>


> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 45 ++++++++++++++++++---------------
> > ---
> >  1 file changed, 22 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 0b6eabf..b6541a0 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -8821,10 +8821,9 @@ static bool ironlake_needs_fb_cb_tune(struct dpll
> > *dpll, int factor)
> >  	return i9xx_dpll_compute_m(dpll) < factor * dpll->n;
> >  }
> >  
> > -static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> > -				      struct intel_crtc_state *crtc_state,
> > -				      u32 *fp,
> > -				      intel_clock_t *reduced_clock, u32
> > *fp2)
> > +static void ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> > +				  struct intel_crtc_state *crtc_state,
> > +				  intel_clock_t *reduced_clock)
> >  {
> >  	struct drm_crtc *crtc = &intel_crtc->base;
> >  	struct drm_device *dev = crtc->dev;
> > @@ -8833,7 +8832,7 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> >  	struct drm_connector *connector;
> >  	struct drm_connector_state *connector_state;
> >  	struct intel_encoder *encoder;
> > -	uint32_t dpll;
> > +	u32 dpll, fp, fp2;
> >  	int factor, i;
> >  	bool is_lvds = false, is_sdvo = false;
> >  
> > @@ -8866,11 +8865,19 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> >  	} else if (crtc_state->sdvo_tv_clock)
> >  		factor = 20;
> >  
> > +	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> > +
> >  	if (ironlake_needs_fb_cb_tune(&crtc_state->dpll, factor))
> > -		*fp |= FP_CB_TUNE;
> > +		fp |= FP_CB_TUNE;
> > +
> > +	if (reduced_clock) {
> > +		fp2 = i9xx_dpll_compute_fp(reduced_clock);
> >  
> > -	if (fp2 && (reduced_clock->m < factor * reduced_clock->n))
> > -		*fp2 |= FP_CB_TUNE;
> > +		if (reduced_clock->m < factor * reduced_clock->n)
> > +			fp2 |= FP_CB_TUNE;
> > +	} else {
> > +		fp2 = fp;
> > +	}
> >  
> >  	dpll = 0;
> >  
> > @@ -8912,14 +8919,17 @@ static uint32_t ironlake_compute_dpll(struct
> > intel_crtc *intel_crtc,
> >  	else
> >  		dpll |= PLL_REF_INPUT_DREFCLK;
> >  
> > -	return dpll | DPLL_VCO_ENABLE;
> > +	dpll |= DPLL_VCO_ENABLE;
> > +
> > +	crtc_state->dpll_hw_state.dpll = dpll;
> > +	crtc_state->dpll_hw_state.fp0 = fp;
> > +	crtc_state->dpll_hw_state.fp1 = fp2;
> >  }
> >  
> >  static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
> >  				       struct intel_crtc_state *crtc_state)
> >  {
> >  	intel_clock_t reduced_clock;
> > -	u32 dpll = 0, fp = 0, fp2 = 0;
> >  	bool has_reduced_clock = false;
> >  	struct intel_shared_dpll *pll;
> >  
> > @@ -8941,19 +8951,8 @@ static int ironlake_crtc_compute_clock(struct
> > intel_crtc *crtc,
> >  		return -EINVAL;
> >  	}
> >  
> > -	fp = i9xx_dpll_compute_fp(&crtc_state->dpll);
> > -	if (has_reduced_clock)
> > -		fp2 = i9xx_dpll_compute_fp(&reduced_clock);
> > -	else
> > -		fp2 = fp;
> > -
> > -	dpll = ironlake_compute_dpll(crtc, crtc_state,
> > -				     &fp, &reduced_clock,
> > -				     has_reduced_clock ? &fp2 : NULL);
> > -
> > -	crtc_state->dpll_hw_state.dpll = dpll;
> > -	crtc_state->dpll_hw_state.fp0 = fp;
> > -	crtc_state->dpll_hw_state.fp1 = fp2;
> > +	ironlake_compute_dpll(crtc, crtc_state,
> > +			      has_reduced_clock ? &reduced_clock : NULL);
> >  
> >  	pll = intel_get_shared_dpll(crtc, crtc_state, NULL);
> >  	if (pll == NULL) {
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v2 13/15] drm/i915: Split i8xx_crtc_compute_clock()
  2016-03-22 11:11     ` Daniel Vetter
@ 2016-03-22 13:35       ` Ander Conselvan de Oliveira
  0 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan de Oliveira @ 2016-03-22 13:35 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Split a GEN2 specific version from i9xx_crtc_compute_clock(). With this
there is no need for i9xx_get_refclk() anymore, and the differences
between platforms become more obvious.

v2: Use i8xx as prefix instead of gen2. (Ville and Daniel)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 91 +++++++++++++++++++++---------------
 1 file changed, 53 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dca5b15..1e08667 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -595,7 +595,7 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 	const intel_limit_t *limit;
 
 	if (IS_BROXTON(dev) || IS_CHERRYVIEW(dev) || IS_VALLEYVIEW(dev) ||
-	    HAS_PCH_SPLIT(dev))
+	    HAS_PCH_SPLIT(dev) || IS_GEN2(dev))
 		limit = NULL;
 
 	if (IS_G4X(dev)) {
@@ -610,13 +610,6 @@ intel_limit(struct intel_crtc_state *crtc_state, int refclk)
 			limit = &intel_limits_i9xx_lvds;
 		else
 			limit = &intel_limits_i9xx_sdvo;
-	} else {
-		if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS))
-			limit = &intel_limits_i8xx_lvds;
-		else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO))
-			limit = &intel_limits_i8xx_dvo;
-		else
-			limit = &intel_limits_i8xx_dac;
 	}
 
 	WARN_ON(limit == NULL);
@@ -7102,27 +7095,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 		&& !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE);
 }
 
-static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state)
-{
-	struct drm_device *dev = crtc_state->base.crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk;
-
-	WARN_ON(!crtc_state->base.state);
-
-	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
-	    intel_panel_use_ssc(dev_priv)) {
-		refclk = dev_priv->vbt.lvds_ssc_freq;
-		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
-	} else if (!IS_GEN2(dev)) {
-		refclk = 96000;
-	} else {
-		refclk = 48000;
-	}
-
-	return refclk;
-}
-
 static uint32_t pnv_dpll_compute_fp(struct dpll *dpll)
 {
 	return (1 << dpll->n) << 16 | dpll->m2;
@@ -7877,14 +7849,50 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 	POSTING_READ(PIPECONF(intel_crtc->pipe));
 }
 
+static int i8xx_crtc_compute_clock(struct intel_crtc *crtc,
+				   struct intel_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	const intel_limit_t *limit;
+	int refclk = 48000;
+
+	memset(&crtc_state->dpll_hw_state, 0,
+	       sizeof(crtc_state->dpll_hw_state));
+
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS)) {
+		if (intel_panel_use_ssc(dev_priv)) {
+			refclk = dev_priv->vbt.lvds_ssc_freq;
+			DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+		}
+
+		limit = &intel_limits_i8xx_lvds;
+	} else if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_DVO)) {
+		limit = &intel_limits_i8xx_dvo;
+	} else {
+		limit = &intel_limits_i8xx_dac;
+	}
+
+	if (!crtc_state->clock_set &&
+	    !i9xx_find_best_dpll(limit, crtc_state, crtc_state->port_clock,
+				 refclk, NULL, &crtc_state->dpll)) {
+		DRM_ERROR("Couldn't find PLL settings for mode!\n");
+		return -EINVAL;
+	}
+
+	i8xx_compute_dpll(crtc, crtc_state, NULL);
+
+	return 0;
+}
+
 static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 				   struct intel_crtc_state *crtc_state)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int refclk;
 	bool ok;
 	const intel_limit_t *limit;
+	int refclk = 96000;
 
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
@@ -7892,9 +7900,13 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	if (crtc_state->has_dsi_encoder)
 		return 0;
 
-	if (!crtc_state->clock_set) {
-		refclk = i9xx_get_refclk(crtc_state);
+	if (intel_pipe_will_have_type(crtc_state, INTEL_OUTPUT_LVDS) &&
+	    intel_panel_use_ssc(dev_priv)) {
+		refclk = dev_priv->vbt.lvds_ssc_freq;
+		DRM_DEBUG_KMS("using SSC reference clock of %d kHz\n", refclk);
+	}
 
+	if (!crtc_state->clock_set) {
 		/*
 		 * Returns a set of divisors for the desired target clock with
 		 * the given refclk, or FALSE.  The returned values represent
@@ -7912,11 +7924,7 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 		}
 	}
 
-	if (IS_GEN2(dev)) {
-		i8xx_compute_dpll(crtc, crtc_state, NULL);
-	} else {
-		i9xx_compute_dpll(crtc, crtc_state, NULL);
-	}
+	i9xx_compute_dpll(crtc, crtc_state, NULL);
 
 	return 0;
 }
@@ -14943,13 +14951,20 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.crtc_compute_clock = vlv_crtc_compute_clock;
 		dev_priv->display.crtc_enable = valleyview_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
-	} else {
+	} else if (!IS_GEN2(dev_priv)) {
 		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
 		dev_priv->display.get_initial_plane_config =
 			i9xx_get_initial_plane_config;
 		dev_priv->display.crtc_compute_clock = i9xx_crtc_compute_clock;
 		dev_priv->display.crtc_enable = i9xx_crtc_enable;
 		dev_priv->display.crtc_disable = i9xx_crtc_disable;
+	} else {
+		dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
+		dev_priv->display.get_initial_plane_config =
+			i9xx_get_initial_plane_config;
+		dev_priv->display.crtc_compute_clock = i8xx_crtc_compute_clock;
+		dev_priv->display.crtc_enable = i9xx_crtc_enable;
+		dev_priv->display.crtc_disable = i9xx_crtc_disable;
 	}
 
 	/* Returns the core display clock speed */
-- 
2.4.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4)
  2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
                   ` (15 preceding siblings ...)
  2016-03-22  9:33 ` ✗ Fi.CI.BAT: warning for Clean up ironlake clock computation code (rev3) Patchwork
@ 2016-03-22 14:32 ` Patchwork
  2016-03-23 12:26   ` Ander Conselvan De Oliveira
  16 siblings, 1 reply; 30+ messages in thread
From: Patchwork @ 2016-03-22 14:32 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Clean up ironlake clock computation code (rev4)
URL   : https://patchwork.freedesktop.org/series/4367/
State : success

== Summary ==

Series 4367v4 Clean up ironlake clock computation code
http://patchwork.freedesktop.org/api/1.0/series/4367/revisions/4/mbox/

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ilk-hp8440p)
Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> FAIL       (ilk-hp8440p) UNSTABLE
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                dmesg-warn -> PASS       (byt-nuc)

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:171  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:192  pass:154  dwarn:1   dfail:0   fail:0   skip:37 
byt-nuc          total:192  pass:157  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:192  pass:170  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:192  pass:127  dwarn:1   dfail:0   fail:1   skip:63 
ivb-t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:192  pass:158  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:192  pass:158  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1675/

94c7a2ce8a4f659840625a318b1d3d8832f4ca46 drm-intel-nightly: 2016y-03m-22d-12h-51m-12s UTC integration manifest
ea35ece59759aa200a53adfb9ed6cda48f1750e3 drm/i915: Split PNV version of crtc_compute_clock()
ede2843aabc6eec8e77f11bf3b21a543e8af5388 drm/i915: Split g4x_crtc_compute_clock()
a2cfa5150683dd0e6560bab55aed9d1eccc1f952 drm/i915: Split i8xx_crtc_compute_clock()
a0830177a2490f7dbb7816c53a2fb533a3e07d8a drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks
70fbc459af4e0425d36030496f7e245a2cca1d93 drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock()
a125eb27a18cb4f8995e6d6e4f0e3a4d8ede8363 drm/i915: Move fp divisor calculation into ironlake_compute_dpll()
e362025b9c2151c7378aaf74917c47b56dde3877 drm/i915: Pass crtc_state->dpll directly to ->find_dpll()
e453ab21b391ea55070a6524d3a8259fbeca0998 drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case
0d9416317f7171a10ffc60f84d9cbc4c77a86e7f drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock()
3cf3d010e9a301d57880cbef2087dbe35bb2f198 drm/i915: Don't calculate a new clock in ILK+ code if it is already set
679167339e291b18fcd965cf45a3a6ff10dcc900 drm/i915: Simplify ironlake reduced clock logic a bit
bae0742ed14038f2d8881c05496832ce839ae197 drm/i915: Call g4x_find_best_dpll() directly from ILK+ code
4660c0dbc8111d8202fb32b42c6815f0482935d8 drm/i915: Fold intel_ironlake_limit() into clock computation function
8c843b6a2e9bc4f2b4b4d6352d2ed504ab414346 drm/i915: Merge ironlake_get_refclk() into its only caller
e9cb4da98edb60650f68cd0e0954f0f1e450ae70 drm/i915: Remove checks for cloned config with LVDS in dpll code

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4)
  2016-03-22 14:32 ` ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4) Patchwork
@ 2016-03-23 12:26   ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 30+ messages in thread
From: Ander Conselvan De Oliveira @ 2016-03-23 12:26 UTC (permalink / raw)
  To: intel-gfx

On Tue, 2016-03-22 at 14:32 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: Clean up ironlake clock computation code (rev4)
> URL   : https://patchwork.freedesktop.org/series/4367/
> State : success

Series pushed. Thanks for reviewing.

Ander

> 
> == Summary ==
> 
> Series 4367v4 Clean up ironlake clock computation code
> http://patchwork.freedesktop.org/api/1.0/series/4367/revisions/4/mbox/
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 fail       -> PASS       (ilk-hp8440p)
> Test gem_exec_suspend:
>         Subgroup basic-s3:
>                 dmesg-warn -> PASS       (bsw-nuc-2)
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
>         Subgroup basic-flip-vs-wf_vblank:
>                 pass       -> FAIL       (ilk-hp8440p) UNSTABLE
> Test pm_rpm:
>         Subgroup basic-pci-d3-state:
>                 dmesg-warn -> PASS       (byt-nuc)
> 
> bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
> bdw-ultra        total:192  pass:171  dwarn:0   dfail:0   fail:0   skip:21 
> bsw-nuc-2        total:192  pass:154  dwarn:1   dfail:0   fail:0   skip:37 
> byt-nuc          total:192  pass:157  dwarn:0   dfail:0   fail:0   skip:35 
> hsw-brixbox      total:192  pass:170  dwarn:0   dfail:0   fail:0   skip:22 
> hsw-gt2          total:192  pass:175  dwarn:0   dfail:0   fail:0   skip:17 
> ilk-hp8440p      total:192  pass:127  dwarn:1   dfail:0   fail:1   skip:63 
> ivb-t430s        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
> skl-i5k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
> skl-i7k-2        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
> skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
> snb-dellxps      total:192  pass:158  dwarn:0   dfail:0   fail:0   skip:34 
> snb-x220t        total:192  pass:158  dwarn:0   dfail:0   fail:1   skip:33 
> 
> Results at /archive/results/CI_IGT_test/Patchwork_1675/
> 
> 94c7a2ce8a4f659840625a318b1d3d8832f4ca46 drm-intel-nightly: 2016y-03m-22d-12h
> -51m-12s UTC integration manifest
> ea35ece59759aa200a53adfb9ed6cda48f1750e3 drm/i915: Split PNV version of
> crtc_compute_clock()
> ede2843aabc6eec8e77f11bf3b21a543e8af5388 drm/i915: Split
> g4x_crtc_compute_clock()
> a2cfa5150683dd0e6560bab55aed9d1eccc1f952 drm/i915: Split
> i8xx_crtc_compute_clock()
> a0830177a2490f7dbb7816c53a2fb533a3e07d8a drm/i915: Split CHV and VLV specific
> crtc_compute_clock() hooks
> 70fbc459af4e0425d36030496f7e245a2cca1d93 drm/i915: Merge
> ironlake_compute_clocks() and ironlake_crtc_compute_clock()
> a125eb27a18cb4f8995e6d6e4f0e3a4d8ede8363 drm/i915: Move fp divisor calculation
> into ironlake_compute_dpll()
> e362025b9c2151c7378aaf74917c47b56dde3877 drm/i915: Pass crtc_state->dpll
> directly to ->find_dpll()
> e453ab21b391ea55070a6524d3a8259fbeca0998 drm/i915: Simplify
> ironlake_crtc_compute_clock() CPU eDP case
> 0d9416317f7171a10ffc60f84d9cbc4c77a86e7f drm/i915: Remove PCH type checks from
> ironlake_crtc_compute_clock()
> 3cf3d010e9a301d57880cbef2087dbe35bb2f198 drm/i915: Don't calculate a new clock
> in ILK+ code if it is already set
> 679167339e291b18fcd965cf45a3a6ff10dcc900 drm/i915: Simplify ironlake reduced
> clock logic a bit
> bae0742ed14038f2d8881c05496832ce839ae197 drm/i915: Call g4x_find_best_dpll()
> directly from ILK+ code
> 4660c0dbc8111d8202fb32b42c6815f0482935d8 drm/i915: Fold intel_ironlake_limit()
> into clock computation function
> 8c843b6a2e9bc4f2b4b4d6352d2ed504ab414346 drm/i915: Merge ironlake_get_refclk()
> into its only caller
> e9cb4da98edb60650f68cd0e0954f0f1e450ae70 drm/i915: Remove checks for cloned
> config with LVDS in dpll code
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2016-03-23 12:26 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-21 16:00 [PATCH v3 00/15] Clean up ironlake clock computation code Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 01/15] drm/i915: Remove checks for cloned config with LVDS in dpll code Ander Conselvan de Oliveira
2016-03-22 10:11   ` Ville Syrjälä
2016-03-21 16:00 ` [PATCH 02/15] drm/i915: Merge ironlake_get_refclk() into its only caller Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 03/15] drm/i915: Fold intel_ironlake_limit() into clock computation function Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 04/15] drm/i915: Call g4x_find_best_dpll() directly from ILK+ code Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 05/15] drm/i915: Simplify ironlake reduced clock logic a bit Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 06/15] drm/i915: Don't calculate a new clock in ILK+ code if it is already set Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 07/15] drm/i915: Remove PCH type checks from ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 08/15] drm/i915: Simplify ironlake_crtc_compute_clock() CPU eDP case Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 09/15] drm/i915: Pass crtc_state->dpll directly to ->find_dpll() Ander Conselvan de Oliveira
2016-03-22 10:18   ` Ville Syrjälä
2016-03-21 16:00 ` [PATCH 10/15] drm/i915: Move fp divisor calculation into ironlake_compute_dpll() Ander Conselvan de Oliveira
2016-03-22 12:49   ` Ville Syrjälä
2016-03-22 13:22     ` Ander Conselvan De Oliveira
2016-03-21 16:00 ` [PATCH 11/15] drm/i915: Merge ironlake_compute_clocks() and ironlake_crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-22 12:51   ` Ville Syrjälä
2016-03-21 16:00 ` [PATCH 12/15] drm/i915: Split CHV and VLV specific crtc_compute_clock() hooks Ander Conselvan de Oliveira
2016-03-22 10:26   ` Ville Syrjälä
2016-03-21 16:00 ` [PATCH 13/15] drm/i915: Split gen2_crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-22 10:24   ` Ville Syrjälä
2016-03-22 11:11     ` Daniel Vetter
2016-03-22 13:35       ` [PATCH v2 13/15] drm/i915: Split i8xx_crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-21 16:00 ` [PATCH 14/15] drm/i915: Split g4x_crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-22 12:52   ` Ville Syrjälä
2016-03-21 16:00 ` [PATCH 15/15] drm/i915: Split PNV version of crtc_compute_clock() Ander Conselvan de Oliveira
2016-03-22 12:55   ` Ville Syrjälä
2016-03-22  9:33 ` ✗ Fi.CI.BAT: warning for Clean up ironlake clock computation code (rev3) Patchwork
2016-03-22 14:32 ` ✓ Fi.CI.BAT: success for Clean up ironlake clock computation code (rev4) Patchwork
2016-03-23 12:26   ` Ander Conselvan De Oliveira

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.