[Intel-gfx] [PATCH 0/4] Move CDCLK checks to atomic check phase

[Intel-gfx] [PATCH 0/4] Move CDCLK checks to atomic check phase

[Anusha Srivatsa]

The intention is to check for squashing, crawling conditions
at atomic check phase and prepare for commit phase. This basically
means the in-flight cdclk state is available. intel_cdclk_can_squash(),
intel_cdclk_can_crawl() and intel_cdclk_needs_modeset() have changes
to accommodate this.

Cc: Matt Roper <matthew.d.roper@intel.com>

Anusha Srivatsa (4):
  drm/i915/display: Add CDCLK actions to intel_cdclk_state
  drm/i915/display: s/intel_cdclk_can_squash/intel_cdclk_squash
  drm/i915/display: s/intel_cdclk_can_crawl/intel_cdclk_crawl
  drm/i915/display: Add cdclk checks to atomic check

 drivers/gpu/drm/i915/display/intel_cdclk.c | 151 ++++++++++++++-------
 drivers/gpu/drm/i915/display/intel_cdclk.h |  13 ++
 2 files changed, 112 insertions(+), 52 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Anusha Srivatsa]

This is a prep patch for what the rest of the series does.

Add existing actions that change cdclk - squash, crawl, modeset to
intel_cdclk_state so we have access to the cdclk values
that are in transition.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index b535cf6a7d9e..43835688ee02 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct drm_i915_private;
 struct intel_atomic_state;
 struct intel_crtc_state;
 
+enum cdclk_actions {
+	INTEL_CDCLK_MODESET = 0,
+	INTEL_CDCLK_SQUASH,
+	INTEL_CDCLK_CRAWL,
+	INTEL_CDCLK_NOOP,
+	MAX_CDCLK_ACTIONS
+};
+
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
@@ -51,6 +59,11 @@ struct intel_cdclk_state {
 
 	/* bitmask of active pipes */
 	u8 active_pipes;
+
+	struct cdclk_step {
+		enum cdclk_actions action;
+		u32 cdclk;
+	} steps[MAX_CDCLK_ACTIONS];
 };
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
-- 
2.25.1

[Intel-gfx] [PATCH 2/4] drm/i915/display: s/intel_cdclk_can_squash/intel_cdclk_squash

[Anusha Srivatsa]

Apart from checking if squashing can be performed,
accommodate accessing in-flight cdclk state for any changes
that are needed during commit phase.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 86a22c3766e5..4081b880a6ef 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1966,10 +1966,11 @@ static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv,
 		a->ref == b->ref;
 }
 
-static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
-				   const struct intel_cdclk_config *a,
-				   const struct intel_cdclk_config *b)
+static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
+			       const struct intel_cdclk_state *a,
+			       struct intel_cdclk_state *b)
 {
+
 	/*
 	 * FIXME should store a bit more state in intel_cdclk_config
 	 * to differentiate squasher vs. cd2x divider properly. For
@@ -1979,10 +1980,10 @@ static bool intel_cdclk_can_squash(struct drm_i915_private *dev_priv,
 	if (!has_cdclk_squasher(dev_priv))
 		return false;
 
-	return a->cdclk != b->cdclk &&
-		a->vco != 0 &&
-		a->vco == b->vco &&
-		a->ref == b->ref;
+	return a->actual.cdclk != b->actual.cdclk &&
+		a->actual.vco != 0 &&
+		a->actual.vco == b->actual.vco &&
+		a->actual.ref == b->actual.ref;
 }
 
 /**
@@ -2758,9 +2759,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			pipe = INVALID_PIPE;
 	}
 
-	if (intel_cdclk_can_squash(dev_priv,
-				   &old_cdclk_state->actual,
-				   &new_cdclk_state->actual)) {
+	if (intel_cdclk_squash(dev_priv,
+			       old_cdclk_state,
+			       new_cdclk_state)) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Can change cdclk via squasher\n");
 	} else if (intel_cdclk_can_crawl(dev_priv,
-- 
2.25.1

[Intel-gfx] [PATCH 3/4] drm/i915/display: s/intel_cdclk_can_crawl/intel_cdclk_crawl

[Anusha Srivatsa]

Apart from checking if crawling can be performed,
accommodate accessing in-flight cdclk state for any changes
that are needed during commit phase.

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 4081b880a6ef..cb6e419562dd 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1944,9 +1944,9 @@ void intel_cdclk_uninit_hw(struct drm_i915_private *i915)
 		skl_cdclk_uninit_hw(i915);
 }
 
-static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv,
-				  const struct intel_cdclk_config *a,
-				  const struct intel_cdclk_config *b)
+static bool intel_cdclk_crawl(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *a,
+				  struct intel_cdclk_state *b)
 {
 	int a_div, b_div;
 
@@ -1957,13 +1957,13 @@ static bool intel_cdclk_can_crawl(struct drm_i915_private *dev_priv,
 	 * The vco and cd2x divider will change independently
 	 * from each, so we disallow cd2x change when crawling.
 	 */
-	a_div = DIV_ROUND_CLOSEST(a->vco, a->cdclk);
-	b_div = DIV_ROUND_CLOSEST(b->vco, b->cdclk);
+	a_div = DIV_ROUND_CLOSEST(a->actual.vco, a->actual.cdclk);
+	b_div = DIV_ROUND_CLOSEST(b->actual.vco, b->actual.cdclk);
 
-	return a->vco != 0 && b->vco != 0 &&
-		a->vco != b->vco &&
+	return a->actual.vco != 0 && b->actual.vco != 0 &&
+		a->actual.vco != b->actual.vco &&
 		a_div == b_div &&
-		a->ref == b->ref;
+		a->actual.ref == b->actual.ref;
 }
 
 static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
