* [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
@ 2018-04-20 22:27 José Roberto de Souza
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: José Roberto de Souza @ 2018-04-20 22:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
This will be helpful to debug what hardware is actually tracking
and causing PSR to exit.
BSpec: 7721
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
New patch in this series.
drivers/gpu/drm/i915/i915_reg.h | 23 ++++++++++++++++
drivers/gpu/drm/i915/intel_psr.c | 45 ++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2dad655a710c..073b4502b30a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4095,6 +4095,29 @@ enum {
#define EDP_PSR2_IDLE_FRAME_MASK 0xf
#define EDP_PSR2_IDLE_FRAME_SHIFT 0
+#define _PSR_EVENT_TRANS_A 0x60848
+#define _PSR_EVENT_TRANS_B 0x61848
+#define _PSR_EVENT_TRANS_C 0x62848
+#define _PSR_EVENT_TRANS_D 0x63848
+#define _PSR_EVENT_TRANS_EDP 0x6F848
+#define PSR_EVENT(trans) (trans == TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans, _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
+#define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
+#define PSR_EVENT_PSR2_DISABLED (1 << 16)
+#define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
+#define PSR_EVENT_SU_CRC_FIFO_UNDERRUN (1 << 14)
+#define PSR_EVENT_GRAPHICS_RESET (1 << 12)
+#define PSR_EVENT_PCH_INTERRUPT (1 << 11)
+#define PSR_EVENT_MEMORY_UP (1 << 10)
+#define PSR_EVENT_FRONT_BUFFER_MODIFY (1 << 9)
+#define PSR_EVENT_WD_TIMER_EXPIRE (1 << 8)
+#define PSR_EVENT_PIPE_REGISTERS_UPDATE (1 << 6)
+#define PSR_EVENT_REGISTER_UPDATE (1 << 5)
+#define PSR_EVENT_HDCP_ENABLE (1 << 4)
+#define PSR_EVENT_KVMR_SESSION_ENABLE (1 << 3)
+#define PSR_EVENT_VBI_ENABLE (1 << 2)
+#define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
+#define PSR_EVENT_PSR_DISABLE (1 << 0)
+
#define EDP_PSR2_STATUS _MMIO(0x6f940)
#define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
#define EDP_PSR2_STATUS_STATE_SHIFT 28
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0d548292dd09..0938df48107a 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -125,6 +125,43 @@ void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
I915_WRITE(EDP_PSR_IMR, ~mask);
}
+static void psr_event_print(u32 val, bool psr2_enabled)
+{
+ DRM_DEBUG_KMS("PSR exit causes: 0x%x\n", val);
+ if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE)
+ DRM_DEBUG_KMS("\tPSR2 watchdog timer expired\n");
+ if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled)
+ DRM_DEBUG_KMS("\tPSR2 disabled\n");
+ if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN)
+ DRM_DEBUG_KMS("\tSU dirty FIFO underrun\n");
+ if (val & PSR_EVENT_SU_CRC_FIFO_UNDERRUN)
+ DRM_DEBUG_KMS("\tSU CRC FIFO underrun\n");
+ if (val & PSR_EVENT_GRAPHICS_RESET)
+ DRM_DEBUG_KMS("\tGraphics reset\n");
+ if (val & PSR_EVENT_PCH_INTERRUPT)
+ DRM_DEBUG_KMS("\tPCH interrupt\n");
+ if (val & PSR_EVENT_MEMORY_UP)
+ DRM_DEBUG_KMS("\tMemory up\n");
+ if (val & PSR_EVENT_FRONT_BUFFER_MODIFY)
+ DRM_DEBUG_KMS("\tFront buffer modification\n");
+ if (val & PSR_EVENT_WD_TIMER_EXPIRE)
+ DRM_DEBUG_KMS("\tPSR watchdog timer expired\n");
+ if (val & PSR_EVENT_PIPE_REGISTERS_UPDATE)
+ DRM_DEBUG_KMS("\tPIPE registers updated\n");
+ if (val & PSR_EVENT_REGISTER_UPDATE)
+ DRM_DEBUG_KMS("\tRegister updated\n");
+ if (val & PSR_EVENT_HDCP_ENABLE)
+ DRM_DEBUG_KMS("\tHDCP enabled\n");
+ if (val & PSR_EVENT_KVMR_SESSION_ENABLE)
+ DRM_DEBUG_KMS("\tKVMR session enabled\n");
+ if (val & PSR_EVENT_VBI_ENABLE)
+ DRM_DEBUG_KMS("\tVBI enabled\n");
+ if (val & PSR_EVENT_LPSP_MODE_EXIT)
+ DRM_DEBUG_KMS("\tLPSP mode exited\n");
+ if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled)
+ DRM_DEBUG_KMS("\tPSR disabled\n");
+}
+
void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
{
u32 transcoders = BIT(TRANSCODER_EDP);
@@ -152,6 +189,14 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
dev_priv->psr.last_exit = time_ns;
DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
transcoder_name(cpu_transcoder));
+
+ if (INTEL_GEN(dev_priv) >= 9) {
+ u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
+ bool psr2_enabled = dev_priv->psr.psr2_enabled;
+
+ psr_event_print(val, psr2_enabled);
+ I915_WRITE(PSR_EVENT(cpu_transcoder), val);
+ }
}
}
}
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
@ 2018-04-20 22:27 ` José Roberto de Souza
2018-04-20 22:57 ` Rodrigo Vivi
2018-04-20 22:58 ` Rodrigo Vivi
2018-04-20 22:27 ` [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status José Roberto de Souza
` (5 subsequent siblings)
6 siblings, 2 replies; 18+ messages in thread
From: José Roberto de Souza @ 2018-04-20 22:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Dhinakaran Pandiyan, Rodrigo Vivi
Any write in any display register was causing HW to exit PSR,
masking it to allow more power savings. Writes to pipe related
registers will still cause HW to exit PSR.
This is already masked for PSR2.
Bspec: 7721 and 8042
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
New patch in this series.
drivers/gpu/drm/i915/intel_psr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0938df48107a..c907282dc82d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
I915_WRITE(EDP_PSR_DEBUG,
EDP_PSR_DEBUG_MASK_MEMUP |
EDP_PSR_DEBUG_MASK_HPD |
- EDP_PSR_DEBUG_MASK_LPSP);
+ EDP_PSR_DEBUG_MASK_LPSP |
+ EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
}
}
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
@ 2018-04-20 22:27 ` José Roberto de Souza
2018-04-25 0:18 ` Dhinakaran Pandiyan
2018-04-20 22:27 ` [PATCH v3 4/4] drm/i915/psr/cnl: Set y-coordinate as valid in SDP José Roberto de Souza
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: José Roberto de Souza @ 2018-04-20 22:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
IGT tests could be improved with sink status, knowing for sure that
hardware have activate or exit PSR.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
No changes since v2, Dhinakaran asked to not merge this patch in v2
because reading i915_edp_psr_status was causing PSR to exit but now
with 'drm/i915/psr: Prevent PSR exit when a non-pipe related register
is written' it is fixed.
drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2f05f5262bba..536d93322451 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
return "unknown";
}
+static const char *psr_sink_status(u8 val)
+{
+ static const char * const sink_status[] = {
+ "inactive",
+ "transition to active, capture and display",
+ "active, display from RFB",
+ "active, capture and display on sink device timings",
+ "transition to inactive, capture and display, timing re-sync",
+ "reserved",
+ "reserved",
+ "sink internal error"
+ };
+
+ val &= DP_PSR_SINK_STATE_MASK;
+ if (val < ARRAY_SIZE(sink_status))
+ return sink_status[val];
+
+ return "unknown";
+}
+
static int i915_edp_psr_status(struct seq_file *m, void *data)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
psr2, psr2_live_status(psr2));
}
+
+ if (dev_priv->psr.enabled) {
+ struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
+ u8 val;
+
+ if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) == 1)
+ seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
+ psr_sink_status(val));
+ }
mutex_unlock(&dev_priv->psr.lock);
if (READ_ONCE(dev_priv->psr.debug)) {
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 4/4] drm/i915/psr/cnl: Set y-coordinate as valid in SDP
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
2018-04-20 22:27 ` [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status José Roberto de Souza
@ 2018-04-20 22:27 ` José Roberto de Souza
2018-04-20 22:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit Patchwork
` (3 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: José Roberto de Souza @ 2018-04-20 22:27 UTC (permalink / raw)
To: intel-gfx; +Cc: Rodrigo Vivi
This was my bad, spec says that the name of this bit is
'Y-coordinate valid' but the values for it is:
0: Include Y-coordinate valid eDP1.4a
1: Do not include Y-coordinate valid eDP 1.4
So not setting it.
BSpec: 7713
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
No changes since v2.
drivers/gpu/drm/i915/intel_psr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c907282dc82d..7b928eb9ad35 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -508,9 +508,8 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
* mesh at all with our frontbuffer tracking. And the hw alone isn't
* good enough. */
val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
- if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
- val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
- }
+ if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
+ val |= EDP_Y_COORDINATE_ENABLE;
val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
--
2.17.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 18+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
` (2 preceding siblings ...)
2018-04-20 22:27 ` [PATCH v3 4/4] drm/i915/psr/cnl: Set y-coordinate as valid in SDP José Roberto de Souza
@ 2018-04-20 22:44 ` Patchwork
2018-04-20 23:04 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-04-20 22:44 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
URL : https://patchwork.freedesktop.org/series/42058/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
ac17ffd9158a drm/i915/psr/skl+: Print information about what caused a PSR exit
-:32: WARNING:LONG_LINE: line over 100 characters
#32: FILE: drivers/gpu/drm/i915/i915_reg.h:4103:
+#define PSR_EVENT(trans) (trans == TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans, _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
-:32: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'trans' - possible side-effects?
#32: FILE: drivers/gpu/drm/i915/i915_reg.h:4103:
+#define PSR_EVENT(trans) (trans == TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans, _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
total: 0 errors, 1 warnings, 1 checks, 86 lines checked
e44dc65462c8 drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
60be21d471f0 drm/i915/debugfs: Print sink PSR status
f1c8e186effd drm/i915/psr/cnl: Set y-coordinate as valid in SDP
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
@ 2018-04-20 22:57 ` Rodrigo Vivi
2018-04-24 0:42 ` Souza, Jose
2018-04-20 22:58 ` Rodrigo Vivi
1 sibling, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-04-20 22:57 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza wrote:
> Any write in any display register was causing HW to exit PSR,
> masking it to allow more power savings. Writes to pipe related
> registers will still cause HW to exit PSR.
> This is already masked for PSR2.
This seems a good idea indeed with the test case on perspective.
But it needs more tests to make sure it doesn't break
"Display WA #0884: all"
Or we might need to revert that patch before moving with this idea.
>
> Bspec: 7721 and 8042
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> New patch in this series.
>
> drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0938df48107a..c907282dc82d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> I915_WRITE(EDP_PSR_DEBUG,
> EDP_PSR_DEBUG_MASK_MEMUP |
> EDP_PSR_DEBUG_MASK_HPD |
> - EDP_PSR_DEBUG_MASK_LPSP);
> + EDP_PSR_DEBUG_MASK_LPSP |
> + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> }
> }
>
> --
> 2.17.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
2018-04-20 22:57 ` Rodrigo Vivi
@ 2018-04-20 22:58 ` Rodrigo Vivi
1 sibling, 0 replies; 18+ messages in thread
From: Rodrigo Vivi @ 2018-04-20 22:58 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Dhinakaran Pandiyan
On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza wrote:
> Any write in any display register was causing HW to exit PSR,
> masking it to allow more power savings. Writes to pipe related
> registers will still cause HW to exit PSR.
> This is already masked for PSR2.
>
> Bspec: 7721 and 8042
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> New patch in this series.
>
> drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0938df48107a..c907282dc82d 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp,
> I915_WRITE(EDP_PSR_DEBUG,
> EDP_PSR_DEBUG_MASK_MEMUP |
> EDP_PSR_DEBUG_MASK_HPD |
> - EDP_PSR_DEBUG_MASK_LPSP);
> + EDP_PSR_DEBUG_MASK_LPSP |
> + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
What about only setting this bit before debugfs reads
and unseting it after that?
> }
> }
>
> --
> 2.17.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
` (3 preceding siblings ...)
2018-04-20 22:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit Patchwork
@ 2018-04-20 23:04 ` Patchwork
2018-04-20 23:50 ` ✓ Fi.CI.IGT: " Patchwork
2018-04-24 23:47 ` [PATCH v3 1/4] " Dhinakaran Pandiyan
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-04-20 23:04 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
URL : https://patchwork.freedesktop.org/series/42058/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4073 -> Patchwork_8769 =
== Summary - SUCCESS ==
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/42058/revisions/1/mbox/
== Known issues ==
Here are the changes found in Patchwork_8769 that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-cfl-s3: PASS -> FAIL (fdo#103928, fdo#100368)
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
fi-snb-2520m: PASS -> INCOMPLETE (fdo#103713)
==== Possible fixes ====
igt@gem_exec_suspend@basic-s4-devices:
fi-kbl-7500u: DMESG-WARN (fdo#105128) -> PASS
igt@kms_flip@basic-flip-vs-wf_vblank:
fi-cnl-psr: FAIL (fdo#103928, fdo#100368) -> PASS
igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
fi-cnl-psr: FAIL (fdo#103481) -> PASS
igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
fi-ivb-3520m: DMESG-WARN (fdo#106084) -> PASS +1
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#106084 https://bugs.freedesktop.org/show_bug.cgi?id=106084
== Participating hosts (35 -> 33) ==
Missing (2): fi-ilk-m540 fi-skl-6700hq
== Build changes ==
* Linux: CI_DRM_4073 -> Patchwork_8769
CI_DRM_4073: 071ca2e05b60374c8bcac11e181018dc12b101b9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4443: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8769: f1c8e186effddd1bfd45adc03da70b5b9535f8ec @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4443: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Linux commits ==
f1c8e186effd drm/i915/psr/cnl: Set y-coordinate as valid in SDP
60be21d471f0 drm/i915/debugfs: Print sink PSR status
e44dc65462c8 drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
ac17ffd9158a drm/i915/psr/skl+: Print information about what caused a PSR exit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8769/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* ✓ Fi.CI.IGT: success for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
` (4 preceding siblings ...)
2018-04-20 23:04 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-04-20 23:50 ` Patchwork
2018-04-24 23:47 ` [PATCH v3 1/4] " Dhinakaran Pandiyan
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2018-04-20 23:50 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
URL : https://patchwork.freedesktop.org/series/42058/
State : success
== Summary ==
= CI Bug Log - changes from CI_DRM_4073_full -> Patchwork_8769_full =
== Summary - WARNING ==
Minor unknown changes coming with Patchwork_8769_full need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_8769_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/42058/revisions/1/mbox/
== Possible new issues ==
Here are the unknown changes that may have been introduced in Patchwork_8769_full:
=== IGT changes ===
==== Warnings ====
igt@gem_exec_schedule@deep-bsd2:
shard-kbl: SKIP -> PASS
igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
shard-kbl: PASS -> SKIP
igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
shard-glk: PASS -> SKIP +111
igt@kms_vblank@pipe-c-wait-forked-busy:
shard-glk: SKIP -> PASS +85
== Known issues ==
Here are the changes found in Patchwork_8769_full that come from known issues:
=== IGT changes ===
==== Issues hit ====
igt@kms_flip@flip-vs-expired-vblank-interruptible:
shard-glk: PASS -> FAIL (fdo#102887)
igt@kms_setmode@basic:
shard-kbl: PASS -> FAIL (fdo#99912)
igt@kms_sysfs_edid_timing:
shard-apl: PASS -> WARN (fdo#100047)
igt@kms_vblank@pipe-a-accuracy-idle:
shard-hsw: PASS -> FAIL (fdo#102583)
==== Possible fixes ====
igt@gem_pwrite_pread@display-pwrite-blt-gtt_mmap-performance:
shard-kbl: DMESG-WARN (fdo#105602, fdo#103558) -> PASS
igt@kms_flip@2x-blocking-wf_vblank:
shard-hsw: FAIL (fdo#100368) -> PASS
igt@kms_setmode@basic:
shard-glk: FAIL (fdo#99912) -> PASS
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
== Participating hosts (6 -> 5) ==
Missing (1): shard-glkb
== Build changes ==
* Linux: CI_DRM_4073 -> Patchwork_8769
CI_DRM_4073: 071ca2e05b60374c8bcac11e181018dc12b101b9 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4443: 8168bb65d5e64d4df4e5d847d448bab2d2825d73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_8769: f1c8e186effddd1bfd45adc03da70b5b9535f8ec @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4443: a2f486679f467cd6e82578384f56d4aabaa8cf2e @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8769/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-20 22:57 ` Rodrigo Vivi
@ 2018-04-24 0:42 ` Souza, Jose
2018-04-24 21:20 ` Rodrigo Vivi
0 siblings, 1 reply; 18+ messages in thread
From: Souza, Jose @ 2018-04-24 0:42 UTC (permalink / raw)
To: Vivi, Rodrigo; +Cc: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran
On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> wrote:
> > Any write in any display register was causing HW to exit PSR,
> > masking it to allow more power savings. Writes to pipe related
> > registers will still cause HW to exit PSR.
> > This is already masked for PSR2.
>
> This seems a good idea indeed with the test case on perspective.
what test cases are thinking? the current ones already do pages flips
that will only touch the pipe related registers.
>
> But it needs more tests to make sure it doesn't break
> "Display WA #0884: all"
I just tested the WA #0884 and it still causes PSR to exit. I have
added a new debugfs that when read it writes to
'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
>
> Or we might need to revert that patch before moving with this idea.
>
> >
> > Bspec: 7721 and 8042
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >
> > New patch in this series.
> >
> > drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 0938df48107a..c907282dc82d 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > intel_dp *intel_dp,
> > I915_WRITE(EDP_PSR_DEBUG,
> > EDP_PSR_DEBUG_MASK_MEMUP |
> > EDP_PSR_DEBUG_MASK_HPD |
> > - EDP_PSR_DEBUG_MASK_LPSP);
> > + EDP_PSR_DEBUG_MASK_LPSP |
> > + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > }
> > }
> >
> > --
> > 2.17.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-24 0:42 ` Souza, Jose
@ 2018-04-24 21:20 ` Rodrigo Vivi
2018-04-25 0:16 ` Dhinakaran Pandiyan
0 siblings, 1 reply; 18+ messages in thread
From: Rodrigo Vivi @ 2018-04-24 21:20 UTC (permalink / raw)
To: Souza, Jose; +Cc: intel-gfx@lists.freedesktop.org, Pandiyan, Dhinakaran
On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > wrote:
> > > Any write in any display register was causing HW to exit PSR,
> > > masking it to allow more power savings. Writes to pipe related
> > > registers will still cause HW to exit PSR.
> > > This is already masked for PSR2.
> >
> > This seems a good idea indeed with the test case on perspective.
>
> what test cases are thinking? the current ones already do pages flips
> that will only touch the pipe related registers.
>
> >
> > But it needs more tests to make sure it doesn't break
> > "Display WA #0884: all"
>
> I just tested the WA #0884 and it still causes PSR to exit. I have
> added a new debugfs that when read it writes to
> 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
Interesting. Thanks a lot for checking this.
I guess we are safe then. DK?!
>
> >
> > Or we might need to revert that patch before moving with this idea.
> >
> > >
> > > Bspec: 7721 and 8042
> > >
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >
> > > New patch in this series.
> > >
> > > drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 0938df48107a..c907282dc82d 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > intel_dp *intel_dp,
> > > I915_WRITE(EDP_PSR_DEBUG,
> > > EDP_PSR_DEBUG_MASK_MEMUP |
> > > EDP_PSR_DEBUG_MASK_HPD |
> > > - EDP_PSR_DEBUG_MASK_LPSP);
> > > + EDP_PSR_DEBUG_MASK_LPSP |
> > > + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > > }
> > > }
> > >
> > > --
> > > 2.17.0
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
` (5 preceding siblings ...)
2018-04-20 23:50 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-04-24 23:47 ` Dhinakaran Pandiyan
2018-04-24 23:57 ` Dhinakaran Pandiyan
2018-04-25 20:27 ` Souza, Jose
6 siblings, 2 replies; 18+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-24 23:47 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Rodrigo Vivi
On Fri, 2018-04-20 at 15:27 -0700, José Roberto de Souza wrote:
> This will be helpful to debug what hardware is actually tracking
> and causing PSR to exit.
>
> BSpec: 7721
>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>
> New patch in this series.
>
> drivers/gpu/drm/i915/i915_reg.h | 23 ++++++++++++++++
> drivers/gpu/drm/i915/intel_psr.c | 45 ++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 2dad655a710c..073b4502b30a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4095,6 +4095,29 @@ enum {
> #define EDP_PSR2_IDLE_FRAME_MASK 0xf
> #define EDP_PSR2_IDLE_FRAME_SHIFT 0
>
> +#define _PSR_EVENT_TRANS_A 0x60848
> +#define _PSR_EVENT_TRANS_B 0x61848
> +#define _PSR_EVENT_TRANS_C 0x62848
> +#define _PSR_EVENT_TRANS_D 0x63848
> +#define _PSR_EVENT_TRANS_EDP 0x6F848
> +#define PSR_EVENT(trans) (trans == TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans, _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
You get this for free using _MMIO_TRANS2(), see TRANS_DDI_FUNC_CTL
> +#define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
> +#define PSR_EVENT_PSR2_DISABLED (1 << 16)
> +#define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
> +#define PSR_EVENT_SU_CRC_FIFO_UNDERRUN (1 << 14)
> +#define PSR_EVENT_GRAPHICS_RESET (1 << 12)
> +#define PSR_EVENT_PCH_INTERRUPT (1 << 11)
> +#define PSR_EVENT_MEMORY_UP (1 << 10)
> +#define PSR_EVENT_FRONT_BUFFER_MODIFY (1 << 9)
> +#define PSR_EVENT_WD_TIMER_EXPIRE (1 << 8)
> +#define PSR_EVENT_PIPE_REGISTERS_UPDATE (1 << 6)
> +#define PSR_EVENT_REGISTER_UPDATE (1 << 5)
> +#define PSR_EVENT_HDCP_ENABLE (1 << 4)
> +#define PSR_EVENT_KVMR_SESSION_ENABLE (1 << 3)
> +#define PSR_EVENT_VBI_ENABLE (1 << 2)
> +#define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
> +#define PSR_EVENT_PSR_DISABLE (1 << 0)
> +
> #define EDP_PSR2_STATUS _MMIO(0x6f940)
> #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> #define EDP_PSR2_STATUS_STATE_SHIFT 28
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 0d548292dd09..0938df48107a 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -125,6 +125,43 @@ void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
> I915_WRITE(EDP_PSR_IMR, ~mask);
> }
>
> +static void psr_event_print(u32 val, bool psr2_enabled)
Is psr2_enabled needed? Do the bits get set incorrectly?
> +{
> + DRM_DEBUG_KMS("PSR exit causes: 0x%x\n", val);
How about s/causes/events ?
> + if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE)
> + DRM_DEBUG_KMS("\tPSR2 watchdog timer expired\n");
> + if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled)
> + DRM_DEBUG_KMS("\tPSR2 disabled\n");
> + if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN)
> + DRM_DEBUG_KMS("\tSU dirty FIFO underrun\n");
> + if (val & PSR_EVENT_SU_CRC_FIFO_UNDERRUN)
> + DRM_DEBUG_KMS("\tSU CRC FIFO underrun\n");
> + if (val & PSR_EVENT_GRAPHICS_RESET)
> + DRM_DEBUG_KMS("\tGraphics reset\n");
> + if (val & PSR_EVENT_PCH_INTERRUPT)
> + DRM_DEBUG_KMS("\tPCH interrupt\n");
> + if (val & PSR_EVENT_MEMORY_UP)
> + DRM_DEBUG_KMS("\tMemory up\n");
> + if (val & PSR_EVENT_FRONT_BUFFER_MODIFY)
> + DRM_DEBUG_KMS("\tFront buffer modification\n");
> + if (val & PSR_EVENT_WD_TIMER_EXPIRE)
> + DRM_DEBUG_KMS("\tPSR watchdog timer expired\n");
> + if (val & PSR_EVENT_PIPE_REGISTERS_UPDATE)
> + DRM_DEBUG_KMS("\tPIPE registers updated\n");
> + if (val & PSR_EVENT_REGISTER_UPDATE)
> + DRM_DEBUG_KMS("\tRegister updated\n");
> + if (val & PSR_EVENT_HDCP_ENABLE)
> + DRM_DEBUG_KMS("\tHDCP enabled\n");
> + if (val & PSR_EVENT_KVMR_SESSION_ENABLE)
> + DRM_DEBUG_KMS("\tKVMR session enabled\n");
> + if (val & PSR_EVENT_VBI_ENABLE)
> + DRM_DEBUG_KMS("\tVBI enabled\n");
> + if (val & PSR_EVENT_LPSP_MODE_EXIT)
> + DRM_DEBUG_KMS("\tLPSP mode exited\n");
> + if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled)
> + DRM_DEBUG_KMS("\tPSR disabled\n");
> +}
> +
> void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> {
> u32 transcoders = BIT(TRANSCODER_EDP);
> @@ -152,6 +189,14 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> dev_priv->psr.last_exit = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> transcoder_name(cpu_transcoder));
This line can be removed now.
> +
> + if (INTEL_GEN(dev_priv) >= 9) {
> + u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
> + bool psr2_enabled = dev_priv->psr.psr2_enabled;
> +
> + psr_event_print(val, psr2_enabled);
> + I915_WRITE(PSR_EVENT(cpu_transcoder), val);
nit: How about printing the debug after the write? It is probably better
to reset the event bits as soon as we've done the read.
So this write will cause a second PSR exit. How about making this the
second patch?
> + }
> }
> }
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-24 23:47 ` [PATCH v3 1/4] " Dhinakaran Pandiyan
@ 2018-04-24 23:57 ` Dhinakaran Pandiyan
2018-04-25 20:27 ` Souza, Jose
1 sibling, 0 replies; 18+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-24 23:57 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Rodrigo Vivi
On Tue, 2018-04-24 at 16:47 -0700, Dhinakaran Pandiyan wrote:
>
>
> On Fri, 2018-04-20 at 15:27 -0700, José Roberto de Souza wrote:
> > This will be helpful to debug what hardware is actually tracking
> > and causing PSR to exit.
> >
> > BSpec: 7721
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >
> > New patch in this series.
> >
> > drivers/gpu/drm/i915/i915_reg.h | 23 ++++++++++++++++
> > drivers/gpu/drm/i915/intel_psr.c | 45 ++++++++++++++++++++++++++++++++
> > 2 files changed, 68 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 2dad655a710c..073b4502b30a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4095,6 +4095,29 @@ enum {
> > #define EDP_PSR2_IDLE_FRAME_MASK 0xf
> > #define EDP_PSR2_IDLE_FRAME_SHIFT 0
> >
> > +#define _PSR_EVENT_TRANS_A 0x60848
> > +#define _PSR_EVENT_TRANS_B 0x61848
> > +#define _PSR_EVENT_TRANS_C 0x62848
> > +#define _PSR_EVENT_TRANS_D 0x63848
> > +#define _PSR_EVENT_TRANS_EDP 0x6F848
> > +#define PSR_EVENT(trans) (trans == TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans, _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
>
> You get this for free using _MMIO_TRANS2(), see TRANS_DDI_FUNC_CTL
>
> > +#define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
> > +#define PSR_EVENT_PSR2_DISABLED (1 << 16)
> > +#define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
> > +#define PSR_EVENT_SU_CRC_FIFO_UNDERRUN (1 << 14)
> > +#define PSR_EVENT_GRAPHICS_RESET (1 << 12)
> > +#define PSR_EVENT_PCH_INTERRUPT (1 << 11)
> > +#define PSR_EVENT_MEMORY_UP (1 << 10)
> > +#define PSR_EVENT_FRONT_BUFFER_MODIFY (1 << 9)
> > +#define PSR_EVENT_WD_TIMER_EXPIRE (1 << 8)
> > +#define PSR_EVENT_PIPE_REGISTERS_UPDATE (1 << 6)
> > +#define PSR_EVENT_REGISTER_UPDATE (1 << 5)
> > +#define PSR_EVENT_HDCP_ENABLE (1 << 4)
> > +#define PSR_EVENT_KVMR_SESSION_ENABLE (1 << 3)
> > +#define PSR_EVENT_VBI_ENABLE (1 << 2)
> > +#define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
> > +#define PSR_EVENT_PSR_DISABLE (1 << 0)
> > +
> > #define EDP_PSR2_STATUS _MMIO(0x6f940)
> > #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> > #define EDP_PSR2_STATUS_STATE_SHIFT 28
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 0d548292dd09..0938df48107a 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -125,6 +125,43 @@ void intel_psr_irq_control(struct drm_i915_private *dev_priv, bool debug)
> > I915_WRITE(EDP_PSR_IMR, ~mask);
> > }
> >
> > +static void psr_event_print(u32 val, bool psr2_enabled)
>
> Is psr2_enabled needed? Do the bits get set incorrectly?
>
> > +{
> > + DRM_DEBUG_KMS("PSR exit causes: 0x%x\n", val);
> How about s/causes/events ?
> > + if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE)
> > + DRM_DEBUG_KMS("\tPSR2 watchdog timer expired\n");
> > + if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled)
> > + DRM_DEBUG_KMS("\tPSR2 disabled\n");
> > + if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN)
> > + DRM_DEBUG_KMS("\tSU dirty FIFO underrun\n");
> > + if (val & PSR_EVENT_SU_CRC_FIFO_UNDERRUN)
> > + DRM_DEBUG_KMS("\tSU CRC FIFO underrun\n");
> > + if (val & PSR_EVENT_GRAPHICS_RESET)
> > + DRM_DEBUG_KMS("\tGraphics reset\n");
> > + if (val & PSR_EVENT_PCH_INTERRUPT)
> > + DRM_DEBUG_KMS("\tPCH interrupt\n");
> > + if (val & PSR_EVENT_MEMORY_UP)
> > + DRM_DEBUG_KMS("\tMemory up\n");
> > + if (val & PSR_EVENT_FRONT_BUFFER_MODIFY)
> > + DRM_DEBUG_KMS("\tFront buffer modification\n");
> > + if (val & PSR_EVENT_WD_TIMER_EXPIRE)
> > + DRM_DEBUG_KMS("\tPSR watchdog timer expired\n");
> > + if (val & PSR_EVENT_PIPE_REGISTERS_UPDATE)
> > + DRM_DEBUG_KMS("\tPIPE registers updated\n");
> > + if (val & PSR_EVENT_REGISTER_UPDATE)
> > + DRM_DEBUG_KMS("\tRegister updated\n");
> > + if (val & PSR_EVENT_HDCP_ENABLE)
> > + DRM_DEBUG_KMS("\tHDCP enabled\n");
> > + if (val & PSR_EVENT_KVMR_SESSION_ENABLE)
> > + DRM_DEBUG_KMS("\tKVMR session enabled\n");
> > + if (val & PSR_EVENT_VBI_ENABLE)
> > + DRM_DEBUG_KMS("\tVBI enabled\n");
> > + if (val & PSR_EVENT_LPSP_MODE_EXIT)
> > + DRM_DEBUG_KMS("\tLPSP mode exited\n");
> > + if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled)
> > + DRM_DEBUG_KMS("\tPSR disabled\n");
> > +}
> > +
> > void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> > {
> > u32 transcoders = BIT(TRANSCODER_EDP);
> > @@ -152,6 +189,14 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> > dev_priv->psr.last_exit = time_ns;
> > DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> > transcoder_name(cpu_transcoder));
>
> This line can be removed now.
>
> > +
> > + if (INTEL_GEN(dev_priv) >= 9) {
> > + u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
> > + bool psr2_enabled = dev_priv->psr.psr2_enabled;
> > +
> > + psr_event_print(val, psr2_enabled);
> > + I915_WRITE(PSR_EVENT(cpu_transcoder), val);
>
> nit: How about printing the debug after the write? It is probably better
> to reset the event bits as soon as we've done the read.
>
>
> So this write will cause a second PSR exit. How about making this the
> second patch?
To be clear, after patch 2/4 "drm/i915/psr: Prevent PSR exit when a
non-pipe related register is written"
>
> > + }
> > }
> > }
> > }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-24 21:20 ` Rodrigo Vivi
@ 2018-04-25 0:16 ` Dhinakaran Pandiyan
2018-04-25 20:37 ` Souza, Jose
0 siblings, 1 reply; 18+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-25 0:16 UTC (permalink / raw)
To: Rodrigo Vivi; +Cc: intel-gfx@lists.freedesktop.org
On Tue, 2018-04-24 at 14:20 -0700, Rodrigo Vivi wrote:
> On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> > On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > > wrote:
> > > > Any write in any display register was causing HW to exit PSR,
> > > > masking it to allow more power savings. Writes to pipe related
> > > > registers will still cause HW to exit PSR.
> > > > This is already masked for PSR2.
> > >
> > > This seems a good idea indeed with the test case on perspective.
> >
> > what test cases are thinking? the current ones already do pages flips
> > that will only touch the pipe related registers.
> >
> > >
> > > But it needs more tests to make sure it doesn't break
> > > "Display WA #0884: all"
> >
> > I just tested the WA #0884 and it still causes PSR to exit. I have
> > added a new debugfs that when read it writes to
> > 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
>
> Interesting. Thanks a lot for checking this.
> I guess we are safe then. DK?!
>
Not sure why it works though, let's give it a try.
It might be worth adding more information about test platform and how
was this was tested in the commit message. Since this is not clearly
documented in bspec, someone is going read this in a few months and
think the code does not make any sense.
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> >
> > >
> > > Or we might need to revert that patch before moving with this idea.
> > >
> > > >
> > > > Bspec: 7721 and 8042
> > > >
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > ---
> > > >
> > > > New patch in this series.
> > > >
> > > > drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > index 0938df48107a..c907282dc82d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > > intel_dp *intel_dp,
> > > > I915_WRITE(EDP_PSR_DEBUG,
> > > > EDP_PSR_DEBUG_MASK_MEMUP |
> > > > EDP_PSR_DEBUG_MASK_HPD |
> > > > - EDP_PSR_DEBUG_MASK_LPSP);
> > > > + EDP_PSR_DEBUG_MASK_LPSP |
> > > > + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> > > > }
> > > > }
> > > >
> > > > --
> > > > 2.17.0
> > > >
> _______________________________________________
> 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] 18+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status
2018-04-20 22:27 ` [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status José Roberto de Souza
@ 2018-04-25 0:18 ` Dhinakaran Pandiyan
2018-04-25 20:33 ` Souza, Jose
0 siblings, 1 reply; 18+ messages in thread
From: Dhinakaran Pandiyan @ 2018-04-25 0:18 UTC (permalink / raw)
To: José Roberto de Souza; +Cc: intel-gfx, Rodrigo Vivi
On Fri, 2018-04-20 at 15:27 -0700, José Roberto de Souza wrote:
> IGT tests could be improved with sink status, knowing for sure that
> hardware have activate or exit PSR.
>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>
> No changes since v2, Dhinakaran asked to not merge this patch in v2
> because reading i915_edp_psr_status was causing PSR to exit but now
> with 'drm/i915/psr: Prevent PSR exit when a non-pipe related register
> is written' it is fixed.
>
Do you mind adding "reading i915_edp_psr_status was causing PSR to exit
but now with 'drm/i915/psr: Prevent PSR exit when a non-pipe related
register is written' it is fixed." to the commit message?
> drivers/gpu/drm/i915/i915_debugfs.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 2f05f5262bba..536d93322451 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
> return "unknown";
> }
>
> +static const char *psr_sink_status(u8 val)
> +{
> + static const char * const sink_status[] = {
> + "inactive",
> + "transition to active, capture and display",
> + "active, display from RFB",
> + "active, capture and display on sink device timings",
> + "transition to inactive, capture and display, timing re-sync",
> + "reserved",
> + "reserved",
> + "sink internal error"
> + };
> +
> + val &= DP_PSR_SINK_STATE_MASK;
> + if (val < ARRAY_SIZE(sink_status))
> + return sink_status[val];
> +
> + return "unknown";
> +}
> +
> static int i915_edp_psr_status(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
> seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> psr2, psr2_live_status(psr2));
> }
> +
> + if (dev_priv->psr.enabled) {
> + struct drm_dp_aux *aux = &dev_priv->psr.enabled->aux;
> + u8 val;
> +
> + if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) == 1)
> + seq_printf(m, "Sink PSR status: 0x%x [%s]\n", val,
> + psr_sink_status(val));
> + }
> mutex_unlock(&dev_priv->psr.lock);
>
> if (READ_ONCE(dev_priv->psr.debug)) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit
2018-04-24 23:47 ` [PATCH v3 1/4] " Dhinakaran Pandiyan
2018-04-24 23:57 ` Dhinakaran Pandiyan
@ 2018-04-25 20:27 ` Souza, Jose
1 sibling, 0 replies; 18+ messages in thread
From: Souza, Jose @ 2018-04-25 20:27 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo
On Tue, 2018-04-24 at 16:47 -0700, Dhinakaran Pandiyan wrote:
>
>
> On Fri, 2018-04-20 at 15:27 -0700, José Roberto de Souza wrote:
> > This will be helpful to debug what hardware is actually tracking
> > and causing PSR to exit.
> >
> > BSpec: 7721
> >
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >
> > New patch in this series.
> >
> > drivers/gpu/drm/i915/i915_reg.h | 23 ++++++++++++++++
> > drivers/gpu/drm/i915/intel_psr.c | 45
> > ++++++++++++++++++++++++++++++++
> > 2 files changed, 68 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 2dad655a710c..073b4502b30a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4095,6 +4095,29 @@ enum {
> > #define EDP_PSR2_IDLE_FRAME_MASK 0xf
> > #define EDP_PSR2_IDLE_FRAME_SHIFT 0
> >
> > +#define _PSR_EVENT_TRANS_A 0x60848
> > +#define _PSR_EVENT_TRANS_B 0x61848
> > +#define _PSR_EVENT_TRANS_C 0x62848
> > +#define _PSR_EVENT_TRANS_D 0x63848
> > +#define _PSR_EVENT_TRANS_EDP 0x6F848
> > +#define PSR_EVENT(trans) (trans ==
> > TRANSCODER_EDP ? _MMIO(_PSR_EVENT_TRANS_EDP) : _MMIO_PORT(trans,
> > _PSR_EVENT_TRANS_A, _PSR_EVENT_TRANS_B))
>
> You get this for free using _MMIO_TRANS2(), see TRANS_DDI_FUNC_CTL
Oh nice, thanks
>
> > +#define PSR_EVENT_PSR2_WD_TIMER_EXPIRE (1 << 17)
> > +#define PSR_EVENT_PSR2_DISABLED (1 << 16)
> > +#define PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN (1 << 15)
> > +#define PSR_EVENT_SU_CRC_FIFO_UNDERRUN (1 << 14)
> > +#define PSR_EVENT_GRAPHICS_RESET (1 << 12)
> > +#define PSR_EVENT_PCH_INTERRUPT (1 << 11)
> > +#define PSR_EVENT_MEMORY_UP (1 << 10)
> > +#define PSR_EVENT_FRONT_BUFFER_MODIFY (1 << 9)
> > +#define PSR_EVENT_WD_TIMER_EXPIRE (1 << 8)
> > +#define PSR_EVENT_PIPE_REGISTERS_UPDATE (1 << 6)
> > +#define PSR_EVENT_REGISTER_UPDATE (1 << 5)
> > +#define PSR_EVENT_HDCP_ENABLE (1 << 4)
> > +#define PSR_EVENT_KVMR_SESSION_ENABLE (1 << 3)
> > +#define PSR_EVENT_VBI_ENABLE (1 << 2)
> > +#define PSR_EVENT_LPSP_MODE_EXIT (1 << 1)
> > +#define PSR_EVENT_PSR_DISABLE (1 << 0)
> > +
> > #define EDP_PSR2_STATUS _MMIO(0x6f940)
> > #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
> > #define EDP_PSR2_STATUS_STATE_SHIFT 28
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 0d548292dd09..0938df48107a 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -125,6 +125,43 @@ void intel_psr_irq_control(struct
> > drm_i915_private *dev_priv, bool debug)
> > I915_WRITE(EDP_PSR_IMR, ~mask);
> > }
> >
> > +static void psr_event_print(u32 val, bool psr2_enabled)
>
> Is psr2_enabled needed? Do the bits get set incorrectly?
Yes, when PSR is enabled the PSR_EVENT_PSR2_DISABLED bit always set the
same happens with PSR_EVENT_PSR_DISABLE when PSR2 is enabled.
>
> > +{
> > + DRM_DEBUG_KMS("PSR exit causes: 0x%x\n", val);
>
> How about s/causes/events ?
Okay
> > + if (val & PSR_EVENT_PSR2_WD_TIMER_EXPIRE)
> > + DRM_DEBUG_KMS("\tPSR2 watchdog timer expired\n");
> > + if ((val & PSR_EVENT_PSR2_DISABLED) && psr2_enabled)
> > + DRM_DEBUG_KMS("\tPSR2 disabled\n");
> > + if (val & PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN)
> > + DRM_DEBUG_KMS("\tSU dirty FIFO underrun\n");
> > + if (val & PSR_EVENT_SU_CRC_FIFO_UNDERRUN)
> > + DRM_DEBUG_KMS("\tSU CRC FIFO underrun\n");
> > + if (val & PSR_EVENT_GRAPHICS_RESET)
> > + DRM_DEBUG_KMS("\tGraphics reset\n");
> > + if (val & PSR_EVENT_PCH_INTERRUPT)
> > + DRM_DEBUG_KMS("\tPCH interrupt\n");
> > + if (val & PSR_EVENT_MEMORY_UP)
> > + DRM_DEBUG_KMS("\tMemory up\n");
> > + if (val & PSR_EVENT_FRONT_BUFFER_MODIFY)
> > + DRM_DEBUG_KMS("\tFront buffer modification\n");
> > + if (val & PSR_EVENT_WD_TIMER_EXPIRE)
> > + DRM_DEBUG_KMS("\tPSR watchdog timer expired\n");
> > + if (val & PSR_EVENT_PIPE_REGISTERS_UPDATE)
> > + DRM_DEBUG_KMS("\tPIPE registers updated\n");
> > + if (val & PSR_EVENT_REGISTER_UPDATE)
> > + DRM_DEBUG_KMS("\tRegister updated\n");
> > + if (val & PSR_EVENT_HDCP_ENABLE)
> > + DRM_DEBUG_KMS("\tHDCP enabled\n");
> > + if (val & PSR_EVENT_KVMR_SESSION_ENABLE)
> > + DRM_DEBUG_KMS("\tKVMR session enabled\n");
> > + if (val & PSR_EVENT_VBI_ENABLE)
> > + DRM_DEBUG_KMS("\tVBI enabled\n");
> > + if (val & PSR_EVENT_LPSP_MODE_EXIT)
> > + DRM_DEBUG_KMS("\tLPSP mode exited\n");
> > + if ((val & PSR_EVENT_PSR_DISABLE) && !psr2_enabled)
> > + DRM_DEBUG_KMS("\tPSR disabled\n");
> > +}
> > +
> > void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32
> > psr_iir)
> > {
> > u32 transcoders = BIT(TRANSCODER_EDP);
> > @@ -152,6 +189,14 @@ void intel_psr_irq_handler(struct
> > drm_i915_private *dev_priv, u32 psr_iir)
> > dev_priv->psr.last_exit = time_ns;
> > DRM_DEBUG_KMS("[transcoder %s] PSR exit
> > completed\n",
> > transcoder_name(cpu_transcod
> > er));
>
> This line can be removed now.
You mean the '[transcoder %s] PSR exit completed' line? Well it can be
removed for gen9 or better. Should we do it? If so I will add the
transcoder name 'PSR exit events'.
>
> > +
> > + if (INTEL_GEN(dev_priv) >= 9) {
> > + u32 val =
> > I915_READ(PSR_EVENT(cpu_transcoder));
> > + bool psr2_enabled = dev_priv-
> > >psr.psr2_enabled;
> > +
> > + psr_event_print(val,
> > psr2_enabled);
> > + I915_WRITE(PSR_EVENT(cpu_transcode
> > r), val);
>
> nit: How about printing the debug after the write? It is probably
> better
> to reset the event bits as soon as we've done the read.
Okay
>
>
> So this write will cause a second PSR exit. How about making this the
> second patch?
Yeah makes sense, moving it.
>
> > + }
> > }
> > }
> > }
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status
2018-04-25 0:18 ` Dhinakaran Pandiyan
@ 2018-04-25 20:33 ` Souza, Jose
0 siblings, 0 replies; 18+ messages in thread
From: Souza, Jose @ 2018-04-25 20:33 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx@lists.freedesktop.org, Vivi, Rodrigo
On Tue, 2018-04-24 at 17:18 -0700, Dhinakaran Pandiyan wrote:
>
>
> On Fri, 2018-04-20 at 15:27 -0700, José Roberto de Souza wrote:
> > IGT tests could be improved with sink status, knowing for sure that
> > hardware have activate or exit PSR.
> >
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >
> > No changes since v2, Dhinakaran asked to not merge this patch in v2
> > because reading i915_edp_psr_status was causing PSR to exit but now
> > with 'drm/i915/psr: Prevent PSR exit when a non-pipe related
> > register
> > is written' it is fixed.
> >
>
> Do you mind adding "reading i915_edp_psr_status was causing PSR to
> exit
> but now with 'drm/i915/psr: Prevent PSR exit when a non-pipe related
> register is written' it is fixed." to the commit message?
Done
>
>
>
> > drivers/gpu/drm/i915/i915_debugfs.c | 29
> > +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 2f05f5262bba..536d93322451 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
> > return "unknown";
> > }
> >
> > +static const char *psr_sink_status(u8 val)
> > +{
> > + static const char * const sink_status[] = {
> > + "inactive",
> > + "transition to active, capture and display",
> > + "active, display from RFB",
> > + "active, capture and display on sink device
> > timings",
> > + "transition to inactive, capture and display,
> > timing re-sync",
> > + "reserved",
> > + "reserved",
> > + "sink internal error"
> > + };
> > +
> > + val &= DP_PSR_SINK_STATE_MASK;
> > + if (val < ARRAY_SIZE(sink_status))
> > + return sink_status[val];
> > +
> > + return "unknown";
> > +}
> > +
> > static int i915_edp_psr_status(struct seq_file *m, void *data)
> > {
> > struct drm_i915_private *dev_priv = node_to_i915(m-
> > >private);
> > @@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct
> > seq_file *m, void *data)
> > seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
> > psr2, psr2_live_status(psr2));
> > }
> > +
> > + if (dev_priv->psr.enabled) {
> > + struct drm_dp_aux *aux = &dev_priv->psr.enabled-
> > >aux;
> > + u8 val;
> > +
> > + if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, &val) ==
> > 1)
> > + seq_printf(m, "Sink PSR status: 0x%x
> > [%s]\n", val,
> > + psr_sink_status(val));
> > + }
> > mutex_unlock(&dev_priv->psr.lock);
> >
> > if (READ_ONCE(dev_priv->psr.debug)) {
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
2018-04-25 0:16 ` Dhinakaran Pandiyan
@ 2018-04-25 20:37 ` Souza, Jose
0 siblings, 0 replies; 18+ messages in thread
From: Souza, Jose @ 2018-04-25 20:37 UTC (permalink / raw)
To: Vivi, Rodrigo, Pandiyan, Dhinakaran; +Cc: intel-gfx@lists.freedesktop.org
On Tue, 2018-04-24 at 17:16 -0700, Dhinakaran Pandiyan wrote:
>
>
> On Tue, 2018-04-24 at 14:20 -0700, Rodrigo Vivi wrote:
> > On Mon, Apr 23, 2018 at 05:42:40PM -0700, Souza, Jose wrote:
> > > On Fri, 2018-04-20 at 15:57 -0700, Rodrigo Vivi wrote:
> > > > On Fri, Apr 20, 2018 at 03:27:56PM -0700, José Roberto de Souza
> > > > wrote:
> > > > > Any write in any display register was causing HW to exit PSR,
> > > > > masking it to allow more power savings. Writes to pipe
> > > > > related
> > > > > registers will still cause HW to exit PSR.
> > > > > This is already masked for PSR2.
> > > >
> > > > This seems a good idea indeed with the test case on
> > > > perspective.
> > >
> > > what test cases are thinking? the current ones already do pages
> > > flips
> > > that will only touch the pipe related registers.
> > >
> > > >
> > > > But it needs more tests to make sure it doesn't break
> > > > "Display WA #0884: all"
> > >
> > > I just tested the WA #0884 and it still causes PSR to exit. I
> > > have
> > > added a new debugfs that when read it writes to
> > > 'I915_WRITE(CURSURFLIVE(pipe), 0);' causing a PSR exit.
> >
> > Interesting. Thanks a lot for checking this.
> > I guess we are safe then. DK?!
> >
>
> Not sure why it works though, let's give it a try.
>
> It might be worth adding more information about test platform and how
> was this was tested in the commit message. Since this is not clearly
> documented in bspec, someone is going read this in a few months and
> think the code does not make any sense.
>
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Done
>
>
> > >
> > > >
> > > > Or we might need to revert that patch before moving with this
> > > > idea.
> > > >
> > > > >
> > > > > Bspec: 7721 and 8042
> > > > >
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > > > ---
> > > > >
> > > > > New patch in this series.
> > > > >
> > > > > drivers/gpu/drm/i915/intel_psr.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/intel_psr.c
> > > > > index 0938df48107a..c907282dc82d 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > > > @@ -712,7 +712,8 @@ static void hsw_psr_enable_source(struct
> > > > > intel_dp *intel_dp,
> > > > > I915_WRITE(EDP_PSR_DEBUG,
> > > > > EDP_PSR_DEBUG_MASK_MEMUP |
> > > > > EDP_PSR_DEBUG_MASK_HPD |
> > > > > - EDP_PSR_DEBUG_MASK_LPSP);
> > > > > + EDP_PSR_DEBUG_MASK_LPSP |
> > > > > + EDP_PSR_DEBUG_MASK_DISP_REG_WRITE
> > > > > );
> > > > > }
> > > > > }
> > > > >
> > > > > --
> > > > > 2.17.0
> > > > >
> >
> > _______________________________________________
> > 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] 18+ messages in thread
end of thread, other threads:[~2018-04-25 20:37 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-20 22:27 [PATCH v3 1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit José Roberto de Souza
2018-04-20 22:27 ` [PATCH v3 2/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written José Roberto de Souza
2018-04-20 22:57 ` Rodrigo Vivi
2018-04-24 0:42 ` Souza, Jose
2018-04-24 21:20 ` Rodrigo Vivi
2018-04-25 0:16 ` Dhinakaran Pandiyan
2018-04-25 20:37 ` Souza, Jose
2018-04-20 22:58 ` Rodrigo Vivi
2018-04-20 22:27 ` [PATCH v3 3/4] drm/i915/debugfs: Print sink PSR status José Roberto de Souza
2018-04-25 0:18 ` Dhinakaran Pandiyan
2018-04-25 20:33 ` Souza, Jose
2018-04-20 22:27 ` [PATCH v3 4/4] drm/i915/psr/cnl: Set y-coordinate as valid in SDP José Roberto de Souza
2018-04-20 22:44 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit Patchwork
2018-04-20 23:04 ` ✓ Fi.CI.BAT: success " Patchwork
2018-04-20 23:50 ` ✓ Fi.CI.IGT: " Patchwork
2018-04-24 23:47 ` [PATCH v3 1/4] " Dhinakaran Pandiyan
2018-04-24 23:57 ` Dhinakaran Pandiyan
2018-04-25 20:27 ` Souza, Jose
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.