Re: [PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw

Ville Syrjälä
2016-05-18T18:26:39Z

On Wed, May 18, 2016 at 06:47:10PM +0200, Daniel Vetter wrote:
> 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: Drop the REG_WRITE masking since edp interrupt still work and we
> still enter PSR.
>
> Cc: "Runyan, Arthur J" <arthur.j.runyan@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Cc: Durgadoss R <durgadoss.r@intel.com>
> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I guess we might enable the error interrupt unconditionally, but the
entry/exit interrupts are going to cause constant dmesg spam, so those I
don't think we want enabled normally. Might be interesting to have some
kind of debug knob for those though.

Hmm. Actually is the PSR error interrupt tied to the "error interrupt"
bit in the PSR AUX register? We don't enable that bit, so maybe we can't
actually get the error interrupt?

> ---
> drivers/gpu/drm/i915/i915_irq.c | 35 +++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h | 10 ++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index f0d941455bed..579f582ef987 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2185,6 +2185,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)
> {
> @@ -2197,6 +2217,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);
>
> @@ -3381,6 +3404,11 @@ static void ironlake_irq_reset(struct drm_device *dev)
> if (IS_GEN7(dev))
> I915_WRITE(GEN7_ERR_INT, 0xffffffff);
>
> + if (IS_HASWELL(dev)) {
> + I915_WRITE(EDP_PSR_IMR, 0xffffffff);
> + I915_WRITE(EDP_PSR_IIR, 0xffffffff);
> + }
> +
> gen5_gt_irq_reset(dev);
>
> ibx_irq_reset(dev);
> @@ -3676,6 +3704,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> DE_PCH_EVENT_IVB | DE_PLANEC_FLIP_DONE_IVB |
> DE_PLANEB_FLIP_DONE_IVB |
> DE_PLANEA_FLIP_DONE_IVB | DE_AUX_CHANNEL_A_IVB);
> +
> extra_mask = (DE_PIPEC_VBLANK_IVB | DE_PIPEB_VBLANK_IVB |
> DE_PIPEA_VBLANK_IVB | DE_ERR_INT_IVB |
> DE_DP_A_HOTPLUG_IVB);
> @@ -3690,6 +3719,12 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
> DE_DP_A_HOTPLUG);
> }
>
> + if (IS_HASWELL(dev)) {
> + gen5_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;
>
> I915_WRITE(HWSTAM, 0xeffe);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 86fbf723eca7..0f99e67f2114 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3225,6 +3225,13 @@ enum skl_disp_power_wells {
> #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_DATA(i) _MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */
>
> @@ -3259,6 +3266,7 @@ enum skl_disp_power_wells {
> #define EDP_PSR_DEBUG_MASK_LPSP (1<<27)
> #define EDP_PSR_DEBUG_MASK_MEMUP (1<<26)
> #define EDP_PSR_DEBUG_MASK_HPD (1<<25)
> +#define EDP_PSR_DEBUG_MASK_REG_WRITE (1<<16)
>
> #define EDP_PSR2_CTL _MMIO(0x6f900)
> #define EDP_PSR2_ENABLE (1<<31)
> @@ -5886,6 +5894,7 @@ enum skl_disp_power_wells {
> #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)
> @@ -5998,6 +6007,7 @@ enum skl_disp_power_wells {
> #define GEN8_DE_MISC_IIR _MMIO(0x44468)
> #define GEN8_DE_MISC_IER _MMIO(0x4446c)
> #define GEN8_DE_MISC_GSE (1 << 27)
> +#define GEN8_DE_EDP_PSR (1 << 19)
>
> #define GEN8_PCU_ISR _MMIO(0x444e0)
> #define GEN8_PCU_IMR _MMIO(0x444e4)
> --
> 2.8.1

--
Ville Syrjälä
Intel OTC