All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
@ 2021-09-21  0:41 José Roberto de Souza
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: José Roberto de Souza @ 2021-09-21  0:41 UTC (permalink / raw)
  To: intel-gfx
  Cc: Gwan-gyeong Mun, Daniel Vetter, Ville Syrjälä,
	Jani Nikula, Rodrigo Vivi, José Roberto de Souza

By now all the userspace applications should have migrated to atomic
or at least be calling DRM_IOCTL_MODE_DIRTYFB.
But we still can't kill frontbuffer rendering support for good as
we have some performance issues to be solved in desktop environments
that do not use compositors.

But PSR2 selective fetch is not compatible with frontbuffer rendering
so here dropping frontbuffer rendering support when
enable_psr2_sel_fetch is set.
This way we leave for OEM and users the decision of enable this
feature or not.

Here converting legacy APIs into atomic commits so it can be properly
handled by driver i915.

v2:
- return earlier to not set fb_tracking.busy/flip_bits
- added a warn on to make sure we are not setting the busy/flip_bits

v3:
- only dropping frontbuffer rendering support when
enable_psr2_sel_fetch is set

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com> # v2
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cursor.c    |  3 ++-
 drivers/gpu/drm/i915/display/intel_fb.c        |  8 +++++++-
 .../gpu/drm/i915/display/intel_frontbuffer.c   | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h                |  2 ++
 4 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c b/drivers/gpu/drm/i915/display/intel_cursor.c
index c7618fef01439..7686446c29c13 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -617,6 +617,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 			   u32 src_w, u32 src_h,
 			   struct drm_modeset_acquire_ctx *ctx)
 {
+	struct drm_i915_private *i915 = to_i915(_crtc->dev);
 	struct intel_plane *plane = to_intel_plane(_plane);
 	struct intel_crtc *crtc = to_intel_crtc(_crtc);
 	struct intel_plane_state *old_plane_state =
@@ -638,7 +639,7 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
 	 */
 	if (!crtc_state->hw.active || intel_crtc_needs_modeset(crtc_state) ||
 	    crtc_state->update_pipe || crtc_state->bigjoiner ||
-	    crtc_state->enable_psr2_sel_fetch)
+	    !HAS_FRONTBUFFER_RENDERING(i915))
 		goto slow;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index e4b8602ec0cd2..3eb60785c9f29 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -3,6 +3,7 @@
  * Copyright © 2021 Intel Corporation
  */
 
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_modeset_helper.h>
 
