Re: [Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Rodrigo Vivi]

On Tue, Apr 17, 2018 at 01:01:39PM -0700, Dhinakaran Pandiyan wrote:
> 
> 
> 
> On Tue, 2018-04-17 at 20:41 +0300, Ville Syrjälä wrote:
> > On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> > > Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > > > From: Daniel Vetter 
> > > > 
> > > > The definitions for the error register should be valid on bdw/skl
> > > > too,
> > > > but there we haven't even enabled DE_MISC handling yet.
> > > > 
> > > > Somewhat confusing the the moved register offset on bdw is only for
> > > > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > > > on bdw.
> > > > 
> > > > v2: Fixes from Ville.
> > > > 
> > > > v3: From DK
> > > >  * Rebased on drm-tip
> > > >  * Removed BDW IIR bit definition, looks like an unintentional change
> > > > that
> > > > should be in the following patch.
> > > > 
> > > > v4: From DK
> > > >  * Don't mask REG_WRITE.
> > > > 
> > > > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > > > Cc: Ville Syrjälä 
> > > > Cc: Rodrigo Vivi 
> > > > Cc: Daniel Vetter 
> > > > Signed-off-by: Daniel Vetter 
> > > > Signed-off-by: Dhinakaran Pandiyan 
> > > > Reviewed-by: Jose Roberto de Souza 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_irq.c | 34
> > > > ++
> > > >  drivers/gpu/drm/i915/i915_reg.h |  8 
> > > >  2 files changed, 42 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > > b/drivers/gpu/drm/i915/i915_irq.c
> > > > index 27aee25429b7..c2d3f30778ee 100644
> > > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > > > drm_i915_private *dev_priv,
> > > > ironlake_rps_change_irq_handler(dev_priv);
> > > >  }
> > > >  
> > > > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > > > *dev_priv)
> > > > +{
> > > > +   u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > > > +
> > > > +   if (edp_psr_iir & EDP_PSR_ERROR)
> > > > +   DRM_DEBUG_KMS("PSR error\n");
> > > > +
> > > > +   if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > > > +   DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > > > +   I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> > > 
> > > Why are we masking it here? During these 2 vblanks it's possible that
> > > something will happen (e.g., frontbuffer writing, cursor moving, page
> > > flipping).
> > 
> > The masking was here to avoid seeing a big flood of entry interrupts.
> > 
> > > Are we guaranteed to get a POST_EXIT interrupt even if we
> > > give up entering PSR before it is actually entered?
> > 
> > No. The exit interrupt only happens if you actually reached PSR.
> > 
> > > 
> > > 
> > > > +   }
> > > > +
> > > > +   if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > > > +   DRM_DEBUG_KMS("PSR exit completed\n");
> > > > +   I915_WRITE(EDP_PSR_IMR, 0);
> > > > +   }
> > > > +
> > > > +   I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > > > +}
> > > > +
> > > >  static void ivb_display_irq_handler(struct drm_i915_private
> > > > *dev_priv,
> > > > u32 de_iir)
> > > >  {
> > > > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > > > drm_i915_private *dev_priv,
> > > > if (de_iir & DE_ERR_INT_IVB)
> > > > ivb_err_int_handler(dev_priv);
> > > >  
> > > > +   if (de_iir & DE_EDP_PSR_INT_HSW)
> > > > +   hsw_edp_psr_irq_handler(dev_priv);
> > > > +
> > > > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > > > dp_aux_irq_handler(dev_priv);
> > > >  
> > > > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > > > drm_device *dev)
> > > > if (IS_GEN7(dev_priv))
> > > > I915_WRITE(GEN7_ERR_INT, 0x);
> > > >  
> > > > +   if (IS_HASWELL(dev_priv)) {
> > > > +   I915_WRITE(EDP_PSR_IMR, 0x);
> > > > +   I915_WRITE(EDP_PSR_IIR, 0x);
> > > > +   }
> > > 
> > > We need another IIR write we do for the other platforms. This is not
> > > cargo cult (as mentioned in previous review emails), this is required
> > > since our hardware is able to store more than one IIR interrupt. Please
> > > do it like we do for the other interrupts.
> > 
> 
> My reply and the review were specifically about the POSTING_READ(). I
> agree on the second IIR write, but that was not what the original review
> comment was about.
> 
> I'm still skeptical about inserting the POSTING_READ() between write's
> There are at commit messages and emails that indicate the
> POSTING_READS() are indeed cargo culted. Let me find more data about
> this and get back.
> 
> > I don't 

