All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/psr : Add psr1 live status
@ 2018-04-20  9:36 vathsala nagaraju
  2018-04-20  9:48 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 27+ messages in thread
From: vathsala nagaraju @ 2018-04-20  9:36 UTC (permalink / raw)
  To: rodrigo.vivi, intel-gfx; +Cc: Dhinakaran Pandiyan

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.

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 | 68 ++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 2 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e0274f4..3056f04 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";
 }
@@ -2611,6 +2628,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;
@@ -2678,12 +2696,14 @@ 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_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));
+
 	mutex_unlock(&dev_priv->psr.lock);
 
 	intel_runtime_pm_put(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index fb10602..c9598b4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4058,6 +4058,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
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
@ 2018-04-20  9:48 ` Patchwork
  2018-04-20 10:03 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-04-20  9:48 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr : Add psr1 live status
URL   : https://patchwork.freedesktop.org/series/42021/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ea1d0148dd9e drm/i915/psr : Add psr1 live status
-:103: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#103: FILE: drivers/gpu/drm/i915/i915_debugfs.c:2703:
+	seq_printf(m, "EDP_PSR%s_STATUS: %x [%s]\n",
+		      dev_priv->psr.psr2_enabled ? "2" : "1",

total: 0 errors, 0 warnings, 1 checks, 94 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
  2018-04-20  9:48 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-04-20 10:03 ` Patchwork
  2018-04-20 11:25 ` Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-04-20 10:03 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr : Add psr1 live status
URL   : https://patchwork.freedesktop.org/series/42021/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4072 -> Patchwork_8761 =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42021/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cfl-s3:          PASS -> FAIL (fdo#103928, fdo#100368)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084


== Participating hosts (33 -> 31) ==

  Missing    (2): fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4072 -> Patchwork_8761

  CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8761: ea1d0148dd9e79df738557729c0da1920146afed @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

ea1d0148dd9e drm/i915/psr : Add psr1 live status

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8761/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
  2018-04-20  9:48 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-04-20 10:03 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-20 11:25 ` Patchwork
  2018-04-20 12:23 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-04-20 11:25 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr : Add psr1 live status
URL   : https://patchwork.freedesktop.org/series/42021/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4072 -> Patchwork_8761 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42021/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         PASS -> FAIL (fdo#102575)

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-cfl-s3:          PASS -> FAIL (fdo#100368, fdo#103928)

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928


== Participating hosts (33 -> 4) ==

  Missing    (29): fi-skl-6770hq fi-bdw-gvtdvm fi-skl-6260u fi-snb-2520m fi-bxt-j4205 fi-byt-n2820 fi-skl-6700hq fi-skl-6600u fi-snb-2600 fi-bxt-dsi fi-bdw-5557u fi-bsw-n3050 fi-byt-j1900 fi-bwr-2160 fi-ilk-650 fi-hsw-4770 fi-ivb-3770 fi-elk-e7500 fi-ivb-3520m fi-skl-6700k2 fi-kbl-r fi-kbl-7567u fi-ilk-m540 fi-skl-gvtdvm fi-skl-guc fi-cnl-y3 fi-cfl-8700k fi-cfl-u fi-glk-j4005 


== Build changes ==

    * Linux: CI_DRM_4072 -> Patchwork_8761

  CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8761: ea1d0148dd9e79df738557729c0da1920146afed @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

ea1d0148dd9e drm/i915/psr : Add psr1 live status

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8761/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
                   ` (2 preceding siblings ...)
  2018-04-20 11:25 ` Patchwork
@ 2018-04-20 12:23 ` Patchwork
  2018-04-20 17:14 ` [PATCH] " Souza, Jose
  2018-04-20 17:35 ` Rodrigo Vivi
  5 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-04-20 12:23 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/psr : Add psr1 live status
URL   : https://patchwork.freedesktop.org/series/42021/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4072_full -> Patchwork_8761_full =

== Summary - WARNING ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42021/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_mocs_settings@mocs-rc6-dirty-render:
      shard-kbl:          PASS -> SKIP

    igt@gem_mocs_settings@mocs-rc6-render:
      shard-kbl:          SKIP -> PASS

    igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
      shard-glk:          PASS -> SKIP +80

    igt@kms_vblank@pipe-b-wait-forked-busy-hang:
      shard-glk:          SKIP -> PASS +87

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_flip@blocking-wf_vblank:
      shard-hsw:          PASS -> FAIL (fdo#100368)

    igt@kms_flip@flip-vs-absolute-wf_vblank:
      shard-hsw:          PASS -> FAIL (fdo#103928)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-kbl:          PASS -> FAIL (fdo#103060)

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@perf@rc6-disable:
      shard-kbl:          PASS -> FAIL (fdo#103179)

    
    ==== Possible fixes ====

    igt@kms_flip@dpms-vs-vblank-race-interruptible:
      shard-glk:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@plain-flip-fb-recreate:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@plain-flip-ts-check-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS +2

    igt@perf@blocking:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103179 https://bugs.freedesktop.org/show_bug.cgi?id=103179
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4072 -> Patchwork_8761

  CI_DRM_4072: b35e59e5c6a9cae11d5183d2bf9c5c99ceedbc7c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4442: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8761: ea1d0148dd9e79df738557729c0da1920146afed @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4442: e60d247eb359f044caf0c09904da14e39d7adca1 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8761/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
                   ` (3 preceding siblings ...)
  2018-04-20 12:23 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-04-20 17:14 ` Souza, Jose
  2018-04-25  0:56   ` Dhinakaran Pandiyan
  2018-04-20 17:35 ` Rodrigo Vivi
  5 siblings, 1 reply; 27+ messages in thread
From: Souza, Jose @ 2018-04-20 17:14 UTC (permalink / raw)
  To: intel-gfx, Vivi, Rodrigo, Nagaraju, Vathsala; +Cc: Pandiyan, Dhinakaran

On Fri, 2018-04-20 at 15:06 +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.
> 
> 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 | 68 ++++++++++++++++++++++++---
> ----------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index e0274f4..3056f04 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";
>  }
> @@ -2611,6 +2628,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;
> @@ -2678,12 +2696,14 @@ 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);

Maybe move the read of the PSR or PSR2 status to inside of
psr_live_status()


Other than that looks good to me.

> +	seq_printf(m, "EDP_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));
> +
>  	mutex_unlock(&dev_priv->psr.lock);
>  
>  	intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index fb10602..c9598b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4058,6 +4058,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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
                   ` (4 preceding siblings ...)
  2018-04-20 17:14 ` [PATCH] " Souza, Jose
@ 2018-04-20 17:35 ` Rodrigo Vivi
  2018-04-21  4:00   ` Nagaraju, Vathsala
  5 siblings, 1 reply; 27+ messages in thread
From: Rodrigo Vivi @ 2018-04-20 17:35 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx, Dhinakaran Pandiyan

On Fri, Apr 20, 2018 at 03:06:03PM +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.

Does it really work?

I mean... I heard DK complaining that any read to these
MMIO in some gen9 platforms were triggering the PSR exit
or something like that. So, is this really reliable?

Or it is one of those info that will misslead users to file
non existent bugs?

> 
> 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 | 68 ++++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e0274f4..3056f04 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";
>  }
> @@ -2611,6 +2628,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;
> @@ -2678,12 +2696,14 @@ 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_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));
> +
>  	mutex_unlock(&dev_priv->psr.lock);
>  
>  	intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index fb10602..c9598b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4058,6 +4058,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
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-20 17:35 ` Rodrigo Vivi
@ 2018-04-21  4:00   ` Nagaraju, Vathsala
  2018-04-23  8:00     ` vathsala nagaraju
  0 siblings, 1 reply; 27+ messages in thread
From: Nagaraju, Vathsala @ 2018-04-21  4:00 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx, Pandiyan, Dhinakaran



-----Original Message-----
From: Vivi, Rodrigo 
Sent: Friday, April 20, 2018 11:06 PM
To: Nagaraju, Vathsala <vathsala.nagaraju@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
Subject: Re: [PATCH] drm/i915/psr : Add psr1 live status

On Fri, Apr 20, 2018 at 03:06:03PM +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.

Does it really work?

I mean... I heard DK complaining that any read to these MMIO in some gen9 platforms were triggering the PSR exit or something like that. So, is this really reliable?

https://patchwork.freedesktop.org/patch/218153/ 
https://patchwork.freedesktop.org/patch/218154/
"Writes to pipe related registers will still cause HW to exit PSR."

Or it is one of those info that will misslead users to file non existent bugs?
Google used this heavily for psr2 status during video playback etc,  so far no has filed any non-existent bug using this interface.
This status is useful.
> 
> 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 | 68 ++++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 45 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index e0274f4..3056f04 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";
>  }
> @@ -2611,6 +2628,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;
> @@ -2678,12 +2696,14 @@ 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_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));
> +
>  	mutex_unlock(&dev_priv->psr.lock);
>  
>  	intel_runtime_pm_put(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> b/drivers/gpu/drm/i915/i915_reg.h index fb10602..c9598b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4058,6 +4058,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
> --
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-21  4:00   ` Nagaraju, Vathsala
@ 2018-04-23  8:00     ` vathsala nagaraju
  0 siblings, 0 replies; 27+ messages in thread
From: vathsala nagaraju @ 2018-04-23  8:00 UTC (permalink / raw)
  To: Vivi, Rodrigo; +Cc: intel-gfx, Pandiyan, Dhinakaran

On Saturday 21 April 2018 09:30 AM, Nagaraju, Vathsala wrote:
>
> -----Original Message-----
> From: Vivi, Rodrigo
> Sent: Friday, April 20, 2018 11:06 PM
> To: Nagaraju, Vathsala <vathsala.nagaraju@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>
> Subject: Re: [PATCH] drm/i915/psr : Add psr1 live status
>
> On Fri, Apr 20, 2018 at 03:06:03PM +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.
> Does it really work?
>
> I mean... I heard DK complaining that any read to these MMIO in some gen9 platforms were triggering the PSR exit or something like that. So, is this really reliable?
https://patchwork.freedesktop.org/patch/218153/ 
https://patchwork.freedesktop.org/patch/218154/ "Writes to pipe related 
registers will still cause HW to exit PSR."

Reading of PSR_STATUS is not causing  the exit. Confirmed by reading pipe_event before to read and after read of psr_status reg.

Test case: pause the youtube video full screen.

Currenlty for psr1 , PSR_mask (bit 16 , mask display reg write ) is unset , there is continous psr_exit.

Or it is one of those info that will misslead users to file non existent bugs?

  Google used this heavily for psr2 status during video playback etc,  so far no has filed any non-existent bug using this interface.
This status is useful.


if you think it will confuse the user to file more bugs, we can add exit reason from PSR_EVENT register.


>> 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 | 68 ++++++++++++++++++++++++-------------
>>   drivers/gpu/drm/i915/i915_reg.h     |  1 +
>>   2 files changed, 45 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index e0274f4..3056f04 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";
>>   }
>> @@ -2611,6 +2628,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;
>> @@ -2678,12 +2696,14 @@ 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_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));
>> +
>>   	mutex_unlock(&dev_priv->psr.lock);
>>   
>>   	intel_runtime_pm_put(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index fb10602..c9598b4 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4058,6 +4058,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
>> --
>> 1.9.1
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-20 17:14 ` [PATCH] " Souza, Jose
@ 2018-04-25  0:56   ` Dhinakaran Pandiyan
  2018-04-27  5:58     ` vathsala nagaraju
  0 siblings, 1 reply; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-25  0:56 UTC (permalink / raw)
  To: Souza, Jose; +Cc: intel-gfx, Vivi, Rodrigo




On Fri, 2018-04-20 at 17:14 +0000, Souza, Jose wrote:
> On Fri, 2018-04-20 at 15:06 +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.
> > 
> > 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 | 68 ++++++++++++++++++++++++---
> > ----------
> >  drivers/gpu/drm/i915/i915_reg.h     |  1 +
> >  2 files changed, 45 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index e0274f4..3056f04 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";
> >  }
> > @@ -2611,6 +2628,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;
> > @@ -2678,12 +2696,14 @@ 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);
> 
> Maybe move the read of the PSR or PSR2 status to inside of
> psr_live_status()

I am thinking we could reduce some clutter by changing both the status
functions to have this signature. 


static void psr_source_status(dev_priv, m) 
{

}

static void psr_sink_status(dev_priv, m)
{

}


> 
> 
> Other than that looks good to me.
> 
> > +	seq_printf(m, "EDP_PSR%s_STATUS: %x [%s]\n",
			^source_status or whatever the correct parallel to sink status that
Jose is using.


> > +		      dev_priv->psr.psr2_enabled ? "2" : "1",
> > +		      psr_status,
> > +		      psr_live_status(dev_priv->psr.psr2_enabled,
> > psr_status));
> > +
> >  	mutex_unlock(&dev_priv->psr.lock);
> >  
> >  	intel_runtime_pm_put(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index fb10602..c9598b4 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4058,6 +4058,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-25  0:56   ` Dhinakaran Pandiyan
@ 2018-04-27  5:58     ` vathsala nagaraju
  0 siblings, 0 replies; 27+ messages in thread
From: vathsala nagaraju @ 2018-04-27  5:58 UTC (permalink / raw)
  To: dhinakaran.pandiyan, Souza, Jose; +Cc: intel-gfx, Vivi, Rodrigo

On Wednesday 25 April 2018 06:26 AM, Dhinakaran Pandiyan wrote:
>
>
> On Fri, 2018-04-20 at 17:14 +0000, Souza, Jose wrote:
>> On Fri, 2018-04-20 at 15:06 +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.
>>>
>>> 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 | 68 ++++++++++++++++++++++++---
>>> ----------
>>>   drivers/gpu/drm/i915/i915_reg.h     |  1 +
>>>   2 files changed, 45 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index e0274f4..3056f04 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";
>>>   }
>>> @@ -2611,6 +2628,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;
>>> @@ -2678,12 +2696,14 @@ 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);
>> Maybe move the read of the PSR or PSR2 status to inside of
>> psr_live_status()
We are printing psr_status  and it's live status[ additional debug 
information] ,
reading the psr_status here and only getting live status from 
psr_live_status function.

I am thinking we could reduce some clutter by changing both the status
functions to have this signature.


static void psr_source_status(dev_priv, m)
{

}

static void psr_sink_status(dev_priv, m)
{

}


Sure , we can change. Will send the v2 version.

>>
>> Other than that looks good to me.
>>
>>> +	seq_printf(m, "EDP_PSR%s_STATUS: %x [%s]\n",
> 			^source_status or whatever the correct parallel to sink status that
> Jose is using.
>
>
>>> +		      dev_priv->psr.psr2_enabled ? "2" : "1",
>>> +		      psr_status,
>>> +		      psr_live_status(dev_priv->psr.psr2_enabled,
>>> psr_status));
>>> +
>>>   	mutex_unlock(&dev_priv->psr.lock);
>>>   
>>>   	intel_runtime_pm_put(dev_priv);
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index fb10602..c9598b4 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -4058,6 +4058,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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr: Add psr1 live status
  2018-06-27  8:08 vathsala nagaraju
@ 2018-07-02 18:38 ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-07-02 18:38 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx, Rodrigo Vivi

On Wed, 2018-06-27 at 13:38 +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.
> 
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status.
> v4: DK
>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>     Pass seq to psr_source_status, handle source status prints in
>     psr_source_status.
> v5: Fixed CI warning messages
> v6:
>     Remove extra space in the title before the colon.(DK)
>     Rebase. (Jani)
> v7: Use tabs for indenting the values.(Jani)
> v8: Addressed dk's review comments.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>

I have pushed this version to -dinq, thanks for addressing the reviews.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr: Add psr1 live status
@ 2018-06-27  8:08 vathsala nagaraju
  2018-07-02 18:38 ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-06-27  8:08 UTC (permalink / raw)
  To: dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status.
v4: DK
    Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
    Pass seq to psr_source_status, handle source status prints in
    psr_source_status.
v5: Fixed CI warning messages
v6:
    Remove extra space in the title before the colon.(DK)
    Rebase. (Jani)
v7: Use tabs for indenting the values.(Jani)
v8: Addressed dk's review comments.

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

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

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c400f42..14e4d6c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2597,27 +2597,55 @@ 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"
-	};
+static void
+psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
+{
+	u32 val, psr_status;
 
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+	if (dev_priv->psr.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"
+		};
+		psr_status = I915_READ(EDP_PSR2_STATUS);
+		val = (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: 0x%x [%s]\n",
+				   psr_status, live_status[val]);
+			return;
+		}
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		psr_status = I915_READ(EDP_PSR_STATUS);
+		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: 0x%x [%s]\n",
+				   psr_status, live_status[val]);
+			return;
+		}
+	}
 
-	return "unknown";
+	seq_printf(m, "Source PSR status: 0x%x [%s]\n", psr_status, "unknown");
 }
 
 static const char *psr_sink_status(u8 val)
@@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
 
 	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 43db91c..f35df07 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4073,6 +4073,7 @@ enum {
 
 #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
+#define   EDP_PSR_STATUS_STATE_SHIFT		29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDENT		(2 << 29)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr: Add psr1 live status
  2018-06-22  3:59 vathsala nagaraju
@ 2018-06-22 18:51 ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-22 18:51 UTC (permalink / raw)
  To: vathsala nagaraju, jani.nikula, intel-gfx; +Cc: Rodrigo Vivi

On Fri, 2018-06-22 at 09:29 +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.
> 
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status.
> v4: DK
>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>     Pass seq to psr_source_status, handle source status prints in
>     psr_source_status.
> v5: Fixed CI warning messages
> v6:
>     Remove extra space in the title before the colon.(DK)
>     Rebase. (Jani)
> v7: use tabs for indenting the values.(Jani)

This made me go through the patch again, I'm afraid I see more
formatting (mostly) issues. Sorry for not noticing earlier.



> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 72 ++++++++++++++++++++++++---
> ----------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 49 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c400f42..3941d85 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2597,27 +2597,55 @@ 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"
> -	};
> +static void
> +psr_source_status(struct drm_i915_private *dev_priv, struct seq_file
> *m)
> +{
> +	u32 val, psr_status = 0;
		 unnecessary initialization.

>  
> -	val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> EDP_PSR2_STATUS_STATE_SHIFT;
> -	if (val < ARRAY_SIZE(live_status))
> -		return live_status[val];
> +	if (dev_priv->psr.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"
> +		};
> +		psr_status = I915_READ(EDP_PSR2_STATUS);
> +		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
                      ^extra space here.

> +			EDP_PSR2_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status)) {
> +			seq_printf(m, "Source PSR status: %x[%s]\n",
                                                          0x%x [%s]
i.e., you're missing the 0x prefix to denote hex numerals and a space.

> psr_status,
> +				   live_status[val]);
> +			return;
> +		}
> +	} else {
> +		static const char * const live_status[] = {
> +			"IDLE",
> +			"SRDONACK",
> +			"SRDENT",
> +			"BUFOFF",
> +			"BUFON",
> +			"AUXACK",
> +			"SRDOFFACK",
> +			"SRDENT_ON",
> +		};
> +		psr_status = I915_READ(EDP_PSR_STATUS);
> +		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> +			EDP_PSR_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status)) {
> +			seq_printf(m, "Source PSR status: %x[%s]\n",
> psr_status,
> +				   live_status[val]);
> +			return;
> +		}
> +	}
>  
> -	return "unknown";
> +	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, 
                              ^lower case is inconsistent with the
other debugs you are printing.

> "unknown");
>  }
>  
>  static const char *psr_sink_status(u8 val)
> @@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
>  
>  	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 caad19f..4efad4d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4072,6 +4072,7 @@ enum {
>  
>  #define EDP_PSR_STATUS				_MMIO(dev_priv
> ->psr_mmio_base + 0x40)
>  #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
> +#define   EDP_PSR_STATUS_STATE_SHIFT		29
>  #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDONACK		(1 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDENT		(2 << 29)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr: Add psr1 live status
@ 2018-06-22  3:59 vathsala nagaraju
  2018-06-22 18:51 ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-06-22  3:59 UTC (permalink / raw)
  To: dhinakaran.pandiyan, jani.nikula, intel-gfx; +Cc: Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status.
v4: DK
    Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
    Pass seq to psr_source_status, handle source status prints in
    psr_source_status.
v5: Fixed CI warning messages
v6:
    Remove extra space in the title before the colon.(DK)
    Rebase. (Jani)
v7: use tabs for indenting the values.(Jani)

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

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

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c400f42..3941d85 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2597,27 +2597,55 @@ 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"
-	};
+static void
+psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
+{
+	u32 val, psr_status = 0;
 
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+	if (dev_priv->psr.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"
+		};
+		psr_status = I915_READ(EDP_PSR2_STATUS);
+		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		psr_status = I915_READ(EDP_PSR_STATUS);
+		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	}
 
-	return "unknown";
+	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, "unknown");
 }
 
 static const char *psr_sink_status(u8 val)
@@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
 
 	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 caad19f..4efad4d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4072,6 +4072,7 @@ enum {
 
 #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
+#define   EDP_PSR_STATUS_STATE_SHIFT		29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDENT		(2 << 29)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr: Add psr1 live status
  2018-06-21  8:06 [PATCH] drm/i915/psr: " vathsala nagaraju
@ 2018-06-21  9:01 ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-06-21  9:01 UTC (permalink / raw)
  To: vathsala nagaraju, dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi

On Thu, 21 Jun 2018, vathsala nagaraju <vathsala.nagaraju@intel.com> 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.
>
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status.
> v4: DK
>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>     Pass seq to psr_source_status, handle source status prints in
>     psr_source_status.
> v5: Fixed CI warning messages
> v6:
>     Remove extra space in the title before the colon.(DK)
>     Rebase. (Jani)
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 72 ++++++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 49 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c400f42..3941d85 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2597,27 +2597,55 @@ 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"
> -	};
> +static void
> +psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
> +{
> +	u32 val, psr_status = 0;
>  
> -	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
> -	if (val < ARRAY_SIZE(live_status))
> -		return live_status[val];
> +	if (dev_priv->psr.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"
> +		};
> +		psr_status = I915_READ(EDP_PSR2_STATUS);
> +		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
> +			EDP_PSR2_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status)) {
> +			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
> +				   live_status[val]);
> +			return;
> +		}
> +	} else {
> +		static const char * const live_status[] = {
> +			"IDLE",
> +			"SRDONACK",
> +			"SRDENT",
> +			"BUFOFF",
> +			"BUFON",
> +			"AUXACK",
> +			"SRDOFFACK",
> +			"SRDENT_ON",
> +		};
> +		psr_status = I915_READ(EDP_PSR_STATUS);
> +		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> +			EDP_PSR_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status)) {
> +			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
> +				   live_status[val]);
> +			return;
> +		}
> +	}
>  
> -	return "unknown";
> +	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, "unknown");
>  }
>  
>  static const char *psr_sink_status(u8 val)
> @@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
>  
>  	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 4bfd7a9..f026492 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4072,6 +4072,7 @@ enum {
>  
>  #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
>  #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
> +#define   EDP_PSR_STATUS_STATE_SHIFT            29

Please use tabs for indenting the values.

BR,
Jani.

>  #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDONACK		(1 << 29)
>  #define   EDP_PSR_STATUS_STATE_SRDENT		(2 << 29)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr: Add psr1 live status
@ 2018-06-21  8:06 vathsala nagaraju
  2018-06-21  9:01 ` Jani Nikula
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-06-21  8:06 UTC (permalink / raw)
  To: dhinakaran.pandiyan, jani.nikula; +Cc: intel-gfx, Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status.
v4: DK
    Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
    Pass seq to psr_source_status, handle source status prints in
    psr_source_status.
v5: Fixed CI warning messages
v6:
    Remove extra space in the title before the colon.(DK)
    Rebase. (Jani)

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

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

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c400f42..3941d85 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2597,27 +2597,55 @@ 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"
-	};
+static void
+psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
+{
+	u32 val, psr_status = 0;
 
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+	if (dev_priv->psr.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"
+		};
+		psr_status = I915_READ(EDP_PSR2_STATUS);
+		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		psr_status = I915_READ(EDP_PSR_STATUS);
+		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	}
 
