[Intel-gfx] [PATCH 0/1] drm/i915/display: Communicate display configuration to pcode

[Intel-gfx] [PATCH 0/1] drm/i915/display: Communicate display configuration to pcode

[Jigar Bhatt]

Display to communicate "display configuration" to Pcode for more accurate
power accounting for DG2. Existing sequence is sending only voltage.
New sequence is sending cdclk along with number pipe of active pipes
to Pcode.

Jigar Bhatt (1):
  drm/i915/display: Communicate display configuration to pcode

 drivers/gpu/drm/i915/display/intel_cdclk.c   | 93 +++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_cdclk.h   |  2 +
 drivers/gpu/drm/i915/display/intel_display.c |  6 ++
 drivers/gpu/drm/i915/i915_reg.h              |  4 +
 4 files changed, 101 insertions(+), 4 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH RFC 1/1] drm/i915/display: Communicate display configuration to pcode

[Jigar Bhatt]

Display to communicate "display configuration" to Pcode for more accurate
power accounting for DG2. Existing sequence is only sending the voltage
value to the Pcode. Adding new sequence with current cdclk associate
with voltage value masking. Adding pcode request when any power well
will disable or enable.

Signed-off-by: Jigar Bhatt <jigar.bhatt@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c   | 93 +++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_cdclk.h   |  2 +
 drivers/gpu/drm/i915/display/intel_display.c |  6 ++
 drivers/gpu/drm/i915/i915_reg.h              |  4 +
 4 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index b2017d8161b4..ac134b1a28ff 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1701,12 +1701,12 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	int ret;
 
 	/* Inform power controller of upcoming frequency change. */
-	if (DISPLAY_VER(dev_priv) >= 11)
+	if (DISPLAY_VER(dev_priv) == 11)
 		ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
 					SKL_CDCLK_PREPARE_FOR_CHANGE,
 					SKL_CDCLK_READY_FOR_CHANGE,
 					SKL_CDCLK_READY_FOR_CHANGE, 3);