Re: [Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Dhinakaran Pandiyan]

On Tue, 2018-04-17 at 20:41 +0300, Ville Syrjälä wrote:
> On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> > Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > > From: Daniel Vetter 
> > > 
> > > The definitions for the error register should be valid on bdw/skl
> > > too,
> > > but there we haven't even enabled DE_MISC handling yet.
> > > 
> > > Somewhat confusing the the moved register offset on bdw is only for
> > > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > > on bdw.
> > > 
> > > v2: Fixes from Ville.
> > > 
> > > v3: From DK
> > >  * Rebased on drm-tip
> > >  * Removed BDW IIR bit definition, looks like an unintentional change
> > > that
> > > should be in the following patch.
> > > 
> > > v4: From DK
> > >  * Don't mask REG_WRITE.
> > > 
> > > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > > Cc: Ville Syrjälä 
> > > Cc: Rodrigo Vivi 
> > > Cc: Daniel Vetter 
> > > Signed-off-by: Daniel Vetter 
> > > Signed-off-by: Dhinakaran Pandiyan 
> > > Reviewed-by: Jose Roberto de Souza 
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 34
> > > ++
> > >  drivers/gpu/drm/i915/i915_reg.h |  8 
> > >  2 files changed, 42 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > > b/drivers/gpu/drm/i915/i915_irq.c
> > > index 27aee25429b7..c2d3f30778ee 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > >   ironlake_rps_change_irq_handler(dev_priv);
> > >  }
> > >  
> > > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > > +
> > > + if (edp_psr_iir & EDP_PSR_ERROR)
> > > + DRM_DEBUG_KMS("PSR error\n");
> > > +
> > > + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > > + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > > + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> > 
> > Why are we masking it here? During these 2 vblanks it's possible that
> > something will happen (e.g., frontbuffer writing, cursor moving, page
> > flipping).
> 
> The masking was here to avoid seeing a big flood of entry interrupts.
> 
> > Are we guaranteed to get a POST_EXIT interrupt even if we
> > give up entering PSR before it is actually entered?
> 
> No. The exit interrupt only happens if you actually reached PSR.
> 
> > 
> > 
> > > + }
> > > +
> > > + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > > + DRM_DEBUG_KMS("PSR exit completed\n");
> > > + I915_WRITE(EDP_PSR_IMR, 0);
> > > + }
> > > +
> > > + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > > +}
> > > +
> > >  static void ivb_display_irq_handler(struct drm_i915_private
> > > *dev_priv,
> > >   u32 de_iir)
> > >  {
> > > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > > drm_i915_private *dev_priv,
> > >   if (de_iir & DE_ERR_INT_IVB)
> > >   ivb_err_int_handler(dev_priv);
> > >  
> > > + if (de_iir & DE_EDP_PSR_INT_HSW)
> > > + hsw_edp_psr_irq_handler(dev_priv);
> > > +
> > >   if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > >   dp_aux_irq_handler(dev_priv);
> > >  
> > > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > > drm_device *dev)
> > >   if (IS_GEN7(dev_priv))
> > >   I915_WRITE(GEN7_ERR_INT, 0x);
> > >  
> > > + if (IS_HASWELL(dev_priv)) {
> > > + I915_WRITE(EDP_PSR_IMR, 0x);
> > > + I915_WRITE(EDP_PSR_IIR, 0x);
> > > + }
> > 
> > We need another IIR write we do for the other platforms. This is not
> > cargo cult (as mentioned in previous review emails), this is required
> > since our hardware is able to store more than one IIR interrupt. Please
> > do it like we do for the other interrupts.
> 

My reply and the review were specifically about the POSTING_READ(). I
agree on the second IIR write, but that was not what the original review
comment was about.

I'm still skeptical about inserting the POSTING_READ() between write's
There are at commit messages and emails that indicate the
POSTING_READS() are indeed cargo culted. Let me find more data about
this and get back.

> I don't remember if the PSR IIR is double buffered or not. I would
> assumeme it is. It is super easy to verify though if you want to be
> sure:
> 1. unmask the interrupt
> 2. generate at least two interrupts
> 3. mask the interrupt
> 4. clear IIR
> 5. read IIR
> 6. clear IIR
> 7. read IIR
> 
> Step 5 should still show the IIR bit set if the IIR is double buffered.
> And step 7 it should show it to be clear.
> 

I'll give it a try, thanks!

Re: [Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Ville Syrjälä]

