[Intel-gfx] [PATCH 1/7] drm/i915: Enable edp psr error interrupts on hsw
Jindal, Sonika
sonika.jindal at intel.com
Thu May 19 09:36:17 UTC 2016
On 5/18/2016 11:56 PM, Ville Syrjälä wrote:
> 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 at intel.com>
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Cc: Sonika Jindal <sonika.jindal at intel.com>
>> Cc: Durgadoss R <durgadoss.r at intel.com>
>> Cc: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan at intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter at 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?
Only bit 2 (SRD Aux Error) must be tied to the error interrupt bit of
SRD_AUX register.
The entry and exit interrupts should be independent as per my understanding.
I agree the entry/exit messages will be too much in dmesg.
>> ---
>> 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 line.
>> 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)
Is this remaining from some previous version? or left intentionally?
>>
>> #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)
We are not enabling it for bdw now right?
>>
>> #define GEN8_PCU_ISR _MMIO(0x444e0)
>> #define GEN8_PCU_IMR _MMIO(0x444e4)
>> --
>> 2.8.1
More information about the Intel-gfx
mailing list