[PATCH v7 0/6] Panel replay phase1 implementation

[PATCH v7 0/6] Panel replay phase1 implementation

[Animesh Manna]

Panel Replay is a power saving feature for DP 2.0 monitor and similar
to PSR on EDP.

These patches are basic enablement patches added on top of
existing psr framework to enable full-screen live active frame
update mode of panel replay. Panel replay also can be enabled
in selective update mode which will be enabled in a incremental
approach.

As per current design panel replay priority is higher than psr.
intel_dp->psr.panel_replay_enabled flag indicate panel replay is enabled.
intel_dp->psr.panel_replay_enabled + intel_dp->psr.psr2_enabled indicates
panel replay is enabled in selective update mode.
intel_dp->psr.panel_replay_enabled + intel_dp->psr.psr2_enabled +
intel_psr.selective_fetch enabled indicates panel replay is
enabled in selective update mode with selective fetch.
PSR replated flags remain same like before.

Note: The patches are under testing by using panel replay emulator and
panel is not avalible.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>

Animesh Manna (5):
  drm/panelreplay: dpcd register definition for panelreplay
  drm/i915/panelreplay: Initializaton and compute config for panel
    replay
  drm/i915/panelreplay: Enable panel replay dpcd initialization for DP
  drm/i915/panelreplay: enable/disable panel replay
  drm/i915/panelreplay: Debugfs support for panel replay

Jouni Högander (1):
  drm/i915/psr: Move psr specific dpcd init into own function

 drivers/gpu/drm/i915/display/intel_ddi.c      |   7 +-
 .../drm/i915/display/intel_display_types.h    |  15 +-
 drivers/gpu/drm/i915/display/intel_dp.c       |  49 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +
 drivers/gpu/drm/i915/display/intel_psr.c      | 335 +++++++++++++-----
 drivers/gpu/drm/i915/display/intel_psr.h      |   7 +
 include/drm/display/drm_dp.h                  |  23 ++
 7 files changed, 327 insertions(+), 112 deletions(-)

-- 
2.29.0

[PATCH v7 1/6] drm/panelreplay: dpcd register definition for panelreplay

[Animesh Manna]

Add DPCD register definition for discovering, enabling and
checking status of panel replay of the sink.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 include/drm/display/drm_dp.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index e69cece404b3..fc42b622ef32 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -543,6 +543,10 @@
 /* DFP Capability Extension */
 #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT	0x0a3	/* 2.0 */
 
+#define DP_PANEL_REPLAY_CAP                 0x0b0  /* DP 2.0 */
+# define DP_PANEL_REPLAY_SUPPORT            (1 << 0)
+# define DP_PANEL_REPLAY_SU_SUPPORT         (1 << 1)
+
 /* Link Configuration */
 #define	DP_LINK_BW_SET		            0x100
 # define DP_LINK_RATE_TABLE		    0x00    /* eDP 1.4 */
@@ -716,6 +720,13 @@
 #define DP_BRANCH_DEVICE_CTRL		    0x1a1
 # define DP_BRANCH_DEVICE_IRQ_HPD	    (1 << 0)
 
+#define PANEL_REPLAY_CONFIG                             0x1b0  /* DP 2.0 */
+# define DP_PANEL_REPLAY_ENABLE                         (1 << 0)
+# define DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN         (1 << 3)
+# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN           (1 << 4)
+# define DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN      (1 << 5)
+# define DP_PANEL_REPLAY_SU_ENABLE                      (1 << 6)
+
 #define DP_PAYLOAD_ALLOCATE_SET		    0x1c0
 #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1
 #define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2
@@ -1105,6 +1116,18 @@
 #define DP_LANE_ALIGN_STATUS_UPDATED_ESI       0x200e /* status same as 0x204 */
 #define DP_SINK_STATUS_ESI                     0x200f /* status same as 0x205 */
 
+#define DP_PANEL_REPLAY_ERROR_STATUS                   0x2020  /* DP 2.1*/
+# define DP_PANEL_REPLAY_LINK_CRC_ERROR                (1 << 0)
+# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR             (1 << 1)
+# define DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR   (1 << 2)
+
+#define DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS        0x2022  /* DP 2.1 */
+# define DP_SINK_DEVICE_PANEL_REPLAY_STATUS_MASK       (7 << 0)
+# define DP_SINK_FRAME_LOCKED_SHIFT                    3
+# define DP_SINK_FRAME_LOCKED_MASK                     (3 << 3)
+# define DP_SINK_FRAME_LOCKED_STATUS_VALID_SHIFT       5
+# define DP_SINK_FRAME_LOCKED_STATUS_VALID_MASK        (1 << 5)
+
 /* Extended Receiver Capability: See DP_DPCD_REV for definitions */
 #define DP_DP13_DPCD_REV                    0x2200
 
-- 
2.29.0

[PATCH v7 2/6] drm/i915/psr: Move psr specific dpcd init into own function

[Animesh Manna]

From: Jouni Högander <jouni.hogander@intel.com>

This patch is preparing adding panel replay specific dpcd init.

Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 41 +++++++++++++-----------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index bb65881e87cc..0669ab7a9191 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -474,27 +474,22 @@ static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
 	intel_dp->psr.su_y_granularity = y;
 }
 
-void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+static void _psr_init_dpcd(struct intel_dp *intel_dp)
 {
-	struct drm_i915_private *dev_priv =
+	struct drm_i915_private *i915 =
 		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
 
-	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
-			 sizeof(intel_dp->psr_dpcd));
-
-	if (!intel_dp->psr_dpcd[0])
-		return;
-	drm_dbg_kms(&dev_priv->drm, "eDP panel supports PSR version %x\n",
+	drm_dbg_kms(&i915->drm, "eDP panel supports PSR version %x\n",
 		    intel_dp->psr_dpcd[0]);
 
 	if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_NO_PSR)) {
-		drm_dbg_kms(&dev_priv->drm,
+		drm_dbg_kms(&i915->drm,
 			    "PSR support not currently available for this panel\n");
 		return;
 	}
 
 	if (!(intel_dp->edp_dpcd[1] & DP_EDP_SET_POWER_CAP)) {
-		drm_dbg_kms(&dev_priv->drm,
+		drm_dbg_kms(&i915->drm,
 			    "Panel lacks power state control, PSR cannot be enabled\n");
 		return;
 	}
@@ -503,8 +498,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 	intel_dp->psr.sink_sync_latency =
 		intel_dp_get_sink_sync_latency(intel_dp);
 
-	if (DISPLAY_VER(dev_priv) >= 9 &&
-	    (intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
+	if (DISPLAY_VER(i915) >= 9 &&
+	    intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED) {
 		bool y_req = intel_dp->psr_dpcd[1] &
 			     DP_PSR2_SU_Y_COORDINATE_REQUIRED;
 		bool alpm = intel_dp_get_alpm_status(intel_dp);
@@ -521,14 +516,24 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 		 * GTC first.
 		 */
 		intel_dp->psr.sink_psr2_support = y_req && alpm;
-		drm_dbg_kms(&dev_priv->drm, "PSR2 %ssupported\n",
+		drm_dbg_kms(&i915->drm, "PSR2 %ssupported\n",
 			    intel_dp->psr.sink_psr2_support ? "" : "not ");
+	}
+}
 
-		if (intel_dp->psr.sink_psr2_support) {
-			intel_dp->psr.colorimetry_support =
-				intel_dp_get_colorimetry_status(intel_dp);
-			intel_dp_get_su_granularity(intel_dp);
-		}
+void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+{
+	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
+			 sizeof(intel_dp->psr_dpcd));
+
+	if (intel_dp->psr_dpcd[0])
+		_psr_init_dpcd(intel_dp);
+	/* TODO: Add PR case here */
+
+	if (intel_dp->psr.sink_psr2_support) {
+		intel_dp->psr.colorimetry_support =
+			intel_dp_get_colorimetry_status(intel_dp);
+		intel_dp_get_su_granularity(intel_dp);
 	}
 }
 
-- 
2.29.0

[PATCH v7 3/6] drm/i915/panelreplay: Initializaton and compute config for panel replay

[Animesh Manna]

Modify existing PSR implementation to enable panel replay feature of DP 2.0
which is similar to PSR feature of EDP panel. There is different DPCD
address to check panel capability compare to PSR and vsc sdp header
is different.

v1: Initial version.
v2:
- Set source_panel_replay_support flag under HAS_PANEL_REPLAY()
condition check. [Jouni]
- Code restructured around intel_panel_replay_init
and renamed to intel_panel_replay_init_dpcd. [Jouni]
- Remove the initial code modification around has_psr2 flag. [Jouni]
- Add CAN_PANEL_REPLAY() in intel_encoder_can_psr which is used to
enable in intel_psr_post_plane_update. [Jouni]
v3:
- Initialize both psr and panel-replay. [Jouni]
- Initialize both panel replay and psr if detected. [Jouni]
- Refactoring psr function by introducing _psr_compute_config(). [Jouni]
- Add check for !is_edp while deriving source_panel_replay_support. [Jouni]
- Enable panel replay dpcd initialization in a separate patch. [Jouni]

v4:
- HAS_PANEL_REPLAY() check not needed during sink capability check. [Jouni]
- Set either panel replay source support or psr. [Jouni]

v5:
- HAS_PANEL_REPLAY() removed and use HAS_DP20() instead. [Jouni]
- Move psr related code to intel_psr.c. [Jani]
- Reset sink_panel_replay_support flag during disconnection. [Jani]

v6: return statement restored which is removed by misatke. [Jouni]
v7: cosmetic changes. [Arun]

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 .../drm/i915/display/intel_display_types.h    | 14 +--
 drivers/gpu/drm/i915/display/intel_dp.c       | 49 ++++++++--
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |  3 +
 drivers/gpu/drm/i915/display/intel_psr.c      | 96 ++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_psr.h      |  7 ++
 5 files changed, 123 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 8d8b2f8d37a9..95b318f7b2b8 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1204,6 +1204,7 @@ struct intel_crtc_state {
 	bool has_psr2;
 	bool enable_psr2_sel_fetch;
 	bool req_psr2_sdp_prior_scanline;
+	bool has_panel_replay;
 	bool wm_level_disabled;
 	u32 dc3co_exitline;
 	u16 su_y_granularity;
@@ -1701,6 +1702,8 @@ struct intel_psr {
 	bool irq_aux_error;
 	u16 su_w_granularity;
 	u16 su_y_granularity;
+	bool source_panel_replay_support;
+	bool sink_panel_replay_support;
 	u32 dc3co_exitline;
 	u32 dc3co_exit_delay;
 	struct delayed_work dc3co_work;
@@ -1988,17 +1991,6 @@ dp_to_lspcon(struct intel_dp *intel_dp)
 
 #define dp_to_i915(__intel_dp) to_i915(dp_to_dig_port(__intel_dp)->base.base.dev)
 
-#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
-			   (intel_dp)->psr.source_support)
-
-static inline bool intel_encoder_can_psr(struct intel_encoder *encoder)
-{
-	if (!intel_encoder_is_dp(encoder))
-		return false;
-
-	return CAN_PSR(enc_to_intel_dp(encoder));
-}
-
 static inline struct intel_digital_port *
 hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0ef7cb8134b6..b038f1d2a7ad 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2432,12 +2432,22 @@ static void intel_dp_compute_vsc_colorimetry(const struct intel_crtc_state *crtc
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	/*
-	 * Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
-	 * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
-	 * Colorimetry Format indication.
-	 */
-	vsc->revision = 0x5;
+	if (crtc_state->has_panel_replay) {
+		/*
+		 * Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
+		 * VSC SDP supporting 3D stereo, Panel Replay, and Pixel
+		 * Encoding/Colorimetry Format indication.
+		 */
+		vsc->revision = 0x7;
+	} else {
+		/*
+		 * Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
+		 * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
+		 * Colorimetry Format indication.
+		 */
+		vsc->revision = 0x5;
+	}
+
 	vsc->length = 0x13;
 
 	/* DP 1.4a spec, Table 2-120 */
@@ -2546,6 +2556,21 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
 			vsc->revision = 0x4;
 			vsc->length = 0xe;
 		}
+	} else if (crtc_state->has_panel_replay) {
+		if (intel_dp->psr.colorimetry_support &&
+		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
+			/* [Panel Replay with colorimetry info] */
+			intel_dp_compute_vsc_colorimetry(crtc_state, conn_state,
+							 vsc);
+		} else {
+			/*
+			 * [Panel Replay without colorimetry info]
+			 * Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
+			 * VSC SDP supporting 3D stereo + Panel Replay.
+			 */
+			vsc->revision = 0x6;
+			vsc->length = 0x10;
+		}
 	} else {
 		/*
 		 * [PSR1]
@@ -3843,11 +3868,16 @@ static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
 	sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
 	sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
 
+	if (vsc->revision == 0x6) {
+		sdp->db[0] = 1;
+		sdp->db[3] = 1;
+	}
+
 	/*
-	 * Only revision 0x5 supports Pixel Encoding/Colorimetry Format as
-	 * per DP 1.4a spec.
+	 * Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
+	 * Format as per DP 1.4a spec and DP 2.0 respectively.
 	 */
-	if (vsc->revision != 0x5)
+	if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
 		goto out;
 
 	/* VSC SDP Payload for DB16 through DB18 */
@@ -5368,6 +5398,7 @@ intel_dp_detect(struct drm_connector *connector,
 	if (status == connector_status_disconnected) {
 		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
 		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
+		intel_dp->psr.sink_panel_replay_support = false;
 
 		if (intel_dp->is_mst) {
 			drm_dbg_kms(&dev_priv->drm,
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 73e397736463..617b7afbcb94 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -43,6 +43,7 @@
 #include "intel_dpio_phy.h"
 #include "intel_hdcp.h"
 #include "intel_hotplug.h"
+#include "intel_psr.h"
 #include "skl_scaler.h"
 
 static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp,
@@ -422,6 +423,8 @@ static int intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 
+	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 0669ab7a9191..f9837001aa5f 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -172,6 +172,15 @@
  * irrelevant for normal operation.
  */
 
+bool intel_encoder_can_psr(struct intel_encoder *encoder)
+{
+	if (intel_encoder_is_dp(encoder) || encoder->type == INTEL_OUTPUT_DP_MST)
+		return CAN_PSR(enc_to_intel_dp(encoder)) ||
+		       CAN_PANEL_REPLAY(enc_to_intel_dp(encoder));
+	else
+		return false;
+}
+
 static bool psr_global_enabled(struct intel_dp *intel_dp)
 {
 	struct intel_connector *connector = intel_dp->attached_connector;
@@ -474,6 +483,25 @@ static void intel_dp_get_su_granularity(struct intel_dp *intel_dp)
 	intel_dp->psr.su_y_granularity = y;
 }
 
+static void _panel_replay_init_dpcd(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
+	u8 pr_dpcd = 0;
+
+	intel_dp->psr.sink_panel_replay_support = false;
+	drm_dp_dpcd_readb(&intel_dp->aux, DP_PANEL_REPLAY_CAP, &pr_dpcd);
+
+	if (!(pr_dpcd & DP_PANEL_REPLAY_SUPPORT)) {
+		drm_dbg_kms(&i915->drm,
+			    "Panel replay is not supported by panel\n");
+		return;
+	}
+
+	drm_dbg_kms(&i915->drm,
+		    "Panel replay is supported by panel\n");
+	intel_dp->psr.sink_panel_replay_support = true;
+}
+
 static void _psr_init_dpcd(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 =
@@ -523,12 +551,13 @@ static void _psr_init_dpcd(struct intel_dp *intel_dp)
 
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
+	_panel_replay_init_dpcd(intel_dp);
+
 	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
 			 sizeof(intel_dp->psr_dpcd));
 
 	if (intel_dp->psr_dpcd[0])
 		_psr_init_dpcd(intel_dp);
-	/* TODO: Add PR case here */
 
 	if (intel_dp->psr.sink_psr2_support) {
 		intel_dp->psr.colorimetry_support =
@@ -1209,13 +1238,11 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 	return false;
 }
 
-void intel_psr_compute_config(struct intel_dp *intel_dp,
-			      struct intel_crtc_state *crtc_state,
-			      struct drm_connector_state *conn_state)
+static bool _psr_compute_config(struct intel_dp *intel_dp,
+				struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	const struct drm_display_mode *adjusted_mode =
-		&crtc_state->hw.adjusted_mode;
+	const struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	int psr_setup_time;
 
 	/*
@@ -1223,10 +1250,36 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 	 * So if VRR is enabled, do not enable PSR.
 	 */
 	if (crtc_state->vrr.enable)
-		return;
+		return false;
 
 	if (!CAN_PSR(intel_dp))
-		return;
+		return false;
+
+	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
+	if (psr_setup_time < 0) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "PSR condition failed: Invalid PSR setup time (0x%02x)\n",
+			    intel_dp->psr_dpcd[1]);
+		return false;
+	}
+
+	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
+	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "PSR condition failed: PSR setup time (%d us) too long\n",
+			    psr_setup_time);
+		return false;
+	}
+
+	return true;
+}
+
+void intel_psr_compute_config(struct intel_dp *intel_dp,
+			      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 = &crtc_state->hw.adjusted_mode;
 
 	if (!psr_global_enabled(intel_dp)) {
 		drm_dbg_kms(&dev_priv->drm, "PSR disabled by flag\n");
@@ -1245,23 +1298,11 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 		return;
 	}
 
-	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
-	if (psr_setup_time < 0) {
-		drm_dbg_kms(&dev_priv->drm,
-			    "PSR condition failed: Invalid PSR setup time (0x%02x)\n",
-			    intel_dp->psr_dpcd[1]);
-		return;
-	}
-
-	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
-	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
-		drm_dbg_kms(&dev_priv->drm,
-			    "PSR condition failed: PSR setup time (%d us) too long\n",
-			    psr_setup_time);
-		return;
-	}
+	if (CAN_PANEL_REPLAY(intel_dp))
+		crtc_state->has_panel_replay = true;
+	else
+		crtc_state->has_psr = _psr_compute_config(intel_dp, crtc_state);
 
-	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);
@@ -2694,7 +2735,7 @@ void intel_psr_init(struct intel_dp *intel_dp)
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 
-	if (!HAS_PSR(dev_priv))
+	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
 		return;
 
 	/*
@@ -2712,7 +2753,10 @@ void intel_psr_init(struct intel_dp *intel_dp)
 		return;
 	}
 
-	intel_dp->psr.source_support = true;
+	if (HAS_DP20(dev_priv) && !intel_dp_is_edp(intel_dp))
+		intel_dp->psr.source_panel_replay_support = true;
+	else
+		intel_dp->psr.source_support = true;
 
 	/* Set link_standby x link_off defaults */
 	if (DISPLAY_VER(dev_priv) < 12)
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index bf35f42df6bc..6a1f4573852b 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -21,6 +21,13 @@ struct intel_encoder;
 struct intel_plane;
 struct intel_plane_state;
 
+#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
+			   (intel_dp)->psr.source_support)
+
+#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)->psr.sink_panel_replay_support && \
+				    (intel_dp)->psr.source_panel_replay_support)
+
+bool intel_encoder_can_psr(struct intel_encoder *encoder);
 void intel_psr_init_dpcd(struct intel_dp *intel_dp);
 void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc);
