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

[PATCH v3 1/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

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

[PATCH v3 2/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.

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

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

[Rodrigo Vivi]

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

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

[Souza, Jose]

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

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

[Rodrigo Vivi]

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

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

[Dhinakaran Pandiyan]

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

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

[Souza, Jose]

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

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

[Rodrigo Vivi]

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

[PATCH v3 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.

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

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

[Dhinakaran Pandiyan]

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

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

[Souza, Jose]

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

[PATCH v3 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>
---

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Patchwork]

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

✓ Fi.CI.BAT: success for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Patchwork]

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

✓ Fi.CI.IGT: success for series starting with [v3,1/4] drm/i915/psr/skl+: Print information about what caused a PSR exit

[Patchwork]

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

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

[Dhinakaran Pandiyan]

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

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

[Dhinakaran Pandiyan]

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

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

[Souza, Jose]

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