All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
@ 2019-06-18 22:50 José Roberto de Souza
  2019-06-18 22:50 ` [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks José Roberto de Souza
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: José Roberto de Souza @ 2019-06-18 22:50 UTC (permalink / raw)
  To: intel-gfx

Now 180, 172.8 and 192 MHz are supported.

180 and 172.8 MHz CD clocks will only be used when audio is not
enabled as state by BSpec and implemented in
intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
but we are not reading it.

BSpec: 20598
BSpec: 15729
Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++-------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 8993ab283562..d560e25d3fb5 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 
 static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
 {
-	int ranges_24[] = { 312000, 552000, 648000 };
-	int ranges_19_38[] = { 307200, 556800, 652800 };
-	int *ranges;
+	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
+	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
+	const int *ranges;
+	unsigned int len, i;
 
 	switch (ref) {
 	default:
@@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
 		/* fall through */
 	case 24000:
 		ranges = ranges_24;
+		len = ARRAY_SIZE(ranges_24);
 		break;
 	case 19200:
 	case 38400:
 		ranges = ranges_19_38;
+		len = ARRAY_SIZE(ranges_19_38);
 		break;
 	}
 
-	if (min_cdclk > ranges[1])
-		return ranges[2];
-	else if (min_cdclk > ranges[0])
-		return ranges[1];
-	else
-		return ranges[0];
+	for (i = 0; i < len; i++) {
+		if (min_cdclk <= ranges[i])
+			return ranges[i];
+	}
+
+	WARN_ON(min_cdclk > ranges[len - 1]);
+	return ranges[len - 1];
 }
 
 static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
@@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
 	default:
 		MISSING_CASE(cdclk);
 		/* fall through */
+	case 172800:
 	case 307200:
 	case 556800:
 	case 652800:
 		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
 			dev_priv->cdclk.hw.ref != 38400);
 		break;
+	case 180000:
 	case 312000:
 	case 552000:
 	case 648000:
 		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
+		break;
+	case 192000:
+		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
+			dev_priv->cdclk.hw.ref != 38400 &&
+			dev_priv->cdclk.hw.ref != 24000);
 	}
 
 	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
-- 
2.22.0

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

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

* [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
@ 2019-06-18 22:50 ` José Roberto de Souza
  2019-06-19 11:40   ` Ville Syrjälä
  2019-06-18 22:50 ` [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table José Roberto de Souza
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-06-18 22:50 UTC (permalink / raw)
  To: intel-gfx

EHL do not support 648 and 652.8 MHz.

BSpec: 20598
Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index d560e25d3fb5..26c17ecf2083 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1754,7 +1754,8 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
 	dev_priv->cdclk.hw.vco = -1;
 }
 
-static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
+static int icl_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk,
+			  unsigned int ref)
 {
 	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
 	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
@@ -1776,6 +1777,12 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
 		break;
 	}
 
+	/*
+	 * EHL do not support 648 and 652.8 MHz, so just decrement the len
+	 */
+	if (IS_ELKHARTLAKE(dev_priv))
+		len--;
+
 	for (i = 0; i < len; i++) {
 		if (min_cdclk <= ranges[i])
 			return ranges[i];
@@ -1954,7 +1961,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
 	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
 
 	sanitized_state.ref = dev_priv->cdclk.hw.ref;
-	sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
+	sanitized_state.cdclk = icl_calc_cdclk(dev_priv, 0,
+					       sanitized_state.ref);
 	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
 						     sanitized_state.cdclk);
 	sanitized_state.voltage_level =
@@ -2554,7 +2562,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
 	if (min_cdclk < 0)
 		return min_cdclk;
 
-	cdclk = icl_calc_cdclk(min_cdclk, ref);
+	cdclk = icl_calc_cdclk(dev_priv, min_cdclk, ref);
 	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
 
 	state->cdclk.logical.vco = vco;