-	else
+	if (DISPLAY_VER(dev_priv) < 11) {
 		/*
 		 * BSpec requires us to wait up to 150usec, but that leads to
 		 * timeouts; the 2ms used here is based on experiment.
@@ -1714,6 +1714,7 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		ret = snb_pcode_write_timeout(dev_priv,
 					      HSW_PCODE_DE_WRITE_FREQ_REQ,
 					      0x80000000, 150, 2);
+	}
 	if (ret) {
 		drm_err(&dev_priv->drm,
 			"Failed to inform PCU about cdclk change (err %d, freq %d)\n",
@@ -1773,10 +1774,11 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	if (pipe != INVALID_PIPE)
 		intel_crtc_wait_for_next_vblank(intel_crtc_for_pipe(dev_priv, pipe));
 
-	if (DISPLAY_VER(dev_priv) >= 11) {
+	if (DISPLAY_VER(dev_priv) == 11) {
 		ret = snb_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
 				      cdclk_config->voltage_level);
-	} else {
+	}
+	if (DISPLAY_VER(dev_priv) < 11) {
 		/*
 		 * The timeout isn't specified, the 2ms used here is based on
 		 * experiment.
@@ -2062,6 +2064,34 @@ void intel_cdclk_dump_config(struct drm_i915_private *i915,
 		    cdclk_config->voltage_level);
 }
 
+/**
+ * intel_display_to_pcode- inform pcode for display config
+ * @cdclk: current cdclk as per new or old state
+ * @voltage_level: current voltage_level send to Pcode
+ * @active_pipes: active pipes for more accurate power accounting
+ */
+static void intel_display_to_pcode(struct drm_i915_private *dev_priv,
+					unsigned int cdclk, u8 voltage_level,
+					u8 active_pipes)
+{
+	int ret;
+
+	if (DISPLAY_VER(dev_priv) >= 12) {
+		ret = skl_pcode_request(dev_priv, SKL_PCODE_CDCLK_CONTROL,
+					SKL_CDCLK_PREPARE_FOR_CHANGE |
+					DISPLAY_TO_PCODE_MASK
+					(cdclk, active_pipes, voltage_level),
+					SKL_CDCLK_READY_FOR_CHANGE,
+					SKL_CDCLK_READY_FOR_CHANGE, 3);
+		if (ret) {
+			drm_err(&dev_priv->drm,
+					"Failed to inform PCU about display config (err %d)\n",
+					ret);
+			return;
+		}
+	}
+}
+
 /**
  * intel_set_cdclk - Push the CDCLK configuration to the hardware
  * @dev_priv: i915 device
@@ -2131,6 +2161,61 @@ static void intel_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 }
 
+/**
+ * intel_display_to_pcode_pre_notification: display to pcode notification
+ * before the enabling power wells.
+ * send notification with cdclk, number of pipes, voltage_level.
+ * @state: intel atomic state
+ */
+void intel_display_to_pcode_pre_notification(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_cdclk_state *old_cdclk_state =
+		intel_atomic_get_old_cdclk_state(state);
+	const struct intel_cdclk_state *new_cdclk_state =
+		intel_atomic_get_new_cdclk_state(state);
+	if (!intel_cdclk_changed(&old_cdclk_state->actual,
+				&new_cdclk_state->actual) &&
+				(new_cdclk_state->active_pipes ==
+				old_cdclk_state->active_pipes))
+		return;
+	if (old_cdclk_state->actual.cdclk <= new_cdclk_state->actual.cdclk) {
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,
+					new_cdclk_state->active_pipes);
+		return;
+	}
+	if (old_cdclk_state->actual.cdclk >= new_cdclk_state->actual.cdclk) {
+		intel_display_to_pcode(dev_priv, old_cdclk_state->actual.cdclk,
+					old_cdclk_state->actual.voltage_level,
+					old_cdclk_state->active_pipes);
+		return;
+	}
+	if (old_cdclk_state->active_pipes != new_cdclk_state->active_pipes) {
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,
+					new_cdclk_state->active_pipes);
+		return;
+	}
+	intel_display_to_pcode(dev_priv, DISPLAY_TO_PCODE_CDCLK_MAX,
+				new_cdclk_state->actual.voltage_level,
+				new_cdclk_state->active_pipes);
+}
+
+/*intel_display_to_pcode_post_notification: after frequency change sending
+ * voltage_level, active pipes, current CDCLK frequency.
+ * @state: intel atomic state
+ */
+void intel_display_to_pcode_post_notification(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_cdclk_state *new_cdclk_state =
+		intel_atomic_get_new_cdclk_state(state);
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,
+					new_cdclk_state->active_pipes);
+}
+
 /**
  * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
  * @state: intel atomic state
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index b535cf6a7d9e..18ccb4f16dbc 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -64,6 +64,8 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_config *a,
 			       const struct intel_cdclk_config *b);
 void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state);
 void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state);
+void intel_display_to_pcode_pre_notification(struct intel_atomic_state *state);
+void intel_display_to_pcode_post_notification(struct intel_atomic_state *state);
 void intel_cdclk_dump_config(struct drm_i915_private *i915,
 			     const struct intel_cdclk_config *cdclk_config,
 			     const char *context);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ca997a0a0517..436f04c69c65 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8518,6 +8518,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 
 	intel_atomic_prepare_plane_clear_colors(state);
 
+	if (IS_DG2(dev_priv))
+		intel_display_to_pcode_pre_notification(state);
+
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
 		if (intel_crtc_needs_modeset(new_crtc_state) ||
@@ -8542,6 +8545,9 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		intel_modeset_verify_disabled(dev_priv, state);
 	}
 
+	if (IS_DG2(dev_priv))
+		intel_display_to_pcode_post_notification(state);
+
 	intel_sagv_pre_plane_update(state);
 
 	/* Complete the events for pipes that have now been disabled */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fef71b242706..2ad8ef3d9010 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6717,6 +6717,10 @@
 #define     ICL_PCODE_MEM_SS_READ_GLOBAL_INFO	(0x0 << 8)
 #define     ICL_PCODE_MEM_SS_READ_QGV_POINT_INFO(point)	(((point) << 16) | (0x1 << 8))
 #define     ADL_PCODE_MEM_SS_READ_PSF_GV_INFO	((0) | (0x2 << 8))
+#define   DISPLAY_TO_PCODE_CDCLK_MAX		0x28D
+#define   DISPLAY_TO_PCODE_MASK(cdclk, num_pipes, voltage_level) \
+		((1 << 31) | (num_pipes << 28) | \
+		(cdclk << 16) | (1 << 27) | voltage_level)
 #define   ICL_PCODE_SAGV_DE_MEM_SS_CONFIG	0xe
 #define     ICL_PCODE_REP_QGV_MASK		REG_GENMASK(1, 0)
 #define     ICL_PCODE_REP_QGV_SAFE		REG_FIELD_PREP(ICL_PCODE_REP_QGV_MASK, 0)
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/display: Communicate display configuration to pcode

[Patchwork]

== Series Details ==

Series: drm/i915/display: Communicate display configuration to pcode
URL   : https://patchwork.freedesktop.org/series/102678/
State : warning

== Summary ==

