All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
@ 2017-07-07 10:24 ville.syrjala
  2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Follow the GLK path when computing cdclk and related limits. CNL
pipes also produce two pixels per clock, so that's what we should
use.

For the HBR2 vs. audio issue the limit should more correctly be 336
MHz, but the GLK limit of 316.8 MHz works just as well and results
in picking at least 336 MHz. Also toss in some related w/a numbers.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 1241e5891b29..9e0deebae279 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	    crtc_state->has_audio &&
 	    crtc_state->port_clock >= 540000 &&
 	    crtc_state->lane_count == 4) {
-		if (IS_CANNONLAKE(dev_priv))
-			pixel_rate = max(316800, pixel_rate);
-		else if (IS_GEMINILAKE(dev_priv))
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+			/* Display WA #1145: glk,cnl */
 			pixel_rate = max(2 * 316800, pixel_rate);
-		else
+		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
+			/* Display WA #1144: skl,bxt */
 			pixel_rate = max(432000, pixel_rate);
+		}
 	}
 
 	/* According to BSpec, "The CD clock frequency must be at least twice
@@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	 * two pixels per clock.
 	 */
 	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
-		if (IS_GEMINILAKE(dev_priv))
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			pixel_rate = max(2 * 2 * 96000, pixel_rate);
 		else
 			pixel_rate = max(2 * 96000, pixel_rate);
@@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 {
 	int max_cdclk_freq = dev_priv->max_cdclk_freq;
 
-	if (IS_GEMINILAKE(dev_priv))
+	if (IS_CANNONLAKE(dev_priv))
+		return 2 * max_cdclk_freq;
+	else if (IS_GEMINILAKE(dev_priv))
 		/*
 		 * FIXME: Limiting to 99% as a temporary workaround. See
 		 * glk_calc_cdclk() for details.
-- 
2.13.0

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

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

* [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
@ 2017-07-07 10:24 ` ville.syrjala
  2017-07-07 18:28   ` Ville Syrjälä
  2017-07-10 13:02   ` [PATCH v2 " ville.syrjala
  2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.

The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  12 ++-
 drivers/gpu/drm/i915/intel_cdclk.c   | 179 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c |  21 ++--
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 4 files changed, 107 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..c80176424d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,15 @@ struct i915_hotplug {
 	     (__i)++) \
 		for_each_if (plane_state)
 
+#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if (crtc)
+
+
 struct drm_i915_private;
 struct i915_mm_struct;
 struct i915_mmu_object;
@@ -2268,7 +2277,8 @@ struct drm_i915_private {
 	struct mutex dpll_lock;
 
 	unsigned int active_crtcs;
-	unsigned int min_pixclk[I915_MAX_PIPES];
+	/* minimum acceptable cdclk for each pipe */
+	int min_cdclk[I915_MAX_PIPES];
 
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9e0deebae279..82e5bc967cca 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -417,24 +417,21 @@ static void hsw_get_cdclk(struct drm_i915_private *dev_priv,
 		cdclk_state->cdclk = 540000;
 }
 
-static int vlv_calc_cdclk(struct drm_i915_private *dev_priv,
-			  int max_pixclk)
+static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk)
 {
 	int freq_320 = (dev_priv->hpll_freq <<  1) % 320000 != 0 ?
 		333333 : 320000;
-	int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
 
 	/*
 	 * We seem to get an unstable or solid color picture at 200MHz.
 	 * Not sure what's wrong. For now use 200MHz only when all pipes
 	 * are off.
 	 */
-	if (!IS_CHERRYVIEW(dev_priv) &&
-	    max_pixclk > freq_320*limit/100)
+	if (IS_VALLEYVIEW(dev_priv) && min_cdclk > freq_320)
 		return 400000;
-	else if (max_pixclk > 266667*limit/100)
+	else if (min_cdclk > 266667)
 		return freq_320;
-	else if (max_pixclk > 0)
+	else if (min_cdclk > 0)
 		return 266667;
 	else
 		return 200000;
@@ -612,13 +609,13 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
 }
 
-static int bdw_calc_cdclk(int max_pixclk)
+static int bdw_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 540000)
+	if (min_cdclk > 540000)
 		return 675000;
-	else if (max_pixclk > 450000)
+	else if (min_cdclk > 450000)
 		return 540000;
-	else if (max_pixclk > 337500)
+	else if (min_cdclk > 337500)
 		return 450000;
 	else
 		return 337500;
@@ -724,23 +721,23 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
 	     cdclk, dev_priv->cdclk.hw.cdclk);
 }
 
-static int skl_calc_cdclk(int max_pixclk, int vco)
+static int skl_calc_cdclk(int min_cdclk, int vco)
 {
 	if (vco == 8640000) {
-		if (max_pixclk > 540000)
+		if (min_cdclk > 540000)
 			return 617143;
-		else if (max_pixclk > 432000)
+		else if (min_cdclk > 432000)
 			return 540000;
-		else if (max_pixclk > 308571)
+		else if (min_cdclk > 308571)
 			return 432000;
 		else
 			return 308571;
 	} else {
-		if (max_pixclk > 540000)
+		if (min_cdclk > 540000)
 			return 675000;
-		else if (max_pixclk > 450000)
+		else if (min_cdclk > 450000)
 			return 540000;
-		else if (max_pixclk > 337500)
+		else if (min_cdclk > 337500)
 			return 450000;
 		else
 			return 337500;
@@ -1075,31 +1072,25 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	skl_set_cdclk(dev_priv, &cdclk_state);
 }
 
-static int bxt_calc_cdclk(int max_pixclk)
+static int bxt_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 576000)
+	if (min_cdclk > 576000)
 		return 624000;
-	else if (max_pixclk > 384000)
+	else if (min_cdclk > 384000)
 		return 576000;
-	else if (max_pixclk > 288000)
+	else if (min_cdclk > 288000)
 		return 384000;
-	else if (max_pixclk > 144000)
+	else if (min_cdclk > 144000)
 		return 288000;
 	else
 		return 144000;
 }
 
-static int glk_calc_cdclk(int max_pixclk)
+static int glk_calc_cdclk(int min_cdclk)
 {
-	/*
-	 * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
-	 * as a temporary workaround. Use a higher cdclk instead. (Note that
-	 * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
-	 * cdclk.)
-	 */
-	if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
+	if (min_cdclk > 158400)
 		return 316800;
-	else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
+	else if (min_cdclk > 79200)
 		return 158400;
 	else
 		return 79200;
@@ -1420,11 +1411,11 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
 	bxt_set_cdclk(dev_priv, &cdclk_state);
 }
 
-static int cnl_calc_cdclk(int max_pixclk)
+static int cnl_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 336000)
+	if (min_cdclk > 336000)
 		return 528000;
-	else if (max_pixclk > 168000)
+	else if (min_cdclk > 168000)
 		return 336000;
 	else
 		return 168000;
@@ -1732,21 +1723,47 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
 }
 
-static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
-					  int pixel_rate)
+static int intel_min_cdclk(struct drm_i915_private *dev_priv,
+			   int pixel_rate)
+{
+	if (INTEL_GEN(dev_priv) >= 10)
+		return DIV_ROUND_UP(pixel_rate, 2);
+	else if (IS_GEMINILAKE(dev_priv))
+		/*
+		 * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
+		 * as a temporary workaround. Use a higher cdclk instead. (Note that
+		 * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
+		 * cdclk.)
+		 */
+		return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
+	else if (IS_GEN9(dev_priv) ||
+		 IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		return pixel_rate;
+	else if (IS_CHERRYVIEW(dev_priv))
+		return DIV_ROUND_UP(pixel_rate * 100, 95);
+	else
+		return DIV_ROUND_UP(pixel_rate * 100, 90);
+}
+
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->base.crtc->dev);
+	int min_cdclk;
+
+	if (!crtc_state->base.enable)
+		return 0;
+
+	min_cdclk = intel_min_cdclk(dev_priv, crtc_state->pixel_rate);
 
 	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
 	if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+		min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95);
 
 	/* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz,
 	 * audio enabled, port width x4, and link rate HBR2 (5.4 GHz), or else
 	 * there may be audio corruption or screen corruption." This cdclk
-	 * restriction for GLK is 316.8 MHz and since GLK can output two
-	 * pixels per clock, the pixel rate becomes 2 * 316.8 MHz.
+	 * restriction for GLK is 316.8 MHz.
 	 */
 	if (intel_crtc_has_dp_encoder(crtc_state) &&
 	    crtc_state->has_audio &&
@@ -1754,77 +1771,53 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	    crtc_state->lane_count == 4) {
 		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			/* Display WA #1145: glk,cnl */
-			pixel_rate = max(2 * 316800, pixel_rate);
+			min_cdclk = max(316800, min_cdclk);
 		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
 			/* Display WA #1144: skl,bxt */
-			pixel_rate = max(432000, pixel_rate);
+			min_cdclk = max(432000, min_cdclk);
 		}
 	}
 
 	/* According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
-	 * The check for GLK has to be adjusted as the platform can output
-	 * two pixels per clock.
 	 */