-	return "unknown";
+	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, "unknown");
 }
 
 static const char *psr_sink_status(u8 val)
@@ -2681,12 +2709,8 @@ 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_source_status(dev_priv, m);
 
 	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 4bfd7a9..f026492 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4072,6 +4072,7 @@ enum {
 
 #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7 << 29)
+#define   EDP_PSR_STATUS_STATE_SHIFT            29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1 << 29)
 #define   EDP_PSR_STATUS_STATE_SRDENT		(2 << 29)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-06-12 23:29 ` Dhinakaran Pandiyan
@ 2018-06-19 15:03   ` Jani Nikula
  0 siblings, 0 replies; 27+ messages in thread
From: Jani Nikula @ 2018-06-19 15:03 UTC (permalink / raw)
  To: dhinakaran.pandiyan, vathsala nagaraju; +Cc: intel-gfx, Rodrigo Vivi

On Tue, 12 Jun 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> On Fri, 2018-05-25 at 11:50 +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.
>> 
>> v2: rebase
>> v3: DK
>>     Rename psr2_live_status to psr_source_status.
>> v4: DK
>>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>>     Pass seq to psr_source_status, handle source status prints in
>>     psr_source_status.
>> v5: Fixed CI warning messages
>> 
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
>> 
>
> nit: Noticed an extra space in the title before the colon.
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

I'm afraid this no longer applies cleanly. Please resend if it's still
needed.

BR,
Jani.


>
>> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 72 ++++++++++++++++++++++++---
>> ----------
>>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>>  2 files changed, 49 insertions(+), 24 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 5251544..1d45cb9 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2596,27 +2596,55 @@ 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"
>> -	};
>> +static void
>> +psr_source_status(struct drm_i915_private *dev_priv, struct seq_file
>> *m)
>> +{
>> +	u32 val, psr_status = 0;
>>  
>> -	val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
>> EDP_PSR2_STATUS_STATE_SHIFT;
>> -	if (val < ARRAY_SIZE(live_status))
>> -		return live_status[val];
>> +	if (dev_priv->psr.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"
>> +		};
>> +		psr_status = I915_READ(EDP_PSR2_STATUS);
>> +		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
>> +			EDP_PSR2_STATUS_STATE_SHIFT;
>> +		if (val < ARRAY_SIZE(live_status)) {
>> +			seq_printf(m, "Source PSR status: %x[%s]\n",
>> psr_status,
>> +				   live_status[val]);
>> +			return;
>> +		}
>> +	} else {
>> +		static const char * const live_status[] = {
>> +			"IDLE",
>> +			"SRDONACK",
>> +			"SRDENT",
>> +			"BUFOFF",
>> +			"BUFON",
>> +			"AUXACK",
>> +			"SRDOFFACK",
>> +			"SRDENT_ON",
>> +		};
>> +		psr_status = I915_READ(EDP_PSR_STATUS);
>> +		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
>> +			EDP_PSR_STATUS_STATE_SHIFT;
> 			^alignment is different from the PSR2 block.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-05-25  6:20 vathsala nagaraju
@ 2018-06-12 23:29 ` Dhinakaran Pandiyan
  2018-06-19 15:03   ` Jani Nikula
  0 siblings, 1 reply; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-06-12 23:29 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx, Rodrigo Vivi

On Fri, 2018-05-25 at 11:50 +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.
> 
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status.
> v4: DK
>     Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
>     Pass seq to psr_source_status, handle source status prints in
>     psr_source_status.
> v5: Fixed CI warning messages
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 

nit: Noticed an extra space in the title before the colon.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 72 ++++++++++++++++++++++++---
> ----------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 49 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5251544..1d45cb9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2596,27 +2596,55 @@ 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"
> -	};
> +static void
> +psr_source_status(struct drm_i915_private *dev_priv, struct seq_file
> *m)
> +{
> +	u32 val, psr_status = 0;
>  
> -	val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> EDP_PSR2_STATUS_STATE_SHIFT;
> -	if (val < ARRAY_SIZE(live_status))
> -		return live_status[val];
> +	if (dev_priv->psr.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"
> +		};
> +		psr_status = I915_READ(EDP_PSR2_STATUS);
> +		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
> +			EDP_PSR2_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status)) {
> +			seq_printf(m, "Source PSR status: %x[%s]\n",
> psr_status,
> +				   live_status[val]);
> +			return;
> +		}
> +	} else {
> +		static const char * const live_status[] = {
> +			"IDLE",
> +			"SRDONACK",
> +			"SRDENT",
> +			"BUFOFF",
> +			"BUFON",
> +			"AUXACK",
> +			"SRDOFFACK",
> +			"SRDENT_ON",
> +		};
> +		psr_status = I915_READ(EDP_PSR_STATUS);
> +		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
> +			EDP_PSR_STATUS_STATE_SHIFT;
			^alignment is different from the PSR2 block.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr : Add psr1 live status
@ 2018-05-25  6:20 vathsala nagaraju
  2018-06-12 23:29 ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-05-25  6:20 UTC (permalink / raw)
  To: dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status.
v4: DK
    Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
    Pass seq to psr_source_status, handle source status prints in
    psr_source_status.
v5: Fixed CI warning messages

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 | 72 ++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 2 files changed, 49 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5251544..1d45cb9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2596,27 +2596,55 @@ 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"
-	};
+static void
+psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
+{
+	u32 val, psr_status = 0;
 
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+	if (dev_priv->psr.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"
+		};
+		psr_status = I915_READ(EDP_PSR2_STATUS);
+		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		psr_status = I915_READ(EDP_PSR_STATUS);
+		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+				   live_status[val]);
+			return;
+		}
+	}
 
-	return "unknown";
+	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, "unknown");
 }
 
 static const char *psr_sink_status(u8 val)
@@ -2714,12 +2742,8 @@ 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_source_status(dev_priv, m);
 
 	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 513b4a4..0ac25d9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4048,6 +4048,7 @@ enum {
 
 #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
+#define   EDP_PSR_STATUS_STATE_SHIFT            29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0<<29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1<<29)
 #define   EDP_PSR_STATUS_STATE_SRDENT		(2<<29)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr : Add psr1 live status
@ 2018-05-25  5:37 vathsala nagaraju
  0 siblings, 0 replies; 27+ messages in thread
From: vathsala nagaraju @ 2018-05-25  5:37 UTC (permalink / raw)
  To: dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status.
v4: DK
    Move EDP_PSR_STATUS_STATE_SHIFT below EDP_PSR_STATUS_STATE_MASK.
    Pass seq to psr_source_status, handle source status prints in
    psr_source_status.

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 | 71 ++++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 2 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5251544..9e6594c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2596,27 +2596,54 @@ 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"
-	};
+void psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m)
+{
+	u32 val, psr_status = 0;
 
-	val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT;
-	if (val < ARRAY_SIZE(live_status))
-		return live_status[val];
+	if (dev_priv->psr.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"
+		};
+		psr_status = I915_READ(EDP_PSR2_STATUS);
+		val =  (psr_status & EDP_PSR2_STATUS_STATE_MASK) >>
+			EDP_PSR2_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+					live_status[val]);
+			return;
+		}
+	} else {
+		static const char * const live_status[] = {
+			"IDLE",
+			"SRDONACK",
+			"SRDENT",
+			"BUFOFF",
+			"BUFON",
+			"AUXACK",
+			"SRDOFFACK",
+			"SRDENT_ON",
+		};
+		psr_status = I915_READ(EDP_PSR_STATUS);
+		val = (psr_status & EDP_PSR_STATUS_STATE_MASK) >>
+			EDP_PSR_STATUS_STATE_SHIFT;
+		if (val < ARRAY_SIZE(live_status)) {
+			seq_printf(m, "Source PSR status: %x[%s]\n", psr_status,
+					live_status[val]);
+			return;
+		}
+	}
 
-	return "unknown";
+	seq_printf(m, "Source psr status: %x[%s]\n", psr_status, "unknown");
 }
 
 static const char *psr_sink_status(u8 val)
@@ -2714,12 +2741,8 @@ 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_source_status(dev_priv, m);
 
 	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 513b4a4..0ac25d9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4048,6 +4048,7 @@ enum {
 
 #define EDP_PSR_STATUS				_MMIO(dev_priv->psr_mmio_base + 0x40)
 #define   EDP_PSR_STATUS_STATE_MASK		(7<<29)
+#define   EDP_PSR_STATUS_STATE_SHIFT            29
 #define   EDP_PSR_STATUS_STATE_IDLE		(0<<29)
 #define   EDP_PSR_STATUS_STATE_SRDONACK		(1<<29)
 #define   EDP_PSR_STATUS_STATE_SRDENT		(2<<29)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-05-23  5:37   ` Nagaraju, Vathsala
@ 2018-05-23 17:41     ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-05-23 17:41 UTC (permalink / raw)
  To: Nagaraju, Vathsala; +Cc: intel-gfx, Rodrigo Vivi

On Wed, 2018-05-23 at 11:07 +0530, Nagaraju, Vathsala wrote:
> 
> On 5/23/2018 1:28 AM, Dhinakaran Pandiyan wrote:
> > 
> > On Tue, 2018-05-22 at 14:27 +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.
> > > 
> > > v2: rebase
> > > v3: DK
> > >      Rename psr2_live_status to psr_source_status
> > > 
> > > 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 | 66
> > > +++++++++++++++++++++++--
> > > ------------
> > >   drivers/gpu/drm/i915/i915_reg.h     |  1 +
> > >   2 files changed, 43 insertions(+), 24 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 5251544..e4a2f15 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -2596,25 +2596,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_source_status(u32 val, bool
> > > is_psr2_enabled)
> > Please change this to psr_source_status(drm_i915_private *dev_priv)
> to print in format , source psr status %x [%s] , where %x = complete
> psr 
> source register value(0x6f940) , %s = psr_status_bits [31 :28/29].
> if we want handle everything as part of psr_source_status() , then
> we 
> need to return register value in some pointer.
> if not then we read the  reg and then pass it to psr_source_status
> which 
> returns live status string.

Pass struct seq_file *m to psr_source_status() and print the status
there.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-05-22 19:58 ` Dhinakaran Pandiyan
@ 2018-05-23  5:37   ` Nagaraju, Vathsala
  2018-05-23 17:41     ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: Nagaraju, Vathsala @ 2018-05-23  5:37 UTC (permalink / raw)
  To: dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi



On 5/23/2018 1:28 AM, Dhinakaran Pandiyan wrote:
> On Tue, 2018-05-22 at 14:27 +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.
>>
>> v2: rebase
>> v3: DK
>>      Rename psr2_live_status to psr_source_status
>>
>> 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 | 66 +++++++++++++++++++++++--
>> ------------
>>   drivers/gpu/drm/i915/i915_reg.h     |  1 +
>>   2 files changed, 43 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 5251544..e4a2f15 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2596,25 +2596,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_source_status(u32 val, bool is_psr2_enabled)
> Please change this to psr_source_status(drm_i915_private *dev_priv)
to print in format , source psr status %x [%s] , where %x = complete psr 
source register value(0x6f940) , %s = psr_status_bits [31 :28/29].
if we want handle everything as part of psr_source_status() , then we 
need to return register value in some pointer.
if not then we read the  reg and then pass it to psr_source_status which 
returns live status string.
>> +{
>> +	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"
>> +		};
> With that, you can		
> 		live_status = I915_READ(EDP_PSR2_STATUS);
>> +		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",
>> +		};
> 		live_status = I915_READ(EDP_PSR_STATUS);
>> +		val = (val & EDP_PSR_STATUS_STATE_MASK) >>
>> +			EDP_PSR_STATUS_STATE_SHIFT;
>> +		if (val < ARRAY_SIZE(live_status))
>> +			return live_status[val];
>> +	}
>>   
>>   	return "unknown";
>>   }
>> @@ -2647,6 +2664,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;
>> @@ -2714,12 +2732,12 @@ 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);
> Please move this inside psr_source_status(), I don't see a point in
> checking for psr2_enabled here and again in psr_source_status()
>
>
>
>> +	seq_printf(m, "SOURCE_PSR_STATUS: %x[%s]\n",
> It's easier on the eyes if these strings are consistent, there is no
> benefit in writing only this string differently.
>
> Sink status is printed as "Sink PSR status: 0x%x [%s]\n". Please do the
> same by writing this as "Source PSR status: 0x%x [%s]\n"
>   
>> +		     psr_status,
>> +		     psr_source_status(psr_status, dev_priv-
>>> psr.psr2_enabled));
>>   
>>   	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 513b4a4..3c42021 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -4069,6 +4069,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
> You ignored my review from last time, please move this where the mask
> 'EDP_PSR_STATUS_STATE_MASK' is defined. The rest of the file maintains
> this consistently and there is no reason to change that.
>
>
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-05-22  8:57 vathsala nagaraju
@ 2018-05-22 19:58 ` Dhinakaran Pandiyan
  2018-05-23  5:37   ` Nagaraju, Vathsala
  0 siblings, 1 reply; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-05-22 19:58 UTC (permalink / raw)
  To: vathsala nagaraju; +Cc: intel-gfx, Rodrigo Vivi