On Mon, Apr 16, 2018 at 05:43:54PM -0700, Paulo Zanoni wrote:
> Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> > From: Daniel Vetter 
> > 
> > The definitions for the error register should be valid on bdw/skl
> > too,
> > but there we haven't even enabled DE_MISC handling yet.
> > 
> > Somewhat confusing the the moved register offset on bdw is only for
> > the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> > on bdw.
> > 
> > v2: Fixes from Ville.
> > 
> > v3: From DK
> >  * Rebased on drm-tip
> >  * Removed BDW IIR bit definition, looks like an unintentional change
> > that
> > should be in the following patch.
> > 
> > v4: From DK
> >  * Don't mask REG_WRITE.
> > 
> > References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> > Cc: Ville Syrjälä 
> > Cc: Rodrigo Vivi 
> > Cc: Daniel Vetter 
> > Signed-off-by: Daniel Vetter 
> > Signed-off-by: Dhinakaran Pandiyan 
> > Reviewed-by: Jose Roberto de Souza 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 34
> > ++
> >  drivers/gpu/drm/i915/i915_reg.h |  8 
> >  2 files changed, 42 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index 27aee25429b7..c2d3f30778ee 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > ironlake_rps_change_irq_handler(dev_priv);
> >  }
> >  
> > +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> > *dev_priv)
> > +{
> > +   u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> > +
> > +   if (edp_psr_iir & EDP_PSR_ERROR)
> > +   DRM_DEBUG_KMS("PSR error\n");
> > +
> > +   if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> > +   DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> > +   I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
> 
> Why are we masking it here? During these 2 vblanks it's possible that
> something will happen (e.g., frontbuffer writing, cursor moving, page
> flipping).

The masking was here to avoid seeing a big flood of entry interrupts.

> Are we guaranteed to get a POST_EXIT interrupt even if we
> give up entering PSR before it is actually entered?

No. The exit interrupt only happens if you actually reached PSR.

> 
> 
> > +   }
> > +
> > +   if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> > +   DRM_DEBUG_KMS("PSR exit completed\n");
> > +   I915_WRITE(EDP_PSR_IMR, 0);
> > +   }
> > +
> > +   I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> > +}
> > +
> >  static void ivb_display_irq_handler(struct drm_i915_private
> > *dev_priv,
> > u32 de_iir)
> >  {
> > @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> > drm_i915_private *dev_priv,
> > if (de_iir & DE_ERR_INT_IVB)
> > ivb_err_int_handler(dev_priv);
> >  
> > +   if (de_iir & DE_EDP_PSR_INT_HSW)
> > +   hsw_edp_psr_irq_handler(dev_priv);
> > +
> > if (de_iir & DE_AUX_CHANNEL_A_IVB)
> > dp_aux_irq_handler(dev_priv);
> >  
> > @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> > drm_device *dev)
> > if (IS_GEN7(dev_priv))
> > I915_WRITE(GEN7_ERR_INT, 0x);
> >  
> > +   if (IS_HASWELL(dev_priv)) {
> > +   I915_WRITE(EDP_PSR_IMR, 0x);
> > +   I915_WRITE(EDP_PSR_IIR, 0x);
> > +   }
> 
> We need another IIR write we do for the other platforms. This is not
> cargo cult (as mentioned in previous review emails), this is required
> since our hardware is able to store more than one IIR interrupt. Please
> do it like we do for the other interrupts.

I don't remember if the PSR IIR is double buffered or not. I would
assumeme it is. It is super easy to verify though if you want to be
sure:
1. unmask the interrupt
2. generate at least two interrupts
3. mask the interrupt
4. clear IIR
5. read IIR
6. clear IIR
7. read IIR

Step 5 should still show the IIR bit set if the IIR is double buffered.
And step 7 it should show it to be clear.

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Paulo Zanoni]

Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu:
> From: Daniel Vetter 
> 
> The definitions for the error register should be valid on bdw/skl
> too,
> but there we haven't even enabled DE_MISC handling yet.
> 
> Somewhat confusing the the moved register offset on bdw is only for
> the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
> on bdw.
> 
> v2: Fixes from Ville.
> 
> v3: From DK
>  * Rebased on drm-tip
>  * Removed BDW IIR bit definition, looks like an unintentional change
> that
> should be in the following patch.
> 
> v4: From DK
>  * Don't mask REG_WRITE.
> 
> References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
> Cc: Ville Syrjälä 
> Cc: Rodrigo Vivi 
> Cc: Daniel Vetter 
> Signed-off-by: Daniel Vetter 
> Signed-off-by: Dhinakaran Pandiyan 
> Reviewed-by: Jose Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 34
> ++
>  drivers/gpu/drm/i915/i915_reg.h |  8 
>  2 files changed, 42 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> b/drivers/gpu/drm/i915/i915_irq.c
> index 27aee25429b7..c2d3f30778ee 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct
> drm_i915_private *dev_priv,
>   ironlake_rps_change_irq_handler(dev_priv);
>  }
>  
> +static void hsw_edp_psr_irq_handler(struct drm_i915_private
> *dev_priv)
> +{
> + u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
> +
> + if (edp_psr_iir & EDP_PSR_ERROR)
> + DRM_DEBUG_KMS("PSR error\n");
> +
> + if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
> + DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
> + I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);