-	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
-		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
-			pixel_rate = max(2 * 2 * 96000, pixel_rate);
-		else
-			pixel_rate = max(2 * 96000, pixel_rate);
-	}
+	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+		min_cdclk = max(2 * 96000, min_cdclk);
 
-	return pixel_rate;
+	return min_cdclk;
 }
 
-/* compute the max rate for new configuration */
-static int intel_max_pixel_rate(struct drm_atomic_state *state)
+static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *cstate;
+	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
-	unsigned int max_pixel_rate = 0, i;
+	int min_cdclk = 0, i;
 	enum pipe pipe;
 
-	memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
-	       sizeof(intel_state->min_pixclk));
-
-	for_each_new_crtc_in_state(state, crtc, cstate, i) {
-		int pixel_rate;
+	memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
+	       sizeof(intel_state->min_cdclk));
 
-		crtc_state = to_intel_crtc_state(cstate);
-		if (!crtc_state->base.enable) {
-			intel_state->min_pixclk[i] = 0;
-			continue;
-		}
-
-		pixel_rate = crtc_state->pixel_rate;
-
-		if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
-			pixel_rate =
-				bdw_adjust_min_pipe_pixel_rate(crtc_state,
-							       pixel_rate);
-
-		intel_state->min_pixclk[i] = pixel_rate;
-	}
+	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
+		intel_state->min_cdclk[i] =
+			intel_crtc_compute_min_cdclk(crtc_state);
 
 	for_each_pipe(dev_priv, pipe)
-		max_pixel_rate = max(intel_state->min_pixclk[pipe],
-				     max_pixel_rate);
+		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
-	return max_pixel_rate;
+	return min_cdclk;
 }
 
 static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 	int cdclk;
 
-	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
+	cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1850,14 +1843,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk;
 
 	/*
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	cdclk = bdw_calc_cdclk(max_pixclk);
+	cdclk = bdw_calc_cdclk(min_cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1883,7 +1876,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	const int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk, vco;
 
 	vco = intel_state->cdclk.logical.vco;
@@ -1894,7 +1887,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	cdclk = skl_calc_cdclk(max_pixclk, vco);
+	cdclk = skl_calc_cdclk(min_cdclk, vco);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1921,16 +1914,16 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 	int cdclk, vco;
 
 	if (IS_GEMINILAKE(dev_priv)) {
-		cdclk = glk_calc_cdclk(max_pixclk);
+		cdclk = glk_calc_cdclk(min_cdclk);
 		vco = glk_de_pll_vco(dev_priv, cdclk);
 	} else {
-		cdclk = bxt_calc_cdclk(max_pixclk);
+		cdclk = bxt_calc_cdclk(min_cdclk);
 		vco = bxt_de_pll_vco(dev_priv, cdclk);
 	}
 
@@ -1967,10 +1960,10 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk, vco;
 
-	cdclk = cnl_calc_cdclk(max_pixclk);
+	cdclk = cnl_calc_cdclk(min_cdclk);
 	vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
@@ -2005,11 +1998,11 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 	else if (IS_GEMINILAKE(dev_priv))
 		/*
 		 * FIXME: Limiting to 99% as a temporary workaround. See
-		 * glk_calc_cdclk() for details.
+		 * intel_min_cdclk() for details.
 		 */
 		return 2 * max_cdclk_freq * 99 / 100;
