[CI 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[CI 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Imre Deak]

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

CDCLK has to be at least twice the BLCK regardless of audio. Audio
driver has to probe using this hook and increase the clock even in
absence of any display.

v2: Use atomic refcount for get_power, put_power so that we can
    call each once(Abhay).
v3: Reset power well 2 to avoid any transaction on iDisp link
    during cdclk change(Abhay).
v4: Remove Power well 2 reset workaround(Ville).
v5: Remove unwanted Power well 2 register defined in v4(Abhay).
v6:
- Use a dedicated flag instead of state->modeset for min CDCLK changes
- Make get/put audio power domain symmetric
- Rebased on top of intel_wakeref tracking changes.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  3 ++
 drivers/gpu/drm/i915/intel_audio.c   | 64 ++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_cdclk.c   | 30 ++++++-----------
 drivers/gpu/drm/i915/intel_display.c |  9 ++++-
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++
 5 files changed, 86 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c65c2e6649df..6b10cee4e77f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1624,6 +1624,8 @@ struct drm_i915_private {
 		struct intel_cdclk_state actual;
 		/* The current hardware cdclk state */
 		struct intel_cdclk_state hw;
+
+		int force_min_cdclk;
 	} cdclk;
 
 	/**
@@ -1743,6 +1745,7 @@ struct drm_i915_private {
 	 *
 	 */
 	struct mutex av_mutex;
+	int audio_power_refcount;
 
 	struct {
 		struct mutex mutex;
diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
index 502b57ce72ab..20324b0d34c7 100644
--- a/drivers/gpu/drm/i915/intel_audio.c
+++ b/drivers/gpu/drm/i915/intel_audio.c
@@ -741,18 +741,78 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
+				  bool enable)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_atomic_state *state;
+	int ret;
+
+	drm_modeset_acquire_init(&ctx, 0);
+	state = drm_atomic_state_alloc(&dev_priv->drm);
+	if (WARN_ON(!state))
+		return;
+
+	state->acquire_ctx = &ctx;
+
+retry:
+	to_intel_atomic_state(state)->cdclk.force_min_cdclk_changed = true;
+	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
+		enable ? 2 * 96000 : 0;
+
+	/*
+	 * Protects dev_priv->cdclk.force_min_cdclk
+	 * Need to lock this here in case we have no active pipes
+	 * and thus wouldn't lock it during the commit otherwise.
+	 */
+	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
+			       &ctx);
+	if (!ret)
+		ret = drm_atomic_commit(state);
+
+	if (ret == -EDEADLK) {
+		drm_atomic_state_clear(state);
+		drm_modeset_backoff(&ctx);
+		goto retry;
+	}
+
+	WARN_ON(ret);
+
+	drm_atomic_state_put(state);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 static unsigned long i915_audio_component_get_power(struct device *kdev)
 {
+	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+	intel_wakeref_t ret;
+
 	/* Catch potential impedance mismatches before they occur! */
 	BUILD_BUG_ON(sizeof(intel_wakeref_t) > sizeof(unsigned long));
 
-	return intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
+	ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
+
+	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
+	if (dev_priv->audio_power_refcount++ == 0)
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			glk_force_audio_cdclk(dev_priv, true);
+
+	return ret;
 }
 
 static void i915_audio_component_put_power(struct device *kdev,
 					   unsigned long cookie)
 {
-	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO, cookie);
+	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
+
+	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
+	if (--dev_priv->audio_power_refcount == 0)
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			glk_force_audio_cdclk(dev_priv, false);
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO, cookie);
 }
 
 static void i915_audio_component_codec_wake_override(struct device *kdev,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 21fb4e0d6c4e..7dcca84f31d1 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2187,19 +2187,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
 	/*
 	 * According to BSpec, "The CD clock frequency must be at least twice
 	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
-	 *
-	 * FIXME: Check the actual, not default, BCLK being used.
-	 *
-	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
-	 * is required for audio probe, also when there are no audio capable
-	 * displays connected at probe time. This leads to unnecessarily high
-	 * CDCLK when audio is not required.
-	 *
-	 * FIXME: This limit is only applied when there are displays connected
-	 * at probe time. If we probe without displays, we'll still end up using
-	 * the platform minimum CDCLK, failing audio probe.
 	 */
-	if (INTEL_GEN(dev_priv) >= 9)
+	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
 		min_cdclk = max(2 * 96000, min_cdclk);
 
 	/*
@@ -2239,7 +2228,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
 		intel_state->min_cdclk[i] = min_cdclk;
 	}
 
-	min_cdclk = 0;
+	min_cdclk = intel_state->cdclk.force_min_cdclk;
 	for_each_pipe(dev_priv, pipe)
 		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
 
@@ -2300,7 +2289,8 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
 		vlv_calc_voltage_level(dev_priv, cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = vlv_calc_cdclk(dev_priv, 0);
+		cdclk = vlv_calc_cdclk(dev_priv,
+				       intel_state->cdclk.force_min_cdclk);
 
 		intel_state->cdclk.actual.cdclk = cdclk;
 		intel_state->cdclk.actual.voltage_level =
@@ -2333,7 +2323,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
 		bdw_calc_voltage_level(cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = bdw_calc_cdclk(0);
+		cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 
 		intel_state->cdclk.actual.cdclk = cdclk;
 		intel_state->cdclk.actual.voltage_level =
@@ -2405,7 +2395,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
 		skl_calc_voltage_level(cdclk);
 
 	if (!intel_state->active_crtcs) {
-		cdclk = skl_calc_cdclk(0, vco);
+		cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
 
 		intel_state->cdclk.actual.vco = vco;
 		intel_state->cdclk.actual.cdclk = cdclk;
@@ -2444,10 +2434,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
 
 	if (!intel_state->active_crtcs) {
 		if (IS_GEMINILAKE(dev_priv)) {
-			cdclk = glk_calc_cdclk(0);
+			cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 			vco = glk_de_pll_vco(dev_priv, cdclk);
 		} else {
-			cdclk = bxt_calc_cdclk(0);
+			cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 			vco = bxt_de_pll_vco(dev_priv, cdclk);
 		}
 
@@ -2483,7 +2473,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
 		    cnl_compute_min_voltage_level(intel_state));
 
 	if (!intel_state->active_crtcs) {
-		cdclk = cnl_calc_cdclk(0);
+		cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
 		vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
 
 		intel_state->cdclk.actual.vco = vco;
@@ -2519,7 +2509,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
 		    cnl_compute_min_voltage_level(intel_state));
 
 	if (!intel_state->active_crtcs) {
-		cdclk = icl_calc_cdclk(0, ref);
+		cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
 		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
 
 		intel_state->cdclk.actual.vco = vco;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61acbaf2af75..b4199cd53349 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12973,6 +12973,11 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 		return -EINVAL;
 	}
 
+	/* keep the current setting */
+	if (!intel_state->cdclk.force_min_cdclk_changed)
+		intel_state->cdclk.force_min_cdclk =
+			dev_priv->cdclk.force_min_cdclk;
+
 	intel_state->modeset = true;
 	intel_state->active_crtcs = dev_priv->active_crtcs;
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
@@ -13068,7 +13073,7 @@ static int intel_atomic_check(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *crtc_state;
 	int ret, i;
-	bool any_ms = false;
+	bool any_ms = intel_state->cdclk.force_min_cdclk_changed;
 
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
@@ -13660,6 +13665,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		dev_priv->active_crtcs = intel_state->active_crtcs;
 		dev_priv->cdclk.logical = intel_state->cdclk.logical;
 		dev_priv->cdclk.actual = intel_state->cdclk.actual;
+		dev_priv->cdclk.force_min_cdclk =
+			intel_state->cdclk.force_min_cdclk;
 	}
 
 	drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d9f188ef21f4..0b84e557c267 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -556,6 +556,9 @@ struct intel_atomic_state {
 		 * state only when all crtc's are DPMS off.
 		 */
 		struct intel_cdclk_state actual;
+
+		int force_min_cdclk;
+		bool force_min_cdclk_changed;
 	} cdclk;
 
 	bool dpll_set, modeset;
-- 
2.13.2

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

[CI 2/4] drm/i915: Save the old CDCLK atomic state

[Imre Deak]

The old state will be needed by an upcoming patch to determine if the
commit increases or decreases CDCLK, so move the old state to the atomic
state (while keeping the new one in dev_priv). cdclk.logical and
cdclk.actual in the atomic state isn't used atm anywhere after the
atomic check phase, so this should be safe.

v2:
- Use swap() instead of opencoding it. (Ville)

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_cdclk.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c |  4 ++--
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 7dcca84f31d1..5c25626f8cf0 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -2100,6 +2100,26 @@ bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 		a->voltage_level != b->voltage_level;
 }
 
+/**
+ * intel_cdclk_swap_state - make atomic CDCLK configuration effective
+ * @state: atomic state
+ *
+ * This is the CDCLK version of drm_atomic_helper_swap_state() since the
+ * helper does not handle driver-specific global state.
+ *
+ * Similarly to the atomic helpers this function does a complete swap,
+ * i.e. it also puts the old state into @state. This is used by the commit
+ * code to determine how CDCLK has changed (for instance did it increase or
+ * decrease).
+ */
+void intel_cdclk_swap_state(struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+
+	swap(state->cdclk.logical, dev_priv->cdclk.logical);
+	swap(state->cdclk.actual, dev_priv->cdclk.actual);
+}
+
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 			    const char *context)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b4199cd53349..9c4ad124302c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13663,10 +13663,10 @@ static int intel_atomic_commit(struct drm_device *dev,
 		       intel_state->min_voltage_level,
 		       sizeof(intel_state->min_voltage_level));
 		dev_priv->active_crtcs = intel_state->active_crtcs;
-		dev_priv->cdclk.logical = intel_state->cdclk.logical;
-		dev_priv->cdclk.actual = intel_state->cdclk.actual;
 		dev_priv->cdclk.force_min_cdclk =
 			intel_state->cdclk.force_min_cdclk;
+
+		intel_cdclk_swap_state(intel_state);
 	}
 
 	drm_atomic_state_get(state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0b84e557c267..85dd6a9d1e42 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1698,6 +1698,7 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
 bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 			 const struct intel_cdclk_state *b);
+void intel_cdclk_swap_state(struct intel_atomic_state *state);
 void intel_set_cdclk(struct drm_i915_private *dev_priv,
 		     const struct intel_cdclk_state *cdclk_state);
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
-- 
2.13.2

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

[CI 3/4] drm/i915: Remove redundant store of logical CDCLK state

[Imre Deak]

We copied the original state into the atomic state already earlier in
the function, so no need to do it a second time.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c4ad124302c..f7b44773e1e7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13031,8 +13031,6 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 		DRM_DEBUG_KMS("New voltage level calculated to be logical %u, actual %u\n",
 			      intel_state->cdclk.logical.voltage_level,
 			      intel_state->cdclk.actual.voltage_level);
-	} else {
-		to_intel_atomic_state(state)->cdclk.logical = dev_priv->cdclk.logical;
 	}
 
 	intel_modeset_clear_plls(state);
-- 
2.13.2

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

[CI 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Imre Deak]

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

If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.

v2:
- Wait for vblank after an optimized CDCLK change.
- Avoid optimization if the pipe needs a modeset (or was disabled).
- Split CDCLK change to a pre/post plane update step.
v3:
- Use correct version of CDCLK state as old state. (Ville)
- Remove unused intel_cdclk_can_skip_modeset()
v4:
- For consistency call intel_set_cdclk_post_plane_update() only during
  modesets (and not fastsets).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   3 +-
 drivers/gpu/drm/i915/i915_reg.h      |   3 +-
 drivers/gpu/drm/i915/intel_cdclk.c   | 142 +++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
 5 files changed, 170 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6b10cee4e77f..d8f91525c94c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -277,7 +277,8 @@ struct drm_i915_display_funcs {
 	void (*get_cdclk)(struct drm_i915_private *dev_priv,
 			  struct intel_cdclk_state *cdclk_state);
 	void (*set_cdclk)(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state);
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe);
 	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
 			     enum i9xx_plane_id i9xx_plane);
 	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9b69cec21f7b..12b8170ced96 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9521,7 +9521,8 @@ enum skl_power_gate {
 #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
 #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
 #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
-#define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
+#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
+#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
 #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1 << 16)
 #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
 
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 5c25626f8cf0..48cc42a7ef4f 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
 }
 
 static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val, cmd = cdclk_state->voltage_level;
@@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 }
 
 static void chv_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val, cmd = cdclk_state->voltage_level;
@@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
 }
 
 static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val;
@@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
 }
 
 static void skl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
 	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
 
-	skl_set_cdclk(dev_priv, &cdclk_state);
+	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
 
-	skl_set_cdclk(dev_priv, &cdclk_state);
+	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 static int bxt_calc_cdclk(int min_cdclk)
@@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
 }
 
 static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		bxt_de_pll_enable(dev_priv, vco);
 
 	val = divider | skl_cdclk_decimal(cdclk);
-	/*
-	 * FIXME if only the cd2x divider needs changing, it could be done
-	 * without shutting off the pipe (if only one pipe is active).
-	 */
-	val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	if (pipe == INVALID_PIPE)
+		val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	else
+		val |= BXT_CDCLK_CD2X_PIPE(pipe);
 	/*
 	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
 	 * enable otherwise.
@@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 	I915_WRITE(CDCLK_CTL, val);
 
+	if (pipe != INVALID_PIPE)
+		intel_wait_for_vblank(dev_priv, pipe);
+
 	mutex_lock(&dev_priv->pcu_lock);
 	/*
 	 * The timeout isn't specified, the 2ms used here is based on
@@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
 	}
 	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
 
-	bxt_set_cdclk(dev_priv, &cdclk_state);
+	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
 
-	bxt_set_cdclk(dev_priv, &cdclk_state);
+	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 static int cnl_calc_cdclk(int min_cdclk)
@@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
 }
 
 static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 		cnl_cdclk_pll_enable(dev_priv, vco);
 
 	val = divider | skl_cdclk_decimal(cdclk);
-	/*
-	 * FIXME if only the cd2x divider needs changing, it could be done
-	 * without shutting off the pipe (if only one pipe is active).
-	 */
-	val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	if (pipe == INVALID_PIPE)
+		val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	else
+		val |= BXT_CDCLK_CD2X_PIPE(pipe);
 	I915_WRITE(CDCLK_CTL, val);
 
+	if (pipe != INVALID_PIPE)
+		intel_wait_for_vblank(dev_priv, pipe);
+
 	/* inform PCU of the change */
 	mutex_lock(&dev_priv->pcu_lock);
 	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -1847,10 +1857,12 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
 }
 
 static void icl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	unsigned int cdclk = cdclk_state->cdclk;
 	unsigned int vco = cdclk_state->vco;
+	u32 val;
 	int ret;
 
 	mutex_lock(&dev_priv->pcu_lock);
@@ -1872,8 +1884,15 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
 	if (dev_priv->cdclk.hw.vco != vco)
 		cnl_cdclk_pll_enable(dev_priv, vco);
 
-	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
-			      skl_cdclk_decimal(cdclk));
+	val = skl_cdclk_decimal(cdclk);
+	if (pipe == INVALID_PIPE)
+		val |= ICL_CDCLK_CD2X_PIPE_NONE;
+	else
+		val |= ICL_CDCLK_CD2X_PIPE(pipe);
+	I915_WRITE(CDCLK_CTL, val);
+
+	if (pipe != INVALID_PIPE)
+		intel_wait_for_vblank(dev_priv, pipe);
 
 	mutex_lock(&dev_priv->pcu_lock);
 	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -2002,7 +2021,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
 	sanitized_state.voltage_level =
 				icl_calc_voltage_level(sanitized_state.cdclk);
 
-	icl_set_cdclk(dev_priv, &sanitized_state);
+	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
 }
 
 /**
@@ -2020,7 +2039,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
 
-	icl_set_cdclk(dev_priv, &cdclk_state);
+	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2048,7 +2067,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
 	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
 
-	cnl_set_cdclk(dev_priv, &cdclk_state);
+	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2066,7 +2085,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
 
-	cnl_set_cdclk(dev_priv, &cdclk_state);
+	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2086,6 +2105,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 }
 
 /**
+ * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
+ * @a: first CDCLK state
+ * @b: second CDCLK state
+ *
+ * Returns:
+ * True if the CDCLK states require just a cd2x divider update, false if not.
+ */
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+				   const struct intel_cdclk_state *a,
+				   const struct intel_cdclk_state *b)
+{
+	/* Older hw doesn't have the capability */
+	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
+		return false;
+
+	return a->cdclk != b->cdclk &&
+		a->vco == b->vco &&
+		a->ref == b->ref;
+}
+
+/**
  * intel_cdclk_changed - Determine if two CDCLK states are different
  * @a: first CDCLK state
  * @b: second CDCLK state
@@ -2133,12 +2173,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
  * intel_set_cdclk - Push the CDCLK state to the hardware
  * @dev_priv: i915 device
  * @cdclk_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
  *
  * Program the hardware based on the passed in CDCLK state,
  * if necessary.
  */
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-		     const struct intel_cdclk_state *cdclk_state)
+static void intel_set_cdclk(struct drm_i915_private *dev_priv,
+			    const struct intel_cdclk_state *cdclk_state,
+			    enum pipe pipe)
 {
 	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
 		return;
@@ -2148,7 +2190,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 
 	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
 
-	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
+	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
 
 	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
 		 "cdclk state doesn't match!\n")) {
@@ -2157,6 +2199,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 }
 
+/**
+ * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware before updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+				 const struct intel_cdclk_state *old_state,
+				 const struct intel_cdclk_state *new_state,
+				 enum pipe pipe)
+{
+	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
+		intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
+/**
+ * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware after updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *old_state,
+				  const struct intel_cdclk_state *new_state,
+				  enum pipe pipe)
+{
+	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
+		intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
 static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
 				     int pixel_rate)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f7b44773e1e7..bcec03f43d3a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12982,6 +12982,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	intel_state->active_crtcs = dev_priv->active_crtcs;
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
 	intel_state->cdclk.actual = dev_priv->cdclk.actual;
+	intel_state->cdclk.pipe = INVALID_PIPE;
 
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		if (new_crtc_state->active)
@@ -13001,6 +13002,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	 * adjusted_mode bits in the crtc directly.
 	 */
 	if (dev_priv->display.modeset_calc_cdclk) {
+		enum pipe pipe;
+
 		ret = dev_priv->display.modeset_calc_cdclk(state);
 		if (ret < 0)
 			return ret;
@@ -13017,12 +13020,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 				return ret;
 		}
 
