| Message ID | 20180405220023.9449-1-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Qui, 2018-04-05 às 15:00 -0700, Dhinakaran Pandiyan escreveu: > From: Daniel Vetter <daniel.vetter@ffwll.ch> > > 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ä <ville.syrjala@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com> > --- > 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, 0xffffffff); > > + if (IS_HASWELL(dev_priv)) { > + I915_WRITE(EDP_PSR_IMR, 0xffffffff); > + I915_WRITE(EDP_PSR_IIR, 0xffffffff); > + } 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 (1<<1) > +#define EDP_PSR_PRE_ENTRY (1<<0) > + I know you'll remove these definitions in the next patch, but please make sure you add the definitions following the new standard described at the top of the file: (1 << 1) instead of (1<<1). > #define EDP_PSR_AUX_CTL _MMIO(dev_pri > v->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) For this one you're allowed to not put the spaces since the other bits are like this. But you also have the option of drive-by fixing the bits of this register if you want since you're touching it. > #define DE_SPRITEC_FLIP_DONE_IVB (1<<14) > #define DE_PLANEC_FLIP_DONE_IVB (1<<13) > #define DE_PIPEC_VBLANK_IVB (1<<10)
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 <daniel.vetter@ffwll.ch> > > > > 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ä <ville.syrjala@linux.intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com> > > --- > > 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, 0xffffffff); > > > > + if (IS_HASWELL(dev_priv)) { > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff); > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff); > > + } > > 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.
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 <daniel.vetter@ffwll.ch> > > > > > > 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ä <ville.syrjala@linux.intel.com> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com> > > > --- > > > 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, 0xffffffff); > > > > > > + if (IS_HASWELL(dev_priv)) { > > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff); > > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff); > > > + } > > > > 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!
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 <daniel.vetter@ffwll.ch> > > > > > > > > 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ä <ville.syrjala@linux.intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > Reviewed-by: Jose Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > 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, 0xffffffff); > > > > > > > > + if (IS_HASWELL(dev_priv)) { > > > > + I915_WRITE(EDP_PSR_IMR, 0xffffffff); > > > > + I915_WRITE(EDP_PSR_IIR, 0xffffffff); > > > > + } > > > > > > 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! I'm far behind this week and merging the series was on top of my todo list... :( sorry for that. I know that you guys already talked about it and know how to do the follow-up ;) > > >
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, 0xffffffff); + if (IS_HASWELL(dev_priv)) { + I915_WRITE(EDP_PSR_IMR, 0xffffffff); + I915_WRITE(EDP_PSR_IIR, 0xffffffff); + } + 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)