-	else if (INTEL_INFO(dev_priv)->gen >= 9 ||
-		 IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	else if (IS_GEN9(dev_priv) ||
+		 IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
 		return max_cdclk_freq;
 	else if (IS_CHERRYVIEW(dev_priv))
 		return max_cdclk_freq*95/100;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2144adc5b1d5..b47535f5d95d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5920,7 +5920,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	intel_crtc->enabled_power_domains = 0;
 
 	dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
-	dev_priv->min_pixclk[intel_crtc->pipe] = 0;
+	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
 }
 
 /*
@@ -13292,8 +13292,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	intel_atomic_track_fbs(state);
 
 	if (intel_state->modeset) {
-		memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
-		       sizeof(intel_state->min_pixclk));
+		memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
+		       sizeof(intel_state->min_cdclk));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
@@ -15569,7 +15569,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	for_each_intel_crtc(dev, crtc) {
 		struct intel_crtc_state *crtc_state =
 			to_intel_crtc_state(crtc->base.state);
-		int pixclk = 0;
+		int min_cdclk = 0;
 
 		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
 		if (crtc_state->base.active) {
@@ -15590,22 +15590,15 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 			intel_crtc_compute_pixel_rate(crtc_state);
 
-			if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv) ||
-			    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-				pixclk = crtc_state->pixel_rate;
-			else
-				WARN_ON(dev_priv->display.modeset_calc_cdclk);
-
-			/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-			if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-				pixclk = DIV_ROUND_UP(pixclk * 100, 95);
+			if (dev_priv->display.modeset_calc_cdclk)
+				min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
 
 			drm_calc_timestamping_constants(&crtc->base,
 							&crtc_state->base.adjusted_mode);
 			update_scanline_offset(crtc);
 		}
 
-		dev_priv->min_pixclk[crtc->pipe] = pixclk;
+		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
 
 		intel_pipe_config_sanity_check(dev_priv, crtc_state);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..8cc1b86b799a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,7 +383,8 @@ struct intel_atomic_state {
 	unsigned int active_pipe_changes;
 
 	unsigned int active_crtcs;
-	unsigned int min_pixclk[I915_MAX_PIPES];
+	/* minimum acceptable cdclk for each pipe */
+	int min_cdclk[I915_MAX_PIPES];
 
 	struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
 
@@ -1308,6 +1309,7 @@ void intel_audio_init(struct drm_i915_private *dev_priv);
 void intel_audio_deinit(struct drm_i915_private *dev_priv);
 
 /* intel_cdclk.c */
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
 void skl_init_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
 void cnl_init_cdclk(struct drm_i915_private *dev_priv);
-- 
2.13.0

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

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

* [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
  2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
@ 2017-07-07 10:24 ` ville.syrjala
  2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-07 10:24 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently the .modeset_calc_cdclk() hooks check the final cdclk value
against the max allowed. That's not really sufficient since the low
level calc_cdclk() functions effectively clamp the minimum required
cdclk to the max supported by the platform. Hence if the minimum
required exceeds the platforms capabilities we'd keep going anyway
using the max cdclk frequency.

To fix that let's move the check earlier into
intel_crtc_compute_min_cdclk() and we'll check the minimum required
cdclk of the pipe against the maximum supported by the platform.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c   | 96 +++++++++++++++++-------------------
 drivers/gpu/drm/i915/intel_display.c |  5 +-
 2 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 82e5bc967cca..d7a77123a7e5 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1784,6 +1784,12 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
+	if (min_cdclk > dev_priv->max_cdclk_freq) {
+		DRM_DEBUG_KMS("required cdclk (%d kHz) exceeds max (%d kHz)\n",
+			      min_cdclk, dev_priv->max_cdclk_freq);
+		return -EINVAL;
+	}
+
 	return min_cdclk;
 }
 
@@ -1793,16 +1799,21 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
-	int min_cdclk = 0, i;
+	int min_cdclk, i;
 	enum pipe pipe;
 
 	memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
 	       sizeof(intel_state->min_cdclk));
 
-	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
-		intel_state->min_cdclk[i] =
-			intel_crtc_compute_min_cdclk(crtc_state);
+	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i) {
+		min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+		if (min_cdclk < 0)
+			return min_cdclk;
+
+		intel_state->min_cdclk[i] = min_cdclk;
+	}
 
+	min_cdclk = 0;
 	for_each_pipe(dev_priv, pipe)
 		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
@@ -1812,18 +1823,14 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int min_cdclk = intel_compute_min_cdclk(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	int min_cdclk, cdclk;
 
-	cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
+	cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
 
 	intel_state->cdclk.logical.cdclk = cdclk;
 
@@ -1841,10 +1848,12 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	int min_cdclk = intel_compute_min_cdclk(state);
-	int cdclk;
+	int min_cdclk, cdclk;
+
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
 
 	/*
 	 * FIXME should also account for plane ratio
@@ -1852,12 +1861,6 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 	 */
 	cdclk = bdw_calc_cdclk(min_cdclk);
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
 	intel_state->cdclk.logical.cdclk = cdclk;
 
 	if (!intel_state->active_crtcs) {
@@ -1874,10 +1877,13 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
-	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int min_cdclk = intel_compute_min_cdclk(state);
-	int cdclk, vco;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	int min_cdclk, cdclk, vco;
+
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
 
 	vco = intel_state->cdclk.logical.vco;
 	if (!vco)
@@ -1889,12 +1895,6 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	 */
 	cdclk = skl_calc_cdclk(min_cdclk, vco);
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
 	intel_state->cdclk.logical.vco = vco;
 	intel_state->cdclk.logical.cdclk = cdclk;
 
@@ -1914,10 +1914,12 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int min_cdclk = intel_compute_min_cdclk(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk, vco;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	int min_cdclk, cdclk, vco;
+
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
 
 	if (IS_GEMINILAKE(dev_priv)) {
 		cdclk = glk_calc_cdclk(min_cdclk);
@@ -1927,12 +1929,6 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 		vco = bxt_de_pll_vco(dev_priv, cdclk);
 	}
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
 	intel_state->cdclk.logical.vco = vco;
 	intel_state->cdclk.logical.cdclk = cdclk;
 
@@ -1958,20 +1954,16 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int min_cdclk = intel_compute_min_cdclk(state);
-	int cdclk, vco;
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	int min_cdclk, cdclk, vco;
+
+	min_cdclk = intel_compute_min_cdclk(state);
+	if (min_cdclk < 0)
+		return min_cdclk;
 
 	cdclk = cnl_calc_cdclk(min_cdclk);
 	vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
 	intel_state->cdclk.logical.vco = vco;
 	intel_state->cdclk.logical.cdclk = cdclk;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b47535f5d95d..1caf0ef82e36 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15590,8 +15590,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 			intel_crtc_compute_pixel_rate(crtc_state);
 
-			if (dev_priv->display.modeset_calc_cdclk)
+			if (dev_priv->display.modeset_calc_cdclk) {
 				min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
+				if (WARN_ON(min_cdclk < 0))
+					min_cdclk = 0;
+			}
 
 			drm_calc_timestamping_constants(&crtc->base,
 							&crtc_state->base.adjusted_mode);
-- 
2.13.0

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

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

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
  2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
  2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
@ 2017-07-07 11:13 ` Patchwork
  2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-07-07 11:13 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915: Fix up CNL cdclk related limits
URL   : https://patchwork.freedesktop.org/series/26988/
State : success

== Summary ==

Series 26988v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/26988/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-byt-n2820) fdo#101705

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:442s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:427s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:350s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:531s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:513s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:593s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:433s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:419s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:484s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:470s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:460s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:572s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:584s
fi-pnv-d510      total:279  pass:223  dwarn:1   dfail:0   fail:0   skip:55  time:561s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:460s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:585s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:467s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:481s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:488s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:540s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:407s

c07f01228c2240ec9604cb9fa4647ccfe575b8a6 drm-tip: 2017y-07m-07d-10h-10m-58s UTC integration manifest
565b523 drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
78592a4b drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
11d4ee5 drm/i915: Fix up CNL cdclk related limits

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5142/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
                   ` (2 preceding siblings ...)
  2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
@ 2017-07-07 17:54 ` Rodrigo Vivi
  2017-07-07 18:24   ` Ville Syrjälä
  2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
  2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork
  5 siblings, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2017-07-07 17:54 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni

I will review the series more carefully later,
but my concern is that with this series applied I got a blank screen on eDP...


On Fri, Jul 7, 2017 at 3:24 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> use.
>
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..9e0deebae279 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>             crtc_state->has_audio &&
>             crtc_state->port_clock >= 540000 &&
>             crtc_state->lane_count == 4) {
> -               if (IS_CANNONLAKE(dev_priv))
> -                       pixel_rate = max(316800, pixel_rate);
> -               else if (IS_GEMINILAKE(dev_priv))
> +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +                       /* Display WA #1145: glk,cnl */
>                         pixel_rate = max(2 * 316800, pixel_rate);
> -               else
> +               } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> +                       /* Display WA #1144: skl,bxt */
>                         pixel_rate = max(432000, pixel_rate);
> +               }
>         }
>
>         /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>          * two pixels per clock.
>          */
>         if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> -               if (IS_GEMINILAKE(dev_priv))
> +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>                         pixel_rate = max(2 * 2 * 96000, pixel_rate);
>                 else
>                         pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  {
>         int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> -       if (IS_GEMINILAKE(dev_priv))
> +       if (IS_CANNONLAKE(dev_priv))
> +               return 2 * max_cdclk_freq;
> +       else if (IS_GEMINILAKE(dev_priv))
>                 /*
>                  * FIXME: Limiting to 99% as a temporary workaround. See
>                  * glk_calc_cdclk() for details.
> --
> 2.13.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
@ 2017-07-07 18:24   ` Ville Syrjälä
  2017-07-07 18:33     ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-07 18:24 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni

On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...

Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.

> 
> 
> On Fri, Jul 7, 2017 at 3:24 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >             crtc_state->has_audio &&
> >             crtc_state->port_clock >= 540000 &&
> >             crtc_state->lane_count == 4) {
> > -               if (IS_CANNONLAKE(dev_priv))
> > -                       pixel_rate = max(316800, pixel_rate);
> > -               else if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +                       /* Display WA #1145: glk,cnl */
> >                         pixel_rate = max(2 * 316800, pixel_rate);
> > -               else
> > +               } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +                       /* Display WA #1144: skl,bxt */
> >                         pixel_rate = max(432000, pixel_rate);
> > +               }
> >         }
> >
> >         /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >          * two pixels per clock.
> >          */
> >         if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -               if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >                         pixel_rate = max(2 * 2 * 96000, pixel_rate);
> >                 else
> >                         pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> >  {
> >         int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > -       if (IS_GEMINILAKE(dev_priv))
> > +       if (IS_CANNONLAKE(dev_priv))
> > +               return 2 * max_cdclk_freq;
> > +       else if (IS_GEMINILAKE(dev_priv))
> >                 /*
> >                  * FIXME: Limiting to 99% as a temporary workaround. See
> >                  * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
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] 16+ messages in thread

* Re: [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
  2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
@ 2017-07-07 18:28   ` Ville Syrjälä
  2017-07-10 13:02   ` [PATCH v2 " ville.syrjala
  1 sibling, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-07 18:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

On Fri, Jul 07, 2017 at 01:24:49PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make the min_pixclk thing less confusing by changing it to track
> the minimum acceptable cdclk frequency instead. This means moving
> the application of the guardbands to a slightly higher level from
> the low level platform specific calc_cdclk() functions.
> 
> The immediate benefit is elimination of the confusing 2x factors
> on GLK/CNL+ in the audio workarounds (which stems from the fact
> that the pipes produce two pixels per clock).
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  12 ++-
>  drivers/gpu/drm/i915/intel_cdclk.c   | 179 +++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_display.c |  21 ++--
>  drivers/gpu/drm/i915/intel_drv.h     |   4 +-
>  4 files changed, 107 insertions(+), 109 deletions(-)
>
<snip>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 9e0deebae279..82e5bc967cca 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1732,21 +1723,47 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>  	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
>  }
>  
> -static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> -					  int pixel_rate)
> +static int intel_min_cdclk(struct drm_i915_private *dev_priv,
> +			   int pixel_rate)
> +{
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		return DIV_ROUND_UP(pixel_rate, 2);

Rodrigo, so this part here could be why your CNL no longer works.

If you have time to try it, then I think changing this to just
'return pixel_rate;' should get us back to the old behaviour.

> +	else if (IS_GEMINILAKE(dev_priv))
> +		/*
> +		 * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
> +		 * as a temporary workaround. Use a higher cdclk instead. (Note that
> +		 * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
> +		 * cdclk.)
> +		 */
> +		return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
> +	else if (IS_GEN9(dev_priv) ||
> +		 IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> +		return pixel_rate;
> +	else if (IS_CHERRYVIEW(dev_priv))
> +		return DIV_ROUND_UP(pixel_rate * 100, 95);
> +	else
> +		return DIV_ROUND_UP(pixel_rate * 100, 90);
> +}
> +

-- 
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] 16+ messages in thread

