diff mbox series

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

Message ID 20231011110936.1851563-7-animesh.manna@intel.com (mailing list archive)
State New, archived
Headers show
Series Panel replay phase1 implementation | expand

Commit Message

Manna, Animesh Oct. 11, 2023, 11:09 a.m. UTC
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(-)

Comments

Arun R Murthy Oct. 16, 2023, 5:27 a.m. UTC | #1
> -----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
Arun R Murthy Oct. 30, 2023, 9:36 a.m. UTC | #2
> -----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
Jouni Högander Nov. 2, 2023, 7:38 a.m. UTC | #3
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);
>  }
Manna, Animesh Nov. 3, 2023, 6:10 a.m. UTC | #4
> -----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);
> >  }
Jouni Högander Nov. 3, 2023, 7:02 a.m. UTC | #5
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);
> > >  }
>
Manna, Animesh Nov. 3, 2023, 9:16 p.m. UTC | #6
> -----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);
> > > >  }
> >
diff mbox series

Patch

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);
 }