+		if (is_power_of_2(intel_state->active_crtcs)) {
+			struct drm_crtc *crtc;
+			struct drm_crtc_state *crtc_state;
+
+			pipe = ilog2(intel_state->active_crtcs);
+			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+			if (crtc_state && needs_modeset(crtc_state))
+				pipe = INVALID_PIPE;
+		} else {
+			pipe = INVALID_PIPE;
+		}
+
 		/* All pipes must be switched off while we change the cdclk. */
-		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
-					      &intel_state->cdclk.actual)) {
+		if (pipe != INVALID_PIPE &&
+		    intel_cdclk_needs_cd2x_update(dev_priv,
+						  &dev_priv->cdclk.actual,
+						  &intel_state->cdclk.actual)) {
+			ret = intel_lock_all_pipes(state);
+			if (ret < 0)
+				return ret;
+
+			intel_state->cdclk.pipe = pipe;
+		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
+						     &intel_state->cdclk.actual)) {
 			ret = intel_modeset_all_pipes(state);
 			if (ret < 0)
 				return ret;
+
+			intel_state->cdclk.pipe = INVALID_PIPE;
 		}
 
 		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
@@ -13431,7 +13458,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	if (intel_state->modeset) {
 		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
 
-		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
+		intel_set_cdclk_pre_plane_update(dev_priv,
+						 &intel_state->cdclk.actual,
+						 &dev_priv->cdclk.actual,
+						 intel_state->cdclk.pipe);
 
 		/*
 		 * SKL workaround: bspec recommends we disable the SAGV when we
@@ -13460,6 +13490,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.update_crtcs(state);
 
+	if (intel_state->modeset)
+		intel_set_cdclk_post_plane_update(dev_priv,
+						  &intel_state->cdclk.actual,
+						  &dev_priv->cdclk.actual,
+						  intel_state->cdclk.pipe);
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 85dd6a9d1e42..d22a0e92e0d4 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -559,6 +559,8 @@ struct intel_atomic_state {
 
 		int force_min_cdclk;
 		bool force_min_cdclk_changed;
+		/* pipe to which cd2x update is synchronized */
+		enum pipe pipe;
 	} cdclk;
 
 	bool dpll_set, modeset;
@@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
 void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
 void intel_update_cdclk(struct drm_i915_private *dev_priv);
 void intel_update_rawclk(struct drm_i915_private *dev_priv);
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+				   const struct intel_cdclk_state *a,
+				   const struct intel_cdclk_state *b);
 bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
 bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 			 const struct intel_cdclk_state *b);
 void intel_cdclk_swap_state(struct intel_atomic_state *state);
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-		     const struct intel_cdclk_state *cdclk_state);
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+				 const struct intel_cdclk_state *old_state,
+				 const struct intel_cdclk_state *new_state,
+				 enum pipe pipe);
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *old_state,
+				  const struct intel_cdclk_state *new_state,
+				  enum pipe pipe);
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 			    const char *context);
 
-- 
2.13.2

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

Re: [CI 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Clinton Taylor]

On 3/20/19 6:54 AM, Imre Deak wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we have only a single active pipe and the cdclk change only requires
> the cd2x divider to be updated bxt+ can do the update with forcing a full
> modeset on the pipe. Try to hook that up.
>
> v2:
> - Wait for vblank after an optimized CDCLK change.
> - Avoid optimization if the pipe needs a modeset (or was disabled).
> - Split CDCLK change to a pre/post plane update step.
> v3:
> - Use correct version of CDCLK state as old state. (Ville)
> - Remove unused intel_cdclk_can_skip_modeset()
> v4:
> - For consistency call intel_set_cdclk_post_plane_update() only during
>    modesets (and not fastsets).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> Tested-by: Abhay Kumar <abhay.kumar@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |   3 +-
>   drivers/gpu/drm/i915/i915_reg.h      |   3 +-
>   drivers/gpu/drm/i915/intel_cdclk.c   | 142 +++++++++++++++++++++++++++--------
>   drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
>   drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
>   5 files changed, 170 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6b10cee4e77f..d8f91525c94c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -277,7 +277,8 @@ struct drm_i915_display_funcs {
>   	void (*get_cdclk)(struct drm_i915_private *dev_priv,
>   			  struct intel_cdclk_state *cdclk_state);
>   	void (*set_cdclk)(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state);
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe);
>   	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
>   			     enum i9xx_plane_id i9xx_plane);
>   	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9b69cec21f7b..12b8170ced96 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9521,7 +9521,8 @@ enum skl_power_gate {
>   #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
>   #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
>   #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> -#define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
> +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)

Unfortunately bits 21:19 of CDCLK_CTL don't match the pipe enum. This 
MACRO will not work except for PIPE_A.

Pipe_A is 0x00, PIPE_B is 0x02, PIPE_C is 0x06.

In BXT only bits 21:20 were used for CD2X pipe select and the pipe enum 
does work.


-Clint



> +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
>   #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1 << 16)
>   #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
>   
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 5c25626f8cf0..48cc42a7ef4f 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
>   }
>   
>   static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val, cmd = cdclk_state->voltage_level;
> @@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
>   }
>   
>   static void chv_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val, cmd = cdclk_state->voltage_level;
> @@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
>   }
>   
>   static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val;
> @@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
>   }
>   
>   static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
>   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	skl_set_cdclk(dev_priv, &cdclk_state);
> +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	skl_set_cdclk(dev_priv, &cdclk_state);
> +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   static int bxt_calc_cdclk(int min_cdclk)
> @@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
>   }
>   
>   static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		bxt_de_pll_enable(dev_priv, vco);
>   
>   	val = divider | skl_cdclk_decimal(cdclk);
> -	/*
> -	 * FIXME if only the cd2x divider needs changing, it could be done
> -	 * without shutting off the pipe (if only one pipe is active).
> -	 */
> -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	if (pipe == INVALID_PIPE)
> +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	else
> +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
>   	/*
>   	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
>   	 * enable otherwise.
> @@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>   	I915_WRITE(CDCLK_CTL, val);
>   
> +	if (pipe != INVALID_PIPE)
> +		intel_wait_for_vblank(dev_priv, pipe);
> +
>   	mutex_lock(&dev_priv->pcu_lock);
>   	/*
>   	 * The timeout isn't specified, the 2ms used here is based on
> @@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
>   	}
>   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
>   
> -	bxt_set_cdclk(dev_priv, &cdclk_state);
> +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
>   
> -	bxt_set_cdclk(dev_priv, &cdclk_state);
> +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   static int cnl_calc_cdclk(int min_cdclk)
> @@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
>   }
>   
>   static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
>   		cnl_cdclk_pll_enable(dev_priv, vco);
>   
>   	val = divider | skl_cdclk_decimal(cdclk);
> -	/*
> -	 * FIXME if only the cd2x divider needs changing, it could be done
> -	 * without shutting off the pipe (if only one pipe is active).
> -	 */
> -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	if (pipe == INVALID_PIPE)
> +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	else
> +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
>   	I915_WRITE(CDCLK_CTL, val);
>   
> +	if (pipe != INVALID_PIPE)
> +		intel_wait_for_vblank(dev_priv, pipe);
> +
>   	/* inform PCU of the change */
>   	mutex_lock(&dev_priv->pcu_lock);
>   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> @@ -1847,10 +1857,12 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>   }
>   
>   static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	unsigned int cdclk = cdclk_state->cdclk;
>   	unsigned int vco = cdclk_state->vco;
> +	u32 val;
>   	int ret;
>   
>   	mutex_lock(&dev_priv->pcu_lock);
> @@ -1872,8 +1884,15 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
>   	if (dev_priv->cdclk.hw.vco != vco)
>   		cnl_cdclk_pll_enable(dev_priv, vco);
>   
> -	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> -			      skl_cdclk_decimal(cdclk));
> +	val = skl_cdclk_decimal(cdclk);
> +	if (pipe == INVALID_PIPE)
> +		val |= ICL_CDCLK_CD2X_PIPE_NONE;
> +	else
> +		val |= ICL_CDCLK_CD2X_PIPE(pipe);
> +	I915_WRITE(CDCLK_CTL, val);
> +
> +	if (pipe != INVALID_PIPE)
> +		intel_wait_for_vblank(dev_priv, pipe);
>   
>   	mutex_lock(&dev_priv->pcu_lock);
>   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> @@ -2002,7 +2021,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
>   	sanitized_state.voltage_level =
>   				icl_calc_voltage_level(sanitized_state.cdclk);
>   
> -	icl_set_cdclk(dev_priv, &sanitized_state);
> +	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2020,7 +2039,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	icl_set_cdclk(dev_priv, &cdclk_state);
> +	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2048,7 +2067,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
>   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	cnl_set_cdclk(dev_priv, &cdclk_state);
> +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2066,7 +2085,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	cnl_set_cdclk(dev_priv, &cdclk_state);
> +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2086,6 +2105,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
>   }
>   
>   /**
> + * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
> + * @a: first CDCLK state
> + * @b: second CDCLK state
> + *
> + * Returns:
> + * True if the CDCLK states require just a cd2x divider update, false if not.
> + */
> +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> +				   const struct intel_cdclk_state *a,
> +				   const struct intel_cdclk_state *b)
> +{
> +	/* Older hw doesn't have the capability */
> +	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
> +		return false;
> +
> +	return a->cdclk != b->cdclk &&
> +		a->vco == b->vco &&
> +		a->ref == b->ref;
> +}
> +
> +/**
>    * intel_cdclk_changed - Determine if two CDCLK states are different
>    * @a: first CDCLK state
>    * @b: second CDCLK state
> @@ -2133,12 +2173,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>    * intel_set_cdclk - Push the CDCLK state to the hardware
>    * @dev_priv: i915 device
>    * @cdclk_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
>    *
>    * Program the hardware based on the passed in CDCLK state,
>    * if necessary.
>    */
> -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> -		     const struct intel_cdclk_state *cdclk_state)
> +static void intel_set_cdclk(struct drm_i915_private *dev_priv,
> +			    const struct intel_cdclk_state *cdclk_state,
> +			    enum pipe pipe)
>   {
>   	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
>   		return;
> @@ -2148,7 +2190,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>   
>   	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
>   
> -	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> +	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
>   
>   	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
>   		 "cdclk state doesn't match!\n")) {
> @@ -2157,6 +2199,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>   	}
>   }
>   
> +/**
> + * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
> + * @dev_priv: i915 device
> + * @old_state: old CDCLK state
> + * @new_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
> + *
> + * Program the hardware before updating the HW plane state based on the passed
> + * in CDCLK state, if necessary.
> + */
> +void
> +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> +				 const struct intel_cdclk_state *old_state,
> +				 const struct intel_cdclk_state *new_state,
> +				 enum pipe pipe)
> +{
> +	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
> +		intel_set_cdclk(dev_priv, new_state, pipe);
> +}
> +
> +/**
> + * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
> + * @dev_priv: i915 device
> + * @old_state: old CDCLK state
> + * @new_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
> + *
> + * Program the hardware after updating the HW plane state based on the passed
> + * in CDCLK state, if necessary.
> + */
> +void
> +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> +				  const struct intel_cdclk_state *old_state,
> +				  const struct intel_cdclk_state *new_state,
> +				  enum pipe pipe)
> +{
> +	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
> +		intel_set_cdclk(dev_priv, new_state, pipe);
> +}
> +
>   static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
>   				     int pixel_rate)
>   {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f7b44773e1e7..bcec03f43d3a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12982,6 +12982,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   	intel_state->active_crtcs = dev_priv->active_crtcs;
>   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
>   	intel_state->cdclk.actual = dev_priv->cdclk.actual;
> +	intel_state->cdclk.pipe = INVALID_PIPE;
>   
>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>   		if (new_crtc_state->active)
> @@ -13001,6 +13002,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   	 * adjusted_mode bits in the crtc directly.
>   	 */
>   	if (dev_priv->display.modeset_calc_cdclk) {
> +		enum pipe pipe;
> +
>   		ret = dev_priv->display.modeset_calc_cdclk(state);
>   		if (ret < 0)
>   			return ret;
> @@ -13017,12 +13020,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   				return ret;
>   		}
>   
> +		if (is_power_of_2(intel_state->active_crtcs)) {
> +			struct drm_crtc *crtc;
> +			struct drm_crtc_state *crtc_state;
> +
> +			pipe = ilog2(intel_state->active_crtcs);
> +			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
> +			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +			if (crtc_state && needs_modeset(crtc_state))
> +				pipe = INVALID_PIPE;
> +		} else {
> +			pipe = INVALID_PIPE;
> +		}
> +
>   		/* All pipes must be switched off while we change the cdclk. */
> -		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> -					      &intel_state->cdclk.actual)) {
> +		if (pipe != INVALID_PIPE &&
> +		    intel_cdclk_needs_cd2x_update(dev_priv,
> +						  &dev_priv->cdclk.actual,
> +						  &intel_state->cdclk.actual)) {
> +			ret = intel_lock_all_pipes(state);
> +			if (ret < 0)
> +				return ret;
> +
> +			intel_state->cdclk.pipe = pipe;
> +		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> +						     &intel_state->cdclk.actual)) {
>   			ret = intel_modeset_all_pipes(state);
>   			if (ret < 0)
>   				return ret;
> +
> +			intel_state->cdclk.pipe = INVALID_PIPE;
>   		}
>   
>   		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
> @@ -13431,7 +13458,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>   	if (intel_state->modeset) {
>   		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>   
> -		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> +		intel_set_cdclk_pre_plane_update(dev_priv,
> +						 &intel_state->cdclk.actual,
> +						 &dev_priv->cdclk.actual,
> +						 intel_state->cdclk.pipe);
>   
>   		/*
>   		 * SKL workaround: bspec recommends we disable the SAGV when we
> @@ -13460,6 +13490,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>   	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>   	dev_priv->display.update_crtcs(state);
>   
> +	if (intel_state->modeset)
> +		intel_set_cdclk_post_plane_update(dev_priv,
> +						  &intel_state->cdclk.actual,
> +						  &dev_priv->cdclk.actual,
> +						  intel_state->cdclk.pipe);
> +
>   	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>   	 * already, but still need the state for the delayed optimization. To
>   	 * fix this:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 85dd6a9d1e42..d22a0e92e0d4 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -559,6 +559,8 @@ struct intel_atomic_state {
>   
>   		int force_min_cdclk;
>   		bool force_min_cdclk_changed;
> +		/* pipe to which cd2x update is synchronized */
> +		enum pipe pipe;
>   	} cdclk;
>   
>   	bool dpll_set, modeset;
> @@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
>   void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
>   void intel_update_cdclk(struct drm_i915_private *dev_priv);
>   void intel_update_rawclk(struct drm_i915_private *dev_priv);
> +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> +				   const struct intel_cdclk_state *a,
> +				   const struct intel_cdclk_state *b);
>   bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
>   			       const struct intel_cdclk_state *b);
>   bool intel_cdclk_changed(const struct intel_cdclk_state *a,
>   			 const struct intel_cdclk_state *b);
>   void intel_cdclk_swap_state(struct intel_atomic_state *state);
> -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> -		     const struct intel_cdclk_state *cdclk_state);
> +void
> +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> +				 const struct intel_cdclk_state *old_state,
> +				 const struct intel_cdclk_state *new_state,
> +				 enum pipe pipe);
> +void
> +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> +				  const struct intel_cdclk_state *old_state,
> +				  const struct intel_cdclk_state *new_state,
> +				  enum pipe pipe);
>   void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>   			    const char *context);
>   
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Imre Deak]