* Re: [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-07 18:24   ` Ville Syrjälä
@ 2017-07-07 18:33     ` Pandiyan, Dhinakaran
  0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-07 18:33 UTC (permalink / raw)
  To: Ville Syrjälä, Rodrigo Vivi
  Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo

iirc I assumed 1 pixel per clk for CNL when I originally submitted the workaround patch because cnl_xxx_calc_cdclk() functions assume that. Are we missing something in the cdclk setup code to enable 2 pixels per clk?

-DK
________________________________________
From: Ville Syrjälä [ville.syrjala@linux.intel.com]
Sent: Friday, July 07, 2017 11:24 AM
To: Rodrigo Vivi
Cc: intel-gfx; Pandiyan, Dhinakaran; Zanoni, Paulo R; Vivi, Rodrigo
Subject: Re: [Intel-gfx] [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits

On Fri, Jul 07, 2017 at 10:54:47AM -0700, Rodrigo Vivi wrote:
> I will review the series more carefully later,
> but my concern is that with this series applied I got a blank screen on eDP...

Hmm. Oh, I guess we could now be going for a lower cdclk that before
since we now account for 2 pixels per clock factor, whereas previously
didn't. That in itself should be fine of course, but I guess the
difference could be down to the system agent DVFS, which we still don't
handle it seems. So possibly we need to get that sorted before we can
change how CNL picks its cdclk.

>
>
> On Fri, Jul 7, 2017 at 3:24 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > use.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..9e0deebae279 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >             crtc_state->has_audio &&
> >             crtc_state->port_clock >= 540000 &&
> >             crtc_state->lane_count == 4) {
> > -               if (IS_CANNONLAKE(dev_priv))
> > -                       pixel_rate = max(316800, pixel_rate);
> > -               else if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +                       /* Display WA #1145: glk,cnl */
> >                         pixel_rate = max(2 * 316800, pixel_rate);
> > -               else
> > +               } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +                       /* Display WA #1144: skl,bxt */
> >                         pixel_rate = max(432000, pixel_rate);
> > +               }
> >         }
> >
> >         /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >          * two pixels per clock.
> >          */
> >         if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -               if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >                         pixel_rate = max(2 * 2 * 96000, pixel_rate);
> >                 else
> >                         pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,9 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> >  {
> >         int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > -       if (IS_GEMINILAKE(dev_priv))
> > +       if (IS_CANNONLAKE(dev_priv))
> > +               return 2 * max_cdclk_freq;
> > +       else if (IS_GEMINILAKE(dev_priv))
> >                 /*
> >                  * FIXME: Limiting to 99% as a temporary workaround. See
> >                  * glk_calc_cdclk() for details.
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

--
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] 16+ messages in thread

* [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
                   ` (3 preceding siblings ...)
  2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
@ 2017-07-10 13:02 ` ville.syrjala
  2017-07-10 17:34   ` Pandiyan, Dhinakaran
  2017-07-10 17:34   ` Rodrigo Vivi
  2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork
  5 siblings, 2 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-10 13:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Follow the GLK path when computing cdclk and related limits. CNL
pipes also produce two pixels per clock, so that's what we should
really use. However for the purposes of pixel rate calculations we
will assume one pixel per clock to keep the voltage higher, at least
until the missing voltage scaling for DDI clocks is implemented.

For the HBR2 vs. audio issue the limit should more correctly be 336
MHz, but the GLK limit of 316.8 MHz works just as well and results
in picking at least 336 MHz. Also toss in some related w/a numbers.

v2: Assume 1 pixel per clock for the purposes of max pixel rate
    calculation until DDI clock voltage scaling is handled

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 1241e5891b29..4b8eb6a7d852 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	    crtc_state->has_audio &&
 	    crtc_state->port_clock >= 540000 &&
 	    crtc_state->lane_count == 4) {
-		if (IS_CANNONLAKE(dev_priv))
-			pixel_rate = max(316800, pixel_rate);
-		else if (IS_GEMINILAKE(dev_priv))
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
+			/* Display WA #1145: glk,cnl */
 			pixel_rate = max(2 * 316800, pixel_rate);
-		else
+		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
+			/* Display WA #1144: skl,bxt */
 			pixel_rate = max(432000, pixel_rate);
+		}
 	}
 
 	/* According to BSpec, "The CD clock frequency must be at least twice
@@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	 * two pixels per clock.
 	 */
 	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
-		if (IS_GEMINILAKE(dev_priv))
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			pixel_rate = max(2 * 2 * 96000, pixel_rate);
 		else
 			pixel_rate = max(2 * 96000, pixel_rate);
@@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 {
 	int max_cdclk_freq = dev_priv->max_cdclk_freq;
 
-	if (IS_GEMINILAKE(dev_priv))
+	if (INTEL_GEN(dev_priv) >= 10)
+		/*
+		 * FIXME: Allow '2 * max_cdclk_freq'
+		 * once DDI clock voltage requirements are
+		 * handled correctly.
+		 */
+		return max_cdclk_freq;
+	else if (IS_GEMINILAKE(dev_priv))
 		/*
 		 * FIXME: Limiting to 99% as a temporary workaround. See
 		 * glk_calc_cdclk() for details.
-- 
2.13.0

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

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

* [PATCH v2 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
  2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
  2017-07-07 18:28   ` Ville Syrjälä
@ 2017-07-10 13:02   ` ville.syrjala
  1 sibling, 0 replies; 16+ messages in thread
From: ville.syrjala @ 2017-07-10 13:02 UTC (permalink / raw)
  To: intel-gfx; +Cc: Dhinakaran Pandiyan, Paulo Zanoni, Rodrigo Vivi

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Make the min_pixclk thing less confusing by changing it to track
the minimum acceptable cdclk frequency instead. This means moving
the application of the guardbands to a slightly higher level from
the low level platform specific calc_cdclk() functions.

The immediate benefit is elimination of the confusing 2x factors
on GLK/CNL+ in the audio workarounds (which stems from the fact
that the pipes produce two pixels per clock).

v2: Keep cdclk higher on CNL to workaround missing DDI clock voltage handling

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  12 ++-
 drivers/gpu/drm/i915/intel_cdclk.c   | 184 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_display.c |  21 ++--
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 4 files changed, 112 insertions(+), 109 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..c80176424d84 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -563,6 +563,15 @@ struct i915_hotplug {
 	     (__i)++) \
 		for_each_if (plane_state)
 
+#define for_each_new_intel_crtc_in_state(__state, crtc, new_crtc_state, __i) \
+	for ((__i) = 0; \
+	     (__i) < (__state)->base.dev->mode_config.num_crtc && \
+		     ((crtc) = to_intel_crtc((__state)->base.crtcs[__i].ptr), \
+		      (new_crtc_state) = to_intel_crtc_state((__state)->base.crtcs[__i].new_state), 1); \
+	     (__i)++) \
+		for_each_if (crtc)
+
+
 struct drm_i915_private;
 struct i915_mm_struct;
 struct i915_mmu_object;
@@ -2268,7 +2277,8 @@ struct drm_i915_private {
 	struct mutex dpll_lock;
 
 	unsigned int active_crtcs;
-	unsigned int min_pixclk[I915_MAX_PIPES];
+	/* minimum acceptable cdclk for each pipe */
+	int min_cdclk[I915_MAX_PIPES];
 
 	int dpio_phy_iosf_port[I915_NUM_PHYS_VLV];
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 9e0deebae279..9c0b3ca5256c 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -417,24 +417,21 @@ static void hsw_get_cdclk(struct drm_i915_private *dev_priv,
 		cdclk_state->cdclk = 540000;
 }
 
-static int vlv_calc_cdclk(struct drm_i915_private *dev_priv,
-			  int max_pixclk)
+static int vlv_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk)
 {
 	int freq_320 = (dev_priv->hpll_freq <<  1) % 320000 != 0 ?
 		333333 : 320000;
-	int limit = IS_CHERRYVIEW(dev_priv) ? 95 : 90;
 
 	/*
 	 * We seem to get an unstable or solid color picture at 200MHz.
 	 * Not sure what's wrong. For now use 200MHz only when all pipes
 	 * are off.
 	 */
-	if (!IS_CHERRYVIEW(dev_priv) &&
-	    max_pixclk > freq_320*limit/100)
+	if (IS_VALLEYVIEW(dev_priv) && min_cdclk > freq_320)
 		return 400000;
-	else if (max_pixclk > 266667*limit/100)
+	else if (min_cdclk > 266667)
 		return freq_320;
-	else if (max_pixclk > 0)
+	else if (min_cdclk > 0)
 		return 266667;
 	else
 		return 200000;
@@ -612,13 +609,13 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv,
 	intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A);
 }
 