@@ -2564,7 +2572,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
 		    cnl_compute_min_voltage_level(state));
 
 	if (!state->active_crtcs) {
-		cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
+		cdclk = icl_calc_cdclk(dev_priv, state->cdclk.force_min_cdclk,
+				       ref);
 		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
 
 		state->cdclk.actual.vco = vco;
-- 
2.22.0

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

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

* [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
  2019-06-18 22:50 ` [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks José Roberto de Souza
@ 2019-06-18 22:50 ` José Roberto de Souza
  2019-06-19 11:43   ` Ville Syrjälä
  2019-06-18 23:25 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks Patchwork
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 20+ messages in thread
From: José Roberto de Souza @ 2019-06-18 22:50 UTC (permalink / raw)
  To: intel-gfx

EHL has it own voltage level requirement depending on cd clock.

BSpec: 21809
Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 26c17ecf2083..575ab1a96bbc 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1872,8 +1872,17 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
 	dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
 }
 
-static u8 icl_calc_voltage_level(int cdclk)
+static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int cdclk)
 {
+	if (IS_ELKHARTLAKE(dev_priv)) {
+		if (cdclk > 312000)
+			return 2;
+		else if (cdclk > 180000)
+			return 1;
+		else
+			return 0;
+	}
+
 	if (cdclk > 556800)
 		return 2;
 	else if (cdclk > 312000)
@@ -1930,7 +1939,7 @@ static void icl_get_cdclk(struct drm_i915_private *dev_priv,
 	 * at least what the CDCLK frequency requires.
 	 */
 	cdclk_state->voltage_level =
-		icl_calc_voltage_level(cdclk_state->cdclk);
+		icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
 }
 
 static void icl_init_cdclk(struct drm_i915_private *dev_priv)
@@ -1966,7 +1975,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
 	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
 						     sanitized_state.cdclk);
 	sanitized_state.voltage_level =
-				icl_calc_voltage_level(sanitized_state.cdclk);
+				icl_calc_voltage_level(dev_priv,
+						       sanitized_state.cdclk);
 
 	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
 }
@@ -1977,7 +1987,8 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
 
 	cdclk_state.cdclk = cdclk_state.bypass;
 	cdclk_state.vco = 0;
-	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
+	cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
+							   cdclk_state.cdclk);
 
 	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
@@ -2568,7 +2579,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
 	state->cdclk.logical.vco = vco;
 	state->cdclk.logical.cdclk = cdclk;
 	state->cdclk.logical.voltage_level =
-		max(icl_calc_voltage_level(cdclk),
+		max(icl_calc_voltage_level(dev_priv, cdclk),
 		    cnl_compute_min_voltage_level(state));
 
 	if (!state->active_crtcs) {
@@ -2579,7 +2590,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
 		state->cdclk.actual.vco = vco;
 		state->cdclk.actual.cdclk = cdclk;
 		state->cdclk.actual.voltage_level =
-			icl_calc_voltage_level(cdclk);
+			icl_calc_voltage_level(dev_priv, cdclk);
 	} else {
 		state->cdclk.actual = state->cdclk.logical;
 	}
-- 
2.22.0

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

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

* ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
  2019-06-18 22:50 ` [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks José Roberto de Souza
  2019-06-18 22:50 ` [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table José Roberto de Souza
@ 2019-06-18 23:25 ` Patchwork
  2019-06-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-06-18 23:25 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/icl: Add new supported CD clocks
URL   : https://patchwork.freedesktop.org/series/62355/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/icl: Add new supported CD clocks
Okay!

Commit: drm/i915/ehl: Remove unsupported cd clocks
Okay!

Commit: drm/i915/ehl: Add voltage level requirement table
-O:drivers/gpu/drm/i915/display/intel_cdclk.c:2571:17: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/display/intel_cdclk.c:2571:17: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/display/intel_cdclk.c:2582:17: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/display/intel_cdclk.c:2582:17: warning: expression using sizeof(void)

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
                   ` (2 preceding siblings ...)
  2019-06-18 23:25 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks Patchwork
@ 2019-06-18 23:44 ` Patchwork
  2019-06-19  7:43 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2) Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-06-18 23:44 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/icl: Add new supported CD clocks
URL   : https://patchwork.freedesktop.org/series/62355/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6299 -> Patchwork_13337
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13337 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13337, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13337:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_sync@basic-store-each:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-bdw-5557u/igt@gem_sync@basic-store-each.html

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [DMESG-FAIL][2] ([fdo#110235]) -> [INCOMPLETE][3]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  
Known issues
------------

  Here are the changes found in Patchwork_13337 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u2:          [PASS][4] -> [INCOMPLETE][5] ([fdo#107713] / [fdo#108569])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-icl-u2/igt@i915_selftest@live_hangcheck.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-icl-u2/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-dsi:         [INCOMPLETE][6] ([fdo#107713] / [fdo#110246]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-icl-dsi/igt@gem_cpu_reloc@basic.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-icl-dsi/igt@gem_cpu_reloc@basic.html

  * igt@gem_sync@basic-store-each:
    - fi-kbl-7567u:       [FAIL][8] -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-kbl-7567u/igt@gem_sync@basic-store-each.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-kbl-7567u/igt@gem_sync@basic-store-each.html

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [INCOMPLETE][10] ([fdo#107718]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-blb-e6850/igt@i915_module_load@reload.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-blb-e6850/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][12] ([fdo#108511]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [DMESG-FAIL][14] ([fdo#110235]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][16] ([fdo#102614]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6299/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246


Participating hosts (44 -> 35)
------------------------------

  Additional (1): fi-bdw-5557u 
  Missing    (10): fi-kbl-soraka fi-ilk-m540 fi-bsw-n3050 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6299 -> Patchwork_13337

  CI_DRM_6299: 2a6cf9f4dc05b77beab4b7d9d1c9f16c7e55a001 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5059: 1f67ee0d09d6513f487f2be74aae9700e755258a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13337: e9f434aaa5f9e67d0aee4ab23bb9e98e40841e91 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e9f434aaa5f9 drm/i915/ehl: Add voltage level requirement table
70a07288126a drm/i915/ehl: Remove unsupported cd clocks
7b8941b203bd drm/i915/icl: Add new supported CD clocks

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13337/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2)
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
                   ` (3 preceding siblings ...)
  2019-06-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-06-19  7:43 ` Patchwork
  2019-06-19  8:09 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-06-19  7:43 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2)
URL   : https://patchwork.freedesktop.org/series/62355/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/icl: Add new supported CD clocks
Okay!

Commit: drm/i915/ehl: Remove unsupported cd clocks
Okay!

Commit: drm/i915/ehl: Add voltage level requirement table
-O:drivers/gpu/drm/i915/display/intel_cdclk.c:2571:17: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/display/intel_cdclk.c:2571:17: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/display/intel_cdclk.c:2582:17: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/display/intel_cdclk.c:2582:17: warning: expression using sizeof(void)

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

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

* ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2)
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
                   ` (4 preceding siblings ...)
  2019-06-19  7:43 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2) Patchwork
@ 2019-06-19  8:09 ` Patchwork
  2019-06-19 11:48 ` [PATCH 1/3] drm/i915/icl: Add new supported CD clocks Ville Syrjälä
  2019-06-19 17:47 ` Ville Syrjälä
  7 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-06-19  8:09 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2)
URL   : https://patchwork.freedesktop.org/series/62355/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6300 -> Patchwork_13342
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13342 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13342, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_13342:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_sync@basic-store-each:
    - fi-skl-6770hq:      [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-skl-6770hq/igt@gem_sync@basic-store-each.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-skl-6770hq/igt@gem_sync@basic-store-each.html

  * igt@i915_selftest@live_gtt:
    - fi-apl-guc:         [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-apl-guc/igt@i915_selftest@live_gtt.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-apl-guc/igt@i915_selftest@live_gtt.html

  
Known issues
------------

  Here are the changes found in Patchwork_13342 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][5] -> [INCOMPLETE][6] ([fdo#107718])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [PASS][7] -> [DMESG-FAIL][8] ([fdo#110235])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@prime_busy@basic-after-default:
    - fi-icl-u3:          [PASS][9] -> [DMESG-WARN][10] ([fdo#107724])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-icl-u3/igt@prime_busy@basic-after-default.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-icl-u3/igt@prime_busy@basic-after-default.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-read-no-prefault:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-icl-u3/igt@gem_mmap_gtt@basic-read-no-prefault.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-icl-u3/igt@gem_mmap_gtt@basic-read-no-prefault.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [DMESG-FAIL][13] ([fdo#110235]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          [FAIL][15] ([fdo#103167]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_busy@basic-wait-after-default:
    - fi-icl-dsi:         [INCOMPLETE][17] ([fdo#107713]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6300/fi-icl-dsi/igt@prime_busy@basic-wait-after-default.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/fi-icl-dsi/igt@prime_busy@basic-wait-after-default.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235


Participating hosts (44 -> 42)
------------------------------

  Additional (6): fi-cml-u2 fi-kbl-guc fi-whl-u fi-kbl-x1275 fi-gdg-551 fi-cml-u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_6300 -> Patchwork_13342

  CI_DRM_6300: a0694108fecf62a79e0be32e578f25fdcbf466e4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5059: 1f67ee0d09d6513f487f2be74aae9700e755258a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13342: 58c6fb7f6c3888ce67bff4294fa89371a246c690 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

58c6fb7f6c38 drm/i915/ehl: Add voltage level requirement table
a9968a30cb98 drm/i915/ehl: Remove unsupported cd clocks
e2f4342fa7ce drm/i915/icl: Add new supported CD clocks

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13342/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks
  2019-06-18 22:50 ` [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks José Roberto de Souza
@ 2019-06-19 11:40   ` Ville Syrjälä
  2019-06-19 15:21     ` Jani Nikula
  2019-06-20  0:37     ` Souza, Jose
  0 siblings, 2 replies; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-19 11:40 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:50:34PM -0700, José Roberto de Souza wrote:
> EHL do not support 648 and 652.8 MHz.

You should modify the max_cdclk() function instead. I think that along
should be sufficient.

> 
> BSpec: 20598
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index d560e25d3fb5..26c17ecf2083 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1754,7 +1754,8 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  	dev_priv->cdclk.hw.vco = -1;
>  }
>  
> -static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> +static int icl_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk,
> +			  unsigned int ref)
>  {
>  	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
>  	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
> @@ -1776,6 +1777,12 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  		break;
>  	}
>  
> +	/*
> +	 * EHL do not support 648 and 652.8 MHz, so just decrement the len
> +	 */
> +	if (IS_ELKHARTLAKE(dev_priv))
> +		len--;
> +
>  	for (i = 0; i < len; i++) {
>  		if (min_cdclk <= ranges[i])
>  			return ranges[i];
> @@ -1954,7 +1961,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
>  	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
>  
>  	sanitized_state.ref = dev_priv->cdclk.hw.ref;
> -	sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
> +	sanitized_state.cdclk = icl_calc_cdclk(dev_priv, 0,
> +					       sanitized_state.ref);
>  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
>  						     sanitized_state.cdclk);
>  	sanitized_state.voltage_level =
> @@ -2554,7 +2562,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>  	if (min_cdclk < 0)
>  		return min_cdclk;
>  
> -	cdclk = icl_calc_cdclk(min_cdclk, ref);
> +	cdclk = icl_calc_cdclk(dev_priv, min_cdclk, ref);
>  	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>  
>  	state->cdclk.logical.vco = vco;
> @@ -2564,7 +2572,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>  		    cnl_compute_min_voltage_level(state));
>  
>  	if (!state->active_crtcs) {
> -		cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
> +		cdclk = icl_calc_cdclk(dev_priv, state->cdclk.force_min_cdclk,
> +				       ref);
>  		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>  
>  		state->cdclk.actual.vco = vco;
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table
  2019-06-18 22:50 ` [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table José Roberto de Souza
@ 2019-06-19 11:43   ` Ville Syrjälä
  2019-06-20  0:36     ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-19 11:43 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:50:35PM -0700, José Roberto de Souza wrote:
> EHL has it own voltage level requirement depending on cd clock.
> 
> BSpec: 21809
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 26c17ecf2083..575ab1a96bbc 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1872,8 +1872,17 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
>  	dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
>  }
>  
> -static u8 icl_calc_voltage_level(int cdclk)
> +static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int cdclk)
>  {
> +	if (IS_ELKHARTLAKE(dev_priv)) {
> +		if (cdclk > 312000)
> +			return 2;
> +		else if (cdclk > 180000)
> +			return 1;
> +		else
> +			return 0;
> +	}
> +
>  	if (cdclk > 556800)
>  		return 2;
>  	else if (cdclk > 312000)

I would move the rest into and else branch to make it clear the
two are just the two sides of the same coin.

> @@ -1930,7 +1939,7 @@ static void icl_get_cdclk(struct drm_i915_private *dev_priv,
>  	 * at least what the CDCLK frequency requires.
>  	 */
>  	cdclk_state->voltage_level =
> -		icl_calc_voltage_level(cdclk_state->cdclk);
> +		icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
>  }
>  
>  static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> @@ -1966,7 +1975,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
>  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
>  						     sanitized_state.cdclk);
>  	sanitized_state.voltage_level =
> -				icl_calc_voltage_level(sanitized_state.cdclk);
> +				icl_calc_voltage_level(dev_priv,
> +						       sanitized_state.cdclk);
>  
>  	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
>  }
> @@ -1977,7 +1987,8 @@ static void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
>  
>  	cdclk_state.cdclk = cdclk_state.bypass;
>  	cdclk_state.vco = 0;
> -	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
> +	cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
> +							   cdclk_state.cdclk);
>  
>  	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>  }
> @@ -2568,7 +2579,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>  	state->cdclk.logical.vco = vco;
>  	state->cdclk.logical.cdclk = cdclk;
>  	state->cdclk.logical.voltage_level =
> -		max(icl_calc_voltage_level(cdclk),
> +		max(icl_calc_voltage_level(dev_priv, cdclk),
>  		    cnl_compute_min_voltage_level(state));
>  
>  	if (!state->active_crtcs) {
> @@ -2579,7 +2590,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>  		state->cdclk.actual.vco = vco;
>  		state->cdclk.actual.cdclk = cdclk;
>  		state->cdclk.actual.voltage_level =
> -			icl_calc_voltage_level(cdclk);
> +			icl_calc_voltage_level(dev_priv, cdclk);
>  	} else {
>  		state->cdclk.actual = state->cdclk.logical;
>  	}
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
                   ` (5 preceding siblings ...)
  2019-06-19  8:09 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-06-19 11:48 ` Ville Syrjälä
  2019-06-20  0:36   ` Souza, Jose
  2019-06-19 17:47 ` Ville Syrjälä
  7 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-19 11:48 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza wrote:
> Now 180, 172.8 and 192 MHz are supported.
> 
> 180 and 172.8 MHz CD clocks will only be used when audio is not
> enabled as state by BSpec and implemented in
> intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
> but we are not reading it.
> 
> BSpec: 20598
> BSpec: 15729
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++-------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 8993ab283562..d560e25d3fb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  
>  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  {
> -	int ranges_24[] = { 312000, 552000, 648000 };
> -	int ranges_19_38[] = { 307200, 556800, 652800 };
> -	int *ranges;
> +	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
> +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };

static?

> +	const int *ranges;
> +	unsigned int len, i;

just 'int' please

>  
>  	switch (ref) {
>  	default:
> @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  		/* fall through */
>  	case 24000:
>  		ranges = ranges_24;
> +		len = ARRAY_SIZE(ranges_24);
>  		break;
>  	case 19200:
>  	case 38400:
>  		ranges = ranges_19_38;
> +		len = ARRAY_SIZE(ranges_19_38);
>  		break;
>  	}
>  
> -	if (min_cdclk > ranges[1])
> -		return ranges[2];
> -	else if (min_cdclk > ranges[0])
> -		return ranges[1];
> -	else
> -		return ranges[0];
> +	for (i = 0; i < len; i++) {
> +		if (min_cdclk <= ranges[i])
> +			return ranges[i];
> +	}
> +
> +	WARN_ON(min_cdclk > ranges[len - 1]);
> +	return ranges[len - 1];
>  }
>  
>  static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>  	default:
>  		MISSING_CASE(cdclk);
>  		/* fall through */
> +	case 172800:
>  	case 307200:
>  	case 556800:
>  	case 652800:
>  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
>  			dev_priv->cdclk.hw.ref != 38400);
>  		break;
> +	case 180000:
>  	case 312000:
>  	case 552000:
>  	case 648000:
>  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> +		break;
> +	case 192000:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> +			dev_priv->cdclk.hw.ref != 38400 &&
> +			dev_priv->cdclk.hw.ref != 24000);

I'd probably put a break here too for symmetry.

>  	}
>  
>  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks
  2019-06-19 11:40   ` Ville Syrjälä
@ 2019-06-19 15:21     ` Jani Nikula
  2019-06-20  0:37     ` Souza, Jose
  1 sibling, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2019-06-19 15:21 UTC (permalink / raw)
  To: Ville Syrjälä, José Roberto de Souza; +Cc: intel-gfx

On Wed, 19 Jun 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Jun 18, 2019 at 03:50:34PM -0700, José Roberto de Souza wrote:
>> EHL do not support 648 and 652.8 MHz.
>
> You should modify the max_cdclk() function instead. I think that along
> should be sufficient.

Side note, these "if (foo) len--;" constructs lead to later
trouble. Either pick a different array based on the platform, or have a
way to filter the values (e.g. by max or by min).

The last "len--" I untangled was the DP link rate support.


BR,
Jani.


>
>> 
>> BSpec: 20598
>> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index d560e25d3fb5..26c17ecf2083 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -1754,7 +1754,8 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>>  	dev_priv->cdclk.hw.vco = -1;
>>  }
>>  
>> -static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>> +static int icl_calc_cdclk(struct drm_i915_private *dev_priv, int min_cdclk,
>> +			  unsigned int ref)
>>  {
>>  	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
>>  	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
>> @@ -1776,6 +1777,12 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>>  		break;
>>  	}
>>  
>> +	/*
>> +	 * EHL do not support 648 and 652.8 MHz, so just decrement the len
>> +	 */
>> +	if (IS_ELKHARTLAKE(dev_priv))
>> +		len--;
>> +
>>  	for (i = 0; i < len; i++) {
>>  		if (min_cdclk <= ranges[i])
>>  			return ranges[i];
>> @@ -1954,7 +1961,8 @@ static void icl_init_cdclk(struct drm_i915_private *dev_priv)
>>  	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
>>  
>>  	sanitized_state.ref = dev_priv->cdclk.hw.ref;
>> -	sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
>> +	sanitized_state.cdclk = icl_calc_cdclk(dev_priv, 0,
>> +					       sanitized_state.ref);
>>  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
>>  						     sanitized_state.cdclk);
>>  	sanitized_state.voltage_level =
>> @@ -2554,7 +2562,7 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>>  	if (min_cdclk < 0)
>>  		return min_cdclk;
>>  
>> -	cdclk = icl_calc_cdclk(min_cdclk, ref);
>> +	cdclk = icl_calc_cdclk(dev_priv, min_cdclk, ref);
>>  	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>>  
>>  	state->cdclk.logical.vco = vco;
>> @@ -2564,7 +2572,8 @@ static int icl_modeset_calc_cdclk(struct intel_atomic_state *state)
>>  		    cnl_compute_min_voltage_level(state));
>>  
>>  	if (!state->active_crtcs) {
>> -		cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk, ref);
>> +		cdclk = icl_calc_cdclk(dev_priv, state->cdclk.force_min_cdclk,
>> +				       ref);
>>  		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>>  
>>  		state->cdclk.actual.vco = vco;
>> -- 
>> 2.22.0
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
                   ` (6 preceding siblings ...)
  2019-06-19 11:48 ` [PATCH 1/3] drm/i915/icl: Add new supported CD clocks Ville Syrjälä
@ 2019-06-19 17:47 ` Ville Syrjälä
  2019-06-20 23:33   ` Souza, Jose
  7 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-19 17:47 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza wrote:
> Now 180, 172.8 and 192 MHz are supported.
> 
> 180 and 172.8 MHz CD clocks will only be used when audio is not
> enabled as state by BSpec and implemented in
> intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48 MHz
> but we are not reading it.

Do we know whether this thing still suffers from the glk/cnl issue where
we can't even talk to the codec if cdclk is too low? If it does we need
to adjust the platform check for the "force min cdclk for audio" thing.
Also that is going to suck on icl due to the cd2x divider always being
1 and hence we can't do the optimized cdclk change around audio power
requests.

> 
> BSpec: 20598
> BSpec: 15729
> Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++-------
>  1 file changed, 20 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 8993ab283562..d560e25d3fb5 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct drm_i915_private *dev_priv)
>  
>  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  {
> -	int ranges_24[] = { 312000, 552000, 648000 };
> -	int ranges_19_38[] = { 307200, 556800, 652800 };
> -	int *ranges;
> +	const int ranges_24[] = { 180000, 192000, 312000, 552000, 648000 };
> +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800, 652800 };
> +	const int *ranges;
> +	unsigned int len, i;
>  
>  	switch (ref) {
>  	default:
> @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
>  		/* fall through */
>  	case 24000:
>  		ranges = ranges_24;
> +		len = ARRAY_SIZE(ranges_24);
>  		break;
>  	case 19200:
>  	case 38400:
>  		ranges = ranges_19_38;
> +		len = ARRAY_SIZE(ranges_19_38);
>  		break;
>  	}
>  
> -	if (min_cdclk > ranges[1])
> -		return ranges[2];
> -	else if (min_cdclk > ranges[0])
> -		return ranges[1];
> -	else
> -		return ranges[0];
> +	for (i = 0; i < len; i++) {
> +		if (min_cdclk <= ranges[i])
> +			return ranges[i];
> +	}
> +
> +	WARN_ON(min_cdclk > ranges[len - 1]);
> +	return ranges[len - 1];
>  }
>  
>  static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>  	default:
>  		MISSING_CASE(cdclk);
>  		/* fall through */
> +	case 172800:
>  	case 307200:
>  	case 556800:
>  	case 652800:
>  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
>  			dev_priv->cdclk.hw.ref != 38400);
>  		break;
> +	case 180000:
>  	case 312000:
>  	case 552000:
>  	case 648000:
>  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> +		break;
> +	case 192000:
> +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> +			dev_priv->cdclk.hw.ref != 38400 &&
> +			dev_priv->cdclk.hw.ref != 24000);
>  	}
>  
>  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table
  2019-06-19 11:43   ` Ville Syrjälä
@ 2019-06-20  0:36     ` Souza, Jose
  2019-06-20 10:01       ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Souza, Jose @ 2019-06-20  0:36 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-06-19 at 14:43 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:35PM -0700, José Roberto de Souza
> wrote:
> > EHL has it own voltage level requirement depending on cd clock.
> > 
> > BSpec: 21809
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++++++++++++++++
> > ------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 26c17ecf2083..575ab1a96bbc 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1872,8 +1872,17 @@ static void icl_set_cdclk(struct
> > drm_i915_private *dev_priv,
> >  	dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
> >  }
> >  
> > -static u8 icl_calc_voltage_level(int cdclk)
> > +static u8 icl_calc_voltage_level(struct drm_i915_private
> > *dev_priv, int cdclk)
> >  {
> > +	if (IS_ELKHARTLAKE(dev_priv)) {
> > +		if (cdclk > 312000)
> > +			return 2;
> > +		else if (cdclk > 180000)
> > +			return 1;
> > +		else
> > +			return 0;
> > +	}
> > +
> >  	if (cdclk > 556800)
> >  		return 2;
> >  	else if (cdclk > 312000)
> 
> I would move the rest into and else branch to make it clear the
> two are just the two sides of the same coin.

You mean like this?

static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int
cdclk)
{
	if (IS_ELKHARTLAKE(dev_priv)) {
		if (cdclk > 312000)
			return 2;
		else if (cdclk > 180000)
			return 1;
		else
			return 0;
	} else {
		if (cdclk > 556800)
			return 2;
		else if (cdclk > 312000)
			return 1;
		else
			return 0;
	}
}


> 
> > @@ -1930,7 +1939,7 @@ static void icl_get_cdclk(struct
> > drm_i915_private *dev_priv,
> >  	 * at least what the CDCLK frequency requires.
> >  	 */
> >  	cdclk_state->voltage_level =
> > -		icl_calc_voltage_level(cdclk_state->cdclk);
> > +		icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
> >  }
> >  
> >  static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > @@ -1966,7 +1975,8 @@ static void icl_init_cdclk(struct
> > drm_i915_private *dev_priv)
> >  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> >  						     sanitized_state.cd
> > clk);
> >  	sanitized_state.voltage_level =
> > -				icl_calc_voltage_level(sanitized_state.
> > cdclk);
> > +				icl_calc_voltage_level(dev_priv,
> > +						       sanitized_state.
> > cdclk);
> >  
> >  	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> >  }
> > @@ -1977,7 +1987,8 @@ static void icl_uninit_cdclk(struct
> > drm_i915_private *dev_priv)
> >  
> >  	cdclk_state.cdclk = cdclk_state.bypass;
> >  	cdclk_state.vco = 0;
> > -	cdclk_state.voltage_level =
> > icl_calc_voltage_level(cdclk_state.cdclk);
> > +	cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
> > +							   cdclk_state.
> > cdclk);
> >  
> >  	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >  }
> > @@ -2568,7 +2579,7 @@ static int icl_modeset_calc_cdclk(struct
> > intel_atomic_state *state)
> >  	state->cdclk.logical.vco = vco;
> >  	state->cdclk.logical.cdclk = cdclk;
> >  	state->cdclk.logical.voltage_level =
> > -		max(icl_calc_voltage_level(cdclk),
> > +		max(icl_calc_voltage_level(dev_priv, cdclk),
> >  		    cnl_compute_min_voltage_level(state));
> >  
> >  	if (!state->active_crtcs) {
> > @@ -2579,7 +2590,7 @@ static int icl_modeset_calc_cdclk(struct
> > intel_atomic_state *state)
> >  		state->cdclk.actual.vco = vco;
> >  		state->cdclk.actual.cdclk = cdclk;
> >  		state->cdclk.actual.voltage_level =
> > -			icl_calc_voltage_level(cdclk);
> > +			icl_calc_voltage_level(dev_priv, cdclk);
> >  	} else {
> >  		state->cdclk.actual = state->cdclk.logical;
> >  	}
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-19 11:48 ` [PATCH 1/3] drm/i915/icl: Add new supported CD clocks Ville Syrjälä
@ 2019-06-20  0:36   ` Souza, Jose
  2019-06-20 10:03     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Souza, Jose @ 2019-06-20  0:36 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-06-19 at 14:48 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> wrote:
> > Now 180, 172.8 and 192 MHz are supported.
> > 
> > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > enabled as state by BSpec and implemented in
> > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > MHz
> > but we are not reading it.
> > 
> > BSpec: 20598
> > BSpec: 15729
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++---
> > ----
> >  1 file changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 8993ab283562..d560e25d3fb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >  
> >  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> >  {
> > -	int ranges_24[] = { 312000, 552000, 648000 };
> > -	int ranges_19_38[] = { 307200, 556800, 652800 };
> > -	int *ranges;
> > +	const int ranges_24[] = { 180000, 192000, 312000, 552000,
> > 648000 };
> > +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > 652800 };
> 
> static?

We don't need to keep this in memory all the time.

> 
> > +	const int *ranges;
> > +	unsigned int len, i;
> 
> just 'int' please

Ok

> 
> >  
> >  	switch (ref) {
> >  	default:
> > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > unsigned int ref)
> >  		/* fall through */
> >  	case 24000:
> >  		ranges = ranges_24;
> > +		len = ARRAY_SIZE(ranges_24);
> >  		break;
> >  	case 19200:
> >  	case 38400:
> >  		ranges = ranges_19_38;
> > +		len = ARRAY_SIZE(ranges_19_38);
> >  		break;
> >  	}
> >  
> > -	if (min_cdclk > ranges[1])
> > -		return ranges[2];
> > -	else if (min_cdclk > ranges[0])
> > -		return ranges[1];
> > -	else
> > -		return ranges[0];
> > +	for (i = 0; i < len; i++) {
> > +		if (min_cdclk <= ranges[i])
> > +			return ranges[i];
> > +	}
> > +
> > +	WARN_ON(min_cdclk > ranges[len - 1]);
> > +	return ranges[len - 1];
> >  }
> >  
> >  static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > drm_i915_private *dev_priv, int cdclk)
> >  	default:
> >  		MISSING_CASE(cdclk);
> >  		/* fall through */
> > +	case 172800:
> >  	case 307200:
> >  	case 556800:
> >  	case 652800:
> >  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> >  			dev_priv->cdclk.hw.ref != 38400);
> >  		break;
> > +	case 180000:
> >  	case 312000:
> >  	case 552000:
> >  	case 648000:
> >  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > +		break;
> > +	case 192000:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > +			dev_priv->cdclk.hw.ref != 38400 &&
> > +			dev_priv->cdclk.hw.ref != 24000);
> 
> I'd probably put a break here too for symmetry.

Ok

> 
> >  	}
> >  
> >  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks
  2019-06-19 11:40   ` Ville Syrjälä
  2019-06-19 15:21     ` Jani Nikula
@ 2019-06-20  0:37     ` Souza, Jose
  1 sibling, 0 replies; 20+ messages in thread
From: Souza, Jose @ 2019-06-20  0:37 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-06-19 at 14:40 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:34PM -0700, José Roberto de Souza
> wrote:
> > EHL do not support 648 and 652.8 MHz.
> 
> You should modify the max_cdclk() function instead. I think that
> along
> should be sufficient.

Okay and it makes the patch smaller, thanks

> 
> > BSpec: 20598
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index d560e25d3fb5..26c17ecf2083 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1754,7 +1754,8 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >  	dev_priv->cdclk.hw.vco = -1;
> >  }
> >  
> > -static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > +static int icl_calc_cdclk(struct drm_i915_private *dev_priv, int
> > min_cdclk,
> > +			  unsigned int ref)
> >  {
> >  	const int ranges_24[] = { 180000, 192000, 312000, 552000,
> > 648000 };
> >  	const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > 652800 };
> > @@ -1776,6 +1777,12 @@ static int icl_calc_cdclk(int min_cdclk,
> > unsigned int ref)
> >  		break;
> >  	}
> >  
> > +	/*
> > +	 * EHL do not support 648 and 652.8 MHz, so just decrement the
> > len
> > +	 */
> > +	if (IS_ELKHARTLAKE(dev_priv))
> > +		len--;
> > +
> >  	for (i = 0; i < len; i++) {
> >  		if (min_cdclk <= ranges[i])
> >  			return ranges[i];
> > @@ -1954,7 +1961,8 @@ static void icl_init_cdclk(struct
> > drm_i915_private *dev_priv)
> >  	DRM_DEBUG_KMS("Sanitizing cdclk programmed by pre-os\n");
> >  
> >  	sanitized_state.ref = dev_priv->cdclk.hw.ref;
> > -	sanitized_state.cdclk = icl_calc_cdclk(0, sanitized_state.ref);
> > +	sanitized_state.cdclk = icl_calc_cdclk(dev_priv, 0,
> > +					       sanitized_state.ref);
> >  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> >  						     sanitized_state.cd
> > clk);
> >  	sanitized_state.voltage_level =
> > @@ -2554,7 +2562,7 @@ static int icl_modeset_calc_cdclk(struct
> > intel_atomic_state *state)
> >  	if (min_cdclk < 0)
> >  		return min_cdclk;
> >  
> > -	cdclk = icl_calc_cdclk(min_cdclk, ref);
> > +	cdclk = icl_calc_cdclk(dev_priv, min_cdclk, ref);
> >  	vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> >  
> >  	state->cdclk.logical.vco = vco;
> > @@ -2564,7 +2572,8 @@ static int icl_modeset_calc_cdclk(struct
> > intel_atomic_state *state)
> >  		    cnl_compute_min_voltage_level(state));
> >  
> >  	if (!state->active_crtcs) {
> > -		cdclk = icl_calc_cdclk(state->cdclk.force_min_cdclk,
> > ref);
> > +		cdclk = icl_calc_cdclk(dev_priv, state-
> > >cdclk.force_min_cdclk,
> > +				       ref);
> >  		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
> >  
> >  		state->cdclk.actual.vco = vco;
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table
  2019-06-20  0:36     ` Souza, Jose
@ 2019-06-20 10:01       ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-20 10:01 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Thu, Jun 20, 2019 at 12:36:03AM +0000, Souza, Jose wrote:
> On Wed, 2019-06-19 at 14:43 +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2019 at 03:50:35PM -0700, José Roberto de Souza
> > wrote:
> > > EHL has it own voltage level requirement depending on cd clock.
> > > 
> > > BSpec: 21809
> > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > Cc: Matt Roper <matthew.d.roper@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 23 ++++++++++++++++
> > > ------
> > >  1 file changed, 17 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 26c17ecf2083..575ab1a96bbc 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1872,8 +1872,17 @@ static void icl_set_cdclk(struct
> > > drm_i915_private *dev_priv,
> > >  	dev_priv->cdclk.hw.voltage_level = cdclk_state->voltage_level;
> > >  }
> > >  
> > > -static u8 icl_calc_voltage_level(int cdclk)
> > > +static u8 icl_calc_voltage_level(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > >  {
> > > +	if (IS_ELKHARTLAKE(dev_priv)) {
> > > +		if (cdclk > 312000)
> > > +			return 2;
> > > +		else if (cdclk > 180000)
> > > +			return 1;
> > > +		else
> > > +			return 0;
> > > +	}
> > > +
> > >  	if (cdclk > 556800)
> > >  		return 2;
> > >  	else if (cdclk > 312000)
> > 
> > I would move the rest into and else branch to make it clear the
> > two are just the two sides of the same coin.
> 
> You mean like this?

Yes.

> 
> static u8 icl_calc_voltage_level(struct drm_i915_private *dev_priv, int
> cdclk)
> {
> 	if (IS_ELKHARTLAKE(dev_priv)) {
> 		if (cdclk > 312000)
> 			return 2;
> 		else if (cdclk > 180000)
> 			return 1;
> 		else
> 			return 0;
> 	} else {
> 		if (cdclk > 556800)
> 			return 2;
> 		else if (cdclk > 312000)
> 			return 1;
> 		else
> 			return 0;
> 	}
> }
> 
> 
> > 
> > > @@ -1930,7 +1939,7 @@ static void icl_get_cdclk(struct
> > > drm_i915_private *dev_priv,
> > >  	 * at least what the CDCLK frequency requires.
> > >  	 */
> > >  	cdclk_state->voltage_level =
> > > -		icl_calc_voltage_level(cdclk_state->cdclk);
> > > +		icl_calc_voltage_level(dev_priv, cdclk_state->cdclk);
> > >  }
> > >  
> > >  static void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > > @@ -1966,7 +1975,8 @@ static void icl_init_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >  	sanitized_state.vco = icl_calc_cdclk_pll_vco(dev_priv,
> > >  						     sanitized_state.cd
> > > clk);
> > >  	sanitized_state.voltage_level =
> > > -				icl_calc_voltage_level(sanitized_state.
> > > cdclk);
> > > +				icl_calc_voltage_level(dev_priv,
> > > +						       sanitized_state.
> > > cdclk);
> > >  
> > >  	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> > >  }
> > > @@ -1977,7 +1987,8 @@ static void icl_uninit_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >  
> > >  	cdclk_state.cdclk = cdclk_state.bypass;
> > >  	cdclk_state.vco = 0;
> > > -	cdclk_state.voltage_level =
> > > icl_calc_voltage_level(cdclk_state.cdclk);
> > > +	cdclk_state.voltage_level = icl_calc_voltage_level(dev_priv,
> > > +							   cdclk_state.
> > > cdclk);
> > >  
> > >  	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >  }
> > > @@ -2568,7 +2579,7 @@ static int icl_modeset_calc_cdclk(struct
> > > intel_atomic_state *state)
> > >  	state->cdclk.logical.vco = vco;
> > >  	state->cdclk.logical.cdclk = cdclk;
> > >  	state->cdclk.logical.voltage_level =
> > > -		max(icl_calc_voltage_level(cdclk),
> > > +		max(icl_calc_voltage_level(dev_priv, cdclk),
> > >  		    cnl_compute_min_voltage_level(state));
> > >  
> > >  	if (!state->active_crtcs) {
> > > @@ -2579,7 +2590,7 @@ static int icl_modeset_calc_cdclk(struct
> > > intel_atomic_state *state)
> > >  		state->cdclk.actual.vco = vco;
> > >  		state->cdclk.actual.cdclk = cdclk;
> > >  		state->cdclk.actual.voltage_level =
> > > -			icl_calc_voltage_level(cdclk);
> > > +			icl_calc_voltage_level(dev_priv, cdclk);
> > >  	} else {
> > >  		state->cdclk.actual = state->cdclk.logical;
> > >  	}
> > > -- 
> > > 2.22.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-20  0:36   ` Souza, Jose
@ 2019-06-20 10:03     ` Ville Syrjälä
  0 siblings, 0 replies; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-20 10:03 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Thu, Jun 20, 2019 at 12:36:57AM +0000, Souza, Jose wrote:
> On Wed, 2019-06-19 at 14:48 +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > wrote:
> > > Now 180, 172.8 and 192 MHz are supported.
> > > 
> > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > enabled as state by BSpec and implemented in
> > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > > MHz
> > > but we are not reading it.
> > > 
> > > BSpec: 20598
> > > BSpec: 15729
> > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++---
> > > ----
> > >  1 file changed, 20 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 8993ab283562..d560e25d3fb5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >  
> > >  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > >  {
> > > -	int ranges_24[] = { 312000, 552000, 648000 };
> > > -	int ranges_19_38[] = { 307200, 556800, 652800 };
> > > -	int *ranges;
> > > +	const int ranges_24[] = { 180000, 192000, 312000, 552000,
> > > 648000 };
> > > +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > > 652800 };
> > 
> > static?
> 
> We don't need to keep this in memory all the time.

It'll be in memory either way. It's going to end up
as either text or bss. bss seems better.

> 
> > 
> > > +	const int *ranges;
> > > +	unsigned int len, i;
> > 
> > just 'int' please
> 
> Ok
> 
> > 
> > >  
> > >  	switch (ref) {
> > >  	default:
> > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > > unsigned int ref)
> > >  		/* fall through */
> > >  	case 24000:
> > >  		ranges = ranges_24;
> > > +		len = ARRAY_SIZE(ranges_24);
> > >  		break;
> > >  	case 19200:
> > >  	case 38400:
> > >  		ranges = ranges_19_38;
> > > +		len = ARRAY_SIZE(ranges_19_38);
> > >  		break;
> > >  	}
> > >  
> > > -	if (min_cdclk > ranges[1])
> > > -		return ranges[2];
> > > -	else if (min_cdclk > ranges[0])
> > > -		return ranges[1];
> > > -	else
> > > -		return ranges[0];
> > > +	for (i = 0; i < len; i++) {
> > > +		if (min_cdclk <= ranges[i])
> > > +			return ranges[i];
> > > +	}
> > > +
> > > +	WARN_ON(min_cdclk > ranges[len - 1]);
> > > +	return ranges[len - 1];
> > >  }
> > >  
> > >  static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > > drm_i915_private *dev_priv, int cdclk)
> > >  	default:
> > >  		MISSING_CASE(cdclk);
> > >  		/* fall through */
> > > +	case 172800:
> > >  	case 307200:
> > >  	case 556800:
> > >  	case 652800:
> > >  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > >  			dev_priv->cdclk.hw.ref != 38400);
> > >  		break;
> > > +	case 180000:
> > >  	case 312000:
> > >  	case 552000:
> > >  	case 648000:
> > >  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > +		break;
> > > +	case 192000:
> > > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > +			dev_priv->cdclk.hw.ref != 38400 &&
> > > +			dev_priv->cdclk.hw.ref != 24000);
> > 
> > I'd probably put a break here too for symmetry.
> 
> Ok
> 
> > 
> > >  	}
> > >  
> > >  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > -- 
> > > 2.22.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-19 17:47 ` Ville Syrjälä
@ 2019-06-20 23:33   ` Souza, Jose
  2019-06-24 12:39     ` Ville Syrjälä
  0 siblings, 1 reply; 20+ messages in thread
From: Souza, Jose @ 2019-06-20 23:33 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> wrote:
> > Now 180, 172.8 and 192 MHz are supported.
> > 
> > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > enabled as state by BSpec and implemented in
> > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > MHz
> > but we are not reading it.
> 
> Do we know whether this thing still suffers from the glk/cnl issue
> where
> we can't even talk to the codec if cdclk is too low? If it does we
> need
> to adjust the platform check for the "force min cdclk for audio"
> thing.
> Also that is going to suck on icl due to the cd2x divider always
> being
> 1 and hence we can't do the optimized cdclk change around audio power
> requests.

Not sure how to test if we can talk to the codec when cdclock is too
low(you mean lower than 192000khz(96MHz*2)?) but this workflow worked:

1- only full HD eDP panel connected  | cdclock = 172800khz
2- connected a full HD DP monitor with audio | cdclock = 192000khz
	* audio works
3- disconnected full HD DP monitor | cdclock = 172800khz
4- connected again a full HD DP monitor with audio | cdclock =
192000khz
	* audio works

Do you have any other test in mind?

> 
> > BSpec: 20598
> > BSpec: 15729
> > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++---
> > ----
> >  1 file changed, 20 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > index 8993ab283562..d560e25d3fb5 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > drm_i915_private *dev_priv)
> >  
> >  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> >  {
> > -	int ranges_24[] = { 312000, 552000, 648000 };
> > -	int ranges_19_38[] = { 307200, 556800, 652800 };
> > -	int *ranges;
> > +	const int ranges_24[] = { 180000, 192000, 312000, 552000,
> > 648000 };
> > +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > 652800 };
> > +	const int *ranges;
> > +	unsigned int len, i;
> >  
> >  	switch (ref) {
> >  	default:
> > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > unsigned int ref)
> >  		/* fall through */
> >  	case 24000:
> >  		ranges = ranges_24;
> > +		len = ARRAY_SIZE(ranges_24);
> >  		break;
> >  	case 19200:
> >  	case 38400:
> >  		ranges = ranges_19_38;
> > +		len = ARRAY_SIZE(ranges_19_38);
> >  		break;
> >  	}
> >  
> > -	if (min_cdclk > ranges[1])
> > -		return ranges[2];
> > -	else if (min_cdclk > ranges[0])
> > -		return ranges[1];
> > -	else
> > -		return ranges[0];
> > +	for (i = 0; i < len; i++) {
> > +		if (min_cdclk <= ranges[i])
> > +			return ranges[i];
> > +	}
> > +
> > +	WARN_ON(min_cdclk > ranges[len - 1]);
> > +	return ranges[len - 1];
> >  }
> >  
> >  static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > *dev_priv, int cdclk)
> > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > drm_i915_private *dev_priv, int cdclk)
> >  	default:
> >  		MISSING_CASE(cdclk);
> >  		/* fall through */
> > +	case 172800:
> >  	case 307200:
> >  	case 556800:
> >  	case 652800:
> >  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> >  			dev_priv->cdclk.hw.ref != 38400);
> >  		break;
> > +	case 180000:
> >  	case 312000:
> >  	case 552000:
> >  	case 648000:
> >  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > +		break;
> > +	case 192000:
> > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > +			dev_priv->cdclk.hw.ref != 38400 &&
> > +			dev_priv->cdclk.hw.ref != 24000);
> >  	}
> >  
> >  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > -- 
> > 2.22.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-20 23:33   ` Souza, Jose
@ 2019-06-24 12:39     ` Ville Syrjälä
  2019-06-24 21:05       ` Souza, Jose
  0 siblings, 1 reply; 20+ messages in thread
From: Ville Syrjälä @ 2019-06-24 12:39 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx

On Thu, Jun 20, 2019 at 11:33:27PM +0000, Souza, Jose wrote:
> On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > wrote:
> > > Now 180, 172.8 and 192 MHz are supported.
> > > 
> > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > enabled as state by BSpec and implemented in
> > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice of
> > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to 48
> > > MHz
> > > but we are not reading it.
> > 
> > Do we know whether this thing still suffers from the glk/cnl issue
> > where
> > we can't even talk to the codec if cdclk is too low? If it does we
> > need
> > to adjust the platform check for the "force min cdclk for audio"
> > thing.
> > Also that is going to suck on icl due to the cd2x divider always
> > being
> > 1 and hence we can't do the optimized cdclk change around audio power
> > requests.
> 
> Not sure how to test if we can talk to the codec when cdclock is too
> low(you mean lower than 192000khz(96MHz*2)?) but this workflow worked:
> 
> 1- only full HD eDP panel connected  | cdclock = 172800khz
> 2- connected a full HD DP monitor with audio | cdclock = 192000khz
> 	* audio works
> 3- disconnected full HD DP monitor | cdclock = 172800khz
> 4- connected again a full HD DP monitor with audio | cdclock =
> 192000khz
> 	* audio works
> 
> Do you have any other test in mind?

Try to load/resume the audio driver when cdclk is low maybe?

> 
> > 
> > > BSpec: 20598
> > > BSpec: 15729
> > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 29 +++++++++++++++---
> > > ----
> > >  1 file changed, 20 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > index 8993ab283562..d560e25d3fb5 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > drm_i915_private *dev_priv)
> > >  
> > >  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > >  {
> > > -	int ranges_24[] = { 312000, 552000, 648000 };
> > > -	int ranges_19_38[] = { 307200, 556800, 652800 };
> > > -	int *ranges;
> > > +	const int ranges_24[] = { 180000, 192000, 312000, 552000,
> > > 648000 };
> > > +	const int ranges_19_38[] = { 172800, 192000, 307200, 556800,
> > > 652800 };
> > > +	const int *ranges;
> > > +	unsigned int len, i;
> > >  
> > >  	switch (ref) {
> > >  	default:
> > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int min_cdclk,
> > > unsigned int ref)
> > >  		/* fall through */
> > >  	case 24000:
> > >  		ranges = ranges_24;
> > > +		len = ARRAY_SIZE(ranges_24);
> > >  		break;
> > >  	case 19200:
> > >  	case 38400:
> > >  		ranges = ranges_19_38;
> > > +		len = ARRAY_SIZE(ranges_19_38);
> > >  		break;
> > >  	}
> > >  
> > > -	if (min_cdclk > ranges[1])
> > > -		return ranges[2];
> > > -	else if (min_cdclk > ranges[0])
> > > -		return ranges[1];
> > > -	else
> > > -		return ranges[0];
> > > +	for (i = 0; i < len; i++) {
> > > +		if (min_cdclk <= ranges[i])
> > > +			return ranges[i];
> > > +	}
> > > +
> > > +	WARN_ON(min_cdclk > ranges[len - 1]);
> > > +	return ranges[len - 1];
> > >  }
> > >  
> > >  static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > *dev_priv, int cdclk)
> > > @@ -1792,16 +1796,23 @@ static int icl_calc_cdclk_pll_vco(struct
> > > drm_i915_private *dev_priv, int cdclk)
> > >  	default:
> > >  		MISSING_CASE(cdclk);
> > >  		/* fall through */
> > > +	case 172800:
> > >  	case 307200:
> > >  	case 556800:
> > >  	case 652800:
> > >  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > >  			dev_priv->cdclk.hw.ref != 38400);
> > >  		break;
> > > +	case 180000:
> > >  	case 312000:
> > >  	case 552000:
> > >  	case 648000:
> > >  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > +		break;
> > > +	case 192000:
> > > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > +			dev_priv->cdclk.hw.ref != 38400 &&
> > > +			dev_priv->cdclk.hw.ref != 24000);
> > >  	}
> > >  
> > >  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > -- 
> > > 2.22.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 1/3] drm/i915/icl: Add new supported CD clocks
  2019-06-24 12:39     ` Ville Syrjälä