-- 
2.29.0

[PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP

[Animesh Manna]

Due to similarity panel replay dpcd initialization got added in psr
function which is specific for edp panel. This patch enables panel
replay initialization for dp connector.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index f9837001aa5f..a2e0637c53fb 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2738,6 +2738,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
 	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
 		return;
 
+	if (!intel_dp_is_edp(intel_dp))
+		intel_psr_init_dpcd(intel_dp);
+
 	/*
 	 * HSW spec explicitly says PSR is tied to port A.
 	 * BDW+ platforms have a instance of PSR registers per transcoder but
-- 
2.29.0

[PATCH v7 5/6] drm/i915/panelreplay: enable/disable panel replay

[Animesh Manna]

TRANS_DP2_CTL register is programmed to enable panel replay from source
and sink is enabled through panel replay dpcd configuration address.

Bspec: 1407940617

v1: Initial version.
v2:
- Use pr_* flags instead psr_* flags. [Jouni]
- Remove intel_dp_is_edp check as edp1.5 also has panel replay. [Jouni]

v3: Cover letter updated and selective fetch condition check is added
before updating its bit in PSR2_MAN_TRK_CTL register. [Jouni]

v4: Selective fetch related PSR2_MAN_TRK_CTL programmming dropped. [Jouni]

v5: Added PSR2_MAN_TRK_CTL programming as needed for Continuous Full
Frame (CFF) update.

v6: Rebased on latest.

Note: Initial plan is to enable panel replay in  full-screen live active
frame update mode. In a incremental approach panel replay will be enabled
in selctive update mode if there is any gap in curent implementation.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c      |  7 ++-
 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_psr.c      | 63 ++++++++++++++-----
 3 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9151d5add960..16f98a7a5f20 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2717,10 +2717,15 @@ static void intel_ddi_pre_enable_dp(struct intel_atomic_state *state,
 				    const struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 
-	if (HAS_DP20(dev_priv))
+	if (HAS_DP20(dev_priv)) {
 		intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
 					    crtc_state);
+		if (crtc_state->has_panel_replay)
+			drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG,
+					   DP_PANEL_REPLAY_ENABLE);
+	}
 
 	if (DISPLAY_VER(dev_priv) >= 14)
 		mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 95b318f7b2b8..d8f35054bc11 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1704,6 +1704,7 @@ struct intel_psr {
 	u16 su_y_granularity;
 	bool source_panel_replay_support;
 	bool sink_panel_replay_support;
+	bool panel_replay_enabled;
 	u32 dc3co_exitline;
 	u32 dc3co_exit_delay;
 	struct delayed_work dc3co_work;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a2e0637c53fb..80de831c2f60 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -608,8 +608,11 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	u8 dpcd_val = DP_PSR_ENABLE;
 
-	/* Enable ALPM at sink for psr2 */
+	if (intel_dp->psr.panel_replay_enabled)
+		return;
+
 	if (intel_dp->psr.psr2_enabled) {
+		/* Enable ALPM at sink for psr2 */
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE |
 				   DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
@@ -759,6 +762,17 @@ static int psr2_block_count(struct intel_dp *intel_dp)
 	return psr2_block_count_lines(intel_dp) / 4;
 }
 
+static void dg2_activate_panel_replay(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+	intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
+		     0, ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME);
+
+	intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp->psr.transcoder), 0,
+		     TRANS_DP2_PANEL_REPLAY_ENABLE);
+}
+
 static void hsw_activate_psr2(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1323,18 +1337,23 @@ void intel_psr_get_config(struct intel_encoder *encoder,
 		return;
 
 	intel_dp = &dig_port->dp;
-	if (!CAN_PSR(intel_dp))
+	if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp)))
 		return;
 
 	mutex_lock(&intel_dp->psr.lock);
 	if (!intel_dp->psr.enabled)
 		goto unlock;
 
-	/*
-	 * Not possible to read EDP_PSR/PSR2_CTL registers as it is
-	 * enabled/disabled because of frontbuffer tracking and others.
-	 */
-	pipe_config->has_psr = true;
+	if (intel_dp->psr.panel_replay_enabled) {
+		pipe_config->has_panel_replay = true;
+	} else {
+		/*
+		 * Not possible to read EDP_PSR/PSR2_CTL registers as it is
+		 * enabled/disabled because of frontbuffer tracking and others.
+		 */
+		pipe_config->has_psr = true;
+	}
+
 	pipe_config->has_psr2 = intel_dp->psr.psr2_enabled;
 	pipe_config->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_VSC);
 
@@ -1371,8 +1390,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
 
 	lockdep_assert_held(&intel_dp->psr.lock);
 
-	/* psr1 and psr2 are mutually exclusive.*/
-	if (intel_dp->psr.psr2_enabled)
+	/* psr1, psr2 and panel-replay are mutually exclusive.*/
+	if (intel_dp->psr.panel_replay_enabled)
+		dg2_activate_panel_replay(intel_dp);
+	else if (intel_dp->psr.psr2_enabled)
 		hsw_activate_psr2(intel_dp);
 	else
 		hsw_activate_psr1(intel_dp);
@@ -1550,6 +1571,7 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
 	drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
 
 	intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
+	intel_dp->psr.panel_replay_enabled = crtc_state->has_panel_replay;
 	intel_dp->psr.busy_frontbuffer_bits = 0;
 	intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
 	intel_dp->psr.transcoder = crtc_state->cpu_transcoder;
@@ -1565,8 +1587,12 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
 	if (!psr_interrupt_error_check(intel_dp))
 		return;
 
-	drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
-		    intel_dp->psr.psr2_enabled ? "2" : "1");
+	if (intel_dp->psr.panel_replay_enabled)
+		drm_dbg_kms(&dev_priv->drm, "Enabling Panel Replay\n");
+	else
+		drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
+			    intel_dp->psr.psr2_enabled ? "2" : "1");
+
 	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);
@@ -1595,7 +1621,10 @@ static void intel_psr_exit(struct intel_dp *intel_dp)
 		return;
 	}
 
-	if (intel_dp->psr.psr2_enabled) {
+	if (intel_dp->psr.panel_replay_enabled) {
+		intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp->psr.transcoder),
+			     TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
+	} else if (intel_dp->psr.psr2_enabled) {
 		tgl_disallow_dc3co_on_psr2_exit(intel_dp);
 
 		val = intel_de_rmw(dev_priv, EDP_PSR2_CTL(cpu_transcoder),
@@ -1644,8 +1673,11 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
 	if (!intel_dp->psr.enabled)
 		return;
 
-	drm_dbg_kms(&dev_priv->drm, "Disabling PSR%s\n",
-		    intel_dp->psr.psr2_enabled ? "2" : "1");
+	if (intel_dp->psr.panel_replay_enabled)
+		drm_dbg_kms(&dev_priv->drm, "Disabling Panel Replay\n");
+	else
+		drm_dbg_kms(&dev_priv->drm, "Disabling PSR%s\n",
+			    intel_dp->psr.psr2_enabled ? "2" : "1");
 
 	intel_psr_exit(intel_dp);
 	intel_psr_wait_exit_locked(intel_dp);
@@ -1678,6 +1710,7 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG, 0);
 
 	intel_dp->psr.enabled = false;
+	intel_dp->psr.panel_replay_enabled = false;
 	intel_dp->psr.psr2_enabled = false;
 	intel_dp->psr.psr2_sel_fetch_enabled = false;
 	intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
@@ -2249,7 +2282,7 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state,
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_encoder *encoder;
 
-	if (!crtc_state->has_psr)
+	if (!(crtc_state->has_psr || crtc_state->has_panel_replay))
 		return;
 
 	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
-- 
2.29.0

[PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Animesh Manna]

Add debugfs support which will print source and sink status
per connector basis.

v1: Initial version. [rb-ed by Arun]
v2: Added check for DP 2.0 and connector type in connector_debugfs_add().

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Arun R Murthy <arun.r.murthy@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 136 +++++++++++++++++------
 1 file changed, 102 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 80de831c2f60..399fc0a8e636 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct intel_dp *intel_dp,
 	return 0;
 }
 
+static int panel_replay_get_status_and_error_status(struct intel_dp *intel_dp,
+						    u8 *status, u8 *error_status)
+{
+	struct drm_dp_aux *aux = &intel_dp->aux;
+	int ret;
+
+	ret = drm_dp_dpcd_readb(aux, DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
+	if (ret != 1)
+		return ret;
+
+	ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS, error_status);
+	if (ret != 1)
+		return ret;
+
+	*status = *status & DP_PSR_SINK_STATE_MASK;
+
+	return 0;
+}
+
 static void psr_alpm_check(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
 			status = live_status[status_val];
 	}
 
-	seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
+	seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status, val);
 }
 
 static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp)
@@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp)
 	bool enabled;
 	u32 val;
 
-	seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
-	if (psr->sink_support)
+	seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
+		   str_yes_no(psr->sink_support),
+		   str_yes_no(psr->sink_panel_replay_support));
+
+	if (psr->sink_support || psr->sink_panel_replay_support)
 		seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
 	seq_puts(m, "\n");
 
-	if (!psr->sink_support)
+	if (!(psr->sink_support || psr->sink_panel_replay_support))
 		return 0;
 
 	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
 	mutex_lock(&psr->lock);
 
-	if (psr->enabled)
+	if (psr->panel_replay_enabled)
+		status = "Panel Replay Enabled";
+	else if (psr->enabled)
 		status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
 	else
 		status = "disabled";
@@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (psr->psr2_enabled) {
+	if (psr->panel_replay_enabled) {
+		val = intel_de_read(dev_priv, TRANS_DP2_CTL(cpu_transcoder));
+		enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
+	} else if (psr->psr2_enabled) {
 		val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
 		enabled = val & EDP_PSR2_ENABLE;
 	} else {
 		val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv, cpu_transcoder));
 		enabled = val & EDP_PSR_ENABLE;
 	}
-	seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
+	seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
 		   str_enabled_disabled(enabled), val);
 	psr_source_status(intel_dp, m);
 	seq_printf(m, "Busy frontbuffer bits: 0x%08x\n",
@@ -3221,6 +3248,7 @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)
 {
 	struct intel_connector *connector = m->private;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct intel_psr *psr = &intel_dp->psr;
 	static const char * const sink_status[] = {
 		"inactive",
 		"transition to active, capture and display",
@@ -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)
 		"reserved",
 		"sink internal error",
 	};
+	static const char * const panel_replay_status[] = {
+		"Sink device frame is locked to the Source device",
+		"Sink device is coasting, using the VTotal target",
+		"Sink device is governing the frame rate (frame rate unlock is granted)",
+		"Sink device in the process of re-locking with the Source device",
+	};
 	const char *str;
 	int ret;
 	u8 status, error_status;
 
-	if (!CAN_PSR(intel_dp)) {
-		seq_puts(m, "PSR Unsupported\n");
+	if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
+		seq_puts(m, "PSR/Panel-Replay Unsupported\n");
 		return -ENODEV;
 	}
 
 	if (connector->base.status != connector_status_connected)
 		return -ENODEV;
 
-	ret = psr_get_status_and_error_status(intel_dp, &status, &error_status);
-	if (ret)
-		return ret;
+	if (psr->panel_replay_enabled) {
+		u32 temp;
 
-	status &= DP_PSR_SINK_STATE_MASK;
-	if (status < ARRAY_SIZE(sink_status))
-		str = sink_status[status];
-	else
-		str = "unknown";
+		ret = panel_replay_get_status_and_error_status(intel_dp, &status, &error_status);
+		if (ret)
+			return ret;
 
-	seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
+		temp = status & DP_SINK_FRAME_LOCKED_MASK;
+		temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
+		if (temp < ARRAY_SIZE(panel_replay_status))
+			str = panel_replay_status[temp];
+		else
+			str = "unknown";
 
-	seq_printf(m, "Sink PSR error status: 0x%x", error_status);
+		seq_printf(m, "Sink Panel Replay status: 0x%x [%s]\n", status, str);
 
-	if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
-			    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
-			    DP_PSR_LINK_CRC_ERROR))
-		seq_puts(m, ":\n");
-	else
-		seq_puts(m, "\n");
-	if (error_status & DP_PSR_RFB_STORAGE_ERROR)
-		seq_puts(m, "\tPSR RFB storage error\n");
-	if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
-		seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
-	if (error_status & DP_PSR_LINK_CRC_ERROR)
-		seq_puts(m, "\tPSR Link CRC error\n");
+		seq_printf(m, "Sink Panel Replay error status: 0x%x", error_status);
+
+		if (error_status & (DP_PANEL_REPLAY_RFB_STORAGE_ERROR |
+				    DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
+				    DP_PANEL_REPLAY_LINK_CRC_ERROR))
+			seq_puts(m, ":\n");
+		else
+			seq_puts(m, "\n");
+		if (error_status & DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
+			seq_puts(m, "\tPANEL-REPLAY RFB storage error\n");
+		if (error_status & DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
+			seq_puts(m, "\tPANEL-REPLAY VSC SDP uncorrectable error\n");
+		if (error_status & DP_PANEL_REPLAY_LINK_CRC_ERROR)
+			seq_puts(m, "\tPANEL-REPLAY Link CRC error\n");
+	} else {
+		ret = psr_get_status_and_error_status(intel_dp, &status, &error_status);
+		if (ret)
+			return ret;
+
+		status &= DP_PSR_SINK_STATE_MASK;
+		if (status < ARRAY_SIZE(sink_status))
+			str = sink_status[status];
+		else
+			str = "unknown";
+
+		seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
+
+		seq_printf(m, "Sink PSR error status: 0x%x", error_status);
 
+		if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
+				    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
+				    DP_PSR_LINK_CRC_ERROR))
+			seq_puts(m, ":\n");
+		else
+			seq_puts(m, "\n");
+		if (error_status & DP_PSR_RFB_STORAGE_ERROR)
+			seq_puts(m, "\tPSR RFB storage error\n");
+		if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
+			seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
+		if (error_status & DP_PSR_LINK_CRC_ERROR)
+			seq_puts(m, "\tPSR Link CRC error\n");
+	}
 	return ret;
 }
 DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
@@ -3288,13 +3353,16 @@ void intel_psr_connector_debugfs_add(struct intel_connector *connector)
 	struct drm_i915_private *i915 = to_i915(connector->base.dev);
 	struct dentry *root = connector->base.debugfs_entry;
 
