[PATCH 06/14] drm/i915/display: Only keep PSR enabled if there is active planes

José Roberto de Souza jose.souza at intel.com
Fri Sep 3 00:26:18 UTC 2021


PSR always had a requirement to only be enabled if there is active
planes but not following that never caused any issues.
But that changes in Alderlake-P, leaving PSR enabled without
active planes causes transcoder/port underruns.

Similar behavior was fixed during the pipe disable sequence by
commit 84030adb9e27 ("drm/i915/display: Disable audio, DRRS and PSR before planes").

Cc: Gwan-gyeong Mun <gwan-gyeong.mun at intel.com>
Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c     |   2 -
 drivers/gpu/drm/i915/display/intel_display.c |  14 +-
 drivers/gpu/drm/i915/display/intel_psr.c     | 155 ++++++++++++-------
 drivers/gpu/drm/i915/display/intel_psr.h     |   8 +-
 4 files changed, 111 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 23ef291f7b30a..0cb36044bd5bc 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3010,7 +3010,6 @@ static void intel_enable_ddi_dp(struct intel_atomic_state *state,
 		intel_dp_stop_link_train(intel_dp, crtc_state);
 
 	intel_edp_backlight_on(crtc_state, conn_state);
-	intel_psr_enable(intel_dp, crtc_state, conn_state);
 
 	if (!dig_port->lspcon.active || dig_port->dp.has_hdmi_sink)
 		intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
@@ -3231,7 +3230,6 @@ static void intel_ddi_update_pipe_dp(struct intel_atomic_state *state,
 
 	intel_ddi_set_dp_msa(crtc_state, conn_state);
 
-	intel_psr_update(intel_dp, crtc_state, conn_state);
 	intel_dp_set_infoframes(encoder, true, crtc_state, conn_state);
 	intel_drrs_update(intel_dp, crtc_state);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 134c792e1dbda..cd190d02c83d9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8087,10 +8087,12 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		if (bp_gamma)
 			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, hw.gamma_lut, bp_gamma);
 
-		PIPE_CONF_CHECK_BOOL(has_psr);
-		PIPE_CONF_CHECK_BOOL(has_psr2);
-		PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
-		PIPE_CONF_CHECK_I(dc3co_exitline);
+		if (current_config->active_planes) {
+			PIPE_CONF_CHECK_BOOL(has_psr);
+			PIPE_CONF_CHECK_BOOL(has_psr2);
+			PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
+			PIPE_CONF_CHECK_I(dc3co_exitline);
+		}
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -8147,7 +8149,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		PIPE_CONF_CHECK_I(min_voltage_level);
 	}
 
-	if (fastset && (current_config->has_psr || pipe_config->has_psr))
+	if (current_config->has_psr || pipe_config->has_psr)
 		PIPE_CONF_CHECK_X_WITH_MASK(infoframes.enable,
 					    ~intel_hdmi_infoframe_enable(DP_SDP_VSC));
 	else
@@ -10201,6 +10203,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 		intel_encoders_update_prepare(state);
 
 	intel_dbuf_pre_plane_update(state);
+	intel_psr_pre_plane_update(state);
 
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (new_crtc_state->uapi.async_flip)
@@ -10264,6 +10267,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	}
 
 	intel_dbuf_post_plane_update(state);
+	intel_psr_post_plane_update(state);
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
 		intel_post_plane_update(state, crtc);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 8c9bd5846a8d0..715e133d957e6 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1193,33 +1193,6 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
 	intel_psr_activate(intel_dp);
 }
 
-/**
- * intel_psr_enable - Enable PSR
- * @intel_dp: Intel DP
- * @crtc_state: new CRTC state
- * @conn_state: new CONNECTOR state
- *
- * This function can only be called after the pipe is fully trained and enabled.
- */
-void intel_psr_enable(struct intel_dp *intel_dp,
-		      const struct intel_crtc_state *crtc_state,
-		      const struct drm_connector_state *conn_state)
-{
-	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-
-	if (!CAN_PSR(intel_dp))
-		return;
-
-	if (!crtc_state->has_psr)
-		return;
-
-	drm_WARN_ON(&dev_priv->drm, dev_priv->drrs.dp);
-
-	mutex_lock(&intel_dp->psr.lock);
-	intel_psr_enable_locked(intel_dp, crtc_state, conn_state);
-	mutex_unlock(&intel_dp->psr.lock);
-}
-
 static void intel_psr_exit(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1668,48 +1641,120 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 	return 0;
 }
 