On Thu, Mar 21, 2019 at 02:53:00PM -0700, Clinton Taylor wrote:
> 
> On 3/20/19 6:54 AM, Imre Deak wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we have only a single active pipe and the cdclk change only requires
> > the cd2x divider to be updated bxt+ can do the update with forcing a full
> > modeset on the pipe. Try to hook that up.
> > 
> > v2:
> > - Wait for vblank after an optimized CDCLK change.
> > - Avoid optimization if the pipe needs a modeset (or was disabled).
> > - Split CDCLK change to a pre/post plane update step.
> > v3:
> > - Use correct version of CDCLK state as old state. (Ville)
> > - Remove unused intel_cdclk_can_skip_modeset()
> > v4:
> > - For consistency call intel_set_cdclk_post_plane_update() only during
> >    modesets (and not fastsets).
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> > Tested-by: Abhay Kumar <abhay.kumar@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_drv.h      |   3 +-
> >   drivers/gpu/drm/i915/i915_reg.h      |   3 +-
> >   drivers/gpu/drm/i915/intel_cdclk.c   | 142 +++++++++++++++++++++++++++--------
> >   drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
> >   drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
> >   5 files changed, 170 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 6b10cee4e77f..d8f91525c94c 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -277,7 +277,8 @@ struct drm_i915_display_funcs {
> >   	void (*get_cdclk)(struct drm_i915_private *dev_priv,
> >   			  struct intel_cdclk_state *cdclk_state);
> >   	void (*set_cdclk)(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state);
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe);
> >   	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
> >   			     enum i9xx_plane_id i9xx_plane);
> >   	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 9b69cec21f7b..12b8170ced96 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -9521,7 +9521,8 @@ enum skl_power_gate {
> >   #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
> >   #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
> >   #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> > -#define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
> > +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> 
> Unfortunately bits 21:19 of CDCLK_CTL don't match the pipe enum. This MACRO
> will not work except for PIPE_A.
> 
> Pipe_A is 0x00, PIPE_B is 0x02, PIPE_C is 0x06.

Good catch.

The pipe B and C encodings could be typoed though and I couldn't trigger
any problems with either encodings (i.e. using either 0x1 or 0x2 for
pipe B updates while pipe B was the only pipe enabled). So I opened a
ticket for this in BSpec, let's wait for a clarification on this.

> In BXT only bits 21:20 were used for CD2X pipe select and the pipe enum does
> work.
> 
> 
> -Clint
> 
> 
> 
> > +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
> >   #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1 << 16)
> >   #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > index 5c25626f8cf0..48cc42a7ef4f 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> >   }
> >   static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	u32 val, cmd = cdclk_state->voltage_level;
> > @@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> >   }
> >   static void chv_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	u32 val, cmd = cdclk_state->voltage_level;
> > @@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
> >   }
> >   static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	u32 val;
> > @@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
> >   }
> >   static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	int vco = cdclk_state->vco;
> > @@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
> >   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
> > -	skl_set_cdclk(dev_priv, &cdclk_state);
> > +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.vco = 0;
> >   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
> > -	skl_set_cdclk(dev_priv, &cdclk_state);
> > +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   static int bxt_calc_cdclk(int min_cdclk)
> > @@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> >   }
> >   static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	int vco = cdclk_state->vco;
> > @@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> >   		bxt_de_pll_enable(dev_priv, vco);
> >   	val = divider | skl_cdclk_decimal(cdclk);
> > -	/*
> > -	 * FIXME if only the cd2x divider needs changing, it could be done
> > -	 * without shutting off the pipe (if only one pipe is active).
> > -	 */
> > -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > +	if (pipe == INVALID_PIPE)
> > +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > +	else
> > +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
> >   	/*
> >   	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> >   	 * enable otherwise.
> > @@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> >   		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> >   	I915_WRITE(CDCLK_CTL, val);
> > +	if (pipe != INVALID_PIPE)
> > +		intel_wait_for_vblank(dev_priv, pipe);
> > +
> >   	mutex_lock(&dev_priv->pcu_lock);
> >   	/*
> >   	 * The timeout isn't specified, the 2ms used here is based on
> > @@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> >   	}
> >   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> > -	bxt_set_cdclk(dev_priv, &cdclk_state);
> > +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.vco = 0;
> >   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> > -	bxt_set_cdclk(dev_priv, &cdclk_state);
> > +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   static int cnl_calc_cdclk(int min_cdclk)
> > @@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
> >   }
> >   static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	int cdclk = cdclk_state->cdclk;
> >   	int vco = cdclk_state->vco;
> > @@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> >   		cnl_cdclk_pll_enable(dev_priv, vco);
> >   	val = divider | skl_cdclk_decimal(cdclk);
> > -	/*
> > -	 * FIXME if only the cd2x divider needs changing, it could be done
> > -	 * without shutting off the pipe (if only one pipe is active).
> > -	 */
> > -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > +	if (pipe == INVALID_PIPE)
> > +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > +	else
> > +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
> >   	I915_WRITE(CDCLK_CTL, val);
> > +	if (pipe != INVALID_PIPE)
> > +		intel_wait_for_vblank(dev_priv, pipe);
> > +
> >   	/* inform PCU of the change */
> >   	mutex_lock(&dev_priv->pcu_lock);
> >   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > @@ -1847,10 +1857,12 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> >   }
> >   static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > -			  const struct intel_cdclk_state *cdclk_state)
> > +			  const struct intel_cdclk_state *cdclk_state,
> > +			  enum pipe pipe)
> >   {
> >   	unsigned int cdclk = cdclk_state->cdclk;
> >   	unsigned int vco = cdclk_state->vco;
> > +	u32 val;
> >   	int ret;
> >   	mutex_lock(&dev_priv->pcu_lock);
> > @@ -1872,8 +1884,15 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> >   	if (dev_priv->cdclk.hw.vco != vco)
> >   		cnl_cdclk_pll_enable(dev_priv, vco);
> > -	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> > -			      skl_cdclk_decimal(cdclk));
> > +	val = skl_cdclk_decimal(cdclk);
> > +	if (pipe == INVALID_PIPE)
> > +		val |= ICL_CDCLK_CD2X_PIPE_NONE;
> > +	else
> > +		val |= ICL_CDCLK_CD2X_PIPE(pipe);
> > +	I915_WRITE(CDCLK_CTL, val);
> > +
> > +	if (pipe != INVALID_PIPE)
> > +		intel_wait_for_vblank(dev_priv, pipe);
> >   	mutex_lock(&dev_priv->pcu_lock);
> >   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > @@ -2002,7 +2021,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
> >   	sanitized_state.voltage_level =
> >   				icl_calc_voltage_level(sanitized_state.cdclk);
> > -	icl_set_cdclk(dev_priv, &sanitized_state);
> > +	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -2020,7 +2039,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.vco = 0;
> >   	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
> > -	icl_set_cdclk(dev_priv, &cdclk_state);
> > +	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -2048,7 +2067,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> >   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> > -	cnl_set_cdclk(dev_priv, &cdclk_state);
> > +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -2066,7 +2085,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> >   	cdclk_state.vco = 0;
> >   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> > -	cnl_set_cdclk(dev_priv, &cdclk_state);
> > +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> >   }
> >   /**
> > @@ -2086,6 +2105,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
> >   }
> >   /**
> > + * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
> > + * @a: first CDCLK state
> > + * @b: second CDCLK state
> > + *
> > + * Returns:
> > + * True if the CDCLK states require just a cd2x divider update, false if not.
> > + */
> > +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> > +				   const struct intel_cdclk_state *a,
> > +				   const struct intel_cdclk_state *b)
> > +{
> > +	/* Older hw doesn't have the capability */
> > +	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
> > +		return false;
> > +
> > +	return a->cdclk != b->cdclk &&
> > +		a->vco == b->vco &&
> > +		a->ref == b->ref;
> > +}
> > +
> > +/**
> >    * intel_cdclk_changed - Determine if two CDCLK states are different
> >    * @a: first CDCLK state
> >    * @b: second CDCLK state
> > @@ -2133,12 +2173,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> >    * intel_set_cdclk - Push the CDCLK state to the hardware
> >    * @dev_priv: i915 device
> >    * @cdclk_state: new CDCLK state
> > + * @pipe: pipe with which to synchronize the update
> >    *
> >    * Program the hardware based on the passed in CDCLK state,
> >    * if necessary.
> >    */
> > -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > -		     const struct intel_cdclk_state *cdclk_state)
> > +static void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > +			    const struct intel_cdclk_state *cdclk_state,
> > +			    enum pipe pipe)
> >   {
> >   	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
> >   		return;
> > @@ -2148,7 +2190,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> >   	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
> > -	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> > +	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
> >   	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
> >   		 "cdclk state doesn't match!\n")) {
> > @@ -2157,6 +2199,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> >   	}
> >   }
> > +/**
> > + * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
> > + * @dev_priv: i915 device
> > + * @old_state: old CDCLK state
> > + * @new_state: new CDCLK state
> > + * @pipe: pipe with which to synchronize the update
> > + *
> > + * Program the hardware before updating the HW plane state based on the passed
> > + * in CDCLK state, if necessary.
> > + */
> > +void
> > +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> > +				 const struct intel_cdclk_state *old_state,
> > +				 const struct intel_cdclk_state *new_state,
> > +				 enum pipe pipe)
> > +{
> > +	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
> > +		intel_set_cdclk(dev_priv, new_state, pipe);
> > +}
> > +
> > +/**
> > + * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
> > + * @dev_priv: i915 device
> > + * @old_state: old CDCLK state
> > + * @new_state: new CDCLK state
> > + * @pipe: pipe with which to synchronize the update
> > + *
> > + * Program the hardware after updating the HW plane state based on the passed
> > + * in CDCLK state, if necessary.
> > + */
> > +void
> > +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> > +				  const struct intel_cdclk_state *old_state,
> > +				  const struct intel_cdclk_state *new_state,
> > +				  enum pipe pipe)
> > +{
> > +	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
> > +		intel_set_cdclk(dev_priv, new_state, pipe);
> > +}
> > +
> >   static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
> >   				     int pixel_rate)
> >   {
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f7b44773e1e7..bcec03f43d3a 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -12982,6 +12982,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> >   	intel_state->active_crtcs = dev_priv->active_crtcs;
> >   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
> >   	intel_state->cdclk.actual = dev_priv->cdclk.actual;
> > +	intel_state->cdclk.pipe = INVALID_PIPE;
> >   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> >   		if (new_crtc_state->active)
> > @@ -13001,6 +13002,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> >   	 * adjusted_mode bits in the crtc directly.
> >   	 */
> >   	if (dev_priv->display.modeset_calc_cdclk) {
> > +		enum pipe pipe;
> > +
> >   		ret = dev_priv->display.modeset_calc_cdclk(state);
> >   		if (ret < 0)
> >   			return ret;
> > @@ -13017,12 +13020,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> >   				return ret;
> >   		}
> > +		if (is_power_of_2(intel_state->active_crtcs)) {
> > +			struct drm_crtc *crtc;
> > +			struct drm_crtc_state *crtc_state;
> > +
> > +			pipe = ilog2(intel_state->active_crtcs);
> > +			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
> > +			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > +			if (crtc_state && needs_modeset(crtc_state))
> > +				pipe = INVALID_PIPE;
> > +		} else {
> > +			pipe = INVALID_PIPE;
> > +		}
> > +
> >   		/* All pipes must be switched off while we change the cdclk. */
> > -		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> > -					      &intel_state->cdclk.actual)) {
> > +		if (pipe != INVALID_PIPE &&
> > +		    intel_cdclk_needs_cd2x_update(dev_priv,
> > +						  &dev_priv->cdclk.actual,
> > +						  &intel_state->cdclk.actual)) {
> > +			ret = intel_lock_all_pipes(state);
> > +			if (ret < 0)
> > +				return ret;
> > +
> > +			intel_state->cdclk.pipe = pipe;
> > +		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> > +						     &intel_state->cdclk.actual)) {
> >   			ret = intel_modeset_all_pipes(state);
> >   			if (ret < 0)
> >   				return ret;
> > +
> > +			intel_state->cdclk.pipe = INVALID_PIPE;
> >   		}
> >   		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
> > @@ -13431,7 +13458,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >   	if (intel_state->modeset) {
> >   		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
> > -		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> > +		intel_set_cdclk_pre_plane_update(dev_priv,
> > +						 &intel_state->cdclk.actual,
> > +						 &dev_priv->cdclk.actual,
> > +						 intel_state->cdclk.pipe);
> >   		/*
> >   		 * SKL workaround: bspec recommends we disable the SAGV when we
> > @@ -13460,6 +13490,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> >   	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> >   	dev_priv->display.update_crtcs(state);
> > +	if (intel_state->modeset)
> > +		intel_set_cdclk_post_plane_update(dev_priv,
> > +						  &intel_state->cdclk.actual,
> > +						  &dev_priv->cdclk.actual,
> > +						  intel_state->cdclk.pipe);
> > +
> >   	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
> >   	 * already, but still need the state for the delayed optimization. To
> >   	 * fix this:
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 85dd6a9d1e42..d22a0e92e0d4 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -559,6 +559,8 @@ struct intel_atomic_state {
> >   		int force_min_cdclk;
> >   		bool force_min_cdclk_changed;
> > +		/* pipe to which cd2x update is synchronized */
> > +		enum pipe pipe;
> >   	} cdclk;
> >   	bool dpll_set, modeset;
> > @@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
> >   void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
> >   void intel_update_cdclk(struct drm_i915_private *dev_priv);
> >   void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> > +				   const struct intel_cdclk_state *a,
> > +				   const struct intel_cdclk_state *b);
> >   bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
> >   			       const struct intel_cdclk_state *b);
> >   bool intel_cdclk_changed(const struct intel_cdclk_state *a,
> >   			 const struct intel_cdclk_state *b);
> >   void intel_cdclk_swap_state(struct intel_atomic_state *state);
> > -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > -		     const struct intel_cdclk_state *cdclk_state);
> > +void
> > +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> > +				 const struct intel_cdclk_state *old_state,
> > +				 const struct intel_cdclk_state *new_state,
> > +				 enum pipe pipe);
> > +void
> > +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> > +				  const struct intel_cdclk_state *old_state,
> > +				  const struct intel_cdclk_state *new_state,
> > +				  enum pipe pipe);
> >   void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> >   			    const char *context);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [CI 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Imre Deak]