-	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
-		return;
+	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP) {
+		if (!(HAS_DP20(i915) &&
+		      connector->base.connector_type == DRM_MODE_CONNECTOR_DisplayPort))
+			return;
+	}
 
 	debugfs_create_file("i915_psr_sink_status", 0444, root,
 			    connector, &i915_psr_sink_status_fops);
 
-	if (HAS_PSR(i915))
+	if (HAS_PSR(i915) || HAS_DP20(i915))
 		debugfs_create_file("i915_psr_status", 0444, root,
 				    connector, &i915_psr_status_fops);
 }
-- 
2.29.0

RE: [PATCH v7 3/6] drm/i915/panelreplay: Initializaton and compute config for panel replay

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni <jouni.hogander@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v7 3/6] drm/i915/panelreplay: Initializaton and compute config
> for panel replay
> 
> Modify existing PSR implementation to enable panel replay feature of DP 2.0
> which is similar to PSR feature of EDP panel. There is different DPCD address to
> check panel capability compare to PSR and vsc sdp header is different.
> 
> v1: Initial version.
> v2:
> - Set source_panel_replay_support flag under HAS_PANEL_REPLAY() condition
> check. [Jouni]
> - Code restructured around intel_panel_replay_init and renamed to
> intel_panel_replay_init_dpcd. [Jouni]
> - Remove the initial code modification around has_psr2 flag. [Jouni]
> - Add CAN_PANEL_REPLAY() in intel_encoder_can_psr which is used to enable in
> intel_psr_post_plane_update. [Jouni]
> v3:
> - Initialize both psr and panel-replay. [Jouni]
> - Initialize both panel replay and psr if detected. [Jouni]
> - Refactoring psr function by introducing _psr_compute_config(). [Jouni]
> - Add check for !is_edp while deriving source_panel_replay_support. [Jouni]
> - Enable panel replay dpcd initialization in a separate patch. [Jouni]
> 
> v4:
> - HAS_PANEL_REPLAY() check not needed during sink capability check. [Jouni]
> - Set either panel replay source support or psr. [Jouni]
> 
> v5:
> - HAS_PANEL_REPLAY() removed and use HAS_DP20() instead. [Jouni]
> - Move psr related code to intel_psr.c. [Jani]
> - Reset sink_panel_replay_support flag during disconnection. [Jani]
> 
> v6: return statement restored which is removed by misatke. [Jouni]
> v7: cosmetic changes. [Arun]
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---

Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------

>  .../drm/i915/display/intel_display_types.h    | 14 +--
>  drivers/gpu/drm/i915/display/intel_dp.c       | 49 ++++++++--
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  3 +
>  drivers/gpu/drm/i915/display/intel_psr.c      | 96 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_psr.h      |  7 ++
>  5 files changed, 123 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8d8b2f8d37a9..95b318f7b2b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1204,6 +1204,7 @@ struct intel_crtc_state {
>  	bool has_psr2;
>  	bool enable_psr2_sel_fetch;
>  	bool req_psr2_sdp_prior_scanline;
> +	bool has_panel_replay;
>  	bool wm_level_disabled;
>  	u32 dc3co_exitline;
>  	u16 su_y_granularity;
> @@ -1701,6 +1702,8 @@ struct intel_psr {
>  	bool irq_aux_error;
>  	u16 su_w_granularity;
>  	u16 su_y_granularity;
> +	bool source_panel_replay_support;
> +	bool sink_panel_replay_support;
>  	u32 dc3co_exitline;
>  	u32 dc3co_exit_delay;
>  	struct delayed_work dc3co_work;
> @@ -1988,17 +1991,6 @@ dp_to_lspcon(struct intel_dp *intel_dp)
> 
>  #define dp_to_i915(__intel_dp) to_i915(dp_to_dig_port(__intel_dp)-
> >base.base.dev)
> 
> -#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
> -			   (intel_dp)->psr.source_support)
> -
> -static inline bool intel_encoder_can_psr(struct intel_encoder *encoder) -{
> -	if (!intel_encoder_is_dp(encoder))
> -		return false;
> -
> -	return CAN_PSR(enc_to_intel_dp(encoder));
> -}
> -
>  static inline struct intel_digital_port *  hdmi_to_dig_port(struct intel_hdmi
> *intel_hdmi)  { diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0ef7cb8134b6..b038f1d2a7ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2432,12 +2432,22 @@ static void
> intel_dp_compute_vsc_colorimetry(const struct intel_crtc_state *crtc
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 
> -	/*
> -	 * Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
> -	 * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> -	 * Colorimetry Format indication.
> -	 */
> -	vsc->revision = 0x5;
> +	if (crtc_state->has_panel_replay) {
> +		/*
> +		 * Prepare VSC Header for SU as per DP 2.0 spec, Table 2-223
> +		 * VSC SDP supporting 3D stereo, Panel Replay, and Pixel
> +		 * Encoding/Colorimetry Format indication.
> +		 */
> +		vsc->revision = 0x7;
> +	} else {
> +		/*
> +		 * Prepare VSC Header for SU as per DP 1.4 spec, Table 2-118
> +		 * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> +		 * Colorimetry Format indication.
> +		 */
> +		vsc->revision = 0x5;
> +	}
> +
>  	vsc->length = 0x13;
> 
>  	/* DP 1.4a spec, Table 2-120 */
> @@ -2546,6 +2556,21 @@ void intel_dp_compute_psr_vsc_sdp(struct intel_dp
> *intel_dp,
>  			vsc->revision = 0x4;
>  			vsc->length = 0xe;
>  		}
> +	} else if (crtc_state->has_panel_replay) {
> +		if (intel_dp->psr.colorimetry_support &&
> +		    intel_dp_needs_vsc_sdp(crtc_state, conn_state)) {
> +			/* [Panel Replay with colorimetry info] */
> +			intel_dp_compute_vsc_colorimetry(crtc_state,
> conn_state,
> +							 vsc);
> +		} else {
> +			/*
> +			 * [Panel Replay without colorimetry info]
> +			 * Prepare VSC Header for SU as per DP 2.0 spec, Table
> 2-223
> +			 * VSC SDP supporting 3D stereo + Panel Replay.
> +			 */
> +			vsc->revision = 0x6;
> +			vsc->length = 0x10;
> +		}
>  	} else {
>  		/*
>  		 * [PSR1]
> @@ -3843,11 +3868,16 @@ static ssize_t intel_dp_vsc_sdp_pack(const struct
> drm_dp_vsc_sdp *vsc,
>  	sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
>  	sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
> 
> +	if (vsc->revision == 0x6) {
> +		sdp->db[0] = 1;
> +		sdp->db[3] = 1;
> +	}
> +
>  	/*
> -	 * Only revision 0x5 supports Pixel Encoding/Colorimetry Format as
> -	 * per DP 1.4a spec.
> +	 * Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
> +	 * Format as per DP 1.4a spec and DP 2.0 respectively.
>  	 */
> -	if (vsc->revision != 0x5)
> +	if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
>  		goto out;
> 
>  	/* VSC SDP Payload for DB16 through DB18 */ @@ -5368,6 +5398,7
> @@ intel_dp_detect(struct drm_connector *connector,
>  	if (status == connector_status_disconnected) {
>  		memset(&intel_dp->compliance, 0, sizeof(intel_dp-
> >compliance));
>  		memset(intel_dp->dsc_dpcd, 0, sizeof(intel_dp->dsc_dpcd));
> +		intel_dp->psr.sink_panel_replay_support = false;
> 
>  		if (intel_dp->is_mst) {
>  			drm_dbg_kms(&dev_priv->drm,
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 73e397736463..617b7afbcb94 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -43,6 +43,7 @@
>  #include "intel_dpio_phy.h"
>  #include "intel_hdcp.h"
>  #include "intel_hotplug.h"
> +#include "intel_psr.h"
>  #include "skl_scaler.h"
> 
>  static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int
> bpp, @@ -422,6 +423,8 @@ static int intel_dp_mst_compute_config(struct
> intel_encoder *encoder,
> 
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> 
> +	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 0669ab7a9191..f9837001aa5f 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -172,6 +172,15 @@
>   * irrelevant for normal operation.
>   */
> 
> +bool intel_encoder_can_psr(struct intel_encoder *encoder) {
> +	if (intel_encoder_is_dp(encoder) || encoder->type ==
> INTEL_OUTPUT_DP_MST)
> +		return CAN_PSR(enc_to_intel_dp(encoder)) ||
> +		       CAN_PANEL_REPLAY(enc_to_intel_dp(encoder));
> +	else
> +		return false;
> +}
> +
>  static bool psr_global_enabled(struct intel_dp *intel_dp)  {
>  	struct intel_connector *connector = intel_dp->attached_connector; @@
> -474,6 +483,25 @@ static void intel_dp_get_su_granularity(struct intel_dp
> *intel_dp)
>  	intel_dp->psr.su_y_granularity = y;
>  }
> 
> +static void _panel_replay_init_dpcd(struct intel_dp *intel_dp) {
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	u8 pr_dpcd = 0;
> +
> +	intel_dp->psr.sink_panel_replay_support = false;
> +	drm_dp_dpcd_readb(&intel_dp->aux, DP_PANEL_REPLAY_CAP,
> &pr_dpcd);
> +
> +	if (!(pr_dpcd & DP_PANEL_REPLAY_SUPPORT)) {
> +		drm_dbg_kms(&i915->drm,
> +			    "Panel replay is not supported by panel\n");
> +		return;
> +	}
> +
> +	drm_dbg_kms(&i915->drm,
> +		    "Panel replay is supported by panel\n");
> +	intel_dp->psr.sink_panel_replay_support = true; }
> +
>  static void _psr_init_dpcd(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *i915 =
> @@ -523,12 +551,13 @@ static void _psr_init_dpcd(struct intel_dp *intel_dp)
> 
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp)  {
> +	_panel_replay_init_dpcd(intel_dp);
> +
>  	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp-
> >psr_dpcd,
>  			 sizeof(intel_dp->psr_dpcd));
> 
>  	if (intel_dp->psr_dpcd[0])
>  		_psr_init_dpcd(intel_dp);
> -	/* TODO: Add PR case here */
> 
>  	if (intel_dp->psr.sink_psr2_support) {
>  		intel_dp->psr.colorimetry_support =
> @@ -1209,13 +1238,11 @@ static bool intel_psr2_config_valid(struct intel_dp
> *intel_dp,
>  	return false;
>  }
> 
> -void intel_psr_compute_config(struct intel_dp *intel_dp,
> -			      struct intel_crtc_state *crtc_state,
> -			      struct drm_connector_state *conn_state)
> +static bool _psr_compute_config(struct intel_dp *intel_dp,
> +				struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	const struct drm_display_mode *adjusted_mode =
> -		&crtc_state->hw.adjusted_mode;
> +	const struct drm_display_mode *adjusted_mode =
> +&crtc_state->hw.adjusted_mode;
>  	int psr_setup_time;
> 
>  	/*
> @@ -1223,10 +1250,36 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
>  	 * So if VRR is enabled, do not enable PSR.
>  	 */
>  	if (crtc_state->vrr.enable)
> -		return;
> +		return false;
> 
>  	if (!CAN_PSR(intel_dp))
> -		return;
> +		return false;
> +
> +	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> +	if (psr_setup_time < 0) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "PSR condition failed: Invalid PSR setup time
> (0x%02x)\n",
> +			    intel_dp->psr_dpcd[1]);
> +		return false;
> +	}
> +
> +	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> +	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "PSR condition failed: PSR setup time (%d us) too
> long\n",
> +			    psr_setup_time);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +void intel_psr_compute_config(struct intel_dp *intel_dp,
> +			      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 =
> +&crtc_state->hw.adjusted_mode;
> 
>  	if (!psr_global_enabled(intel_dp)) {
>  		drm_dbg_kms(&dev_priv->drm, "PSR disabled by flag\n"); @@
> -1245,23 +1298,11 @@ void intel_psr_compute_config(struct intel_dp
> *intel_dp,
>  		return;
>  	}
> 
> -	psr_setup_time = drm_dp_psr_setup_time(intel_dp->psr_dpcd);
> -	if (psr_setup_time < 0) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "PSR condition failed: Invalid PSR setup time
> (0x%02x)\n",
> -			    intel_dp->psr_dpcd[1]);
> -		return;
> -	}
> -
> -	if (intel_usecs_to_scanlines(adjusted_mode, psr_setup_time) >
> -	    adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay - 1) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "PSR condition failed: PSR setup time (%d us) too
> long\n",
> -			    psr_setup_time);
> -		return;
> -	}
> +	if (CAN_PANEL_REPLAY(intel_dp))
> +		crtc_state->has_panel_replay = true;
> +	else
> +		crtc_state->has_psr = _psr_compute_config(intel_dp,
> crtc_state);
> 
> -	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);
> @@ -2694,7 +2735,7 @@ void intel_psr_init(struct intel_dp *intel_dp)
>  	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> 
> -	if (!HAS_PSR(dev_priv))
> +	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
>  		return;
> 
>  	/*
> @@ -2712,7 +2753,10 @@ void intel_psr_init(struct intel_dp *intel_dp)
>  		return;
>  	}
> 
> -	intel_dp->psr.source_support = true;
> +	if (HAS_DP20(dev_priv) && !intel_dp_is_edp(intel_dp))
> +		intel_dp->psr.source_panel_replay_support = true;
> +	else
> +		intel_dp->psr.source_support = true;
> 
>  	/* Set link_standby x link_off defaults */
>  	if (DISPLAY_VER(dev_priv) < 12)
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index bf35f42df6bc..6a1f4573852b 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -21,6 +21,13 @@ struct intel_encoder;  struct intel_plane;  struct
> intel_plane_state;
> 
> +#define CAN_PSR(intel_dp) ((intel_dp)->psr.sink_support && \
> +			   (intel_dp)->psr.source_support)
> +
> +#define CAN_PANEL_REPLAY(intel_dp) ((intel_dp)-
> >psr.sink_panel_replay_support && \
> +				    (intel_dp)-
> >psr.source_panel_replay_support)
> +
> +bool intel_encoder_can_psr(struct intel_encoder *encoder);
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp);  void
> intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  				struct intel_crtc *crtc);
> --
> 2.29.0

RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni <jouni.hogander@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd
> initialization for DP
> 
> Due to similarity panel replay dpcd initialization got added in psr function which
> is specific for edp panel. This patch enables panel replay initialization for dp
> connector.
> 
If panelreplay initialization then why is the function name psr_init_dpcd() ?
Also it its similar to PSR then these dpcd should already be available.

Thanks and Regards,
Arun R Murthy
--------------------

> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index f9837001aa5f..a2e0637c53fb 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2738,6 +2738,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
>  	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
>  		return;
> 
> +	if (!intel_dp_is_edp(intel_dp))
> +		intel_psr_init_dpcd(intel_dp);
> +
>  	/*
>  	 * HSW spec explicitly says PSR is tied to port A.
>  	 * BDW+ platforms have a instance of PSR registers per transcoder but
> --
> 2.29.0

RE: [PATCH v7 5/6] drm/i915/panelreplay: enable/disable panel replay

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni <jouni.hogander@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v7 5/6] drm/i915/panelreplay: enable/disable panel replay
> 
> TRANS_DP2_CTL register is programmed to enable panel replay from source
> and sink is enabled through panel replay dpcd configuration address.
> 
> Bspec: 1407940617
> 
> v1: Initial version.
> v2:
> - Use pr_* flags instead psr_* flags. [Jouni]
> - Remove intel_dp_is_edp check as edp1.5 also has panel replay. [Jouni]
> 
> v3: Cover letter updated and selective fetch condition check is added before
> updating its bit in PSR2_MAN_TRK_CTL register. [Jouni]
> 
> v4: Selective fetch related PSR2_MAN_TRK_CTL programmming dropped.
> [Jouni]
> 
> v5: Added PSR2_MAN_TRK_CTL programming as needed for Continuous Full
> Frame (CFF) update.
> 
> v6: Rebased on latest.
> 
> Note: Initial plan is to enable panel replay in  full-screen live active frame
> update mode. In a incremental approach panel replay will be enabled in selctive
> update mode if there is any gap in curent implementation.
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  7 ++-
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_psr.c      | 63 ++++++++++++++-----
>  3 files changed, 55 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9151d5add960..16f98a7a5f20 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2717,10 +2717,15 @@ static void intel_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
>  				    const struct drm_connector_state
> *conn_state)  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> 
> -	if (HAS_DP20(dev_priv))
> +	if (HAS_DP20(dev_priv)) {
>  		intel_dp_128b132b_sdp_crc16(enc_to_intel_dp(encoder),
>  					    crtc_state);
> +		if (crtc_state->has_panel_replay)
> +			drm_dp_dpcd_writeb(&intel_dp->aux,
> PANEL_REPLAY_CONFIG,
> +					   DP_PANEL_REPLAY_ENABLE);
> +	}
> 
>  	if (DISPLAY_VER(dev_priv) >= 14)
>  		mtl_ddi_pre_enable_dp(state, encoder, crtc_state, conn_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 95b318f7b2b8..d8f35054bc11 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1704,6 +1704,7 @@ struct intel_psr {
>  	u16 su_y_granularity;
>  	bool source_panel_replay_support;
>  	bool sink_panel_replay_support;
> +	bool panel_replay_enabled;
>  	u32 dc3co_exitline;
>  	u32 dc3co_exit_delay;
>  	struct delayed_work dc3co_work;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index a2e0637c53fb..80de831c2f60 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -608,8 +608,11 @@ static void intel_psr_enable_sink(struct intel_dp
> *intel_dp)
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  	u8 dpcd_val = DP_PSR_ENABLE;
> 
> -	/* Enable ALPM at sink for psr2 */
> +	if (intel_dp->psr.panel_replay_enabled)
> +		return;
> +
>  	if (intel_dp->psr.psr2_enabled) {
> +		/* Enable ALPM at sink for psr2 */
Unrelated change

Upon removing this
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------
>  		drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE |
>  				   DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
> @@ -759,6 +762,17 @@ static int psr2_block_count(struct intel_dp *intel_dp)
>  	return psr2_block_count_lines(intel_dp) / 4;  }
> 
> +static void dg2_activate_panel_replay(struct intel_dp *intel_dp) {
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> +	intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
> +		     0,
> ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME);
> +
> +	intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp->psr.transcoder), 0,
> +		     TRANS_DP2_PANEL_REPLAY_ENABLE);
> +}
> +
>  static void hsw_activate_psr2(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 1323,18 +1337,23 @@ void intel_psr_get_config(struct intel_encoder
> *encoder,
>  		return;
> 
>  	intel_dp = &dig_port->dp;
> -	if (!CAN_PSR(intel_dp))
> +	if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp)))
>  		return;
> 
>  	mutex_lock(&intel_dp->psr.lock);
>  	if (!intel_dp->psr.enabled)
>  		goto unlock;
> 
> -	/*
> -	 * Not possible to read EDP_PSR/PSR2_CTL registers as it is
> -	 * enabled/disabled because of frontbuffer tracking and others.
> -	 */
> -	pipe_config->has_psr = true;
> +	if (intel_dp->psr.panel_replay_enabled) {
> +		pipe_config->has_panel_replay = true;
> +	} else {
> +		/*
> +		 * Not possible to read EDP_PSR/PSR2_CTL registers as it is
> +		 * enabled/disabled because of frontbuffer tracking and others.
> +		 */
> +		pipe_config->has_psr = true;
> +	}
> +
>  	pipe_config->has_psr2 = intel_dp->psr.psr2_enabled;
>  	pipe_config->infoframes.enable |=
> intel_hdmi_infoframe_enable(DP_SDP_VSC);
> 
> @@ -1371,8 +1390,10 @@ static void intel_psr_activate(struct intel_dp
> *intel_dp)
> 
>  	lockdep_assert_held(&intel_dp->psr.lock);
> 
> -	/* psr1 and psr2 are mutually exclusive.*/
> -	if (intel_dp->psr.psr2_enabled)
> +	/* psr1, psr2 and panel-replay are mutually exclusive.*/
> +	if (intel_dp->psr.panel_replay_enabled)
> +		dg2_activate_panel_replay(intel_dp);
> +	else if (intel_dp->psr.psr2_enabled)
>  		hsw_activate_psr2(intel_dp);
>  	else
>  		hsw_activate_psr1(intel_dp);
> @@ -1550,6 +1571,7 @@ static void intel_psr_enable_locked(struct intel_dp
> *intel_dp,
>  	drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
> 
>  	intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
> +	intel_dp->psr.panel_replay_enabled = crtc_state->has_panel_replay;
>  	intel_dp->psr.busy_frontbuffer_bits = 0;
>  	intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
>  	intel_dp->psr.transcoder = crtc_state->cpu_transcoder; @@ -1565,8
> +1587,12 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
>  	if (!psr_interrupt_error_check(intel_dp))
>  		return;
> 
> -	drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> -		    intel_dp->psr.psr2_enabled ? "2" : "1");
> +	if (intel_dp->psr.panel_replay_enabled)
> +		drm_dbg_kms(&dev_priv->drm, "Enabling Panel Replay\n");
> +	else
> +		drm_dbg_kms(&dev_priv->drm, "Enabling PSR%s\n",
> +			    intel_dp->psr.psr2_enabled ? "2" : "1");
> +
>  	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);
> @@ -1595,7 +1621,10 @@ static void intel_psr_exit(struct intel_dp *intel_dp)
>  		return;
>  	}
> 
> -	if (intel_dp->psr.psr2_enabled) {
> +	if (intel_dp->psr.panel_replay_enabled) {
> +		intel_de_rmw(dev_priv, TRANS_DP2_CTL(intel_dp-
> >psr.transcoder),
> +			     TRANS_DP2_PANEL_REPLAY_ENABLE, 0);
> +	} else if (intel_dp->psr.psr2_enabled) {
>  		tgl_disallow_dc3co_on_psr2_exit(intel_dp);
> 
>  		val = intel_de_rmw(dev_priv, EDP_PSR2_CTL(cpu_transcoder),
> @@ -1644,8 +1673,11 @@ static void intel_psr_disable_locked(struct intel_dp
> *intel_dp)
>  	if (!intel_dp->psr.enabled)
>  		return;
> 
> -	drm_dbg_kms(&dev_priv->drm, "Disabling PSR%s\n",
> -		    intel_dp->psr.psr2_enabled ? "2" : "1");
> +	if (intel_dp->psr.panel_replay_enabled)
> +		drm_dbg_kms(&dev_priv->drm, "Disabling Panel Replay\n");
> +	else
> +		drm_dbg_kms(&dev_priv->drm, "Disabling PSR%s\n",
> +			    intel_dp->psr.psr2_enabled ? "2" : "1");
> 
>  	intel_psr_exit(intel_dp);
>  	intel_psr_wait_exit_locked(intel_dp);
> @@ -1678,6 +1710,7 @@ static void intel_psr_disable_locked(struct intel_dp
> *intel_dp)
>  		drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG, 0);
> 
>  	intel_dp->psr.enabled = false;
> +	intel_dp->psr.panel_replay_enabled = false;
>  	intel_dp->psr.psr2_enabled = false;
>  	intel_dp->psr.psr2_sel_fetch_enabled = false;
>  	intel_dp->psr.psr2_sel_fetch_cff_enabled = false; @@ -2249,7 +2282,7
> @@ void intel_psr_post_plane_update(struct intel_atomic_state *state,
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct intel_encoder *encoder;
> 
> -	if (!crtc_state->has_psr)
> +	if (!(crtc_state->has_psr || crtc_state->has_panel_replay))
>  		return;
> 
>  	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> --
> 2.29.0

RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni <jouni.hogander@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel
> replay
> 
> Add debugfs support which will print source and sink status per connector
> basis.
> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in connector_debugfs_add().
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---

Reviewed-:by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------
>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +++++++++++++++++------
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct
> intel_dp *intel_dp,
>  	return 0;
>  }
> 
> +static int panel_replay_get_status_and_error_status(struct intel_dp *intel_dp,
> +						    u8 *status, u8
> *error_status) {
> +	struct drm_dp_aux *aux = &intel_dp->aux;
> +	int ret;
> +
> +	ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> +	if (ret != 1)
> +		return ret;
> +
> +	ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> +	if (ret != 1)
> +		return ret;
> +
> +	*status = *status & DP_PSR_SINK_STATE_MASK;
> +
> +	return 0;
> +}
> +
>  static void psr_alpm_check(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file
> *m)
>  			status = live_status[status_val];
>  	}
> 
> -	seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> +	seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status,
> +val);
>  }
> 
>  static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) @@ -
> 3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct
> intel_dp *intel_dp)
>  	bool enabled;
>  	u32 val;
> 
> -	seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
> -	if (psr->sink_support)
> +	seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +		   str_yes_no(psr->sink_support),
> +		   str_yes_no(psr->sink_panel_replay_support));
> +
> +	if (psr->sink_support || psr->sink_panel_replay_support)
>  		seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>  	seq_puts(m, "\n");
> 
> -	if (!psr->sink_support)
> +	if (!(psr->sink_support || psr->sink_panel_replay_support))
>  		return 0;
> 
>  	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  	mutex_lock(&psr->lock);
> 
> -	if (psr->enabled)
> +	if (psr->panel_replay_enabled)
> +		status = "Panel Replay Enabled";
> +	else if (psr->enabled)
>  		status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
>  	else
>  		status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m,
> struct intel_dp *intel_dp)
>  		goto unlock;
>  	}
> 
> -	if (psr->psr2_enabled) {
> +	if (psr->panel_replay_enabled) {
> +		val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> +		enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> +	} else if (psr->psr2_enabled) {
>  		val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
>  		enabled = val & EDP_PSR2_ENABLE;
>  	} else {
>  		val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
>  		enabled = val & EDP_PSR_ENABLE;
>  	}
> -	seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> +	seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>  		   str_enabled_disabled(enabled), val);
>  	psr_source_status(intel_dp, m);
>  	seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -3221,6 +3248,7
> @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)  {
>  	struct intel_connector *connector = m->private;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_psr *psr = &intel_dp->psr;
>  	static const char * const sink_status[] = {
>  		"inactive",
>  		"transition to active, capture and display", @@ -3231,45
> +3259,82 @@ static int i915_psr_sink_status_show(struct seq_file *m, void
> *data)
>  		"reserved",
>  		"sink internal error",
>  	};
> +	static const char * const panel_replay_status[] = {
> +		"Sink device frame is locked to the Source device",
> +		"Sink device is coasting, using the VTotal target",
> +		"Sink device is governing the frame rate (frame rate unlock is
> granted)",
> +		"Sink device in the process of re-locking with the Source
> device",
> +	};
>  	const char *str;
>  	int ret;
>  	u8 status, error_status;
> 
> -	if (!CAN_PSR(intel_dp)) {
> -		seq_puts(m, "PSR Unsupported\n");
> +	if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> +		seq_puts(m, "PSR/Panel-Replay Unsupported\n");
>  		return -ENODEV;
>  	}
> 
>  	if (connector->base.status != connector_status_connected)
>  		return -ENODEV;
> 
> -	ret = psr_get_status_and_error_status(intel_dp, &status,
> &error_status);
> -	if (ret)
> -		return ret;
> +	if (psr->panel_replay_enabled) {
> +		u32 temp;
> 
> -	status &= DP_PSR_SINK_STATE_MASK;
> -	if (status < ARRAY_SIZE(sink_status))
> -		str = sink_status[status];
> -	else
> -		str = "unknown";
> +		ret = panel_replay_get_status_and_error_status(intel_dp,
> &status, &error_status);
> +		if (ret)
> +			return ret;
> 
> -	seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> +		temp = status & DP_SINK_FRAME_LOCKED_MASK;
> +		temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> +		if (temp < ARRAY_SIZE(panel_replay_status))
> +			str = panel_replay_status[temp];
> +		else
> +			str = "unknown";
> 
> -	seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> +		seq_printf(m, "Sink Panel Replay status: 0x%x [%s]\n", status,
> str);
> 
> -	if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> -			    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> -			    DP_PSR_LINK_CRC_ERROR))
> -		seq_puts(m, ":\n");
> -	else
> -		seq_puts(m, "\n");
> -	if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> -		seq_puts(m, "\tPSR RFB storage error\n");
> -	if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> -		seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> -	if (error_status & DP_PSR_LINK_CRC_ERROR)
> -		seq_puts(m, "\tPSR Link CRC error\n");
> +		seq_printf(m, "Sink Panel Replay error status: 0x%x",
> error_status);
> +
> +		if (error_status & (DP_PANEL_REPLAY_RFB_STORAGE_ERROR |
> +
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> +				    DP_PANEL_REPLAY_LINK_CRC_ERROR))
> +			seq_puts(m, ":\n");
> +		else
> +			seq_puts(m, "\n");
> +		if (error_status & DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY RFB storage error\n");
> +		if (error_status &
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY VSC SDP uncorrectable
> error\n");
> +		if (error_status & DP_PANEL_REPLAY_LINK_CRC_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY Link CRC error\n");
> +	} else {
> +		ret = psr_get_status_and_error_status(intel_dp, &status,
> &error_status);
> +		if (ret)
> +			return ret;
> +
> +		status &= DP_PSR_SINK_STATE_MASK;
> +		if (status < ARRAY_SIZE(sink_status))
> +			str = sink_status[status];
> +		else
> +			str = "unknown";
> +
> +		seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> +
> +		seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> 
> +		if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> +				    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR
> |
> +				    DP_PSR_LINK_CRC_ERROR))
> +			seq_puts(m, ":\n");
> +		else
> +			seq_puts(m, "\n");
> +		if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> +			seq_puts(m, "\tPSR RFB storage error\n");
> +		if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> +			seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> +		if (error_status & DP_PSR_LINK_CRC_ERROR)
> +			seq_puts(m, "\tPSR Link CRC error\n");
> +	}
>  	return ret;
>  }
>  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> @@ -3288,13 +3353,16 @@ void intel_psr_connector_debugfs_add(struct
> intel_connector *connector)
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct dentry *root = connector->base.debugfs_entry;
> 
> -	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> -		return;
> +	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> {
> +		if (!(HAS_DP20(i915) &&
> +		      connector->base.connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort))
> +			return;
> +	}
> 
>  	debugfs_create_file("i915_psr_sink_status", 0444, root,
>  			    connector, &i915_psr_sink_status_fops);
> 
> -	if (HAS_PSR(i915))
> +	if (HAS_PSR(i915) || HAS_DP20(i915))
>  		debugfs_create_file("i915_psr_status", 0444, root,
>  				    connector, &i915_psr_status_fops);  }
> --
> 2.29.0

RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP

[Manna, Animesh]

> -----Original Message-----
> From: Murthy, Arun R <arun.r.murthy@intel.com>
> Sent: Monday, October 16, 2023 9:56 AM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd
> initialization for DP
> 
> 
> > -----Original Message-----
> > From: Manna, Animesh <animesh.manna@intel.com>
> > Sent: Wednesday, October 11, 2023 4:40 PM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> > <animesh.manna@intel.com>; Hogander, Jouni
> <jouni.hogander@intel.com>;
> > Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> > <jani.nikula@intel.com>
> > Subject: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd
> > initialization for DP
> >
> > Due to similarity panel replay dpcd initialization got added in psr
> > function which is specific for edp panel. This patch enables panel
> > replay initialization for dp connector.
> >
> If panelreplay initialization then why is the function name psr_init_dpcd() ?
> Also it its similar to PSR then these dpcd should already be available.

Hi Arun,

The first call for intel_psr_init_dpcd() get called from intel_edp_init_dpcd() which is not reachable for DP.
So, in this patch need to add intel_psr_init_dpcd() for DP(non-edp) in intel_psr_init().
Panel replay initialization added in intel_psr_init() as per previous feedback just to align panel-replay with psr framework. 

