[PATCH 1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

[PATCH 1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

[José Roberto de Souza]

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.

It also do not break the Display WA #0884, writes to CURSURFLIVE
are still causing hardware to exit PSR. This was tested in CNL machine
by triggering a write to CURSURFLIVE when a debugfs was read by user.

Bspec: 7721 and 8042

v4: Checked that it do not breaks WA #0884 and added this information
to the commit message.

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>
---
 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 0d548292dd09..e35a3b94fa69 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -667,7 +667,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

[PATCH 2/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[José Roberto de Souza]

This will be helpful to debug what hardware is actually tracking
and causing PSR to exit.

BSpec: 7721

v4:
- Using _MMIO_TRANS2() in PSR_EVENT
- Cleaning events before printing

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>
---
 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..391825ae2361 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)			_MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
+#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 e35a3b94fa69..c8d5cdce544f 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 events: 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;
+
+				I915_WRITE(PSR_EVENT(cpu_transcoder), val);
+				psr_event_print(val, psr2_enabled);
+			}
 		}
 	}
 }
-- 
2.17.0

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

[PATCH 3/4] drm/i915/debugfs: Print sink PSR status

[José Roberto de Souza]

IGT tests could be improved with sink status, knowing for sure that
hardware have activate or exit PSR.

v3:
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.

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>
---
 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 1c88805d3354..cb1a804bf72e 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

[PATCH 4/4] drm/i915/psr/cnl: Set y-coordinate as valid in SDP

[José Roberto de Souza]

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>
---
 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 c8d5cdce544f..6233a322aac5 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

Re: [PATCH 2/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 02:23:32PM -0700, José Roberto de Souza wrote:
> This will be helpful to debug what hardware is actually tracking
> and causing PSR to exit.
> 
> BSpec: 7721
> 
> v4:
> - Using _MMIO_TRANS2() in PSR_EVENT
> - Cleaning events before printing
> 
> 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>
> ---
>  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..391825ae2361 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)			_MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
> +#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 e35a3b94fa69..c8d5cdce544f 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 events: 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)

I'm not sure if we should add this extra psr2_enable check here.
Probably better to just print the bit reference and move one.

otherwise we might have the risk of having a message
"PSR exit events:"
followed by nothing below it

> +		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;
> +
> +				I915_WRITE(PSR_EVENT(cpu_transcoder), val);

writing the value back really clears it?
or should we clear by writting 0 to it?

> +				psr_event_print(val, psr2_enabled);
> +			}
>  		}
>  	}
>  }
> -- 
> 2.17.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Souza, Jose]

On Wed, 2018-04-25 at 14:40 -0700, Rodrigo Vivi wrote:
> On Wed, Apr 25, 2018 at 02:23:32PM -0700, José Roberto de Souza
> wrote:
> > This will be helpful to debug what hardware is actually tracking
> > and causing PSR to exit.
> > 
> > BSpec: 7721
> > 
> > v4:
> > - Using _MMIO_TRANS2() in PSR_EVENT
> > - Cleaning events before printing
> > 
> > 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>
> > ---
> >  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..391825ae2361 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)			_MMIO_TRANS2(trans
> > , _PSR_EVENT_TRANS_A)
> > +#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 e35a3b94fa69..c8d5cdce544f 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 events: 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)
> 
> I'm not sure if we should add this extra psr2_enable check here.
> Probably better to just print the bit reference and move one.
> 
> otherwise we might have the risk of having a message
> "PSR exit events:"
> followed by nothing below it