-static int bdw_calc_cdclk(int max_pixclk)
+static int bdw_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 540000)
+	if (min_cdclk > 540000)
 		return 675000;
-	else if (max_pixclk > 450000)
+	else if (min_cdclk > 450000)
 		return 540000;
-	else if (max_pixclk > 337500)
+	else if (min_cdclk > 337500)
 		return 450000;
 	else
 		return 337500;
@@ -724,23 +721,23 @@ static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
 	     cdclk, dev_priv->cdclk.hw.cdclk);
 }
 
-static int skl_calc_cdclk(int max_pixclk, int vco)
+static int skl_calc_cdclk(int min_cdclk, int vco)
 {
 	if (vco == 8640000) {
-		if (max_pixclk > 540000)
+		if (min_cdclk > 540000)
 			return 617143;
-		else if (max_pixclk > 432000)
+		else if (min_cdclk > 432000)
 			return 540000;
-		else if (max_pixclk > 308571)
+		else if (min_cdclk > 308571)
 			return 432000;
 		else
 			return 308571;
 	} else {
-		if (max_pixclk > 540000)
+		if (min_cdclk > 540000)
 			return 675000;
-		else if (max_pixclk > 450000)
+		else if (min_cdclk > 450000)
 			return 540000;
-		else if (max_pixclk > 337500)
+		else if (min_cdclk > 337500)
 			return 450000;
 		else
 			return 337500;
@@ -1075,31 +1072,25 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	skl_set_cdclk(dev_priv, &cdclk_state);
 }
 
-static int bxt_calc_cdclk(int max_pixclk)
+static int bxt_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 576000)
+	if (min_cdclk > 576000)
 		return 624000;
-	else if (max_pixclk > 384000)
+	else if (min_cdclk > 384000)
 		return 576000;
-	else if (max_pixclk > 288000)
+	else if (min_cdclk > 288000)
 		return 384000;
-	else if (max_pixclk > 144000)
+	else if (min_cdclk > 144000)
 		return 288000;
 	else
 		return 144000;
 }
 
-static int glk_calc_cdclk(int max_pixclk)
+static int glk_calc_cdclk(int min_cdclk)
 {
-	/*
-	 * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
-	 * as a temporary workaround. Use a higher cdclk instead. (Note that
-	 * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
-	 * cdclk.)
-	 */
-	if (max_pixclk > DIV_ROUND_UP(2 * 158400 * 99, 100))
+	if (min_cdclk > 158400)
 		return 316800;
-	else if (max_pixclk > DIV_ROUND_UP(2 * 79200 * 99, 100))
+	else if (min_cdclk > 79200)
 		return 158400;
 	else
 		return 79200;
@@ -1420,11 +1411,11 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
 	bxt_set_cdclk(dev_priv, &cdclk_state);
 }
 
-static int cnl_calc_cdclk(int max_pixclk)
+static int cnl_calc_cdclk(int min_cdclk)
 {
-	if (max_pixclk > 336000)
+	if (min_cdclk > 336000)
 		return 528000;
-	else if (max_pixclk > 168000)
+	else if (min_cdclk > 168000)
 		return 336000;
 	else
 		return 168000;
@@ -1732,21 +1723,52 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
 }
 
-static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
-					  int pixel_rate)
+static int intel_min_cdclk(struct drm_i915_private *dev_priv,
+			   int pixel_rate)
+{
+	if (INTEL_GEN(dev_priv) >= 10)
+		/*
+		 * FIXME: Switch to DIV_ROUND_UP(pixel_rate, 2)
+		 * once DDI clock voltage requirements are
+		 * handled correctly.
+		 */
+		return pixel_rate;
+	else if (IS_GEMINILAKE(dev_priv))
+		/*
+		 * FIXME: Avoid using a pixel clock that is more than 99% of the cdclk
+		 * as a temporary workaround. Use a higher cdclk instead. (Note that
+		 * intel_compute_max_dotclk() limits the max pixel clock to 99% of max
+		 * cdclk.)
+		 */
+		return DIV_ROUND_UP(pixel_rate * 100, 2 * 99);
+	else if (IS_GEN9(dev_priv) ||
+		 IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
+		return pixel_rate;
+	else if (IS_CHERRYVIEW(dev_priv))
+		return DIV_ROUND_UP(pixel_rate * 100, 95);
+	else
+		return DIV_ROUND_UP(pixel_rate * 100, 90);
+}
+
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(crtc_state->base.crtc->dev);
+	int min_cdclk;
+
+	if (!crtc_state->base.enable)
+		return 0;
+
+	min_cdclk = intel_min_cdclk(dev_priv, crtc_state->pixel_rate);
 
 	/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
 	if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-		pixel_rate = DIV_ROUND_UP(pixel_rate * 100, 95);
+		min_cdclk = DIV_ROUND_UP(min_cdclk * 100, 95);
 
 	/* BSpec says "Do not use DisplayPort with CDCLK less than 432 MHz,
 	 * audio enabled, port width x4, and link rate HBR2 (5.4 GHz), or else
 	 * there may be audio corruption or screen corruption." This cdclk
-	 * restriction for GLK is 316.8 MHz and since GLK can output two
-	 * pixels per clock, the pixel rate becomes 2 * 316.8 MHz.
+	 * restriction for GLK is 316.8 MHz.
 	 */
 	if (intel_crtc_has_dp_encoder(crtc_state) &&
 	    crtc_state->has_audio &&
@@ -1754,77 +1776,53 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
 	    crtc_state->lane_count == 4) {
 		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			/* Display WA #1145: glk,cnl */
-			pixel_rate = max(2 * 316800, pixel_rate);
+			min_cdclk = max(316800, min_cdclk);
 		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
 			/* Display WA #1144: skl,bxt */
-			pixel_rate = max(432000, pixel_rate);
+			min_cdclk = max(432000, min_cdclk);
 		}
 	}
 
 	/* According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
-	 * The check for GLK has to be adjusted as the platform can output
-	 * two pixels per clock.
 	 */
-	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
-		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
-			pixel_rate = max(2 * 2 * 96000, pixel_rate);
-		else
-			pixel_rate = max(2 * 96000, pixel_rate);
-	}
+	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
+		min_cdclk = max(2 * 96000, min_cdclk);
 
-	return pixel_rate;
+	return min_cdclk;
 }
 
-/* compute the max rate for new configuration */
-static int intel_max_pixel_rate(struct drm_atomic_state *state)
+static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *cstate;
+	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
-	unsigned int max_pixel_rate = 0, i;
+	int min_cdclk = 0, i;
 	enum pipe pipe;
 
-	memcpy(intel_state->min_pixclk, dev_priv->min_pixclk,
-	       sizeof(intel_state->min_pixclk));
-
-	for_each_new_crtc_in_state(state, crtc, cstate, i) {
-		int pixel_rate;
+	memcpy(intel_state->min_cdclk, dev_priv->min_cdclk,
+	       sizeof(intel_state->min_cdclk));
 
-		crtc_state = to_intel_crtc_state(cstate);
-		if (!crtc_state->base.enable) {
-			intel_state->min_pixclk[i] = 0;
-			continue;
-		}
-
-		pixel_rate = crtc_state->pixel_rate;
-
-		if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9)
-			pixel_rate =
-				bdw_adjust_min_pipe_pixel_rate(crtc_state,
-							       pixel_rate);
-
-		intel_state->min_pixclk[i] = pixel_rate;
-	}
+	for_each_new_intel_crtc_in_state(intel_state, crtc, crtc_state, i)
+		intel_state->min_cdclk[i] =
+			intel_crtc_compute_min_cdclk(crtc_state);
 
 	for_each_pipe(dev_priv, pipe)
-		max_pixel_rate = max(intel_state->min_pixclk[pipe],
-				     max_pixel_rate);
+		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
-	return max_pixel_rate;
+	return min_cdclk;
 }
 
 static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 	int cdclk;
 