On Tue, 2018-05-22 at 14:27 +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.
> 
> v2: rebase
> v3: DK
>     Rename psr2_live_status to psr_source_status
> 
> 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 | 66 +++++++++++++++++++++++--
> ------------
>  drivers/gpu/drm/i915/i915_reg.h     |  1 +
>  2 files changed, 43 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5251544..e4a2f15 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2596,25 +2596,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_source_status(u32 val, bool is_psr2_enabled)
Please change this to psr_source_status(drm_i915_private *dev_priv)

> +{
> +	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"
> +		};
With that, you can		
		live_status = I915_READ(EDP_PSR2_STATUS);
> +		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",
> +		};

		live_status = I915_READ(EDP_PSR_STATUS);
> +		val = (val & EDP_PSR_STATUS_STATE_MASK) >>
> +			EDP_PSR_STATUS_STATE_SHIFT;
> +		if (val < ARRAY_SIZE(live_status))
> +			return live_status[val];
> +	}
>  
>  	return "unknown";
>  }
> @@ -2647,6 +2664,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;
> @@ -2714,12 +2732,12 @@ 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);

Please move this inside psr_source_status(), I don't see a point in
checking for psr2_enabled here and again in psr_source_status()



> +	seq_printf(m, "SOURCE_PSR_STATUS: %x[%s]\n",
It's easier on the eyes if these strings are consistent, there is no
benefit in writing only this string differently.

Sink status is printed as "Sink PSR status: 0x%x [%s]\n". Please do the
same by writing this as "Source PSR status: 0x%x [%s]\n"
 
> +		     psr_status,
> +		     psr_source_status(psr_status, dev_priv-
> >psr.psr2_enabled));
>  
>  	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 513b4a4..3c42021 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4069,6 +4069,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