On Fri, Mar 22, 2019 at 10:23:07PM +0200, Imre Deak wrote:
> On Thu, Mar 21, 2019 at 02:53:00PM -0700, Clinton Taylor wrote:
> > 
> > On 3/20/19 6:54 AM, Imre Deak wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > If we have only a single active pipe and the cdclk change only requires
> > > the cd2x divider to be updated bxt+ can do the update with forcing a full
> > > modeset on the pipe. Try to hook that up.
> > > 
> > > v2:
> > > - Wait for vblank after an optimized CDCLK change.
> > > - Avoid optimization if the pipe needs a modeset (or was disabled).
> > > - Split CDCLK change to a pre/post plane update step.
> > > v3:
> > > - Use correct version of CDCLK state as old state. (Ville)
> > > - Remove unused intel_cdclk_can_skip_modeset()
> > > v4:
> > > - For consistency call intel_set_cdclk_post_plane_update() only during
> > >    modesets (and not fastsets).
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> > > Tested-by: Abhay Kumar <abhay.kumar@intel.com>
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/i915_drv.h      |   3 +-
> > >   drivers/gpu/drm/i915/i915_reg.h      |   3 +-
> > >   drivers/gpu/drm/i915/intel_cdclk.c   | 142 +++++++++++++++++++++++++++--------
> > >   drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
> > >   drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
> > >   5 files changed, 170 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 6b10cee4e77f..d8f91525c94c 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -277,7 +277,8 @@ struct drm_i915_display_funcs {
> > >   	void (*get_cdclk)(struct drm_i915_private *dev_priv,
> > >   			  struct intel_cdclk_state *cdclk_state);
> > >   	void (*set_cdclk)(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state);
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe);
> > >   	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
> > >   			     enum i9xx_plane_id i9xx_plane);
> > >   	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 9b69cec21f7b..12b8170ced96 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -9521,7 +9521,8 @@ enum skl_power_gate {
> > >   #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe) << 20)
> > >   #define  CDCLK_DIVMUX_CD_OVERRIDE	(1 << 19)
> > >   #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> > > -#define  ICL_CDCLK_CD2X_PIPE_NONE	(7 << 19)
> > > +#define  ICL_CDCLK_CD2X_PIPE(pipe)	((pipe) << 19)
> > 
> > Unfortunately bits 21:19 of CDCLK_CTL don't match the pipe enum. This MACRO
> > will not work except for PIPE_A.
> > 
> > Pipe_A is 0x00, PIPE_B is 0x02, PIPE_C is 0x06.
> 
> Good catch.
> 
> The pipe B and C encodings could be typoed though and I couldn't trigger
> any problems with either encodings (i.e. using either 0x1 or 0x2 for
> pipe B updates while pipe B was the only pipe enabled). So I opened a
> ticket for this in BSpec, let's wait for a clarification on this.

There is no conclusion on this in BSpec yet. However imo, we don't need
to wait for that since on ICL there is actually no support for a CD clock
divider other than 1, see the description for CDCLK_CTL in the spec.

So on ICL we'd never change the CDCLK rate while it's active and so we
don't actually need to ever set the pipe select bits to other than the
current no-pipe setting.

Ville pointed out that in the future we'll still call icl_set_cdclk()
while the CD clock is active, but only to change the voltage level
(as required by port clock rates), keeping the CD clock rate the same.
But that doesn't need the pipe select bits either nor the vblank wait.

Based on the above I'll remove the ICL specifc parts from this patch to
set the pipe select bits and wait for vblank in icl_set_cdclk().

> 
> > In BXT only bits 21:20 were used for CD2X pipe select and the pipe enum does
> > work.
> > 
> > 
> > -Clint
> > 
> > 
> > 
> > > +#define  ICL_CDCLK_CD2X_PIPE_NONE	ICL_CDCLK_CD2X_PIPE(7)
> > >   #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1 << 16)
> > >   #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 5c25626f8cf0..48cc42a7ef4f 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -516,7 +516,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
> > >   }
> > >   static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	u32 val, cmd = cdclk_state->voltage_level;
> > > @@ -598,7 +599,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> > >   }
> > >   static void chv_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	u32 val, cmd = cdclk_state->voltage_level;
> > > @@ -697,7 +699,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
> > >   }
> > >   static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	u32 val;
> > > @@ -987,7 +990,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
> > >   }
> > >   static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	int vco = cdclk_state->vco;
> > > @@ -1158,7 +1162,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
> > >   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
> > > -	skl_set_cdclk(dev_priv, &cdclk_state);
> > > +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -1176,7 +1180,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.vco = 0;
> > >   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
> > > -	skl_set_cdclk(dev_priv, &cdclk_state);
> > > +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   static int bxt_calc_cdclk(int min_cdclk)
> > > @@ -1355,7 +1359,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
> > >   }
> > >   static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	int vco = cdclk_state->vco;
> > > @@ -1408,11 +1413,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > >   		bxt_de_pll_enable(dev_priv, vco);
> > >   	val = divider | skl_cdclk_decimal(cdclk);
> > > -	/*
> > > -	 * FIXME if only the cd2x divider needs changing, it could be done
> > > -	 * without shutting off the pipe (if only one pipe is active).
> > > -	 */
> > > -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > > +	if (pipe == INVALID_PIPE)
> > > +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > > +	else
> > > +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
> > >   	/*
> > >   	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
> > >   	 * enable otherwise.
> > > @@ -1421,6 +1425,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> > >   		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
> > >   	I915_WRITE(CDCLK_CTL, val);
> > > +	if (pipe != INVALID_PIPE)
> > > +		intel_wait_for_vblank(dev_priv, pipe);
> > > +
> > >   	mutex_lock(&dev_priv->pcu_lock);
> > >   	/*
> > >   	 * The timeout isn't specified, the 2ms used here is based on
> > > @@ -1525,7 +1532,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
> > >   	}
> > >   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> > > -	bxt_set_cdclk(dev_priv, &cdclk_state);
> > > +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -1543,7 +1550,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.vco = 0;
> > >   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
> > > -	bxt_set_cdclk(dev_priv, &cdclk_state);
> > > +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   static int cnl_calc_cdclk(int min_cdclk)
> > > @@ -1663,7 +1670,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
> > >   }
> > >   static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	int cdclk = cdclk_state->cdclk;
> > >   	int vco = cdclk_state->vco;
> > > @@ -1704,13 +1712,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> > >   		cnl_cdclk_pll_enable(dev_priv, vco);
> > >   	val = divider | skl_cdclk_decimal(cdclk);
> > > -	/*
> > > -	 * FIXME if only the cd2x divider needs changing, it could be done
> > > -	 * without shutting off the pipe (if only one pipe is active).
> > > -	 */
> > > -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > > +	if (pipe == INVALID_PIPE)
> > > +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> > > +	else
> > > +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
> > >   	I915_WRITE(CDCLK_CTL, val);
> > > +	if (pipe != INVALID_PIPE)
> > > +		intel_wait_for_vblank(dev_priv, pipe);
> > > +
> > >   	/* inform PCU of the change */
> > >   	mutex_lock(&dev_priv->pcu_lock);
> > >   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > > @@ -1847,10 +1857,12 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
> > >   }
> > >   static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > > -			  const struct intel_cdclk_state *cdclk_state)
> > > +			  const struct intel_cdclk_state *cdclk_state,
> > > +			  enum pipe pipe)
> > >   {
> > >   	unsigned int cdclk = cdclk_state->cdclk;
> > >   	unsigned int vco = cdclk_state->vco;
> > > +	u32 val;
> > >   	int ret;
> > >   	mutex_lock(&dev_priv->pcu_lock);
> > > @@ -1872,8 +1884,15 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> > >   	if (dev_priv->cdclk.hw.vco != vco)
> > >   		cnl_cdclk_pll_enable(dev_priv, vco);
> > > -	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
> > > -			      skl_cdclk_decimal(cdclk));
> > > +	val = skl_cdclk_decimal(cdclk);
> > > +	if (pipe == INVALID_PIPE)
> > > +		val |= ICL_CDCLK_CD2X_PIPE_NONE;
> > > +	else
> > > +		val |= ICL_CDCLK_CD2X_PIPE(pipe);
> > > +	I915_WRITE(CDCLK_CTL, val);
> > > +
> > > +	if (pipe != INVALID_PIPE)
> > > +		intel_wait_for_vblank(dev_priv, pipe);
> > >   	mutex_lock(&dev_priv->pcu_lock);
> > >   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> > > @@ -2002,7 +2021,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
> > >   	sanitized_state.voltage_level =
> > >   				icl_calc_voltage_level(sanitized_state.cdclk);
> > > -	icl_set_cdclk(dev_priv, &sanitized_state);
> > > +	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -2020,7 +2039,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.vco = 0;
> > >   	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
> > > -	icl_set_cdclk(dev_priv, &cdclk_state);
> > > +	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -2048,7 +2067,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
> > >   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> > > -	cnl_set_cdclk(dev_priv, &cdclk_state);
> > > +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -2066,7 +2085,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
> > >   	cdclk_state.vco = 0;
> > >   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
> > > -	cnl_set_cdclk(dev_priv, &cdclk_state);
> > > +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
> > >   }
> > >   /**
> > > @@ -2086,6 +2105,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
> > >   }
> > >   /**
> > > + * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
> > > + * @a: first CDCLK state
> > > + * @b: second CDCLK state
> > > + *
> > > + * Returns:
> > > + * True if the CDCLK states require just a cd2x divider update, false if not.
> > > + */
> > > +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> > > +				   const struct intel_cdclk_state *a,
> > > +				   const struct intel_cdclk_state *b)
> > > +{
> > > +	/* Older hw doesn't have the capability */
> > > +	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
> > > +		return false;
> > > +
> > > +	return a->cdclk != b->cdclk &&
> > > +		a->vco == b->vco &&
> > > +		a->ref == b->ref;
> > > +}
> > > +
> > > +/**
> > >    * intel_cdclk_changed - Determine if two CDCLK states are different
> > >    * @a: first CDCLK state
> > >    * @b: second CDCLK state
> > > @@ -2133,12 +2173,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> > >    * intel_set_cdclk - Push the CDCLK state to the hardware
> > >    * @dev_priv: i915 device
> > >    * @cdclk_state: new CDCLK state
> > > + * @pipe: pipe with which to synchronize the update
> > >    *
> > >    * Program the hardware based on the passed in CDCLK state,
> > >    * if necessary.
> > >    */
> > > -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > > -		     const struct intel_cdclk_state *cdclk_state)
> > > +static void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > > +			    const struct intel_cdclk_state *cdclk_state,
> > > +			    enum pipe pipe)
> > >   {
> > >   	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
> > >   		return;
> > > @@ -2148,7 +2190,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > >   	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
> > > -	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> > > +	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
> > >   	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
> > >   		 "cdclk state doesn't match!\n")) {
> > > @@ -2157,6 +2199,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > >   	}
> > >   }
> > > +/**
> > > + * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
> > > + * @dev_priv: i915 device
> > > + * @old_state: old CDCLK state
> > > + * @new_state: new CDCLK state
> > > + * @pipe: pipe with which to synchronize the update
> > > + *
> > > + * Program the hardware before updating the HW plane state based on the passed
> > > + * in CDCLK state, if necessary.
> > > + */
> > > +void
> > > +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> > > +				 const struct intel_cdclk_state *old_state,
> > > +				 const struct intel_cdclk_state *new_state,
> > > +				 enum pipe pipe)
> > > +{
> > > +	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
> > > +		intel_set_cdclk(dev_priv, new_state, pipe);
> > > +}
> > > +
> > > +/**
> > > + * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
> > > + * @dev_priv: i915 device
> > > + * @old_state: old CDCLK state
> > > + * @new_state: new CDCLK state
> > > + * @pipe: pipe with which to synchronize the update
> > > + *
> > > + * Program the hardware after updating the HW plane state based on the passed
> > > + * in CDCLK state, if necessary.
> > > + */
> > > +void
> > > +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> > > +				  const struct intel_cdclk_state *old_state,
> > > +				  const struct intel_cdclk_state *new_state,
> > > +				  enum pipe pipe)
> > > +{
> > > +	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
> > > +		intel_set_cdclk(dev_priv, new_state, pipe);
> > > +}
> > > +
> > >   static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
> > >   				     int pixel_rate)
> > >   {
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index f7b44773e1e7..bcec03f43d3a 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -12982,6 +12982,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> > >   	intel_state->active_crtcs = dev_priv->active_crtcs;
> > >   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
> > >   	intel_state->cdclk.actual = dev_priv->cdclk.actual;
> > > +	intel_state->cdclk.pipe = INVALID_PIPE;
> > >   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
> > >   		if (new_crtc_state->active)
> > > @@ -13001,6 +13002,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> > >   	 * adjusted_mode bits in the crtc directly.
> > >   	 */
> > >   	if (dev_priv->display.modeset_calc_cdclk) {
> > > +		enum pipe pipe;
> > > +
> > >   		ret = dev_priv->display.modeset_calc_cdclk(state);
> > >   		if (ret < 0)
> > >   			return ret;
> > > @@ -13017,12 +13020,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
> > >   				return ret;
> > >   		}
> > > +		if (is_power_of_2(intel_state->active_crtcs)) {
> > > +			struct drm_crtc *crtc;
> > > +			struct drm_crtc_state *crtc_state;
> > > +
> > > +			pipe = ilog2(intel_state->active_crtcs);
> > > +			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
> > > +			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> > > +			if (crtc_state && needs_modeset(crtc_state))
> > > +				pipe = INVALID_PIPE;
> > > +		} else {
> > > +			pipe = INVALID_PIPE;
> > > +		}
> > > +
> > >   		/* All pipes must be switched off while we change the cdclk. */
> > > -		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> > > -					      &intel_state->cdclk.actual)) {
> > > +		if (pipe != INVALID_PIPE &&
> > > +		    intel_cdclk_needs_cd2x_update(dev_priv,
> > > +						  &dev_priv->cdclk.actual,
> > > +						  &intel_state->cdclk.actual)) {
> > > +			ret = intel_lock_all_pipes(state);
> > > +			if (ret < 0)
> > > +				return ret;
> > > +
> > > +			intel_state->cdclk.pipe = pipe;
> > > +		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> > > +						     &intel_state->cdclk.actual)) {
> > >   			ret = intel_modeset_all_pipes(state);
> > >   			if (ret < 0)
> > >   				return ret;
> > > +
> > > +			intel_state->cdclk.pipe = INVALID_PIPE;
> > >   		}
> > >   		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
> > > @@ -13431,7 +13458,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> > >   	if (intel_state->modeset) {
> > >   		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
> > > -		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> > > +		intel_set_cdclk_pre_plane_update(dev_priv,
> > > +						 &intel_state->cdclk.actual,
> > > +						 &dev_priv->cdclk.actual,
> > > +						 intel_state->cdclk.pipe);
> > >   		/*
> > >   		 * SKL workaround: bspec recommends we disable the SAGV when we
> > > @@ -13460,6 +13490,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
> > >   	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
> > >   	dev_priv->display.update_crtcs(state);
> > > +	if (intel_state->modeset)
> > > +		intel_set_cdclk_post_plane_update(dev_priv,
> > > +						  &intel_state->cdclk.actual,
> > > +						  &dev_priv->cdclk.actual,
> > > +						  intel_state->cdclk.pipe);
> > > +
> > >   	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
> > >   	 * already, but still need the state for the delayed optimization. To
> > >   	 * fix this:
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index 85dd6a9d1e42..d22a0e92e0d4 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -559,6 +559,8 @@ struct intel_atomic_state {
> > >   		int force_min_cdclk;
> > >   		bool force_min_cdclk_changed;
> > > +		/* pipe to which cd2x update is synchronized */
> > > +		enum pipe pipe;
> > >   	} cdclk;
> > >   	bool dpll_set, modeset;
> > > @@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
> > >   void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
> > >   void intel_update_cdclk(struct drm_i915_private *dev_priv);
> > >   void intel_update_rawclk(struct drm_i915_private *dev_priv);
> > > +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> > > +				   const struct intel_cdclk_state *a,
> > > +				   const struct intel_cdclk_state *b);
> > >   bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
> > >   			       const struct intel_cdclk_state *b);
> > >   bool intel_cdclk_changed(const struct intel_cdclk_state *a,
> > >   			 const struct intel_cdclk_state *b);
> > >   void intel_cdclk_swap_state(struct intel_atomic_state *state);
> > > -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> > > -		     const struct intel_cdclk_state *cdclk_state);
> > > +void
> > > +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> > > +				 const struct intel_cdclk_state *old_state,
> > > +				 const struct intel_cdclk_state *new_state,
> > > +				 enum pipe pipe);
> > > +void
> > > +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> > > +				  const struct intel_cdclk_state *old_state,
> > > +				  const struct intel_cdclk_state *new_state,
> > > +				  enum pipe pipe);
> > >   void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
> > >   			    const char *context);
> _______________________________________________
> 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

[PATCH v9 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Imre Deak]

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

If we have only a single active pipe and the cdclk change only requires
the cd2x divider to be updated bxt+ can do the update with forcing a full
modeset on the pipe. Try to hook that up.

v2:
- Wait for vblank after an optimized CDCLK change.
- Avoid optimization if the pipe needs a modeset (or was disabled).
- Split CDCLK change to a pre/post plane update step.
v3:
- Use correct version of CDCLK state as old state. (Ville)
- Remove unused intel_cdclk_can_skip_modeset()
v4:
- For consistency call intel_set_cdclk_post_plane_update() only during
  modesets (and not fastsets).
v5:
- Remove the logic to update the CD2X divider on-the-fly on ICL, since
  only a divider of 1 is supported there. Clint also noticed that the
  pipe select bits in CDCLK_CTL are oddly defined on ICL, it's not clear
  yet whether that's only an error in the specification.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
Tested-by: Abhay Kumar <abhay.kumar@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   3 +-
 drivers/gpu/drm/i915/intel_cdclk.c   | 135 +++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
 drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
 4 files changed, 163 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 78e6bb4a54bf..1cb8d99ba0c3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -282,7 +282,8 @@ struct drm_i915_display_funcs {
 	void (*get_cdclk)(struct drm_i915_private *dev_priv,
 			  struct intel_cdclk_state *cdclk_state);
 	void (*set_cdclk)(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state);
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe);
 	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
 			     enum i9xx_plane_id i9xx_plane);
 	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index 2fc443468706..b8cd481f5e33 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -517,7 +517,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
 }
 
 static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val, cmd = cdclk_state->voltage_level;
@@ -599,7 +600,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
 }
 
 static void chv_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val, cmd = cdclk_state->voltage_level;
@@ -698,7 +700,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
 }
 
 static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	u32 val;
@@ -988,7 +991,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
 }
 
 static void skl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1159,7 +1163,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
 	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
 
-	skl_set_cdclk(dev_priv, &cdclk_state);
+	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -1177,7 +1181,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
 
-	skl_set_cdclk(dev_priv, &cdclk_state);
+	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 static int bxt_calc_cdclk(int min_cdclk)
@@ -1356,7 +1360,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
 }
 
 static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1409,11 +1414,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		bxt_de_pll_enable(dev_priv, vco);
 
 	val = divider | skl_cdclk_decimal(cdclk);
-	/*
-	 * FIXME if only the cd2x divider needs changing, it could be done
-	 * without shutting off the pipe (if only one pipe is active).
-	 */
-	val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	if (pipe == INVALID_PIPE)
+		val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	else
+		val |= BXT_CDCLK_CD2X_PIPE(pipe);
 	/*
 	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
 	 * enable otherwise.
@@ -1422,6 +1426,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 	I915_WRITE(CDCLK_CTL, val);
 
+	if (pipe != INVALID_PIPE)
+		intel_wait_for_vblank(dev_priv, pipe);
+
 	mutex_lock(&dev_priv->pcu_lock);
 	/*
 	 * The timeout isn't specified, the 2ms used here is based on
@@ -1526,7 +1533,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
 	}
 	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
 
-	bxt_set_cdclk(dev_priv, &cdclk_state);
+	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -1544,7 +1551,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
 
-	bxt_set_cdclk(dev_priv, &cdclk_state);
+	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 static int cnl_calc_cdclk(int min_cdclk)
@@ -1664,7 +1671,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
 }
 
 static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	int cdclk = cdclk_state->cdclk;
 	int vco = cdclk_state->vco;
@@ -1705,13 +1713,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
 		cnl_cdclk_pll_enable(dev_priv, vco);
 
 	val = divider | skl_cdclk_decimal(cdclk);
-	/*
-	 * FIXME if only the cd2x divider needs changing, it could be done
-	 * without shutting off the pipe (if only one pipe is active).
-	 */
-	val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	if (pipe == INVALID_PIPE)
+		val |= BXT_CDCLK_CD2X_PIPE_NONE;
+	else
+		val |= BXT_CDCLK_CD2X_PIPE(pipe);
 	I915_WRITE(CDCLK_CTL, val);
 
+	if (pipe != INVALID_PIPE)
+		intel_wait_for_vblank(dev_priv, pipe);
+
 	/* inform PCU of the change */
 	mutex_lock(&dev_priv->pcu_lock);
 	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
@@ -1848,7 +1858,8 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
 }
 
 static void icl_set_cdclk(struct drm_i915_private *dev_priv,
-			  const struct intel_cdclk_state *cdclk_state)
+			  const struct intel_cdclk_state *cdclk_state,
+			  enum pipe pipe)
 {
 	unsigned int cdclk = cdclk_state->cdclk;
 	unsigned int vco = cdclk_state->vco;
@@ -1873,6 +1884,11 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
 	if (dev_priv->cdclk.hw.vco != vco)
 		cnl_cdclk_pll_enable(dev_priv, vco);
 
+	/*
+	 * On ICL CD2X_DIV can only be 1, so we'll never end up changing the
+	 * divider here synchronized to a pipe while CDCLK is on, nor will we
+	 * need the corresponding vblank wait.
+	 */
 	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
 			      skl_cdclk_decimal(cdclk));
 
@@ -2003,7 +2019,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
 	sanitized_state.voltage_level =
 				icl_calc_voltage_level(sanitized_state.cdclk);
 
-	icl_set_cdclk(dev_priv, &sanitized_state);
+	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
 }
 
 /**
@@ -2021,7 +2037,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
 
-	icl_set_cdclk(dev_priv, &cdclk_state);
+	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2049,7 +2065,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
 	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
 
-	cnl_set_cdclk(dev_priv, &cdclk_state);
+	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2067,7 +2083,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
 	cdclk_state.vco = 0;
 	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
 
-	cnl_set_cdclk(dev_priv, &cdclk_state);
+	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
 }
 
 /**
@@ -2087,6 +2103,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 }
 
 /**
+ * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
+ * @a: first CDCLK state
+ * @b: second CDCLK state
+ *
+ * Returns:
+ * True if the CDCLK states require just a cd2x divider update, false if not.
+ */
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+				   const struct intel_cdclk_state *a,
+				   const struct intel_cdclk_state *b)
+{
+	/* Older hw doesn't have the capability */
+	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
+		return false;
+
+	return a->cdclk != b->cdclk &&
+		a->vco == b->vco &&
+		a->ref == b->ref;
+}
+
+/**
  * intel_cdclk_changed - Determine if two CDCLK states are different
  * @a: first CDCLK state
  * @b: second CDCLK state
@@ -2134,12 +2171,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
  * intel_set_cdclk - Push the CDCLK state to the hardware
  * @dev_priv: i915 device
  * @cdclk_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
  *
  * Program the hardware based on the passed in CDCLK state,
  * if necessary.
  */
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-		     const struct intel_cdclk_state *cdclk_state)
+static void intel_set_cdclk(struct drm_i915_private *dev_priv,
+			    const struct intel_cdclk_state *cdclk_state,
+			    enum pipe pipe)
 {
 	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
 		return;
@@ -2149,7 +2188,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 
 	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
 
-	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
+	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
 
 	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
 		 "cdclk state doesn't match!\n")) {
@@ -2158,6 +2197,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
 	}
 }
 
+/**
+ * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware before updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+				 const struct intel_cdclk_state *old_state,
+				 const struct intel_cdclk_state *new_state,
+				 enum pipe pipe)
+{
+	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
+		intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
+/**
+ * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
+ * @dev_priv: i915 device
+ * @old_state: old CDCLK state
+ * @new_state: new CDCLK state
+ * @pipe: pipe with which to synchronize the update
+ *
+ * Program the hardware after updating the HW plane state based on the passed
+ * in CDCLK state, if necessary.
+ */
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *old_state,
+				  const struct intel_cdclk_state *new_state,
+				  enum pipe pipe)
+{
+	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
+		intel_set_cdclk(dev_priv, new_state, pipe);
+}
+
 static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
 				     int pixel_rate)
 {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f92bc1853595..6f4db68059cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12999,6 +12999,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	intel_state->active_crtcs = dev_priv->active_crtcs;
 	intel_state->cdclk.logical = dev_priv->cdclk.logical;
 	intel_state->cdclk.actual = dev_priv->cdclk.actual;
+	intel_state->cdclk.pipe = INVALID_PIPE;
 
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		if (new_crtc_state->active)
@@ -13018,6 +13019,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	 * adjusted_mode bits in the crtc directly.
 	 */
 	if (dev_priv->display.modeset_calc_cdclk) {
+		enum pipe pipe;
+
 		ret = dev_priv->display.modeset_calc_cdclk(state);
 		if (ret < 0)
 			return ret;
@@ -13034,12 +13037,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 				return ret;
 		}
 
+		if (is_power_of_2(intel_state->active_crtcs)) {
+			struct drm_crtc *crtc;
+			struct drm_crtc_state *crtc_state;
+
+			pipe = ilog2(intel_state->active_crtcs);
+			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+			if (crtc_state && needs_modeset(crtc_state))
+				pipe = INVALID_PIPE;
+		} else {
+			pipe = INVALID_PIPE;
+		}
+
 		/* All pipes must be switched off while we change the cdclk. */
-		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
-					      &intel_state->cdclk.actual)) {
+		if (pipe != INVALID_PIPE &&
+		    intel_cdclk_needs_cd2x_update(dev_priv,
+						  &dev_priv->cdclk.actual,
+						  &intel_state->cdclk.actual)) {
+			ret = intel_lock_all_pipes(state);
+			if (ret < 0)
+				return ret;
+
+			intel_state->cdclk.pipe = pipe;
+		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
+						     &intel_state->cdclk.actual)) {
 			ret = intel_modeset_all_pipes(state);
 			if (ret < 0)
 				return ret;
+
+			intel_state->cdclk.pipe = INVALID_PIPE;
 		}
 
 		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