-	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
+	cdclk = vlv_calc_cdclk(dev_priv, min_cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1850,14 +1848,14 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk;
 
 	/*
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	cdclk = bdw_calc_cdclk(max_pixclk);
+	cdclk = bdw_calc_cdclk(min_cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1883,7 +1881,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	const int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk, vco;
 
 	vco = intel_state->cdclk.logical.vco;
@@ -1894,7 +1892,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	 * FIXME should also account for plane ratio
 	 * once 64bpp pixel formats are supported.
 	 */
-	cdclk = skl_calc_cdclk(max_pixclk, vco);
+	cdclk = skl_calc_cdclk(min_cdclk, vco);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
@@ -1921,16 +1919,16 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
 	int cdclk, vco;
 
 	if (IS_GEMINILAKE(dev_priv)) {
-		cdclk = glk_calc_cdclk(max_pixclk);
+		cdclk = glk_calc_cdclk(min_cdclk);
 		vco = glk_de_pll_vco(dev_priv, cdclk);
 	} else {
-		cdclk = bxt_calc_cdclk(max_pixclk);
+		cdclk = bxt_calc_cdclk(min_cdclk);
 		vco = bxt_de_pll_vco(dev_priv, cdclk);
 	}
 
@@ -1967,10 +1965,10 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct intel_atomic_state *intel_state =
 		to_intel_atomic_state(state);
-	int max_pixclk = intel_max_pixel_rate(state);
+	int min_cdclk = intel_compute_min_cdclk(state);
 	int cdclk, vco;
 
-	cdclk = cnl_calc_cdclk(max_pixclk);
+	cdclk = cnl_calc_cdclk(min_cdclk);
 	vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
 
 	if (cdclk > dev_priv->max_cdclk_freq) {
@@ -2005,11 +2003,11 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
 	else if (IS_GEMINILAKE(dev_priv))
 		/*
 		 * FIXME: Limiting to 99% as a temporary workaround. See
-		 * glk_calc_cdclk() for details.
+		 * intel_min_cdclk() for details.
 		 */
 		return 2 * max_cdclk_freq * 99 / 100;
-	else if (INTEL_INFO(dev_priv)->gen >= 9 ||
-		 IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
+	else if (IS_GEN9(dev_priv) ||
+		 IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
 		return max_cdclk_freq;
 	else if (IS_CHERRYVIEW(dev_priv))
 		return max_cdclk_freq*95/100;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2144adc5b1d5..b47535f5d95d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5920,7 +5920,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
 	intel_crtc->enabled_power_domains = 0;
 
 	dev_priv->active_crtcs &= ~(1 << intel_crtc->pipe);
-	dev_priv->min_pixclk[intel_crtc->pipe] = 0;
+	dev_priv->min_cdclk[intel_crtc->pipe] = 0;
 }
 
 /*
@@ -13292,8 +13292,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	intel_atomic_track_fbs(state);
 
 	if (intel_state->modeset) {
-		memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
-		       sizeof(intel_state->min_pixclk));
+		memcpy(dev_priv->min_cdclk, intel_state->min_cdclk,
+		       sizeof(intel_state->min_cdclk));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
@@ -15569,7 +15569,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	for_each_intel_crtc(dev, crtc) {
 		struct intel_crtc_state *crtc_state =
 			to_intel_crtc_state(crtc->base.state);
-		int pixclk = 0;
+		int min_cdclk = 0;
 
 		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
 		if (crtc_state->base.active) {
@@ -15590,22 +15590,15 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 			intel_crtc_compute_pixel_rate(crtc_state);
 
-			if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv) ||
-			    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-				pixclk = crtc_state->pixel_rate;
-			else
-				WARN_ON(dev_priv->display.modeset_calc_cdclk);
-
-			/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-			if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
-				pixclk = DIV_ROUND_UP(pixclk * 100, 95);
+			if (dev_priv->display.modeset_calc_cdclk)
+				min_cdclk = intel_crtc_compute_min_cdclk(crtc_state);
 
 			drm_calc_timestamping_constants(&crtc->base,
 							&crtc_state->base.adjusted_mode);
 			update_scanline_offset(crtc);
 		}
 
-		dev_priv->min_pixclk[crtc->pipe] = pixclk;
+		dev_priv->min_cdclk[crtc->pipe] = min_cdclk;
 
 		intel_pipe_config_sanity_check(dev_priv, crtc_state);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d17a32437f07..8cc1b86b799a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -383,7 +383,8 @@ struct intel_atomic_state {
 	unsigned int active_pipe_changes;
 
 	unsigned int active_crtcs;
-	unsigned int min_pixclk[I915_MAX_PIPES];
+	/* minimum acceptable cdclk for each pipe */
+	int min_cdclk[I915_MAX_PIPES];
 
 	struct intel_shared_dpll_state shared_dpll[I915_NUM_PLLS];
 
@@ -1308,6 +1309,7 @@ void intel_audio_init(struct drm_i915_private *dev_priv);
 void intel_audio_deinit(struct drm_i915_private *dev_priv);
 
 /* intel_cdclk.c */
+int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
 void skl_init_cdclk(struct drm_i915_private *dev_priv);
 void skl_uninit_cdclk(struct drm_i915_private *dev_priv);
 void cnl_init_cdclk(struct drm_i915_private *dev_priv);
-- 
2.13.0

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

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

* ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3)
  2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
                   ` (4 preceding siblings ...)
  2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
@ 2017-07-10 13:21 ` Patchwork
  5 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2017-07-10 13:21 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3)
URL   : https://patchwork.freedesktop.org/series/26988/
State : success

== Summary ==

Series 26988v3 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/26988/revisions/3/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-pnv-d510) fdo#101597 +1

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:445s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:427s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:355s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:520s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:507s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:484s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:596s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:435s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:498s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:576s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:582s
fi-pnv-d510      total:279  pass:221  dwarn:3   dfail:0   fail:0   skip:55  time:562s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:587s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:462s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:479s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:431s
fi-skl-x1585l    total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:481s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:544s
fi-snb-2600      total:279  pass:249  dwarn:0   dfail:0   fail:1   skip:29  time:410s

7484481f41b972c21f2d69c7b53df0d2522d9c70 drm-tip: 2017y-07m-10d-12h-14m-03s UTC integration manifest
9ad3738 drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk()
3be1884 drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock"
e14e106 drm/i915: Fix up CNL cdclk related limits

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5154/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
@ 2017-07-10 17:34   ` Pandiyan, Dhinakaran
  2017-07-10 18:11     ` Ville Syrjälä
  2017-07-10 17:34   ` Rodrigo Vivi
  1 sibling, 1 reply; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-10 17:34 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo




On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> really use. However for the purposes of pixel rate calculations we
> will assume one pixel per clock to keep the voltage higher, at least
> until the missing voltage scaling for DDI clocks is implemented.
> 

Does the lack of  correct voltage scaling implementation affect only
intel_compute_max_dotclk()? i.e., allowing a pixel rate of
2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
account pixel_rate <= 2*cdclk_freq for any frequency?


With this patch, 
bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
Isn't that a discrepancy?


> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.

In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
isn't the correct workaround cdclk 336 MHz?


> 
> v2: Assume 1 pixel per clock for the purposes of max pixel rate
>     calculation until DDI clock voltage scaling is handled
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..4b8eb6a7d852 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>  	    crtc_state->has_audio &&
>  	    crtc_state->port_clock >= 540000 &&
>  	    crtc_state->lane_count == 4) {
> -		if (IS_CANNONLAKE(dev_priv))
> -			pixel_rate = max(316800, pixel_rate);
> -		else if (IS_GEMINILAKE(dev_priv))
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +			/* Display WA #1145: glk,cnl */
>  			pixel_rate = max(2 * 316800, pixel_rate);
> -		else
> +		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> +			/* Display WA #1144: skl,bxt */
>  			pixel_rate = max(432000, pixel_rate);
> +		}
>  	}
>  
>  	/* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>  	 * two pixels per clock.
>  	 */
>  	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> -		if (IS_GEMINILAKE(dev_priv))
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>  			pixel_rate = max(2 * 2 * 96000, pixel_rate);
>  		else
>  			pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  {
>  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
>  
> -	if (IS_GEMINILAKE(dev_priv))
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		/*
> +		 * FIXME: Allow '2 * max_cdclk_freq'
> +		 * once DDI clock voltage requirements are
> +		 * handled correctly.
> +		 */
> +		return max_cdclk_freq;
> +	else if (IS_GEMINILAKE(dev_priv))
>  		/*
>  		 * FIXME: Limiting to 99% as a temporary workaround. See
>  		 * glk_calc_cdclk() for details.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
  2017-07-10 17:34   ` Pandiyan, Dhinakaran