@ 2019-06-24 21:05       ` Souza, Jose
  0 siblings, 0 replies; 20+ messages in thread
From: Souza, Jose @ 2019-06-24 21:05 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

On Mon, 2019-06-24 at 15:39 +0300, Ville Syrjälä wrote:
> On Thu, Jun 20, 2019 at 11:33:27PM +0000, Souza, Jose wrote:
> > On Wed, 2019-06-19 at 20:47 +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 18, 2019 at 03:50:33PM -0700, José Roberto de Souza
> > > wrote:
> > > > Now 180, 172.8 and 192 MHz are supported.
> > > > 
> > > > 180 and 172.8 MHz CD clocks will only be used when audio is not
> > > > enabled as state by BSpec and implemented in
> > > > intel_crtc_compute_min_cdclk(), CD clock must be at least twice
> > > > of
> > > > Azalia BCLK and BCLK by default is 96 MHz, it could be set to
> > > > 48
> > > > MHz
> > > > but we are not reading it.
> > > 
> > > Do we know whether this thing still suffers from the glk/cnl
> > > issue
> > > where
> > > we can't even talk to the codec if cdclk is too low? If it does
> > > we
> > > need
> > > to adjust the platform check for the "force min cdclk for audio"
> > > thing.
> > > Also that is going to suck on icl due to the cd2x divider always
> > > being
> > > 1 and hence we can't do the optimized cdclk change around audio
> > > power
> > > requests.
> > 
> > Not sure how to test if we can talk to the codec when cdclock is
> > too
> > low(you mean lower than 192000khz(96MHz*2)?) but this workflow
> > worked:
> > 
> > 1- only full HD eDP panel connected  | cdclock = 172800khz
> > 2- connected a full HD DP monitor with audio | cdclock = 192000khz
> > 	* audio works
> > 3- disconnected full HD DP monitor | cdclock = 172800khz
> > 4- connected again a full HD DP monitor with audio | cdclock =
> > 192000khz
> > 	* audio works
> > 
> > Do you have any other test in mind?
> 
> Try to load/resume the audio driver when cdclk is low maybe?

Oh good idea.

It worked, snd_hda_intel loaded without errors while cdclk was 172800
kHz and then connecting a monitor with audio the cdclk was set to
192000 kHz and audio was send to speakers.	

> 
> > > > BSpec: 20598
> > > > BSpec: 15729
> > > > Cc: Clint Taylor <Clinton.A.Taylor@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_cdclk.c | 29
> > > > +++++++++++++++---
> > > > ----
> > > >  1 file changed, 20 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > index 8993ab283562..d560e25d3fb5 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> > > > @@ -1756,9 +1756,10 @@ static void cnl_sanitize_cdclk(struct
> > > > drm_i915_private *dev_priv)
> > > >  
> > > >  static int icl_calc_cdclk(int min_cdclk, unsigned int ref)
> > > >  {
> > > > -	int ranges_24[] = { 312000, 552000, 648000 };
> > > > -	int ranges_19_38[] = { 307200, 556800, 652800 };
> > > > -	int *ranges;
> > > > +	const int ranges_24[] = { 180000, 192000, 312000,
> > > > 552000,
> > > > 648000 };
> > > > +	const int ranges_19_38[] = { 172800, 192000, 307200,
> > > > 556800,
> > > > 652800 };
> > > > +	const int *ranges;
> > > > +	unsigned int len, i;
> > > >  
> > > >  	switch (ref) {
> > > >  	default:
> > > > @@ -1766,19 +1767,22 @@ static int icl_calc_cdclk(int
> > > > min_cdclk,
> > > > unsigned int ref)
> > > >  		/* fall through */
> > > >  	case 24000:
> > > >  		ranges = ranges_24;
> > > > +		len = ARRAY_SIZE(ranges_24);
> > > >  		break;
> > > >  	case 19200:
> > > >  	case 38400:
> > > >  		ranges = ranges_19_38;
> > > > +		len = ARRAY_SIZE(ranges_19_38);
> > > >  		break;
> > > >  	}
> > > >  
> > > > -	if (min_cdclk > ranges[1])
> > > > -		return ranges[2];
> > > > -	else if (min_cdclk > ranges[0])
> > > > -		return ranges[1];
> > > > -	else
> > > > -		return ranges[0];
> > > > +	for (i = 0; i < len; i++) {
> > > > +		if (min_cdclk <= ranges[i])
> > > > +			return ranges[i];
> > > > +	}
> > > > +
> > > > +	WARN_ON(min_cdclk > ranges[len - 1]);
> > > > +	return ranges[len - 1];
> > > >  }
> > > >  
> > > >  static int icl_calc_cdclk_pll_vco(struct drm_i915_private
> > > > *dev_priv, int cdclk)
> > > > @@ -1792,16 +1796,23 @@ static int
> > > > icl_calc_cdclk_pll_vco(struct
> > > > drm_i915_private *dev_priv, int cdclk)
> > > >  	default:
> > > >  		MISSING_CASE(cdclk);
> > > >  		/* fall through */
> > > > +	case 172800:
> > > >  	case 307200:
> > > >  	case 556800:
> > > >  	case 652800:
> > > >  		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > >  			dev_priv->cdclk.hw.ref != 38400);
> > > >  		break;
> > > > +	case 180000:
> > > >  	case 312000:
> > > >  	case 552000:
> > > >  	case 648000:
> > > >  		WARN_ON(dev_priv->cdclk.hw.ref != 24000);
> > > > +		break;
> > > > +	case 192000:
> > > > +		WARN_ON(dev_priv->cdclk.hw.ref != 19200 &&
> > > > +			dev_priv->cdclk.hw.ref != 38400 &&
> > > > +			dev_priv->cdclk.hw.ref != 24000);
> > > >  	}
> > > >  
> > > >  	ratio = cdclk / (dev_priv->cdclk.hw.ref / 2);
> > > > -- 
> > > > 2.22.0
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2019-06-24 21:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-18 22:50 [PATCH 1/3] drm/i915/icl: Add new supported CD clocks José Roberto de Souza
2019-06-18 22:50 ` [PATCH 2/3] drm/i915/ehl: Remove unsupported cd clocks José Roberto de Souza
2019-06-19 11:40   ` Ville Syrjälä
2019-06-19 15:21     ` Jani Nikula
2019-06-20  0:37     ` Souza, Jose
2019-06-18 22:50 ` [PATCH 3/3] drm/i915/ehl: Add voltage level requirement table José Roberto de Souza
2019-06-19 11:43   ` Ville Syrjälä
2019-06-20  0:36     ` Souza, Jose
2019-06-20 10:01       ` Ville Syrjälä
2019-06-18 23:25 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks Patchwork
2019-06-18 23:44 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-06-19  7:43 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915/icl: Add new supported CD clocks (rev2) Patchwork
2019-06-19  8:09 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-06-19 11:48 ` [PATCH 1/3] drm/i915/icl: Add new supported CD clocks Ville Syrjälä
2019-06-20  0:36   ` Souza, Jose
2019-06-20 10:03     ` Ville Syrjälä
2019-06-19 17:47 ` Ville Syrjälä
2019-06-20 23:33   ` Souza, Jose
2019-06-24 12:39     ` Ville Syrjälä
2019-06-24 21:05       ` Souza, Jose

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.