Error: dim checkpatch failed
a4a6dfddbdd5 drm/i915/display: Communicate display configuration to pcode
-:66: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#66: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2074:
+static void intel_display_to_pcode(struct drm_i915_private *dev_priv,
+					unsigned int cdclk, u8 voltage_level,

-:80: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#80: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2088:
+			drm_err(&dev_priv->drm,
+					"Failed to inform PCU about display config (err %d)\n",

-:107: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'new_cdclk_state->active_pipes ==
 				old_cdclk_state->active_pipes'
#107: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2177:
+	if (!intel_cdclk_changed(&old_cdclk_state->actual,
+				&new_cdclk_state->actual) &&
+				(new_cdclk_state->active_pipes ==
+				old_cdclk_state->active_pipes))

-:108: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#108: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2178:
+	if (!intel_cdclk_changed(&old_cdclk_state->actual,
+				&new_cdclk_state->actual) &&

-:114: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#114: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2184:
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,

-:120: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#120: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2190:
+		intel_display_to_pcode(dev_priv, old_cdclk_state->actual.cdclk,
+					old_cdclk_state->actual.voltage_level,

-:126: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#126: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2196:
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,

-:131: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#131: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2201:
+	intel_display_to_pcode(dev_priv, DISPLAY_TO_PCODE_CDCLK_MAX,
+				new_cdclk_state->actual.voltage_level,

-:145: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#145: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2215:
+		intel_display_to_pcode(dev_priv, new_cdclk_state->actual.cdclk,
+					new_cdclk_state->actual.voltage_level,

-:198: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'cdclk' may be better as '(cdclk)' to avoid precedence issues
#198: FILE: drivers/gpu/drm/i915/i915_reg.h:6721:
+#define   DISPLAY_TO_PCODE_MASK(cdclk, num_pipes, voltage_level) \
+		((1 << 31) | (num_pipes << 28) | \
+		(cdclk << 16) | (1 << 27) | voltage_level)

-:198: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'num_pipes' may be better as '(num_pipes)' to avoid precedence issues
#198: FILE: drivers/gpu/drm/i915/i915_reg.h:6721:
+#define   DISPLAY_TO_PCODE_MASK(cdclk, num_pipes, voltage_level) \
+		((1 << 31) | (num_pipes << 28) | \
+		(cdclk << 16) | (1 << 27) | voltage_level)

-:198: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'voltage_level' may be better as '(voltage_level)' to avoid precedence issues
#198: FILE: drivers/gpu/drm/i915/i915_reg.h:6721:
+#define   DISPLAY_TO_PCODE_MASK(cdclk, num_pipes, voltage_level) \
+		((1 << 31) | (num_pipes << 28) | \
+		(cdclk << 16) | (1 << 27) | voltage_level)

total: 0 errors, 0 warnings, 12 checks, 165 lines checked

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/display: Communicate display configuration to pcode

[Patchwork]

== Series Details ==

Series: drm/i915/display: Communicate display configuration to pcode
URL   : https://patchwork.freedesktop.org/series/102678/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/display: Communicate display configuration to pcode

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5358 bytes --]

== Series Details ==

Series: drm/i915/display: Communicate display configuration to pcode
URL   : https://patchwork.freedesktop.org/series/102678/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11498 -> Patchwork_102678v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_102678v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_102678v1, 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_102678v1/index.html

Participating hosts (48 -> 45)
------------------------------

  Missing    (3): fi-hsw-4770 fi-bsw-cyan fi-bdw-samus 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3@lmem0:
    - bat-dg1-5:          [PASS][1] -> [DMESG-WARN][2] +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11498/bat-dg1-5/igt@gem_exec_suspend@basic-s3@lmem0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/bat-dg1-5/igt@gem_exec_suspend@basic-s3@lmem0.html

  * igt@runner@aborted:
    - fi-rkl-11600:       NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-rkl-11600/igt@runner@aborted.html
    - fi-rkl-guc:         NOTRUN -> [FAIL][4]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-rkl-guc/igt@runner@aborted.html
    - fi-adl-ddr5:        NOTRUN -> [FAIL][5]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-adl-ddr5/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-rpls-1}:       NOTRUN -> [DMESG-WARN][6]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html
    - {bat-adlp-6}:       [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11498/bat-adlp-6/igt@gem_exec_suspend@basic-s0@smem.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/bat-adlp-6/igt@gem_exec_suspend@basic-s0@smem.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][9] ([i915#4312])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-bdw-5557u/igt@runner@aborted.html
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][10] ([i915#5257])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-tgl-1115g4/igt@runner@aborted.html
    - fi-tgl-u2:          NOTRUN -> [FAIL][11] ([i915#5257])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-tgl-u2/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [DMESG-FAIL][12] ([i915#4528]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11498/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/fi-blb-e6850/igt@i915_selftest@live@requests.html

  
#### Warnings ####

  * igt@runner@aborted:
    - bat-dg1-5:          [FAIL][14] ([i915#4312]) -> [FAIL][15] ([i915#4312] / [i915#5257])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11498/bat-dg1-5/igt@runner@aborted.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/bat-dg1-5/igt@runner@aborted.html
    - bat-dg1-6:          [FAIL][16] ([i915#4312]) -> [FAIL][17] ([i915#5257])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11498/bat-dg1-6/igt@runner@aborted.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/bat-dg1-6/igt@runner@aborted.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5602]: https://gitlab.freedesktop.org/drm/intel/issues/5602


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

  * Linux: CI_DRM_11498 -> Patchwork_102678v1

  CI-20190529: 20190529
  CI_DRM_11498: 0fd740aed3bba981ac9bea34f31471d2e0e4ddad @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6420: a3885810ccc0ce9e6552a20c910a0a322eca466c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_102678v1: 0fd740aed3bba981ac9bea34f31471d2e0e4ddad @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2be96a98cab4 drm/i915/display: Communicate display configuration to pcode

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_102678v1/index.html

[-- Attachment #2: Type: text/html, Size: 6466 bytes --]