You ignored my review from last time, please move this where the mask
'EDP_PSR_STATUS_STATE_MASK' is defined. The rest of the file maintains
this consistently and there is no reason to change that.



_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr : Add psr1 live status
@ 2018-05-22  8:57 vathsala nagaraju
  2018-05-22 19:58 ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-05-22  8:57 UTC (permalink / raw)
  To: dhinakaran.pandiyan; +Cc: intel-gfx, Rodrigo Vivi

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
v3: DK
    Rename psr2_live_status to psr_source_status

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 | 66 +++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_reg.h     |  1 +
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5251544..e4a2f15 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2596,25 +2596,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_source_status(u32 val, bool is_psr2_enabled)
+{
+	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";
 }
@@ -2647,6 +2664,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;
@@ -2714,12 +2732,12 @@ 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, "SOURCE_PSR_STATUS: %x[%s]\n",
+		     psr_status,
+		     psr_source_status(psr_status, dev_priv->psr.psr2_enabled));
 
 	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 513b4a4..3c42021 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4069,6 +4069,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
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915/psr : Add psr1 live status
  2018-04-27  6:24 vathsala nagaraju
@ 2018-05-10  2:02 ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 27+ messages in thread
From: Dhinakaran Pandiyan @ 2018-05-10  2:02 UTC (permalink / raw)
  To: vathsala nagaraju, rodrigo.vivi, intel-gfx

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/psr : Add psr1 live status
@ 2018-04-27  6:24 vathsala nagaraju
  2018-05-10  2:02 ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 27+ messages in thread