I'm okay with this too but I think is not necessary have 'PSR2
disabled' printed everytime when PSR is 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));
> > +
> > +			if (INTEL_GEN(dev_priv) >= 9) {
> > +				u32 val =
> > I915_READ(PSR_EVENT(cpu_transcoder));
> > +				bool psr2_enabled = dev_priv-
> > >psr.psr2_enabled;
> > +
> > +				I915_WRITE(PSR_EVENT(cpu_transcode
> > r), val);
> 
> writing the value back really clears it?
> or should we clear by writting 0 to it?

Yes, it is cleared by writing 1 to the bit.

> 
> > +				psr_event_print(val,
> > psr2_enabled);
> > +			}
> >  		}
> >  	}
> >  }
> > -- 
> > 2.17.0
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 02:47:35PM -0700, Souza, Jose wrote:
> On Wed, 2018-04-25 at 14:40 -0700, Rodrigo Vivi wrote:
> > On Wed, Apr 25, 2018 at 02:23:32PM -0700, José Roberto de Souza
> > wrote:
> > > This will be helpful to debug what hardware is actually tracking
> > > and causing PSR to exit.
> > > 
> > > BSpec: 7721
> > > 
> > > v4:
> > > - Using _MMIO_TRANS2() in PSR_EVENT
> > > - Cleaning events before printing
> > > 
> > > 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>
> > > ---
> > >  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..391825ae2361 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)			_MMIO_TRANS2(trans
> > > , _PSR_EVENT_TRANS_A)
> > > +#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 e35a3b94fa69..c8d5cdce544f 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 events: 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)
> > 
> > I'm not sure if we should add this extra psr2_enable check here.
> > Probably better to just print the bit reference and move one.
> > 
> > otherwise we might have the risk of having a message
> > "PSR exit events:"
> > followed by nothing below it
> 
> I'm okay with this too but I think is not necessary have 'PSR2
> disabled' printed everytime when PSR is enabled.

oh! I see your point now...
Makes sense to minimize this noise, and let's hope we
don't face the empty case.

> 
> > 
> > > +		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));
> > > +
> > > +			if (INTEL_GEN(dev_priv) >= 9) {
> > > +				u32 val =
> > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > +				bool psr2_enabled = dev_priv-
> > > >psr.psr2_enabled;
> > > +
> > > +				I915_WRITE(PSR_EVENT(cpu_transcode
> > > r), val);
> > 
> > writing the value back really clears it?
> > or should we clear by writting 0 to it?
> 
> Yes, it is cleared by writing 1 to the bit.

cool, thanks for the confirmation

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

(as soon as CI give okay I will push them)

> 
> > 
> > > +				psr_event_print(val,
> > > psr2_enabled);
> > > +			}
> > >  		}
> > >  	}
> > >  }
> > > -- 
> > > 2.17.0
> > > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
URL   : https://patchwork.freedesktop.org/series/42304/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4099 -> Patchwork_8802 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713


== Participating hosts (38 -> 35) ==

  Missing    (3): fi-ilk-m540 fi-skl-6700hq fi-pnv-d510 


== Build changes ==

    * Linux: CI_DRM_4099 -> Patchwork_8802

  CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8802: cd94e9f8e4937f47d54a2eb6e821020f388b5cdf @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit


== Linux commits ==