-/**
- * intel_psr_update - Update PSR state
- * @intel_dp: Intel DP
- * @crtc_state: new CRTC state
- * @conn_state: new CONNECTOR state
- *
- * This functions will update PSR states, disabling, enabling or switching PSR
- * version when executing fastsets. For full modeset, intel_psr_disable() and
- * intel_psr_enable() should be called instead.
- */
-void intel_psr_update(struct intel_dp *intel_dp,
-		      const struct intel_crtc_state *crtc_state,
-		      const struct drm_connector_state *conn_state)
+static void _intel_psr_pre_plane_update(const struct intel_atomic_state *state,
+					const struct intel_crtc_state *crtc_state)
 {
-	struct intel_psr *psr = &intel_dp->psr;
-	bool enable, psr2_enable;
+	struct drm_connector *conn;
+	struct drm_connector_state *conn_state;
+	int i;
 
-	if (!CAN_PSR(intel_dp))
+	for_each_new_connector_in_state(&state->base, conn, conn_state, i) {
+		struct intel_encoder *encoder;
+		struct intel_dp *intel_dp;
+		struct intel_psr *psr;
+		bool needs_to_disable = false;
+
+		if (conn_state->crtc != crtc_state->uapi.crtc)
+			continue;
+
+		encoder = to_intel_encoder(conn_state->best_encoder);
+		if (encoder->type != INTEL_OUTPUT_EDP)
+			continue;
+
+		intel_dp = enc_to_intel_dp(encoder);
+		if (!CAN_PSR(intel_dp))
+			continue;
+		psr = &intel_dp->psr;
+
+		mutex_lock(&psr->lock);
+
+		/*
+		 * Reasons to disable:
+		 * - PSR disabled in new state
+		 * - All planes will go inactive
+		 * - Changing between PSR versions
+		 */
+		needs_to_disable |= !crtc_state->has_psr;
+		needs_to_disable |= !crtc_state->active_planes;
+		needs_to_disable |= crtc_state->has_psr2 != psr->psr2_enabled;
+
+		if (psr->enabled && needs_to_disable)
+			intel_psr_disable_locked(intel_dp);
+
+		mutex_unlock(&psr->lock);
+	}
+}
+
+void intel_psr_pre_plane_update(const struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int i;
+
+	if (!HAS_PSR(dev_priv))
 		return;
 
-	mutex_lock(&intel_dp->psr.lock);
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
+		_intel_psr_pre_plane_update(state, crtc_state);
+}
 
-	enable = crtc_state->has_psr;
-	psr2_enable = crtc_state->has_psr2;
+static void _intel_psr_post_plane_update(const struct intel_atomic_state *state,
+					 const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct drm_connector *conn;
+	struct drm_connector_state *conn_state;
+	int i;
+
+	if (!crtc_state->has_psr)
+		return;
+
+	for_each_new_connector_in_state(&state->base, conn, conn_state, i) {
+		struct intel_encoder *encoder;
+		struct intel_dp *intel_dp;
+		struct intel_psr *psr;
+
+		if (conn_state->crtc != crtc_state->uapi.crtc)
+			continue;
+
+		encoder = to_intel_encoder(conn_state->best_encoder);
+		if (encoder->type != INTEL_OUTPUT_EDP)
+			continue;
+
+		intel_dp = enc_to_intel_dp(encoder);
+		if (!CAN_PSR(intel_dp))
+			continue;
+		psr = &intel_dp->psr;
+
+		mutex_lock(&psr->lock);
+
+		drm_WARN_ON(&dev_priv->drm, psr->enabled && !crtc_state->active_planes);
+
+		/* Only enable if there is active planes */
+		if (!psr->enabled && crtc_state->active_planes)
+			intel_psr_enable_locked(intel_dp, crtc_state, conn_state);
 
-	if (enable == psr->enabled && psr2_enable == psr->psr2_enabled &&
-	    crtc_state->enable_psr2_sel_fetch == psr->psr2_sel_fetch_enabled) {
 		/* Force a PSR exit when enabling CRC to avoid CRC timeouts */
 		if (crtc_state->crc_enabled && psr->enabled)
 			psr_force_hw_tracking_exit(intel_dp);
 
-		goto unlock;
+		mutex_unlock(&psr->lock);
 	}
+}
 
-	if (psr->enabled)
-		intel_psr_disable_locked(intel_dp);
+void intel_psr_post_plane_update(const struct intel_atomic_state *state)
+{
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc_state *crtc_state;
+	struct intel_crtc *crtc;
+	int i;
 
-	if (enable)
-		intel_psr_enable_locked(intel_dp, crtc_state, conn_state);
+	if (!HAS_PSR(dev_priv))
+		return;
 
-unlock:
-	mutex_unlock(&intel_dp->psr.lock);
+	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i)
+		_intel_psr_post_plane_update(state, crtc_state);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index 58e2e5c2b81ef..c44ad6299d9bc 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -20,14 +20,10 @@ struct intel_plane;
 struct intel_encoder;
 
 void intel_psr_init_dpcd(struct intel_dp *intel_dp);
-void intel_psr_enable(struct intel_dp *intel_dp,
-		      const struct intel_crtc_state *crtc_state,
-		      const struct drm_connector_state *conn_state);
+void intel_psr_pre_plane_update(const struct intel_atomic_state *state);
+void intel_psr_post_plane_update(const struct intel_atomic_state *state);
 void intel_psr_disable(struct intel_dp *intel_dp,
 		       const struct intel_crtc_state *old_crtc_state);
-void intel_psr_update(struct intel_dp *intel_dp,
-		      const struct intel_crtc_state *crtc_state,
-		      const struct drm_connector_state *conn_state);
 int intel_psr_debug_set(struct intel_dp *intel_dp, u64 value);
 void intel_psr_init(struct intel_dp *intel_dp);
 void intel_psr_compute_config(struct intel_dp *intel_dp,
-- 
2.33.0



More information about the Intel-gfx-trybot mailing list