diff mbox

drm/i915/psr : Add psr1 live status

Message ID 1524810255-32342-1-git-send-email-vathsala.nagaraju@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

vathsala nagaraju April 27, 2018, 6:24 a.m. UTC 
From: Vathsala Nagaraju <vathsala.nagaraju@intel.com>

Prints live state of psr1.Extending the existing
PSR2 live state function to cover psr1.

Tested on KBL with psr2 and psr1 panel.

v2: rebase

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 67 ++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 2 files changed, 44 insertions(+), 24 deletions(-)

Comments

Dhinakaran Pandiyan May 10, 2018, 2:02 a.m. UTC | #1 
On Fri, 2018-04-27 at 11:54 +0530, vathsala nagaraju wrote:
> From: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> 
> Prints live state of psr1.Extending the existing
> PSR2 live state function to cover psr1.
> 
> Tested on KBL with psr2 and psr1 panel.

Thanks for the patch, please see comments below.

> 
> v2: rebase
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 67 ++++++++++++++++++++++++---
> ----------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 44 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index cb1a804..1bf2245 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2580,25 +2580,42 @@ static int i915_guc_log_relay_release(struct
> inode *inode, struct file *file)
>  	.release = i915_guc_log_relay_release,
>  };
>  
> -static const char *psr2_live_status(u32 val)
> -{
> -	static const char * const live_status[] = {
> -		"IDLE",
> -		"CAPTURE",
> -		"CAPTURE_FS",
> -		"SLEEP",
> -		"BUFON_FW",
> -		"ML_UP",
> -		"SU_STANDBY",
> -		"FAST_SLEEP",
> -		"DEEP_SLEEP",
> -		"BUF_ON",
> -		"TG_ON"
> -	};
> -
> -	val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> EDP_PSR2_STATUS_STATE_SHIFT;
> -	if (val < ARRAY_SIZE(live_status))
> -		return live_status[val];
> +static const char *psr_live_status(bool is_psr2_enabled, u32 val)

I thought you'd agreed to change the function name to psr_source_status
in v2.

If you change the signature to
static void psr_source_status(struct drm_i915_private *dev_priv,
			      struct seq_file *m)

> +{
> +	if (is_psr2_enabled) {
	if (dev_priv->psr.psr2_enabled) {
		status = I915_READ(EDP_PSR2_STATUS);
> +		static const char * const live_status[] = {
> +			"IDLE",
> +			"CAPTURE",
> +			"CAPTURE_FS",
> +			"SLEEP",
> +			"BUFON_FW",
> +			"ML_UP",
> +			"SU_STANDBY",
> +			"FAST_SLEEP",
> +			"DEEP_SLEEP",
> +			"BUF_ON",
> +			"TG_ON"
> +		};
> +		val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> +			EDP_PSR2_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status))
> +			return live_status[val];
		seq_printf(m, "Source status: %x [%s]", status, 			 
 live_status[index]);
> +	} else {


<snip>
> @@ -2698,12 +2716,13 @@ static int i915_edp_psr_status(struct
> seq_file *m, void *data)
>  
>  		seq_printf(m, "Performance_Counter: %u\n", psrperf);
>  	}
> -	if (dev_priv->psr.psr2_enabled) {
> -		u32 psr2 = I915_READ(EDP_PSR2_STATUS);
>  
> -		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> -			   psr2, psr2_live_status(psr2));
> -	}
> +	psr_status = (dev_priv->psr.psr2_enabled) ?
> I915_READ(EDP_PSR2_STATUS) :
> +						    I915_READ(EDP_PS
> R_STATUS);
> +	seq_printf(m, "EDP_SOURCE_PSR%s_STATUS: %x [%s]\n",
> +			dev_priv->psr.psr2_enabled ? "2" : "1",

you can avoid these extra branches on psr2_enabled. And there is also
too much going on in i915_edp_psr_status() .



> +			psr_status,
> +			psr_live_status(dev_priv->psr.psr2_enabled,
> psr_status));
>  
>  	if (dev_priv->psr.enabled) {
>  		struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> >aux;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 391825a..2642b97 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4065,6 +4065,7 @@ enum {
>  #define   EDP_PSR_STATUS_SENDING_TP2_TP3	(1<<8)
>  #define   EDP_PSR_STATUS_SENDING_TP1		(1<<4)
>  #define   EDP_PSR_STATUS_IDLE_MASK		0xf
> +#define   EDP_PSR_STATUS_STATE_SHIFT		29

Please move this below EDP_PSR_STATUS_STATE_MASK


>  
>  #define EDP_PSR_PERF_CNT		_MMIO(dev_priv-
> >psr_mmio_base + 0x44)
>  #define   EDP_PSR_PERF_CNT_MASK		0xffffff
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index cb1a804..1bf2245 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2580,25 +2580,42 @@  static int i915_guc_log_relay_release(struct inode *inode, struct file *file)
 	.release = i915_guc_log_relay_release,
 };
 