From: vathsala nagaraju @ 2018-04-27  6:24 UTC (permalink / raw)
  To: rodrigo.vivi, intel-gfx; +Cc: Dhinakaran Pandiyan

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(-)

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
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2018-07-02 18:11 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20  9:36 [PATCH] drm/i915/psr : Add psr1 live status vathsala nagaraju
2018-04-20  9:48 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-04-20 10:03 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-20 11:25 ` Patchwork
2018-04-20 12:23 ` ✓ Fi.CI.IGT: " Patchwork
2018-04-20 17:14 ` [PATCH] " Souza, Jose
2018-04-25  0:56   ` Dhinakaran Pandiyan
2018-04-27  5:58     ` vathsala nagaraju
2018-04-20 17:35 ` Rodrigo Vivi
2018-04-21  4:00   ` Nagaraju, Vathsala
2018-04-23  8:00     ` vathsala nagaraju
2018-04-27  6:24 vathsala nagaraju
2018-05-10  2:02 ` Dhinakaran Pandiyan
2018-05-22  8:57 vathsala nagaraju
2018-05-22 19:58 ` Dhinakaran Pandiyan
2018-05-23  5:37   ` Nagaraju, Vathsala
2018-05-23 17:41     ` Dhinakaran Pandiyan
2018-05-25  5:37 vathsala nagaraju
2018-05-25  6:20 vathsala nagaraju
2018-06-12 23:29 ` Dhinakaran Pandiyan
2018-06-19 15:03   ` Jani Nikula
2018-06-21  8:06 [PATCH] drm/i915/psr: " vathsala nagaraju
2018-06-21  9:01 ` Jani Nikula
2018-06-22  3:59 vathsala nagaraju
2018-06-22 18:51 ` Dhinakaran Pandiyan
2018-06-27  8:08 vathsala nagaraju
2018-07-02 18:38 ` Dhinakaran Pandiyan

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