Regards,
Animesh
> 
> Thanks and Regards,
> Arun R Murthy
> --------------------
> 
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index f9837001aa5f..a2e0637c53fb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2738,6 +2738,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
> >  	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
> >  		return;
> >
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		intel_psr_init_dpcd(intel_dp);
> > +
> >  	/*
> >  	 * HSW spec explicitly says PSR is tied to port A.
> >  	 * BDW+ platforms have a instance of PSR registers per transcoder
> > but
> > --
> > 2.29.0

RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd initialization for DP

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Tuesday, October 17, 2023 1:52 PM
> To: Murthy, Arun R <arun.r.murthy@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> <jouni.hogander@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay dpcd
> initialization for DP
> 
> 
> 
> > -----Original Message-----
> > From: Murthy, Arun R <arun.r.murthy@intel.com>
> > Sent: Monday, October 16, 2023 9:56 AM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Hogander, Jouni
> > <jouni.hogander@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> > Subject: RE: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay
> > dpcd initialization for DP
> >
> >
> > > -----Original Message-----
> > > From: Manna, Animesh <animesh.manna@intel.com>
> > > Sent: Wednesday, October 11, 2023 4:40 PM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> > > <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>;
> > > Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> > > <jani.nikula@intel.com>
> > > Subject: [PATCH v7 4/6] drm/i915/panelreplay: Enable panel replay
> > > dpcd initialization for DP
> > >
> > > Due to similarity panel replay dpcd initialization got added in psr
> > > function which is specific for edp panel. This patch enables panel
> > > replay initialization for dp connector.
> > >
> > If panelreplay initialization then why is the function name psr_init_dpcd() ?
> > Also it its similar to PSR then these dpcd should already be available.
> 
> Hi Arun,
> 
> The first call for intel_psr_init_dpcd() get called from intel_edp_init_dpcd()
> which is not reachable for DP.
> So, in this patch need to add intel_psr_init_dpcd() for DP(non-edp) in
> intel_psr_init().
> Panel replay initialization added in intel_psr_init() as per previous feedback just
> to align panel-replay with psr framework.
> 
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
-------------------

> Regards,
> Animesh
> >
> > Thanks and Regards,
> > Arun R Murthy
> > --------------------
> >
> > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index f9837001aa5f..a2e0637c53fb 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -2738,6 +2738,9 @@ void intel_psr_init(struct intel_dp *intel_dp)
> > >  	if (!(HAS_PSR(dev_priv) || HAS_DP20(dev_priv)))
> > >  		return;
> > >
> > > +	if (!intel_dp_is_edp(intel_dp))
> > > +		intel_psr_init_dpcd(intel_dp);
> > > +
> > >  	/*
> > >  	 * HSW spec explicitly says PSR is tied to port A.
> > >  	 * BDW+ platforms have a instance of PSR registers per transcoder
> > > but
> > > --
> > > 2.29.0

RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Murthy, Arun R]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna@intel.com>
> Sent: Wednesday, October 11, 2023 4:40 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni <jouni.hogander@intel.com>;
> Murthy, Arun R <arun.r.murthy@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>
> Subject: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel
> replay
> 
> Add debugfs support which will print source and sink status per connector
> basis.
> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in connector_debugfs_add().
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>

Thanks and Regards,
Arun R Murthy
--------------------

>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +++++++++++++++++------
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int psr_get_status_and_error_status(struct
> intel_dp *intel_dp,
>  	return 0;
>  }
> 
> +static int panel_replay_get_status_and_error_status(struct intel_dp *intel_dp,
> +						    u8 *status, u8
> *error_status) {
> +	struct drm_dp_aux *aux = &intel_dp->aux;
> +	int ret;
> +
> +	ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> +	if (ret != 1)
> +		return ret;
> +
> +	ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> +	if (ret != 1)
> +		return ret;
> +
> +	*status = *status & DP_PSR_SINK_STATE_MASK;
> +
> +	return 0;
> +}
> +
>  static void psr_alpm_check(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file
> *m)
>  			status = live_status[status_val];
>  	}
> 
> -	seq_printf(m, "Source PSR status: %s [0x%08x]\n", status, val);
> +	seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n", status,
> +val);
>  }
> 
>  static int intel_psr_status(struct seq_file *m, struct intel_dp *intel_dp) @@ -
> 3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file *m, struct
> intel_dp *intel_dp)
>  	bool enabled;
>  	u32 val;
> 
> -	seq_printf(m, "Sink support: %s", str_yes_no(psr->sink_support));
> -	if (psr->sink_support)
> +	seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +		   str_yes_no(psr->sink_support),
> +		   str_yes_no(psr->sink_panel_replay_support));
> +
> +	if (psr->sink_support || psr->sink_panel_replay_support)
>  		seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>  	seq_puts(m, "\n");
> 
> -	if (!psr->sink_support)
> +	if (!(psr->sink_support || psr->sink_panel_replay_support))
>  		return 0;
> 
>  	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>  	mutex_lock(&psr->lock);
> 
> -	if (psr->enabled)
> +	if (psr->panel_replay_enabled)
> +		status = "Panel Replay Enabled";
> +	else if (psr->enabled)
>  		status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1 enabled";
>  	else
>  		status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file *m,
> struct intel_dp *intel_dp)
>  		goto unlock;
>  	}
> 
> -	if (psr->psr2_enabled) {
> +	if (psr->panel_replay_enabled) {
> +		val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> +		enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> +	} else if (psr->psr2_enabled) {
>  		val = intel_de_read(dev_priv, EDP_PSR2_CTL(cpu_transcoder));
>  		enabled = val & EDP_PSR2_ENABLE;
>  	} else {
>  		val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
>  		enabled = val & EDP_PSR_ENABLE;
>  	}
> -	seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> +	seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>  		   str_enabled_disabled(enabled), val);
>  	psr_source_status(intel_dp, m);
>  	seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -3221,6 +3248,7
> @@ static int i915_psr_sink_status_show(struct seq_file *m, void *data)  {
>  	struct intel_connector *connector = m->private;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> +	struct intel_psr *psr = &intel_dp->psr;
>  	static const char * const sink_status[] = {
>  		"inactive",
>  		"transition to active, capture and display", @@ -3231,45
> +3259,82 @@ static int i915_psr_sink_status_show(struct seq_file *m, void
> *data)
>  		"reserved",
>  		"sink internal error",
>  	};
> +	static const char * const panel_replay_status[] = {
> +		"Sink device frame is locked to the Source device",
> +		"Sink device is coasting, using the VTotal target",
> +		"Sink device is governing the frame rate (frame rate unlock is
> granted)",
> +		"Sink device in the process of re-locking with the Source
> device",
> +	};
>  	const char *str;
>  	int ret;
>  	u8 status, error_status;
> 
> -	if (!CAN_PSR(intel_dp)) {
> -		seq_puts(m, "PSR Unsupported\n");
> +	if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> +		seq_puts(m, "PSR/Panel-Replay Unsupported\n");
>  		return -ENODEV;
>  	}
> 
>  	if (connector->base.status != connector_status_connected)
>  		return -ENODEV;
> 
> -	ret = psr_get_status_and_error_status(intel_dp, &status,
> &error_status);
> -	if (ret)
> -		return ret;
> +	if (psr->panel_replay_enabled) {
> +		u32 temp;
> 
> -	status &= DP_PSR_SINK_STATE_MASK;
> -	if (status < ARRAY_SIZE(sink_status))
> -		str = sink_status[status];
> -	else
> -		str = "unknown";
> +		ret = panel_replay_get_status_and_error_status(intel_dp,
> &status, &error_status);
> +		if (ret)
> +			return ret;
> 
> -	seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> +		temp = status & DP_SINK_FRAME_LOCKED_MASK;
> +		temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> +		if (temp < ARRAY_SIZE(panel_replay_status))
> +			str = panel_replay_status[temp];
> +		else
> +			str = "unknown";
> 
> -	seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> +		seq_printf(m, "Sink Panel Replay status: 0x%x [%s]\n", status,
> str);
> 
> -	if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> -			    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> -			    DP_PSR_LINK_CRC_ERROR))
> -		seq_puts(m, ":\n");
> -	else
> -		seq_puts(m, "\n");
> -	if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> -		seq_puts(m, "\tPSR RFB storage error\n");
> -	if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> -		seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> -	if (error_status & DP_PSR_LINK_CRC_ERROR)
> -		seq_puts(m, "\tPSR Link CRC error\n");
> +		seq_printf(m, "Sink Panel Replay error status: 0x%x",
> error_status);
> +
> +		if (error_status & (DP_PANEL_REPLAY_RFB_STORAGE_ERROR |
> +
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> +				    DP_PANEL_REPLAY_LINK_CRC_ERROR))
> +			seq_puts(m, ":\n");
> +		else
> +			seq_puts(m, "\n");
> +		if (error_status & DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY RFB storage error\n");
> +		if (error_status &
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY VSC SDP uncorrectable
> error\n");
> +		if (error_status & DP_PANEL_REPLAY_LINK_CRC_ERROR)
> +			seq_puts(m, "\tPANEL-REPLAY Link CRC error\n");
> +	} else {
> +		ret = psr_get_status_and_error_status(intel_dp, &status,
> &error_status);
> +		if (ret)
> +			return ret;
> +
> +		status &= DP_PSR_SINK_STATE_MASK;
> +		if (status < ARRAY_SIZE(sink_status))
> +			str = sink_status[status];
> +		else
> +			str = "unknown";
> +
> +		seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> +
> +		seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> 
> +		if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> +				    DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR
> |
> +				    DP_PSR_LINK_CRC_ERROR))
> +			seq_puts(m, ":\n");
> +		else
> +			seq_puts(m, "\n");
> +		if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> +			seq_puts(m, "\tPSR RFB storage error\n");
> +		if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> +			seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> +		if (error_status & DP_PSR_LINK_CRC_ERROR)
> +			seq_puts(m, "\tPSR Link CRC error\n");
> +	}
>  	return ret;
>  }
>  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> @@ -3288,13 +3353,16 @@ void intel_psr_connector_debugfs_add(struct
> intel_connector *connector)
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
>  	struct dentry *root = connector->base.debugfs_entry;
> 
> -	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> -		return;
> +	if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> {
> +		if (!(HAS_DP20(i915) &&
> +		      connector->base.connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort))
> +			return;
> +	}
> 
>  	debugfs_create_file("i915_psr_sink_status", 0444, root,
>  			    connector, &i915_psr_sink_status_fops);
> 
> -	if (HAS_PSR(i915))
> +	if (HAS_PSR(i915) || HAS_DP20(i915))
>  		debugfs_create_file("i915_psr_status", 0444, root,
>  				    connector, &i915_psr_status_fops);  }
> --
> 2.29.0

Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Hogander, Jouni]

On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> Add debugfs support which will print source and sink status
> per connector basis.

Sorry for late review. Noticed only by now that you have added this
patch into you set.

Can you please describe in commit message how you see the output of
debugfs interface will look like after your changes?

> 
> v1: Initial version. [rb-ed by Arun]
> v2: Added check for DP 2.0 and connector type in
> connector_debugfs_add().
> 
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 136 +++++++++++++++++----
> --
>  1 file changed, 102 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80de831c2f60..399fc0a8e636 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2823,6 +2823,25 @@ static int
> psr_get_status_and_error_status(struct intel_dp *intel_dp,
>         return 0;
>  }
>  
> +static int panel_replay_get_status_and_error_status(struct intel_dp
> *intel_dp,
> +                                                   u8 *status, u8
> *error_status)
> +{
> +       struct drm_dp_aux *aux = &intel_dp->aux;
> +       int ret;
> +
> +       ret = drm_dp_dpcd_readb(aux,
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> +       if (ret != 1)
> +               return ret;
> +
> +       ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> error_status);
> +       if (ret != 1)
> +               return ret;
> +
> +       *status = *status & DP_PSR_SINK_STATE_MASK;
> +
> +       return 0;
> +}
> +

I think you should modify  psr_get_status_and_error_status instead of
duplicating most of it.

>  static void psr_alpm_check(struct intel_dp *intel_dp)
>  {
>         struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> struct seq_file *m)
>                         status = live_status[status_val];
>         }
>  
> -       seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> val);
> +       seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> status, val);
>  }
>  
>  static int intel_psr_status(struct seq_file *m, struct intel_dp
> *intel_dp)
> @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file
> *m, struct intel_dp *intel_dp)
>         bool enabled;
>         u32 val;
>  
> -       seq_printf(m, "Sink support: %s", str_yes_no(psr-
> >sink_support));
> -       if (psr->sink_support)
> +       seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> +                  str_yes_no(psr->sink_support),
> +                  str_yes_no(psr->sink_panel_replay_support));
> +
> +       if (psr->sink_support || psr->sink_panel_replay_support)
>                 seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
>         seq_puts(m, "\n");
>  
> -       if (!psr->sink_support)
> +       if (!(psr->sink_support || psr->sink_panel_replay_support))
>                 return 0;
>  
>         wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>         mutex_lock(&psr->lock);
>  
> -       if (psr->enabled)
> +       if (psr->panel_replay_enabled)
> +               status = "Panel Replay Enabled";
> +       else if (psr->enabled)
>                 status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> enabled";
>         else
>                 status = "disabled";
> @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file
> *m, struct intel_dp *intel_dp)
>                 goto unlock;
>         }
>  
> -       if (psr->psr2_enabled) {
> +       if (psr->panel_replay_enabled) {
> +               val = intel_de_read(dev_priv,
> TRANS_DP2_CTL(cpu_transcoder));
> +               enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> +       } else if (psr->psr2_enabled) {
>                 val = intel_de_read(dev_priv,
> EDP_PSR2_CTL(cpu_transcoder));
> 
>                 enabled = val & EDP_PSR2_ENABLE;
>         } else {
>                 val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> cpu_transcoder));
>                 enabled = val & EDP_PSR_ENABLE;
>         }
> -       seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> +       seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
>                    str_enabled_disabled(enabled), val);
>         psr_source_status(intel_dp, m);
>         seq_printf(m, "Busy frontbuffer bits: 0x%08x\n",
> @@ -3221,6 +3248,7 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, void *data)
>  {
>         struct intel_connector *connector = m->private;
>         struct intel_dp *intel_dp = intel_attached_dp(connector);
> +       struct intel_psr *psr = &intel_dp->psr;
>         static const char * const sink_status[] = {
>                 "inactive",
>                 "transition to active, capture and display",
> @@ -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct
> seq_file *m, void *data)
>                 "reserved",
>                 "sink internal error",
>         };
> +       static const char * const panel_replay_status[] = {
> +               "Sink device frame is locked to the Source device",
> +               "Sink device is coasting, using the VTotal target",
> +               "Sink device is governing the frame rate (frame rate
> unlock is granted)",
> +               "Sink device in the process of re-locking with the
> Source device",
> +       };
>         const char *str;
>         int ret;
>         u8 status, error_status;
>  
> -       if (!CAN_PSR(intel_dp)) {
> -               seq_puts(m, "PSR Unsupported\n");
> +       if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> +               seq_puts(m, "PSR/Panel-Replay Unsupported\n");
>                 return -ENODEV;
>         }
>  
>         if (connector->base.status != connector_status_connected)
>                 return -ENODEV;
>  
> -       ret = psr_get_status_and_error_status(intel_dp, &status,
> &error_status);
> -       if (ret)
> -               return ret;
> +       if (psr->panel_replay_enabled) {
> +               u32 temp;
>  
> -       status &= DP_PSR_SINK_STATE_MASK;
> -       if (status < ARRAY_SIZE(sink_status))
> -               str = sink_status[status];
> -       else
> -               str = "unknown";
> +               ret =
> panel_replay_get_status_and_error_status(intel_dp, &status,
> &error_status);
> +               if (ret)
> +                       return ret;
>  
> -       seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> +               temp = status & DP_SINK_FRAME_LOCKED_MASK;
> +               temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> +               if (temp < ARRAY_SIZE(panel_replay_status))
> +                       str = panel_replay_status[temp];
> +               else
> +                       str = "unknown";
>  
> -       seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> +               seq_printf(m, "Sink Panel Replay status: 0x%x
> [%s]\n", status, str);
>  
> -       if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> -                           DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> -                           DP_PSR_LINK_CRC_ERROR))
> -               seq_puts(m, ":\n");
> -       else
> -               seq_puts(m, "\n");
> -       if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> -               seq_puts(m, "\tPSR RFB storage error\n");
> -       if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> -               seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> -       if (error_status & DP_PSR_LINK_CRC_ERROR)
> -               seq_puts(m, "\tPSR Link CRC error\n");
> +               seq_printf(m, "Sink Panel Replay error status: 0x%x",
> error_status);
> +
> +               if (error_status & (DP_PANEL_REPLAY_RFB_STORAGE_ERROR
> |
> +                                  
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> +                                   DP_PANEL_REPLAY_LINK_CRC_ERROR))
> +                       seq_puts(m, ":\n");
> +               else
> +                       seq_puts(m, "\n");
> +               if (error_status & DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> +                       seq_puts(m, "\tPANEL-REPLAY RFB storage
> error\n");
> +               if (error_status &
> DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> +                       seq_puts(m, "\tPANEL-REPLAY VSC SDP
> uncorrectable error\n");
> +               if (error_status & DP_PANEL_REPLAY_LINK_CRC_ERROR)
> +                       seq_puts(m, "\tPANEL-REPLAY Link CRC
> error\n");
> +       } else {
> +               ret = psr_get_status_and_error_status(intel_dp,
> &status, &error_status);
> +               if (ret)
> +                       return ret;
> +
> +               status &= DP_PSR_SINK_STATE_MASK;
> +               if (status < ARRAY_SIZE(sink_status))
> +                       str = sink_status[status];
> +               else
> +                       str = "unknown";
> +
> +               seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status,
> str);
> +
> +               seq_printf(m, "Sink PSR error status: 0x%x",
> error_status);
>  
> +               if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> +                                  
> DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> +                                   DP_PSR_LINK_CRC_ERROR))
> +                       seq_puts(m, ":\n");
> +               else
> +                       seq_puts(m, "\n");
> +               if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> +                       seq_puts(m, "\tPSR RFB storage error\n");
> +               if (error_status &
> DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> +                       seq_puts(m, "\tPSR VSC SDP uncorrectable
> error\n");
> +               if (error_status & DP_PSR_LINK_CRC_ERROR)
> +                       seq_puts(m, "\tPSR Link CRC error\n");
> +       }

Instead of duplicating so much here I think it woukd be ok to just drop
PSR and do (same applies for all the statuses):


if (error_status & (DP_PSR_LINK_CRC_ERROR | 
DP_PANEL_REPLAY_LINK_CRC_ERROR))
    seq_puts(m, "\tLink CRC error\n");

What do you think?


>         return ret;
>  }
>  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> @@ -3288,13 +3353,16 @@ void intel_psr_connector_debugfs_add(struct
> intel_connector *connector)
>         struct drm_i915_private *i915 = to_i915(connector->base.dev);
>         struct dentry *root = connector->base.debugfs_entry;
>  
> -       if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> -               return;
> +       if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP)
> {
> +               if (!(HAS_DP20(i915) &&
> +                     connector->base.connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort))
> +                       return;
> +       }

Why do you need to check DRM_MODE_CONNECTOR_DisplayPort ? I think
connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
!HAS_DP20(i915) is enough.

BR,

Jouni Högander
>  
>         debugfs_create_file("i915_psr_sink_status", 0444, root,
>                             connector, &i915_psr_sink_status_fops);
>  
> -       if (HAS_PSR(i915))
> +       if (HAS_PSR(i915) || HAS_DP20(i915))
>                 debugfs_create_file("i915_psr_status", 0444, root,
>                                     connector,
> &i915_psr_status_fops);
>  }

RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Manna, Animesh]

> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Thursday, November 2, 2023 1:08 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
> 
> On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > Add debugfs support which will print source and sink status per
> > connector basis.
> 
> Sorry for late review. Noticed only by now that you have added this patch
> into you set.

Added from v5.

> 
> Can you please describe in commit message how you see the output of
> debugfs interface will look like after your changes?

Sure.

> 
> >
> > v1: Initial version. [rb-ed by Arun]
> > v2: Added check for DP 2.0 and connector type in
> > connector_debugfs_add().
> >
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 136 +++++++++++++++++----
> > --
> >  1 file changed, 102 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 80de831c2f60..399fc0a8e636 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2823,6 +2823,25 @@ static int
> > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> >         return 0;
> >  }
> >
> > +static int panel_replay_get_status_and_error_status(struct intel_dp
> > *intel_dp,
> > +                                                   u8 *status, u8
> > *error_status)
> > +{
> > +       struct drm_dp_aux *aux = &intel_dp->aux;
> > +       int ret;
> > +
> > +       ret = drm_dp_dpcd_readb(aux,
> > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > +       if (ret != 1)
> > +               return ret;
> > +
> > +       ret = drm_dp_dpcd_readb(aux, DP_PANEL_REPLAY_ERROR_STATUS,
> > error_status);
> > +       if (ret != 1)
> > +               return ret;
> > +
> > +       *status = *status & DP_PSR_SINK_STATE_MASK;
> > +
> > +       return 0;
> > +}
> > +
> 
> I think you should modify  psr_get_status_and_error_status instead of
> duplicating most of it.

DPCD addresses are different for panel replay, I did not get the need of it. 
 
> 
> >  static void psr_alpm_check(struct intel_dp *intel_dp)
> >  {
> >         struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@
> > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp, struct
> > seq_file *m)
> >                         status = live_status[status_val];
> >         }
> >
> > -       seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > val);
> > +       seq_printf(m, "Source PSR/PanelReplay status: %s [0x%08x]\n",
> > status, val);
> >  }
> >
> >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > *intel_dp)
> > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> >         bool enabled;
> >         u32 val;
> >
> > -       seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > >sink_support));
> > -       if (psr->sink_support)
> > +       seq_printf(m, "Sink support: PSR = %s, Panel Replay = %s",
> > +                  str_yes_no(psr->sink_support),
> > +                  str_yes_no(psr->sink_panel_replay_support));
> > +
> > +       if (psr->sink_support || psr->sink_panel_replay_support)
> >                 seq_printf(m, " [0x%02x]", intel_dp->psr_dpcd[0]);
> >         seq_puts(m, "\n");
> >
> > -       if (!psr->sink_support)
> > +       if (!(psr->sink_support || psr->sink_panel_replay_support))
> >                 return 0;
> >
> >         wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> >         mutex_lock(&psr->lock);
> >
> > -       if (psr->enabled)
> > +       if (psr->panel_replay_enabled)
> > +               status = "Panel Replay Enabled";
> > +       else if (psr->enabled)
> >                 status = psr->psr2_enabled ? "PSR2 enabled" : "PSR1
> > enabled";
> >         else
> >                 status = "disabled";
> > @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct seq_file
> > *m, struct intel_dp *intel_dp)
> >                 goto unlock;
> >         }
> >
> > -       if (psr->psr2_enabled) {
> > +       if (psr->panel_replay_enabled) {
> > +               val = intel_de_read(dev_priv,
> > TRANS_DP2_CTL(cpu_transcoder));
> > +               enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> > +       } else if (psr->psr2_enabled) {
> >                 val = intel_de_read(dev_priv,
> > EDP_PSR2_CTL(cpu_transcoder));
> >
> >                 enabled = val & EDP_PSR2_ENABLE;
> >         } else {
> >                 val = intel_de_read(dev_priv, psr_ctl_reg(dev_priv,
> > cpu_transcoder));
> >                 enabled = val & EDP_PSR_ENABLE;
> >         }
> > -       seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> > +       seq_printf(m, "Source PSR/PanelReplay ctl: %s [0x%08x]\n",
> >                    str_enabled_disabled(enabled), val);
> >         psr_source_status(intel_dp, m);
> >         seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -3221,6
> > +3248,7 @@ static int i915_psr_sink_status_show(struct seq_file *m,
> > void *data)
> >  {
> >         struct intel_connector *connector = m->private;
> >         struct intel_dp *intel_dp = intel_attached_dp(connector);
> > +       struct intel_psr *psr = &intel_dp->psr;
> >         static const char * const sink_status[] = {
> >                 "inactive",
> >                 "transition to active, capture and display", @@
> > -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct
> > seq_file *m, void *data)
> >                 "reserved",
> >                 "sink internal error",
> >         };
> > +       static const char * const panel_replay_status[] = {
> > +               "Sink device frame is locked to the Source device",
> > +               "Sink device is coasting, using the VTotal target",
> > +               "Sink device is governing the frame rate (frame rate
> > unlock is granted)",
> > +               "Sink device in the process of re-locking with the
> > Source device",
> > +       };
> >         const char *str;
> >         int ret;
> >         u8 status, error_status;
> >
> > -       if (!CAN_PSR(intel_dp)) {
> > -               seq_puts(m, "PSR Unsupported\n");
> > +       if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> > +               seq_puts(m, "PSR/Panel-Replay Unsupported\n");
> >                 return -ENODEV;
> >         }
> >
> >         if (connector->base.status != connector_status_connected)
> >                 return -ENODEV;
> >
> > -       ret = psr_get_status_and_error_status(intel_dp, &status,
> > &error_status);
> > -       if (ret)
> > -               return ret;
> > +       if (psr->panel_replay_enabled) {
> > +               u32 temp;
> >
> > -       status &= DP_PSR_SINK_STATE_MASK;
> > -       if (status < ARRAY_SIZE(sink_status))
> > -               str = sink_status[status];
> > -       else
> > -               str = "unknown";
> > +               ret =
> > panel_replay_get_status_and_error_status(intel_dp, &status,
> > &error_status);
> > +               if (ret)
> > +                       return ret;
> >
> > -       seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status, str);
> > +               temp = status & DP_SINK_FRAME_LOCKED_MASK;
> > +               temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> > +               if (temp < ARRAY_SIZE(panel_replay_status))
> > +                       str = panel_replay_status[temp];
> > +               else
> > +                       str = "unknown";
> >
> > -       seq_printf(m, "Sink PSR error status: 0x%x", error_status);
> > +               seq_printf(m, "Sink Panel Replay status: 0x%x
> > [%s]\n", status, str);
> >
> > -       if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > -                           DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > -                           DP_PSR_LINK_CRC_ERROR))
> > -               seq_puts(m, ":\n");
> > -       else
> > -               seq_puts(m, "\n");
> > -       if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > -               seq_puts(m, "\tPSR RFB storage error\n");
> > -       if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > -               seq_puts(m, "\tPSR VSC SDP uncorrectable error\n");
> > -       if (error_status & DP_PSR_LINK_CRC_ERROR)
> > -               seq_puts(m, "\tPSR Link CRC error\n");
> > +               seq_printf(m, "Sink Panel Replay error status: 0x%x",
> > error_status);
> > +
> > +               if (error_status & (DP_PANEL_REPLAY_RFB_STORAGE_ERROR
> > |
> > +
> > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> > +                                   DP_PANEL_REPLAY_LINK_CRC_ERROR))
> > +                       seq_puts(m, ":\n");
> > +               else
> > +                       seq_puts(m, "\n");
> > +               if (error_status & DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> > +                       seq_puts(m, "\tPANEL-REPLAY RFB storage
> > error\n");
> > +               if (error_status &
> > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> > +                       seq_puts(m, "\tPANEL-REPLAY VSC SDP
> > uncorrectable error\n");
> > +               if (error_status & DP_PANEL_REPLAY_LINK_CRC_ERROR)
> > +                       seq_puts(m, "\tPANEL-REPLAY Link CRC
> > error\n");
> > +       } else {
> > +               ret = psr_get_status_and_error_status(intel_dp,
> > &status, &error_status);
> > +               if (ret)
> > +                       return ret;
> > +
> > +               status &= DP_PSR_SINK_STATE_MASK;
> > +               if (status < ARRAY_SIZE(sink_status))
> > +                       str = sink_status[status];
> > +               else
> > +                       str = "unknown";
> > +
> > +               seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status,
> > str);
> > +
> > +               seq_printf(m, "Sink PSR error status: 0x%x",
> > error_status);
> >
> > +               if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > +
> > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > +                                   DP_PSR_LINK_CRC_ERROR))
> > +                       seq_puts(m, ":\n");
> > +               else
> > +                       seq_puts(m, "\n");
> > +               if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > +                       seq_puts(m, "\tPSR RFB storage error\n");
> > +               if (error_status &
> > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > +                       seq_puts(m, "\tPSR VSC SDP uncorrectable
> > error\n");
> > +               if (error_status & DP_PSR_LINK_CRC_ERROR)
> > +                       seq_puts(m, "\tPSR Link CRC error\n");
> > +       }
> 
> Instead of duplicating so much here I think it woukd be ok to just drop PSR
> and do (same applies for all the statuses):
> 
> 
> if (error_status & (DP_PSR_LINK_CRC_ERROR |
> DP_PANEL_REPLAY_LINK_CRC_ERROR))
>     seq_puts(m, "\tLink CRC error\n");
> 
> What do you think?

Thinking of backward compatibility, I have added separately for panel replay.
I feel good to have separate cleanup patch for psr and panel-replay together, not part of this patch.

> 
> 
> >         return ret;
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > @@ -3288,13 +3353,16 @@ void intel_psr_connector_debugfs_add(struct
> > intel_connector *connector)
> >         struct drm_i915_private *i915 = to_i915(connector->base.dev);
> >         struct dentry *root = connector->base.debugfs_entry;
> >
> > -       if (connector->base.connector_type !=
> DRM_MODE_CONNECTOR_eDP)
> > -               return;
> > +       if (connector->base.connector_type !=
> DRM_MODE_CONNECTOR_eDP)
> > {
> > +               if (!(HAS_DP20(i915) &&
> > +                     connector->base.connector_type ==
> > DRM_MODE_CONNECTOR_DisplayPort))
> > +                       return;
> > +       }
> 
> Why do you need to check DRM_MODE_CONNECTOR_DisplayPort ? I think
> connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
> !HAS_DP20(i915) is enough.

As per your suggestion, the code will look like below:

        if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP && !HAS_DP20(i915))
                return;

For example in case of hdmi connector type in MTL still we go ahead and create debugfs entry... rt? Please let me if I am missing anything.

Regards,
Animesh
 
> 
> BR,
> 
> Jouni Högander
> >
> >         debugfs_create_file("i915_psr_sink_status", 0444, root,
> >                             connector, &i915_psr_sink_status_fops);
> >
> > -       if (HAS_PSR(i915))
> > +       if (HAS_PSR(i915) || HAS_DP20(i915))
> >                 debugfs_create_file("i915_psr_status", 0444, root,
> >                                     connector, &i915_psr_status_fops);
> >  }

Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Hogander, Jouni]

On Fri, 2023-11-03 at 06:10 +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Hogander, Jouni <jouni.hogander@intel.com>
> > Sent: Thursday, November 2, 2023 1:08 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> > Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support
> > for
> > panel replay
> > 
> > On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > > Add debugfs support which will print source and sink status per
> > > connector basis.
> > 
> > Sorry for late review. Noticed only by now that you have added this
> > patch
> > into you set.
> 
> Added from v5.
> 
> > 
> > Can you please describe in commit message how you see the output of
> > debugfs interface will look like after your changes?
> 
> Sure.
> 
> > 
> > > 
> > > v1: Initial version. [rb-ed by Arun]
> > > v2: Added check for DP 2.0 and connector type in
> > > connector_debugfs_add().
> > > 
> > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 136
> > > +++++++++++++++++----
> > > --
> > >  1 file changed, 102 insertions(+), 34 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 80de831c2f60..399fc0a8e636 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -2823,6 +2823,25 @@ static int
> > > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > >         return 0;
> > >  }
> > > 
> > > +static int panel_replay_get_status_and_error_status(struct
> > > intel_dp
> > > *intel_dp,
> > > +                                                   u8 *status,
> > > u8
> > > *error_status)
> > > +{
> > > +       struct drm_dp_aux *aux = &intel_dp->aux;
> > > +       int ret;
> > > +
> > > +       ret = drm_dp_dpcd_readb(aux,
> > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > > +       if (ret != 1)
> > > +               return ret;
> > > +
> > > +       ret = drm_dp_dpcd_readb(aux,
> > > DP_PANEL_REPLAY_ERROR_STATUS,
> > > error_status);
> > > +       if (ret != 1)
> > > +               return ret;
> > > +
> > > +       *status = *status & DP_PSR_SINK_STATE_MASK;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > 
> > I think you should modify  psr_get_status_and_error_status instead
> > of
> > duplicating most of it.
> 
> DPCD addresses are different for panel replay, I did not get the need
> of it. 

I would like to see:

unsigned int offset = intel_dp->psr.panel_replay_enabled ?
DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS : DP_PSR_STATUS;

ret = drm_dp_dpcd_readb(aux, offset, status);

rather than duplicating it.

>  
> > 
> > >  static void psr_alpm_check(struct intel_dp *intel_dp)
> > >  {
> > >         struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > @@
> > > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > > struct
> > > seq_file *m)
> > >                         status = live_status[status_val];
> > >         }
> > > 
> > > -       seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > > val);
> > > +       seq_printf(m, "Source PSR/PanelReplay status: %s
> > > [0x%08x]\n",
> > > status, val);
> > >  }
> > > 
> > >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > > *intel_dp)
> > > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct
> > > seq_file
> > > *m, struct intel_dp *intel_dp)
> > >         bool enabled;
> > >         u32 val;
> > > 
> > > -       seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > > > sink_support));
> > > -       if (psr->sink_support)
> > > +       seq_printf(m, "Sink support: PSR = %s, Panel Replay =
> > > %s",
> > > +                  str_yes_no(psr->sink_support),
> > > +                  str_yes_no(psr->sink_panel_replay_support));
> > > +
> > > +       if (psr->sink_support || psr->sink_panel_replay_support)
> > >                 seq_printf(m, " [0x%02x]", intel_dp-
> > > >psr_dpcd[0]);
> > >         seq_puts(m, "\n");
> > > 
> > > -       if (!psr->sink_support)
> > > +       if (!(psr->sink_support || psr-
> > > >sink_panel_replay_support))
> > >                 return 0;
> > > 
> > >         wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> > >         mutex_lock(&psr->lock);
> > > 
> > > -       if (psr->enabled)
> > > +       if (psr->panel_replay_enabled)
> > > +               status = "Panel Replay Enabled";
> > > +       else if (psr->enabled)
> > >                 status = psr->psr2_enabled ? "PSR2 enabled" :
> > > "PSR1
> > > enabled";
> > >         else
> > >                 status = "disabled";
> > > @@ -3072,14 +3096,17 @@ static int intel_psr_status(struct
> > > seq_file
> > > *m, struct intel_dp *intel_dp)
> > >                 goto unlock;
> > >         }
> > > 
> > > -       if (psr->psr2_enabled) {
> > > +       if (psr->panel_replay_enabled) {
> > > +               val = intel_de_read(dev_priv,
> > > TRANS_DP2_CTL(cpu_transcoder));
> > > +               enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> > > +       } else if (psr->psr2_enabled) {
> > >                 val = intel_de_read(dev_priv,
> > > EDP_PSR2_CTL(cpu_transcoder));
> > > 
> > >                 enabled = val & EDP_PSR2_ENABLE;
> > >         } else {
> > >                 val = intel_de_read(dev_priv,
> > > psr_ctl_reg(dev_priv,
> > > cpu_transcoder));
> > >                 enabled = val & EDP_PSR_ENABLE;
> > >         }
> > > -       seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> > > +       seq_printf(m, "Source PSR/PanelReplay ctl: %s
> > > [0x%08x]\n",
> > >                    str_enabled_disabled(enabled), val);
> > >         psr_source_status(intel_dp, m);
> > >         seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -
> > > 3221,6
> > > +3248,7 @@ static int i915_psr_sink_status_show(struct seq_file
> > > *m,
> > > void *data)
> > >  {
> > >         struct intel_connector *connector = m->private;
> > >         struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > +       struct intel_psr *psr = &intel_dp->psr;
> > >         static const char * const sink_status[] = {
> > >                 "inactive",
> > >                 "transition to active, capture and display", @@
> > > -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct
> > > seq_file *m, void *data)
> > >                 "reserved",
> > >                 "sink internal error",
> > >         };
> > > +       static const char * const panel_replay_status[] = {
> > > +               "Sink device frame is locked to the Source
> > > device",
> > > +               "Sink device is coasting, using the VTotal
> > > target",
> > > +               "Sink device is governing the frame rate (frame
> > > rate
> > > unlock is granted)",
> > > +               "Sink device in the process of re-locking with
> > > the
> > > Source device",
> > > +       };
> > >         const char *str;
> > >         int ret;
> > >         u8 status, error_status;
> > > 
> > > -       if (!CAN_PSR(intel_dp)) {
> > > -               seq_puts(m, "PSR Unsupported\n");
> > > +       if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> > > +               seq_puts(m, "PSR/Panel-Replay Unsupported\n");
> > >                 return -ENODEV;
> > >         }
> > > 
> > >         if (connector->base.status != connector_status_connected)
> > >                 return -ENODEV;
> > > 
> > > -       ret = psr_get_status_and_error_status(intel_dp, &status,
> > > &error_status);
> > > -       if (ret)
> > > -               return ret;
> > > +       if (psr->panel_replay_enabled) {
> > > +               u32 temp;
> > > 
> > > -       status &= DP_PSR_SINK_STATE_MASK;
> > > -       if (status < ARRAY_SIZE(sink_status))
> > > -               str = sink_status[status];
> > > -       else
> > > -               str = "unknown";
> > > +               ret =
> > > panel_replay_get_status_and_error_status(intel_dp, &status,
> > > &error_status);
> > > +               if (ret)
> > > +                       return ret;
> > > 
> > > -       seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status,
> > > str);
> > > +               temp = status & DP_SINK_FRAME_LOCKED_MASK;
> > > +               temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> > > +               if (temp < ARRAY_SIZE(panel_replay_status))
> > > +                       str = panel_replay_status[temp];
> > > +               else
> > > +                       str = "unknown";
> > > 
> > > -       seq_printf(m, "Sink PSR error status: 0x%x",
> > > error_status);
> > > +               seq_printf(m, "Sink Panel Replay status: 0x%x
> > > [%s]\n", status, str);
> > > 
> > > -       if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > > -                           DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > > -                           DP_PSR_LINK_CRC_ERROR))
> > > -               seq_puts(m, ":\n");
> > > -       else
> > > -               seq_puts(m, "\n");
> > > -       if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > > -               seq_puts(m, "\tPSR RFB storage error\n");
> > > -       if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > > -               seq_puts(m, "\tPSR VSC SDP uncorrectable
> > > error\n");
> > > -       if (error_status & DP_PSR_LINK_CRC_ERROR)
> > > -               seq_puts(m, "\tPSR Link CRC error\n");
> > > +               seq_printf(m, "Sink Panel Replay error status:
> > > 0x%x",
> > > error_status);
> > > +
> > > +               if (error_status &
> > > (DP_PANEL_REPLAY_RFB_STORAGE_ERROR
> > > > 
> > > +
> > > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> > > +                                  
> > > DP_PANEL_REPLAY_LINK_CRC_ERROR))
> > > +                       seq_puts(m, ":\n");
> > > +               else
> > > +                       seq_puts(m, "\n");
> > > +               if (error_status &
> > > DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> > > +                       seq_puts(m, "\tPANEL-REPLAY RFB storage
> > > error\n");
> > > +               if (error_status &
> > > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> > > +                       seq_puts(m, "\tPANEL-REPLAY VSC SDP
> > > uncorrectable error\n");
> > > +               if (error_status &
> > > DP_PANEL_REPLAY_LINK_CRC_ERROR)
> > > +                       seq_puts(m, "\tPANEL-REPLAY Link CRC
> > > error\n");
> > > +       } else {
> > > +               ret = psr_get_status_and_error_status(intel_dp,
> > > &status, &error_status);
> > > +               if (ret)
> > > +                       return ret;
> > > +
> > > +               status &= DP_PSR_SINK_STATE_MASK;
> > > +               if (status < ARRAY_SIZE(sink_status))
> > > +                       str = sink_status[status];
> > > +               else
> > > +                       str = "unknown";
> > > +
> > > +               seq_printf(m, "Sink PSR status: 0x%x [%s]\n",
> > > status,
> > > str);
> > > +
> > > +               seq_printf(m, "Sink PSR error status: 0x%x",
> > > error_status);
> > > 
> > > +               if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > > +
> > > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > > +                                   DP_PSR_LINK_CRC_ERROR))
> > > +                       seq_puts(m, ":\n");
> > > +               else
> > > +                       seq_puts(m, "\n");
> > > +               if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > > +                       seq_puts(m, "\tPSR RFB storage error\n");
> > > +               if (error_status &
> > > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > > +                       seq_puts(m, "\tPSR VSC SDP uncorrectable
> > > error\n");
> > > +               if (error_status & DP_PSR_LINK_CRC_ERROR)
> > > +                       seq_puts(m, "\tPSR Link CRC error\n");
> > > +       }
> > 
> > Instead of duplicating so much here I think it woukd be ok to just
> > drop PSR
> > and do (same applies for all the statuses):
> > 
> > 
> > if (error_status & (DP_PSR_LINK_CRC_ERROR |
> > DP_PANEL_REPLAY_LINK_CRC_ERROR))
> >     seq_puts(m, "\tLink CRC error\n");
> > 
> > What do you think?
> 
> Thinking of backward compatibility, I have added separately for panel
> replay.
> I feel good to have separate cleanup patch for psr and panel-replay
> together, not part of this patch.

If you want to keep the format as it is, then you should solve it by
changing just the mode in printout rather than duplicating everything.
One way:

static const char *psr_mode_str(struct intel_dp *intel_dp)
{
	if (intel_dp->psr.panel_replay_enabled)
		return "PANEL-REPLAY";
	else if(intel_dp->psr.psr_enabled)
		return "PSR";

	return "unknown";
}
...
seq_puts(m, "\t%s RFB storage error\n", psr_mode_str(intel_dp));
...

> 
> > 
> > 
> > >         return ret;
> > >  }
> > >  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > @@ -3288,13 +3353,16 @@ void
> > > intel_psr_connector_debugfs_add(struct
> > > intel_connector *connector)
> > >         struct drm_i915_private *i915 = to_i915(connector-
> > > >base.dev);
> > >         struct dentry *root = connector->base.debugfs_entry;
> > > 
> > > -       if (connector->base.connector_type !=
> > DRM_MODE_CONNECTOR_eDP)
> > > -               return;
> > > +       if (connector->base.connector_type !=
> > DRM_MODE_CONNECTOR_eDP)
> > > {
> > > +               if (!(HAS_DP20(i915) &&
> > > +                     connector->base.connector_type ==
> > > DRM_MODE_CONNECTOR_DisplayPort))
> > > +                       return;
> > > +       }
> > 
> > Why do you need to check DRM_MODE_CONNECTOR_DisplayPort ? I think
> > connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
> > !HAS_DP20(i915) is enough.
> 
> As per your suggestion, the code will look like below:
> 
>         if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP
> && !HAS_DP20(i915))
>                 return;
> 
> For example in case of hdmi connector type in MTL still we go ahead
> and create debugfs entry... rt? Please let me if I am missing
> anything.

Ok, I got this now.

BR,

Jouni Högander
> 
> Regards,
> Animesh
>  
> > 
> > BR,
> > 
> > Jouni Högander
> > > 
> > >         debugfs_create_file("i915_psr_sink_status", 0444, root,
> > >                             connector,
> > > &i915_psr_sink_status_fops);
> > > 
> > > -       if (HAS_PSR(i915))
> > > +       if (HAS_PSR(i915) || HAS_DP20(i915))
> > >                 debugfs_create_file("i915_psr_status", 0444,
> > > root,
> > >                                     connector,
> > > &i915_psr_status_fops);
> > >  }
> 

Re: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for panelreplay

[Jani Nikula]

On Wed, 11 Oct 2023, Animesh Manna <animesh.manna@intel.com> wrote:
> Add DPCD register definition for discovering, enabling and
> checking status of panel replay of the sink.
>
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Arun R Murthy <arun.r.murthy@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>

Maarten, Maxime, Thomas -

Ack for merging this via drm-intel-next?

Thanks,
Jani.

> ---
>  include/drm/display/drm_dp.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index e69cece404b3..fc42b622ef32 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -543,6 +543,10 @@
>  /* DFP Capability Extension */
>  #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT	0x0a3	/* 2.0 */
>  
> +#define DP_PANEL_REPLAY_CAP                 0x0b0  /* DP 2.0 */
> +# define DP_PANEL_REPLAY_SUPPORT            (1 << 0)
> +# define DP_PANEL_REPLAY_SU_SUPPORT         (1 << 1)
> +
>  /* Link Configuration */
>  #define	DP_LINK_BW_SET		            0x100
>  # define DP_LINK_RATE_TABLE		    0x00    /* eDP 1.4 */
> @@ -716,6 +720,13 @@
>  #define DP_BRANCH_DEVICE_CTRL		    0x1a1
>  # define DP_BRANCH_DEVICE_IRQ_HPD	    (1 << 0)
>  
> +#define PANEL_REPLAY_CONFIG                             0x1b0  /* DP 2.0 */
> +# define DP_PANEL_REPLAY_ENABLE                         (1 << 0)
> +# define DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN         (1 << 3)
> +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN           (1 << 4)
> +# define DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN      (1 << 5)
> +# define DP_PANEL_REPLAY_SU_ENABLE                      (1 << 6)
> +
>  #define DP_PAYLOAD_ALLOCATE_SET		    0x1c0
>  #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1
>  #define DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2
> @@ -1105,6 +1116,18 @@
>  #define DP_LANE_ALIGN_STATUS_UPDATED_ESI       0x200e /* status same as 0x204 */
>  #define DP_SINK_STATUS_ESI                     0x200f /* status same as 0x205 */
>  
> +#define DP_PANEL_REPLAY_ERROR_STATUS                   0x2020  /* DP 2.1*/
> +# define DP_PANEL_REPLAY_LINK_CRC_ERROR                (1 << 0)
> +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR             (1 << 1)
> +# define DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR   (1 << 2)
> +
> +#define DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS        0x2022  /* DP 2.1 */
> +# define DP_SINK_DEVICE_PANEL_REPLAY_STATUS_MASK       (7 << 0)
> +# define DP_SINK_FRAME_LOCKED_SHIFT                    3
> +# define DP_SINK_FRAME_LOCKED_MASK                     (3 << 3)
> +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_SHIFT       5
> +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_MASK        (1 << 5)
> +
>  /* Extended Receiver Capability: See DP_DPCD_REV for definitions */
>  #define DP_DP13_DPCD_REV                    0x2200