-static const char *psr2_live_status(u32 val)
-{
-	static const char * const live_status[] = {
-		"IDLE",
-		"CAPTURE",
-		"CAPTURE_FS",
-		"SLEEP",
-		"BUFON_FW",
-		"ML_UP",
-		"SU_STANDBY",
-		"FAST_SLEEP",
-		"DEEP_SLEEP",
-		"BUF_ON",
-		"TG_ON"
-	};
-
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+static const char *psr_live_status(bool is_psr2_enabled, u32 val)
+{
+	if (is_psr2_enabled) {
+		static const char * const live_status[] = {
+			"IDLE",
+			"CAPTURE",
+			"CAPTURE_FS",
+			"SLEEP",
+			"BUFON_FW",
+			"ML_UP",
+			"SU_STANDBY",
+			"FAST_SLEEP",
+			"DEEP_SLEEP",
+			"BUF_ON",
+			"TG_ON"
+		};
+		val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status))
+			return live_status[val];
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		val = (val & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status))
+			return live_status[val];
+	}
 
 	return "unknown";
 }
@@ -2631,6 +2648,7 @@  static int i915_edp_psr_status(struct seq_file *m, void *data)
 	enum pipe pipe;
 	bool enabled = false;
 	bool sink_support;
+	u32 psr_status;
 
 	if (!HAS_PSR(dev_priv))
 		return -ENODEV;
@@ -2698,12 +2716,13 @@  static int i915_edp_psr_status(struct seq_file *m, void *data)
 
 		seq_printf(m, "Performance_Counter: %u\n", psrperf);
 	}
-	if (dev_priv->psr.psr2_enabled) {
-		u32 psr2 = I915_READ(EDP_PSR2_STATUS);
 
-		seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
-			   psr2, psr2_live_status(psr2));
-	}
+	psr_status = (dev_priv->psr.psr2_enabled) ? I915_READ(EDP_PSR2_STATUS) :
+						    I915_READ(EDP_PSR_STATUS);
+	seq_printf(m, "EDP_SOURCE_PSR%s_STATUS: %x [%s]\n",
+			dev_priv->psr.psr2_enabled ? "2" : "1",
+			psr_status,
+			psr_live_status(dev_priv->psr.psr2_enabled, psr_status));
 
 	if (dev_priv->psr.enabled) {
 		struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 391825a..2642b97 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4065,6 +4065,7 @@  enum {
 #define   EDP_PSR_STATUS_SENDING_TP2_TP3	(1<<8)
 #define   EDP_PSR_STATUS_SENDING_TP1		(1<<4)
 #define   EDP_PSR_STATUS_IDLE_MASK		0xf
+#define   EDP_PSR_STATUS_STATE_SHIFT		29
 
 #define EDP_PSR_PERF_CNT		_MMIO(dev_priv->psr_mmio_base + 0x44)
 #define   EDP_PSR_PERF_CNT_MASK		0xffffff