@@ -2764,9 +2764,9 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			       new_cdclk_state)) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Can change cdclk via squasher\n");
-	} else if (intel_cdclk_can_crawl(dev_priv,
-					 &old_cdclk_state->actual,
-					 &new_cdclk_state->actual)) {
+	} else if (intel_cdclk_crawl(dev_priv,
+				     old_cdclk_state,
+				     new_cdclk_state)) {
 		drm_dbg_kms(&dev_priv->drm,
 			    "Can change cdclk via crawl\n");
 	} else if (pipe != INVALID_PIPE) {
-- 
2.25.1

[Intel-gfx] [PATCH 4/4] drm/i915/display: Add cdclk checks to atomic check

[Anusha Srivatsa]

Checking cdclk conditions during atomic check and preparing
for commit phase so we can have atomic commit as simple
as possible. Add the specific steps to be taken during
cdclk changes, prepare for squashing, crawling and modeset
scenarios.

v2: Add intel_cdclk_modeset() similar to intel_cdclk_squash()
and intel_cdclk_crawl().

Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 108 +++++++++++++++------
 1 file changed, 77 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index cb6e419562dd..2efc1f09abab 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1693,12 +1693,23 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 			  const struct intel_cdclk_config *cdclk_config,
 			  enum pipe pipe)
 {
+	struct intel_atomic_state *state;
+	struct intel_cdclk_state *new_cdclk_state;
+	struct cdclk_step *cdclk_steps;
+	struct intel_cdclk_state *cdclk_state;
 	int cdclk = cdclk_config->cdclk;
 	int vco = cdclk_config->vco;
+	u32 squash_ctl = 0;
 	u32 val;
 	u16 waveform;
 	int clock;
 	int ret;
+	int i;
+
+	cdclk_state =  to_intel_cdclk_state(dev_priv->cdclk.obj.state);
+	state = cdclk_state->base.state;
+	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
+	cdclk_steps = new_cdclk_state->steps;
 
 	/* Inform power controller of upcoming frequency change. */
 	if (DISPLAY_VER(dev_priv) >= 11)
@@ -1721,40 +1732,42 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		return;
 	}
 