-- 
Jani Nikula, Intel

RE: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for panel replay

[Manna, Animesh]

> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander@intel.com>
> Sent: Friday, November 3, 2023 12:32 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support for
> panel replay
> 
> On Fri, 2023-11-03 at 06:10 +0000, Manna, Animesh wrote:
> >
> >
> > > -----Original Message-----
> > > From: Hogander, Jouni <jouni.hogander@intel.com>
> > > Sent: Thursday, November 2, 2023 1:08 PM
> > > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: dri-devel@lists.freedesktop.org; Murthy, Arun R
> > > <arun.r.murthy@intel.com>; Nikula, Jani <jani.nikula@intel.com>
> > > Subject: Re: [PATCH v7 6/6] drm/i915/panelreplay: Debugfs support
> > > for panel replay
> > >
> > > On Wed, 2023-10-11 at 16:39 +0530, Animesh Manna wrote:
> > > > Add debugfs support which will print source and sink status per
> > > > connector basis.
> > >
> > > Sorry for late review. Noticed only by now that you have added this
> > > patch into you set.
> >
> > Added from v5.
> >
> > >
> > > Can you please describe in commit message how you see the output of
> > > debugfs interface will look like after your changes?
> >
> > Sure.
> >
> > >
> > > >
> > > > v1: Initial version. [rb-ed by Arun]
> > > > v2: Added check for DP 2.0 and connector type in
> > > > connector_debugfs_add().
> > > >
> > > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/display/intel_psr.c | 136
> > > > +++++++++++++++++----
> > > > --
> > > >  1 file changed, 102 insertions(+), 34 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 80de831c2f60..399fc0a8e636 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -2823,6 +2823,25 @@ static int
> > > > psr_get_status_and_error_status(struct intel_dp *intel_dp,
> > > >         return 0;
> > > >  }
> > > >
> > > > +static int panel_replay_get_status_and_error_status(struct
> > > > intel_dp
> > > > *intel_dp,
> > > > +                                                   u8 *status,
> > > > u8
> > > > *error_status)
> > > > +{
> > > > +       struct drm_dp_aux *aux = &intel_dp->aux;
> > > > +       int ret;
> > > > +
> > > > +       ret = drm_dp_dpcd_readb(aux,
> > > > DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS, status);
> > > > +       if (ret != 1)
> > > > +               return ret;
> > > > +
> > > > +       ret = drm_dp_dpcd_readb(aux,
> > > > DP_PANEL_REPLAY_ERROR_STATUS,
> > > > error_status);
> > > > +       if (ret != 1)
> > > > +               return ret;
> > > > +
> > > > +       *status = *status & DP_PSR_SINK_STATE_MASK;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > >
> > > I think you should modify  psr_get_status_and_error_status instead
> > > of duplicating most of it.
> >
> > DPCD addresses are different for panel replay, I did not get the need
> > of it.
> 
> I would like to see:
> 
> unsigned int offset = intel_dp->psr.panel_replay_enabled ?
> DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS : DP_PSR_STATUS;
> 
> ret = drm_dp_dpcd_readb(aux, offset, status);
> 
> rather than duplicating it.

Added in v8.

> 
> >
> > >
> > > >  static void psr_alpm_check(struct intel_dp *intel_dp)
> > > >  {
> > > >         struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > > @@
> > > > -3035,7 +3054,7 @@ psr_source_status(struct intel_dp *intel_dp,
> > > > struct seq_file *m)
> > > >                         status = live_status[status_val];
> > > >         }
> > > >
> > > > -       seq_printf(m, "Source PSR status: %s [0x%08x]\n", status,
> > > > val);
> > > > +       seq_printf(m, "Source PSR/PanelReplay status: %s
> > > > [0x%08x]\n",
> > > > status, val);
> > > >  }
> > > >
> > > >  static int intel_psr_status(struct seq_file *m, struct intel_dp
> > > > *intel_dp)
> > > > @@ -3048,18 +3067,23 @@ static int intel_psr_status(struct
> > > > seq_file *m, struct intel_dp *intel_dp)
> > > >         bool enabled;
> > > >         u32 val;
> > > >
> > > > -       seq_printf(m, "Sink support: %s", str_yes_no(psr-
> > > > > sink_support));
> > > > -       if (psr->sink_support)
> > > > +       seq_printf(m, "Sink support: PSR = %s, Panel Replay =
> > > > %s",
> > > > +                  str_yes_no(psr->sink_support),
> > > > +                  str_yes_no(psr->sink_panel_replay_support));
> > > > +
> > > > +       if (psr->sink_support || psr->sink_panel_replay_support)
> > > >                 seq_printf(m, " [0x%02x]", intel_dp-
> > > > >psr_dpcd[0]);
> > > >         seq_puts(m, "\n");
> > > >
> > > > -       if (!psr->sink_support)
> > > > +       if (!(psr->sink_support || psr-
> > > > >sink_panel_replay_support))
> > > >                 return 0;
> > > >
> > > >         wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
> > > >         mutex_lock(&psr->lock);
> > > >
> > > > -       if (psr->enabled)
> > > > +       if (psr->panel_replay_enabled)
> > > > +               status = "Panel Replay Enabled";
> > > > +       else if (psr->enabled)
> > > >                 status = psr->psr2_enabled ? "PSR2 enabled" :
> > > > "PSR1
> > > > enabled";
> > > >         else
> > > >                 status = "disabled"; @@ -3072,14 +3096,17 @@
> > > > static int intel_psr_status(struct seq_file *m, struct intel_dp
> > > > *intel_dp)
> > > >                 goto unlock;
> > > >         }
> > > >
> > > > -       if (psr->psr2_enabled) {
> > > > +       if (psr->panel_replay_enabled) {
> > > > +               val = intel_de_read(dev_priv,
> > > > TRANS_DP2_CTL(cpu_transcoder));
> > > > +               enabled = val & TRANS_DP2_PANEL_REPLAY_ENABLE;
> > > > +       } else if (psr->psr2_enabled) {
> > > >                 val = intel_de_read(dev_priv,
> > > > EDP_PSR2_CTL(cpu_transcoder));
> > > >
> > > >                 enabled = val & EDP_PSR2_ENABLE;
> > > >         } else {
> > > >                 val = intel_de_read(dev_priv,
> > > > psr_ctl_reg(dev_priv, cpu_transcoder));
> > > >                 enabled = val & EDP_PSR_ENABLE;
> > > >         }
> > > > -       seq_printf(m, "Source PSR ctl: %s [0x%08x]\n",
> > > > +       seq_printf(m, "Source PSR/PanelReplay ctl: %s
> > > > [0x%08x]\n",
> > > >                    str_enabled_disabled(enabled), val);
> > > >         psr_source_status(intel_dp, m);
> > > >         seq_printf(m, "Busy frontbuffer bits: 0x%08x\n", @@ -
> > > > 3221,6
> > > > +3248,7 @@ static int i915_psr_sink_status_show(struct seq_file
> > > > *m,
> > > > void *data)
> > > >  {
> > > >         struct intel_connector *connector = m->private;
> > > >         struct intel_dp *intel_dp = intel_attached_dp(connector);
> > > > +       struct intel_psr *psr = &intel_dp->psr;
> > > >         static const char * const sink_status[] = {
> > > >                 "inactive",
> > > >                 "transition to active, capture and display", @@
> > > > -3231,45 +3259,82 @@ static int i915_psr_sink_status_show(struct
> > > > seq_file *m, void *data)
> > > >                 "reserved",
> > > >                 "sink internal error",
> > > >         };
> > > > +       static const char * const panel_replay_status[] = {
> > > > +               "Sink device frame is locked to the Source
> > > > device",
> > > > +               "Sink device is coasting, using the VTotal
> > > > target",
> > > > +               "Sink device is governing the frame rate (frame
> > > > rate
> > > > unlock is granted)",
> > > > +               "Sink device in the process of re-locking with
> > > > the
> > > > Source device",
> > > > +       };
> > > >         const char *str;
> > > >         int ret;
> > > >         u8 status, error_status;
> > > >
> > > > -       if (!CAN_PSR(intel_dp)) {
> > > > -               seq_puts(m, "PSR Unsupported\n");
> > > > +       if (!(CAN_PSR(intel_dp) || CAN_PANEL_REPLAY(intel_dp))) {
> > > > +               seq_puts(m, "PSR/Panel-Replay Unsupported\n");
> > > >                 return -ENODEV;
> > > >         }
> > > >
> > > >         if (connector->base.status != connector_status_connected)
> > > >                 return -ENODEV;
> > > >
> > > > -       ret = psr_get_status_and_error_status(intel_dp, &status,
> > > > &error_status);
> > > > -       if (ret)
> > > > -               return ret;
> > > > +       if (psr->panel_replay_enabled) {
> > > > +               u32 temp;
> > > >
> > > > -       status &= DP_PSR_SINK_STATE_MASK;
> > > > -       if (status < ARRAY_SIZE(sink_status))
> > > > -               str = sink_status[status];
> > > > -       else
> > > > -               str = "unknown";
> > > > +               ret =
> > > > panel_replay_get_status_and_error_status(intel_dp, &status,
> > > > &error_status);
> > > > +               if (ret)
> > > > +                       return ret;
> > > >
> > > > -       seq_printf(m, "Sink PSR status: 0x%x [%s]\n", status,
> > > > str);
> > > > +               temp = status & DP_SINK_FRAME_LOCKED_MASK;
> > > > +               temp >>= DP_SINK_FRAME_LOCKED_SHIFT;
> > > > +               if (temp < ARRAY_SIZE(panel_replay_status))
> > > > +                       str = panel_replay_status[temp];
> > > > +               else
> > > > +                       str = "unknown";
> > > >
> > > > -       seq_printf(m, "Sink PSR error status: 0x%x",
> > > > error_status);
> > > > +               seq_printf(m, "Sink Panel Replay status: 0x%x
> > > > [%s]\n", status, str);
> > > >
> > > > -       if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > > > -                           DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > > > -                           DP_PSR_LINK_CRC_ERROR))
> > > > -               seq_puts(m, ":\n");
> > > > -       else
> > > > -               seq_puts(m, "\n");
> > > > -       if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > > > -               seq_puts(m, "\tPSR RFB storage error\n");
> > > > -       if (error_status & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > > > -               seq_puts(m, "\tPSR VSC SDP uncorrectable
> > > > error\n");
> > > > -       if (error_status & DP_PSR_LINK_CRC_ERROR)
> > > > -               seq_puts(m, "\tPSR Link CRC error\n");
> > > > +               seq_printf(m, "Sink Panel Replay error status:
> > > > 0x%x",
> > > > error_status);
> > > > +
> > > > +               if (error_status &
> > > > (DP_PANEL_REPLAY_RFB_STORAGE_ERROR
> > > > >
> > > > +
> > > > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR |
> > > > +
> > > > DP_PANEL_REPLAY_LINK_CRC_ERROR))
> > > > +                       seq_puts(m, ":\n");
> > > > +               else
> > > > +                       seq_puts(m, "\n");
> > > > +               if (error_status &
> > > > DP_PANEL_REPLAY_RFB_STORAGE_ERROR)
> > > > +                       seq_puts(m, "\tPANEL-REPLAY RFB storage
> > > > error\n");
> > > > +               if (error_status &
> > > > DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR)
> > > > +                       seq_puts(m, "\tPANEL-REPLAY VSC SDP
> > > > uncorrectable error\n");
> > > > +               if (error_status &
> > > > DP_PANEL_REPLAY_LINK_CRC_ERROR)
> > > > +                       seq_puts(m, "\tPANEL-REPLAY Link CRC
> > > > error\n");
> > > > +       } else {
> > > > +               ret = psr_get_status_and_error_status(intel_dp,
> > > > &status, &error_status);
> > > > +               if (ret)
> > > > +                       return ret;
> > > > +
> > > > +               status &= DP_PSR_SINK_STATE_MASK;
> > > > +               if (status < ARRAY_SIZE(sink_status))
> > > > +                       str = sink_status[status];
> > > > +               else
> > > > +                       str = "unknown";
> > > > +
> > > > +               seq_printf(m, "Sink PSR status: 0x%x [%s]\n",
> > > > status,
> > > > str);
> > > > +
> > > > +               seq_printf(m, "Sink PSR error status: 0x%x",
> > > > error_status);
> > > >
> > > > +               if (error_status & (DP_PSR_RFB_STORAGE_ERROR |
> > > > +
> > > > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> > > > +                                   DP_PSR_LINK_CRC_ERROR))
> > > > +                       seq_puts(m, ":\n");
> > > > +               else
> > > > +                       seq_puts(m, "\n");
> > > > +               if (error_status & DP_PSR_RFB_STORAGE_ERROR)
> > > > +                       seq_puts(m, "\tPSR RFB storage error\n");
> > > > +               if (error_status &
> > > > DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
> > > > +                       seq_puts(m, "\tPSR VSC SDP uncorrectable
> > > > error\n");
> > > > +               if (error_status & DP_PSR_LINK_CRC_ERROR)
> > > > +                       seq_puts(m, "\tPSR Link CRC error\n");
> > > > +       }
> > >
> > > Instead of duplicating so much here I think it woukd be ok to just
> > > drop PSR and do (same applies for all the statuses):
> > >
> > >
> > > if (error_status & (DP_PSR_LINK_CRC_ERROR |
> > > DP_PANEL_REPLAY_LINK_CRC_ERROR))
> > >     seq_puts(m, "\tLink CRC error\n");
> > >
> > > What do you think?
> >
> > Thinking of backward compatibility, I have added separately for panel
> > replay.
> > I feel good to have separate cleanup patch for psr and panel-replay
> > together, not part of this patch.
> 
> If you want to keep the format as it is, then you should solve it by changing
> just the mode in printout rather than duplicating everything.
> One way:
> 
> static const char *psr_mode_str(struct intel_dp *intel_dp) {
> 	if (intel_dp->psr.panel_replay_enabled)
> 		return "PANEL-REPLAY";
> 	else if(intel_dp->psr.psr_enabled)
> 		return "PSR";
> 
> 	return "unknown";
> }
> ...
> seq_puts(m, "\t%s RFB storage error\n", psr_mode_str(intel_dp)); ...
> 

Added in v8, please have a look if it looks good to you.

Regards,
Animesh
> >
> > >
> > >
> > > >         return ret;
> > > >  }
> > > >  DEFINE_SHOW_ATTRIBUTE(i915_psr_sink_status);
> > > > @@ -3288,13 +3353,16 @@ void
> > > > intel_psr_connector_debugfs_add(struct
> > > > intel_connector *connector)
> > > >         struct drm_i915_private *i915 = to_i915(connector-
> > > > >base.dev);
> > > >         struct dentry *root = connector->base.debugfs_entry;
> > > >
> > > > -       if (connector->base.connector_type !=
> > > DRM_MODE_CONNECTOR_eDP)
> > > > -               return;
> > > > +       if (connector->base.connector_type !=
> > > DRM_MODE_CONNECTOR_eDP)
> > > > {
> > > > +               if (!(HAS_DP20(i915) &&
> > > > +                     connector->base.connector_type ==
> > > > DRM_MODE_CONNECTOR_DisplayPort))
> > > > +                       return;
> > > > +       }
> > >
> > > Why do you need to check DRM_MODE_CONNECTOR_DisplayPort ? I
> think
> > > connector->base.connector_type != DRM_MODE_CONNECTOR_eDP &&
> > > !HAS_DP20(i915) is enough.
> >
> > As per your suggestion, the code will look like below:
> >
> >         if (connector->base.connector_type != DRM_MODE_CONNECTOR_eDP
> > && !HAS_DP20(i915))
> >                 return;
> >
> > For example in case of hdmi connector type in MTL still we go ahead
> > and create debugfs entry... rt? Please let me if I am missing
> > anything.
> 
> Ok, I got this now.
> 
> BR,
> 
> Jouni Högander
> >
> > Regards,
> > Animesh
> >
> > >
> > > BR,
> > >
> > > Jouni Högander
> > > >
> > > >         debugfs_create_file("i915_psr_sink_status", 0444, root,
> > > >                             connector,
> > > > &i915_psr_sink_status_fops);
> > > >
> > > > -       if (HAS_PSR(i915))
> > > > +       if (HAS_PSR(i915) || HAS_DP20(i915))
> > > >                 debugfs_create_file("i915_psr_status", 0444, root,
> > > >                                     connector,
> > > > &i915_psr_status_fops);
> > > >  }
> >

RE: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for panelreplay

[Manna, Animesh]

> -----Original Message-----
> From: Nikula, Jani <jani.nikula@intel.com>
> Sent: Friday, November 3, 2023 2:55 PM
> To: Manna, Animesh <animesh.manna@intel.com>; intel-
> gfx@lists.freedesktop.org; Maxime Ripard <mripard@kernel.org>; Thomas
> Zimmermann <tzimmermann@suse.de>; Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com>
> Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> <animesh.manna@intel.com>; Hogander, Jouni
> <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> Subject: Re: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for
> panelreplay
> 
> On Wed, 11 Oct 2023, Animesh Manna <animesh.manna@intel.com> wrote:
> > Add DPCD register definition for discovering, enabling and checking
> > status of panel replay of the sink.
> >
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
> > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> 
> Maarten, Maxime, Thomas -
> 
> Ack for merging this via drm-intel-next?

Ping!

Regards,
Animesh

> 
> Thanks,
> Jani.
> 
> > ---
> >  include/drm/display/drm_dp.h | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/include/drm/display/drm_dp.h
> > b/include/drm/display/drm_dp.h index e69cece404b3..fc42b622ef32
> 100644
> > --- a/include/drm/display/drm_dp.h
> > +++ b/include/drm/display/drm_dp.h
> > @@ -543,6 +543,10 @@
> >  /* DFP Capability Extension */
> >  #define DP_DFP_CAPABILITY_EXTENSION_SUPPORT	0x0a3	/* 2.0 */
> >
> > +#define DP_PANEL_REPLAY_CAP                 0x0b0  /* DP 2.0 */
> > +# define DP_PANEL_REPLAY_SUPPORT            (1 << 0)
> > +# define DP_PANEL_REPLAY_SU_SUPPORT         (1 << 1)
> > +
> >  /* Link Configuration */
> >  #define	DP_LINK_BW_SET		            0x100
> >  # define DP_LINK_RATE_TABLE		    0x00    /* eDP 1.4 */
> > @@ -716,6 +720,13 @@
> >  #define DP_BRANCH_DEVICE_CTRL		    0x1a1
> >  # define DP_BRANCH_DEVICE_IRQ_HPD	    (1 << 0)
> >
> > +#define PANEL_REPLAY_CONFIG                             0x1b0  /* DP 2.0 */
> > +# define DP_PANEL_REPLAY_ENABLE                         (1 << 0)
> > +# define DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN         (1 << 3)
> > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN           (1 << 4)
> > +# define DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN      (1 << 5)
> > +# define DP_PANEL_REPLAY_SU_ENABLE                      (1 << 6)
> > +
> >  #define DP_PAYLOAD_ALLOCATE_SET		    0x1c0
> >  #define DP_PAYLOAD_ALLOCATE_START_TIME_SLOT 0x1c1  #define
> > DP_PAYLOAD_ALLOCATE_TIME_SLOT_COUNT 0x1c2 @@ -1105,6 +1116,18
> @@
> >  #define DP_LANE_ALIGN_STATUS_UPDATED_ESI       0x200e /* status same
> as 0x204 */
> >  #define DP_SINK_STATUS_ESI                     0x200f /* status same as 0x205 */
> >
> > +#define DP_PANEL_REPLAY_ERROR_STATUS                   0x2020  /* DP 2.1*/
> > +# define DP_PANEL_REPLAY_LINK_CRC_ERROR                (1 << 0)
> > +# define DP_PANEL_REPLAY_RFB_STORAGE_ERROR             (1 << 1)
> > +# define DP_PANEL_REPLAY_VSC_SDP_UNCORRECTABLE_ERROR   (1 << 2)
> > +
> > +#define DP_SINK_DEVICE_PR_AND_FRAME_LOCK_STATUS        0x2022  /*
> DP 2.1 */
> > +# define DP_SINK_DEVICE_PANEL_REPLAY_STATUS_MASK       (7 << 0)
> > +# define DP_SINK_FRAME_LOCKED_SHIFT                    3
> > +# define DP_SINK_FRAME_LOCKED_MASK                     (3 << 3)
> > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_SHIFT       5
> > +# define DP_SINK_FRAME_LOCKED_STATUS_VALID_MASK        (1 << 5)
> > +
> >  /* Extended Receiver Capability: See DP_DPCD_REV for definitions */
> >  #define DP_DP13_DPCD_REV                    0x2200
> 
> --
> Jani Nikula, Intel

Re: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for panelreplay

[Maxime Ripard]

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

On Mon, Nov 06, 2023 at 01:01:19PM +0000, Manna, Animesh wrote:
> 
> 
> > -----Original Message-----
> > From: Nikula, Jani <jani.nikula@intel.com>
> > Sent: Friday, November 3, 2023 2:55 PM
> > To: Manna, Animesh <animesh.manna@intel.com>; intel-
> > gfx@lists.freedesktop.org; Maxime Ripard <mripard@kernel.org>; Thomas
> > Zimmermann <tzimmermann@suse.de>; Maarten Lankhorst
> > <maarten.lankhorst@linux.intel.com>
> > Cc: dri-devel@lists.freedesktop.org; Manna, Animesh
> > <animesh.manna@intel.com>; Hogander, Jouni
> > <jouni.hogander@intel.com>; Murthy, Arun R <arun.r.murthy@intel.com>
> > Subject: Re: [PATCH v7 1/6] drm/panelreplay: dpcd register definition for
> > panelreplay
> > 
> > On Wed, 11 Oct 2023, Animesh Manna <animesh.manna@intel.com> wrote:
> > > Add DPCD register definition for discovering, enabling and checking
> > > status of panel replay of the sink.
> > >
> > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > Cc: Arun R Murthy <arun.r.murthy@intel.com>
> > > Cc: Jani Nikula <jani.nikula@intel.com>
> > > Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
> > > Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> > 
> > Maarten, Maxime, Thomas -
> > 
> > Ack for merging this via drm-intel-next?
> 
> Ping!

Ack

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]