Why are we masking it here? During these 2 vblanks it's possible that
something will happen (e.g., frontbuffer writing, cursor moving, page
flipping). Are we guaranteed to get a POST_EXIT interrupt even if we
give up entering PSR before it is actually entered?


> + }
> +
> + if (edp_psr_iir & EDP_PSR_POST_EXIT) {
> + DRM_DEBUG_KMS("PSR exit completed\n");
> + I915_WRITE(EDP_PSR_IMR, 0);
> + }
> +
> + I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
> +}
> +
>  static void ivb_display_irq_handler(struct drm_i915_private
> *dev_priv,
>   u32 de_iir)
>  {
> @@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct
> drm_i915_private *dev_priv,
>   if (de_iir & DE_ERR_INT_IVB)
>   ivb_err_int_handler(dev_priv);
>  
> + if (de_iir & DE_EDP_PSR_INT_HSW)
> + hsw_edp_psr_irq_handler(dev_priv);
> +
>   if (de_iir & DE_AUX_CHANNEL_A_IVB)
>   dp_aux_irq_handler(dev_priv);
>  
> @@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct
> drm_device *dev)
>   if (IS_GEN7(dev_priv))
>   I915_WRITE(GEN7_ERR_INT, 0x);
>  
> + if (IS_HASWELL(dev_priv)) {
> + I915_WRITE(EDP_PSR_IMR, 0x);
> + I915_WRITE(EDP_PSR_IIR, 0x);
> + }

We need another IIR write we do for the other platforms. This is not
cargo cult (as mentioned in previous review emails), this is required
since our hardware is able to store more than one IIR interrupt. Please
do it like we do for the other interrupts.

Do git-blame in the relevant lines and read the commit messages for
more information on why stuff is the way it is. If you still think this
is unnecessary, feel free to write a patch removing the unnecessary
stuff for every interrupt instead of of making just one register be
not-cargo-culted.