@@ -13448,7 +13475,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	if (intel_state->modeset) {
 		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
 
-		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
+		intel_set_cdclk_pre_plane_update(dev_priv,
+						 &intel_state->cdclk.actual,
+						 &dev_priv->cdclk.actual,
+						 intel_state->cdclk.pipe);
 
 		/*
 		 * SKL workaround: bspec recommends we disable the SAGV when we
@@ -13477,6 +13507,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	dev_priv->display.update_crtcs(state);
 
+	if (intel_state->modeset)
+		intel_set_cdclk_post_plane_update(dev_priv,
+						  &intel_state->cdclk.actual,
+						  &dev_priv->cdclk.actual,
+						  intel_state->cdclk.pipe);
+
 	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
 	 * already, but still need the state for the delayed optimization. To
 	 * fix this:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 23968df4e3ea..f8c988fe4516 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -559,6 +559,8 @@ struct intel_atomic_state {
 
 		int force_min_cdclk;
 		bool force_min_cdclk_changed;
+		/* pipe to which cd2x update is synchronized */
+		enum pipe pipe;
 	} cdclk;
 
 	bool dpll_set, modeset;
@@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
 void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
 void intel_update_cdclk(struct drm_i915_private *dev_priv);
 void intel_update_rawclk(struct drm_i915_private *dev_priv);
+bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
+				   const struct intel_cdclk_state *a,
+				   const struct intel_cdclk_state *b);
 bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
 bool intel_cdclk_changed(const struct intel_cdclk_state *a,
 			 const struct intel_cdclk_state *b);
 void intel_cdclk_swap_state(struct intel_atomic_state *state);
-void intel_set_cdclk(struct drm_i915_private *dev_priv,
-		     const struct intel_cdclk_state *cdclk_state);
+void
+intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
+				 const struct intel_cdclk_state *old_state,
+				 const struct intel_cdclk_state *new_state,
+				 enum pipe pipe);
+void
+intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
+				  const struct intel_cdclk_state *old_state,
+				  const struct intel_cdclk_state *new_state,
+				  enum pipe pipe);
 void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 			    const char *context);
 
-- 
2.13.2

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

Re: [PATCH v9 4/4] drm/i915: Skip modeset for cdclk changes if possible

[Clinton Taylor]

Looks good.

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>


On 3/27/19 3:13 AM, Imre Deak wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If we have only a single active pipe and the cdclk change only requires
> the cd2x divider to be updated bxt+ can do the update with forcing a full
> modeset on the pipe. Try to hook that up.
>
> v2:
> - Wait for vblank after an optimized CDCLK change.
> - Avoid optimization if the pipe needs a modeset (or was disabled).
> - Split CDCLK change to a pre/post plane update step.
> v3:
> - Use correct version of CDCLK state as old state. (Ville)
> - Remove unused intel_cdclk_can_skip_modeset()
> v4:
> - For consistency call intel_set_cdclk_post_plane_update() only during
>    modesets (and not fastsets).
> v5:
> - Remove the logic to update the CD2X divider on-the-fly on ICL, since
>    only a divider of 1 is supported there. Clint also noticed that the
>    pipe select bits in CDCLK_CTL are oddly defined on ICL, it's not clear
>    yet whether that's only an error in the specification.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> Tested-by: Abhay Kumar <abhay.kumar@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |   3 +-
>   drivers/gpu/drm/i915/intel_cdclk.c   | 135 +++++++++++++++++++++++++++--------
>   drivers/gpu/drm/i915/intel_display.c |  42 ++++++++++-
>   drivers/gpu/drm/i915/intel_drv.h     |  17 ++++-
>   4 files changed, 163 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 78e6bb4a54bf..1cb8d99ba0c3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -282,7 +282,8 @@ struct drm_i915_display_funcs {
>   	void (*get_cdclk)(struct drm_i915_private *dev_priv,
>   			  struct intel_cdclk_state *cdclk_state);
>   	void (*set_cdclk)(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state);
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe);
>   	int (*get_fifo_size)(struct drm_i915_private *dev_priv,
>   			     enum i9xx_plane_id i9xx_plane);
>   	int (*compute_pipe_wm)(struct intel_crtc_state *cstate);
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 2fc443468706..b8cd481f5e33 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -517,7 +517,8 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
>   }
>   
>   static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val, cmd = cdclk_state->voltage_level;
> @@ -599,7 +600,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv,
>   }
>   
>   static void chv_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val, cmd = cdclk_state->voltage_level;
> @@ -698,7 +700,8 @@ static void bdw_get_cdclk(struct drm_i915_private *dev_priv,
>   }
>   
>   static void bdw_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	u32 val;
> @@ -988,7 +991,8 @@ static void skl_dpll0_disable(struct drm_i915_private *dev_priv)
>   }
>   
>   static void skl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1159,7 +1163,7 @@ void skl_init_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.cdclk = skl_calc_cdclk(0, cdclk_state.vco);
>   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	skl_set_cdclk(dev_priv, &cdclk_state);
> +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -1177,7 +1181,7 @@ void skl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = skl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	skl_set_cdclk(dev_priv, &cdclk_state);
> +	skl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   static int bxt_calc_cdclk(int min_cdclk)
> @@ -1356,7 +1360,8 @@ static void bxt_de_pll_enable(struct drm_i915_private *dev_priv, int vco)
>   }
>   
>   static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1409,11 +1414,10 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		bxt_de_pll_enable(dev_priv, vco);
>   
>   	val = divider | skl_cdclk_decimal(cdclk);
> -	/*
> -	 * FIXME if only the cd2x divider needs changing, it could be done
> -	 * without shutting off the pipe (if only one pipe is active).
> -	 */
> -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	if (pipe == INVALID_PIPE)
> +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	else
> +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
>   	/*
>   	 * Disable SSA Precharge when CD clock frequency < 500 MHz,
>   	 * enable otherwise.
> @@ -1422,6 +1426,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>   		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>   	I915_WRITE(CDCLK_CTL, val);
>   
> +	if (pipe != INVALID_PIPE)
> +		intel_wait_for_vblank(dev_priv, pipe);
> +
>   	mutex_lock(&dev_priv->pcu_lock);
>   	/*
>   	 * The timeout isn't specified, the 2ms used here is based on
> @@ -1526,7 +1533,7 @@ void bxt_init_cdclk(struct drm_i915_private *dev_priv)
>   	}
>   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
>   
> -	bxt_set_cdclk(dev_priv, &cdclk_state);
> +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -1544,7 +1551,7 @@ void bxt_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = bxt_calc_voltage_level(cdclk_state.cdclk);
>   
> -	bxt_set_cdclk(dev_priv, &cdclk_state);
> +	bxt_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   static int cnl_calc_cdclk(int min_cdclk)
> @@ -1664,7 +1671,8 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco)
>   }
>   
>   static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	int cdclk = cdclk_state->cdclk;
>   	int vco = cdclk_state->vco;
> @@ -1705,13 +1713,15 @@ static void cnl_set_cdclk(struct drm_i915_private *dev_priv,
>   		cnl_cdclk_pll_enable(dev_priv, vco);
>   
>   	val = divider | skl_cdclk_decimal(cdclk);
> -	/*
> -	 * FIXME if only the cd2x divider needs changing, it could be done
> -	 * without shutting off the pipe (if only one pipe is active).
> -	 */
> -	val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	if (pipe == INVALID_PIPE)
> +		val |= BXT_CDCLK_CD2X_PIPE_NONE;
> +	else
> +		val |= BXT_CDCLK_CD2X_PIPE(pipe);
>   	I915_WRITE(CDCLK_CTL, val);
>   
> +	if (pipe != INVALID_PIPE)
> +		intel_wait_for_vblank(dev_priv, pipe);
> +
>   	/* inform PCU of the change */
>   	mutex_lock(&dev_priv->pcu_lock);
>   	sandybridge_pcode_write(dev_priv, SKL_PCODE_CDCLK_CONTROL,
> @@ -1848,7 +1858,8 @@ static int icl_calc_cdclk_pll_vco(struct drm_i915_private *dev_priv, int cdclk)
>   }
>   
>   static void icl_set_cdclk(struct drm_i915_private *dev_priv,
> -			  const struct intel_cdclk_state *cdclk_state)
> +			  const struct intel_cdclk_state *cdclk_state,
> +			  enum pipe pipe)
>   {
>   	unsigned int cdclk = cdclk_state->cdclk;
>   	unsigned int vco = cdclk_state->vco;
> @@ -1873,6 +1884,11 @@ static void icl_set_cdclk(struct drm_i915_private *dev_priv,
>   	if (dev_priv->cdclk.hw.vco != vco)
>   		cnl_cdclk_pll_enable(dev_priv, vco);
>   
> +	/*
> +	 * On ICL CD2X_DIV can only be 1, so we'll never end up changing the
> +	 * divider here synchronized to a pipe while CDCLK is on, nor will we
> +	 * need the corresponding vblank wait.
> +	 */
>   	I915_WRITE(CDCLK_CTL, ICL_CDCLK_CD2X_PIPE_NONE |
>   			      skl_cdclk_decimal(cdclk));
>   
> @@ -2003,7 +2019,7 @@ void icl_init_cdclk(struct drm_i915_private *dev_priv)
>   	sanitized_state.voltage_level =
>   				icl_calc_voltage_level(sanitized_state.cdclk);
>   
> -	icl_set_cdclk(dev_priv, &sanitized_state);
> +	icl_set_cdclk(dev_priv, &sanitized_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2021,7 +2037,7 @@ void icl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = icl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	icl_set_cdclk(dev_priv, &cdclk_state);
> +	icl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2049,7 +2065,7 @@ void cnl_init_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = cnl_cdclk_pll_vco(dev_priv, cdclk_state.cdclk);
>   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	cnl_set_cdclk(dev_priv, &cdclk_state);
> +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2067,7 +2083,7 @@ void cnl_uninit_cdclk(struct drm_i915_private *dev_priv)
>   	cdclk_state.vco = 0;
>   	cdclk_state.voltage_level = cnl_calc_voltage_level(cdclk_state.cdclk);
>   
> -	cnl_set_cdclk(dev_priv, &cdclk_state);
> +	cnl_set_cdclk(dev_priv, &cdclk_state, INVALID_PIPE);
>   }
>   
>   /**
> @@ -2087,6 +2103,27 @@ bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
>   }
>   
>   /**
> + * intel_cdclk_needs_cd2x_update - Determine if two CDCLK states require a cd2x divider update
> + * @a: first CDCLK state
> + * @b: second CDCLK state
> + *
> + * Returns:
> + * True if the CDCLK states require just a cd2x divider update, false if not.
> + */
> +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> +				   const struct intel_cdclk_state *a,
> +				   const struct intel_cdclk_state *b)
> +{
> +	/* Older hw doesn't have the capability */
> +	if (INTEL_GEN(dev_priv) < 10 && !IS_GEN9_LP(dev_priv))
> +		return false;
> +
> +	return a->cdclk != b->cdclk &&
> +		a->vco == b->vco &&
> +		a->ref == b->ref;
> +}
> +
> +/**
>    * intel_cdclk_changed - Determine if two CDCLK states are different
>    * @a: first CDCLK state
>    * @b: second CDCLK state
> @@ -2134,12 +2171,14 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>    * intel_set_cdclk - Push the CDCLK state to the hardware
>    * @dev_priv: i915 device
>    * @cdclk_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
>    *
>    * Program the hardware based on the passed in CDCLK state,
>    * if necessary.
>    */
> -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> -		     const struct intel_cdclk_state *cdclk_state)
> +static void intel_set_cdclk(struct drm_i915_private *dev_priv,
> +			    const struct intel_cdclk_state *cdclk_state,
> +			    enum pipe pipe)
>   {
>   	if (!intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state))
>   		return;
> @@ -2149,7 +2188,7 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>   
>   	intel_dump_cdclk_state(cdclk_state, "Changing CDCLK to");
>   
> -	dev_priv->display.set_cdclk(dev_priv, cdclk_state);
> +	dev_priv->display.set_cdclk(dev_priv, cdclk_state, pipe);
>   
>   	if (WARN(intel_cdclk_changed(&dev_priv->cdclk.hw, cdclk_state),
>   		 "cdclk state doesn't match!\n")) {
> @@ -2158,6 +2197,46 @@ void intel_set_cdclk(struct drm_i915_private *dev_priv,
>   	}
>   }
>   
> +/**
> + * intel_set_cdclk_pre_plane_update - Push the CDCLK state to the hardware
> + * @dev_priv: i915 device
> + * @old_state: old CDCLK state
> + * @new_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
> + *
> + * Program the hardware before updating the HW plane state based on the passed
> + * in CDCLK state, if necessary.
> + */
> +void
> +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> +				 const struct intel_cdclk_state *old_state,
> +				 const struct intel_cdclk_state *new_state,
> +				 enum pipe pipe)
> +{
> +	if (pipe == INVALID_PIPE || old_state->cdclk <= new_state->cdclk)
> +		intel_set_cdclk(dev_priv, new_state, pipe);
> +}
> +
> +/**
> + * intel_set_cdclk_post_plane_update - Push the CDCLK state to the hardware
> + * @dev_priv: i915 device
> + * @old_state: old CDCLK state
> + * @new_state: new CDCLK state
> + * @pipe: pipe with which to synchronize the update
> + *
> + * Program the hardware after updating the HW plane state based on the passed
> + * in CDCLK state, if necessary.
> + */
> +void
> +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> +				  const struct intel_cdclk_state *old_state,
> +				  const struct intel_cdclk_state *new_state,
> +				  enum pipe pipe)
> +{
> +	if (pipe != INVALID_PIPE && old_state->cdclk > new_state->cdclk)
> +		intel_set_cdclk(dev_priv, new_state, pipe);
> +}
> +
>   static int intel_pixel_rate_to_cdclk(struct drm_i915_private *dev_priv,
>   				     int pixel_rate)
>   {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f92bc1853595..6f4db68059cf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12999,6 +12999,7 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   	intel_state->active_crtcs = dev_priv->active_crtcs;
>   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
>   	intel_state->cdclk.actual = dev_priv->cdclk.actual;
> +	intel_state->cdclk.pipe = INVALID_PIPE;
>   
>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
>   		if (new_crtc_state->active)
> @@ -13018,6 +13019,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   	 * adjusted_mode bits in the crtc directly.
>   	 */
>   	if (dev_priv->display.modeset_calc_cdclk) {
> +		enum pipe pipe;
> +
>   		ret = dev_priv->display.modeset_calc_cdclk(state);
>   		if (ret < 0)
>   			return ret;
> @@ -13034,12 +13037,36 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   				return ret;
>   		}
>   
> +		if (is_power_of_2(intel_state->active_crtcs)) {
> +			struct drm_crtc *crtc;
> +			struct drm_crtc_state *crtc_state;
> +
> +			pipe = ilog2(intel_state->active_crtcs);
> +			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
> +			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +			if (crtc_state && needs_modeset(crtc_state))
> +				pipe = INVALID_PIPE;
> +		} else {
> +			pipe = INVALID_PIPE;
> +		}
> +
>   		/* All pipes must be switched off while we change the cdclk. */
> -		if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> -					      &intel_state->cdclk.actual)) {
> +		if (pipe != INVALID_PIPE &&
> +		    intel_cdclk_needs_cd2x_update(dev_priv,
> +						  &dev_priv->cdclk.actual,
> +						  &intel_state->cdclk.actual)) {
> +			ret = intel_lock_all_pipes(state);
> +			if (ret < 0)
> +				return ret;
> +
> +			intel_state->cdclk.pipe = pipe;
> +		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
> +						     &intel_state->cdclk.actual)) {
>   			ret = intel_modeset_all_pipes(state);
>   			if (ret < 0)
>   				return ret;
> +
> +			intel_state->cdclk.pipe = INVALID_PIPE;
>   		}
>   
>   		DRM_DEBUG_KMS("New cdclk calculated to be logical %u kHz, actual %u kHz\n",
> @@ -13448,7 +13475,10 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>   	if (intel_state->modeset) {
>   		drm_atomic_helper_update_legacy_modeset_state(state->dev, state);
>   
> -		intel_set_cdclk(dev_priv, &dev_priv->cdclk.actual);
> +		intel_set_cdclk_pre_plane_update(dev_priv,
> +						 &intel_state->cdclk.actual,
> +						 &dev_priv->cdclk.actual,
> +						 intel_state->cdclk.pipe);
>   
>   		/*
>   		 * SKL workaround: bspec recommends we disable the SAGV when we
> @@ -13477,6 +13507,12 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>   	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>   	dev_priv->display.update_crtcs(state);
>   
> +	if (intel_state->modeset)
> +		intel_set_cdclk_post_plane_update(dev_priv,
> +						  &intel_state->cdclk.actual,
> +						  &dev_priv->cdclk.actual,
> +						  intel_state->cdclk.pipe);
> +
>   	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
>   	 * already, but still need the state for the delayed optimization. To
>   	 * fix this:
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 23968df4e3ea..f8c988fe4516 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -559,6 +559,8 @@ struct intel_atomic_state {
>   
>   		int force_min_cdclk;
>   		bool force_min_cdclk_changed;
> +		/* pipe to which cd2x update is synchronized */
> +		enum pipe pipe;
>   	} cdclk;
>   
>   	bool dpll_set, modeset;
> @@ -1694,13 +1696,24 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv);
>   void intel_update_max_cdclk(struct drm_i915_private *dev_priv);
>   void intel_update_cdclk(struct drm_i915_private *dev_priv);
>   void intel_update_rawclk(struct drm_i915_private *dev_priv);
> +bool intel_cdclk_needs_cd2x_update(struct drm_i915_private *dev_priv,
> +				   const struct intel_cdclk_state *a,
> +				   const struct intel_cdclk_state *b);
>   bool intel_cdclk_needs_modeset(const struct intel_cdclk_state *a,
>   			       const struct intel_cdclk_state *b);
>   bool intel_cdclk_changed(const struct intel_cdclk_state *a,
>   			 const struct intel_cdclk_state *b);
>   void intel_cdclk_swap_state(struct intel_atomic_state *state);
> -void intel_set_cdclk(struct drm_i915_private *dev_priv,
> -		     const struct intel_cdclk_state *cdclk_state);
> +void
> +intel_set_cdclk_pre_plane_update(struct drm_i915_private *dev_priv,
> +				 const struct intel_cdclk_state *old_state,
> +				 const struct intel_cdclk_state *new_state,
> +				 enum pipe pipe);
> +void
> +intel_set_cdclk_post_plane_update(struct drm_i915_private *dev_priv,
> +				  const struct intel_cdclk_state *old_state,
> +				  const struct intel_cdclk_state *new_state,
> +				  enum pipe pipe);
>   void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>   			    const char *context);
>   
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
URL   : https://patchwork.freedesktop.org/series/58273/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
-O:drivers/gpu/drm/i915/intel_cdclk.c:2204:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2193:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2245:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2245:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2234:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2234:29: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3559:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3562:16: warning: expression using sizeof(void)