-	if (HAS_CDCLK_CRAWL(dev_priv) && dev_priv->cdclk.hw.vco > 0 && vco > 0) {
-		if (dev_priv->cdclk.hw.vco != vco)
+	for (i = 0; i < MAX_CDCLK_ACTIONS; i++) {
+		switch (cdclk_steps[i].action) {
+		case INTEL_CDCLK_MODESET:
+			if (DISPLAY_VER(dev_priv) >= 11) {
+				if (dev_priv->cdclk.hw.vco != 0 &&
+				    dev_priv->cdclk.hw.vco != vco)
+					icl_cdclk_pll_disable(dev_priv);
+
+				if (dev_priv->cdclk.hw.vco != vco)
+					icl_cdclk_pll_enable(dev_priv, vco);
+			} else {
+				if (dev_priv->cdclk.hw.vco != 0 &&
+				    dev_priv->cdclk.hw.vco != vco)
+					bxt_de_pll_disable(dev_priv);
+
+				if (dev_priv->cdclk.hw.vco != vco)
+					bxt_de_pll_enable(dev_priv, vco);
+			}
+			clock = cdclk;
+			break;
+		case INTEL_CDCLK_CRAWL:
 			adlp_cdclk_pll_crawl(dev_priv, vco);
-	} else if (DISPLAY_VER(dev_priv) >= 11) {
-		if (dev_priv->cdclk.hw.vco != 0 &&
-		    dev_priv->cdclk.hw.vco != vco)
-			icl_cdclk_pll_disable(dev_priv);
-
-		if (dev_priv->cdclk.hw.vco != vco)
-			icl_cdclk_pll_enable(dev_priv, vco);
-	} else {
-		if (dev_priv->cdclk.hw.vco != 0 &&
-		    dev_priv->cdclk.hw.vco != vco)
-			bxt_de_pll_disable(dev_priv);
-
-		if (dev_priv->cdclk.hw.vco != vco)
-			bxt_de_pll_enable(dev_priv, vco);
-	}
-
-	waveform = cdclk_squash_waveform(dev_priv, cdclk);
-
-	if (waveform)
-		clock = vco / 2;
-	else
-		clock = cdclk;
-
-	if (has_cdclk_squasher(dev_priv)) {
-		u32 squash_ctl = 0;
-
-		if (waveform)
+			clock = cdclk;
+			break;
+		case INTEL_CDCLK_SQUASH:
+			waveform =  cdclk_squash_waveform(dev_priv, cdclk_steps[i].cdclk);
+			clock = vco / 2;
 			squash_ctl = CDCLK_SQUASH_ENABLE |
 				CDCLK_SQUASH_WINDOW_SIZE(0xf) | waveform;
-
-		intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
+			intel_de_write(dev_priv, CDCLK_SQUASH_CTL, squash_ctl);
+			break;
+		case INTEL_CDCLK_NOOP:
+			break;
+		default:
+			break;
+		}
 	}
 
 	val = bxt_cdclk_cd2x_div_sel(dev_priv, clock, vco) |
@@ -1949,6 +1962,7 @@ static bool intel_cdclk_crawl(struct drm_i915_private *dev_priv,
 				  struct intel_cdclk_state *b)
 {
 	int a_div, b_div;
+	struct cdclk_step *cdclk_transition = b->steps;
 
 	if (!HAS_CDCLK_CRAWL(dev_priv))
 		return false;
@@ -1960,6 +1974,11 @@ static bool intel_cdclk_crawl(struct drm_i915_private *dev_priv,
 	a_div = DIV_ROUND_CLOSEST(a->actual.vco, a->actual.cdclk);
 	b_div = DIV_ROUND_CLOSEST(b->actual.vco, b->actual.cdclk);
 
+	cdclk_transition[0].action = INTEL_CDCLK_CRAWL;
+	cdclk_transition[0].cdclk = b->actual.cdclk;
+	cdclk_transition[1].action = INTEL_CDCLK_NOOP;
+	cdclk_transition[1].cdclk = b->actual.cdclk;
+
 	return a->actual.vco != 0 && b->actual.vco != 0 &&
 		a->actual.vco != b->actual.vco &&
 		a_div == b_div &&
@@ -1971,6 +1990,7 @@ static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
 			       struct intel_cdclk_state *b)
 {
 
+	struct cdclk_step *cdclk_transition = b->steps;
 	/*
 	 * FIXME should store a bit more state in intel_cdclk_config
 	 * to differentiate squasher vs. cd2x divider properly. For
@@ -1980,12 +2000,35 @@ static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
 	if (!has_cdclk_squasher(dev_priv))
 		return false;
 
+	cdclk_transition[0].action = INTEL_CDCLK_SQUASH;
+	cdclk_transition[0].cdclk = b->actual.cdclk;
+	cdclk_transition[1].action = INTEL_CDCLK_NOOP;
+	cdclk_transition[1].cdclk = b->actual.cdclk;
+
 	return a->actual.cdclk != b->actual.cdclk &&
 		a->actual.vco != 0 &&
 		a->actual.vco == b->actual.vco &&
 		a->actual.ref == b->actual.ref;
 }
 
+static void intel_cdclk_modeset(struct drm_i915_private *i915,
+				const struct intel_cdclk_config *a,
+				const struct intel_cdclk_config *b)
+{
+	struct intel_cdclk_state *new_cdclk_state;
+	struct cdclk_step *cdclk_transition;
+	struct intel_cdclk_state *cdclk_state =  to_intel_cdclk_state(i915->cdclk.obj.state);
+	struct intel_atomic_state *state = cdclk_state->base.state;
+
+	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
+	cdclk_transition = new_cdclk_state->steps;
+
+	cdclk_transition[0].action = INTEL_CDCLK_MODESET;
+	cdclk_transition[0].cdclk = b->cdclk;
+	cdclk_transition[1].action = INTEL_CDCLK_NOOP;
+	cdclk_transition[1].cdclk = b->cdclk;
+}
+
 /**
  * intel_cdclk_needs_modeset - Determine if changong between the CDCLK
  *                             configurations requires a modeset on all pipes
@@ -1999,6 +2042,7 @@ static bool intel_cdclk_squash(struct drm_i915_private *dev_priv,
 bool intel_cdclk_needs_modeset(const struct intel_cdclk_config *a,
 			       const struct intel_cdclk_config *b)
 {
+
 	return a->cdclk != b->cdclk ||
 		a->vco != b->vco ||
 		a->ref != b->ref;
@@ -2777,6 +2821,8 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			    pipe_name(pipe));
 	} else if (intel_cdclk_needs_modeset(&old_cdclk_state->actual,
 					     &new_cdclk_state->actual)) {
+		intel_cdclk_modeset(dev_priv, &old_cdclk_state->actual,
+				    &new_cdclk_state->actual);
 		/* All pipes must be switched off while we change the cdclk. */
 		ret = intel_modeset_all_pipes(state);
 		if (ret)
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Move CDCLK checks to atomic check phase

[Patchwork]

== Series Details ==

Series: Move CDCLK checks to atomic check phase
URL   : https://patchwork.freedesktop.org/series/106782/
State : warning

== Summary ==

Error: dim checkpatch failed
b31629d56cc7 drm/i915/display: Add CDCLK actions to intel_cdclk_state
a77796ff5290 drm/i915/display: s/intel_cdclk_can_squash/intel_cdclk_squash
-:28: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#28: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1973:
 {
+

total: 0 errors, 0 warnings, 1 checks, 40 lines checked
a462b1685a1a drm/i915/display: s/intel_cdclk_can_crawl/intel_cdclk_crawl
-:25: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#25: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:1948:
+static bool intel_cdclk_crawl(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *a,

total: 0 errors, 0 warnings, 1 checks, 42 lines checked
18d9d117a9a9 drm/i915/display: Add cdclk checks to atomic check
-:188: CHECK:BRACES: Blank lines aren't necessary after an open brace '{'
#188: FILE: drivers/gpu/drm/i915/display/intel_cdclk.c:2045:
 {
+

total: 0 errors, 0 warnings, 1 checks, 171 lines checked

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Move CDCLK checks to atomic check phase

[Patchwork]

== Series Details ==

Series: Move CDCLK checks to atomic check phase
URL   : https://patchwork.freedesktop.org/series/106782/
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 Move CDCLK checks to atomic check phase

[Patchwork]

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

== Series Details ==

Series: Move CDCLK checks to atomic check phase
URL   : https://patchwork.freedesktop.org/series/106782/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11946 -> Patchwork_106782v1
====================================================

Summary
-------

  **FAILURE**

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

Participating hosts (38 -> 36)
------------------------------

  Additional (4): fi-hsw-4770 bat-adls-5 bat-jsl-1 bat-adlp-4 
  Missing    (6): fi-tgl-dsi fi-bsw-n3050 fi-glk-dsi fi-glk-j4005 fi-bsw-kefka fi-bdw-samus 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - fi-rkl-11600:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-rkl-11600/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-rkl-11600/igt@i915_module_load@load.html
    - fi-rkl-guc:         [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-rkl-guc/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-rkl-guc/igt@i915_module_load@load.html
    - fi-skl-guc:         [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-skl-guc/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-guc/igt@i915_module_load@load.html
    - fi-skl-6700k2:      [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-skl-6700k2/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-6700k2/igt@i915_module_load@load.html
    - fi-cfl-8700k:       [PASS][9] -> [INCOMPLETE][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-cfl-8700k/igt@i915_module_load@load.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-8700k/igt@i915_module_load@load.html
    - fi-adl-ddr5:        [PASS][11] -> [INCOMPLETE][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-adl-ddr5/igt@i915_module_load@load.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-adl-ddr5/igt@i915_module_load@load.html
    - fi-cfl-guc:         [PASS][13] -> [INCOMPLETE][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-cfl-guc/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-guc/igt@i915_module_load@load.html
    - fi-bdw-5557u:       [PASS][15] -> [INCOMPLETE][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-bdw-5557u/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-bdw-5557u/igt@i915_module_load@load.html
    - fi-cfl-8109u:       [PASS][17] -> [INCOMPLETE][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-cfl-8109u/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-8109u/igt@i915_module_load@load.html

  * igt@kms_busy@basic@flip:
    - fi-skl-6600u:       [PASS][19] -> [INCOMPLETE][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-skl-6600u/igt@kms_busy@basic@flip.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-6600u/igt@kms_busy@basic@flip.html

  * igt@kms_force_connector_basic@force-connector-state:
    - bat-dg1-5:          [PASS][21] -> [DMESG-WARN][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-dg1-5/igt@kms_force_connector_basic@force-connector-state.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-dg1-5/igt@kms_force_connector_basic@force-connector-state.html

  
#### Suppressed ####

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

  * igt@i915_module_load@load:
    - {bat-jsl-1}:        NOTRUN -> [INCOMPLETE][23]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-jsl-1/igt@i915_module_load@load.html
    - {bat-rpls-1}:       [PASS][24] -> [INCOMPLETE][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-rpls-1/igt@i915_module_load@load.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-rpls-1/igt@i915_module_load@load.html
    - {bat-jsl-3}:        [PASS][26] -> [INCOMPLETE][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-jsl-3/igt@i915_module_load@load.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-jsl-3/igt@i915_module_load@load.html
    - {bat-dg2-9}:        [PASS][28] -> [INCOMPLETE][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-dg2-9/igt@i915_module_load@load.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-dg2-9/igt@i915_module_load@load.html
    - {bat-rpls-2}:       [PASS][30] -> [INCOMPLETE][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-rpls-2/igt@i915_module_load@load.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-rpls-2/igt@i915_module_load@load.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - {fi-jsl-1}:         [PASS][32] -> [INCOMPLETE][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-jsl-1/igt@i915_pm_rpm@basic-pci-d3-state.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-jsl-1/igt@i915_pm_rpm@basic-pci-d3-state.html
    - {fi-ehl-2}:         [PASS][34] -> [INCOMPLETE][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-ehl-2/igt@i915_pm_rpm@basic-pci-d3-state.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-ehl-2/igt@i915_pm_rpm@basic-pci-d3-state.html
    - {bat-rplp-1}:       [PASS][36] -> [INCOMPLETE][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-rplp-1/igt@i915_pm_rpm@basic-pci-d3-state.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-rplp-1/igt@i915_pm_rpm@basic-pci-d3-state.html
    - {bat-adls-5}:       NOTRUN -> [INCOMPLETE][38]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adls-5/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_busy@basic@flip:
    - {bat-adlp-6}:       [PASS][39] -> [INCOMPLETE][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-adlp-6/igt@kms_busy@basic@flip.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adlp-6/igt@kms_busy@basic@flip.html

  * igt@kms_force_connector_basic@force-connector-state:
    - {bat-adlm-1}:       [PASS][41] -> [DMESG-WARN][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-adlm-1/igt@kms_force_connector_basic@force-connector-state.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adlm-1/igt@kms_force_connector_basic@force-connector-state.html
    - {bat-dg2-8}:        [PASS][43] -> [DMESG-WARN][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-dg2-8/igt@kms_force_connector_basic@force-connector-state.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-dg2-8/igt@kms_force_connector_basic@force-connector-state.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0@smem:
    - fi-bsw-nick:        [PASS][45] -> [INCOMPLETE][46] ([i915#4831])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-bsw-nick/igt@gem_exec_suspend@basic-s0@smem.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-bsw-nick/igt@gem_exec_suspend@basic-s0@smem.html

  * igt@gem_tiled_pread_basic:
    - bat-adlp-4:         NOTRUN -> [SKIP][47] ([i915#3282])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adlp-4/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][48] ([fdo#109271] / [i915#3012])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

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

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][51] ([fdo#109271]) +9 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_busy@basic@flip:
    - bat-adlp-4:         NOTRUN -> [INCOMPLETE][52] ([i915#1982])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adlp-4/igt@kms_busy@basic@flip.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][53] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-hsw-4770:        NOTRUN -> [SKIP][54] ([fdo#109271] / [i915#1072]) +3 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  * igt@runner@aborted:
    - fi-skl-6700k2:      NOTRUN -> [FAIL][55] ([i915#4312])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-6700k2/igt@runner@aborted.html
    - fi-cfl-8109u:       NOTRUN -> [FAIL][56] ([i915#4312])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-8109u/igt@runner@aborted.html
    - bat-adlp-4:         NOTRUN -> [FAIL][57] ([i915#4312])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-adlp-4/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][58] ([fdo#109271] / [i915#2403] / [i915#4312])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-blb-e6850/igt@runner@aborted.html
    - fi-skl-6600u:       NOTRUN -> [FAIL][59] ([i915#4312])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-6600u/igt@runner@aborted.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][60] ([i915#4312])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-bdw-5557u/igt@runner@aborted.html
    - fi-rkl-guc:         NOTRUN -> [FAIL][61] ([i915#4312])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-rkl-guc/igt@runner@aborted.html
    - fi-skl-guc:         NOTRUN -> [FAIL][62] ([i915#4312])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-skl-guc/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][63] ([i915#4312])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-8700k/igt@runner@aborted.html
    - fi-rkl-11600:       NOTRUN -> [FAIL][64] ([i915#4312])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-rkl-11600/igt@runner@aborted.html
    - fi-adl-ddr5:        NOTRUN -> [FAIL][65] ([i915#4312])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-adl-ddr5/igt@runner@aborted.html
    - fi-cfl-guc:         NOTRUN -> [FAIL][66] ([i915#4312])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-cfl-guc/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@debugfs_test@read_all_entries:
    - fi-kbl-guc:         [FAIL][67] ([i915#6253]) -> [PASS][68]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/fi-kbl-guc/igt@debugfs_test@read_all_entries.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/fi-kbl-guc/igt@debugfs_test@read_all_entries.html

  
#### Warnings ####

  * igt@runner@aborted:
    - bat-dg1-5:          [FAIL][69] ([i915#4312] / [i915#5257]) -> [FAIL][70] ([i915#4312])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11946/bat-dg1-5/igt@runner@aborted.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_106782v1/bat-dg1-5/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).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4831]: https://gitlab.freedesktop.org/drm/intel/issues/4831
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#6253]: https://gitlab.freedesktop.org/drm/intel/issues/6253


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

  * Linux: CI_DRM_11946 -> Patchwork_106782v1

  CI-20190529: 20190529
  CI_DRM_11946: 0e9c43d76a145712da46e935d429ce2a3eea80e8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6598: 97e103419021d0863db527e3f2cf39ccdd132db5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_106782v1: 0e9c43d76a145712da46e935d429ce2a3eea80e8 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

dcaadb67602d drm/i915/display: Add cdclk checks to atomic check
f20288cb214a drm/i915/display: s/intel_cdclk_can_crawl/intel_cdclk_crawl
eb3e7380a58b drm/i915/display: s/intel_cdclk_can_squash/intel_cdclk_squash
314bc423aff2 drm/i915/display: Add CDCLK actions to intel_cdclk_state

== Logs ==

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

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

Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Ville Syrjälä]

On Wed, Sep 14, 2022 at 09:58:59PM +0000, Srivatsa, Anusha wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Wednesday, September 14, 2022 2:43 PM
> > To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to
> > intel_cdclk_state
> > 
> > On Thu, Sep 15, 2022 at 12:22:53AM +0300, Ville Syrjälä wrote:
> > > On Fri, Aug 19, 2022 at 05:58:19PM -0700, Anusha Srivatsa wrote:
> > > > This is a prep patch for what the rest of the series does.
> > > >
> > > > Add existing actions that change cdclk - squash, crawl, modeset to
> > > > intel_cdclk_state so we have access to the cdclk values that are in
> > > > transition.
> > > >
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
> > > >  1 file changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > > b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > > index b535cf6a7d9e..43835688ee02 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > > @@ -15,6 +15,14 @@ struct drm_i915_private;  struct
> > > > intel_atomic_state;  struct intel_crtc_state;
> > > >
> > > > +enum cdclk_actions {
> > > > +	INTEL_CDCLK_MODESET = 0,
> > > > +	INTEL_CDCLK_SQUASH,
> > > > +	INTEL_CDCLK_CRAWL,
> > > > +	INTEL_CDCLK_NOOP,
> > > > +	MAX_CDCLK_ACTIONS
> > > > +};
> > >
> > > This whole actions thing feels overly complicated to me.
> > > I think we should only need something like this:
> > >
> > > if (new.squash > old.squash) {
> > > 	mid.vco = old.vco;
> > > 	mid.squash = new.squash;
> > > } else {
> > > 	mid.vco = new.vco;
> > > 	mid.squash = old.squash;
> > > }
> > > /*
> > >  * bunch of asserts here to make sure
> > >  * the mid state looks sane.
> > >  */
> > > set_cdclk(mid);
> > > set_cdclk(new);
> > >
> > > And perhaps the current set_cdclk needs to get chunked up into smaller
> > > pieces so we don't do all the pre/post stuff more than once
> > > needlessly.
> > 
> > One idea might be to pass just a pair of flags to set_cdclk() whether to skip
> > the pre/post steps.
> 
> This is all considering that the new struct cdclk_step is embedded in cdclk_config and not cdclk_state. I am not understanding why cdclk-state is not accessible from bxt_set_cdclk.

.set_cdclk() is lower level than that. It must be able to operate outside
atomic commits (eg. during display core init/uninit), and thus must be
able to do its work purely based on the passed in cdclk_config (which is
the lower level state structure, a pair of which are embedded inside the
atomic intel_cdclk_state).

> What if I add cdclk_state to the dev_priv? bxt_set_cdclk() anyway has dev_priv. 

We definitely don't want to hack around the core ideas
of how the atomic machinery works. That way leads
madness because no one will be able to understand how
anything works.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Srivatsa, Anusha]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, September 14, 2022 2:43 PM
> To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to
> intel_cdclk_state
> 
> On Thu, Sep 15, 2022 at 12:22:53AM +0300, Ville Syrjälä wrote:
> > On Fri, Aug 19, 2022 at 05:58:19PM -0700, Anusha Srivatsa wrote:
> > > This is a prep patch for what the rest of the series does.
> > >
> > > Add existing actions that change cdclk - squash, crawl, modeset to
> > > intel_cdclk_state so we have access to the cdclk values that are in
> > > transition.
> > >
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > index b535cf6a7d9e..43835688ee02 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > > @@ -15,6 +15,14 @@ struct drm_i915_private;  struct
> > > intel_atomic_state;  struct intel_crtc_state;
> > >
> > > +enum cdclk_actions {
> > > +	INTEL_CDCLK_MODESET = 0,
> > > +	INTEL_CDCLK_SQUASH,
> > > +	INTEL_CDCLK_CRAWL,
> > > +	INTEL_CDCLK_NOOP,
> > > +	MAX_CDCLK_ACTIONS
> > > +};
> >
> > This whole actions thing feels overly complicated to me.
> > I think we should only need something like this:
> >
> > if (new.squash > old.squash) {
> > 	mid.vco = old.vco;
> > 	mid.squash = new.squash;
> > } else {
> > 	mid.vco = new.vco;
> > 	mid.squash = old.squash;
> > }
> > /*
> >  * bunch of asserts here to make sure
> >  * the mid state looks sane.
> >  */
> > set_cdclk(mid);
> > set_cdclk(new);
> >
> > And perhaps the current set_cdclk needs to get chunked up into smaller
> > pieces so we don't do all the pre/post stuff more than once
> > needlessly.
> 
> One idea might be to pass just a pair of flags to set_cdclk() whether to skip
> the pre/post steps.

This is all considering that the new struct cdclk_step is embedded in cdclk_config and not cdclk_state. I am not understanding why cdclk-state is not accessible from bxt_set_cdclk. 
What if I add cdclk_state to the dev_priv? bxt_set_cdclk() anyway has dev_priv. 

Anusha 
> --
> Ville Syrjälä
> Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Ville Syrjälä]

On Thu, Sep 15, 2022 at 12:22:53AM +0300, Ville Syrjälä wrote:
> On Fri, Aug 19, 2022 at 05:58:19PM -0700, Anusha Srivatsa wrote:
> > This is a prep patch for what the rest of the series does.
> > 
> > Add existing actions that change cdclk - squash, crawl, modeset to
> > intel_cdclk_state so we have access to the cdclk values
> > that are in transition.
> > 
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > index b535cf6a7d9e..43835688ee02 100644
> > --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> > @@ -15,6 +15,14 @@ struct drm_i915_private;
> >  struct intel_atomic_state;
> >  struct intel_crtc_state;
> >  
> > +enum cdclk_actions {
> > +	INTEL_CDCLK_MODESET = 0,
> > +	INTEL_CDCLK_SQUASH,
> > +	INTEL_CDCLK_CRAWL,
> > +	INTEL_CDCLK_NOOP,
> > +	MAX_CDCLK_ACTIONS
> > +};
> 
> This whole actions thing feels overly complicated to me.
> I think we should only need something like this:
> 
> if (new.squash > old.squash) {
> 	mid.vco = old.vco;
> 	mid.squash = new.squash;
> } else {
> 	mid.vco = new.vco;
> 	mid.squash = old.squash;
> }
> /*
>  * bunch of asserts here to make sure
>  * the mid state looks sane.
>  */
> set_cdclk(mid);
> set_cdclk(new);
> 
> And perhaps the current set_cdclk needs to get chunked up
> into smaller pieces so we don't do all the pre/post stuff
> more than once needlessly.

One idea might be to pass just a pair of flags to set_cdclk()
whether to skip the pre/post steps.

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Ville Syrjälä]

On Fri, Aug 19, 2022 at 05:58:19PM -0700, Anusha Srivatsa wrote:
> This is a prep patch for what the rest of the series does.
> 
> Add existing actions that change cdclk - squash, crawl, modeset to
> intel_cdclk_state so we have access to the cdclk values
> that are in transition.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index b535cf6a7d9e..43835688ee02 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -15,6 +15,14 @@ struct drm_i915_private;
>  struct intel_atomic_state;
>  struct intel_crtc_state;
>  
> +enum cdclk_actions {
> +	INTEL_CDCLK_MODESET = 0,
> +	INTEL_CDCLK_SQUASH,
> +	INTEL_CDCLK_CRAWL,
> +	INTEL_CDCLK_NOOP,
> +	MAX_CDCLK_ACTIONS
> +};

This whole actions thing feels overly complicated to me.
I think we should only need something like this:

if (new.squash > old.squash) {
	mid.vco = old.vco;
	mid.squash = new.squash;
} else {
	mid.vco = new.vco;
	mid.squash = old.squash;
}
/*
 * bunch of asserts here to make sure
 * the mid state looks sane.
 */
set_cdclk(mid);
set_cdclk(new);

And perhaps the current set_cdclk needs to get chunked up
into smaller pieces so we don't do all the pre/post stuff
more than once needlessly.

> +
>  struct intel_cdclk_config {
>  	unsigned int cdclk, vco, ref, bypass;
>  	u8 voltage_level;
> @@ -51,6 +59,11 @@ struct intel_cdclk_state {
>  
>  	/* bitmask of active pipes */
>  	u8 active_pipes;
> +
> +	struct cdclk_step {
> +		enum cdclk_actions action;
> +		u32 cdclk;
> +	} steps[MAX_CDCLK_ACTIONS];
>  };
>  
>  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
> -- 
> 2.25.1

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Navare, Manasi]

On Fri, Aug 19, 2022 at 05:58:19PM -0700, Anusha Srivatsa wrote:
> This is a prep patch for what the rest of the series does.
> 
> Add existing actions that change cdclk - squash, crawl, modeset to
> intel_cdclk_state so we have access to the cdclk values
> that are in transition.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index b535cf6a7d9e..43835688ee02 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -15,6 +15,14 @@ struct drm_i915_private;
>  struct intel_atomic_state;
>  struct intel_crtc_state;
>  
> +enum cdclk_actions {
> +	INTEL_CDCLK_MODESET = 0,
> +	INTEL_CDCLK_SQUASH,
> +	INTEL_CDCLK_CRAWL,
> +	INTEL_CDCLK_NOOP,
> +	MAX_CDCLK_ACTIONS
> +};
> +
>  struct intel_cdclk_config {
>  	unsigned int cdclk, vco, ref, bypass;
>  	u8 voltage_level;
> @@ -51,6 +59,11 @@ struct intel_cdclk_state {
>  
>  	/* bitmask of active pipes */
>  	u8 active_pipes;
> +
> +	struct cdclk_step {
> +		enum cdclk_actions action;
> +		u32 cdclk;
> +	} steps[MAX_CDCLK_ACTIONS];

If this is what you need to access later in bxt_set_cdclk , you needto
add this to intel_cdclk_config which is then part of cdclk_state and
that is what will get programmed in atomic_check and it gets sent to
bxt_set_cdclk in atomic_commit_tail.

This is the way ypu can access it in bxt_set_cdclk, you cannot access
the new_cdclk_state there, you need to use cdclk_config that is already
getting passed to it

Manasi

>  };
>  
>  int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
> -- 
> 2.25.1
> 

[Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Anusha Srivatsa]

This is a prep patch for what the rest of the series does.

Add existing actions that change cdclk - squash, crawl, modeset to
intel_cdclk_state so we have access to the cdclk values
that are in transition.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index b535cf6a7d9e..43835688ee02 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct drm_i915_private;
 struct intel_atomic_state;
 struct intel_crtc_state;
 
+enum cdclk_actions {
+	INTEL_CDCLK_MODESET = 0,
+	INTEL_CDCLK_SQUASH,
+	INTEL_CDCLK_CRAWL,
+	INTEL_CDCLK_NOOP,
+	MAX_CDCLK_ACTIONS
+};
+
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
@@ -51,6 +59,11 @@ struct intel_cdclk_state {
 
 	/* bitmask of active pipes */
 	u8 active_pipes;
+
+	struct cdclk_step {
+		enum cdclk_actions action;
+		u32 cdclk;
+	} steps[MAX_CDCLK_ACTIONS];
 };
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
-- 
2.25.1

[Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Anusha Srivatsa]

This is a prep patch for what the rest of the series does.

Add existing actions that change cdclk - squash, crawl, modeset to
intel_cdclk_state so we have access to the cdclk values
that are in transition.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index df66f66fbad0..06d7f9f0b253 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct drm_i915_private;
 struct intel_atomic_state;
 struct intel_crtc_state;
 
+enum cdclk_actions {
+	INTEL_CDCLK_MODESET = 0,
+	INTEL_CDCLK_SQUASH,
+	INTEL_CDCLK_CRAWL,
+	INTEL_CDCLK_NOOP,
+	MAX_CDCLK_ACTIONS
+};
+
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
@@ -49,6 +57,11 @@ struct intel_cdclk_state {
 
 	/* bitmask of active pipes */
 	u8 active_pipes;
+
+	struct cdclk_step {
+		enum cdclk_actions action;
+		u32 cdclk;
+	} steps[MAX_CDCLK_ACTIONS];
 };
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
-- 
2.25.1

[Intel-gfx] [PATCH 1/4] drm/i915/display: Add CDCLK actions to intel_cdclk_state

[Anusha Srivatsa]

This is a prep patch for what the rest of the series does.

Add existing actions that change cdclk - squash, crawl, modeset to
intel_cdclk_state so we have access to the cdclk values
that are in transition.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index df66f66fbad0..06d7f9f0b253 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -15,6 +15,14 @@ struct drm_i915_private;
 struct intel_atomic_state;
 struct intel_crtc_state;
 
+enum cdclk_actions {
+	INTEL_CDCLK_MODESET = 0,
+	INTEL_CDCLK_SQUASH,
+	INTEL_CDCLK_CRAWL,
+	INTEL_CDCLK_NOOP,
+	MAX_CDCLK_ACTIONS
+};
+
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
@@ -49,6 +57,11 @@ struct intel_cdclk_state {
 
 	/* bitmask of active pipes */
 	u8 active_pipes;
+
+	struct cdclk_step {
+		enum cdclk_actions action;
+		u32 cdclk;
+	} steps[MAX_CDCLK_ACTIONS];
 };
 
 int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state);
-- 
2.25.1