| Message ID | 20180425212334.21109-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 > >
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 > > >
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
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); + } } } }
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(+)