cd94e9f8e493 drm/i915/psr/cnl: Set y-coordinate as valid in SDP
48dc63739f2f drm/i915/debugfs: Print sink PSR status
02bba1aa3a2b drm/i915/psr/skl+: Print information about what caused a PSR exit
632045d33482 drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915/psr: Prevent PSR exit when a non-pipe related register is written
URL   : https://patchwork.freedesktop.org/series/42304/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4099_full -> Patchwork_8802_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_8802_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_8802_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/42304/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS +2

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_workarounds@suspend-resume-context:
      shard-apl:          PASS -> FAIL (fdo#103375)

    igt@kms_cursor_legacy@cursor-vs-flip-toggle:
      shard-hsw:          PASS -> FAIL (fdo#103355)

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

    igt@kms_flip@wf_vblank-ts-check-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    igt@kms_rotation_crc@primary-rotation-90:
      shard-apl:          PASS -> FAIL (fdo#103925)

    igt@kms_rotation_crc@sprite-rotation-90-pos-100-0:
      shard-apl:          PASS -> DMESG-WARN (fdo#105127)

    
    ==== Possible fixes ====

    igt@kms_atomic@plane_invalid_params:
      shard-kbl:          DMESG-WARN -> PASS

    igt@kms_flip@2x-flip-vs-wf_vblank:
      shard-hsw:          FAIL (fdo#100368) -> PASS

    igt@kms_flip@flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#105707) -> PASS

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@flip-vs-wf_vblank-interruptible:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_sysfs_edid_timing:
      shard-apl:          WARN (fdo#100047) -> PASS

    igt@kms_vblank@pipe-c-accuracy-idle:
      shard-hsw:          FAIL (fdo#102583) -> PASS

    igt@perf@buffer-fill:
      shard-kbl:          INCOMPLETE (fdo#103665) -> 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#103355 https://bugs.freedesktop.org/show_bug.cgi?id=103355
  fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103928 https://bugs.freedesktop.org/show_bug.cgi?id=103928
  fdo#105127 https://bugs.freedesktop.org/show_bug.cgi?id=105127
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105707 https://bugs.freedesktop.org/show_bug.cgi?id=105707


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

  Missing    (1): shard-glkb 


== Build changes ==

    * Linux: CI_DRM_4099 -> Patchwork_8802

  CI_DRM_4099: 92a39635c2eca4dfe1c8c1673e0c606a720746e5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4449: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8802: cd94e9f8e4937f47d54a2eb6e821020f388b5cdf @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4449: ad8992d3fb27fd604b9ab15e7963c42421ced85c @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 2/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Rodrigo Vivi]

On Wed, Apr 25, 2018 at 02:53:25PM -0700, Rodrigo Vivi wrote:
> On Wed, Apr 25, 2018 at 02:47:35PM -0700, Souza, Jose wrote:
> > On Wed, 2018-04-25 at 14:40 -0700, Rodrigo Vivi wrote:
> > > On Wed, Apr 25, 2018 at 02:23:32PM -0700, José Roberto de Souza
> > > wrote:
> > > > This will be helpful to debug what hardware is actually tracking
> > > > and causing PSR to exit.
> > > > 
> > > > BSpec: 7721
> > > > 
> > > > v4:
> > > > - Using _MMIO_TRANS2() in PSR_EVENT
> > > > - Cleaning events before printing
> > > > 
> > > > 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>
> > > > ---
> > > >  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..391825ae2361 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)			_MMIO_TRANS2(trans
> > > > , _PSR_EVENT_TRANS_A)
> > > > +#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 e35a3b94fa69..c8d5cdce544f 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 events: 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)
> > > 
> > > I'm not sure if we should add this extra psr2_enable check here.
> > > Probably better to just print the bit reference and move one.
> > > 
> > > otherwise we might have the risk of having a message
> > > "PSR exit events:"
> > > followed by nothing below it
> > 
> > I'm okay with this too but I think is not necessary have 'PSR2
> > disabled' printed everytime when PSR is enabled.
> 
> oh! I see your point now...
> Makes sense to minimize this noise, and let's hope we
> don't face the empty case.
> 
> > 
> > > 
> > > > +		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));
> > > > +
> > > > +			if (INTEL_GEN(dev_priv) >= 9) {
> > > > +				u32 val =
> > > > I915_READ(PSR_EVENT(cpu_transcoder));
> > > > +				bool psr2_enabled = dev_priv-
> > > > >psr.psr2_enabled;
> > > > +
> > > > +				I915_WRITE(PSR_EVENT(cpu_transcode
> > > > r), val);
> > > 
> > > writing the value back really clears it?
> > > or should we clear by writting 0 to it?
> > 
> > Yes, it is cleared by writing 1 to the bit.
> 
> cool, thanks for the confirmation
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> (as soon as CI give okay I will push them)

pushed to dinq, thanks

> 
> > 
> > > 
> > > > +				psr_event_print(val,
> > > > psr2_enabled);
> > > > +			}
> > > >  		}
> > > >  	}
> > > >  }
> > > > -- 
> > > > 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