@ 2017-07-10 17:34   ` Rodrigo Vivi
  2017-07-10 17:55     ` Ville Syrjälä
  1 sibling, 1 reply; 16+ messages in thread
From: Rodrigo Vivi @ 2017-07-10 17:34 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni

cool, with

v2 of patch 1
v2 of patch 2
patch 3

display works properly here on cnl.

On Mon, Jul 10, 2017 at 6:02 AM,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Follow the GLK path when computing cdclk and related limits. CNL
> pipes also produce two pixels per clock, so that's what we should
> really use. However for the purposes of pixel rate calculations we
> will assume one pixel per clock to keep the voltage higher, at least
> until the missing voltage scaling for DDI clocks is implemented.
>
> For the HBR2 vs. audio issue the limit should more correctly be 336
> MHz, but the GLK limit of 316.8 MHz works just as well and results
> in picking at least 336 MHz. Also toss in some related w/a numbers.
>
> v2: Assume 1 pixel per clock for the purposes of max pixel rate
>     calculation until DDI clock voltage scaling is handled
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 1241e5891b29..4b8eb6a7d852 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>             crtc_state->has_audio &&
>             crtc_state->port_clock >= 540000 &&
>             crtc_state->lane_count == 4) {
> -               if (IS_CANNONLAKE(dev_priv))
> -                       pixel_rate = max(316800, pixel_rate);
> -               else if (IS_GEMINILAKE(dev_priv))
> +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> +                       /* Display WA #1145: glk,cnl */
>                         pixel_rate = max(2 * 316800, pixel_rate);
> -               else
> +               } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> +                       /* Display WA #1144: skl,bxt */
>                         pixel_rate = max(432000, pixel_rate);
> +               }
>         }
>
>         /* According to BSpec, "The CD clock frequency must be at least twice
> @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
>          * two pixels per clock.
>          */
>         if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> -               if (IS_GEMINILAKE(dev_priv))
> +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>                         pixel_rate = max(2 * 2 * 96000, pixel_rate);
>                 else
>                         pixel_rate = max(2 * 96000, pixel_rate);
> @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
>  {
>         int max_cdclk_freq = dev_priv->max_cdclk_freq;
>
> -       if (IS_GEMINILAKE(dev_priv))
> +       if (INTEL_GEN(dev_priv) >= 10)
> +               /*
> +                * FIXME: Allow '2 * max_cdclk_freq'
> +                * once DDI clock voltage requirements are
> +                * handled correctly.
> +                */
> +               return max_cdclk_freq;
> +       else if (IS_GEMINILAKE(dev_priv))
>                 /*
>                  * FIXME: Limiting to 99% as a temporary workaround. See
>                  * glk_calc_cdclk() for details.

Are you sure we don't want this workaround also? With so similar
display engines I wonder if we would end with similar issues.
But I'm just asking... because honestly I didn't check that 99%
workaround closely enough yet...

The rest of the patch makes sense for me and works so feel free to use:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

rv-b only for this test for now, but tested-by you could use in all 3
patches mentioned above...


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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-10 17:34   ` Rodrigo Vivi
@ 2017-07-10 17:55     ` Ville Syrjälä
  0 siblings, 0 replies; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-10 17:55 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Rodrigo Vivi, Dhinakaran Pandiyan, Paulo Zanoni

On Mon, Jul 10, 2017 at 10:34:22AM -0700, Rodrigo Vivi wrote:
> cool, with
> 
> v2 of patch 1
> v2 of patch 2
> patch 3
> 
> display works properly here on cnl.
> 
> On Mon, Jul 10, 2017 at 6:02 AM,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > really use. However for the purposes of pixel rate calculations we
> > will assume one pixel per clock to keep the voltage higher, at least
> > until the missing voltage scaling for DDI clocks is implemented.
> >
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> >
> > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> >     calculation until DDI clock voltage scaling is handled
> >
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..4b8eb6a7d852 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >             crtc_state->has_audio &&
> >             crtc_state->port_clock >= 540000 &&
> >             crtc_state->lane_count == 4) {
> > -               if (IS_CANNONLAKE(dev_priv))
> > -                       pixel_rate = max(316800, pixel_rate);
> > -               else if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +                       /* Display WA #1145: glk,cnl */
> >                         pixel_rate = max(2 * 316800, pixel_rate);
> > -               else
> > +               } else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +                       /* Display WA #1144: skl,bxt */
> >                         pixel_rate = max(432000, pixel_rate);
> > +               }
> >         }
> >
> >         /* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >          * two pixels per clock.
> >          */
> >         if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -               if (IS_GEMINILAKE(dev_priv))
> > +               if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >                         pixel_rate = max(2 * 2 * 96000, pixel_rate);
> >                 else
> >                         pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> >  {
> >         int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >
> > -       if (IS_GEMINILAKE(dev_priv))
> > +       if (INTEL_GEN(dev_priv) >= 10)
> > +               /*
> > +                * FIXME: Allow '2 * max_cdclk_freq'
> > +                * once DDI clock voltage requirements are
> > +                * handled correctly.
> > +                */
> > +               return max_cdclk_freq;
> > +       else if (IS_GEMINILAKE(dev_priv))
> >                 /*
> >                  * FIXME: Limiting to 99% as a temporary workaround. See
> >                  * glk_calc_cdclk() for details.
> 
> Are you sure we don't want this workaround also? With so similar
> display engines I wonder if we would end with similar issues.
> But I'm just asking... because honestly I didn't check that 99%
> workaround closely enough yet...

That workaround seems to be based on empirical evidence and I'd like to
people to get to the bottom of it before it spreads all over the place.

One theory I have is that it might be caused by simply using the wrong
pixel clock when we calculate the pipe's pixel rate. That code currently
assumes that the DPLL can generate a 100% accurate clock which
definitely isn't always the case. IIRC I have code somewhere that
tries to correct that by moving the DPLL computation to happen earlier
than the cdclk computation. That would mean we could then update
adjusted_mode.crtc_clock with the actual clock produced by the DPLL.
I think that patch is stuck in my IVB bifurcation branch and I don't
recall if some of the modeset code reorganization I have there is needed
as well. I guess I'd have to try and extract it and see how much I
need to pull out with it.

> 
> The rest of the patch makes sense for me and works so feel free to use:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Tested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> rv-b only for this test for now, but tested-by you could use in all 3
> patches mentioned above...
> 
> 
> > --
> > 2.13.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br

-- 
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] 16+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-10 17:34   ` Pandiyan, Dhinakaran
@ 2017-07-10 18:11     ` Ville Syrjälä
  2017-07-10 18:36       ` Pandiyan, Dhinakaran
  0 siblings, 1 reply; 16+ messages in thread
From: Ville Syrjälä @ 2017-07-10 18:11 UTC (permalink / raw)
  To: Pandiyan, Dhinakaran; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo

On Mon, Jul 10, 2017 at 05:34:11PM +0000, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Follow the GLK path when computing cdclk and related limits. CNL
> > pipes also produce two pixels per clock, so that's what we should
> > really use. However for the purposes of pixel rate calculations we
> > will assume one pixel per clock to keep the voltage higher, at least
> > until the missing voltage scaling for DDI clocks is implemented.
> > 
> 
> Does the lack of  correct voltage scaling implementation affect only
> intel_compute_max_dotclk()? i.e., allowing a pixel rate of
> 2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
> account pixel_rate <= 2*cdclk_freq for any frequency?
> 
> 
> With this patch, 
> bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
> cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
> Isn't that a discrepancy?

Hmm. Yeah. I suppose I should just squash this with patch 2/3. My
original intention for this separate patch was to respect the 2x
limit rather than the 1x limit. But since we couldn't do that I
suppose the justification for this patch has pretty much gone
away, and as you point out, it just leads to a mess.

The combination of patches 1/3+2/3 should still do the right thing
because we no longer use the pixel rate in the audio workarounds.

> 
> 
> > For the HBR2 vs. audio issue the limit should more correctly be 336
> > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > in picking at least 336 MHz. Also toss in some related w/a numbers.
> 
> In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
> 633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
> isn't the correct workaround cdclk 336 MHz?
> 
> 
> > 
> > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> >     calculation until DDI clock voltage scaling is handled
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 1241e5891b29..4b8eb6a7d852 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >  	    crtc_state->has_audio &&
> >  	    crtc_state->port_clock >= 540000 &&
> >  	    crtc_state->lane_count == 4) {
> > -		if (IS_CANNONLAKE(dev_priv))
> > -			pixel_rate = max(316800, pixel_rate);
> > -		else if (IS_GEMINILAKE(dev_priv))
> > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > +			/* Display WA #1145: glk,cnl */
> >  			pixel_rate = max(2 * 316800, pixel_rate);
> > -		else
> > +		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > +			/* Display WA #1144: skl,bxt */
> >  			pixel_rate = max(432000, pixel_rate);
> > +		}
> >  	}
> >  
> >  	/* According to BSpec, "The CD clock frequency must be at least twice
> > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> >  	 * two pixels per clock.
> >  	 */
> >  	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > -		if (IS_GEMINILAKE(dev_priv))
> > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >  			pixel_rate = max(2 * 2 * 96000, pixel_rate);
> >  		else
> >  			pixel_rate = max(2 * 96000, pixel_rate);
> > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> >  {
> >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> >  
> > -	if (IS_GEMINILAKE(dev_priv))
> > +	if (INTEL_GEN(dev_priv) >= 10)
> > +		/*
> > +		 * FIXME: Allow '2 * max_cdclk_freq'
> > +		 * once DDI clock voltage requirements are
> > +		 * handled correctly.
> > +		 */
> > +		return max_cdclk_freq;
> > +	else if (IS_GEMINILAKE(dev_priv))
> >  		/*
> >  		 * FIXME: Limiting to 99% as a temporary workaround. See
> >  		 * glk_calc_cdclk() for details.

-- 
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] 16+ messages in thread

* Re: [PATCH v2 1/3] drm/i915: Fix up CNL cdclk related limits
  2017-07-10 18:11     ` Ville Syrjälä
@ 2017-07-10 18:36       ` Pandiyan, Dhinakaran
  0 siblings, 0 replies; 16+ messages in thread
From: Pandiyan, Dhinakaran @ 2017-07-10 18:36 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, Zanoni, Paulo R, Vivi, Rodrigo

On Mon, 2017-07-10 at 21:11 +0300, Ville Syrjälä wrote:
> On Mon, Jul 10, 2017 at 05:34:11PM +0000, Pandiyan, Dhinakaran wrote:
> > 
> > 
> > 
> > On Mon, 2017-07-10 at 16:02 +0300, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Follow the GLK path when computing cdclk and related limits. CNL
> > > pipes also produce two pixels per clock, so that's what we should
> > > really use. However for the purposes of pixel rate calculations we
> > > will assume one pixel per clock to keep the voltage higher, at least
> > > until the missing voltage scaling for DDI clocks is implemented.
> > > 
> > 
> > Does the lack of  correct voltage scaling implementation affect only
> > intel_compute_max_dotclk()? i.e., allowing a pixel rate of
> > 2*max_cdclk_freq? Or does it mean cnl_calc_cdclk() cannot take into
> > account pixel_rate <= 2*cdclk_freq for any frequency?
> > 
> > 
> > With this patch, 
> > bdw_adjust_min_pipe_pixel_rate() compares pixel_rate to 2*cdclk
> > cnl_calc_cdclk() compares pixel_rate to 1*cdclk.
> > Isn't that a discrepancy?
> 
> Hmm. Yeah. I suppose I should just squash this with patch 2/3. My
> original intention for this separate patch was to respect the 2x
> limit rather than the 1x limit. But since we couldn't do that I
> suppose the justification for this patch has pretty much gone
> away, and as you point out, it just leads to a mess.
> 
> The combination of patches 1/3+2/3 should still do the right thing
> because we no longer use the pixel rate in the audio workarounds.
> 

I just took a look at 2/3, squashing does solve the problem. I'll review
the combined patch when you send it.

-DK 

> > 
> > 
> > > For the HBR2 vs. audio issue the limit should more correctly be 336
> > > MHz, but the GLK limit of 316.8 MHz works just as well and results
> > > in picking at least 336 MHz. Also toss in some related w/a numbers.
> > 
> > In this case, _adjust_min_pipe_pixel_rate() will return pixel_rate as
> > 633.6 MHz, followed by cnl_calc_cdclk() returning 528 MHz cdclk. But,
> > isn't the correct workaround cdclk 336 MHz?
> > 
> > 
> > > 
> > > v2: Assume 1 pixel per clock for the purposes of max pixel rate
> > >     calculation until DDI clock voltage scaling is handled
> > > 
> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++------
> > >  1 file changed, 14 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 1241e5891b29..4b8eb6a7d852 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -1752,12 +1752,13 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > >  	    crtc_state->has_audio &&
> > >  	    crtc_state->port_clock >= 540000 &&
> > >  	    crtc_state->lane_count == 4) {
> > > -		if (IS_CANNONLAKE(dev_priv))
> > > -			pixel_rate = max(316800, pixel_rate);
> > > -		else if (IS_GEMINILAKE(dev_priv))
> > > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> > > +			/* Display WA #1145: glk,cnl */
> > >  			pixel_rate = max(2 * 316800, pixel_rate);
> > > -		else
> > > +		} else if (IS_GEN9(dev_priv) || IS_BROADWELL(dev_priv)) {
> > > +			/* Display WA #1144: skl,bxt */
> > >  			pixel_rate = max(432000, pixel_rate);
> > > +		}
> > >  	}
> > >  
> > >  	/* According to BSpec, "The CD clock frequency must be at least twice
> > > @@ -1766,7 +1767,7 @@ static int bdw_adjust_min_pipe_pixel_rate(struct intel_crtc_state *crtc_state,
> > >  	 * two pixels per clock.
> > >  	 */
> > >  	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9) {
> > > -		if (IS_GEMINILAKE(dev_priv))
> > > +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > >  			pixel_rate = max(2 * 2 * 96000, pixel_rate);
> > >  		else
> > >  			pixel_rate = max(2 * 96000, pixel_rate);
> > > @@ -1999,7 +2000,14 @@ static int intel_compute_max_dotclk(struct drm_i915_private *dev_priv)
> > >  {
> > >  	int max_cdclk_freq = dev_priv->max_cdclk_freq;
> > >  
> > > -	if (IS_GEMINILAKE(dev_priv))
> > > +	if (INTEL_GEN(dev_priv) >= 10)
> > > +		/*
> > > +		 * FIXME: Allow '2 * max_cdclk_freq'
> > > +		 * once DDI clock voltage requirements are
> > > +		 * handled correctly.
> > > +		 */
> > > +		return max_cdclk_freq;
> > > +	else if (IS_GEMINILAKE(dev_priv))
> > >  		/*
> > >  		 * FIXME: Limiting to 99% as a temporary workaround. See
> > >  		 * glk_calc_cdclk() for details.
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-07-10 18:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 10:24 [PATCH 1/3] drm/i915: Fix up CNL cdclk related limits ville.syrjala
2017-07-07 10:24 ` [PATCH 2/3] drm/i915: Track minimum acceptable cdclk instead of "minimum dotclock" ville.syrjala
2017-07-07 18:28   ` Ville Syrjälä
2017-07-10 13:02   ` [PATCH v2 " ville.syrjala
2017-07-07 10:24 ` [PATCH 3/3] drm/i915: Consolidate max_cdclk_freq check in intel_crtc_compute_min_cdclk() ville.syrjala
2017-07-07 11:13 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Fix up CNL cdclk related limits Patchwork
2017-07-07 17:54 ` [PATCH 1/3] " Rodrigo Vivi
2017-07-07 18:24   ` Ville Syrjälä
2017-07-07 18:33     ` Pandiyan, Dhinakaran
2017-07-10 13:02 ` [PATCH v2 " ville.syrjala
2017-07-10 17:34   ` Pandiyan, Dhinakaran
2017-07-10 18:11     ` Ville Syrjälä
2017-07-10 18:36       ` Pandiyan, Dhinakaran
2017-07-10 17:34   ` Rodrigo Vivi
2017-07-10 17:55     ` Ville Syrjälä
2017-07-10 13:21 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Fix up CNL cdclk related limits (rev3) Patchwork

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.