@@ -1235,10 +1236,15 @@ static int intel_user_framebuffer_dirty(struct drm_framebuffer *fb,
 					unsigned int num_clips)
 {
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	struct drm_i915_private *i915 = to_i915(obj->base.dev);
 
 	i915_gem_object_flush_if_display(obj);
-	intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
 
+	if (!HAS_FRONTBUFFER_RENDERING(i915))
+		return drm_atomic_helper_dirtyfb(fb, file, flags, color, clips,
+						 num_clips);
+
+	intel_frontbuffer_flush(to_intel_frontbuffer(fb), ORIGIN_DIRTYFB);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 0492446cd04ad..3860f87dac31c 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -112,6 +112,9 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
 void intel_frontbuffer_flip_prepare(struct drm_i915_private *i915,
 				    unsigned frontbuffer_bits)
 {
+	if (!HAS_FRONTBUFFER_RENDERING(i915))
+		return;
+
 	spin_lock(&i915->fb_tracking.lock);
 	i915->fb_tracking.flip_bits |= frontbuffer_bits;
 	/* Remove stale busy bits due to the old buffer. */
@@ -132,6 +135,12 @@ void intel_frontbuffer_flip_prepare(struct drm_i915_private *i915,
 void intel_frontbuffer_flip_complete(struct drm_i915_private *i915,
 				     unsigned frontbuffer_bits)
 {
+	if (!HAS_FRONTBUFFER_RENDERING(i915)) {
+		drm_WARN_ON_ONCE(&i915->drm, i915->fb_tracking.flip_bits |
+					     i915->fb_tracking.busy_bits);
+		return;
+	}
+
 	spin_lock(&i915->fb_tracking.lock);
 	/* Mask any cancelled flips. */
 	frontbuffer_bits &= i915->fb_tracking.flip_bits;
@@ -156,6 +165,9 @@ void intel_frontbuffer_flip_complete(struct drm_i915_private *i915,
 void intel_frontbuffer_flip(struct drm_i915_private *i915,
 			    unsigned frontbuffer_bits)
 {
+	if (!HAS_FRONTBUFFER_RENDERING(i915))
+		return;
+
 	spin_lock(&i915->fb_tracking.lock);
 	/* Remove stale busy bits due to the old buffer. */
 	i915->fb_tracking.busy_bits &= ~frontbuffer_bits;
@@ -170,6 +182,9 @@ void __intel_fb_invalidate(struct intel_frontbuffer *front,
 {
 	struct drm_i915_private *i915 = to_i915(front->obj->base.dev);
 
+	if (!HAS_FRONTBUFFER_RENDERING(i915))
+		return;
+
 	if (origin == ORIGIN_CS) {
 		spin_lock(&i915->fb_tracking.lock);
 		i915->fb_tracking.busy_bits |= frontbuffer_bits;
@@ -191,6 +206,9 @@ void __intel_fb_flush(struct intel_frontbuffer *front,
 {
 	struct drm_i915_private *i915 = to_i915(front->obj->base.dev);
 
+	if (!HAS_FRONTBUFFER_RENDERING(i915))
+		return;
+
 	if (origin == ORIGIN_CS) {
 		spin_lock(&i915->fb_tracking.lock);
 		/* Filter out new bits since rendering started. */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index cc355aa05dbf4..aca6e4e02e029 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1704,6 +1704,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 
 #define HAS_ASYNC_FLIPS(i915)		(DISPLAY_VER(i915) >= 5)
 
+#define HAS_FRONTBUFFER_RENDERING(i915)	(!(i915)->params.enable_psr2_sel_fetch)
+
 /* Only valid when HAS_DISPLAY() is true */
 #define INTEL_DISPLAY_ENABLED(dev_priv) \
 	(drm_WARN_ON(&(dev_priv)->drm, !HAS_DISPLAY(dev_priv)), !(dev_priv)->params.disable_display)
-- 
2.33.0


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

* [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes
  2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
@ 2021-09-21  0:41 ` José Roberto de Souza
  2021-09-22 16:28   ` Gwan-gyeong Mun
  2021-09-22 16:28   ` Gwan-gyeong Mun
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats José Roberto de Souza
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: José Roberto de Souza @ 2021-09-21  0:41 UTC (permalink / raw)
  To: intel-gfx
  Cc: Ville Syrjälä, Gwan-gyeong Mun, José Roberto de Souza

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").

intel_dp_compute_psr_vsc_sdp() had to move from
intel_psr_enable_locked() to intel_psr_compute_config() because we
need to be able to disable/enable PSR from atomic states without
connector and encoder state.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |   2 -
 drivers/gpu/drm/i915/display/intel_display.c  |  14 +-
 .../drm/i915/display/intel_display_types.h    |   3 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |   6 +-
 drivers/gpu/drm/i915/display/intel_dp.h       |   2 +-
 drivers/gpu/drm/i915/display/intel_psr.c      | 140 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_psr.h      |  11 +-
 7 files changed, 98 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index bba0ab99836b1..a4667741d3548 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -3034,7 +3034,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);
@@ -3255,7 +3254,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 f6c0c595f6313..ddcd8d6efc788 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8093,10 +8093,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);
@@ -8153,7 +8155,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
@@ -10207,6 +10209,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)
@@ -10270,6 +10273,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_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e9e806d90eec4..c900bfbb7cc52 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1056,12 +1056,14 @@ struct intel_crtc_state {
 	struct intel_link_m_n dp_m2_n2;
 	bool has_drrs;
 
+	/* PSR is supported but might not be enabled due the lack of enabled planes */
 	bool has_psr;
 	bool has_psr2;
 	bool enable_psr2_sel_fetch;
 	bool req_psr2_sdp_prior_scanline;
 	u32 dc3co_exitline;
 	u16 su_y_granularity;
+	struct drm_dp_vsc_sdp psr_vsc;
 
 	/*
 	 * Frequence the dpll for the port should run at. Differs from the
@@ -1525,7 +1527,6 @@ struct intel_psr {
 	u32 dc3co_exitline;
 	u32 dc3co_exit_delay;
 	struct delayed_work dc3co_work;
-	struct drm_dp_vsc_sdp vsc;
 };
 
 struct intel_dp {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7559911c140a7..378008873e039 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1674,7 +1674,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
 {
 	vsc->sdp_type = DP_SDP_VSC;
 
-	if (intel_dp->psr.psr2_enabled) {
+	if (crtc_state->has_psr2) {
 		if (intel_dp->psr.colorimetry_support &&
 		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
 			/* [PSR2, +Colorimetry] */
@@ -1828,7 +1828,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		g4x_dp_set_clock(encoder, pipe_config);
 
 	intel_vrr_compute_config(pipe_config, conn_state);
-	intel_psr_compute_config(intel_dp, pipe_config);
+	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
 	intel_drrs_compute_config(intel_dp, pipe_config, output_bpp,
 				  constant_n);
 	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
@@ -2888,7 +2888,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
 
 void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
-			    struct drm_dp_vsc_sdp *vsc)
+			    const struct drm_dp_vsc_sdp *vsc)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 94b568704b22b..3343c25916807 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -88,7 +88,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
 				  struct drm_dp_vsc_sdp *vsc);
 void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
-			    struct drm_dp_vsc_sdp *vsc);
+			    const struct drm_dp_vsc_sdp *vsc);
 void intel_dp_set_infoframes(struct intel_encoder *encoder, bool enable,
 			     const struct intel_crtc_state *crtc_state,
 			     const struct drm_connector_state *conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index c1894b056d6c1..8ceb22c5a1a6b 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -949,7 +949,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 }
 
 void intel_psr_compute_config(struct intel_dp *intel_dp,
-			      struct intel_crtc_state *crtc_state)
+			      struct intel_crtc_state *crtc_state,
+			      struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	const struct drm_display_mode *adjusted_mode =
@@ -1001,7 +1002,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 
 	crtc_state->has_psr = true;
 	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
+
 	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC);
+	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
+				     &crtc_state->psr_vsc);
 }
 
 void intel_psr_get_config(struct intel_encoder *encoder,
@@ -1181,8 +1185,7 @@ static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
 }
 
 static void intel_psr_enable_locked(struct intel_dp *intel_dp,
-				    const struct intel_crtc_state *crtc_state,
-				    const struct drm_connector_state *conn_state)
+				    const struct intel_crtc_state *crtc_state)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1209,9 +1212,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
 
 	drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
 		    intel_dp->psr.psr2_enabled ? "2" : "1");
-	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
-				     &intel_dp->psr.vsc);
-	intel_write_dp_vsc_sdp(encoder, crtc_state, &intel_dp->psr.vsc);
+	intel_write_dp_vsc_sdp(encoder, crtc_state, &crtc_state->psr_vsc);
 	intel_snps_phy_update_psr_power_state(dev_priv, phy, true);
 	intel_psr_enable_sink(intel_dp);
 	intel_psr_enable_source(intel_dp);
@@ -1221,33 +1222,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);
@@ -1719,48 +1693,92 @@ 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 intel_encoder *encoder;
 
-	if (!CAN_PSR(intel_dp))
+	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
+					     crtc_state->uapi.encoder_mask) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+		struct intel_psr *psr = &intel_dp->psr;
+		bool needs_to_disable = false;
+
+		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 intel_encoder *encoder;
+
+	if (!crtc_state->has_psr)
+		return;
+
+	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
+					     crtc_state->uapi.encoder_mask) {
+		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+		struct intel_psr *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);
 
-	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 641521b101c82..2ca50df1f4fba 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_invalidate(struct drm_i915_private *dev_priv,
 			  unsigned frontbuffer_bits,
@@ -37,7 +33,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 		     enum fb_op_origin origin);
 void intel_psr_init(struct intel_dp *intel_dp);
 void intel_psr_compute_config(struct intel_dp *intel_dp,
-			      struct intel_crtc_state *crtc_state);
+			      struct intel_crtc_state *crtc_state,
+			      struct drm_connector_state *conn_state);
 void intel_psr_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config);
 void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir);
-- 
2.33.0


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

* [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats
  2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
@ 2021-09-21  0:41 ` José Roberto de Souza
  2021-09-22  8:28   ` Manna, Animesh
  2021-09-21  1:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: José Roberto de Souza @ 2021-09-21  0:41 UTC (permalink / raw)
  To: intel-gfx; +Cc: Gwan-gyeong Mun, José Roberto de Souza

From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

We are still missing the PSR2 selective fetch handling of biplanar
formats but until proper handle is added we can workaround it by
doing full frames fetch when state has biplanar formats.

We need the second check because an update in a regular format could
intersect with a biplanar plane that was not initialy part of the
atomic commit.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 8ceb22c5a1a6b..e6a4c27975d8c 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		 * TODO: Not clear how to handle planes with negative position,
 		 * also planes are not updated if they have a negative X
 		 * position so for now doing a full update in this cases
+		 *
+		 * TODO: We are missing biplanar formats handling, until it is
+		 * implemented it will send full frame updates.
 		 */
 		if (new_plane_state->uapi.dst.y1 < 0 ||
-		    new_plane_state->uapi.dst.x1 < 0) {
+		    new_plane_state->uapi.dst.x1 < 0 ||
+		    new_plane_state->hw.fb->format->is_yuv) {
 			full_update = true;
 			break;
 		}
@@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
 			continue;
 
+		if (new_plane_state->hw.fb->format->is_yuv) {
+			full_update = true;
+			break;
+		}
+
 		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
 		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
 		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
-- 
2.33.0


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
  2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats José Roberto de Souza
@ 2021-09-21  1:05 ` Patchwork
  2021-09-21  1:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
  2021-09-21  1:36 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2021-09-21  1:05 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
URL   : https://patchwork.freedesktop.org/series/94879/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
32149725ef4a drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
3db7b8dedb4c drm/i915/display: Only keep PSR enabled if there is active planes
-:16: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#16: 
commit 84030adb9e27 ("drm/i915/display: Disable audio, DRRS and PSR before planes").

total: 0 errors, 1 warnings, 0 checks, 321 lines checked
15e05b6c4287 drm/i915/display/psr: Do full fetch when handling biplanar formats



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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
  2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
                   ` (2 preceding siblings ...)
  2021-09-21  1:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled Patchwork
@ 2021-09-21  1:06 ` Patchwork
  2021-09-21  1:36 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2021-09-21  1:06 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
URL   : https://patchwork.freedesktop.org/series/94879/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:27:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:32:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:49:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_engine_stats.h:56:9: warning: trying to copy expression type 31
+drivers/gpu/drm/i915/gt/intel_reset.c:1392:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/i915_perf.c:1442:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1496:15: warning: memset with byte count of 16777216
+./include/asm-generic/bitops/find.h:112:45: warning: shift count is negative (-262080)
+./include/asm-generic/bitops/find.h:32:31: warning: shift count is negative (-262080)
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:418:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
  2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
                   ` (3 preceding siblings ...)
  2021-09-21  1:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2021-09-21  1:36 ` Patchwork
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2021-09-21  1:36 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

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

== Series Details ==

Series: series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled
URL   : https://patchwork.freedesktop.org/series/94879/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_10615 -> Patchwork_21103
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-icl-y:           [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-icl-y/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-icl-y/igt@i915_module_load@reload.html

  
#### Warnings ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-bxt-dsi:         [INCOMPLETE][3] ([i915#4130]) -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-bxt-dsi/igt@core_hotunplug@unbind-rebind.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-bxt-dsi/igt@core_hotunplug@unbind-rebind.html

  
#### Suppressed ####

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

  * igt@i915_module_load@reload:
    - {fi-ehl-2}:         [INCOMPLETE][5] ([i915#4136]) -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-ehl-2/igt@i915_module_load@reload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-ehl-2/igt@i915_module_load@reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-cfl-guc:         NOTRUN -> [SKIP][7] ([fdo#109271]) +17 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-cfl-guc/igt@amdgpu/amd_basic@cs-compute.html

  * igt@amdgpu/amd_basic@cs-gfx:
    - fi-rkl-guc:         NOTRUN -> [SKIP][8] ([fdo#109315]) +17 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-rkl-guc/igt@amdgpu/amd_basic@cs-gfx.html

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][9] ([fdo#109271]) +27 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-bdw-5557u/igt@amdgpu/amd_basic@semaphore.html

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> [SKIP][10] ([fdo#109315]) +17 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html

  * igt@amdgpu/amd_cs_nop@sync-fork-gfx0:
    - fi-skl-6600u:       NOTRUN -> [SKIP][11] ([fdo#109271]) +17 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-skl-6600u/igt@amdgpu/amd_cs_nop@sync-fork-gfx0.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-bdw-5557u:       NOTRUN -> [WARN][12] ([i915#3718])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-bdw-5557u/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_module_load@reload:
    - fi-ivb-3770:        [PASS][13] -> [INCOMPLETE][14] ([i915#4179])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-ivb-3770/igt@i915_module_load@reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-ivb-3770/igt@i915_module_load@reload.html
    - fi-bsw-kefka:       [PASS][15] -> [INCOMPLETE][16] ([i915#4179])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-bsw-kefka/igt@i915_module_load@reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-bsw-kefka/igt@i915_module_load@reload.html
    - fi-glk-dsi:         [PASS][17] -> [INCOMPLETE][18] ([i915#4130])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-glk-dsi/igt@i915_module_load@reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-glk-dsi/igt@i915_module_load@reload.html
    - fi-snb-2600:        [PASS][19] -> [INCOMPLETE][20] ([i915#4179])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-snb-2600/igt@i915_module_load@reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-snb-2600/igt@i915_module_load@reload.html

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

  * igt@kms_flip@basic-flip-vs-modeset@c-dp1:
    - fi-cfl-8109u:       [PASS][22] -> [FAIL][23] ([i915#4165]) +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-cfl-8109u/igt@kms_flip@basic-flip-vs-modeset@c-dp1.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-cfl-8109u/igt@kms_flip@basic-flip-vs-modeset@c-dp1.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b:
    - fi-cfl-8109u:       [PASS][24] -> [DMESG-WARN][25] ([i915#295]) +18 similar issues
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-cfl-8109u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-b.html

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> [FAIL][26] ([i915#3690])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-bsw-kefka/igt@runner@aborted.html
    - fi-snb-2600:        NOTRUN -> [FAIL][27] ([i915#2426])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-snb-2600/igt@runner@aborted.html
    - fi-ivb-3770:        NOTRUN -> [FAIL][28] ([i915#2426])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-ivb-3770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-rkl-guc:         [INCOMPLETE][29] ([i915#4130]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-rkl-guc/igt@core_hotunplug@unbind-rebind.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-rkl-guc/igt@core_hotunplug@unbind-rebind.html
    - fi-cfl-guc:         [INCOMPLETE][31] ([i915#4130]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-cfl-guc/igt@core_hotunplug@unbind-rebind.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-cfl-guc/igt@core_hotunplug@unbind-rebind.html
    - fi-icl-u2:          [INCOMPLETE][33] ([i915#4130]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-icl-u2/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_module_load@reload:
    - fi-skl-6600u:       [INCOMPLETE][35] ([i915#4130]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-skl-6600u/igt@i915_module_load@reload.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-skl-6600u/igt@i915_module_load@reload.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-rkl-guc:         [SKIP][37] ([i915#3919]) -> [PASS][38] +1 similar issue
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-rkl-guc/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-rkl-guc/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-kbl-r:           [TIMEOUT][39] ([i915#4136]) -> [INCOMPLETE][40] ([i915#4130])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10615/fi-kbl-r/igt@i915_module_load@reload.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21103/fi-kbl-r/igt@i915_module_load@reload.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3690]: https://gitlab.freedesktop.org/drm/intel/issues/3690
  [i915#3718]: https://gitlab.freedesktop.org/drm/intel/issues/3718
  [i915#3919]: https://gitlab.freedesktop.org/drm/intel/issues/3919
  [i915#4130]: https://gitlab.freedesktop.org/drm/intel/issues/4130
  [i915#4136]: https://gitlab.freedesktop.org/drm/intel/issues/4136
  [i915#4165]: https://gitlab.freedesktop.org/drm/intel/issues/4165
  [i915#4179]: https://gitlab.freedesktop.org/drm/intel/issues/4179


Participating hosts (37 -> 31)
------------------------------

  Missing    (6): bat-dg1-6 fi-tgl-1115g4 fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus bat-jsl-1 


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

  * Linux: CI_DRM_10615 -> Patchwork_21103

  CI-20190529: 20190529
  CI_DRM_10615: 4aedbb89fd6dc4f0b3c5e9213059b0e84e054d19 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6213: e9ae59cb8b4f1e7bc61a9261f33fc7e52ae06c65 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21103: 15e05b6c42878876acd9031cbae7010e8ab297d6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

15e05b6c4287 drm/i915/display/psr: Do full fetch when handling biplanar formats
3db7b8dedb4c drm/i915/display: Only keep PSR enabled if there is active planes
32149725ef4a drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats José Roberto de Souza
@ 2021-09-22  8:28   ` Manna, Animesh
  2021-09-22 12:08     ` Gwan-gyeong Mun
  0 siblings, 1 reply; 11+ messages in thread
From: Manna, Animesh @ 2021-09-22  8:28 UTC (permalink / raw)
  To: Souza, Jose, intel-gfx; +Cc: Mun, Gwan-gyeong, Souza, Jose



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José
> Roberto de Souza
> Sent: Tuesday, September 21, 2021 6:11 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Souza, Jose
> <jose.souza@intel.com>
> Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when
> handling biplanar formats
> 
> From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> 
> We are still missing the PSR2 selective fetch handling of biplanar formats but
> until proper handle is added we can workaround it by doing full frames fetch
> when state has biplanar formats.
> 
> We need the second check because an update in a regular format could
> intersect with a biplanar plane that was not initialy part of the atomic commit.
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 8ceb22c5a1a6b..e6a4c27975d8c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  		 * TODO: Not clear how to handle planes with negative
> position,
>  		 * also planes are not updated if they have a negative X
>  		 * position so for now doing a full update in this cases
> +		 *
> +		 * TODO: We are missing biplanar formats handling, until it is
> +		 * implemented it will send full frame updates.
>  		 */
>  		if (new_plane_state->uapi.dst.y1 < 0 ||
> -		    new_plane_state->uapi.dst.x1 < 0) {
> +		    new_plane_state->uapi.dst.x1 < 0 ||
> +		    new_plane_state->hw.fb->format->is_yuv) {
>  			full_update = true;
>  			break;
>  		}
> @@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct
> intel_atomic_state *state,
>  		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
>  			continue;
> 

Code comment can be added here why we need this check again in same function.
Enabling full frame update is fine for me for planar format but not sure we need the 2nd check.

Regards,
Animesh

> +		if (new_plane_state->hw.fb->format->is_yuv) {
> +			full_update = true;
> +			break;
> +		}
> +
>  		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
>  		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
>  		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> --
> 2.33.0


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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats
  2021-09-22  8:28   ` Manna, Animesh
@ 2021-09-22 12:08     ` Gwan-gyeong Mun
  2021-09-22 14:17       ` Souza, Jose
  0 siblings, 1 reply; 11+ messages in thread
From: Gwan-gyeong Mun @ 2021-09-22 12:08 UTC (permalink / raw)
  To: Manna, Animesh, Souza, Jose, intel-gfx



On 9/22/21 11:28 AM, Manna, Animesh wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José
>> Roberto de Souza
>> Sent: Tuesday, September 21, 2021 6:11 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Souza, Jose
>> <jose.souza@intel.com>
>> Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when
>> handling biplanar formats
>>
>> From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>
>> We are still missing the PSR2 selective fetch handling of biplanar formats but
>> until proper handle is added we can workaround it by doing full frames fetch
>> when state has biplanar formats.
>>
>> We need the second check because an update in a regular format could
>> intersect with a biplanar plane that was not initialy part of the atomic commit.
>>
>> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> b/drivers/gpu/drm/i915/display/intel_psr.c
>> index 8ceb22c5a1a6b..e6a4c27975d8c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> @@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct
>> intel_atomic_state *state,
>>    * TODO: Not clear how to handle planes with negative
>> position,
>>    * also planes are not updated if they have a negative X
>>    * position so for now doing a full update in this cases
>> + *
>> + * TODO: We are missing biplanar formats handling, until it is
>> + * implemented it will send full frame updates.
>>    */
>>   if (new_plane_state->uapi.dst.y1 < 0 ||
>> -    new_plane_state->uapi.dst.x1 < 0) {
>> +    new_plane_state->uapi.dst.x1 < 0 ||
>> +    new_plane_state->hw.fb->format->is_yuv) {
>>   full_update = true;
>>   break;
>>   }
>> @@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct
>> intel_atomic_state *state,
>>   if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
>>   continue;
>>
> 
> Code comment can be added here why we need this check again in same function.
> Enabling full frame update is fine for me for planar format but not sure we need the 2nd check.
> 
That's right, we don't need to set this code here because we set 
full_update above when " new_plane_state->hw.fb->format->is_yuv" is true.

I will update this in the next version.

Thanks, Animesh.

> Regards,
> Animesh
> 
>> +if (new_plane_state->hw.fb->format->is_yuv) {
>> +full_update = true;
>> +break;
>> +}
>> +
>>   sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
>>   sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
>>   sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
>> --
>> 2.33.0
> 

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

* Re: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats
  2021-09-22 12:08     ` Gwan-gyeong Mun
@ 2021-09-22 14:17       ` Souza, Jose
  0 siblings, 0 replies; 11+ messages in thread
From: Souza, Jose @ 2021-09-22 14:17 UTC (permalink / raw)
  To: Mun, Gwan-gyeong, Manna, Animesh, intel-gfx

On Wed, 2021-09-22 at 15:08 +0300, Gwan-gyeong Mun wrote:
> 
> On 9/22/21 11:28 AM, Manna, Animesh wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José
> > > Roberto de Souza
> > > Sent: Tuesday, September 21, 2021 6:11 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Mun, Gwan-gyeong <gwan-gyeong.mun@intel.com>; Souza, Jose
> > > <jose.souza@intel.com>
> > > Subject: [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when
> > > handling biplanar formats
> > > 
> > > From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > 
> > > We are still missing the PSR2 selective fetch handling of biplanar formats but
> > > until proper handle is added we can workaround it by doing full frames fetch
> > > when state has biplanar formats.
> > > 
> > > We need the second check because an update in a regular format could
> > > intersect with a biplanar plane that was not initialy part of the atomic commit.
> > > 
> > > Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_psr.c | 11 ++++++++++-
> > >   1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 8ceb22c5a1a6b..e6a4c27975d8c 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1601,9 +1601,13 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >    * TODO: Not clear how to handle planes with negative
> > > position,
> > >    * also planes are not updated if they have a negative X
> > >    * position so for now doing a full update in this cases
> > > + *
> > > + * TODO: We are missing biplanar formats handling, until it is
> > > + * implemented it will send full frame updates.
> > >    */
> > >   if (new_plane_state->uapi.dst.y1 < 0 ||
> > > -    new_plane_state->uapi.dst.x1 < 0) {
> > > +    new_plane_state->uapi.dst.x1 < 0 ||
> > > +    new_plane_state->hw.fb->format->is_yuv) {
> > >   full_update = true;
> > >   break;
> > >   }
> > > @@ -1682,6 +1686,11 @@ int intel_psr2_sel_fetch_update(struct
> > > intel_atomic_state *state,
> > >   if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
> > >   continue;
> > > 
> > 
> > Code comment can be added here why we need this check again in same function.
> > Enabling full frame update is fine for me for planar format but not sure we need the 2nd check.
> > 
> That's right, we don't need to set this code here because we set 
> full_update above when " new_plane_state->hw.fb->format->is_yuv" is true.

We need it, the reason is on the commit description.

> 
> I will update this in the next version.
> 
> Thanks, Animesh.
> 
> > Regards,
> > Animesh
> > 
> > > +if (new_plane_state->hw.fb->format->is_yuv) {
> > > +full_update = true;
> > > +break;
> > > +}
> > > +
> > >   sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> > >   sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> > >   sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> > > --
> > > 2.33.0
> > 


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

* Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
@ 2021-09-22 16:28   ` Gwan-gyeong Mun
  2021-09-22 16:28   ` Gwan-gyeong Mun
  1 sibling, 0 replies; 11+ messages in thread
From: Gwan-gyeong Mun @ 2021-09-22 16:28 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx; +Cc: Ville Syrjälä

Looks good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 9/21/21 3:41 AM, José Roberto de Souza wrote:
> 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").
> 
> intel_dp_compute_psr_vsc_sdp() had to move from
> intel_psr_enable_locked() to intel_psr_compute_config() because we
> need to be able to disable/enable PSR from atomic states without
> connector and encoder state.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_ddi.c      |   2 -
>   drivers/gpu/drm/i915/display/intel_display.c  |  14 +-
>   .../drm/i915/display/intel_display_types.h    |   3 +-
>   drivers/gpu/drm/i915/display/intel_dp.c       |   6 +-
>   drivers/gpu/drm/i915/display/intel_dp.h       |   2 +-
>   drivers/gpu/drm/i915/display/intel_psr.c      | 140 ++++++++++--------
>   drivers/gpu/drm/i915/display/intel_psr.h      |  11 +-
>   7 files changed, 98 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index bba0ab99836b1..a4667741d3548 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3034,7 +3034,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);
> @@ -3255,7 +3254,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 f6c0c595f6313..ddcd8d6efc788 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8093,10 +8093,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);
> @@ -8153,7 +8155,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
> @@ -10207,6 +10209,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)
> @@ -10270,6 +10273,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_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e9e806d90eec4..c900bfbb7cc52 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1056,12 +1056,14 @@ struct intel_crtc_state {
>   	struct intel_link_m_n dp_m2_n2;
>   	bool has_drrs;
>   
> +	/* PSR is supported but might not be enabled due the lack of enabled planes */
>   	bool has_psr;
>   	bool has_psr2;
>   	bool enable_psr2_sel_fetch;
>   	bool req_psr2_sdp_prior_scanline;
>   	u32 dc3co_exitline;
>   	u16 su_y_granularity;
> +	struct drm_dp_vsc_sdp psr_vsc;
>   
>   	/*
>   	 * Frequence the dpll for the port should run at. Differs from the
> @@ -1525,7 +1527,6 @@ struct intel_psr {
>   	u32 dc3co_exitline;
>   	u32 dc3co_exit_delay;
>   	struct delayed_work dc3co_work;
> -	struct drm_dp_vsc_sdp vsc;
>   };
>   
>   struct intel_dp {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7559911c140a7..378008873e039 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1674,7 +1674,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
>   {
>   	vsc->sdp_type = DP_SDP_VSC;
>   
> -	if (intel_dp->psr.psr2_enabled) {
> +	if (crtc_state->has_psr2) {
>   		if (intel_dp->psr.colorimetry_support &&
>   		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
>   			/* [PSR2, +Colorimetry] */
> @@ -1828,7 +1828,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   		g4x_dp_set_clock(encoder, pipe_config);
>   
>   	intel_vrr_compute_config(pipe_config, conn_state);
> -	intel_psr_compute_config(intel_dp, pipe_config);
> +	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
>   	intel_drrs_compute_config(intel_dp, pipe_config, output_bpp,
>   				  constant_n);
>   	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> @@ -2888,7 +2888,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>   
>   void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
>   			    const struct intel_crtc_state *crtc_state,
> -			    struct drm_dp_vsc_sdp *vsc)
> +			    const struct drm_dp_vsc_sdp *vsc)
>   {
>   	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 94b568704b22b..3343c25916807 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -88,7 +88,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
>   				  struct drm_dp_vsc_sdp *vsc);
>   void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
>   			    const struct intel_crtc_state *crtc_state,
> -			    struct drm_dp_vsc_sdp *vsc);
> +			    const struct drm_dp_vsc_sdp *vsc);
>   void intel_dp_set_infoframes(struct intel_encoder *encoder, bool enable,
>   			     const struct intel_crtc_state *crtc_state,
>   			     const struct drm_connector_state *conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index c1894b056d6c1..8ceb22c5a1a6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -949,7 +949,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>   }
>   
>   void intel_psr_compute_config(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *crtc_state)
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state)
>   {
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>   	const struct drm_display_mode *adjusted_mode =
> @@ -1001,7 +1002,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>   
>   	crtc_state->has_psr = true;
>   	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> +
>   	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC);
> +	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
> +				     &crtc_state->psr_vsc);
>   }
>   
>   void intel_psr_get_config(struct intel_encoder *encoder,
> @@ -1181,8 +1185,7 @@ static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
>   }
>   
>   static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> -				    const struct intel_crtc_state *crtc_state,
> -				    const struct drm_connector_state *conn_state)
> +				    const struct intel_crtc_state *crtc_state)
>   {
>   	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -1209,9 +1212,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
>   
>   	drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
>   		    intel_dp->psr.psr2_enabled ? "2" : "1");
> -	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
> -				     &intel_dp->psr.vsc);
> -	intel_write_dp_vsc_sdp(encoder, crtc_state, &intel_dp->psr.vsc);
> +	intel_write_dp_vsc_sdp(encoder, crtc_state, &crtc_state->psr_vsc);
>   	intel_snps_phy_update_psr_power_state(dev_priv, phy, true);
>   	intel_psr_enable_sink(intel_dp);
>   	intel_psr_enable_source(intel_dp);
> @@ -1221,33 +1222,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);
> @@ -1719,48 +1693,92 @@ 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 intel_encoder *encoder;
>   
> -	if (!CAN_PSR(intel_dp))
> +	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +					     crtc_state->uapi.encoder_mask) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +		struct intel_psr *psr = &intel_dp->psr;
> +		bool needs_to_disable = false;
> +
> +		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 intel_encoder *encoder;
> +
> +	if (!crtc_state->has_psr)
> +		return;
> +
> +	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +					     crtc_state->uapi.encoder_mask) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +		struct intel_psr *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);
>   
> -	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 641521b101c82..2ca50df1f4fba 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_invalidate(struct drm_i915_private *dev_priv,
>   			  unsigned frontbuffer_bits,
> @@ -37,7 +33,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>   		     enum fb_op_origin origin);
>   void intel_psr_init(struct intel_dp *intel_dp);
>   void intel_psr_compute_config(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *crtc_state);
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state);
>   void intel_psr_get_config(struct intel_encoder *encoder,
>   			  struct intel_crtc_state *pipe_config);
>   void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir);
> 

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

* Re: [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes
  2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
  2021-09-22 16:28   ` Gwan-gyeong Mun
@ 2021-09-22 16:28   ` Gwan-gyeong Mun
  1 sibling, 0 replies; 11+ messages in thread
From: Gwan-gyeong Mun @ 2021-09-22 16:28 UTC (permalink / raw)
  To: José Roberto de Souza, intel-gfx; +Cc: Ville Syrjälä

Looks good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 9/21/21 3:41 AM, José Roberto de Souza wrote:
> 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").
> 
> intel_dp_compute_psr_vsc_sdp() had to move from
> intel_psr_enable_locked() to intel_psr_compute_config() because we
> need to be able to disable/enable PSR from atomic states without
> connector and encoder state.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_ddi.c      |   2 -
>   drivers/gpu/drm/i915/display/intel_display.c  |  14 +-
>   .../drm/i915/display/intel_display_types.h    |   3 +-
>   drivers/gpu/drm/i915/display/intel_dp.c       |   6 +-
>   drivers/gpu/drm/i915/display/intel_dp.h       |   2 +-
>   drivers/gpu/drm/i915/display/intel_psr.c      | 140 ++++++++++--------
>   drivers/gpu/drm/i915/display/intel_psr.h      |  11 +-
>   7 files changed, 98 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index bba0ab99836b1..a4667741d3548 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3034,7 +3034,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);
> @@ -3255,7 +3254,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 f6c0c595f6313..ddcd8d6efc788 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8093,10 +8093,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);
> @@ -8153,7 +8155,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
> @@ -10207,6 +10209,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)
> @@ -10270,6 +10273,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_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e9e806d90eec4..c900bfbb7cc52 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1056,12 +1056,14 @@ struct intel_crtc_state {
>   	struct intel_link_m_n dp_m2_n2;
>   	bool has_drrs;
>   
> +	/* PSR is supported but might not be enabled due the lack of enabled planes */
>   	bool has_psr;
>   	bool has_psr2;
>   	bool enable_psr2_sel_fetch;
>   	bool req_psr2_sdp_prior_scanline;
>   	u32 dc3co_exitline;
>   	u16 su_y_granularity;
> +	struct drm_dp_vsc_sdp psr_vsc;
>   
>   	/*
>   	 * Frequence the dpll for the port should run at. Differs from the
> @@ -1525,7 +1527,6 @@ struct intel_psr {
>   	u32 dc3co_exitline;
>   	u32 dc3co_exit_delay;
>   	struct delayed_work dc3co_work;
> -	struct drm_dp_vsc_sdp vsc;
>   };
>   
>   struct intel_dp {
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7559911c140a7..378008873e039 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1674,7 +1674,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
>   {
>   	vsc->sdp_type = DP_SDP_VSC;
>   
> -	if (intel_dp->psr.psr2_enabled) {
> +	if (crtc_state->has_psr2) {
>   		if (intel_dp->psr.colorimetry_support &&
>   		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
>   			/* [PSR2, +Colorimetry] */
> @@ -1828,7 +1828,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   		g4x_dp_set_clock(encoder, pipe_config);
>   
>   	intel_vrr_compute_config(pipe_config, conn_state);
> -	intel_psr_compute_config(intel_dp, pipe_config);
> +	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
>   	intel_drrs_compute_config(intel_dp, pipe_config, output_bpp,
>   				  constant_n);
>   	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> @@ -2888,7 +2888,7 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>   
>   void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
>   			    const struct intel_crtc_state *crtc_state,
> -			    struct drm_dp_vsc_sdp *vsc)
> +			    const struct drm_dp_vsc_sdp *vsc)
>   {
>   	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 94b568704b22b..3343c25916807 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -88,7 +88,7 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
>   				  struct drm_dp_vsc_sdp *vsc);
>   void intel_write_dp_vsc_sdp(struct intel_encoder *encoder,
>   			    const struct intel_crtc_state *crtc_state,
> -			    struct drm_dp_vsc_sdp *vsc);
> +			    const struct drm_dp_vsc_sdp *vsc);
>   void intel_dp_set_infoframes(struct intel_encoder *encoder, bool enable,
>   			     const struct intel_crtc_state *crtc_state,
>   			     const struct drm_connector_state *conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index c1894b056d6c1..8ceb22c5a1a6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -949,7 +949,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>   }
>   
>   void intel_psr_compute_config(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *crtc_state)
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state)
>   {
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>   	const struct drm_display_mode *adjusted_mode =
> @@ -1001,7 +1002,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>   
>   	crtc_state->has_psr = true;
>   	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> +
>   	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC);
> +	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
> +				     &crtc_state->psr_vsc);
>   }
>   
>   void intel_psr_get_config(struct intel_encoder *encoder,
> @@ -1181,8 +1185,7 @@ static bool psr_interrupt_error_check(struct intel_dp *intel_dp)
>   }
>   
>   static void intel_psr_enable_locked(struct intel_dp *intel_dp,
> -				    const struct intel_crtc_state *crtc_state,
> -				    const struct drm_connector_state *conn_state)
> +				    const struct intel_crtc_state *crtc_state)
>   {
>   	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -1209,9 +1212,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
>   
>   	drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
>   		    intel_dp->psr.psr2_enabled ? "2" : "1");
> -	intel_dp_compute_psr_vsc_sdp(intel_dp, crtc_state, conn_state,
> -				     &intel_dp->psr.vsc);
> -	intel_write_dp_vsc_sdp(encoder, crtc_state, &intel_dp->psr.vsc);
> +	intel_write_dp_vsc_sdp(encoder, crtc_state, &crtc_state->psr_vsc);
>   	intel_snps_phy_update_psr_power_state(dev_priv, phy, true);
>   	intel_psr_enable_sink(intel_dp);
>   	intel_psr_enable_source(intel_dp);
> @@ -1221,33 +1222,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);
> @@ -1719,48 +1693,92 @@ 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 intel_encoder *encoder;
>   
> -	if (!CAN_PSR(intel_dp))
> +	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +					     crtc_state->uapi.encoder_mask) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +		struct intel_psr *psr = &intel_dp->psr;
> +		bool needs_to_disable = false;
> +
> +		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 intel_encoder *encoder;
> +
> +	if (!crtc_state->has_psr)
> +		return;
> +
> +	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> +					     crtc_state->uapi.encoder_mask) {
> +		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +		struct intel_psr *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);
>   
> -	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 641521b101c82..2ca50df1f4fba 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_invalidate(struct drm_i915_private *dev_priv,
>   			  unsigned frontbuffer_bits,
> @@ -37,7 +33,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>   		     enum fb_op_origin origin);
>   void intel_psr_init(struct intel_dp *intel_dp);
>   void intel_psr_compute_config(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *crtc_state);
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state);
>   void intel_psr_get_config(struct intel_encoder *encoder,
>   			  struct intel_crtc_state *pipe_config);
>   void intel_psr_irq_handler(struct intel_dp *intel_dp, u32 psr_iir);
> 

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

end of thread, other threads:[~2021-09-22 16:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21  0:41 [Intel-gfx] [PATCH v3 1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled José Roberto de Souza
2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 2/3] drm/i915/display: Only keep PSR enabled if there is active planes José Roberto de Souza
2021-09-22 16:28   ` Gwan-gyeong Mun
2021-09-22 16:28   ` Gwan-gyeong Mun
2021-09-21  0:41 ` [Intel-gfx] [PATCH v3 3/3] drm/i915/display/psr: Do full fetch when handling biplanar formats José Roberto de Souza
2021-09-22  8:28   ` Manna, Animesh
2021-09-22 12:08     ` Gwan-gyeong Mun
2021-09-22 14:17       ` Souza, Jose
2021-09-21  1:05 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915/display: Disable frontbuffer rendering when PSR2 selective fetch is enabled Patchwork
2021-09-21  1:06 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-21  1:36 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.