[Intel-gfx] [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders
Rodrigo Vivi
rodrigo.vivi at intel.com
Thu Apr 4 21:20:28 UTC 2019
On Thu, Apr 04, 2019 at 12:40:34PM -0700, Souza, Jose wrote:
> On Wed, 2019-04-03 at 17:31 -0700, Rodrigo Vivi wrote:
> > On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza
> > wrote:
> > > PSR is only supported in eDP transcoder and there is only one
> > > instance of it, so lets drop all of this code.
> >
> > Is this sentence true? I mean, in the way it is written it
> > seems like HW doesn't actually support it...
> > Or should we re-phrase for we are not really enabling support
> > for other transcoders than eDP and we do not have plans to do
> > it so soon so let's clean the code...
> > or something like that?
>
> Okay, what about replace it for:
>
> Since BDW all transcoders have PSR registers but only EDP transcoder
> can drive a EDP panel and as PSR is only part of the EDP specification,
> for real usage PSR is only supported in EDP panel, so lets drop all of
> this useless code.
well, this still looks like HW limitation. If PSR is supported by HW on
different transcoders is because there's some possibility of adding
eDP on other transcoders. They wouldn't waste so many register space
if that wasn't the case.
Even though we have a dedicated transcoder for eDP I don't
believe we can assume that eDP is not supported on the other ones.
>
>
>
> >
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at 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 c59cfa83dbaf..18e2991b376d 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4241,13 +4241,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)
> > > @@ -4312,12 +4308,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 bb97c1657493..b984e005b72e 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -84,46 +84,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);
> > > }
> > > @@ -167,62 +133,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);
> > > + }
> > > }
> > > }
> > >
> > > @@ -669,30 +620,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+
> > > @@ -703,13 +634,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);
> > > }
> > >
> > > /*
> > > @@ -1222,7 +1151,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
> > >
More information about the Intel-gfx
mailing list