> +
>   gen5_gt_irq_reset(dev_priv);
>  
>   ibx_irq_reset(dev_priv);
> @@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct
> drm_device *dev)
> DE_DP_A_HOTPLUG);
>   }
>  
> + if (IS_HASWELL(dev_priv)) {
> + gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
> + I915_WRITE(EDP_PSR_IMR, 0);
> + display_mask |= DE_EDP_PSR_INT_HSW;
> + }
> +
>   dev_priv->irq_mask = ~display_mask;
>  
>   ibx_irq_pre_postinstall(dev);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 176dca6554f4..f5783d6db614 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4011,6 +4011,13 @@ enum {
>  #define   EDP_PSR_TP1_TIME_0us   (3<<4)
>  #define   EDP_PSR_IDLE_FRAME_SHIFT   0
>  
> +/* Bspec claims those aren't shifted but stay at 0x64800 */
> +#define EDP_PSR_IMR  _MMIO(0x64834)
> +#define EDP_PSR_IIR  _MMIO(0x64838)
> +#define   EDP_PSR_ERROR  (1<<2)
> +#define   EDP_PSR_POST_EXIT  

[Intel-gfx] [PATCH v4] drm/i915: Enable edp psr error interrupts on hsw

[Dhinakaran Pandiyan]

From: Daniel Vetter 

The definitions for the error register should be valid on bdw/skl too,
but there we haven't even enabled DE_MISC handling yet.

Somewhat confusing the the moved register offset on bdw is only for
the _CTL/_AUX register, and that _IIR/IMR stayed where they have been
on bdw.

v2: Fixes from Ville.

v3: From DK
 * Rebased on drm-tip
 * Removed BDW IIR bit definition, looks like an unintentional change that
should be in the following patch.

v4: From DK
 * Don't mask REG_WRITE.

References: bspec/11974 [SRD Interrupt Bit Definition DevHSW]
Cc: Ville Syrjälä 
Cc: Rodrigo Vivi 
Cc: Daniel Vetter 
Signed-off-by: Daniel Vetter 
Signed-off-by: Dhinakaran Pandiyan 
Reviewed-by: Jose Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_irq.c | 34 ++
 drivers/gpu/drm/i915/i915_reg.h |  8 
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 27aee25429b7..c2d3f30778ee 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2391,6 +2391,26 @@ static void ilk_display_irq_handler(struct 
drm_i915_private *dev_priv,
ironlake_rps_change_irq_handler(dev_priv);
 }
 
+static void hsw_edp_psr_irq_handler(struct drm_i915_private *dev_priv)
+{
+   u32 edp_psr_iir = I915_READ(EDP_PSR_IIR);
+
+   if (edp_psr_iir & EDP_PSR_ERROR)
+   DRM_DEBUG_KMS("PSR error\n");
+
+   if (edp_psr_iir & EDP_PSR_PRE_ENTRY) {
+   DRM_DEBUG_KMS("PSR prepare entry in 2 vblanks\n");
+   I915_WRITE(EDP_PSR_IMR, EDP_PSR_PRE_ENTRY);
+   }
+
+   if (edp_psr_iir & EDP_PSR_POST_EXIT) {
+   DRM_DEBUG_KMS("PSR exit completed\n");
+   I915_WRITE(EDP_PSR_IMR, 0);
+   }
+
+   I915_WRITE(EDP_PSR_IIR, edp_psr_iir);
+}
+
 static void ivb_display_irq_handler(struct drm_i915_private *dev_priv,
u32 de_iir)
 {
@@ -2403,6 +2423,9 @@ static void ivb_display_irq_handler(struct 
drm_i915_private *dev_priv,
if (de_iir & DE_ERR_INT_IVB)
ivb_err_int_handler(dev_priv);
 
+   if (de_iir & DE_EDP_PSR_INT_HSW)
+   hsw_edp_psr_irq_handler(dev_priv);
+
if (de_iir & DE_AUX_CHANNEL_A_IVB)
dp_aux_irq_handler(dev_priv);
 
@@ -3260,6 +3283,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
if (IS_GEN7(dev_priv))
I915_WRITE(GEN7_ERR_INT, 0x);
 
+   if (IS_HASWELL(dev_priv)) {
+   I915_WRITE(EDP_PSR_IMR, 0x);
+   I915_WRITE(EDP_PSR_IIR, 0x);
+   }
+
gen5_gt_irq_reset(dev_priv);
 
ibx_irq_reset(dev_priv);
@@ -3671,6 +3699,12 @@ static int ironlake_irq_postinstall(struct drm_device 
*dev)
  DE_DP_A_HOTPLUG);
}
 
+   if (IS_HASWELL(dev_priv)) {
+   gen3_assert_iir_is_zero(dev_priv, EDP_PSR_IIR);
+   I915_WRITE(EDP_PSR_IMR, 0);
+   display_mask |= DE_EDP_PSR_INT_HSW;
+   }
+
dev_priv->irq_mask = ~display_mask;
 
ibx_irq_pre_postinstall(dev);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 176dca6554f4..f5783d6db614 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4011,6 +4011,13 @@ enum {
 #define   EDP_PSR_TP1_TIME_0us (3<<4)
 #define   EDP_PSR_IDLE_FRAME_SHIFT 0
 
+/* Bspec claims those aren't shifted but stay at 0x64800 */
+#define EDP_PSR_IMR_MMIO(0x64834)
+#define EDP_PSR_IIR_MMIO(0x64838)
+#define   EDP_PSR_ERROR(1<<2)
+#define   EDP_PSR_POST_EXIT(1<<1)
+#define   EDP_PSR_PRE_ENTRY(1<<0)
+
 #define EDP_PSR_AUX_CTL
_MMIO(dev_priv->psr_mmio_base + 0x10)
 #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK(3 << 26)
 #define   EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK(0x1f << 20)
@@ -6820,6 +6827,7 @@ enum {
 #define DE_PCH_EVENT_IVB   (1<<28)
 #define DE_DP_A_HOTPLUG_IVB(1<<27)
 #define DE_AUX_CHANNEL_A_IVB   (1<<26)
+#define DE_EDP_PSR_INT_HSW (1<<19)
 #define DE_SPRITEC_FLIP_DONE_IVB   (1<<14)
 #define DE_PLANEC_FLIP_DONE_IVB(1<<13)
 #define DE_PIPEC_VBLANK_IVB(1<<10)
-- 
2.14.1

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