Commit: drm/i915: Save the old CDCLK atomic state
Okay!

Commit: drm/i915: Remove redundant store of logical CDCLK state
Okay!

Commit: drm/i915: Skip modeset for cdclk changes if possible
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3562:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3563:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
URL   : https://patchwork.freedesktop.org/series/58273/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5781 -> Patchwork_12531
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58273/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] +55

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#105128] / [fdo#107139]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_addfb_basic@addfb25-y-tiled-small:
    - fi-byt-n2820:       NOTRUN -> SKIP [fdo#109271] +56

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_busy@basic-flip-c:
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-byt-n2820:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] +52

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          PASS -> FAIL [fdo#103167]
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS +1

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210


Participating hosts (43 -> 40)
------------------------------

  Additional (5): fi-byt-j1900 fi-hsw-peppy fi-icl-y fi-byt-n2820 fi-bsw-kefka 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-byt-clapper fi-skl-6700k2 


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

    * Linux: CI_DRM_5781 -> Patchwork_12531

  CI_DRM_5781: c6c48b877f32e50dd365885c3b90b988957a8216 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4893: 27939a179fcd143e3a179ffc7b0372718259587a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12531: da8ee225449fbb3d43e61e1490f0d0104b115e93 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

da8ee225449f drm/i915: Skip modeset for cdclk changes if possible
6a17ac210939 drm/i915: Remove redundant store of logical CDCLK state
029cba7e560c drm/i915: Save the old CDCLK atomic state
c3e1f7af4155 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
URL   : https://patchwork.freedesktop.org/series/58273/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5781_full -> Patchwork_12531_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_properties@connector-properties-atomic:
    - shard-iclb:         PASS -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parse@batch-without-end:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@gem_exec_suspend@basic-s3:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@gem_pwrite@stolen-normal:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +117

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107803] / [fdo#107807]

  * igt@i915_pm_rpm@i2c:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_atomic_transition@1x-modeset-transitions-fencing:
    - shard-snb:          PASS -> SKIP [fdo#109271] +4

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          PASS -> FAIL [fdo#109660]

  * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-d:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_busy@extended-modeset-hang-oldfb-render-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_color@pipe-a-gamma:
    - shard-skl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-snb:          PASS -> DMESG-WARN [fdo#102365]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_flip@2x-flip-vs-dpms-off-vs-modeset:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +34

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247] +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +20

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_psr2_su@page_flip:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +18
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_psr@sprite_render:
    - shard-iclb:         PASS -> FAIL [fdo#107383] +2

  * igt@kms_psr@suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107773]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-iclb:         PASS -> FAIL [fdo#104894]

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
    - shard-apl:          PASS -> FAIL [fdo#104894]

  * igt@prime_nv_api@i915_nv_import_twice_check_flink_name:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +1

  
#### Possible fixes ####

  * igt@gem_pwrite@big-cpu-fbr:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS +1

  * igt@i915_pm_rpm@gem-idle:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#105363] -> PASS

  * igt@kms_flip_tiling@flip-x-tiled:
    - shard-iclb:         FAIL [fdo#108303] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +10

  * {igt@kms_plane@pixel-format-pipe-a-planes-source-clamping}:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-iclb:         DMESG-WARN [fdo#106885] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] -> PASS +1

  * igt@kms_setmode@basic:
    - shard-hsw:          FAIL [fdo#99912] -> PASS
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]

  * igt@i915_pm_rpm@pc8-residency:
    - shard-skl:          SKIP [fdo#109271] -> INCOMPLETE [fdo#107807]

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> FAIL [fdo#109358]

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         SKIP [fdo#109441] -> FAIL [fdo#107383] +1

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

  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109358]: https://bugs.freedesktop.org/show_bug.cgi?id=109358
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5781 -> Patchwork_12531

  CI_DRM_5781: c6c48b877f32e50dd365885c3b90b988957a8216 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4893: 27939a179fcd143e3a179ffc7b0372718259587a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12531: da8ee225449fbb3d43e61e1490f0d0104b115e93 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: ✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Imre Deak]

On Thu, Mar 21, 2019 at 12:32:47AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
> URL   : https://patchwork.freedesktop.org/series/58273/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5781_full -> Patchwork_12531_full
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12531_full absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12531_full, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_12531_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@kms_properties@connector-properties-atomic:
>     - shard-iclb:         PASS -> FAIL

(kms_properties:4576) DEBUG: Testing property "max bpc"

with the 'max bpc' property set to 6.

This results in the driver failing the modeset with:

<7>[ 2131.233688] [drm:intel_dp_compute_config [i915]] Force DSC en = 1
<7>[ 2131.233716] [drm:intel_dp_compute_config [i915]] No DSC support for less than 8bpc
<7>[ 2131.233747] [drm:intel_atomic_check [i915]] Encoder config failure: -22

which is in turn caused by an earlier failure in
igt@kms_dp_dsc@basic-dsc-enable-eDP:

(kms_dp_dsc:3075) CRITICAL: Test assertion failure function update_display, file ../tests/kms_dp_dsc.c:182:
(kms_dp_dsc:3075) CRITICAL: Failed assertion: is_dp_dsc_enabled(data)

That one in turn is caused by an incorrect assumption by kms_dp_dsc in
that the DSC compression will only get enabled via a full modeset, while
the test does only a fastset. The test also fails to restore the
original value to the i915_dsc_fec_support debugfs file during an assert
failure, leading to kms_properties seeing the unxepcted
intel_dp->force_dsc_en=true value.

Adding Manasi to look at the kms_dp_dsc fail.

> 
>   
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_12531_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_exec_parse@batch-without-end:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109289]
> 
>   * igt@gem_exec_suspend@basic-s3:
>     - shard-apl:          PASS -> INCOMPLETE [fdo#103927]
> 
>   * igt@gem_pwrite@stolen-normal:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] +117
> 
>   * igt@i915_pm_rpm@gem-execbuf-stress:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#107803] / [fdo#107807]
> 
>   * igt@i915_pm_rpm@i2c:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1
> 
>   * igt@i915_selftest@live_workarounds:
>     - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]
> 
>   * igt@kms_atomic_transition@1x-modeset-transitions-fencing:
>     - shard-snb:          PASS -> SKIP [fdo#109271] +4
> 
>   * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
>     - shard-apl:          PASS -> FAIL [fdo#109660]
> 
>   * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
>     - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-d:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3
> 
>   * igt@kms_busy@extended-modeset-hang-oldfb-render-e:
>     - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1
> 
>   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
>     - shard-glk:          PASS -> DMESG-WARN [fdo#107956]
>     - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]
> 
>   * igt@kms_color@pipe-a-gamma:
>     - shard-skl:          PASS -> FAIL [fdo#104782]
> 
>   * igt@kms_cursor_crc@cursor-256x256-suspend:
>     - shard-snb:          PASS -> DMESG-WARN [fdo#102365]
> 
>   * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
>     - shard-iclb:         PASS -> FAIL [fdo#103355]
> 
>   * igt@kms_flip@2x-flip-vs-dpms-off-vs-modeset:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109274]
> 
>   * igt@kms_flip@flip-vs-expired-vblank-interruptible:
>     - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]
> 
>   * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
>     - shard-iclb:         PASS -> FAIL [fdo#103167] +6
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] +34
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
>     - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247] +1
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
>     - shard-iclb:         PASS -> FAIL [fdo#109247] +20
> 
>   * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
>     - shard-skl:          PASS -> FAIL [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
>     - shard-skl:          NOTRUN -> FAIL [fdo#107815]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
>     - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
>     - shard-kbl:          NOTRUN -> FAIL [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
>     - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1
> 
>   * igt@kms_psr2_su@page_flip:
>     - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +18
>     - shard-iclb:         PASS -> SKIP [fdo#109642]
> 
>   * igt@kms_psr@psr2_cursor_plane_onoff:
>     - shard-iclb:         PASS -> SKIP [fdo#109441] +1
> 
>   * igt@kms_psr@sprite_render:
>     - shard-iclb:         PASS -> FAIL [fdo#107383] +2
> 
>   * igt@kms_psr@suspend:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#107773]
> 
>   * igt@kms_setmode@basic:
>     - shard-skl:          NOTRUN -> FAIL [fdo#99912]
> 
>   * igt@kms_vblank@pipe-a-ts-continuation-suspend:
>     - shard-iclb:         PASS -> FAIL [fdo#104894]
> 
>   * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
>     - shard-apl:          PASS -> FAIL [fdo#104894]
> 
>   * igt@prime_nv_api@i915_nv_import_twice_check_flink_name:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109291]
> 
>   * igt@prime_vgem@fence-wait-bsd2:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +1
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_pwrite@big-cpu-fbr:
>     - shard-iclb:         TIMEOUT [fdo#109673] -> PASS +1
> 
>   * igt@i915_pm_rpm@gem-idle:
>     - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1
> 
>   * igt@kms_flip@flip-vs-expired-vblank:
>     - shard-glk:          FAIL [fdo#105363] -> PASS
> 
>   * igt@kms_flip_tiling@flip-x-tiled:
>     - shard-iclb:         FAIL [fdo#108303] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
>     - shard-iclb:         FAIL [fdo#103167] -> PASS +6
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
>     - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
>     - shard-iclb:         FAIL [fdo#109247] -> PASS +10
> 
>   * {igt@kms_plane@pixel-format-pipe-a-planes-source-clamping}:
>     - shard-glk:          SKIP [fdo#109271] -> PASS
> 
>   * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
>     - shard-iclb:         DMESG-WARN [fdo#106885] -> PASS
> 
>   * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
>     - shard-skl:          FAIL [fdo#108145] -> PASS
> 
>   * igt@kms_psr@psr2_cursor_mmap_gtt:
>     - shard-iclb:         SKIP [fdo#109441] -> PASS +1
> 
>   * igt@kms_psr@sprite_mmap_cpu:
>     - shard-iclb:         FAIL [fdo#107383] -> PASS +1
> 
>   * igt@kms_setmode@basic:
>     - shard-hsw:          FAIL [fdo#99912] -> PASS
>     - shard-kbl:          FAIL [fdo#99912] -> PASS
> 
>   
> #### Warnings ####
> 
>   * igt@i915_pm_rpm@modeset-non-lpsp-stress:
>     - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]
> 
>   * igt@i915_pm_rpm@pc8-residency:
>     - shard-skl:          SKIP [fdo#109271] -> INCOMPLETE [fdo#107807]
> 
>   * igt@kms_dp_dsc@basic-dsc-enable-edp:
>     - shard-iclb:         SKIP [fdo#109349] -> FAIL [fdo#109358]
> 
>   * igt@kms_psr@psr2_primary_mmap_gtt:
>     - shard-iclb:         SKIP [fdo#109441] -> FAIL [fdo#107383] +1
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
>   [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
>   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
>   [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
>   [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
>   [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
>   [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
>   [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
>   [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
>   [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
>   [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
>   [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
>   [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
>   [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
>   [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
>   [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
>   [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
>   [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
>   [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
>   [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
>   [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
>   [fdo#109358]: https://bugs.freedesktop.org/show_bug.cgi?id=109358
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
>   [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
>   [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
>   [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
> 
> 
> Participating hosts (10 -> 10)
> ------------------------------
> 
>   No changes in participating hosts
> 
> 
> Build changes
> -------------
> 
>     * Linux: CI_DRM_5781 -> Patchwork_12531
> 
>   CI_DRM_5781: c6c48b877f32e50dd365885c3b90b988957a8216 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4893: 27939a179fcd143e3a179ffc7b0372718259587a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_12531: da8ee225449fbb3d43e61e1490f0d0104b115e93 @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12531/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Imre Deak]

On Thu, Mar 21, 2019 at 05:56:39PM +0200, Imre Deak wrote:
> On Thu, Mar 21, 2019 at 12:32:47AM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
> > URL   : https://patchwork.freedesktop.org/series/58273/
> > State : failure

Thanks for the patches and reviews, pushed to -dinq.

> > 
> > == Summary ==
> > 
> > CI Bug Log - changes from CI_DRM_5781_full -> Patchwork_12531_full
> > ====================================================
> > 
> > Summary
> > -------
> > 
> >   **FAILURE**
> > 
> >   Serious unknown changes coming with Patchwork_12531_full absolutely need to be
> >   verified manually.
> >   
> >   If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_12531_full, please notify your bug team to allow them
> >   to document this new failure mode, which will reduce false positives in CI.
> > 
> >   
> > 
> > Possible new issues
> > -------------------
> > 
> >   Here are the unknown changes that may have been introduced in Patchwork_12531_full:
> > 
> > ### IGT changes ###
> > 
> > #### Possible regressions ####
> > 
> >   * igt@kms_properties@connector-properties-atomic:
> >     - shard-iclb:         PASS -> FAIL
> 
> (kms_properties:4576) DEBUG: Testing property "max bpc"
> 
> with the 'max bpc' property set to 6.
> 
> This results in the driver failing the modeset with:
> 
> <7>[ 2131.233688] [drm:intel_dp_compute_config [i915]] Force DSC en = 1
> <7>[ 2131.233716] [drm:intel_dp_compute_config [i915]] No DSC support for less than 8bpc
> <7>[ 2131.233747] [drm:intel_atomic_check [i915]] Encoder config failure: -22
> 
> which is in turn caused by an earlier failure in
> igt@kms_dp_dsc@basic-dsc-enable-eDP:
> 
> (kms_dp_dsc:3075) CRITICAL: Test assertion failure function update_display, file ../tests/kms_dp_dsc.c:182:
> (kms_dp_dsc:3075) CRITICAL: Failed assertion: is_dp_dsc_enabled(data)
> 
> That one in turn is caused by an incorrect assumption by kms_dp_dsc in
> that the DSC compression will only get enabled via a full modeset, while
> the test does only a fastset. The test also fails to restore the
> original value to the i915_dsc_fec_support debugfs file during an assert
> failure, leading to kms_properties seeing the unxepcted
> intel_dp->force_dsc_en=true value.
> 
> Adding Manasi to look at the kms_dp_dsc fail.
> 
> > 
> >   
> > Known issues
> > ------------
> > 
> >   Here are the changes found in Patchwork_12531_full that come from known issues:
> > 
> > ### IGT changes ###
> > 
> > #### Issues hit ####
> > 
> >   * igt@gem_exec_parse@batch-without-end:
> >     - shard-iclb:         NOTRUN -> SKIP [fdo#109289]
> > 
> >   * igt@gem_exec_suspend@basic-s3:
> >     - shard-apl:          PASS -> INCOMPLETE [fdo#103927]
> > 
> >   * igt@gem_pwrite@stolen-normal:
> >     - shard-skl:          NOTRUN -> SKIP [fdo#109271] +117
> > 
> >   * igt@i915_pm_rpm@gem-execbuf-stress:
> >     - shard-skl:          PASS -> INCOMPLETE [fdo#107803] / [fdo#107807]
> > 
> >   * igt@i915_pm_rpm@i2c:
> >     - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1
> > 
> >   * igt@i915_selftest@live_workarounds:
> >     - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]
> > 
> >   * igt@kms_atomic_transition@1x-modeset-transitions-fencing:
> >     - shard-snb:          PASS -> SKIP [fdo#109271] +4
> > 
> >   * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
> >     - shard-apl:          PASS -> FAIL [fdo#109660]
> > 
> >   * igt@kms_atomic_transition@5x-modeset-transitions-nonblocking:
> >     - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13
> > 
> >   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
> >     - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]
> > 
> >   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-d:
> >     - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3
> > 
> >   * igt@kms_busy@extended-modeset-hang-oldfb-render-e:
> >     - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1
> > 
> >   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
> >     - shard-glk:          PASS -> DMESG-WARN [fdo#107956]
> >     - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]
> > 
> >   * igt@kms_color@pipe-a-gamma:
> >     - shard-skl:          PASS -> FAIL [fdo#104782]
> > 
> >   * igt@kms_cursor_crc@cursor-256x256-suspend:
> >     - shard-snb:          PASS -> DMESG-WARN [fdo#102365]
> > 
> >   * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
> >     - shard-iclb:         PASS -> FAIL [fdo#103355]
> > 
> >   * igt@kms_flip@2x-flip-vs-dpms-off-vs-modeset:
> >     - shard-iclb:         NOTRUN -> SKIP [fdo#109274]
> > 
> >   * igt@kms_flip@flip-vs-expired-vblank-interruptible:
> >     - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]
> > 
> >   * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
> >     - shard-iclb:         PASS -> FAIL [fdo#103167] +6
> > 
> >   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt:
> >     - shard-snb:          NOTRUN -> SKIP [fdo#109271] +34
> > 
> >   * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
> >     - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +2
> > 
> >   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
> >     - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247] +1
> > 
> >   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
> >     - shard-iclb:         PASS -> FAIL [fdo#109247] +20
> > 
> >   * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
> >     - shard-skl:          PASS -> FAIL [fdo#108145]
> > 
> >   * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
> >     - shard-skl:          NOTRUN -> FAIL [fdo#107815]
> > 
> >   * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
> >     - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]
> > 
> >   * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
> >     - shard-kbl:          NOTRUN -> FAIL [fdo#108145]
> > 
> >   * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
> >     - shard-skl:          NOTRUN -> FAIL [fdo#108145] +1
> > 
> >   * igt@kms_psr2_su@page_flip:
> >     - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +18
> >     - shard-iclb:         PASS -> SKIP [fdo#109642]
> > 
> >   * igt@kms_psr@psr2_cursor_plane_onoff:
> >     - shard-iclb:         PASS -> SKIP [fdo#109441] +1
> > 
> >   * igt@kms_psr@sprite_render:
> >     - shard-iclb:         PASS -> FAIL [fdo#107383] +2
> > 
> >   * igt@kms_psr@suspend:
> >     - shard-skl:          PASS -> INCOMPLETE [fdo#107773]
> > 
> >   * igt@kms_setmode@basic:
> >     - shard-skl:          NOTRUN -> FAIL [fdo#99912]
> > 
> >   * igt@kms_vblank@pipe-a-ts-continuation-suspend:
> >     - shard-iclb:         PASS -> FAIL [fdo#104894]
> > 
> >   * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
> >     - shard-apl:          PASS -> FAIL [fdo#104894]
> > 
> >   * igt@prime_nv_api@i915_nv_import_twice_check_flink_name:
> >     - shard-iclb:         NOTRUN -> SKIP [fdo#109291]
> > 
> >   * igt@prime_vgem@fence-wait-bsd2:
> >     - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +1
> > 
> >   
> > #### Possible fixes ####
> > 
> >   * igt@gem_pwrite@big-cpu-fbr:
> >     - shard-iclb:         TIMEOUT [fdo#109673] -> PASS +1
> > 
> >   * igt@i915_pm_rpm@gem-idle:
> >     - shard-skl:          INCOMPLETE [fdo#107807] -> PASS +1
> > 
> >   * igt@kms_flip@flip-vs-expired-vblank:
> >     - shard-glk:          FAIL [fdo#105363] -> PASS
> > 
> >   * igt@kms_flip_tiling@flip-x-tiled:
> >     - shard-iclb:         FAIL [fdo#108303] -> PASS
> > 
> >   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
> >     - shard-iclb:         FAIL [fdo#103167] -> PASS +6
> > 
> >   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-render:
> >     - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS
> > 
> >   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
> >     - shard-iclb:         FAIL [fdo#109247] -> PASS +10
> > 
> >   * {igt@kms_plane@pixel-format-pipe-a-planes-source-clamping}:
> >     - shard-glk:          SKIP [fdo#109271] -> PASS
> > 
> >   * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
> >     - shard-iclb:         DMESG-WARN [fdo#106885] -> PASS
> > 
> >   * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
> >     - shard-skl:          FAIL [fdo#108145] -> PASS
> > 
> >   * igt@kms_psr@psr2_cursor_mmap_gtt:
> >     - shard-iclb:         SKIP [fdo#109441] -> PASS +1
> > 
> >   * igt@kms_psr@sprite_mmap_cpu:
> >     - shard-iclb:         FAIL [fdo#107383] -> PASS +1
> > 
> >   * igt@kms_setmode@basic:
> >     - shard-hsw:          FAIL [fdo#99912] -> PASS
> >     - shard-kbl:          FAIL [fdo#99912] -> PASS
> > 
> >   
> > #### Warnings ####
> > 
> >   * igt@i915_pm_rpm@modeset-non-lpsp-stress:
> >     - shard-skl:          INCOMPLETE [fdo#107807] -> SKIP [fdo#109271]
> > 
> >   * igt@i915_pm_rpm@pc8-residency:
> >     - shard-skl:          SKIP [fdo#109271] -> INCOMPLETE [fdo#107807]
> > 
> >   * igt@kms_dp_dsc@basic-dsc-enable-edp:
> >     - shard-iclb:         SKIP [fdo#109349] -> FAIL [fdo#109358]
> > 
> >   * igt@kms_psr@psr2_primary_mmap_gtt:
> >     - shard-iclb:         SKIP [fdo#109441] -> FAIL [fdo#107383] +1
> > 
> >   
> >   {name}: This element is suppressed. This means it is ignored when computing
> >           the status of the difference (SUCCESS, WARNING, or FAILURE).
> > 
> >   [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
> >   [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
> >   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
> >   [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
> >   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
> >   [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
> >   [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
> >   [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
> >   [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
> >   [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
> >   [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
> >   [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
> >   [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
> >   [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
> >   [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
> >   [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
> >   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
> >   [fdo#108303]: https://bugs.freedesktop.org/show_bug.cgi?id=108303
> >   [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
> >   [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
> >   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
> >   [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
> >   [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
> >   [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
> >   [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
> >   [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
> >   [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
> >   [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
> >   [fdo#109358]: https://bugs.freedesktop.org/show_bug.cgi?id=109358
> >   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
> >   [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
> >   [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
> >   [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
> >   [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
> > 
> > 
> > Participating hosts (10 -> 10)
> > ------------------------------
> > 
> >   No changes in participating hosts
> > 
> > 
> > Build changes
> > -------------
> > 
> >     * Linux: CI_DRM_5781 -> Patchwork_12531
> > 
> >   CI_DRM_5781: c6c48b877f32e50dd365885c3b90b988957a8216 @ git://anongit.freedesktop.org/gfx-ci/linux
> >   IGT_4893: 27939a179fcd143e3a179ffc7b0372718259587a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
> >   Patchwork_12531: da8ee225449fbb3d43e61e1490f0d0104b115e93 @ git://anongit.freedesktop.org/gfx-ci/linux
> >   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> > 
> > == Logs ==
> > 
> > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12531/
> _______________________________________________
> 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

Re: [CI 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Clinton Taylor]

On 3/20/19 6:54 AM, Imre Deak wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CDCLK has to be at least twice the BLCK regardless of audio. Audio
> driver has to probe using this hook and increase the clock even in
> absence of any display.
>
> v2: Use atomic refcount for get_power, put_power so that we can
>      call each once(Abhay).
> v3: Reset power well 2 to avoid any transaction on iDisp link
>      during cdclk change(Abhay).
> v4: Remove Power well 2 reset workaround(Ville).
> v5: Remove unwanted Power well 2 register defined in v4(Abhay).
> v6:
> - Use a dedicated flag instead of state->modeset for min CDCLK changes
> - Make get/put audio power domain symmetric
> - Rebased on top of intel_wakeref tracking changes.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> Tested-by: Abhay Kumar <abhay.kumar@intel.com>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h      |  3 ++
>   drivers/gpu/drm/i915/intel_audio.c   | 64 ++++++++++++++++++++++++++++++++++--
>   drivers/gpu/drm/i915/intel_cdclk.c   | 30 ++++++-----------
>   drivers/gpu/drm/i915/intel_display.c |  9 ++++-
>   drivers/gpu/drm/i915/intel_drv.h     |  3 ++
>   5 files changed, 86 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c65c2e6649df..6b10cee4e77f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1624,6 +1624,8 @@ struct drm_i915_private {
>   		struct intel_cdclk_state actual;
>   		/* The current hardware cdclk state */
>   		struct intel_cdclk_state hw;
> +
> +		int force_min_cdclk;
>   	} cdclk;
>   
>   	/**
> @@ -1743,6 +1745,7 @@ struct drm_i915_private {
>   	 *
>   	 */
>   	struct mutex av_mutex;
> +	int audio_power_refcount;
>   
>   	struct {
>   		struct mutex mutex;
> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
> index 502b57ce72ab..20324b0d34c7 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -741,18 +741,78 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
>   	}
>   }
>   
> +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
> +				  bool enable)
> +{
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_atomic_state *state;
> +	int ret;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +	state = drm_atomic_state_alloc(&dev_priv->drm);
> +	if (WARN_ON(!state))
> +		return;
> +
> +	state->acquire_ctx = &ctx;
> +
> +retry:
> +	to_intel_atomic_state(state)->cdclk.force_min_cdclk_changed = true;
> +	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
> +		enable ? 2 * 96000 : 0;
This will need to be revisited on later SOCs that may support cdclk 
rates < 192 Mhz.
> +
> +	/*
> +	 * Protects dev_priv->cdclk.force_min_cdclk
> +	 * Need to lock this here in case we have no active pipes
> +	 * and thus wouldn't lock it during the commit otherwise.
> +	 */
> +	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> +			       &ctx);
> +	if (!ret)
> +		ret = drm_atomic_commit(state);
> +
> +	if (ret == -EDEADLK) {
> +		drm_atomic_state_clear(state);
> +		drm_modeset_backoff(&ctx);
> +		goto retry;
> +	}
> +
> +	WARN_ON(ret);
> +
> +	drm_atomic_state_put(state);
> +
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +}
> +
>   static unsigned long i915_audio_component_get_power(struct device *kdev)
>   {
> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +	intel_wakeref_t ret;
> +
>   	/* Catch potential impedance mismatches before they occur! */
>   	BUILD_BUG_ON(sizeof(intel_wakeref_t) > sizeof(unsigned long));
>   
> -	return intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
> +	ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
> +
> +	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
> +	if (dev_priv->audio_power_refcount++ == 0)
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			glk_force_audio_cdclk(dev_priv, true);
> +
> +	return ret;
>   }
>   
>   static void i915_audio_component_put_power(struct device *kdev,
>   					   unsigned long cookie)
>   {
> -	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO, cookie);
> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
> +
> +	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
> +	if (--dev_priv->audio_power_refcount == 0)
> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			glk_force_audio_cdclk(dev_priv, false);
> +
> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO, cookie);
>   }
>   
>   static void i915_audio_component_codec_wake_override(struct device *kdev,
> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
> index 21fb4e0d6c4e..7dcca84f31d1 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -2187,19 +2187,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>   	/*
>   	 * According to BSpec, "The CD clock frequency must be at least twice
>   	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
> -	 *
> -	 * FIXME: Check the actual, not default, BCLK being used.
> -	 *
> -	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
> -	 * is required for audio probe, also when there are no audio capable
> -	 * displays connected at probe time. This leads to unnecessarily high
> -	 * CDCLK when audio is not required.
> -	 *
> -	 * FIXME: This limit is only applied when there are displays connected
> -	 * at probe time. If we probe without displays, we'll still end up using
> -	 * the platform minimum CDCLK, failing audio probe.
>   	 */
> -	if (INTEL_GEN(dev_priv) >= 9)
> +	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>   		min_cdclk = max(2 * 96000, min_cdclk);

Do we have plans to support a 48Mhz BCLK?


>   
>   	/*
> @@ -2239,7 +2228,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
>   		intel_state->min_cdclk[i] = min_cdclk;
>   	}
>   
> -	min_cdclk = 0;
> +	min_cdclk = intel_state->cdclk.force_min_cdclk;
>   	for_each_pipe(dev_priv, pipe)
>   		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
>   
> @@ -2300,7 +2289,8 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>   		vlv_calc_voltage_level(dev_priv, cdclk);
>   
>   	if (!intel_state->active_crtcs) {
> -		cdclk = vlv_calc_cdclk(dev_priv, 0);
> +		cdclk = vlv_calc_cdclk(dev_priv,
> +				       intel_state->cdclk.force_min_cdclk);
>   
>   		intel_state->cdclk.actual.cdclk = cdclk;
>   		intel_state->cdclk.actual.voltage_level =
> @@ -2333,7 +2323,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>   		bdw_calc_voltage_level(cdclk);
>   
>   	if (!intel_state->active_crtcs) {
> -		cdclk = bdw_calc_cdclk(0);
> +		cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>   
>   		intel_state->cdclk.actual.cdclk = cdclk;
>   		intel_state->cdclk.actual.voltage_level =
> @@ -2405,7 +2395,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>   		skl_calc_voltage_level(cdclk);
>   
>   	if (!intel_state->active_crtcs) {
> -		cdclk = skl_calc_cdclk(0, vco);
> +		cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
>   
>   		intel_state->cdclk.actual.vco = vco;
>   		intel_state->cdclk.actual.cdclk = cdclk;
> @@ -2444,10 +2434,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
>   
>   	if (!intel_state->active_crtcs) {
>   		if (IS_GEMINILAKE(dev_priv)) {
> -			cdclk = glk_calc_cdclk(0);
> +			cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>   			vco = glk_de_pll_vco(dev_priv, cdclk);
>   		} else {
> -			cdclk = bxt_calc_cdclk(0);
> +			cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>   			vco = bxt_de_pll_vco(dev_priv, cdclk);
>   		}
>   
> @@ -2483,7 +2473,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
>   		    cnl_compute_min_voltage_level(intel_state));
>   
>   	if (!intel_state->active_crtcs) {
> -		cdclk = cnl_calc_cdclk(0);
> +		cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>   		vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
>   
>   		intel_state->cdclk.actual.vco = vco;
> @@ -2519,7 +2509,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
>   		    cnl_compute_min_voltage_level(intel_state));
>   
>   	if (!intel_state->active_crtcs) {
> -		cdclk = icl_calc_cdclk(0, ref);
> +		cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
>   		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>   
>   		intel_state->cdclk.actual.vco = vco;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 61acbaf2af75..b4199cd53349 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12973,6 +12973,11 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>   		return -EINVAL;
>   	}
>   
> +	/* keep the current setting */
> +	if (!intel_state->cdclk.force_min_cdclk_changed)
> +		intel_state->cdclk.force_min_cdclk =
> +			dev_priv->cdclk.force_min_cdclk;
> +
>   	intel_state->modeset = true;
>   	intel_state->active_crtcs = dev_priv->active_crtcs;
>   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
> @@ -13068,7 +13073,7 @@ static int intel_atomic_check(struct drm_device *dev,
>   	struct drm_crtc *crtc;
>   	struct drm_crtc_state *old_crtc_state, *crtc_state;
>   	int ret, i;
> -	bool any_ms = false;
> +	bool any_ms = intel_state->cdclk.force_min_cdclk_changed;
>   
>   	/* Catch I915_MODE_FLAG_INHERITED */
>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
> @@ -13660,6 +13665,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>   		dev_priv->active_crtcs = intel_state->active_crtcs;
>   		dev_priv->cdclk.logical = intel_state->cdclk.logical;
>   		dev_priv->cdclk.actual = intel_state->cdclk.actual;
> +		dev_priv->cdclk.force_min_cdclk =
> +			intel_state->cdclk.force_min_cdclk;
>   	}
>   
>   	drm_atomic_state_get(state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d9f188ef21f4..0b84e557c267 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -556,6 +556,9 @@ struct intel_atomic_state {
>   		 * state only when all crtc's are DPMS off.
>   		 */
>   		struct intel_cdclk_state actual;
> +
> +		int force_min_cdclk;
> +		bool force_min_cdclk_changed;
>   	} cdclk;
>   
>   	bool dpll_set, modeset;

Some concerns about hard coded 96Mhz BCLK. however, that is the only 
frequency we currently support.

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>



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

Re: [CI 1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

[Jani Nikula]

On Thu, 21 Mar 2019, Clinton Taylor <Clinton.A.Taylor@intel.com> wrote:
> On 3/20/19 6:54 AM, Imre Deak wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> CDCLK has to be at least twice the BLCK regardless of audio. Audio
>> driver has to probe using this hook and increase the clock even in
>> absence of any display.
>>
>> v2: Use atomic refcount for get_power, put_power so that we can
>>      call each once(Abhay).
>> v3: Reset power well 2 to avoid any transaction on iDisp link
>>      during cdclk change(Abhay).
>> v4: Remove Power well 2 reset workaround(Ville).
>> v5: Remove unwanted Power well 2 register defined in v4(Abhay).
>> v6:
>> - Use a dedicated flag instead of state->modeset for min CDCLK changes
>> - Make get/put audio power domain symmetric
>> - Rebased on top of intel_wakeref tracking changes.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
>> Tested-by: Abhay Kumar <abhay.kumar@intel.com>
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_drv.h      |  3 ++
>>   drivers/gpu/drm/i915/intel_audio.c   | 64 ++++++++++++++++++++++++++++++++++--
>>   drivers/gpu/drm/i915/intel_cdclk.c   | 30 ++++++-----------
>>   drivers/gpu/drm/i915/intel_display.c |  9 ++++-
>>   drivers/gpu/drm/i915/intel_drv.h     |  3 ++
>>   5 files changed, 86 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c65c2e6649df..6b10cee4e77f 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1624,6 +1624,8 @@ struct drm_i915_private {
>>   		struct intel_cdclk_state actual;
>>   		/* The current hardware cdclk state */
>>   		struct intel_cdclk_state hw;
>> +
>> +		int force_min_cdclk;
>>   	} cdclk;
>>   
>>   	/**
>> @@ -1743,6 +1745,7 @@ struct drm_i915_private {
>>   	 *
>>   	 */
>>   	struct mutex av_mutex;
>> +	int audio_power_refcount;
>>   
>>   	struct {
>>   		struct mutex mutex;
>> diff --git a/drivers/gpu/drm/i915/intel_audio.c b/drivers/gpu/drm/i915/intel_audio.c
>> index 502b57ce72ab..20324b0d34c7 100644
>> --- a/drivers/gpu/drm/i915/intel_audio.c
>> +++ b/drivers/gpu/drm/i915/intel_audio.c
>> @@ -741,18 +741,78 @@ void intel_init_audio_hooks(struct drm_i915_private *dev_priv)
>>   	}
>>   }
>>   
>> +static void glk_force_audio_cdclk(struct drm_i915_private *dev_priv,
>> +				  bool enable)
>> +{
>> +	struct drm_modeset_acquire_ctx ctx;
>> +	struct drm_atomic_state *state;
>> +	int ret;
>> +
>> +	drm_modeset_acquire_init(&ctx, 0);
>> +	state = drm_atomic_state_alloc(&dev_priv->drm);
>> +	if (WARN_ON(!state))
>> +		return;
>> +
>> +	state->acquire_ctx = &ctx;
>> +
>> +retry:
>> +	to_intel_atomic_state(state)->cdclk.force_min_cdclk_changed = true;
>> +	to_intel_atomic_state(state)->cdclk.force_min_cdclk =
>> +		enable ? 2 * 96000 : 0;
> This will need to be revisited on later SOCs that may support cdclk 
> rates < 192 Mhz.
>> +
>> +	/*
>> +	 * Protects dev_priv->cdclk.force_min_cdclk
>> +	 * Need to lock this here in case we have no active pipes
>> +	 * and thus wouldn't lock it during the commit otherwise.
>> +	 */
>> +	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
>> +			       &ctx);
>> +	if (!ret)
>> +		ret = drm_atomic_commit(state);
>> +
>> +	if (ret == -EDEADLK) {
>> +		drm_atomic_state_clear(state);
>> +		drm_modeset_backoff(&ctx);
>> +		goto retry;
>> +	}
>> +
>> +	WARN_ON(ret);
>> +
>> +	drm_atomic_state_put(state);
>> +
>> +	drm_modeset_drop_locks(&ctx);
>> +	drm_modeset_acquire_fini(&ctx);
>> +}
>> +
>>   static unsigned long i915_audio_component_get_power(struct device *kdev)
>>   {
>> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
>> +	intel_wakeref_t ret;
>> +
>>   	/* Catch potential impedance mismatches before they occur! */
>>   	BUILD_BUG_ON(sizeof(intel_wakeref_t) > sizeof(unsigned long));
>>   
>> -	return intel_display_power_get(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO);
>> +	ret = intel_display_power_get(dev_priv, POWER_DOMAIN_AUDIO);
>> +
>> +	/* Force CDCLK to 2*BCLK as long as we need audio to be powered. */
>> +	if (dev_priv->audio_power_refcount++ == 0)
>> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +			glk_force_audio_cdclk(dev_priv, true);
>> +
>> +	return ret;
>>   }
>>   
>>   static void i915_audio_component_put_power(struct device *kdev,
>>   					   unsigned long cookie)
>>   {
>> -	intel_display_power_put(kdev_to_i915(kdev), POWER_DOMAIN_AUDIO, cookie);
>> +	struct drm_i915_private *dev_priv = kdev_to_i915(kdev);
>> +
>> +	/* Stop forcing CDCLK to 2*BCLK if no need for audio to be powered. */
>> +	if (--dev_priv->audio_power_refcount == 0)
>> +		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +			glk_force_audio_cdclk(dev_priv, false);
>> +
>> +	intel_display_power_put(dev_priv, POWER_DOMAIN_AUDIO, cookie);
>>   }
>>   
>>   static void i915_audio_component_codec_wake_override(struct device *kdev,
>> diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
>> index 21fb4e0d6c4e..7dcca84f31d1 100644
>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>> @@ -2187,19 +2187,8 @@ int intel_crtc_compute_min_cdclk(const struct intel_crtc_state *crtc_state)
>>   	/*
>>   	 * According to BSpec, "The CD clock frequency must be at least twice
>>   	 * the frequency of the Azalia BCLK." and BCLK is 96 MHz by default.
>> -	 *
>> -	 * FIXME: Check the actual, not default, BCLK being used.
>> -	 *
>> -	 * FIXME: This does not depend on ->has_audio because the higher CDCLK
>> -	 * is required for audio probe, also when there are no audio capable
>> -	 * displays connected at probe time. This leads to unnecessarily high
>> -	 * CDCLK when audio is not required.
>> -	 *
>> -	 * FIXME: This limit is only applied when there are displays connected
>> -	 * at probe time. If we probe without displays, we'll still end up using
>> -	 * the platform minimum CDCLK, failing audio probe.
>>   	 */
>> -	if (INTEL_GEN(dev_priv) >= 9)
>> +	if (crtc_state->has_audio && INTEL_GEN(dev_priv) >= 9)
>>   		min_cdclk = max(2 * 96000, min_cdclk);
>
> Do we have plans to support a 48Mhz BCLK?

IIUC that can only be done when we don't care about audio, and AFAIK,
ahem, certain other OS has opted not to support this.

BR,
Jani.



>
>
>>   
>>   	/*
>> @@ -2239,7 +2228,7 @@ static int intel_compute_min_cdclk(struct drm_atomic_state *state)
>>   		intel_state->min_cdclk[i] = min_cdclk;
>>   	}
>>   
>> -	min_cdclk = 0;
>> +	min_cdclk = intel_state->cdclk.force_min_cdclk;
>>   	for_each_pipe(dev_priv, pipe)
>>   		min_cdclk = max(intel_state->min_cdclk[pipe], min_cdclk);
>>   
>> @@ -2300,7 +2289,8 @@ static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   		vlv_calc_voltage_level(dev_priv, cdclk);
>>   
>>   	if (!intel_state->active_crtcs) {
>> -		cdclk = vlv_calc_cdclk(dev_priv, 0);
>> +		cdclk = vlv_calc_cdclk(dev_priv,
>> +				       intel_state->cdclk.force_min_cdclk);
>>   
>>   		intel_state->cdclk.actual.cdclk = cdclk;
>>   		intel_state->cdclk.actual.voltage_level =
>> @@ -2333,7 +2323,7 @@ static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   		bdw_calc_voltage_level(cdclk);
>>   
>>   	if (!intel_state->active_crtcs) {
>> -		cdclk = bdw_calc_cdclk(0);
>> +		cdclk = bdw_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>>   
>>   		intel_state->cdclk.actual.cdclk = cdclk;
>>   		intel_state->cdclk.actual.voltage_level =
>> @@ -2405,7 +2395,7 @@ static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   		skl_calc_voltage_level(cdclk);
>>   
>>   	if (!intel_state->active_crtcs) {
>> -		cdclk = skl_calc_cdclk(0, vco);
>> +		cdclk = skl_calc_cdclk(intel_state->cdclk.force_min_cdclk, vco);
>>   
>>   		intel_state->cdclk.actual.vco = vco;
>>   		intel_state->cdclk.actual.cdclk = cdclk;
>> @@ -2444,10 +2434,10 @@ static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   
>>   	if (!intel_state->active_crtcs) {
>>   		if (IS_GEMINILAKE(dev_priv)) {
>> -			cdclk = glk_calc_cdclk(0);
>> +			cdclk = glk_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>>   			vco = glk_de_pll_vco(dev_priv, cdclk);
>>   		} else {
>> -			cdclk = bxt_calc_cdclk(0);
>> +			cdclk = bxt_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>>   			vco = bxt_de_pll_vco(dev_priv, cdclk);
>>   		}
>>   
>> @@ -2483,7 +2473,7 @@ static int cnl_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   		    cnl_compute_min_voltage_level(intel_state));
>>   
>>   	if (!intel_state->active_crtcs) {
>> -		cdclk = cnl_calc_cdclk(0);
>> +		cdclk = cnl_calc_cdclk(intel_state->cdclk.force_min_cdclk);
>>   		vco = cnl_cdclk_pll_vco(dev_priv, cdclk);
>>   
>>   		intel_state->cdclk.actual.vco = vco;
>> @@ -2519,7 +2509,7 @@ static int icl_modeset_calc_cdclk(struct drm_atomic_state *state)
>>   		    cnl_compute_min_voltage_level(intel_state));
>>   
>>   	if (!intel_state->active_crtcs) {
>> -		cdclk = icl_calc_cdclk(0, ref);
>> +		cdclk = icl_calc_cdclk(intel_state->cdclk.force_min_cdclk, ref);
>>   		vco = icl_calc_cdclk_pll_vco(dev_priv, cdclk);
>>   
>>   		intel_state->cdclk.actual.vco = vco;
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 61acbaf2af75..b4199cd53349 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12973,6 +12973,11 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
>>   		return -EINVAL;
>>   	}
>>   
>> +	/* keep the current setting */
>> +	if (!intel_state->cdclk.force_min_cdclk_changed)
>> +		intel_state->cdclk.force_min_cdclk =
>> +			dev_priv->cdclk.force_min_cdclk;
>> +
>>   	intel_state->modeset = true;
>>   	intel_state->active_crtcs = dev_priv->active_crtcs;
>>   	intel_state->cdclk.logical = dev_priv->cdclk.logical;
>> @@ -13068,7 +13073,7 @@ static int intel_atomic_check(struct drm_device *dev,
>>   	struct drm_crtc *crtc;
>>   	struct drm_crtc_state *old_crtc_state, *crtc_state;
>>   	int ret, i;
>> -	bool any_ms = false;
>> +	bool any_ms = intel_state->cdclk.force_min_cdclk_changed;
>>   
>>   	/* Catch I915_MODE_FLAG_INHERITED */
>>   	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
>> @@ -13660,6 +13665,8 @@ static int intel_atomic_commit(struct drm_device *dev,
>>   		dev_priv->active_crtcs = intel_state->active_crtcs;
>>   		dev_priv->cdclk.logical = intel_state->cdclk.logical;
>>   		dev_priv->cdclk.actual = intel_state->cdclk.actual;
>> +		dev_priv->cdclk.force_min_cdclk =
>> +			intel_state->cdclk.force_min_cdclk;
>>   	}
>>   
>>   	drm_atomic_state_get(state);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index d9f188ef21f4..0b84e557c267 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -556,6 +556,9 @@ struct intel_atomic_state {
>>   		 * state only when all crtc's are DPMS off.
>>   		 */
>>   		struct intel_cdclk_state actual;
>> +
>> +		int force_min_cdclk;
>> +		bool force_min_cdclk_changed;
>>   	} cdclk;
>>   
>>   	bool dpll_set, modeset;
>
> Some concerns about hard coded 96Mhz BCLK. however, that is the only 
> frequency we currently support.
>
> Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
>
>
>
> _______________________________________________
> 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

✗ Fi.CI.SPARSE: warning for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)
URL   : https://patchwork.freedesktop.org/series/58273/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled
-O:drivers/gpu/drm/i915/intel_cdclk.c:2204:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2193:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2245:29: warning: expression using sizeof(void)
-O:drivers/gpu/drm/i915/intel_cdclk.c:2245:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2234:29: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_cdclk.c:2234:29: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3566:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3569:16: warning: expression using sizeof(void)

Commit: drm/i915: Save the old CDCLK atomic state
Okay!

Commit: drm/i915: Remove redundant store of logical CDCLK state
Okay!

Commit: drm/i915: Skip modeset for cdclk changes if possible
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3569:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3570:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)
URL   : https://patchwork.freedesktop.org/series/58273/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5825 -> Patchwork_12613
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58273/revisions/2/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182] +1

  * igt@kms_busy@basic-flip-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-c:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +48

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-ilk-650:         DMESG-WARN [fdo#106387] -> PASS

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110210] -> PASS
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (43 -> 36)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-skl-iommu fi-bdw-samus 


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

    * Linux: CI_DRM_5825 -> Patchwork_12613

  CI_DRM_5825: 26df82b873a8a759a77ea2242f8d2be1f2531178 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4907: 7c8f2616fa0fd3ddb16e050c5b7ea9ce707abbe4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12613: d1b17d256c26b84d6d5a3db60686e7c39e24efb8 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d1b17d256c26 drm/i915: Skip modeset for cdclk changes if possible
e0d156de9197 drm/i915: Remove redundant store of logical CDCLK state
e2b849ebcc96 drm/i915: Save the old CDCLK atomic state
fbb16330c7e0 drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)
URL   : https://patchwork.freedesktop.org/series/58273/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5825_full -> Patchwork_12613_full
====================================================

Summary
-------

  **FAILURE**

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

  

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_universal_plane@cursor-fb-leak-pipe-a:
    - shard-iclb:         PASS -> FAIL

  
#### Warnings ####

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109801]

  * igt@gem_tiled_pread_pwrite:
    - shard-iclb:         PASS -> TIMEOUT [fdo#109673]

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566]

  * igt@i915_pm_rpm@sysfs-read:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1

  * igt@i915_selftest@live_requests:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109644]

  * igt@i915_selftest@mock_fence:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@i915_suspend@debugfs-reader:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109638] / [fdo#109745]

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109593]

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-iclb:         PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          PASS -> INCOMPLETE [fdo#109507]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +9

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +16

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +28

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +19

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          PASS -> FAIL [fdo#107815]

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@primary_mmap_gtt:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215]

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +1

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         PASS -> FAIL [fdo#104894]

  * igt@prime_nv_api@i915_nv_double_import:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291]

  * igt@runner@aborted:
    - shard-iclb:         NOTRUN -> ( 2 FAIL ) [fdo#109593] / [fdo#109745]

  * igt@tools_test@tools_test:
    - shard-skl:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_linear_blits@normal:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-FAIL -> PASS

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          FAIL [fdo#109660] -> PASS

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          FAIL [fdo#104873] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +17

  * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +2

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-skl:          FAIL [fdo#104894] -> PASS

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109745]: https://bugs.freedesktop.org/show_bug.cgi?id=109745
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


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

    * Linux: CI_DRM_5825 -> Patchwork_12613

  CI_DRM_5825: 26df82b873a8a759a77ea2242f8d2be1f2531178 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4907: 7c8f2616fa0fd3ddb16e050c5b7ea9ce707abbe4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12613: d1b17d256c26b84d6d5a3db60686e7c39e24efb8 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: ✗ Fi.CI.IGT: failure for series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)

[Imre Deak]

On Thu, Mar 28, 2019 at 03:53:37AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [CI,1/4] drm/i915: Force 2*96 MHz cdclk on glk/cnl when audio power is enabled (rev2)
> URL   : https://patchwork.freedesktop.org/series/58273/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5825_full -> Patchwork_12613_full
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12613_full absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12613_full, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_12613_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@kms_universal_plane@cursor-fb-leak-pipe-a:
>     - shard-iclb:         PASS -> FAIL

ICL shouldn't be affected by these changes since we'll always have
cdclk.pipe == INVALID_PIPE there. The failure looks like

https://bugs.freedesktop.org/show_bug.cgi?id=109814

not sure why it wasn't marked as a pre-existing issue.

> 
>   
> #### Warnings ####
> 
>   * igt@kms_dp_dsc@basic-dsc-enable-edp:
>     - shard-iclb:         SKIP [fdo#109349] -> FAIL

As above it's unrelated, the test is broken, should do a full modeset to
force DSC on, whereas it only does a fastset.

> 
>   
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_12613_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_ppgtt@blt-vs-render-ctx0:
>     - shard-iclb:         PASS -> INCOMPLETE [fdo#109801]
> 
>   * igt@gem_tiled_pread_pwrite:
>     - shard-iclb:         PASS -> TIMEOUT [fdo#109673]
> 
>   * igt@gem_workarounds@suspend-resume:
>     - shard-apl:          PASS -> DMESG-WARN [fdo#108566]
> 
>   * igt@i915_pm_rpm@sysfs-read:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#107807] +1
> 
>   * igt@i915_selftest@live_requests:
>     - shard-iclb:         PASS -> INCOMPLETE [fdo#109644]
> 
>   * igt@i915_selftest@mock_fence:
>     - shard-apl:          PASS -> INCOMPLETE [fdo#103927]
> 
>   * igt@i915_suspend@debugfs-reader:
>     - shard-iclb:         PASS -> DMESG-WARN [fdo#109638] / [fdo#109745]
> 
>   * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
>     - shard-iclb:         PASS -> FAIL [fdo#103355]
> 
>   * igt@kms_fbcon_fbt@fbc:
>     - shard-iclb:         PASS -> DMESG-WARN [fdo#109593]
> 
>   * igt@kms_flip@flip-vs-expired-vblank-interruptible:
>     - shard-iclb:         PASS -> FAIL [fdo#105363]
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#109507]
> 
>   * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +1
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
>     - shard-iclb:         PASS -> FAIL [fdo#103167] +9
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-spr-indfb-draw-mmap-gtt:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] +16
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-blt:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] +28
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
>     - shard-apl:          NOTRUN -> SKIP [fdo#109271]
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
>     - shard-iclb:         PASS -> FAIL [fdo#109247] +19
> 
>   * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
>     - shard-skl:          NOTRUN -> FAIL [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
>     - shard-skl:          PASS -> FAIL [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
>     - shard-skl:          PASS -> FAIL [fdo#107815]
> 
>   * igt@kms_psr2_su@page_flip:
>     - shard-iclb:         PASS -> SKIP [fdo#109642]
> 
>   * igt@kms_psr@primary_mmap_gtt:
>     - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215]
> 
>   * igt@kms_psr@psr2_cursor_render:
>     - shard-iclb:         PASS -> SKIP [fdo#109441] +2
> 
>   * igt@kms_universal_plane@disable-primary-vs-flip-pipe-f:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1
> 
>   * igt@kms_universal_plane@universal-plane-gen9-features-pipe-e:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5
> 
>   * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
>     - shard-apl:          PASS -> FAIL [fdo#104894] +1
> 
>   * igt@kms_vblank@pipe-c-ts-continuation-suspend:
>     - shard-iclb:         PASS -> FAIL [fdo#104894]
> 
>   * igt@prime_nv_api@i915_nv_double_import:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109291]
> 
>   * igt@runner@aborted:
>     - shard-iclb:         NOTRUN -> ( 2 FAIL ) [fdo#109593] / [fdo#109745]
> 
>   * igt@tools_test@tools_test:
>     - shard-skl:          PASS -> SKIP [fdo#109271]
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_linear_blits@normal:
>     - shard-iclb:         TIMEOUT [fdo#109673] -> PASS
> 
>   * igt@i915_pm_rpm@i2c:
>     - shard-iclb:         DMESG-FAIL -> PASS
> 
>   * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
>     - shard-apl:          FAIL [fdo#109660] -> PASS
> 
>   * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
>     - shard-glk:          FAIL [fdo#104873] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
>     - shard-iclb:         FAIL [fdo#103167] -> PASS +3
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
>     - shard-iclb:         FAIL [fdo#109247] -> PASS +17
> 
>   * {igt@kms_plane@pixel-format-pipe-c-planes-source-clamping}:
>     - shard-glk:          SKIP [fdo#109271] -> PASS
> 
>   * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
>     - shard-glk:          FAIL [fdo#108145] -> PASS
> 
>   * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
>     - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS
> 
>   * igt@kms_psr@psr2_primary_page_flip:
>     - shard-iclb:         SKIP [fdo#109441] -> PASS +2
> 
>   * igt@kms_psr@sprite_mmap_cpu:
>     - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +3
> 
>   * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
>     - shard-kbl:          FAIL [fdo#109016] -> PASS
> 
>   * igt@kms_setmode@basic:
>     - shard-kbl:          FAIL [fdo#99912] -> PASS
> 
>   * igt@kms_vblank@pipe-c-ts-continuation-suspend:
>     - shard-skl:          FAIL [fdo#104894] -> PASS
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
>   [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
>   [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
>   [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
>   [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
>   [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
>   [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
>   [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
>   [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
>   [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
>   [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
>   [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
>   [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
>   [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
>   [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
>   [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
>   [fdo#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
>   [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
>   [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
>   [fdo#109745]: https://bugs.freedesktop.org/show_bug.cgi?id=109745
>   [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
>   [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
>   [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
> 
> 
> Participating hosts (10 -> 9)
> ------------------------------
> 
>   Missing    (1): shard-hsw 
> 
> 
> Build changes
> -------------
> 
>     * Linux: CI_DRM_5825 -> Patchwork_12613
> 
>   CI_DRM_5825: 26df82b873a8a759a77ea2242f8d2be1f2531178 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4907: 7c8f2616fa0fd3ddb16e050c5b7ea9ce707abbe4 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_12613: d1b17d256c26b84d6d5a3db60686e7c39e24efb8 @ git://anongit.freedesktop.org/gfx-ci/linux
>   piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12613/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx