| Message ID | 20190412222909.1711-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Fri, Apr 12, 2019 at 03:29:08PM -0700, José Roberto de Souza wrote: > i915 does not support enabling PSR on any transcoder other than eDP. > Clean up the misleading non-eDP code that currently exists to allow > for the rework of PSR register definitions in the next patch. > > v2: > - Commit message updated (Rodrigo and Dhinakaran) nice code clean-up and thanks for updating the message. Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 17 +--- > drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++----------------------- > 2 files changed, 42 insertions(+), 122 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c1c0f7ab03e9..e2803b120b6d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4243,13 +4243,9 @@ enum { > /* 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(shift) (1 << ((shift) + 2)) > -#define EDP_PSR_POST_EXIT(shift) (1 << ((shift) + 1)) > -#define EDP_PSR_PRE_ENTRY(shift) (1 << (shift)) > -#define EDP_PSR_TRANSCODER_C_SHIFT 24 > -#define EDP_PSR_TRANSCODER_B_SHIFT 16 > -#define EDP_PSR_TRANSCODER_A_SHIFT 8 > -#define EDP_PSR_TRANSCODER_EDP_SHIFT 0 > +#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) > @@ -4314,12 +4310,7 @@ 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 _MMIO(0x6F848) > #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) > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 963663ba0edf..581774748c4c 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -87,46 +87,12 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv, > } > } > > -static int edp_psr_shift(enum transcoder cpu_transcoder) > -{ > - switch (cpu_transcoder) { > - case TRANSCODER_A: > - return EDP_PSR_TRANSCODER_A_SHIFT; > - case TRANSCODER_B: > - return EDP_PSR_TRANSCODER_B_SHIFT; > - case TRANSCODER_C: > - return EDP_PSR_TRANSCODER_C_SHIFT; > - default: > - MISSING_CASE(cpu_transcoder); > - /* fallthrough */ > - case TRANSCODER_EDP: > - return EDP_PSR_TRANSCODER_EDP_SHIFT; > - } > -} > - > void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug) > { > - u32 debug_mask, mask; > - enum transcoder cpu_transcoder; > - u32 transcoders = BIT(TRANSCODER_EDP); > - > - if (INTEL_GEN(dev_priv) >= 8) > - transcoders |= BIT(TRANSCODER_A) | > - BIT(TRANSCODER_B) | > - BIT(TRANSCODER_C); > - > - debug_mask = 0; > - mask = 0; > - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) { > - int shift = edp_psr_shift(cpu_transcoder); > - > - mask |= EDP_PSR_ERROR(shift); > - debug_mask |= EDP_PSR_POST_EXIT(shift) | > - EDP_PSR_PRE_ENTRY(shift); > - } > + u32 mask = EDP_PSR_ERROR; > > if (debug & I915_PSR_DEBUG_IRQ) > - mask |= debug_mask; > + mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY; > > I915_WRITE(EDP_PSR_IMR, ~mask); > } > @@ -170,62 +136,47 @@ static void psr_event_print(u32 val, bool psr2_enabled) > > void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir) > { > - u32 transcoders = BIT(TRANSCODER_EDP); > - enum transcoder cpu_transcoder; > - ktime_t time_ns = ktime_get(); > - u32 mask = 0; > + ktime_t time_ns = ktime_get(); > > - if (INTEL_GEN(dev_priv) >= 8) > - transcoders |= BIT(TRANSCODER_A) | > - BIT(TRANSCODER_B) | > - BIT(TRANSCODER_C); > - > - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) { > - int shift = edp_psr_shift(cpu_transcoder); > - > - if (psr_iir & EDP_PSR_ERROR(shift)) { > - DRM_WARN("[transcoder %s] PSR aux error\n", > - transcoder_name(cpu_transcoder)); > - > - dev_priv->psr.irq_aux_error = true; > - > - /* > - * If this interruption is not masked it will keep > - * interrupting so fast that it prevents the scheduled > - * work to run. > - * Also after a PSR error, we don't want to arm PSR > - * again so we don't care about unmask the interruption > - * or unset irq_aux_error. > - */ > - mask |= EDP_PSR_ERROR(shift); > - } > + if (psr_iir & EDP_PSR_ERROR) { > + u32 mask; > > - if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) { > - dev_priv->psr.last_entry_attempt = time_ns; > - DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n", > - transcoder_name(cpu_transcoder)); > - } > + DRM_WARN("[transcoder %s] PSR aux error\n", > + transcoder_name(TRANSCODER_EDP)); > > - if (psr_iir & EDP_PSR_POST_EXIT(shift)) { > - dev_priv->psr.last_exit = time_ns; > - DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n", > - transcoder_name(cpu_transcoder)); > + dev_priv->psr.irq_aux_error = true; > > - if (INTEL_GEN(dev_priv) >= 9) { > - u32 val = I915_READ(PSR_EVENT(cpu_transcoder)); > - bool psr2_enabled = dev_priv->psr.psr2_enabled; > + /* > + * If this interruption is not masked it will keep > + * interrupting so fast that it prevents the scheduled > + * work to run. > + * Also after a PSR error, we don't want to arm PSR > + * again so we don't care about unmask the interruption > + * or unset irq_aux_error. > + */ > + mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR; > + I915_WRITE(EDP_PSR_IMR, mask); > > - I915_WRITE(PSR_EVENT(cpu_transcoder), val); > - psr_event_print(val, psr2_enabled); > - } > - } > + schedule_work(&dev_priv->psr.work); > } > > - if (mask) { > - mask |= I915_READ(EDP_PSR_IMR); > - I915_WRITE(EDP_PSR_IMR, mask); > + if (psr_iir & EDP_PSR_PRE_ENTRY) { > + dev_priv->psr.last_entry_attempt = time_ns; > + DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n", > + transcoder_name(TRANSCODER_EDP)); > + } > > - schedule_work(&dev_priv->psr.work); > + if (psr_iir & EDP_PSR_POST_EXIT) { > + DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n", > + transcoder_name(TRANSCODER_EDP)); > + > + if (INTEL_GEN(dev_priv) >= 9) { > + u32 val = I915_READ(PSR_EVENT); > + bool psr2_enabled = dev_priv->psr.psr2_enabled; > + > + I915_WRITE(PSR_EVENT, val); > + psr_event_print(val, psr2_enabled); > + } > } > } > > @@ -672,30 +623,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp) > dev_priv->psr.active = true; > } > > -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv, > - enum transcoder cpu_transcoder) > -{ > - static const i915_reg_t regs[] = { > - [TRANSCODER_A] = CHICKEN_TRANS_A, > - [TRANSCODER_B] = CHICKEN_TRANS_B, > - [TRANSCODER_C] = CHICKEN_TRANS_C, > - [TRANSCODER_EDP] = CHICKEN_TRANS_EDP, > - }; > - > - WARN_ON(INTEL_GEN(dev_priv) < 9); > - > - if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) || > - !regs[cpu_transcoder].reg)) > - cpu_transcoder = TRANSCODER_A; > - > - return regs[cpu_transcoder]; > -} > - > static void intel_psr_enable_source(struct intel_dp *intel_dp, > const struct intel_crtc_state *crtc_state) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > - enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > u32 mask; > > /* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+ > @@ -706,13 +637,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > > if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) && > !IS_GEMINILAKE(dev_priv))) { > - i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, > - cpu_transcoder); > - u32 chicken = I915_READ(reg); > + u32 chicken = I915_READ(CHICKEN_TRANS_EDP); > > chicken |= PSR2_VSC_ENABLE_PROG_HEADER | > PSR2_ADD_VERTICAL_LINE_COUNT; > - I915_WRITE(reg, chicken); > + I915_WRITE(CHICKEN_TRANS_EDP, chicken); > } > > /* > @@ -1225,7 +1154,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv) > * to avoid any rendering problems. > */ > val = I915_READ(EDP_PSR_IIR); > - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); > + val &= EDP_PSR_ERROR; > if (val) { > DRM_DEBUG_KMS("PSR interruption error set\n"); > dev_priv->psr.sink_not_reliable = true; > -- > 2.21.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 c1c0f7ab03e9..e2803b120b6d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4243,13 +4243,9 @@ enum { /* 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(shift) (1 << ((shift) + 2)) -#define EDP_PSR_POST_EXIT(shift) (1 << ((shift) + 1)) -#define EDP_PSR_PRE_ENTRY(shift) (1 << (shift)) -#define EDP_PSR_TRANSCODER_C_SHIFT 24 -#define EDP_PSR_TRANSCODER_B_SHIFT 16 -#define EDP_PSR_TRANSCODER_A_SHIFT 8 -#define EDP_PSR_TRANSCODER_EDP_SHIFT 0 +#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) @@ -4314,12 +4310,7 @@ 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 _MMIO(0x6F848) #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) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 963663ba0edf..581774748c4c 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -87,46 +87,12 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv, } } -static int edp_psr_shift(enum transcoder cpu_transcoder) -{ - switch (cpu_transcoder) { - case TRANSCODER_A: - return EDP_PSR_TRANSCODER_A_SHIFT; - case TRANSCODER_B: - return EDP_PSR_TRANSCODER_B_SHIFT; - case TRANSCODER_C: - return EDP_PSR_TRANSCODER_C_SHIFT; - default: - MISSING_CASE(cpu_transcoder); - /* fallthrough */ - case TRANSCODER_EDP: - return EDP_PSR_TRANSCODER_EDP_SHIFT; - } -} - void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug) { - u32 debug_mask, mask; - enum transcoder cpu_transcoder; - u32 transcoders = BIT(TRANSCODER_EDP); - - if (INTEL_GEN(dev_priv) >= 8) - transcoders |= BIT(TRANSCODER_A) | - BIT(TRANSCODER_B) | - BIT(TRANSCODER_C); - - debug_mask = 0; - mask = 0; - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) { - int shift = edp_psr_shift(cpu_transcoder); - - mask |= EDP_PSR_ERROR(shift); - debug_mask |= EDP_PSR_POST_EXIT(shift) | - EDP_PSR_PRE_ENTRY(shift); - } + u32 mask = EDP_PSR_ERROR; if (debug & I915_PSR_DEBUG_IRQ) - mask |= debug_mask; + mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY; I915_WRITE(EDP_PSR_IMR, ~mask); } @@ -170,62 +136,47 @@ static void psr_event_print(u32 val, bool psr2_enabled) void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir) { - u32 transcoders = BIT(TRANSCODER_EDP); - enum transcoder cpu_transcoder; - ktime_t time_ns = ktime_get(); - u32 mask = 0; + ktime_t time_ns = ktime_get(); - if (INTEL_GEN(dev_priv) >= 8) - transcoders |= BIT(TRANSCODER_A) | - BIT(TRANSCODER_B) | - BIT(TRANSCODER_C); - - for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) { - int shift = edp_psr_shift(cpu_transcoder); - - if (psr_iir & EDP_PSR_ERROR(shift)) { - DRM_WARN("[transcoder %s] PSR aux error\n", - transcoder_name(cpu_transcoder)); - - dev_priv->psr.irq_aux_error = true; - - /* - * If this interruption is not masked it will keep - * interrupting so fast that it prevents the scheduled - * work to run. - * Also after a PSR error, we don't want to arm PSR - * again so we don't care about unmask the interruption - * or unset irq_aux_error. - */ - mask |= EDP_PSR_ERROR(shift); - } + if (psr_iir & EDP_PSR_ERROR) { + u32 mask; - if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) { - dev_priv->psr.last_entry_attempt = time_ns; - DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n", - transcoder_name(cpu_transcoder)); - } + DRM_WARN("[transcoder %s] PSR aux error\n", + transcoder_name(TRANSCODER_EDP)); - if (psr_iir & EDP_PSR_POST_EXIT(shift)) { - dev_priv->psr.last_exit = time_ns; - DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n", - transcoder_name(cpu_transcoder)); + dev_priv->psr.irq_aux_error = true; - if (INTEL_GEN(dev_priv) >= 9) { - u32 val = I915_READ(PSR_EVENT(cpu_transcoder)); - bool psr2_enabled = dev_priv->psr.psr2_enabled; + /* + * If this interruption is not masked it will keep + * interrupting so fast that it prevents the scheduled + * work to run. + * Also after a PSR error, we don't want to arm PSR + * again so we don't care about unmask the interruption + * or unset irq_aux_error. + */ + mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR; + I915_WRITE(EDP_PSR_IMR, mask); - I915_WRITE(PSR_EVENT(cpu_transcoder), val); - psr_event_print(val, psr2_enabled); - } - } + schedule_work(&dev_priv->psr.work); } - if (mask) { - mask |= I915_READ(EDP_PSR_IMR); - I915_WRITE(EDP_PSR_IMR, mask); + if (psr_iir & EDP_PSR_PRE_ENTRY) { + dev_priv->psr.last_entry_attempt = time_ns; + DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n", + transcoder_name(TRANSCODER_EDP)); + } - schedule_work(&dev_priv->psr.work); + if (psr_iir & EDP_PSR_POST_EXIT) { + DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n", + transcoder_name(TRANSCODER_EDP)); + + if (INTEL_GEN(dev_priv) >= 9) { + u32 val = I915_READ(PSR_EVENT); + bool psr2_enabled = dev_priv->psr.psr2_enabled; + + I915_WRITE(PSR_EVENT, val); + psr_event_print(val, psr2_enabled); + } } } @@ -672,30 +623,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp) dev_priv->psr.active = true; } -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv, - enum transcoder cpu_transcoder) -{ - static const i915_reg_t regs[] = { - [TRANSCODER_A] = CHICKEN_TRANS_A, - [TRANSCODER_B] = CHICKEN_TRANS_B, - [TRANSCODER_C] = CHICKEN_TRANS_C, - [TRANSCODER_EDP] = CHICKEN_TRANS_EDP, - }; - - WARN_ON(INTEL_GEN(dev_priv) < 9); - - if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) || - !regs[cpu_transcoder].reg)) - cpu_transcoder = TRANSCODER_A; - - return regs[cpu_transcoder]; -} - static void intel_psr_enable_source(struct intel_dp *intel_dp, const struct intel_crtc_state *crtc_state) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); - enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; u32 mask; /* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+ @@ -706,13 +637,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) && !IS_GEMINILAKE(dev_priv))) { - i915_reg_t reg = gen9_chicken_trans_reg(dev_priv, - cpu_transcoder); - u32 chicken = I915_READ(reg); + u32 chicken = I915_READ(CHICKEN_TRANS_EDP); chicken |= PSR2_VSC_ENABLE_PROG_HEADER | PSR2_ADD_VERTICAL_LINE_COUNT; - I915_WRITE(reg, chicken); + I915_WRITE(CHICKEN_TRANS_EDP, chicken); } /* @@ -1225,7 +1154,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv) * to avoid any rendering problems. */ val = I915_READ(EDP_PSR_IIR); - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); + val &= EDP_PSR_ERROR; if (val) { DRM_DEBUG_KMS("PSR interruption error set\n"); dev_priv->psr.sink_not_reliable = true;
i915 does not support enabling PSR on any transcoder other than eDP. Clean up the misleading non-eDP code that currently exists to allow for the rework of PSR register definitions in the next patch. v2: - Commit message updated (Rodrigo and Dhinakaran) Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 17 +--- drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++----------------------- 2 files changed, 42 insertions(+), 122 deletions(-)