| Message ID | 20190820223325.27490-1-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v8,1/3] drm/i915/psr: Make PSR registers relative to transcoders | expand |
On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: > PSR registers are a mess, some have the full address while others just > have the additional offset from psr_mmio_base. > > For BDW+ psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + > 0x800 and using it makes more difficult for people with an PSR > register address or PSR register name from from BSpec as i915 also > don't match the BSpec names. > For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR registers are > only available in DDIA. > > Other reason to make relative to transcoder is that since BDW every > transcoder have PSR registers, so in theory it should be possible to > have PSR enabled in a non-eDP transcoder. > > So for BDW+ we can use _TRANS2() to get the register offset of any > PSR register in any transcoder while for HSW we have _HSW_PSR_ADJ > that will calculate the register offset for the single PSR instance, > noting that we are already guarded about trying to enable PSR in other > port than DDIA on HSW by the 'if (dig_port->base.port != PORT_A)' in > intel_psr_compute_config(), this check should only be valid for HSW > and will be changed in future. > PSR2 registers and PSR_EVENT was added after Haswell so that is why > _PSR_ADJ() is not used in some macros. > > The only registers that can not be relative to transcoder are > PSR_IMR and PSR_IIR that are not relative to anything, so keeping it > hardcoded. That changed for TGL but it will be handled in another > patch. > > Also removing BDW_EDP_PSR_BASE from GVT because it is not used as it > is the only PSR register that GVT have. > > v5: > - Macros changed to be more explicit about HSW (Dhinakaran) > - Squashed with the patch that added the tran parameter to the > macros (Dhinakaran) > > v6: > - Checking for interruption errors after module reload in the > transcoder that will be used (Dhinakaran) > - Using lowercase to the registers offsets > > v7: > - Removing IS_HASWELL() from registers macros(Jani) > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Zhi Wang <zhi.a.wang@intel.com> > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_psr.c | 104 +++++++++++++---------- > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- > drivers/gpu/drm/i915/i915_drv.h | 5 +- > drivers/gpu/drm/i915/i915_reg.h | 57 +++++++++---- > 5 files changed, 113 insertions(+), 73 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c > index 3bfb720560c2..77232f6bca17 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) > > BUILD_BUG_ON(sizeof(aux_msg) > 20); > for (i = 0; i < sizeof(aux_msg); i += 4) > - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), > + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i >> 2), > intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i)); > > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0); > @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) > > /* Select only valid bits for SRD_AUX_CTL */ > aux_ctl &= psr_aux_mask; > - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl); > } > > static void intel_psr_enable_sink(struct intel_dp *intel_dp) > @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) > if (INTEL_GEN(dev_priv) >= 8) > val |= EDP_PSR_CRC_ENABLE; > > - val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; > - I915_WRITE(EDP_PSR_CTL, val); > + val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & > + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > } > > static void hsw_activate_psr2(struct intel_dp *intel_dp) > @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) > * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is > * recommending keep this bit unset while PSR2 is enabled. > */ > - I915_WRITE(EDP_PSR_CTL, 0); > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); > > - I915_WRITE(EDP_PSR2_CTL, val); > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); > } > > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, > > /* > * HSW spec explicitly says PSR is tied to port A. > - * BDW+ platforms with DDI implementation of PSR have different > - * PSR registers per transcoder and we only implement transcoder EDP > - * ones. Since by Display design transcoder EDP is tied to port A > - * we can safely escape based on the port A. > + * BDW+ platforms have a instance of PSR registers per transcoder but > + * for now it only supports one instance of PSR, so lets keep it > + * hardcoded to PORT_A > */ > if (dig_port->base.port != PORT_A) { > DRM_DEBUG_KMS("PSR condition failed: Port not supported\n"); > @@ -649,8 +649,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp) > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > if (INTEL_GEN(dev_priv) >= 9) > - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)) & EDP_PSR2_ENABLE); > + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & EDP_PSR_ENABLE); > WARN_ON(dev_priv->psr.active); > lockdep_assert_held(&dev_priv->psr.lock); > > @@ -720,19 +720,37 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, > if (INTEL_GEN(dev_priv) < 11) > mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; > > - I915_WRITE(EDP_PSR_DEBUG, mask); > + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask); > } > > static void intel_psr_enable_locked(struct drm_i915_private *dev_priv, > const struct intel_crtc_state *crtc_state) > { > struct intel_dp *intel_dp = dev_priv->psr.dp; > + u32 val; > > WARN_ON(dev_priv->psr.enabled); > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); > dev_priv->psr.busy_frontbuffer_bits = 0; > dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; > + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > + > + /* > + * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR > + * will still keep the error set even after the reset done in the > + * irq_preinstall and irq_uninstall hooks. > + * And enabling in this situation cause the screen to freeze in the > + * first time that PSR HW tries to activate so lets keep PSR disabled > + * to avoid any rendering problems. > + */ > + val = I915_READ(EDP_PSR_IIR); > + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv->psr.transcoder)); > + if (val) { > + dev_priv->psr.sink_not_reliable = true; > + DRM_DEBUG_KMS("PSR interruption error set, not enabling PSR\n"); > + return; > + } > > DRM_DEBUG_KMS("Enabling PSR%s\n", > dev_priv->psr.psr2_enabled ? "2" : "1"); > @@ -782,20 +800,27 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv) > u32 val; > > if (!dev_priv->psr.active) { > - if (INTEL_GEN(dev_priv) >= 9) > - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > + if (INTEL_GEN(dev_priv) >= 9) { > + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > + WARN_ON(val & EDP_PSR2_ENABLE); > + } > + > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > + WARN_ON(val & EDP_PSR_ENABLE); > + > return; > } > > if (dev_priv->psr.psr2_enabled) { > - val = I915_READ(EDP_PSR2_CTL); > + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > WARN_ON(!(val & EDP_PSR2_ENABLE)); > - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE); > + val &= ~EDP_PSR2_ENABLE; > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); > } else { > - val = I915_READ(EDP_PSR_CTL); > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > WARN_ON(!(val & EDP_PSR_ENABLE)); > - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); > + val &= ~EDP_PSR_ENABLE; > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > } > dev_priv->psr.active = false; > } > @@ -817,10 +842,10 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) > intel_psr_exit(dev_priv); > > if (dev_priv->psr.psr2_enabled) { > - psr_status = EDP_PSR2_STATUS; > + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder); > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > } else { > - psr_status = EDP_PSR_STATUS; > + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder); > psr_status_mask = EDP_PSR_STATUS_STATE_MASK; > } > > @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state, > * defensive enough to cover everything. > */ > > - return __intel_wait_for_register(&dev_priv->uncore, EDP_PSR_STATUS, > + return __intel_wait_for_register(&dev_priv->uncore, > + EDP_PSR_STATUS(dev_priv->psr.transcoder), > EDP_PSR_STATUS_STATE_MASK, > EDP_PSR_STATUS_STATE_IDLE, 2, 50, > out_value); > @@ -979,10 +1005,10 @@ static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) > return false; > > if (dev_priv->psr.psr2_enabled) { > - reg = EDP_PSR2_STATUS; > + reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder); > mask = EDP_PSR2_STATUS_STATE_MASK; > } else { > - reg = EDP_PSR_STATUS; > + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); > mask = EDP_PSR_STATUS_STATE_MASK; > } > > @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, > */ > void intel_psr_init(struct drm_i915_private *dev_priv) > { > - u32 val; > - > if (!HAS_PSR(dev_priv)) > return; > > - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? > - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; > - > if (!dev_priv->psr.sink_support) > return; > > + if (IS_HASWELL(dev_priv)) > + /* > + * HSW don't have PSR registers on the same space as transcoder > + * so set this to a value that when subtract to the register > + * in transcoder space results in the right offset for HSW > + */ > + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - _HSW_EDP_PSR_BASE; > + > if (i915_modparams.enable_psr == -1) > if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable) > i915_modparams.enable_psr = 0; > > - /* > - * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR > - * will still keep the error set even after the reset done in the > - * irq_preinstall and irq_uninstall hooks. > - * And enabling in this situation cause the screen to freeze in the > - * first time that PSR HW tries to activate so lets keep PSR disabled > - * to avoid any rendering problems. > - */ > - val = I915_READ(EDP_PSR_IIR); > - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); > - if (val) { > - DRM_DEBUG_KMS("PSR interruption error set\n"); > - dev_priv->psr.sink_not_reliable = true; > - } > - Earlier EDP_PSR_IIR was being checked only in driver init path, now it has been checked for every modeset/fastset path in intel_psr_enable_locked(). Is it ok ? If it is justified why are we not checking it in intel_psr_flush() there also it enables psr. > /* Set link_standby x link_off defaults */ > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > /* HSW and BDW require workarounds that we don't implement. */ > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c > index 25f78196b964..45a9124e53b6 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -2796,7 +2796,7 @@ static int init_broadwell_mmio_info(struct intel_gvt *gvt) > MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); > > MMIO_D(WM_MISC, D_BDW); > - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); > + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); > > MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); > MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index b39226d7f8d2..6e4824daafae 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) > "BUF_ON", > "TG_ON" > }; > - val = I915_READ(EDP_PSR2_STATUS); > + val = I915_READ(EDP_PSR2_STATUS(dev_priv->psr.transcoder)); > status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >> > EDP_PSR2_STATUS_STATE_SHIFT; > if (status_val < ARRAY_SIZE(live_status)) > @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) > "SRDOFFACK", > "SRDENT_ON", > }; > - val = I915_READ(EDP_PSR_STATUS); > + val = I915_READ(EDP_PSR_STATUS(dev_priv->psr.transcoder)); > status_val = (val & EDP_PSR_STATUS_STATE_MASK) >> > EDP_PSR_STATUS_STATE_SHIFT; > if (status_val < ARRAY_SIZE(live_status)) > @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > goto unlock; > > if (psr->psr2_enabled) { > - val = I915_READ(EDP_PSR2_CTL); > + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); > enabled = val & EDP_PSR2_ENABLE; > } else { > - val = I915_READ(EDP_PSR_CTL); > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > enabled = val & EDP_PSR_ENABLE; > } > seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", > @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > * SKL+ Perf counter is reset to 0 everytime DC state is entered > */ > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > - val = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK; > + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv->psr.transcoder)); > + val &= EDP_PSR_PERF_CNT_MASK; > seq_printf(m, "Performance counter: %u\n", val); > } > > @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) > * Reading all 3 registers before hand to minimize crossing a > * frame boundary between register reads > */ > - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) > - su_frames_val[frame / 3] = I915_READ(PSR2_SU_STATUS(frame)); > + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) { > + val = I915_READ(PSR2_SU_STATUS(dev_priv->psr.transcoder, > + frame)); > + su_frames_val[frame / 3] = val; > + } > > seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index eb31c1656cea..be999791abca 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -479,6 +479,7 @@ struct i915_psr { > bool enabled; > struct intel_dp *dp; > enum pipe pipe; > + enum transcoder transcoder; > bool active; > struct work_struct work; > unsigned busy_frontbuffer_bits; > @@ -1330,11 +1331,11 @@ struct drm_i915_private { > */ > u32 gpio_mmio_base; > > + u32 hsw_psr_mmio_adjust; > + > /* MMIO base address for MIPI regs */ > u32 mipi_mmio_base; > > - u32 psr_mmio_base; > - > u32 pps_mmio_base; > > wait_queue_head_t gmbus_wait_queue; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 2abd199093c5..a092b34c269d 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -4186,10 +4186,17 @@ enum { > #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC) > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A) > > -/* HSW+ eDP PSR registers */ > -#define HSW_EDP_PSR_BASE 0x64800 > -#define BDW_EDP_PSR_BASE 0x6f800 > -#define EDP_PSR_CTL _MMIO(dev_priv->psr_mmio_base + 0) > +/* > + * HSW+ eDP PSR registers > + * > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) with just one > + * instance of it > + */ > +#define _HSW_EDP_PSR_BASE 0x64800 > +#define _SRD_CTL_A 0x60800 > +#define _SRD_CTL_EDP 0x6f800 > +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, reg) - dev_priv->hsw_psr_mmio_adjust) > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_CTL_A)) > #define EDP_PSR_ENABLE (1 << 31) > #define BDW_PSR_SINGLE_FRAME (1 << 30) > #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW can't modify */ > @@ -4226,16 +4233,22 @@ enum { > #define EDP_PSR_TRANSCODER_A_SHIFT 8 > #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 > > -#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10) > +#define _SRD_AUX_CTL_A 0x60810 > +#define _SRD_AUX_CTL_EDP 0x6f810 > +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_AUX_CTL_A)) > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26) > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) > #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) > #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) > #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) > > -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ > +#define _SRD_AUX_DATA_A 0x60814 > +#define _SRD_AUX_DATA_EDP 0x6f814 > +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ > > -#define EDP_PSR_STATUS _MMIO(dev_priv->psr_mmio_base + 0x40) > +#define _SRD_STATUS_A 0x60840 > +#define _SRD_STATUS_EDP 0x6f840 > +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ(tran, _SRD_STATUS_A)) > #define EDP_PSR_STATUS_STATE_MASK (7 << 29) > #define EDP_PSR_STATUS_STATE_SHIFT 29 > #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) > @@ -4260,10 +4273,15 @@ enum { > #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) > #define EDP_PSR_STATUS_IDLE_MASK 0xf > > -#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + 0x44) > +#define _SRD_PERF_CNT_A 0x60844 > +#define _SRD_PERF_CNT_EDP 0x6f844 > +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, _SRD_PERF_CNT_A)) > #define EDP_PSR_PERF_CNT_MASK 0xffffff > > -#define EDP_PSR_DEBUG _MMIO(dev_priv->psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ > +/* PSR_MASK on SKL+ */ > +#define _SRD_DEBUG_A 0x60860 > +#define _SRD_DEBUG_EDP 0x6f860 > +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ(tran, _SRD_DEBUG_A)) > #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) > #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) > #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) > @@ -4271,7 +4289,9 @@ enum { > #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ > #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */ > > -#define EDP_PSR2_CTL _MMIO(0x6f900) > +#define _PSR2_CTL_A 0x60900 > +#define _PSR2_CTL_EDP 0x6f900 > +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, _PSR2_CTL_A) > #define EDP_PSR2_ENABLE (1 << 31) > #define EDP_SU_TRACK_ENABLE (1 << 30) > #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ > @@ -4293,8 +4313,8 @@ enum { > #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_TRANS_EDP 0x6f848 > +#define PSR_EVENT(tran) _MMIO_TRANS2(tran, _PSR_EVENT_TRANS_A) > #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) > @@ -4312,15 +4332,16 @@ enum { > #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) > #define PSR_EVENT_PSR_DISABLE (1 << 0) > > -#define EDP_PSR2_STATUS _MMIO(0x6f940) > +#define _PSR2_STATUS_A 0x60940 > +#define _PSR2_STATUS_EDP 0x6f940 > +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, _PSR2_STATUS_A) > #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) > #define EDP_PSR2_STATUS_STATE_SHIFT 28 > > -#define _PSR2_SU_STATUS_0 0x6F914 > -#define _PSR2_SU_STATUS_1 0x6F918 > -#define _PSR2_SU_STATUS_2 0x6F91C > -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) > -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame) / 3)) > +#define _PSR2_SU_STATUS_A 0x60914 > +#define _PSR2_SU_STATUS_EDP 0x6f914 > +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, _PSR2_SU_STATUS_A) + (index) * 4) > +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, (frame) / 3)) > #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) > #define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame)) > #define PSR2_SU_STATUS_FRAMES 8 > -- > 2.22.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2019-08-21 at 10:06 +0530, Anshuman Gupta wrote: > On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: > > PSR registers are a mess, some have the full address while others > > just > > have the additional offset from psr_mmio_base. > > > > For BDW+ psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + > > 0x800 and using it makes more difficult for people with an PSR > > register address or PSR register name from from BSpec as i915 also > > don't match the BSpec names. > > For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR registers > > are > > only available in DDIA. > > > > Other reason to make relative to transcoder is that since BDW every > > transcoder have PSR registers, so in theory it should be possible > > to > > have PSR enabled in a non-eDP transcoder. > > > > So for BDW+ we can use _TRANS2() to get the register offset of any > > PSR register in any transcoder while for HSW we have _HSW_PSR_ADJ > > that will calculate the register offset for the single PSR > > instance, > > noting that we are already guarded about trying to enable PSR in > > other > > port than DDIA on HSW by the 'if (dig_port->base.port != PORT_A)' > > in > > intel_psr_compute_config(), this check should only be valid for HSW > > and will be changed in future. > > PSR2 registers and PSR_EVENT was added after Haswell so that is why > > _PSR_ADJ() is not used in some macros. > > > > The only registers that can not be relative to transcoder are > > PSR_IMR and PSR_IIR that are not relative to anything, so keeping > > it > > hardcoded. That changed for TGL but it will be handled in another > > patch. > > > > Also removing BDW_EDP_PSR_BASE from GVT because it is not used as > > it > > is the only PSR register that GVT have. > > > > v5: > > - Macros changed to be more explicit about HSW (Dhinakaran) > > - Squashed with the patch that added the tran parameter to the > > macros (Dhinakaran) > > > > v6: > > - Checking for interruption errors after module reload in the > > transcoder that will be used (Dhinakaran) > > - Using lowercase to the registers offsets > > > > v7: > > - Removing IS_HASWELL() from registers macros(Jani) > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Zhi Wang <zhi.a.wang@intel.com> > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_psr.c | 104 +++++++++++++---- > > ------ > > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- > > drivers/gpu/drm/i915/i915_drv.h | 5 +- > > drivers/gpu/drm/i915/i915_reg.h | 57 +++++++++---- > > 5 files changed, 113 insertions(+), 73 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 3bfb720560c2..77232f6bca17 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct intel_dp > > *intel_dp) > > > > BUILD_BUG_ON(sizeof(aux_msg) > 20); > > for (i = 0; i < sizeof(aux_msg); i += 4) > > - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), > > + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i > > >> 2), > > intel_dp_pack_aux(&aux_msg[i], > > sizeof(aux_msg) - i)); > > > > aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, > > 0); > > @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct intel_dp > > *intel_dp) > > > > /* Select only valid bits for SRD_AUX_CTL */ > > aux_ctl &= psr_aux_mask; > > - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > > + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl); > > } > > > > static void intel_psr_enable_sink(struct intel_dp *intel_dp) > > @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct intel_dp > > *intel_dp) > > if (INTEL_GEN(dev_priv) >= 8) > > val |= EDP_PSR_CRC_ENABLE; > > > > - val |= I915_READ(EDP_PSR_CTL) & > > EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; > > - I915_WRITE(EDP_PSR_CTL, val); > > + val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & > > + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > > } > > > > static void hsw_activate_psr2(struct intel_dp *intel_dp) > > @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct intel_dp > > *intel_dp) > > * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec > > is > > * recommending keep this bit unset while PSR2 is enabled. > > */ > > - I915_WRITE(EDP_PSR_CTL, 0); > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); > > > > - I915_WRITE(EDP_PSR2_CTL, val); > > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); > > } > > > > static bool intel_psr2_config_valid(struct intel_dp *intel_dp, > > @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct intel_dp > > *intel_dp, > > > > /* > > * HSW spec explicitly says PSR is tied to port A. > > - * BDW+ platforms with DDI implementation of PSR have different > > - * PSR registers per transcoder and we only implement > > transcoder EDP > > - * ones. Since by Display design transcoder EDP is tied to port > > A > > - * we can safely escape based on the port A. > > + * BDW+ platforms have a instance of PSR registers per > > transcoder but > > + * for now it only supports one instance of PSR, so lets keep > > it > > + * hardcoded to PORT_A > > */ > > if (dig_port->base.port != PORT_A) { > > DRM_DEBUG_KMS("PSR condition failed: Port not > > supported\n"); > > @@ -649,8 +649,8 @@ static void intel_psr_activate(struct intel_dp > > *intel_dp) > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > if (INTEL_GEN(dev_priv) >= 9) > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv- > > >psr.transcoder)) & EDP_PSR2_ENABLE); > > + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & > > EDP_PSR_ENABLE); > > WARN_ON(dev_priv->psr.active); > > lockdep_assert_held(&dev_priv->psr.lock); > > > > @@ -720,19 +720,37 @@ static void intel_psr_enable_source(struct > > intel_dp *intel_dp, > > if (INTEL_GEN(dev_priv) < 11) > > mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; > > > > - I915_WRITE(EDP_PSR_DEBUG, mask); > > + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask); > > } > > > > static void intel_psr_enable_locked(struct drm_i915_private > > *dev_priv, > > const struct intel_crtc_state > > *crtc_state) > > { > > struct intel_dp *intel_dp = dev_priv->psr.dp; > > + u32 val; > > > > WARN_ON(dev_priv->psr.enabled); > > > > dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, > > crtc_state); > > dev_priv->psr.busy_frontbuffer_bits = 0; > > dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)- > > >pipe; > > + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > > + > > + /* > > + * If a PSR error happened and the driver is reloaded, the > > EDP_PSR_IIR > > + * will still keep the error set even after the reset done in > > the > > + * irq_preinstall and irq_uninstall hooks. > > + * And enabling in this situation cause the screen to freeze in > > the > > + * first time that PSR HW tries to activate so lets keep PSR > > disabled > > + * to avoid any rendering problems. > > + */ > > + val = I915_READ(EDP_PSR_IIR); > > + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv->psr.transcoder)); > > + if (val) { > > + dev_priv->psr.sink_not_reliable = true; > > + DRM_DEBUG_KMS("PSR interruption error set, not enabling > > PSR\n"); > > + return; > > + } > > > > DRM_DEBUG_KMS("Enabling PSR%s\n", > > dev_priv->psr.psr2_enabled ? "2" : "1"); > > @@ -782,20 +800,27 @@ static void intel_psr_exit(struct > > drm_i915_private *dev_priv) > > u32 val; > > > > if (!dev_priv->psr.active) { > > - if (INTEL_GEN(dev_priv) >= 9) > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > EDP_PSR2_ENABLE); > > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > + if (INTEL_GEN(dev_priv) >= 9) { > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > >psr.transcoder)); > > + WARN_ON(val & EDP_PSR2_ENABLE); > > + } > > + > > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > > + WARN_ON(val & EDP_PSR_ENABLE); > > + > > return; > > } > > > > if (dev_priv->psr.psr2_enabled) { > > - val = I915_READ(EDP_PSR2_CTL); > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > >psr.transcoder)); > > WARN_ON(!(val & EDP_PSR2_ENABLE)); > > - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE); > > + val &= ~EDP_PSR2_ENABLE; > > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), > > val); > > } else { > > - val = I915_READ(EDP_PSR_CTL); > > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > > WARN_ON(!(val & EDP_PSR_ENABLE)); > > - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); > > + val &= ~EDP_PSR_ENABLE; > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > > } > > dev_priv->psr.active = false; > > } > > @@ -817,10 +842,10 @@ static void intel_psr_disable_locked(struct > > intel_dp *intel_dp) > > intel_psr_exit(dev_priv); > > > > if (dev_priv->psr.psr2_enabled) { > > - psr_status = EDP_PSR2_STATUS; > > + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder); > > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > > } else { > > - psr_status = EDP_PSR_STATUS; > > + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder); > > psr_status_mask = EDP_PSR_STATUS_STATE_MASK; > > } > > > > @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct > > intel_crtc_state *new_crtc_state, > > * defensive enough to cover everything. > > */ > > > > - return __intel_wait_for_register(&dev_priv->uncore, > > EDP_PSR_STATUS, > > + return __intel_wait_for_register(&dev_priv->uncore, > > + EDP_PSR_STATUS(dev_priv- > > >psr.transcoder), > > EDP_PSR_STATUS_STATE_MASK, > > EDP_PSR_STATUS_STATE_IDLE, 2, > > 50, > > out_value); > > @@ -979,10 +1005,10 @@ static bool > > __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) > > return false; > > > > if (dev_priv->psr.psr2_enabled) { > > - reg = EDP_PSR2_STATUS; > > + reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder); > > mask = EDP_PSR2_STATUS_STATE_MASK; > > } else { > > - reg = EDP_PSR_STATUS; > > + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); > > mask = EDP_PSR_STATUS_STATE_MASK; > > } > > > > @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct > > drm_i915_private *dev_priv, > > */ > > void intel_psr_init(struct drm_i915_private *dev_priv) > > { > > - u32 val; > > - > > if (!HAS_PSR(dev_priv)) > > return; > > > > - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? > > - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; > > - > > if (!dev_priv->psr.sink_support) > > return; > > > > + if (IS_HASWELL(dev_priv)) > > + /* > > + * HSW don't have PSR registers on the same space as > > transcoder > > + * so set this to a value that when subtract to the > > register > > + * in transcoder space results in the right offset for > > HSW > > + */ > > + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - > > _HSW_EDP_PSR_BASE; > > + > > if (i915_modparams.enable_psr == -1) > > if (INTEL_GEN(dev_priv) < 9 || !dev_priv- > > >vbt.psr.enable) > > i915_modparams.enable_psr = 0; > > > > - /* > > - * If a PSR error happened and the driver is reloaded, the > > EDP_PSR_IIR > > - * will still keep the error set even after the reset done in > > the > > - * irq_preinstall and irq_uninstall hooks. > > - * And enabling in this situation cause the screen to freeze in > > the > > - * first time that PSR HW tries to activate so lets keep PSR > > disabled > > - * to avoid any rendering problems. > > - */ > > - val = I915_READ(EDP_PSR_IIR); > > - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); > > - if (val) { > > - DRM_DEBUG_KMS("PSR interruption error set\n"); > > - dev_priv->psr.sink_not_reliable = true; > > - } > > - > Earlier EDP_PSR_IIR was being checked only in driver init path, > now it has been checked for every modeset/fastset path in > intel_psr_enable_locked(). Is it ok ? > If it is justified why are we not checking it in intel_psr_flush() > there also it enables psr. I moved it primarily because on intel_psr_enable_locked() we have the transcoder that will be used, doing on init we would need to check for all transcoders and in case of a error set in one transcoder we would need to fail initialization as PSR could be enabled on that transcoder. No need to do that on intel_psr_flush() because PSR interruptions can be triggered even with PSR inactive. > > /* Set link_standby x link_off defaults */ > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > /* HSW and BDW require workarounds that we don't > > implement. */ > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > > b/drivers/gpu/drm/i915/gvt/handlers.c > > index 25f78196b964..45a9124e53b6 100644 > > --- a/drivers/gpu/drm/i915/gvt/handlers.c > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > > @@ -2796,7 +2796,7 @@ static int init_broadwell_mmio_info(struct > > intel_gvt *gvt) > > MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); > > > > MMIO_D(WM_MISC, D_BDW); > > - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); > > + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); > > > > MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); > > MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > b/drivers/gpu/drm/i915/i915_debugfs.c > > index b39226d7f8d2..6e4824daafae 100644 > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private > > *dev_priv, struct seq_file *m) > > "BUF_ON", > > "TG_ON" > > }; > > - val = I915_READ(EDP_PSR2_STATUS); > > + val = I915_READ(EDP_PSR2_STATUS(dev_priv- > > >psr.transcoder)); > > status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >> > > EDP_PSR2_STATUS_STATE_SHIFT; > > if (status_val < ARRAY_SIZE(live_status)) > > @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private > > *dev_priv, struct seq_file *m) > > "SRDOFFACK", > > "SRDENT_ON", > > }; > > - val = I915_READ(EDP_PSR_STATUS); > > + val = I915_READ(EDP_PSR_STATUS(dev_priv- > > >psr.transcoder)); > > status_val = (val & EDP_PSR_STATUS_STATE_MASK) >> > > EDP_PSR_STATUS_STATE_SHIFT; > > if (status_val < ARRAY_SIZE(live_status)) > > @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct > > seq_file *m, void *data) > > goto unlock; > > > > if (psr->psr2_enabled) { > > - val = I915_READ(EDP_PSR2_CTL); > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > >psr.transcoder)); > > enabled = val & EDP_PSR2_ENABLE; > > } else { > > - val = I915_READ(EDP_PSR_CTL); > > + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); > > enabled = val & EDP_PSR_ENABLE; > > } > > seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", > > @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct > > seq_file *m, void *data) > > * SKL+ Perf counter is reset to 0 everytime DC state is > > entered > > */ > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > - val = I915_READ(EDP_PSR_PERF_CNT) & > > EDP_PSR_PERF_CNT_MASK; > > + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv- > > >psr.transcoder)); > > + val &= EDP_PSR_PERF_CNT_MASK; > > seq_printf(m, "Performance counter: %u\n", val); > > } > > > > @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct > > seq_file *m, void *data) > > * Reading all 3 registers before hand to minimize > > crossing a > > * frame boundary between register reads > > */ > > - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += > > 3) > > - su_frames_val[frame / 3] = > > I915_READ(PSR2_SU_STATUS(frame)); > > + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += > > 3) { > > + val = I915_READ(PSR2_SU_STATUS(dev_priv- > > >psr.transcoder, > > + frame)); > > + su_frames_val[frame / 3] = val; > > + } > > > > seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > b/drivers/gpu/drm/i915/i915_drv.h > > index eb31c1656cea..be999791abca 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -479,6 +479,7 @@ struct i915_psr { > > bool enabled; > > struct intel_dp *dp; > > enum pipe pipe; > > + enum transcoder transcoder; > > bool active; > > struct work_struct work; > > unsigned busy_frontbuffer_bits; > > @@ -1330,11 +1331,11 @@ struct drm_i915_private { > > */ > > u32 gpio_mmio_base; > > > > + u32 hsw_psr_mmio_adjust; > > + > > /* MMIO base address for MIPI regs */ > > u32 mipi_mmio_base; > > > > - u32 psr_mmio_base; > > - > > u32 pps_mmio_base; > > > > wait_queue_head_t gmbus_wait_queue; > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > b/drivers/gpu/drm/i915/i915_reg.h > > index 2abd199093c5..a092b34c269d 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -4186,10 +4186,17 @@ enum { > > #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC) > > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A) > > > > -/* HSW+ eDP PSR registers */ > > -#define HSW_EDP_PSR_BASE 0x64800 > > -#define BDW_EDP_PSR_BASE 0x6f800 > > -#define EDP_PSR_CTL _MMIO(dev_priv- > > >psr_mmio_base + 0) > > +/* > > + * HSW+ eDP PSR registers > > + * > > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) > > with just one > > + * instance of it > > + */ > > +#define _HSW_EDP_PSR_BASE 0x64800 > > +#define _SRD_CTL_A 0x60800 > > +#define _SRD_CTL_EDP 0x6f800 > > +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, > > reg) - dev_priv->hsw_psr_mmio_adjust) > > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, > > _SRD_CTL_A)) > > #define EDP_PSR_ENABLE (1 << 31) > > #define BDW_PSR_SINGLE_FRAME (1 << 30) > > #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW > > can't modify */ > > @@ -4226,16 +4233,22 @@ enum { > > #define EDP_PSR_TRANSCODER_A_SHIFT 8 > > #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 > > > > -#define EDP_PSR_AUX_CTL _MMIO(dev_priv- > > >psr_mmio_base + 0x10) > > +#define _SRD_AUX_CTL_A 0x60810 > > +#define _SRD_AUX_CTL_EDP 0x6f810 > > +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ( > > tran, _SRD_AUX_CTL_A)) > > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26) > > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) > > #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) > > #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) > > #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) > > > > -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv- > > >psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ > > +#define _SRD_AUX_DATA_A 0x60814 > > +#define _SRD_AUX_DATA_EDP 0x6f814 > > +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, > > _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ > > > > -#define EDP_PSR_STATUS _MMIO(dev_priv- > > >psr_mmio_base + 0x40) > > +#define _SRD_STATUS_A 0x60840 > > +#define _SRD_STATUS_EDP 0x6f840 > > +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ( > > tran, _SRD_STATUS_A)) > > #define EDP_PSR_STATUS_STATE_MASK (7 << 29) > > #define EDP_PSR_STATUS_STATE_SHIFT 29 > > #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) > > @@ -4260,10 +4273,15 @@ enum { > > #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) > > #define EDP_PSR_STATUS_IDLE_MASK 0xf > > > > -#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + > > 0x44) > > +#define _SRD_PERF_CNT_A 0x60844 > > +#define _SRD_PERF_CNT_EDP 0x6f844 > > +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, > > _SRD_PERF_CNT_A)) > > #define EDP_PSR_PERF_CNT_MASK 0xffffff > > > > -#define EDP_PSR_DEBUG _MMIO(dev_priv- > > >psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ > > +/* PSR_MASK on SKL+ */ > > +#define _SRD_DEBUG_A 0x60860 > > +#define _SRD_DEBUG_EDP 0x6f860 > > +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ( > > tran, _SRD_DEBUG_A)) > > #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) > > #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) > > #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) > > @@ -4271,7 +4289,9 @@ enum { > > #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* > > Reserved in ICL+ */ > > #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ > > */ > > > > -#define EDP_PSR2_CTL _MMIO(0x6f900) > > +#define _PSR2_CTL_A 0x60900 > > +#define _PSR2_CTL_EDP 0x6f900 > > +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, _PSR2_CTL_A) > > #define EDP_PSR2_ENABLE (1 << 31) > > #define EDP_SU_TRACK_ENABLE (1 << 30) > > #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ > > @@ -4293,8 +4313,8 @@ enum { > > #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_TRANS_EDP 0x6f848 > > +#define PSR_EVENT(tran) _MMIO_TRANS2(tr > > an, _PSR_EVENT_TRANS_A) > > #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) > > @@ -4312,15 +4332,16 @@ enum { > > #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) > > #define PSR_EVENT_PSR_DISABLE (1 << 0) > > > > -#define EDP_PSR2_STATUS _MMIO(0x6f940) > > +#define _PSR2_STATUS_A 0x60940 > > +#define _PSR2_STATUS_EDP 0x6f940 > > +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, > > _PSR2_STATUS_A) > > #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) > > #define EDP_PSR2_STATUS_STATE_SHIFT 28 > > > > -#define _PSR2_SU_STATUS_0 0x6F914 > > -#define _PSR2_SU_STATUS_1 0x6F918 > > -#define _PSR2_SU_STATUS_2 0x6F91C > > -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index > > ), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) > > -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame > > ) / 3)) > > +#define _PSR2_SU_STATUS_A 0x60914 > > +#define _PSR2_SU_STATUS_EDP 0x6f914 > > +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, > > _PSR2_SU_STATUS_A) + (index) * 4) > > +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, > > (frame) / 3)) > > #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) > > #define PSR2_SU_STATUS_MASK(frame) (0x3ff << > > PSR2_SU_STATUS_SHIFT(frame)) > > #define PSR2_SU_STATUS_FRAMES 8 > > -- > > 2.22.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/22/2019 1:36 AM, Souza, Jose wrote: > On Wed, 2019-08-21 at 10:06 +0530, Anshuman Gupta wrote: >> On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: >>> PSR registers are a mess, some have the full address while others >>> just >>> have the additional offset from psr_mmio_base. >>> >>> For BDW+ psr_mmio_base is nothing more than TRANSCODER_EDP_OFFSET + >>> 0x800 and using it makes more difficult for people with an PSR >>> register address or PSR register name from from BSpec as i915 also >>> don't match the BSpec names. >>> For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR registers >>> are >>> only available in DDIA. >>> >>> Other reason to make relative to transcoder is that since BDW every >>> transcoder have PSR registers, so in theory it should be possible >>> to >>> have PSR enabled in a non-eDP transcoder. >>> >>> So for BDW+ we can use _TRANS2() to get the register offset of any >>> PSR register in any transcoder while for HSW we have _HSW_PSR_ADJ >>> that will calculate the register offset for the single PSR >>> instance, >>> noting that we are already guarded about trying to enable PSR in >>> other >>> port than DDIA on HSW by the 'if (dig_port->base.port != PORT_A)' >>> in >>> intel_psr_compute_config(), this check should only be valid for HSW >>> and will be changed in future. >>> PSR2 registers and PSR_EVENT was added after Haswell so that is why >>> _PSR_ADJ() is not used in some macros. >>> >>> The only registers that can not be relative to transcoder are >>> PSR_IMR and PSR_IIR that are not relative to anything, so keeping >>> it >>> hardcoded. That changed for TGL but it will be handled in another >>> patch. >>> >>> Also removing BDW_EDP_PSR_BASE from GVT because it is not used as >>> it >>> is the only PSR register that GVT have. >>> >>> v5: >>> - Macros changed to be more explicit about HSW (Dhinakaran) >>> - Squashed with the patch that added the tran parameter to the >>> macros (Dhinakaran) >>> >>> v6: >>> - Checking for interruption errors after module reload in the >>> transcoder that will be used (Dhinakaran) >>> - Using lowercase to the registers offsets >>> >>> v7: >>> - Removing IS_HASWELL() from registers macros(Jani) >>> >>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> Cc: Zhi Wang <zhi.a.wang@intel.com> >>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> >>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >>> --- >>> drivers/gpu/drm/i915/display/intel_psr.c | 104 +++++++++++++---- >>> ------ >>> drivers/gpu/drm/i915/gvt/handlers.c | 2 +- >>> drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- >>> drivers/gpu/drm/i915/i915_drv.h | 5 +- >>> drivers/gpu/drm/i915/i915_reg.h | 57 +++++++++---- >>> 5 files changed, 113 insertions(+), 73 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c >>> b/drivers/gpu/drm/i915/display/intel_psr.c >>> index 3bfb720560c2..77232f6bca17 100644 >>> --- a/drivers/gpu/drm/i915/display/intel_psr.c >>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c >>> @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct intel_dp >>> *intel_dp) >>> >>> BUILD_BUG_ON(sizeof(aux_msg) > 20); >>> for (i = 0; i < sizeof(aux_msg); i += 4) >>> - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), >>> + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i >>>>> 2), >>> intel_dp_pack_aux(&aux_msg[i], >>> sizeof(aux_msg) - i)); >>> >>> aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, >>> 0); >>> @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct intel_dp >>> *intel_dp) >>> >>> /* Select only valid bits for SRD_AUX_CTL */ >>> aux_ctl &= psr_aux_mask; >>> - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); >>> + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl); >>> } >>> >>> static void intel_psr_enable_sink(struct intel_dp *intel_dp) >>> @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct intel_dp >>> *intel_dp) >>> if (INTEL_GEN(dev_priv) >= 8) >>> val |= EDP_PSR_CRC_ENABLE; >>> >>> - val |= I915_READ(EDP_PSR_CTL) & >>> EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; >>> - I915_WRITE(EDP_PSR_CTL, val); >>> + val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & >>> + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); >>> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); >>> } >>> >>> static void hsw_activate_psr2(struct intel_dp *intel_dp) >>> @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct intel_dp >>> *intel_dp) >>> * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec >>> is >>> * recommending keep this bit unset while PSR2 is enabled. >>> */ >>> - I915_WRITE(EDP_PSR_CTL, 0); >>> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); >>> >>> - I915_WRITE(EDP_PSR2_CTL, val); >>> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); >>> } >>> >>> static bool intel_psr2_config_valid(struct intel_dp *intel_dp, >>> @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct intel_dp >>> *intel_dp, >>> >>> /* >>> * HSW spec explicitly says PSR is tied to port A. >>> - * BDW+ platforms with DDI implementation of PSR have different >>> - * PSR registers per transcoder and we only implement >>> transcoder EDP >>> - * ones. Since by Display design transcoder EDP is tied to port >>> A >>> - * we can safely escape based on the port A. >>> + * BDW+ platforms have a instance of PSR registers per >>> transcoder but >>> + * for now it only supports one instance of PSR, so lets keep >>> it >>> + * hardcoded to PORT_A >>> */ >>> if (dig_port->base.port != PORT_A) { >>> DRM_DEBUG_KMS("PSR condition failed: Port not >>> supported\n"); >>> @@ -649,8 +649,8 @@ static void intel_psr_activate(struct intel_dp >>> *intel_dp) >>> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); >>> >>> if (INTEL_GEN(dev_priv) >= 9) >>> - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); >>> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); >>> + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv- >>>> psr.transcoder)) & EDP_PSR2_ENABLE); >>> + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & >>> EDP_PSR_ENABLE); >>> WARN_ON(dev_priv->psr.active); >>> lockdep_assert_held(&dev_priv->psr.lock); >>> >>> @@ -720,19 +720,37 @@ static void intel_psr_enable_source(struct >>> intel_dp *intel_dp, >>> if (INTEL_GEN(dev_priv) < 11) >>> mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; >>> >>> - I915_WRITE(EDP_PSR_DEBUG, mask); >>> + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask); >>> } >>> >>> static void intel_psr_enable_locked(struct drm_i915_private >>> *dev_priv, >>> const struct intel_crtc_state >>> *crtc_state) >>> { >>> struct intel_dp *intel_dp = dev_priv->psr.dp; >>> + u32 val; >>> >>> WARN_ON(dev_priv->psr.enabled); >>> >>> dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, >>> crtc_state); >>> dev_priv->psr.busy_frontbuffer_bits = 0; >>> dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)- >>>> pipe; >>> + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; >>> + >>> + /* >>> + * If a PSR error happened and the driver is reloaded, the >>> EDP_PSR_IIR >>> + * will still keep the error set even after the reset done in >>> the >>> + * irq_preinstall and irq_uninstall hooks. >>> + * And enabling in this situation cause the screen to freeze in >>> the >>> + * first time that PSR HW tries to activate so lets keep PSR >>> disabled >>> + * to avoid any rendering problems. >>> + */ >>> + val = I915_READ(EDP_PSR_IIR); >>> + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv->psr.transcoder)); >>> + if (val) { >>> + dev_priv->psr.sink_not_reliable = true; >>> + DRM_DEBUG_KMS("PSR interruption error set, not enabling >>> PSR\n"); >>> + return; >>> + } >>> >>> DRM_DEBUG_KMS("Enabling PSR%s\n", >>> dev_priv->psr.psr2_enabled ? "2" : "1"); >>> @@ -782,20 +800,27 @@ static void intel_psr_exit(struct >>> drm_i915_private *dev_priv) >>> u32 val; >>> >>> if (!dev_priv->psr.active) { >>> - if (INTEL_GEN(dev_priv) >= 9) >>> - WARN_ON(I915_READ(EDP_PSR2_CTL) & >>> EDP_PSR2_ENABLE); >>> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); >>> + if (INTEL_GEN(dev_priv) >= 9) { >>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>> psr.transcoder)); >>> + WARN_ON(val & EDP_PSR2_ENABLE); >>> + } >>> + >>> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); >>> + WARN_ON(val & EDP_PSR_ENABLE); >>> + >>> return; >>> } >>> >>> if (dev_priv->psr.psr2_enabled) { >>> - val = I915_READ(EDP_PSR2_CTL); >>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>> psr.transcoder)); >>> WARN_ON(!(val & EDP_PSR2_ENABLE)); >>> - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE); >>> + val &= ~EDP_PSR2_ENABLE; >>> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), >>> val); >>> } else { >>> - val = I915_READ(EDP_PSR_CTL); >>> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); >>> WARN_ON(!(val & EDP_PSR_ENABLE)); >>> - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); >>> + val &= ~EDP_PSR_ENABLE; >>> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); >>> } >>> dev_priv->psr.active = false; >>> } >>> @@ -817,10 +842,10 @@ static void intel_psr_disable_locked(struct >>> intel_dp *intel_dp) >>> intel_psr_exit(dev_priv); >>> >>> if (dev_priv->psr.psr2_enabled) { >>> - psr_status = EDP_PSR2_STATUS; >>> + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder); >>> psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; >>> } else { >>> - psr_status = EDP_PSR_STATUS; >>> + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder); >>> psr_status_mask = EDP_PSR_STATUS_STATE_MASK; >>> } >>> >>> @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct >>> intel_crtc_state *new_crtc_state, >>> * defensive enough to cover everything. >>> */ >>> >>> - return __intel_wait_for_register(&dev_priv->uncore, >>> EDP_PSR_STATUS, >>> + return __intel_wait_for_register(&dev_priv->uncore, >>> + EDP_PSR_STATUS(dev_priv- >>>> psr.transcoder), >>> EDP_PSR_STATUS_STATE_MASK, >>> EDP_PSR_STATUS_STATE_IDLE, 2, >>> 50, >>> out_value); >>> @@ -979,10 +1005,10 @@ static bool >>> __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) >>> return false; >>> >>> if (dev_priv->psr.psr2_enabled) { >>> - reg = EDP_PSR2_STATUS; >>> + reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder); >>> mask = EDP_PSR2_STATUS_STATE_MASK; >>> } else { >>> - reg = EDP_PSR_STATUS; >>> + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); >>> mask = EDP_PSR_STATUS_STATE_MASK; >>> } >>> >>> @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct >>> drm_i915_private *dev_priv, >>> */ >>> void intel_psr_init(struct drm_i915_private *dev_priv) >>> { >>> - u32 val; >>> - >>> if (!HAS_PSR(dev_priv)) >>> return; >>> >>> - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? >>> - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; >>> - >>> if (!dev_priv->psr.sink_support) >>> return; >>> >>> + if (IS_HASWELL(dev_priv)) >>> + /* >>> + * HSW don't have PSR registers on the same space as >>> transcoder >>> + * so set this to a value that when subtract to the >>> register >>> + * in transcoder space results in the right offset for >>> HSW >>> + */ >>> + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - >>> _HSW_EDP_PSR_BASE; >>> + >>> if (i915_modparams.enable_psr == -1) >>> if (INTEL_GEN(dev_priv) < 9 || !dev_priv- >>>> vbt.psr.enable) >>> i915_modparams.enable_psr = 0; >>> >>> - /* >>> - * If a PSR error happened and the driver is reloaded, the >>> EDP_PSR_IIR >>> - * will still keep the error set even after the reset done in >>> the >>> - * irq_preinstall and irq_uninstall hooks. >>> - * And enabling in this situation cause the screen to freeze in >>> the >>> - * first time that PSR HW tries to activate so lets keep PSR >>> disabled >>> - * to avoid any rendering problems. >>> - */ >>> - val = I915_READ(EDP_PSR_IIR); >>> - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); >>> - if (val) { >>> - DRM_DEBUG_KMS("PSR interruption error set\n"); >>> - dev_priv->psr.sink_not_reliable = true; >>> - } >>> - >> Earlier EDP_PSR_IIR was being checked only in driver init path, >> now it has been checked for every modeset/fastset path in >> intel_psr_enable_locked(). Is it ok ? >> If it is justified why are we not checking it in intel_psr_flush() >> there also it enables psr. > > I moved it primarily because on intel_psr_enable_locked() we have the > transcoder that will be used, doing on init we would need to check for > all transcoders and in case of a error set in one transcoder we would > need to fail initialization as PSR could be enabled on that transcoder. That is correct, but with this EDP_PSR_IIR error is getting checked in every modeset/fastset which is redundant, as it is already being check at interrupt handler. IMO it should be only checked somehow at initel_psr_init(), even comment also reflect same thing ("If a PSR error happened and the driver is reloaded..."). If there no other way to handle this, comment should be change accordingly. > > No need to do that on intel_psr_flush() because PSR interruptions can > be triggered even with PSR inactive. > > >>> /* Set link_standby x link_off defaults */ >>> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) >>> /* HSW and BDW require workarounds that we don't >>> implement. */ >>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c >>> b/drivers/gpu/drm/i915/gvt/handlers.c >>> index 25f78196b964..45a9124e53b6 100644 >>> --- a/drivers/gpu/drm/i915/gvt/handlers.c >>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c >>> @@ -2796,7 +2796,7 @@ static int init_broadwell_mmio_info(struct >>> intel_gvt *gvt) >>> MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); >>> >>> MMIO_D(WM_MISC, D_BDW); >>> - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); >>> + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); >>> >>> MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); >>> MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); >>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c >>> b/drivers/gpu/drm/i915/i915_debugfs.c >>> index b39226d7f8d2..6e4824daafae 100644 >>> --- a/drivers/gpu/drm/i915/i915_debugfs.c >>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >>> @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private >>> *dev_priv, struct seq_file *m) >>> "BUF_ON", >>> "TG_ON" >>> }; >>> - val = I915_READ(EDP_PSR2_STATUS); >>> + val = I915_READ(EDP_PSR2_STATUS(dev_priv- >>>> psr.transcoder)); >>> status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >> >>> EDP_PSR2_STATUS_STATE_SHIFT; >>> if (status_val < ARRAY_SIZE(live_status)) >>> @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private >>> *dev_priv, struct seq_file *m) >>> "SRDOFFACK", >>> "SRDENT_ON", >>> }; >>> - val = I915_READ(EDP_PSR_STATUS); >>> + val = I915_READ(EDP_PSR_STATUS(dev_priv- >>>> psr.transcoder)); >>> status_val = (val & EDP_PSR_STATUS_STATE_MASK) >> >>> EDP_PSR_STATUS_STATE_SHIFT; >>> if (status_val < ARRAY_SIZE(live_status)) >>> @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct >>> seq_file *m, void *data) >>> goto unlock; >>> >>> if (psr->psr2_enabled) { >>> - val = I915_READ(EDP_PSR2_CTL); >>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>> psr.transcoder)); >>> enabled = val & EDP_PSR2_ENABLE; >>> } else { >>> - val = I915_READ(EDP_PSR_CTL); >>> + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); >>> enabled = val & EDP_PSR_ENABLE; >>> } >>> seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", >>> @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct >>> seq_file *m, void *data) >>> * SKL+ Perf counter is reset to 0 everytime DC state is >>> entered >>> */ >>> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { >>> - val = I915_READ(EDP_PSR_PERF_CNT) & >>> EDP_PSR_PERF_CNT_MASK; >>> + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv- >>>> psr.transcoder)); >>> + val &= EDP_PSR_PERF_CNT_MASK; >>> seq_printf(m, "Performance counter: %u\n", val); >>> } >>> >>> @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct >>> seq_file *m, void *data) >>> * Reading all 3 registers before hand to minimize >>> crossing a >>> * frame boundary between register reads >>> */ >>> - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += >>> 3) >>> - su_frames_val[frame / 3] = >>> I915_READ(PSR2_SU_STATUS(frame)); >>> + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += >>> 3) { >>> + val = I915_READ(PSR2_SU_STATUS(dev_priv- >>>> psr.transcoder, >>> + frame)); >>> + su_frames_val[frame / 3] = val; >>> + } >>> >>> seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); >>> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>> b/drivers/gpu/drm/i915/i915_drv.h >>> index eb31c1656cea..be999791abca 100644 >>> --- a/drivers/gpu/drm/i915/i915_drv.h >>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>> @@ -479,6 +479,7 @@ struct i915_psr { >>> bool enabled; >>> struct intel_dp *dp; >>> enum pipe pipe; >>> + enum transcoder transcoder; >>> bool active; >>> struct work_struct work; >>> unsigned busy_frontbuffer_bits; >>> @@ -1330,11 +1331,11 @@ struct drm_i915_private { >>> */ >>> u32 gpio_mmio_base; >>> >>> + u32 hsw_psr_mmio_adjust; >>> + >>> /* MMIO base address for MIPI regs */ >>> u32 mipi_mmio_base; >>> >>> - u32 psr_mmio_base; >>> - >>> u32 pps_mmio_base; >>> >>> wait_queue_head_t gmbus_wait_queue; >>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>> b/drivers/gpu/drm/i915/i915_reg.h >>> index 2abd199093c5..a092b34c269d 100644 >>> --- a/drivers/gpu/drm/i915/i915_reg.h >>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>> @@ -4186,10 +4186,17 @@ enum { >>> #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC) >>> #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A) >>> >>> -/* HSW+ eDP PSR registers */ >>> -#define HSW_EDP_PSR_BASE 0x64800 >>> -#define BDW_EDP_PSR_BASE 0x6f800 >>> -#define EDP_PSR_CTL _MMIO(dev_priv- >>>> psr_mmio_base + 0) >>> +/* >>> + * HSW+ eDP PSR registers >>> + * >>> + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) >>> with just one >>> + * instance of it >>> + */ >>> +#define _HSW_EDP_PSR_BASE 0x64800 >>> +#define _SRD_CTL_A 0x60800 >>> +#define _SRD_CTL_EDP 0x6f800 >>> +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, >>> reg) - dev_priv->hsw_psr_mmio_adjust) >>> +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, >>> _SRD_CTL_A)) >>> #define EDP_PSR_ENABLE (1 << 31) >>> #define BDW_PSR_SINGLE_FRAME (1 << 30) >>> #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW >>> can't modify */ >>> @@ -4226,16 +4233,22 @@ enum { >>> #define EDP_PSR_TRANSCODER_A_SHIFT 8 >>> #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 >>> >>> -#define EDP_PSR_AUX_CTL _MMIO(dev_priv- >>>> psr_mmio_base + 0x10) >>> +#define _SRD_AUX_CTL_A 0x60810 >>> +#define _SRD_AUX_CTL_EDP 0x6f810 >>> +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ( >>> tran, _SRD_AUX_CTL_A)) >>> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26) >>> #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) >>> #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) >>> #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) >>> #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) >>> >>> -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv- >>>> psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ >>> +#define _SRD_AUX_DATA_A 0x60814 >>> +#define _SRD_AUX_DATA_EDP 0x6f814 >>> +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, >>> _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ >>> >>> -#define EDP_PSR_STATUS _MMIO(dev_priv- >>>> psr_mmio_base + 0x40) >>> +#define _SRD_STATUS_A 0x60840 >>> +#define _SRD_STATUS_EDP 0x6f840 >>> +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ( >>> tran, _SRD_STATUS_A)) >>> #define EDP_PSR_STATUS_STATE_MASK (7 << 29) >>> #define EDP_PSR_STATUS_STATE_SHIFT 29 >>> #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) >>> @@ -4260,10 +4273,15 @@ enum { >>> #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) >>> #define EDP_PSR_STATUS_IDLE_MASK 0xf >>> >>> -#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + >>> 0x44) >>> +#define _SRD_PERF_CNT_A 0x60844 >>> +#define _SRD_PERF_CNT_EDP 0x6f844 >>> +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, >>> _SRD_PERF_CNT_A)) >>> #define EDP_PSR_PERF_CNT_MASK 0xffffff >>> >>> -#define EDP_PSR_DEBUG _MMIO(dev_priv- >>>> psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ >>> +/* PSR_MASK on SKL+ */ >>> +#define _SRD_DEBUG_A 0x60860 >>> +#define _SRD_DEBUG_EDP 0x6f860 >>> +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ( >>> tran, _SRD_DEBUG_A)) >>> #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) >>> #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) >>> #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) >>> @@ -4271,7 +4289,9 @@ enum { >>> #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* >>> Reserved in ICL+ */ >>> #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ >>> */ >>> >>> -#define EDP_PSR2_CTL _MMIO(0x6f900) >>> +#define _PSR2_CTL_A 0x60900 >>> +#define _PSR2_CTL_EDP 0x6f900 >>> +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, _PSR2_CTL_A) >>> #define EDP_PSR2_ENABLE (1 << 31) >>> #define EDP_SU_TRACK_ENABLE (1 << 30) >>> #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ >>> @@ -4293,8 +4313,8 @@ enum { >>> #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_TRANS_EDP 0x6f848 >>> +#define PSR_EVENT(tran) _MMIO_TRANS2(tr >>> an, _PSR_EVENT_TRANS_A) >>> #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) >>> @@ -4312,15 +4332,16 @@ enum { >>> #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) >>> #define PSR_EVENT_PSR_DISABLE (1 << 0) >>> >>> -#define EDP_PSR2_STATUS _MMIO(0x6f940) >>> +#define _PSR2_STATUS_A 0x60940 >>> +#define _PSR2_STATUS_EDP 0x6f940 >>> +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, >>> _PSR2_STATUS_A) >>> #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) >>> #define EDP_PSR2_STATUS_STATE_SHIFT 28 >>> >>> -#define _PSR2_SU_STATUS_0 0x6F914 >>> -#define _PSR2_SU_STATUS_1 0x6F918 >>> -#define _PSR2_SU_STATUS_2 0x6F91C >>> -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index >>> ), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) >>> -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame >>> ) / 3)) >>> +#define _PSR2_SU_STATUS_A 0x60914 >>> +#define _PSR2_SU_STATUS_EDP 0x6f914 >>> +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, >>> _PSR2_SU_STATUS_A) + (index) * 4) >>> +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, >>> (frame) / 3)) >>> #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) >>> #define PSR2_SU_STATUS_MASK(frame) (0x3ff << >>> PSR2_SU_STATUS_SHIFT(frame)) >>> #define PSR2_SU_STATUS_FRAMES 8 >>> -- >>> 2.22.1 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2019-08-22 at 21:37 +0530, Gupta, Anshuman wrote: > > On 8/22/2019 1:36 AM, Souza, Jose wrote: > > On Wed, 2019-08-21 at 10:06 +0530, Anshuman Gupta wrote: > > > On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: > > > > PSR registers are a mess, some have the full address while > > > > others > > > > just > > > > have the additional offset from psr_mmio_base. > > > > > > > > For BDW+ psr_mmio_base is nothing more than > > > > TRANSCODER_EDP_OFFSET + > > > > 0x800 and using it makes more difficult for people with an PSR > > > > register address or PSR register name from from BSpec as i915 > > > > also > > > > don't match the BSpec names. > > > > For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR > > > > registers > > > > are > > > > only available in DDIA. > > > > > > > > Other reason to make relative to transcoder is that since BDW > > > > every > > > > transcoder have PSR registers, so in theory it should be > > > > possible > > > > to > > > > have PSR enabled in a non-eDP transcoder. > > > > > > > > So for BDW+ we can use _TRANS2() to get the register offset of > > > > any > > > > PSR register in any transcoder while for HSW we have > > > > _HSW_PSR_ADJ > > > > that will calculate the register offset for the single PSR > > > > instance, > > > > noting that we are already guarded about trying to enable PSR > > > > in > > > > other > > > > port than DDIA on HSW by the 'if (dig_port->base.port != > > > > PORT_A)' > > > > in > > > > intel_psr_compute_config(), this check should only be valid for > > > > HSW > > > > and will be changed in future. > > > > PSR2 registers and PSR_EVENT was added after Haswell so that is > > > > why > > > > _PSR_ADJ() is not used in some macros. > > > > > > > > The only registers that can not be relative to transcoder are > > > > PSR_IMR and PSR_IIR that are not relative to anything, so > > > > keeping > > > > it > > > > hardcoded. That changed for TGL but it will be handled in > > > > another > > > > patch. > > > > > > > > Also removing BDW_EDP_PSR_BASE from GVT because it is not used > > > > as > > > > it > > > > is the only PSR register that GVT have. > > > > > > > > v5: > > > > - Macros changed to be more explicit about HSW (Dhinakaran) > > > > - Squashed with the patch that added the tran parameter to the > > > > macros (Dhinakaran) > > > > > > > > v6: > > > > - Checking for interruption errors after module reload in the > > > > transcoder that will be used (Dhinakaran) > > > > - Using lowercase to the registers offsets > > > > > > > > v7: > > > > - Removing IS_HASWELL() from registers macros(Jani) > > > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Zhi Wang <zhi.a.wang@intel.com> > > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_psr.c | 104 +++++++++++++- > > > > --- > > > > ------ > > > > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > > > > drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- > > > > drivers/gpu/drm/i915/i915_drv.h | 5 +- > > > > drivers/gpu/drm/i915/i915_reg.h | 57 +++++++++---- > > > > 5 files changed, 113 insertions(+), 73 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > index 3bfb720560c2..77232f6bca17 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct > > > > intel_dp > > > > *intel_dp) > > > > > > > > BUILD_BUG_ON(sizeof(aux_msg) > 20); > > > > for (i = 0; i < sizeof(aux_msg); i += 4) > > > > - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), > > > > + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv- > > > > >psr.transcoder, i > > > > > > 2), > > > > intel_dp_pack_aux(&aux_msg[i], > > > > sizeof(aux_msg) - i)); > > > > > > > > aux_clock_divider = intel_dp- > > > > >get_aux_clock_divider(intel_dp, > > > > 0); > > > > @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct > > > > intel_dp > > > > *intel_dp) > > > > > > > > /* Select only valid bits for SRD_AUX_CTL */ > > > > aux_ctl &= psr_aux_mask; > > > > - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > > > > + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), > > > > aux_ctl); > > > > } > > > > > > > > static void intel_psr_enable_sink(struct intel_dp *intel_dp) > > > > @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct > > > > intel_dp > > > > *intel_dp) > > > > if (INTEL_GEN(dev_priv) >= 8) > > > > val |= EDP_PSR_CRC_ENABLE; > > > > > > > > - val |= I915_READ(EDP_PSR_CTL) & > > > > EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; > > > > - I915_WRITE(EDP_PSR_CTL, val); > > > > + val |= (I915_READ(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder)) & > > > > + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > > > > } > > > > > > > > static void hsw_activate_psr2(struct intel_dp *intel_dp) > > > > @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct > > > > intel_dp > > > > *intel_dp) > > > > * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and > > > > BSpec > > > > is > > > > * recommending keep this bit unset while PSR2 is > > > > enabled. > > > > */ > > > > - I915_WRITE(EDP_PSR_CTL, 0); > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); > > > > > > > > - I915_WRITE(EDP_PSR2_CTL, val); > > > > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), > > > > val); > > > > } > > > > > > > > static bool intel_psr2_config_valid(struct intel_dp > > > > *intel_dp, > > > > @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct > > > > intel_dp > > > > *intel_dp, > > > > > > > > /* > > > > * HSW spec explicitly says PSR is tied to port A. > > > > - * BDW+ platforms with DDI implementation of PSR have > > > > different > > > > - * PSR registers per transcoder and we only implement > > > > transcoder EDP > > > > - * ones. Since by Display design transcoder EDP is tied > > > > to port > > > > A > > > > - * we can safely escape based on the port A. > > > > + * BDW+ platforms have a instance of PSR registers per > > > > transcoder but > > > > + * for now it only supports one instance of PSR, so > > > > lets keep > > > > it > > > > + * hardcoded to PORT_A > > > > */ > > > > if (dig_port->base.port != PORT_A) { > > > > DRM_DEBUG_KMS("PSR condition failed: Port not > > > > supported\n"); > > > > @@ -649,8 +649,8 @@ static void intel_psr_activate(struct > > > > intel_dp > > > > *intel_dp) > > > > struct drm_i915_private *dev_priv = > > > > dp_to_i915(intel_dp); > > > > > > > > if (INTEL_GEN(dev_priv) >= 9) > > > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > > > EDP_PSR2_ENABLE); > > > > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > psr.transcoder)) & EDP_PSR2_ENABLE); > > > > + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder)) & > > > > EDP_PSR_ENABLE); > > > > WARN_ON(dev_priv->psr.active); > > > > lockdep_assert_held(&dev_priv->psr.lock); > > > > > > > > @@ -720,19 +720,37 @@ static void > > > > intel_psr_enable_source(struct > > > > intel_dp *intel_dp, > > > > if (INTEL_GEN(dev_priv) < 11) > > > > mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; > > > > > > > > - I915_WRITE(EDP_PSR_DEBUG, mask); > > > > + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), > > > > mask); > > > > } > > > > > > > > static void intel_psr_enable_locked(struct drm_i915_private > > > > *dev_priv, > > > > const struct > > > > intel_crtc_state > > > > *crtc_state) > > > > { > > > > struct intel_dp *intel_dp = dev_priv->psr.dp; > > > > + u32 val; > > > > > > > > WARN_ON(dev_priv->psr.enabled); > > > > > > > > dev_priv->psr.psr2_enabled = > > > > intel_psr2_enabled(dev_priv, > > > > crtc_state); > > > > dev_priv->psr.busy_frontbuffer_bits = 0; > > > > dev_priv->psr.pipe = to_intel_crtc(crtc_state- > > > > >base.crtc)- > > > > > pipe; > > > > + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > > > > + > > > > + /* > > > > + * If a PSR error happened and the driver is reloaded, > > > > the > > > > EDP_PSR_IIR > > > > + * will still keep the error set even after the reset > > > > done in > > > > the > > > > + * irq_preinstall and irq_uninstall hooks. > > > > + * And enabling in this situation cause the screen to > > > > freeze in > > > > the > > > > + * first time that PSR HW tries to activate so lets > > > > keep PSR > > > > disabled > > > > + * to avoid any rendering problems. > > > > + */ > > > > + val = I915_READ(EDP_PSR_IIR); > > > > + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv- > > > > >psr.transcoder)); > > > > + if (val) { > > > > + dev_priv->psr.sink_not_reliable = true; > > > > + DRM_DEBUG_KMS("PSR interruption error set, not > > > > enabling > > > > PSR\n"); > > > > + return; > > > > + } > > > > > > > > DRM_DEBUG_KMS("Enabling PSR%s\n", > > > > dev_priv->psr.psr2_enabled ? "2" : "1"); > > > > @@ -782,20 +800,27 @@ static void intel_psr_exit(struct > > > > drm_i915_private *dev_priv) > > > > u32 val; > > > > > > > > if (!dev_priv->psr.active) { > > > > - if (INTEL_GEN(dev_priv) >= 9) > > > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > > > EDP_PSR2_ENABLE); > > > > - WARN_ON(I915_READ(EDP_PSR_CTL) & > > > > EDP_PSR_ENABLE); > > > > + if (INTEL_GEN(dev_priv) >= 9) { > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > psr.transcoder)); > > > > + WARN_ON(val & EDP_PSR2_ENABLE); > > > > + } > > > > + > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder)); > > > > + WARN_ON(val & EDP_PSR_ENABLE); > > > > + > > > > return; > > > > } > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > - val = I915_READ(EDP_PSR2_CTL); > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > psr.transcoder)); > > > > WARN_ON(!(val & EDP_PSR2_ENABLE)); > > > > - I915_WRITE(EDP_PSR2_CTL, val & > > > > ~EDP_PSR2_ENABLE); > > > > + val &= ~EDP_PSR2_ENABLE; > > > > + I915_WRITE(EDP_PSR2_CTL(dev_priv- > > > > >psr.transcoder), > > > > val); > > > > } else { > > > > - val = I915_READ(EDP_PSR_CTL); > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder)); > > > > WARN_ON(!(val & EDP_PSR_ENABLE)); > > > > - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); > > > > + val &= ~EDP_PSR_ENABLE; > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder), val); > > > > } > > > > dev_priv->psr.active = false; > > > > } > > > > @@ -817,10 +842,10 @@ static void > > > > intel_psr_disable_locked(struct > > > > intel_dp *intel_dp) > > > > intel_psr_exit(dev_priv); > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > - psr_status = EDP_PSR2_STATUS; > > > > + psr_status = EDP_PSR2_STATUS(dev_priv- > > > > >psr.transcoder); > > > > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > > > > } else { > > > > - psr_status = EDP_PSR_STATUS; > > > > + psr_status = EDP_PSR_STATUS(dev_priv- > > > > >psr.transcoder); > > > > psr_status_mask = EDP_PSR_STATUS_STATE_MASK; > > > > } > > > > > > > > @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct > > > > intel_crtc_state *new_crtc_state, > > > > * defensive enough to cover everything. > > > > */ > > > > > > > > - return __intel_wait_for_register(&dev_priv->uncore, > > > > EDP_PSR_STATUS, > > > > + return __intel_wait_for_register(&dev_priv->uncore, > > > > + EDP_PSR_STATUS(dev_pri > > > > v- > > > > > psr.transcoder), > > > > EDP_PSR_STATUS_STATE_M > > > > ASK, > > > > EDP_PSR_STATUS_STATE_I > > > > DLE, 2, > > > > 50, > > > > out_value); > > > > @@ -979,10 +1005,10 @@ static bool > > > > __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) > > > > return false; > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > - reg = EDP_PSR2_STATUS; > > > > + reg = EDP_PSR2_STATUS(dev_priv- > > > > >psr.transcoder); > > > > mask = EDP_PSR2_STATUS_STATE_MASK; > > > > } else { > > > > - reg = EDP_PSR_STATUS; > > > > + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); > > > > mask = EDP_PSR_STATUS_STATE_MASK; > > > > } > > > > > > > > @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct > > > > drm_i915_private *dev_priv, > > > > */ > > > > void intel_psr_init(struct drm_i915_private *dev_priv) > > > > { > > > > - u32 val; > > > > - > > > > if (!HAS_PSR(dev_priv)) > > > > return; > > > > > > > > - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? > > > > - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; > > > > - > > > > if (!dev_priv->psr.sink_support) > > > > return; > > > > > > > > + if (IS_HASWELL(dev_priv)) > > > > + /* > > > > + * HSW don't have PSR registers on the same > > > > space as > > > > transcoder > > > > + * so set this to a value that when subtract to > > > > the > > > > register > > > > + * in transcoder space results in the right > > > > offset for > > > > HSW > > > > + */ > > > > + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - > > > > _HSW_EDP_PSR_BASE; > > > > + > > > > if (i915_modparams.enable_psr == -1) > > > > if (INTEL_GEN(dev_priv) < 9 || !dev_priv- > > > > > vbt.psr.enable) > > > > i915_modparams.enable_psr = 0; > > > > > > > > - /* > > > > - * If a PSR error happened and the driver is reloaded, > > > > the > > > > EDP_PSR_IIR > > > > - * will still keep the error set even after the reset > > > > done in > > > > the > > > > - * irq_preinstall and irq_uninstall hooks. > > > > - * And enabling in this situation cause the screen to > > > > freeze in > > > > the > > > > - * first time that PSR HW tries to activate so lets > > > > keep PSR > > > > disabled > > > > - * to avoid any rendering problems. > > > > - */ > > > > - val = I915_READ(EDP_PSR_IIR); > > > > - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); > > > > - if (val) { > > > > - DRM_DEBUG_KMS("PSR interruption error set\n"); > > > > - dev_priv->psr.sink_not_reliable = true; > > > > - } > > > > - > > > Earlier EDP_PSR_IIR was being checked only in driver init path, > > > now it has been checked for every modeset/fastset path in > > > intel_psr_enable_locked(). Is it ok ? > > > If it is justified why are we not checking it in > > > intel_psr_flush() > > > there also it enables psr. > > > > I moved it primarily because on intel_psr_enable_locked() we have > > the > > transcoder that will be used, doing on init we would need to check > > for > > all transcoders and in case of a error set in one transcoder we > > would > > need to fail initialization as PSR could be enabled on that > > transcoder. > That is correct, but with this EDP_PSR_IIR error is getting checked > in > every modeset/fastset which is redundant, as it is already being > check > at interrupt handler. > IMO it should be only checked somehow at initel_psr_init(), even > comment > also reflect same thing ("If a PSR error happened and the driver is > reloaded..."). > If there no other way to handle this, comment should be change > accordingly. Only on full modesets, on real world it will happen only once at every boot also read a register is not a expensive operation. I will merge this as it is, if someone else also thinks that the comment is necessary I will add on top. > > No need to do that on intel_psr_flush() because PSR interruptions > > can > > be triggered even with PSR inactive. > > > > > > > > /* Set link_standby x link_off defaults */ > > > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > > > /* HSW and BDW require workarounds that we > > > > don't > > > > implement. */ > > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > > > > b/drivers/gpu/drm/i915/gvt/handlers.c > > > > index 25f78196b964..45a9124e53b6 100644 > > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c > > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > > > > @@ -2796,7 +2796,7 @@ static int > > > > init_broadwell_mmio_info(struct > > > > intel_gvt *gvt) > > > > MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); > > > > > > > > MMIO_D(WM_MISC, D_BDW); > > > > - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); > > > > + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); > > > > > > > > MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); > > > > MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > index b39226d7f8d2..6e4824daafae 100644 > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private > > > > *dev_priv, struct seq_file *m) > > > > "BUF_ON", > > > > "TG_ON" > > > > }; > > > > - val = I915_READ(EDP_PSR2_STATUS); > > > > + val = I915_READ(EDP_PSR2_STATUS(dev_priv- > > > > > psr.transcoder)); > > > > status_val = (val & EDP_PSR2_STATUS_STATE_MASK) > > > > >> > > > > EDP_PSR2_STATUS_STATE_SHIFT; > > > > if (status_val < ARRAY_SIZE(live_status)) > > > > @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private > > > > *dev_priv, struct seq_file *m) > > > > "SRDOFFACK", > > > > "SRDENT_ON", > > > > }; > > > > - val = I915_READ(EDP_PSR_STATUS); > > > > + val = I915_READ(EDP_PSR_STATUS(dev_priv- > > > > > psr.transcoder)); > > > > status_val = (val & EDP_PSR_STATUS_STATE_MASK) > > > > >> > > > > EDP_PSR_STATUS_STATE_SHIFT; > > > > if (status_val < ARRAY_SIZE(live_status)) > > > > @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct > > > > seq_file *m, void *data) > > > > goto unlock; > > > > > > > > if (psr->psr2_enabled) { > > > > - val = I915_READ(EDP_PSR2_CTL); > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > psr.transcoder)); > > > > enabled = val & EDP_PSR2_ENABLE; > > > > } else { > > > > - val = I915_READ(EDP_PSR_CTL); > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > >psr.transcoder)); > > > > enabled = val & EDP_PSR_ENABLE; > > > > } > > > > seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", > > > > @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct > > > > seq_file *m, void *data) > > > > * SKL+ Perf counter is reset to 0 everytime DC state > > > > is > > > > entered > > > > */ > > > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > > > - val = I915_READ(EDP_PSR_PERF_CNT) & > > > > EDP_PSR_PERF_CNT_MASK; > > > > + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv- > > > > > psr.transcoder)); > > > > + val &= EDP_PSR_PERF_CNT_MASK; > > > > seq_printf(m, "Performance counter: %u\n", > > > > val); > > > > } > > > > > > > > @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct > > > > seq_file *m, void *data) > > > > * Reading all 3 registers before hand to > > > > minimize > > > > crossing a > > > > * frame boundary between register reads > > > > */ > > > > - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; > > > > frame += > > > > 3) > > > > - su_frames_val[frame / 3] = > > > > I915_READ(PSR2_SU_STATUS(frame)); > > > > + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; > > > > frame += > > > > 3) { > > > > + val = > > > > I915_READ(PSR2_SU_STATUS(dev_priv- > > > > > psr.transcoder, > > > > + frame)); > > > > + su_frames_val[frame / 3] = val; > > > > + } > > > > > > > > seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > b/drivers/gpu/drm/i915/i915_drv.h > > > > index eb31c1656cea..be999791abca 100644 > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > @@ -479,6 +479,7 @@ struct i915_psr { > > > > bool enabled; > > > > struct intel_dp *dp; > > > > enum pipe pipe; > > > > + enum transcoder transcoder; > > > > bool active; > > > > struct work_struct work; > > > > unsigned busy_frontbuffer_bits; > > > > @@ -1330,11 +1331,11 @@ struct drm_i915_private { > > > > */ > > > > u32 gpio_mmio_base; > > > > > > > > + u32 hsw_psr_mmio_adjust; > > > > + > > > > /* MMIO base address for MIPI regs */ > > > > u32 mipi_mmio_base; > > > > > > > > - u32 psr_mmio_base; > > > > - > > > > u32 pps_mmio_base; > > > > > > > > wait_queue_head_t gmbus_wait_queue; > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > index 2abd199093c5..a092b34c269d 100644 > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > @@ -4186,10 +4186,17 @@ enum { > > > > #define PIPESRC(trans) _MMIO_TRANS2(trans, > > > > _PIPEASRC) > > > > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, > > > > _PIPE_MULT_A) > > > > > > > > -/* HSW+ eDP PSR registers */ > > > > -#define HSW_EDP_PSR_BASE 0x64800 > > > > -#define BDW_EDP_PSR_BASE 0x6f800 > > > > -#define EDP_PSR_CTL _MMIO(dev_priv- > > > > > psr_mmio_base + 0) > > > > +/* > > > > + * HSW+ eDP PSR registers > > > > + * > > > > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + > > > > 0x800) > > > > with just one > > > > + * instance of it > > > > + */ > > > > +#define _HSW_EDP_PSR_BASE 0x64800 > > > > +#define _SRD_CTL_A 0x60800 > > > > +#define _SRD_CTL_EDP 0x6f800 > > > > +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, > > > > reg) - dev_priv->hsw_psr_mmio_adjust) > > > > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ( > > > > tran, > > > > _SRD_CTL_A)) > > > > #define EDP_PSR_ENABLE (1 << 31) > > > > #define BDW_PSR_SINGLE_FRAME (1 << > > > > 30) > > > > #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW > > > > can't modify */ > > > > @@ -4226,16 +4233,22 @@ enum { > > > > #define EDP_PSR_TRANSCODER_A_SHIFT 8 > > > > #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 > > > > > > > > -#define EDP_PSR_AUX_CTL _MMIO(d > > > > ev_priv- > > > > > psr_mmio_base + 0x10) > > > > +#define _SRD_AUX_CTL_A 0x60810 > > > > +#define _SRD_AUX_CTL_EDP 0x6f810 > > > > +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ( > > > > tran, _SRD_AUX_CTL_A)) > > > > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << > > > > 26) > > > > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) > > > > #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) > > > > #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) > > > > #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) > > > > > > > > -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv- > > > > > psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ > > > > +#define _SRD_AUX_DATA_A 0x60814 > > > > +#define _SRD_AUX_DATA_EDP 0x6f814 > > > > +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ( > > > > tran, > > > > _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ > > > > > > > > -#define EDP_PSR_STATUS _MMIO(dev_priv- > > > > > psr_mmio_base + 0x40) > > > > +#define _SRD_STATUS_A 0x60840 > > > > +#define _SRD_STATUS_EDP 0x6f840 > > > > +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ( > > > > tran, _SRD_STATUS_A)) > > > > #define EDP_PSR_STATUS_STATE_MASK (7 << 29) > > > > #define EDP_PSR_STATUS_STATE_SHIFT 29 > > > > #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) > > > > @@ -4260,10 +4273,15 @@ enum { > > > > #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) > > > > #define EDP_PSR_STATUS_IDLE_MASK 0xf > > > > > > > > -#define EDP_PSR_PERF_CNT _MMIO(dev_priv- > > > > >psr_mmio_base + > > > > 0x44) > > > > +#define _SRD_PERF_CNT_A 0x60844 > > > > +#define _SRD_PERF_CNT_EDP 0x6f844 > > > > +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, > > > > _SRD_PERF_CNT_A)) > > > > #define EDP_PSR_PERF_CNT_MASK 0xffffff > > > > > > > > -#define EDP_PSR_DEBUG _MMIO(dev_priv- > > > > > psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ > > > > +/* PSR_MASK on SKL+ */ > > > > +#define _SRD_DEBUG_A 0x60860 > > > > +#define _SRD_DEBUG_EDP 0x6f860 > > > > +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ( > > > > tran, _SRD_DEBUG_A)) > > > > #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) > > > > #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) > > > > #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) > > > > @@ -4271,7 +4289,9 @@ enum { > > > > #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* > > > > Reserved in ICL+ */ > > > > #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* > > > > SKL+ > > > > */ > > > > > > > > -#define EDP_PSR2_CTL _MMIO(0x6f900) > > > > +#define _PSR2_CTL_A 0x60900 > > > > +#define _PSR2_CTL_EDP 0x6f900 > > > > +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, > > > > _PSR2_CTL_A) > > > > #define EDP_PSR2_ENABLE (1 << 31) > > > > #define EDP_SU_TRACK_ENABLE (1 << 30) > > > > #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and > > > > CNL+ */ > > > > @@ -4293,8 +4313,8 @@ enum { > > > > #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(tr > > > > ans, > > > > _PSR_EVENT_TRANS_A) > > > > +#define _PSR_EVENT_TRANS_EDP 0x6f848 > > > > +#define PSR_EVENT(tran) _MMIO_T > > > > RANS2(tr > > > > an, _PSR_EVENT_TRANS_A) > > > > #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) > > > > @@ -4312,15 +4332,16 @@ enum { > > > > #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) > > > > #define PSR_EVENT_PSR_DISABLE (1 << > > > > 0) > > > > > > > > -#define EDP_PSR2_STATUS _MMIO(0x6f940) > > > > +#define _PSR2_STATUS_A 0x60940 > > > > +#define _PSR2_STATUS_EDP 0x6f940 > > > > +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, > > > > _PSR2_STATUS_A) > > > > #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) > > > > #define EDP_PSR2_STATUS_STATE_SHIFT 28 > > > > > > > > -#define _PSR2_SU_STATUS_0 0x6F914 > > > > -#define _PSR2_SU_STATUS_1 0x6F918 > > > > -#define _PSR2_SU_STATUS_2 0x6F91C > > > > -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index > > > > ), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) > > > > -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame > > > > ) / 3)) > > > > +#define _PSR2_SU_STATUS_A 0x60914 > > > > +#define _PSR2_SU_STATUS_EDP 0x6f914 > > > > +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, > > > > _PSR2_SU_STATUS_A) + (index) * 4) > > > > +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, > > > > (frame) / 3)) > > > > #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) > > > > #define PSR2_SU_STATUS_MASK(frame) (0x3ff << > > > > PSR2_SU_STATUS_SHIFT(frame)) > > > > #define PSR2_SU_STATUS_FRAMES 8 > > > > -- > > > > 2.22.1 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 8/22/2019 10:19 PM, Souza, Jose wrote: > On Thu, 2019-08-22 at 21:37 +0530, Gupta, Anshuman wrote: >> >> On 8/22/2019 1:36 AM, Souza, Jose wrote: >>> On Wed, 2019-08-21 at 10:06 +0530, Anshuman Gupta wrote: >>>> On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: >>>>> PSR registers are a mess, some have the full address while >>>>> others >>>>> just >>>>> have the additional offset from psr_mmio_base. >>>>> >>>>> For BDW+ psr_mmio_base is nothing more than >>>>> TRANSCODER_EDP_OFFSET + >>>>> 0x800 and using it makes more difficult for people with an PSR >>>>> register address or PSR register name from from BSpec as i915 >>>>> also >>>>> don't match the BSpec names. >>>>> For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR >>>>> registers >>>>> are >>>>> only available in DDIA. >>>>> >>>>> Other reason to make relative to transcoder is that since BDW >>>>> every >>>>> transcoder have PSR registers, so in theory it should be >>>>> possible >>>>> to >>>>> have PSR enabled in a non-eDP transcoder. >>>>> >>>>> So for BDW+ we can use _TRANS2() to get the register offset of >>>>> any >>>>> PSR register in any transcoder while for HSW we have >>>>> _HSW_PSR_ADJ >>>>> that will calculate the register offset for the single PSR >>>>> instance, >>>>> noting that we are already guarded about trying to enable PSR >>>>> in >>>>> other >>>>> port than DDIA on HSW by the 'if (dig_port->base.port != >>>>> PORT_A)' >>>>> in >>>>> intel_psr_compute_config(), this check should only be valid for >>>>> HSW >>>>> and will be changed in future. >>>>> PSR2 registers and PSR_EVENT was added after Haswell so that is >>>>> why >>>>> _PSR_ADJ() is not used in some macros. >>>>> >>>>> The only registers that can not be relative to transcoder are >>>>> PSR_IMR and PSR_IIR that are not relative to anything, so >>>>> keeping >>>>> it >>>>> hardcoded. That changed for TGL but it will be handled in >>>>> another >>>>> patch. >>>>> >>>>> Also removing BDW_EDP_PSR_BASE from GVT because it is not used >>>>> as >>>>> it >>>>> is the only PSR register that GVT have. >>>>> >>>>> v5: >>>>> - Macros changed to be more explicit about HSW (Dhinakaran) >>>>> - Squashed with the patch that added the tran parameter to the >>>>> macros (Dhinakaran) >>>>> >>>>> v6: >>>>> - Checking for interruption errors after module reload in the >>>>> transcoder that will be used (Dhinakaran) >>>>> - Using lowercase to the registers offsets >>>>> >>>>> v7: >>>>> - Removing IS_HASWELL() from registers macros(Jani) >>>>> >>>>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>>>> Cc: Jani Nikula <jani.nikula@linux.intel.com> >>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >>>>> Cc: Zhi Wang <zhi.a.wang@intel.com> >>>>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> >>>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >>>>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/display/intel_psr.c | 104 +++++++++++++- >>>>> --- >>>>> ------ >>>>> drivers/gpu/drm/i915/gvt/handlers.c | 2 +- >>>>> drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- >>>>> drivers/gpu/drm/i915/i915_drv.h | 5 +- >>>>> drivers/gpu/drm/i915/i915_reg.h | 57 +++++++++---- >>>>> 5 files changed, 113 insertions(+), 73 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c >>>>> b/drivers/gpu/drm/i915/display/intel_psr.c >>>>> index 3bfb720560c2..77232f6bca17 100644 >>>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c >>>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c >>>>> @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct >>>>> intel_dp >>>>> *intel_dp) >>>>> >>>>> BUILD_BUG_ON(sizeof(aux_msg) > 20); >>>>> for (i = 0; i < sizeof(aux_msg); i += 4) >>>>> - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), >>>>> + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv- >>>>>> psr.transcoder, i >>>>>>> 2), >>>>> intel_dp_pack_aux(&aux_msg[i], >>>>> sizeof(aux_msg) - i)); >>>>> >>>>> aux_clock_divider = intel_dp- >>>>>> get_aux_clock_divider(intel_dp, >>>>> 0); >>>>> @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct >>>>> intel_dp >>>>> *intel_dp) >>>>> >>>>> /* Select only valid bits for SRD_AUX_CTL */ >>>>> aux_ctl &= psr_aux_mask; >>>>> - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); >>>>> + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), >>>>> aux_ctl); >>>>> } >>>>> >>>>> static void intel_psr_enable_sink(struct intel_dp *intel_dp) >>>>> @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct >>>>> intel_dp >>>>> *intel_dp) >>>>> if (INTEL_GEN(dev_priv) >= 8) >>>>> val |= EDP_PSR_CRC_ENABLE; >>>>> >>>>> - val |= I915_READ(EDP_PSR_CTL) & >>>>> EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; >>>>> - I915_WRITE(EDP_PSR_CTL, val); >>>>> + val |= (I915_READ(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder)) & >>>>> + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); >>>>> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); >>>>> } >>>>> >>>>> static void hsw_activate_psr2(struct intel_dp *intel_dp) >>>>> @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct >>>>> intel_dp >>>>> *intel_dp) >>>>> * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and >>>>> BSpec >>>>> is >>>>> * recommending keep this bit unset while PSR2 is >>>>> enabled. >>>>> */ >>>>> - I915_WRITE(EDP_PSR_CTL, 0); >>>>> + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); >>>>> >>>>> - I915_WRITE(EDP_PSR2_CTL, val); >>>>> + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), >>>>> val); >>>>> } >>>>> >>>>> static bool intel_psr2_config_valid(struct intel_dp >>>>> *intel_dp, >>>>> @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct >>>>> intel_dp >>>>> *intel_dp, >>>>> >>>>> /* >>>>> * HSW spec explicitly says PSR is tied to port A. >>>>> - * BDW+ platforms with DDI implementation of PSR have >>>>> different >>>>> - * PSR registers per transcoder and we only implement >>>>> transcoder EDP >>>>> - * ones. Since by Display design transcoder EDP is tied >>>>> to port >>>>> A >>>>> - * we can safely escape based on the port A. >>>>> + * BDW+ platforms have a instance of PSR registers per >>>>> transcoder but >>>>> + * for now it only supports one instance of PSR, so >>>>> lets keep >>>>> it >>>>> + * hardcoded to PORT_A >>>>> */ >>>>> if (dig_port->base.port != PORT_A) { >>>>> DRM_DEBUG_KMS("PSR condition failed: Port not >>>>> supported\n"); >>>>> @@ -649,8 +649,8 @@ static void intel_psr_activate(struct >>>>> intel_dp >>>>> *intel_dp) >>>>> struct drm_i915_private *dev_priv = >>>>> dp_to_i915(intel_dp); >>>>> >>>>> if (INTEL_GEN(dev_priv) >= 9) >>>>> - WARN_ON(I915_READ(EDP_PSR2_CTL) & >>>>> EDP_PSR2_ENABLE); >>>>> - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); >>>>> + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv- >>>>>> psr.transcoder)) & EDP_PSR2_ENABLE); >>>>> + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder)) & >>>>> EDP_PSR_ENABLE); >>>>> WARN_ON(dev_priv->psr.active); >>>>> lockdep_assert_held(&dev_priv->psr.lock); >>>>> >>>>> @@ -720,19 +720,37 @@ static void >>>>> intel_psr_enable_source(struct >>>>> intel_dp *intel_dp, >>>>> if (INTEL_GEN(dev_priv) < 11) >>>>> mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; >>>>> >>>>> - I915_WRITE(EDP_PSR_DEBUG, mask); >>>>> + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), >>>>> mask); >>>>> } >>>>> >>>>> static void intel_psr_enable_locked(struct drm_i915_private >>>>> *dev_priv, >>>>> const struct >>>>> intel_crtc_state >>>>> *crtc_state) >>>>> { >>>>> struct intel_dp *intel_dp = dev_priv->psr.dp; >>>>> + u32 val; >>>>> >>>>> WARN_ON(dev_priv->psr.enabled); >>>>> >>>>> dev_priv->psr.psr2_enabled = >>>>> intel_psr2_enabled(dev_priv, >>>>> crtc_state); >>>>> dev_priv->psr.busy_frontbuffer_bits = 0; >>>>> dev_priv->psr.pipe = to_intel_crtc(crtc_state- >>>>>> base.crtc)- >>>>>> pipe; >>>>> + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; >>>>> + >>>>> + /* >>>>> + * If a PSR error happened and the driver is reloaded, >>>>> the >>>>> EDP_PSR_IIR >>>>> + * will still keep the error set even after the reset >>>>> done in >>>>> the >>>>> + * irq_preinstall and irq_uninstall hooks. >>>>> + * And enabling in this situation cause the screen to >>>>> freeze in >>>>> the >>>>> + * first time that PSR HW tries to activate so lets >>>>> keep PSR >>>>> disabled >>>>> + * to avoid any rendering problems. >>>>> + */ >>>>> + val = I915_READ(EDP_PSR_IIR); >>>>> + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv- >>>>>> psr.transcoder)); >>>>> + if (val) { >>>>> + dev_priv->psr.sink_not_reliable = true; >>>>> + DRM_DEBUG_KMS("PSR interruption error set, not >>>>> enabling >>>>> PSR\n"); >>>>> + return; >>>>> + } >>>>> >>>>> DRM_DEBUG_KMS("Enabling PSR%s\n", >>>>> dev_priv->psr.psr2_enabled ? "2" : "1"); >>>>> @@ -782,20 +800,27 @@ static void intel_psr_exit(struct >>>>> drm_i915_private *dev_priv) >>>>> u32 val; >>>>> >>>>> if (!dev_priv->psr.active) { >>>>> - if (INTEL_GEN(dev_priv) >= 9) >>>>> - WARN_ON(I915_READ(EDP_PSR2_CTL) & >>>>> EDP_PSR2_ENABLE); >>>>> - WARN_ON(I915_READ(EDP_PSR_CTL) & >>>>> EDP_PSR_ENABLE); >>>>> + if (INTEL_GEN(dev_priv) >= 9) { >>>>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> + WARN_ON(val & EDP_PSR2_ENABLE); >>>>> + } >>>>> + >>>>> + val = I915_READ(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> + WARN_ON(val & EDP_PSR_ENABLE); >>>>> + >>>>> return; >>>>> } >>>>> >>>>> if (dev_priv->psr.psr2_enabled) { >>>>> - val = I915_READ(EDP_PSR2_CTL); >>>>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> WARN_ON(!(val & EDP_PSR2_ENABLE)); >>>>> - I915_WRITE(EDP_PSR2_CTL, val & >>>>> ~EDP_PSR2_ENABLE); >>>>> + val &= ~EDP_PSR2_ENABLE; >>>>> + I915_WRITE(EDP_PSR2_CTL(dev_priv- >>>>>> psr.transcoder), >>>>> val); >>>>> } else { >>>>> - val = I915_READ(EDP_PSR_CTL); >>>>> + val = I915_READ(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> WARN_ON(!(val & EDP_PSR_ENABLE)); >>>>> - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); >>>>> + val &= ~EDP_PSR_ENABLE; >>>>> + I915_WRITE(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder), val); >>>>> } >>>>> dev_priv->psr.active = false; >>>>> } >>>>> @@ -817,10 +842,10 @@ static void >>>>> intel_psr_disable_locked(struct >>>>> intel_dp *intel_dp) >>>>> intel_psr_exit(dev_priv); >>>>> >>>>> if (dev_priv->psr.psr2_enabled) { >>>>> - psr_status = EDP_PSR2_STATUS; >>>>> + psr_status = EDP_PSR2_STATUS(dev_priv- >>>>>> psr.transcoder); >>>>> psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; >>>>> } else { >>>>> - psr_status = EDP_PSR_STATUS; >>>>> + psr_status = EDP_PSR_STATUS(dev_priv- >>>>>> psr.transcoder); >>>>> psr_status_mask = EDP_PSR_STATUS_STATE_MASK; >>>>> } >>>>> >>>>> @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct >>>>> intel_crtc_state *new_crtc_state, >>>>> * defensive enough to cover everything. >>>>> */ >>>>> >>>>> - return __intel_wait_for_register(&dev_priv->uncore, >>>>> EDP_PSR_STATUS, >>>>> + return __intel_wait_for_register(&dev_priv->uncore, >>>>> + EDP_PSR_STATUS(dev_pri >>>>> v- >>>>>> psr.transcoder), >>>>> EDP_PSR_STATUS_STATE_M >>>>> ASK, >>>>> EDP_PSR_STATUS_STATE_I >>>>> DLE, 2, >>>>> 50, >>>>> out_value); >>>>> @@ -979,10 +1005,10 @@ static bool >>>>> __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) >>>>> return false; >>>>> >>>>> if (dev_priv->psr.psr2_enabled) { >>>>> - reg = EDP_PSR2_STATUS; >>>>> + reg = EDP_PSR2_STATUS(dev_priv- >>>>>> psr.transcoder); >>>>> mask = EDP_PSR2_STATUS_STATE_MASK; >>>>> } else { >>>>> - reg = EDP_PSR_STATUS; >>>>> + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); >>>>> mask = EDP_PSR_STATUS_STATE_MASK; >>>>> } >>>>> >>>>> @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct >>>>> drm_i915_private *dev_priv, >>>>> */ >>>>> void intel_psr_init(struct drm_i915_private *dev_priv) >>>>> { >>>>> - u32 val; >>>>> - >>>>> if (!HAS_PSR(dev_priv)) >>>>> return; >>>>> >>>>> - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? >>>>> - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; >>>>> - >>>>> if (!dev_priv->psr.sink_support) >>>>> return; >>>>> >>>>> + if (IS_HASWELL(dev_priv)) >>>>> + /* >>>>> + * HSW don't have PSR registers on the same >>>>> space as >>>>> transcoder >>>>> + * so set this to a value that when subtract to >>>>> the >>>>> register >>>>> + * in transcoder space results in the right >>>>> offset for >>>>> HSW >>>>> + */ >>>>> + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - >>>>> _HSW_EDP_PSR_BASE; >>>>> + >>>>> if (i915_modparams.enable_psr == -1) >>>>> if (INTEL_GEN(dev_priv) < 9 || !dev_priv- >>>>>> vbt.psr.enable) >>>>> i915_modparams.enable_psr = 0; >>>>> >>>>> - /* >>>>> - * If a PSR error happened and the driver is reloaded, >>>>> the >>>>> EDP_PSR_IIR >>>>> - * will still keep the error set even after the reset >>>>> done in >>>>> the >>>>> - * irq_preinstall and irq_uninstall hooks. >>>>> - * And enabling in this situation cause the screen to >>>>> freeze in >>>>> the >>>>> - * first time that PSR HW tries to activate so lets >>>>> keep PSR >>>>> disabled >>>>> - * to avoid any rendering problems. >>>>> - */ >>>>> - val = I915_READ(EDP_PSR_IIR); >>>>> - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); >>>>> - if (val) { >>>>> - DRM_DEBUG_KMS("PSR interruption error set\n"); >>>>> - dev_priv->psr.sink_not_reliable = true; >>>>> - } >>>>> - >>>> Earlier EDP_PSR_IIR was being checked only in driver init path, >>>> now it has been checked for every modeset/fastset path in >>>> intel_psr_enable_locked(). Is it ok ? >>>> If it is justified why are we not checking it in >>>> intel_psr_flush() >>>> there also it enables psr. >>> >>> I moved it primarily because on intel_psr_enable_locked() we have >>> the >>> transcoder that will be used, doing on init we would need to check >>> for >>> all transcoders and in case of a error set in one transcoder we >>> would >>> need to fail initialization as PSR could be enabled on that >>> transcoder. >> That is correct, but with this EDP_PSR_IIR error is getting checked >> in >> every modeset/fastset which is redundant, as it is already being >> check >> at interrupt handler. >> IMO it should be only checked somehow at initel_psr_init(), even >> comment >> also reflect same thing ("If a PSR error happened and the driver is >> reloaded..."). >> If there no other way to handle this, comment should be change >> accordingly. > > Only on full modesets, on real world it will happen only once at every AFAIU intel_psr_update() may also call intel_psr_enable_locked(), which means even on fastset this this will get checked ? > boot also read a register is not a expensive operation. > > I will merge this as it is, if someone else also thinks that the > comment is necessary I will add on top. > > >>> No need to do that on intel_psr_flush() because PSR interruptions >>> can >>> be triggered even with PSR inactive. >>> >>> >>>>> /* Set link_standby x link_off defaults */ >>>>> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) >>>>> /* HSW and BDW require workarounds that we >>>>> don't >>>>> implement. */ >>>>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c >>>>> b/drivers/gpu/drm/i915/gvt/handlers.c >>>>> index 25f78196b964..45a9124e53b6 100644 >>>>> --- a/drivers/gpu/drm/i915/gvt/handlers.c >>>>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c >>>>> @@ -2796,7 +2796,7 @@ static int >>>>> init_broadwell_mmio_info(struct >>>>> intel_gvt *gvt) >>>>> MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); >>>>> >>>>> MMIO_D(WM_MISC, D_BDW); >>>>> - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); >>>>> + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); >>>>> >>>>> MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); >>>>> MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); >>>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c >>>>> b/drivers/gpu/drm/i915/i915_debugfs.c >>>>> index b39226d7f8d2..6e4824daafae 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c >>>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >>>>> @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private >>>>> *dev_priv, struct seq_file *m) >>>>> "BUF_ON", >>>>> "TG_ON" >>>>> }; >>>>> - val = I915_READ(EDP_PSR2_STATUS); >>>>> + val = I915_READ(EDP_PSR2_STATUS(dev_priv- >>>>>> psr.transcoder)); >>>>> status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>>>>>> >>>>> EDP_PSR2_STATUS_STATE_SHIFT; >>>>> if (status_val < ARRAY_SIZE(live_status)) >>>>> @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private >>>>> *dev_priv, struct seq_file *m) >>>>> "SRDOFFACK", >>>>> "SRDENT_ON", >>>>> }; >>>>> - val = I915_READ(EDP_PSR_STATUS); >>>>> + val = I915_READ(EDP_PSR_STATUS(dev_priv- >>>>>> psr.transcoder)); >>>>> status_val = (val & EDP_PSR_STATUS_STATE_MASK) >>>>>>> >>>>> EDP_PSR_STATUS_STATE_SHIFT; >>>>> if (status_val < ARRAY_SIZE(live_status)) >>>>> @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct >>>>> seq_file *m, void *data) >>>>> goto unlock; >>>>> >>>>> if (psr->psr2_enabled) { >>>>> - val = I915_READ(EDP_PSR2_CTL); >>>>> + val = I915_READ(EDP_PSR2_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> enabled = val & EDP_PSR2_ENABLE; >>>>> } else { >>>>> - val = I915_READ(EDP_PSR_CTL); >>>>> + val = I915_READ(EDP_PSR_CTL(dev_priv- >>>>>> psr.transcoder)); >>>>> enabled = val & EDP_PSR_ENABLE; >>>>> } >>>>> seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", >>>>> @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct >>>>> seq_file *m, void *data) >>>>> * SKL+ Perf counter is reset to 0 everytime DC state >>>>> is >>>>> entered >>>>> */ >>>>> if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { >>>>> - val = I915_READ(EDP_PSR_PERF_CNT) & >>>>> EDP_PSR_PERF_CNT_MASK; >>>>> + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv- >>>>>> psr.transcoder)); >>>>> + val &= EDP_PSR_PERF_CNT_MASK; >>>>> seq_printf(m, "Performance counter: %u\n", >>>>> val); >>>>> } >>>>> >>>>> @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct >>>>> seq_file *m, void *data) >>>>> * Reading all 3 registers before hand to >>>>> minimize >>>>> crossing a >>>>> * frame boundary between register reads >>>>> */ >>>>> - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; >>>>> frame += >>>>> 3) >>>>> - su_frames_val[frame / 3] = >>>>> I915_READ(PSR2_SU_STATUS(frame)); >>>>> + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; >>>>> frame += >>>>> 3) { >>>>> + val = >>>>> I915_READ(PSR2_SU_STATUS(dev_priv- >>>>>> psr.transcoder, >>>>> + frame)); >>>>> + su_frames_val[frame / 3] = val; >>>>> + } >>>>> >>>>> seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h >>>>> b/drivers/gpu/drm/i915/i915_drv.h >>>>> index eb31c1656cea..be999791abca 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>> @@ -479,6 +479,7 @@ struct i915_psr { >>>>> bool enabled; >>>>> struct intel_dp *dp; >>>>> enum pipe pipe; >>>>> + enum transcoder transcoder; >>>>> bool active; >>>>> struct work_struct work; >>>>> unsigned busy_frontbuffer_bits; >>>>> @@ -1330,11 +1331,11 @@ struct drm_i915_private { >>>>> */ >>>>> u32 gpio_mmio_base; >>>>> >>>>> + u32 hsw_psr_mmio_adjust; >>>>> + >>>>> /* MMIO base address for MIPI regs */ >>>>> u32 mipi_mmio_base; >>>>> >>>>> - u32 psr_mmio_base; >>>>> - >>>>> u32 pps_mmio_base; >>>>> >>>>> wait_queue_head_t gmbus_wait_queue; >>>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h >>>>> b/drivers/gpu/drm/i915/i915_reg.h >>>>> index 2abd199093c5..a092b34c269d 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_reg.h >>>>> +++ b/drivers/gpu/drm/i915/i915_reg.h >>>>> @@ -4186,10 +4186,17 @@ enum { >>>>> #define PIPESRC(trans) _MMIO_TRANS2(trans, >>>>> _PIPEASRC) >>>>> #define PIPE_MULT(trans) _MMIO_TRANS2(trans, >>>>> _PIPE_MULT_A) >>>>> >>>>> -/* HSW+ eDP PSR registers */ >>>>> -#define HSW_EDP_PSR_BASE 0x64800 >>>>> -#define BDW_EDP_PSR_BASE 0x6f800 >>>>> -#define EDP_PSR_CTL _MMIO(dev_priv- >>>>>> psr_mmio_base + 0) >>>>> +/* >>>>> + * HSW+ eDP PSR registers >>>>> + * >>>>> + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + >>>>> 0x800) >>>>> with just one >>>>> + * instance of it >>>>> + */ >>>>> +#define _HSW_EDP_PSR_BASE 0x64800 >>>>> +#define _SRD_CTL_A 0x60800 >>>>> +#define _SRD_CTL_EDP 0x6f800 >>>>> +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, >>>>> reg) - dev_priv->hsw_psr_mmio_adjust) >>>>> +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ( >>>>> tran, >>>>> _SRD_CTL_A)) >>>>> #define EDP_PSR_ENABLE (1 << 31) >>>>> #define BDW_PSR_SINGLE_FRAME (1 << >>>>> 30) >>>>> #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW >>>>> can't modify */ >>>>> @@ -4226,16 +4233,22 @@ enum { >>>>> #define EDP_PSR_TRANSCODER_A_SHIFT 8 >>>>> #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 >>>>> >>>>> -#define EDP_PSR_AUX_CTL _MMIO(d >>>>> ev_priv- >>>>>> psr_mmio_base + 0x10) >>>>> +#define _SRD_AUX_CTL_A 0x60810 >>>>> +#define _SRD_AUX_CTL_EDP 0x6f810 >>>>> +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ( >>>>> tran, _SRD_AUX_CTL_A)) >>>>> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << >>>>> 26) >>>>> #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) >>>>> #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) >>>>> #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) >>>>> #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) >>>>> >>>>> -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv- >>>>>> psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ >>>>> +#define _SRD_AUX_DATA_A 0x60814 >>>>> +#define _SRD_AUX_DATA_EDP 0x6f814 >>>>> +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ( >>>>> tran, >>>>> _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ >>>>> >>>>> -#define EDP_PSR_STATUS _MMIO(dev_priv- >>>>>> psr_mmio_base + 0x40) >>>>> +#define _SRD_STATUS_A 0x60840 >>>>> +#define _SRD_STATUS_EDP 0x6f840 >>>>> +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ( >>>>> tran, _SRD_STATUS_A)) >>>>> #define EDP_PSR_STATUS_STATE_MASK (7 << 29) >>>>> #define EDP_PSR_STATUS_STATE_SHIFT 29 >>>>> #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) >>>>> @@ -4260,10 +4273,15 @@ enum { >>>>> #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) >>>>> #define EDP_PSR_STATUS_IDLE_MASK 0xf >>>>> >>>>> -#define EDP_PSR_PERF_CNT _MMIO(dev_priv- >>>>>> psr_mmio_base + >>>>> 0x44) >>>>> +#define _SRD_PERF_CNT_A 0x60844 >>>>> +#define _SRD_PERF_CNT_EDP 0x6f844 >>>>> +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, >>>>> _SRD_PERF_CNT_A)) >>>>> #define EDP_PSR_PERF_CNT_MASK 0xffffff >>>>> >>>>> -#define EDP_PSR_DEBUG _MMIO(dev_priv- >>>>>> psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ >>>>> +/* PSR_MASK on SKL+ */ >>>>> +#define _SRD_DEBUG_A 0x60860 >>>>> +#define _SRD_DEBUG_EDP 0x6f860 >>>>> +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ( >>>>> tran, _SRD_DEBUG_A)) >>>>> #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) >>>>> #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) >>>>> #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) >>>>> @@ -4271,7 +4289,9 @@ enum { >>>>> #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* >>>>> Reserved in ICL+ */ >>>>> #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* >>>>> SKL+ >>>>> */ >>>>> >>>>> -#define EDP_PSR2_CTL _MMIO(0x6f900) >>>>> +#define _PSR2_CTL_A 0x60900 >>>>> +#define _PSR2_CTL_EDP 0x6f900 >>>>> +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, >>>>> _PSR2_CTL_A) >>>>> #define EDP_PSR2_ENABLE (1 << 31) >>>>> #define EDP_SU_TRACK_ENABLE (1 << 30) >>>>> #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and >>>>> CNL+ */ >>>>> @@ -4293,8 +4313,8 @@ enum { >>>>> #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(tr >>>>> ans, >>>>> _PSR_EVENT_TRANS_A) >>>>> +#define _PSR_EVENT_TRANS_EDP 0x6f848 >>>>> +#define PSR_EVENT(tran) _MMIO_T >>>>> RANS2(tr >>>>> an, _PSR_EVENT_TRANS_A) >>>>> #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) >>>>> @@ -4312,15 +4332,16 @@ enum { >>>>> #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) >>>>> #define PSR_EVENT_PSR_DISABLE (1 << >>>>> 0) >>>>> >>>>> -#define EDP_PSR2_STATUS _MMIO(0x6f940) >>>>> +#define _PSR2_STATUS_A 0x60940 >>>>> +#define _PSR2_STATUS_EDP 0x6f940 >>>>> +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, >>>>> _PSR2_STATUS_A) >>>>> #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) >>>>> #define EDP_PSR2_STATUS_STATE_SHIFT 28 >>>>> >>>>> -#define _PSR2_SU_STATUS_0 0x6F914 >>>>> -#define _PSR2_SU_STATUS_1 0x6F918 >>>>> -#define _PSR2_SU_STATUS_2 0x6F91C >>>>> -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index >>>>> ), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) >>>>> -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame >>>>> ) / 3)) >>>>> +#define _PSR2_SU_STATUS_A 0x60914 >>>>> +#define _PSR2_SU_STATUS_EDP 0x6f914 >>>>> +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, >>>>> _PSR2_SU_STATUS_A) + (index) * 4) >>>>> +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, >>>>> (frame) / 3)) >>>>> #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) >>>>> #define PSR2_SU_STATUS_MASK(frame) (0x3ff << >>>>> PSR2_SU_STATUS_SHIFT(frame)) >>>>> #define PSR2_SU_STATUS_FRAMES 8 >>>>> -- >>>>> 2.22.1 >>>>> >>>>> _______________________________________________ >>>>> Intel-gfx mailing list >>>>> Intel-gfx@lists.freedesktop.org >>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2019-08-22 at 22:39 +0530, Gupta, Anshuman wrote: > > On 8/22/2019 10:19 PM, Souza, Jose wrote: > > On Thu, 2019-08-22 at 21:37 +0530, Gupta, Anshuman wrote: > > > On 8/22/2019 1:36 AM, Souza, Jose wrote: > > > > On Wed, 2019-08-21 at 10:06 +0530, Anshuman Gupta wrote: > > > > > On 2019-08-20 at 15:33:23 -0700, José Roberto de Souza wrote: > > > > > > PSR registers are a mess, some have the full address while > > > > > > others > > > > > > just > > > > > > have the additional offset from psr_mmio_base. > > > > > > > > > > > > For BDW+ psr_mmio_base is nothing more than > > > > > > TRANSCODER_EDP_OFFSET + > > > > > > 0x800 and using it makes more difficult for people with an > > > > > > PSR > > > > > > register address or PSR register name from from BSpec as > > > > > > i915 > > > > > > also > > > > > > don't match the BSpec names. > > > > > > For HSW psr_mmio_base is _DDI_BUF_CTL_A + 0x800 and PSR > > > > > > registers > > > > > > are > > > > > > only available in DDIA. > > > > > > > > > > > > Other reason to make relative to transcoder is that since > > > > > > BDW > > > > > > every > > > > > > transcoder have PSR registers, so in theory it should be > > > > > > possible > > > > > > to > > > > > > have PSR enabled in a non-eDP transcoder. > > > > > > > > > > > > So for BDW+ we can use _TRANS2() to get the register offset > > > > > > of > > > > > > any > > > > > > PSR register in any transcoder while for HSW we have > > > > > > _HSW_PSR_ADJ > > > > > > that will calculate the register offset for the single PSR > > > > > > instance, > > > > > > noting that we are already guarded about trying to enable > > > > > > PSR > > > > > > in > > > > > > other > > > > > > port than DDIA on HSW by the 'if (dig_port->base.port != > > > > > > PORT_A)' > > > > > > in > > > > > > intel_psr_compute_config(), this check should only be valid > > > > > > for > > > > > > HSW > > > > > > and will be changed in future. > > > > > > PSR2 registers and PSR_EVENT was added after Haswell so > > > > > > that is > > > > > > why > > > > > > _PSR_ADJ() is not used in some macros. > > > > > > > > > > > > The only registers that can not be relative to transcoder > > > > > > are > > > > > > PSR_IMR and PSR_IIR that are not relative to anything, so > > > > > > keeping > > > > > > it > > > > > > hardcoded. That changed for TGL but it will be handled in > > > > > > another > > > > > > patch. > > > > > > > > > > > > Also removing BDW_EDP_PSR_BASE from GVT because it is not > > > > > > used > > > > > > as > > > > > > it > > > > > > is the only PSR register that GVT have. > > > > > > > > > > > > v5: > > > > > > - Macros changed to be more explicit about HSW (Dhinakaran) > > > > > > - Squashed with the patch that added the tran parameter to > > > > > > the > > > > > > macros (Dhinakaran) > > > > > > > > > > > > v6: > > > > > > - Checking for interruption errors after module reload in > > > > > > the > > > > > > transcoder that will be used (Dhinakaran) > > > > > > - Using lowercase to the registers offsets > > > > > > > > > > > > v7: > > > > > > - Removing IS_HASWELL() from registers macros(Jani) > > > > > > > > > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Cc: Zhi Wang <zhi.a.wang@intel.com> > > > > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 104 > > > > > > +++++++++++++- > > > > > > --- > > > > > > ------ > > > > > > drivers/gpu/drm/i915/gvt/handlers.c | 2 +- > > > > > > drivers/gpu/drm/i915/i915_debugfs.c | 18 ++-- > > > > > > drivers/gpu/drm/i915/i915_drv.h | 5 +- > > > > > > drivers/gpu/drm/i915/i915_reg.h | 57 > > > > > > +++++++++---- > > > > > > 5 files changed, 113 insertions(+), 73 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > index 3bfb720560c2..77232f6bca17 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct > > > > > > intel_dp > > > > > > *intel_dp) > > > > > > > > > > > > BUILD_BUG_ON(sizeof(aux_msg) > 20); > > > > > > for (i = 0; i < sizeof(aux_msg); i += 4) > > > > > > - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), > > > > > > + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv- > > > > > > > psr.transcoder, i > > > > > > > > 2), > > > > > > intel_dp_pack_aux(&aux_msg[i], > > > > > > sizeof(aux_msg) - i)); > > > > > > > > > > > > aux_clock_divider = intel_dp- > > > > > > > get_aux_clock_divider(intel_dp, > > > > > > 0); > > > > > > @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct > > > > > > intel_dp > > > > > > *intel_dp) > > > > > > > > > > > > /* Select only valid bits for SRD_AUX_CTL */ > > > > > > aux_ctl &= psr_aux_mask; > > > > > > - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); > > > > > > + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), > > > > > > aux_ctl); > > > > > > } > > > > > > > > > > > > static void intel_psr_enable_sink(struct intel_dp > > > > > > *intel_dp) > > > > > > @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct > > > > > > intel_dp > > > > > > *intel_dp) > > > > > > if (INTEL_GEN(dev_priv) >= 8) > > > > > > val |= EDP_PSR_CRC_ENABLE; > > > > > > > > > > > > - val |= I915_READ(EDP_PSR_CTL) & > > > > > > EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; > > > > > > - I915_WRITE(EDP_PSR_CTL, val); > > > > > > + val |= (I915_READ(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder)) & > > > > > > + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); > > > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); > > > > > > } > > > > > > > > > > > > static void hsw_activate_psr2(struct intel_dp *intel_dp) > > > > > > @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct > > > > > > intel_dp > > > > > > *intel_dp) > > > > > > * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and > > > > > > BSpec > > > > > > is > > > > > > * recommending keep this bit unset while PSR2 is > > > > > > enabled. > > > > > > */ > > > > > > - I915_WRITE(EDP_PSR_CTL, 0); > > > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); > > > > > > > > > > > > - I915_WRITE(EDP_PSR2_CTL, val); > > > > > > + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), > > > > > > val); > > > > > > } > > > > > > > > > > > > static bool intel_psr2_config_valid(struct intel_dp > > > > > > *intel_dp, > > > > > > @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct > > > > > > intel_dp > > > > > > *intel_dp, > > > > > > > > > > > > /* > > > > > > * HSW spec explicitly says PSR is tied to port A. > > > > > > - * BDW+ platforms with DDI implementation of PSR have > > > > > > different > > > > > > - * PSR registers per transcoder and we only implement > > > > > > transcoder EDP > > > > > > - * ones. Since by Display design transcoder EDP is tied > > > > > > to port > > > > > > A > > > > > > - * we can safely escape based on the port A. > > > > > > + * BDW+ platforms have a instance of PSR registers per > > > > > > transcoder but > > > > > > + * for now it only supports one instance of PSR, so > > > > > > lets keep > > > > > > it > > > > > > + * hardcoded to PORT_A > > > > > > */ > > > > > > if (dig_port->base.port != PORT_A) { > > > > > > DRM_DEBUG_KMS("PSR condition failed: Port not > > > > > > supported\n"); > > > > > > @@ -649,8 +649,8 @@ static void intel_psr_activate(struct > > > > > > intel_dp > > > > > > *intel_dp) > > > > > > struct drm_i915_private *dev_priv = > > > > > > dp_to_i915(intel_dp); > > > > > > > > > > > > if (INTEL_GEN(dev_priv) >= 9) > > > > > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > > > > > EDP_PSR2_ENABLE); > > > > > > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > > > > > + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > > > psr.transcoder)) & EDP_PSR2_ENABLE); > > > > > > + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder)) & > > > > > > EDP_PSR_ENABLE); > > > > > > WARN_ON(dev_priv->psr.active); > > > > > > lockdep_assert_held(&dev_priv->psr.lock); > > > > > > > > > > > > @@ -720,19 +720,37 @@ static void > > > > > > intel_psr_enable_source(struct > > > > > > intel_dp *intel_dp, > > > > > > if (INTEL_GEN(dev_priv) < 11) > > > > > > mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; > > > > > > > > > > > > - I915_WRITE(EDP_PSR_DEBUG, mask); > > > > > > + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), > > > > > > mask); > > > > > > } > > > > > > > > > > > > static void intel_psr_enable_locked(struct > > > > > > drm_i915_private > > > > > > *dev_priv, > > > > > > const struct > > > > > > intel_crtc_state > > > > > > *crtc_state) > > > > > > { > > > > > > struct intel_dp *intel_dp = dev_priv->psr.dp; > > > > > > + u32 val; > > > > > > > > > > > > WARN_ON(dev_priv->psr.enabled); > > > > > > > > > > > > dev_priv->psr.psr2_enabled = > > > > > > intel_psr2_enabled(dev_priv, > > > > > > crtc_state); > > > > > > dev_priv->psr.busy_frontbuffer_bits = 0; > > > > > > dev_priv->psr.pipe = to_intel_crtc(crtc_state- > > > > > > > base.crtc)- > > > > > > > pipe; > > > > > > + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; > > > > > > + > > > > > > + /* > > > > > > + * If a PSR error happened and the driver is reloaded, > > > > > > the > > > > > > EDP_PSR_IIR > > > > > > + * will still keep the error set even after the reset > > > > > > done in > > > > > > the > > > > > > + * irq_preinstall and irq_uninstall hooks. > > > > > > + * And enabling in this situation cause the screen to > > > > > > freeze in > > > > > > the > > > > > > + * first time that PSR HW tries to activate so lets > > > > > > keep PSR > > > > > > disabled > > > > > > + * to avoid any rendering problems. > > > > > > + */ > > > > > > + val = I915_READ(EDP_PSR_IIR); > > > > > > + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv- > > > > > > > psr.transcoder)); > > > > > > + if (val) { > > > > > > + dev_priv->psr.sink_not_reliable = true; > > > > > > + DRM_DEBUG_KMS("PSR interruption error set, not > > > > > > enabling > > > > > > PSR\n"); > > > > > > + return; > > > > > > + } > > > > > > > > > > > > DRM_DEBUG_KMS("Enabling PSR%s\n", > > > > > > dev_priv->psr.psr2_enabled ? "2" : "1"); > > > > > > @@ -782,20 +800,27 @@ static void intel_psr_exit(struct > > > > > > drm_i915_private *dev_priv) > > > > > > u32 val; > > > > > > > > > > > > if (!dev_priv->psr.active) { > > > > > > - if (INTEL_GEN(dev_priv) >= 9) > > > > > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > > > > > EDP_PSR2_ENABLE); > > > > > > - WARN_ON(I915_READ(EDP_PSR_CTL) & > > > > > > EDP_PSR_ENABLE); > > > > > > + if (INTEL_GEN(dev_priv) >= 9) { > > > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > + WARN_ON(val & EDP_PSR2_ENABLE); > > > > > > + } > > > > > > + > > > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > + WARN_ON(val & EDP_PSR_ENABLE); > > > > > > + > > > > > > return; > > > > > > } > > > > > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > > > - val = I915_READ(EDP_PSR2_CTL); > > > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > WARN_ON(!(val & EDP_PSR2_ENABLE)); > > > > > > - I915_WRITE(EDP_PSR2_CTL, val & > > > > > > ~EDP_PSR2_ENABLE); > > > > > > + val &= ~EDP_PSR2_ENABLE; > > > > > > + I915_WRITE(EDP_PSR2_CTL(dev_priv- > > > > > > > psr.transcoder), > > > > > > val); > > > > > > } else { > > > > > > - val = I915_READ(EDP_PSR_CTL); > > > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > WARN_ON(!(val & EDP_PSR_ENABLE)); > > > > > > - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); > > > > > > + val &= ~EDP_PSR_ENABLE; > > > > > > + I915_WRITE(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder), val); > > > > > > } > > > > > > dev_priv->psr.active = false; > > > > > > } > > > > > > @@ -817,10 +842,10 @@ static void > > > > > > intel_psr_disable_locked(struct > > > > > > intel_dp *intel_dp) > > > > > > intel_psr_exit(dev_priv); > > > > > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > > > - psr_status = EDP_PSR2_STATUS; > > > > > > + psr_status = EDP_PSR2_STATUS(dev_priv- > > > > > > > psr.transcoder); > > > > > > psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; > > > > > > } else { > > > > > > - psr_status = EDP_PSR_STATUS; > > > > > > + psr_status = EDP_PSR_STATUS(dev_priv- > > > > > > > psr.transcoder); > > > > > > psr_status_mask = EDP_PSR_STATUS_STATE_MASK; > > > > > > } > > > > > > > > > > > > @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const > > > > > > struct > > > > > > intel_crtc_state *new_crtc_state, > > > > > > * defensive enough to cover everything. > > > > > > */ > > > > > > > > > > > > - return __intel_wait_for_register(&dev_priv->uncore, > > > > > > EDP_PSR_STATUS, > > > > > > + return __intel_wait_for_register(&dev_priv->uncore, > > > > > > + EDP_PSR_STATUS(dev_pri > > > > > > v- > > > > > > > psr.transcoder), > > > > > > EDP_PSR_STATUS_STATE_M > > > > > > ASK, > > > > > > EDP_PSR_STATUS_STATE_I > > > > > > DLE, 2, > > > > > > 50, > > > > > > out_value); > > > > > > @@ -979,10 +1005,10 @@ static bool > > > > > > __psr_wait_for_idle_locked(struct drm_i915_private > > > > > > *dev_priv) > > > > > > return false; > > > > > > > > > > > > if (dev_priv->psr.psr2_enabled) { > > > > > > - reg = EDP_PSR2_STATUS; > > > > > > + reg = EDP_PSR2_STATUS(dev_priv- > > > > > > > psr.transcoder); > > > > > > mask = EDP_PSR2_STATUS_STATE_MASK; > > > > > > } else { > > > > > > - reg = EDP_PSR_STATUS; > > > > > > + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); > > > > > > mask = EDP_PSR_STATUS_STATE_MASK; > > > > > > } > > > > > > > > > > > > @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct > > > > > > drm_i915_private *dev_priv, > > > > > > */ > > > > > > void intel_psr_init(struct drm_i915_private *dev_priv) > > > > > > { > > > > > > - u32 val; > > > > > > - > > > > > > if (!HAS_PSR(dev_priv)) > > > > > > return; > > > > > > > > > > > > - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? > > > > > > - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; > > > > > > - > > > > > > if (!dev_priv->psr.sink_support) > > > > > > return; > > > > > > > > > > > > + if (IS_HASWELL(dev_priv)) > > > > > > + /* > > > > > > + * HSW don't have PSR registers on the same > > > > > > space as > > > > > > transcoder > > > > > > + * so set this to a value that when subtract to > > > > > > the > > > > > > register > > > > > > + * in transcoder space results in the right > > > > > > offset for > > > > > > HSW > > > > > > + */ > > > > > > + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - > > > > > > _HSW_EDP_PSR_BASE; > > > > > > + > > > > > > if (i915_modparams.enable_psr == -1) > > > > > > if (INTEL_GEN(dev_priv) < 9 || !dev_priv- > > > > > > > vbt.psr.enable) > > > > > > i915_modparams.enable_psr = 0; > > > > > > > > > > > > - /* > > > > > > - * If a PSR error happened and the driver is reloaded, > > > > > > the > > > > > > EDP_PSR_IIR > > > > > > - * will still keep the error set even after the reset > > > > > > done in > > > > > > the > > > > > > - * irq_preinstall and irq_uninstall hooks. > > > > > > - * And enabling in this situation cause the screen to > > > > > > freeze in > > > > > > the > > > > > > - * first time that PSR HW tries to activate so lets > > > > > > keep PSR > > > > > > disabled > > > > > > - * to avoid any rendering problems. > > > > > > - */ > > > > > > - val = I915_READ(EDP_PSR_IIR); > > > > > > - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); > > > > > > - if (val) { > > > > > > - DRM_DEBUG_KMS("PSR interruption error set\n"); > > > > > > - dev_priv->psr.sink_not_reliable = true; > > > > > > - } > > > > > > - > > > > > Earlier EDP_PSR_IIR was being checked only in driver init > > > > > path, > > > > > now it has been checked for every modeset/fastset path in > > > > > intel_psr_enable_locked(). Is it ok ? > > > > > If it is justified why are we not checking it in > > > > > intel_psr_flush() > > > > > there also it enables psr. > > > > > > > > I moved it primarily because on intel_psr_enable_locked() we > > > > have > > > > the > > > > transcoder that will be used, doing on init we would need to > > > > check > > > > for > > > > all transcoders and in case of a error set in one transcoder we > > > > would > > > > need to fail initialization as PSR could be enabled on that > > > > transcoder. > > > That is correct, but with this EDP_PSR_IIR error is getting > > > checked > > > in > > > every modeset/fastset which is redundant, as it is already being > > > check > > > at interrupt handler. > > > IMO it should be only checked somehow at initel_psr_init(), even > > > comment > > > also reflect same thing ("If a PSR error happened and the driver > > > is > > > reloaded..."). > > > If there no other way to handle this, comment should be change > > > accordingly. > > > > Only on full modesets, on real world it will happen only once at > > every > AFAIU intel_psr_update() may also call intel_psr_enable_locked(), > which > means even on fastset this this will get checked ? Yes but in 99% of the times it will fall into the case were PSR was enabled and the modeset still allows PSR, so it will do nothing. > > boot also read a register is not a expensive operation. > > > > I will merge this as it is, if someone else also thinks that the > > comment is necessary I will add on top. > > > > > > > > No need to do that on intel_psr_flush() because PSR > > > > interruptions > > > > can > > > > be triggered even with PSR inactive. > > > > > > > > > > > > > > /* Set link_standby x link_off defaults */ > > > > > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > > > > > > /* HSW and BDW require workarounds that we > > > > > > don't > > > > > > implement. */ > > > > > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > > > > > > b/drivers/gpu/drm/i915/gvt/handlers.c > > > > > > index 25f78196b964..45a9124e53b6 100644 > > > > > > --- a/drivers/gpu/drm/i915/gvt/handlers.c > > > > > > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > > > > > > @@ -2796,7 +2796,7 @@ static int > > > > > > init_broadwell_mmio_info(struct > > > > > > intel_gvt *gvt) > > > > > > MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); > > > > > > > > > > > > MMIO_D(WM_MISC, D_BDW); > > > > > > - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); > > > > > > + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); > > > > > > > > > > > > MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); > > > > > > MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); > > > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > index b39226d7f8d2..6e4824daafae 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > > > > > > @@ -2132,7 +2132,7 @@ psr_source_status(struct > > > > > > drm_i915_private > > > > > > *dev_priv, struct seq_file *m) > > > > > > "BUF_ON", > > > > > > "TG_ON" > > > > > > }; > > > > > > - val = I915_READ(EDP_PSR2_STATUS); > > > > > > + val = I915_READ(EDP_PSR2_STATUS(dev_priv- > > > > > > > psr.transcoder)); > > > > > > status_val = (val & EDP_PSR2_STATUS_STATE_MASK) > > > > > > EDP_PSR2_STATUS_STATE_SHIFT; > > > > > > if (status_val < ARRAY_SIZE(live_status)) > > > > > > @@ -2148,7 +2148,7 @@ psr_source_status(struct > > > > > > drm_i915_private > > > > > > *dev_priv, struct seq_file *m) > > > > > > "SRDOFFACK", > > > > > > "SRDENT_ON", > > > > > > }; > > > > > > - val = I915_READ(EDP_PSR_STATUS); > > > > > > + val = I915_READ(EDP_PSR_STATUS(dev_priv- > > > > > > > psr.transcoder)); > > > > > > status_val = (val & EDP_PSR_STATUS_STATE_MASK) > > > > > > EDP_PSR_STATUS_STATE_SHIFT; > > > > > > if (status_val < ARRAY_SIZE(live_status)) > > > > > > @@ -2191,10 +2191,10 @@ static int > > > > > > i915_edp_psr_status(struct > > > > > > seq_file *m, void *data) > > > > > > goto unlock; > > > > > > > > > > > > if (psr->psr2_enabled) { > > > > > > - val = I915_READ(EDP_PSR2_CTL); > > > > > > + val = I915_READ(EDP_PSR2_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > enabled = val & EDP_PSR2_ENABLE; > > > > > > } else { > > > > > > - val = I915_READ(EDP_PSR_CTL); > > > > > > + val = I915_READ(EDP_PSR_CTL(dev_priv- > > > > > > > psr.transcoder)); > > > > > > enabled = val & EDP_PSR_ENABLE; > > > > > > } > > > > > > seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", > > > > > > @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct > > > > > > seq_file *m, void *data) > > > > > > * SKL+ Perf counter is reset to 0 everytime DC state > > > > > > is > > > > > > entered > > > > > > */ > > > > > > if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { > > > > > > - val = I915_READ(EDP_PSR_PERF_CNT) & > > > > > > EDP_PSR_PERF_CNT_MASK; > > > > > > + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv- > > > > > > > psr.transcoder)); > > > > > > + val &= EDP_PSR_PERF_CNT_MASK; > > > > > > seq_printf(m, "Performance counter: %u\n", > > > > > > val); > > > > > > } > > > > > > > > > > > > @@ -2225,8 +2226,11 @@ static int > > > > > > i915_edp_psr_status(struct > > > > > > seq_file *m, void *data) > > > > > > * Reading all 3 registers before hand to > > > > > > minimize > > > > > > crossing a > > > > > > * frame boundary between register reads > > > > > > */ > > > > > > - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; > > > > > > frame += > > > > > > 3) > > > > > > - su_frames_val[frame / 3] = > > > > > > I915_READ(PSR2_SU_STATUS(frame)); > > > > > > + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; > > > > > > frame += > > > > > > 3) { > > > > > > + val = > > > > > > I915_READ(PSR2_SU_STATUS(dev_priv- > > > > > > > psr.transcoder, > > > > > > + frame)); > > > > > > + su_frames_val[frame / 3] = val; > > > > > > + } > > > > > > > > > > > > seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > > > b/drivers/gpu/drm/i915/i915_drv.h > > > > > > index eb31c1656cea..be999791abca 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > > > @@ -479,6 +479,7 @@ struct i915_psr { > > > > > > bool enabled; > > > > > > struct intel_dp *dp; > > > > > > enum pipe pipe; > > > > > > + enum transcoder transcoder; > > > > > > bool active; > > > > > > struct work_struct work; > > > > > > unsigned busy_frontbuffer_bits; > > > > > > @@ -1330,11 +1331,11 @@ struct drm_i915_private { > > > > > > */ > > > > > > u32 gpio_mmio_base; > > > > > > > > > > > > + u32 hsw_psr_mmio_adjust; > > > > > > + > > > > > > /* MMIO base address for MIPI regs */ > > > > > > u32 mipi_mmio_base; > > > > > > > > > > > > - u32 psr_mmio_base; > > > > > > - > > > > > > u32 pps_mmio_base; > > > > > > > > > > > > wait_queue_head_t gmbus_wait_queue; > > > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h > > > > > > b/drivers/gpu/drm/i915/i915_reg.h > > > > > > index 2abd199093c5..a092b34c269d 100644 > > > > > > --- a/drivers/gpu/drm/i915/i915_reg.h > > > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > > > > > @@ -4186,10 +4186,17 @@ enum { > > > > > > #define PIPESRC(trans) _MMIO_TRANS2(trans, > > > > > > _PIPEASRC) > > > > > > #define PIPE_MULT(trans) _MMIO_TRANS2(trans, > > > > > > _PIPE_MULT_A) > > > > > > > > > > > > -/* HSW+ eDP PSR registers */ > > > > > > -#define HSW_EDP_PSR_BASE 0x64800 > > > > > > -#define BDW_EDP_PSR_BASE 0x6f800 > > > > > > -#define EDP_PSR_CTL _MMIO(d > > > > > > ev_priv- > > > > > > > psr_mmio_base + 0) > > > > > > +/* > > > > > > + * HSW+ eDP PSR registers > > > > > > + * > > > > > > + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + > > > > > > 0x800) > > > > > > with just one > > > > > > + * instance of it > > > > > > + */ > > > > > > +#define _HSW_EDP_PSR_BASE 0x64800 > > > > > > +#define _SRD_CTL_A 0x60800 > > > > > > +#define _SRD_CTL_EDP 0x6f800 > > > > > > +#define _PSR_ADJ(tran, reg) (_TRANS > > > > > > 2(tran, > > > > > > reg) - dev_priv->hsw_psr_mmio_adjust) > > > > > > +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ( > > > > > > tran, > > > > > > _SRD_CTL_A)) > > > > > > #define EDP_PSR_ENABLE (1 << 31) > > > > > > #define BDW_PSR_SINGLE_FRAME (1 << > > > > > > 30) > > > > > > #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << > > > > > > 29) /* SW > > > > > > can't modify */ > > > > > > @@ -4226,16 +4233,22 @@ enum { > > > > > > #define EDP_PSR_TRANSCODER_A_SHIFT 8 > > > > > > #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 > > > > > > > > > > > > -#define EDP_PSR_AUX_CTL _MMIO(d > > > > > > ev_priv- > > > > > > > psr_mmio_base + 0x10) > > > > > > +#define _SRD_AUX_CTL_A 0x60810 > > > > > > +#define _SRD_AUX_CTL_EDP 0x6f810 > > > > > > +#define EDP_PSR_AUX_CTL(tran) _MMIO(_ > > > > > > PSR_ADJ( > > > > > > tran, _SRD_AUX_CTL_A)) > > > > > > #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << > > > > > > 26) > > > > > > #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f > > > > > > << 20) > > > > > > #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << > > > > > > 16) > > > > > > #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << > > > > > > 11) > > > > > > #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) > > > > > > > > > > > > -#define EDP_PSR_AUX_DATA(i) _MMIO(d > > > > > > ev_priv- > > > > > > > psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ > > > > > > +#define _SRD_AUX_DATA_A 0x60814 > > > > > > +#define _SRD_AUX_DATA_EDP 0x6f814 > > > > > > +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ( > > > > > > tran, > > > > > > _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ > > > > > > > > > > > > -#define EDP_PSR_STATUS _MMIO(d > > > > > > ev_priv- > > > > > > > psr_mmio_base + 0x40) > > > > > > +#define _SRD_STATUS_A 0x60840 > > > > > > +#define _SRD_STATUS_EDP 0x6f840 > > > > > > +#define EDP_PSR_STATUS(tran) _MMIO(_ > > > > > > PSR_ADJ( > > > > > > tran, _SRD_STATUS_A)) > > > > > > #define EDP_PSR_STATUS_STATE_MASK (7 << > > > > > > 29) > > > > > > #define EDP_PSR_STATUS_STATE_SHIFT 29 > > > > > > #define EDP_PSR_STATUS_STATE_IDLE (0 << > > > > > > 29) > > > > > > @@ -4260,10 +4273,15 @@ enum { > > > > > > #define EDP_PSR_STATUS_SENDING_TP1 (1 << > > > > > > 4) > > > > > > #define EDP_PSR_STATUS_IDLE_MASK 0xf > > > > > > > > > > > > -#define EDP_PSR_PERF_CNT _MMIO(dev_priv- > > > > > > > psr_mmio_base + > > > > > > 0x44) > > > > > > +#define _SRD_PERF_CNT_A 0x60844 > > > > > > +#define _SRD_PERF_CNT_EDP 0x6f844 > > > > > > +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ( > > > > > > tran, > > > > > > _SRD_PERF_CNT_A)) > > > > > > #define EDP_PSR_PERF_CNT_MASK 0xffffff > > > > > > > > > > > > -#define EDP_PSR_DEBUG _MMIO(d > > > > > > ev_priv- > > > > > > > psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ > > > > > > +/* PSR_MASK on SKL+ */ > > > > > > +#define _SRD_DEBUG_A 0x60860 > > > > > > +#define _SRD_DEBUG_EDP 0x6f860 > > > > > > +#define EDP_PSR_DEBUG(tran) _MMIO(_ > > > > > > PSR_ADJ( > > > > > > tran, _SRD_DEBUG_A)) > > > > > > #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) > > > > > > #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) > > > > > > #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) > > > > > > @@ -4271,7 +4289,9 @@ enum { > > > > > > #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) > > > > > > /* > > > > > > Reserved in ICL+ */ > > > > > > #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) > > > > > > /* > > > > > > SKL+ > > > > > > */ > > > > > > > > > > > > -#define EDP_PSR2_CTL _MMIO(0x6f900) > > > > > > +#define _PSR2_CTL_A 0x60900 > > > > > > +#define _PSR2_CTL_EDP 0x6f900 > > > > > > +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, > > > > > > _PSR2_CTL_A) > > > > > > #define EDP_PSR2_ENABLE (1 << 31) > > > > > > #define EDP_SU_TRACK_ENABLE (1 << 30) > > > > > > #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and > > > > > > CNL+ */ > > > > > > @@ -4293,8 +4313,8 @@ enum { > > > > > > #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(tr > > > > > > ans, > > > > > > _PSR_EVENT_TRANS_A) > > > > > > +#define _PSR_EVENT_TRANS_EDP 0x6f848 > > > > > > +#define PSR_EVENT(tran) _MMIO_T > > > > > > RANS2(tr > > > > > > an, _PSR_EVENT_TRANS_A) > > > > > > #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) > > > > > > @@ -4312,15 +4332,16 @@ enum { > > > > > > #define PSR_EVENT_LPSP_MODE_EXIT (1 << > > > > > > 1) > > > > > > #define PSR_EVENT_PSR_DISABLE (1 << > > > > > > 0) > > > > > > > > > > > > -#define EDP_PSR2_STATUS _MMIO(0x6f940) > > > > > > +#define _PSR2_STATUS_A 0x60940 > > > > > > +#define _PSR2_STATUS_EDP 0x6f940 > > > > > > +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tr > > > > > > an, > > > > > > _PSR2_STATUS_A) > > > > > > #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) > > > > > > #define EDP_PSR2_STATUS_STATE_SHIFT 28 > > > > > > > > > > > > -#define _PSR2_SU_STATUS_0 0x6F914 > > > > > > -#define _PSR2_SU_STATUS_1 0x6F918 > > > > > > -#define _PSR2_SU_STATUS_2 0x6F91C > > > > > > -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVE > > > > > > N((index > > > > > > ), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) > > > > > > -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATU > > > > > > S((frame > > > > > > ) / 3)) > > > > > > +#define _PSR2_SU_STATUS_A 0x60914 > > > > > > +#define _PSR2_SU_STATUS_EDP 0x6f914 > > > > > > +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(t > > > > > > ran, > > > > > > _PSR2_SU_STATUS_A) + (index) * 4) > > > > > > +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATU > > > > > > S(tran, > > > > > > (frame) / 3)) > > > > > > #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) > > > > > > * 10) > > > > > > #define PSR2_SU_STATUS_MASK(frame) (0x3ff << > > > > > > PSR2_SU_STATUS_SHIFT(frame)) > > > > > > #define PSR2_SU_STATUS_FRAMES 8 > > > > > > -- > > > > > > 2.22.1 > > > > > > > > > > > > _______________________________________________ > > > > > > Intel-gfx mailing list > > > > > > Intel-gfx@lists.freedesktop.org > > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2019-08-21 at 13:13 +0000, Patchwork wrote: > == Series Details == > > Series: series starting with [v8,1/3] drm/i915/psr: Make PSR > registers relative to transcoders > URL : https://patchwork.freedesktop.org/series/65507/ > State : success > > == Summary == > > CI Bug Log - changes from CI_DRM_6750_full -> Patchwork_14112_full > ==================================================== > > Summary > ------- > > **SUCCESS** > > No regressions found. > Pushed to dinq, thanks for the reviews Lucas and Anshuman > > > Known issues > ------------ > > Here are the changes found in Patchwork_14112_full that come from > known issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@gem_ctx_isolation@rcs0-s3: > - shard-apl: [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) > +5 similar issues > [1]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html > [2]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html > > * igt@gem_ctx_shared@exec-single-timeline-bsd: > - shard-iclb: [PASS][3] -> [SKIP][4] ([fdo#110841]) > [3]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html > [4]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html > > * igt@gem_exec_schedule@preempt-other-chain-bsd: > - shard-iclb: [PASS][5] -> [SKIP][6] ([fdo#111325]) +5 > similar issues > [5]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb5/igt@gem_exec_schedule@preempt-other-chain-bsd.html > [6]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb2/igt@gem_exec_schedule@preempt-other-chain-bsd.html > > * igt@i915_pm_rpm@basic-pci-d3-state: > - shard-apl: [PASS][7] -> [INCOMPLETE][8] ([fdo#103927]) > +2 similar issues > [7]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl1/igt@i915_pm_rpm@basic-pci-d3-state.html > [8]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-apl2/igt@i915_pm_rpm@basic-pci-d3-state.html > > * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic: > - shard-hsw: [PASS][9] -> [FAIL][10] ([fdo#105767]) > [9]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html > [10]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html > > * igt@kms_flip@flip-vs-expired-vblank: > - shard-skl: [PASS][11] -> [FAIL][12] ([fdo#105363]) +1 > similar issue > [11]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@kms_flip@flip-vs-expired-vblank.html > [12]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl8/igt@kms_flip@flip-vs-expired-vblank.html > > * igt@kms > _frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite: > - shard-iclb: [PASS][13] -> [INCOMPLETE][14] > ([fdo#107713]) > [13]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html > [14]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-pwrite.html > > * igt@kms_frontbuffer_tracking@fbc-1p-rte: > - shard-iclb: [PASS][15] -> [FAIL][16] ([fdo#103167] / > [fdo#110378]) > [15]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-rte.html > [16]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-rte.html > > * igt@kms_frontbuffer_tracking@fbc-suspend: > - shard-skl: [PASS][17] -> [INCOMPLETE][18] > ([fdo#104108]) +1 similar issue > [17]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@kms_frontbuffer_tracking@fbc-suspend.html > [18]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl8/igt@kms_frontbuffer_tracking@fbc-suspend.html > > * igt@kms > _frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render: > - shard-iclb: [PASS][19] -> [FAIL][20] ([fdo#103167]) +2 > similar issues > [19]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render.html > [20]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render.html > > * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc: > - shard-skl: [PASS][21] -> [FAIL][22] ([fdo#108145] / > [fdo#110403]) > [21]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html > [22]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html > > * igt@kms_psr2_su@frontbuffer: > - shard-iclb: [PASS][23] -> [SKIP][24] ([fdo#109642] / > [fdo#111068]) > [23]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr2_su@frontbuffer.html > [24]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb1/igt@kms_psr2_su@frontbuffer.html > > * igt@kms_psr@psr2_cursor_mmap_cpu: > - shard-iclb: [PASS][25] -> [SKIP][26] ([fdo#109441]) +2 > similar issues > [25]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html > [26]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb4/igt@kms_psr@psr2_cursor_mmap_cpu.html > > * igt@perf@polling: > - shard-skl: [PASS][27] -> [FAIL][28] ([fdo#110728]) > [27]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl3/igt@perf@polling.html > [28]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl4/igt@perf@polling.html > > * igt@prime_busy@hang-bsd2: > - shard-iclb: [PASS][29] -> [SKIP][30] ([fdo#109276]) +10 > similar issues > [29]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@prime_busy@hang-bsd2.html > [30]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb8/igt@prime_busy@hang-bsd2.html > > > #### Possible fixes #### > > * igt@gem_eio@reset-stress: > - shard-skl: [FAIL][31] ([fdo#109661]) -> [PASS][32] > [31]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl5/igt@gem_eio@reset-stress.html > [32]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl5/igt@gem_eio@reset-stress.html > > * igt@gem_exec_balancer@smoke: > - shard-iclb: [SKIP][33] ([fdo#110854]) -> [PASS][34] > [33]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_exec_balancer@smoke.html > [34]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb1/igt@gem_exec_balancer@smoke.html > > * igt@gem_exec_schedule@promotion-bsd1: > - shard-iclb: [SKIP][35] ([fdo#109276]) -> [PASS][36] +15 > similar issues > [35]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb8/igt@gem_exec_schedule@promotion-bsd1.html > [36]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb4/igt@gem_exec_schedule@promotion-bsd1.html > > * igt@gem_exec_schedule@reorder-wide-bsd: > - shard-iclb: [SKIP][37] ([fdo#111325]) -> [PASS][38] +2 > similar issues > [37]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@gem_exec_schedule@reorder-wide-bsd.html > [38]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb5/igt@gem_exec_schedule@reorder-wide-bsd.html > > * igt@i915_pm_rps@min-max-config-loaded: > - shard-iclb: [FAIL][39] ([fdo#111409]) -> [PASS][40] > [39]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@i915_pm_rps@min-max-config-loaded.html > [40]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb3/igt@i915_pm_rps@min-max-config-loaded.html > > * igt@i915_suspend@sysfs-reader: > - shard-apl: [DMESG-WARN][41] ([fdo#108566]) -> > [PASS][42] +3 similar issues > [41]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl7/igt@i915_suspend@sysfs-reader.html > [42]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-apl5/igt@i915_suspend@sysfs-reader.html > > * igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding: > - shard-iclb: [INCOMPLETE][43] ([fdo#107713]) -> > [PASS][44] > [43]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb7/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html > [44]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb5/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html > > * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy: > - shard-hsw: [FAIL][45] ([fdo#105767]) -> [PASS][46] > [45]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html > [46]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-hsw1/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html > > * igt@kms_fbcon_fbt@psr-suspend: > - shard-skl: [INCOMPLETE][47] ([fdo#104108]) -> > [PASS][48] > [47]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html > [48]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html > > * igt@kms_flip@blocking-wf_vblank: > - shard-apl: [INCOMPLETE][49] ([fdo#103927]) -> > [PASS][50] +1 similar issue > [49]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-apl4/igt@kms_flip@blocking-wf_vblank.html > [50]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-apl1/igt@kms_flip@blocking-wf_vblank.html > > * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt: > - shard-skl: [FAIL][51] ([fdo#108040]) -> [PASS][52] > [51]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html > [52]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl10/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html > > * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt: > - shard-iclb: [FAIL][53] ([fdo#103167]) -> [PASS][54] +2 > similar issues > [53]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html > [54]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html > > * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt: > - shard-skl: [FAIL][55] ([fdo#103167]) -> [PASS][56] > [55]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt.html > [56]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-blt.html > > * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min: > - shard-skl: [FAIL][57] ([fdo#108145]) -> [PASS][58] +1 > similar issue > [57]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html > [58]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html > > * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc: > - shard-skl: [FAIL][59] ([fdo#108145] / [fdo#110403]) -> > [PASS][60] > [59]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-skl6/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html > [60]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html > > * igt@kms_psr@psr2_primary_render: > - shard-iclb: [SKIP][61] ([fdo#109441]) -> [PASS][62] > [61]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@kms_psr@psr2_primary_render.html > [62]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb2/igt@kms_psr@psr2_primary_render.html > > * igt@kms_setmode@basic: > - shard-kbl: [FAIL][63] ([fdo#99912]) -> [PASS][64] > [63]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-kbl6/igt@kms_setmode@basic.html > [64]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-kbl2/igt@kms_setmode@basic.html > > > #### Warnings #### > > * igt@gem_ctx_isolation@vcs1-nonpriv: > - shard-iclb: [FAIL][65] ([fdo#111329]) -> [SKIP][66] > ([fdo#109276]) > [65]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb1/igt@gem_ctx_isolation@vcs1-nonpriv.html > [66]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb5/igt@gem_ctx_isolation@vcs1-nonpriv.html > > * igt@gem_mocs_settings@mocs-rc6-bsd2: > - shard-iclb: [SKIP][67] ([fdo#109276]) -> [FAIL][68] > ([fdo#111330]) > [67]: > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6750/shard-iclb3/igt@gem_mocs_settings@mocs-rc6-bsd2.html > [68]: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/shard-iclb4/igt@gem_mocs_settings@mocs-rc6-bsd2.html > > > [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167 > [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927 > [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108 > [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363 > [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767 > [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713 > [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040 > [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145 > [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566 > [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276 > [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441 > [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642 > [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661 > [fdo#110378]: https://bugs.freedesktop.org/show_bug.cgi?id=110378 > [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403 > [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728 > [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841 > [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854 > [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068 > [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325 > [fdo#111329]: https://bugs.freedesktop.org/show_bug.cgi?id=111329 > [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330 > [fdo#111409]: https://bugs.freedesktop.org/show_bug.cgi?id=111409 > [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912 > > > Participating hosts (10 -> 10) > ------------------------------ > > No changes in participating hosts > > > Build changes > ------------- > > * CI: CI-20190529 -> None > * Linux: CI_DRM_6750 -> Patchwork_14112 > > CI-20190529: 20190529 > CI_DRM_6750: ba37f74dbdc1e78e70a5a2e5f4ce8d762d06eaa7 @ > git://anongit.freedesktop.org/gfx-ci/linux > IGT_5146: 357dbe1869d88a2f08bcee4eebceff4ee9014424 @ > git://anongit.freedesktop.org/xorg/app/intel-gpu-tools > Patchwork_14112: 1794a17edda30562d292dc4a140797ae38feb71b @ > git://anongit.freedesktop.org/gfx-ci/linux > piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ > git://anongit.freedesktop.org/piglit > > == Logs == > > For more details see: > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14112/
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 3bfb720560c2..77232f6bca17 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -390,7 +390,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) BUILD_BUG_ON(sizeof(aux_msg) > 20); for (i = 0; i < sizeof(aux_msg); i += 4) - I915_WRITE(EDP_PSR_AUX_DATA(i >> 2), + I915_WRITE(EDP_PSR_AUX_DATA(dev_priv->psr.transcoder, i >> 2), intel_dp_pack_aux(&aux_msg[i], sizeof(aux_msg) - i)); aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, 0); @@ -401,7 +401,7 @@ static void hsw_psr_setup_aux(struct intel_dp *intel_dp) /* Select only valid bits for SRD_AUX_CTL */ aux_ctl &= psr_aux_mask; - I915_WRITE(EDP_PSR_AUX_CTL, aux_ctl); + I915_WRITE(EDP_PSR_AUX_CTL(dev_priv->psr.transcoder), aux_ctl); } static void intel_psr_enable_sink(struct intel_dp *intel_dp) @@ -491,8 +491,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp) if (INTEL_GEN(dev_priv) >= 8) val |= EDP_PSR_CRC_ENABLE; - val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK; - I915_WRITE(EDP_PSR_CTL, val); + val |= (I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & + EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK); + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); } static void hsw_activate_psr2(struct intel_dp *intel_dp) @@ -528,9 +529,9 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp) * PSR2 HW is incorrectly using EDP_PSR_TP1_TP3_SEL and BSpec is * recommending keep this bit unset while PSR2 is enabled. */ - I915_WRITE(EDP_PSR_CTL, 0); + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), 0); - I915_WRITE(EDP_PSR2_CTL, val); + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); } static bool intel_psr2_config_valid(struct intel_dp *intel_dp, @@ -606,10 +607,9 @@ void intel_psr_compute_config(struct intel_dp *intel_dp, /* * HSW spec explicitly says PSR is tied to port A. - * BDW+ platforms with DDI implementation of PSR have different - * PSR registers per transcoder and we only implement transcoder EDP - * ones. Since by Display design transcoder EDP is tied to port A - * we can safely escape based on the port A. + * BDW+ platforms have a instance of PSR registers per transcoder but + * for now it only supports one instance of PSR, so lets keep it + * hardcoded to PORT_A */ if (dig_port->base.port != PORT_A) { DRM_DEBUG_KMS("PSR condition failed: Port not supported\n"); @@ -649,8 +649,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); if (INTEL_GEN(dev_priv) >= 9) - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); + WARN_ON(I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)) & EDP_PSR2_ENABLE); + WARN_ON(I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)) & EDP_PSR_ENABLE); WARN_ON(dev_priv->psr.active); lockdep_assert_held(&dev_priv->psr.lock); @@ -720,19 +720,37 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp, if (INTEL_GEN(dev_priv) < 11) mask |= EDP_PSR_DEBUG_MASK_DISP_REG_WRITE; - I915_WRITE(EDP_PSR_DEBUG, mask); + I915_WRITE(EDP_PSR_DEBUG(dev_priv->psr.transcoder), mask); } static void intel_psr_enable_locked(struct drm_i915_private *dev_priv, const struct intel_crtc_state *crtc_state) { struct intel_dp *intel_dp = dev_priv->psr.dp; + u32 val; WARN_ON(dev_priv->psr.enabled); dev_priv->psr.psr2_enabled = intel_psr2_enabled(dev_priv, crtc_state); dev_priv->psr.busy_frontbuffer_bits = 0; dev_priv->psr.pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; + dev_priv->psr.transcoder = crtc_state->cpu_transcoder; + + /* + * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR + * will still keep the error set even after the reset done in the + * irq_preinstall and irq_uninstall hooks. + * And enabling in this situation cause the screen to freeze in the + * first time that PSR HW tries to activate so lets keep PSR disabled + * to avoid any rendering problems. + */ + val = I915_READ(EDP_PSR_IIR); + val &= EDP_PSR_ERROR(edp_psr_shift(dev_priv->psr.transcoder)); + if (val) { + dev_priv->psr.sink_not_reliable = true; + DRM_DEBUG_KMS("PSR interruption error set, not enabling PSR\n"); + return; + } DRM_DEBUG_KMS("Enabling PSR%s\n", dev_priv->psr.psr2_enabled ? "2" : "1"); @@ -782,20 +800,27 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv) u32 val; if (!dev_priv->psr.active) { - if (INTEL_GEN(dev_priv) >= 9) - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); + if (INTEL_GEN(dev_priv) >= 9) { + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); + WARN_ON(val & EDP_PSR2_ENABLE); + } + + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); + WARN_ON(val & EDP_PSR_ENABLE); + return; } if (dev_priv->psr.psr2_enabled) { - val = I915_READ(EDP_PSR2_CTL); + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); WARN_ON(!(val & EDP_PSR2_ENABLE)); - I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE); + val &= ~EDP_PSR2_ENABLE; + I915_WRITE(EDP_PSR2_CTL(dev_priv->psr.transcoder), val); } else { - val = I915_READ(EDP_PSR_CTL); + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); WARN_ON(!(val & EDP_PSR_ENABLE)); - I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE); + val &= ~EDP_PSR_ENABLE; + I915_WRITE(EDP_PSR_CTL(dev_priv->psr.transcoder), val); } dev_priv->psr.active = false; } @@ -817,10 +842,10 @@ static void intel_psr_disable_locked(struct intel_dp *intel_dp) intel_psr_exit(dev_priv); if (dev_priv->psr.psr2_enabled) { - psr_status = EDP_PSR2_STATUS; + psr_status = EDP_PSR2_STATUS(dev_priv->psr.transcoder); psr_status_mask = EDP_PSR2_STATUS_STATE_MASK; } else { - psr_status = EDP_PSR_STATUS; + psr_status = EDP_PSR_STATUS(dev_priv->psr.transcoder); psr_status_mask = EDP_PSR_STATUS_STATE_MASK; } @@ -963,7 +988,8 @@ int intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state, * defensive enough to cover everything. */ - return __intel_wait_for_register(&dev_priv->uncore, EDP_PSR_STATUS, + return __intel_wait_for_register(&dev_priv->uncore, + EDP_PSR_STATUS(dev_priv->psr.transcoder), EDP_PSR_STATUS_STATE_MASK, EDP_PSR_STATUS_STATE_IDLE, 2, 50, out_value); @@ -979,10 +1005,10 @@ static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv) return false; if (dev_priv->psr.psr2_enabled) { - reg = EDP_PSR2_STATUS; + reg = EDP_PSR2_STATUS(dev_priv->psr.transcoder); mask = EDP_PSR2_STATUS_STATE_MASK; } else { - reg = EDP_PSR_STATUS; + reg = EDP_PSR_STATUS(dev_priv->psr.transcoder); mask = EDP_PSR_STATUS_STATE_MASK; } @@ -1208,36 +1234,24 @@ void intel_psr_flush(struct drm_i915_private *dev_priv, */ void intel_psr_init(struct drm_i915_private *dev_priv) { - u32 val; - if (!HAS_PSR(dev_priv)) return; - dev_priv->psr_mmio_base = IS_HASWELL(dev_priv) ? - HSW_EDP_PSR_BASE : BDW_EDP_PSR_BASE; - if (!dev_priv->psr.sink_support) return; + if (IS_HASWELL(dev_priv)) + /* + * HSW don't have PSR registers on the same space as transcoder + * so set this to a value that when subtract to the register + * in transcoder space results in the right offset for HSW + */ + dev_priv->hsw_psr_mmio_adjust = _SRD_CTL_EDP - _HSW_EDP_PSR_BASE; + if (i915_modparams.enable_psr == -1) if (INTEL_GEN(dev_priv) < 9 || !dev_priv->vbt.psr.enable) i915_modparams.enable_psr = 0; - /* - * If a PSR error happened and the driver is reloaded, the EDP_PSR_IIR - * will still keep the error set even after the reset done in the - * irq_preinstall and irq_uninstall hooks. - * And enabling in this situation cause the screen to freeze in the - * first time that PSR HW tries to activate so lets keep PSR disabled - * to avoid any rendering problems. - */ - val = I915_READ(EDP_PSR_IIR); - val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP)); - if (val) { - DRM_DEBUG_KMS("PSR interruption error set\n"); - dev_priv->psr.sink_not_reliable = true; - } - /* Set link_standby x link_off defaults */ if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) /* HSW and BDW require workarounds that we don't implement. */ diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 25f78196b964..45a9124e53b6 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -2796,7 +2796,7 @@ static int init_broadwell_mmio_info(struct intel_gvt *gvt) MMIO_D(CHICKEN_PIPESL_1(PIPE_C), D_BDW_PLUS); MMIO_D(WM_MISC, D_BDW); - MMIO_D(_MMIO(BDW_EDP_PSR_BASE), D_BDW); + MMIO_D(_MMIO(_SRD_CTL_EDP), D_BDW); MMIO_D(_MMIO(0x6671c), D_BDW_PLUS); MMIO_D(_MMIO(0x66c00), D_BDW_PLUS); diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b39226d7f8d2..6e4824daafae 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2132,7 +2132,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) "BUF_ON", "TG_ON" }; - val = I915_READ(EDP_PSR2_STATUS); + val = I915_READ(EDP_PSR2_STATUS(dev_priv->psr.transcoder)); status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >> EDP_PSR2_STATUS_STATE_SHIFT; if (status_val < ARRAY_SIZE(live_status)) @@ -2148,7 +2148,7 @@ psr_source_status(struct drm_i915_private *dev_priv, struct seq_file *m) "SRDOFFACK", "SRDENT_ON", }; - val = I915_READ(EDP_PSR_STATUS); + val = I915_READ(EDP_PSR_STATUS(dev_priv->psr.transcoder)); status_val = (val & EDP_PSR_STATUS_STATE_MASK) >> EDP_PSR_STATUS_STATE_SHIFT; if (status_val < ARRAY_SIZE(live_status)) @@ -2191,10 +2191,10 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) goto unlock; if (psr->psr2_enabled) { - val = I915_READ(EDP_PSR2_CTL); + val = I915_READ(EDP_PSR2_CTL(dev_priv->psr.transcoder)); enabled = val & EDP_PSR2_ENABLE; } else { - val = I915_READ(EDP_PSR_CTL); + val = I915_READ(EDP_PSR_CTL(dev_priv->psr.transcoder)); enabled = val & EDP_PSR_ENABLE; } seq_printf(m, "Source PSR ctl: %s [0x%08x]\n", @@ -2207,7 +2207,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) * SKL+ Perf counter is reset to 0 everytime DC state is entered */ if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) { - val = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK; + val = I915_READ(EDP_PSR_PERF_CNT(dev_priv->psr.transcoder)); + val &= EDP_PSR_PERF_CNT_MASK; seq_printf(m, "Performance counter: %u\n", val); } @@ -2225,8 +2226,11 @@ static int i915_edp_psr_status(struct seq_file *m, void *data) * Reading all 3 registers before hand to minimize crossing a * frame boundary between register reads */ - for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) - su_frames_val[frame / 3] = I915_READ(PSR2_SU_STATUS(frame)); + for (frame = 0; frame < PSR2_SU_STATUS_FRAMES; frame += 3) { + val = I915_READ(PSR2_SU_STATUS(dev_priv->psr.transcoder, + frame)); + su_frames_val[frame / 3] = val; + } seq_puts(m, "Frame:\tPSR2 SU blocks:\n"); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index eb31c1656cea..be999791abca 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -479,6 +479,7 @@ struct i915_psr { bool enabled; struct intel_dp *dp; enum pipe pipe; + enum transcoder transcoder; bool active; struct work_struct work; unsigned busy_frontbuffer_bits; @@ -1330,11 +1331,11 @@ struct drm_i915_private { */ u32 gpio_mmio_base; + u32 hsw_psr_mmio_adjust; + /* MMIO base address for MIPI regs */ u32 mipi_mmio_base; - u32 psr_mmio_base; - u32 pps_mmio_base; wait_queue_head_t gmbus_wait_queue; diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 2abd199093c5..a092b34c269d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4186,10 +4186,17 @@ enum { #define PIPESRC(trans) _MMIO_TRANS2(trans, _PIPEASRC) #define PIPE_MULT(trans) _MMIO_TRANS2(trans, _PIPE_MULT_A) -/* HSW+ eDP PSR registers */ -#define HSW_EDP_PSR_BASE 0x64800 -#define BDW_EDP_PSR_BASE 0x6f800 -#define EDP_PSR_CTL _MMIO(dev_priv->psr_mmio_base + 0) +/* + * HSW+ eDP PSR registers + * + * HSW PSR registers are relative to DDIA(_DDI_BUF_CTL_A + 0x800) with just one + * instance of it + */ +#define _HSW_EDP_PSR_BASE 0x64800 +#define _SRD_CTL_A 0x60800 +#define _SRD_CTL_EDP 0x6f800 +#define _PSR_ADJ(tran, reg) (_TRANS2(tran, reg) - dev_priv->hsw_psr_mmio_adjust) +#define EDP_PSR_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_CTL_A)) #define EDP_PSR_ENABLE (1 << 31) #define BDW_PSR_SINGLE_FRAME (1 << 30) #define EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK (1 << 29) /* SW can't modify */ @@ -4226,16 +4233,22 @@ enum { #define EDP_PSR_TRANSCODER_A_SHIFT 8 #define EDP_PSR_TRANSCODER_EDP_SHIFT 0 -#define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10) +#define _SRD_AUX_CTL_A 0x60810 +#define _SRD_AUX_CTL_EDP 0x6f810 +#define EDP_PSR_AUX_CTL(tran) _MMIO(_PSR_ADJ(tran, _SRD_AUX_CTL_A)) #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26) #define EDP_PSR_AUX_CTL_MESSAGE_SIZE_MASK (0x1f << 20) #define EDP_PSR_AUX_CTL_PRECHARGE_2US_MASK (0xf << 16) #define EDP_PSR_AUX_CTL_ERROR_INTERRUPT (1 << 11) #define EDP_PSR_AUX_CTL_BIT_CLOCK_2X_MASK (0x7ff) -#define EDP_PSR_AUX_DATA(i) _MMIO(dev_priv->psr_mmio_base + 0x14 + (i) * 4) /* 5 registers */ +#define _SRD_AUX_DATA_A 0x60814 +#define _SRD_AUX_DATA_EDP 0x6f814 +#define EDP_PSR_AUX_DATA(tran, i) _MMIO(_PSR_ADJ(tran, _SRD_AUX_DATA_A) + (i) + 4) /* 5 registers */ -#define EDP_PSR_STATUS _MMIO(dev_priv->psr_mmio_base + 0x40) +#define _SRD_STATUS_A 0x60840 +#define _SRD_STATUS_EDP 0x6f840 +#define EDP_PSR_STATUS(tran) _MMIO(_PSR_ADJ(tran, _SRD_STATUS_A)) #define EDP_PSR_STATUS_STATE_MASK (7 << 29) #define EDP_PSR_STATUS_STATE_SHIFT 29 #define EDP_PSR_STATUS_STATE_IDLE (0 << 29) @@ -4260,10 +4273,15 @@ enum { #define EDP_PSR_STATUS_SENDING_TP1 (1 << 4) #define EDP_PSR_STATUS_IDLE_MASK 0xf -#define EDP_PSR_PERF_CNT _MMIO(dev_priv->psr_mmio_base + 0x44) +#define _SRD_PERF_CNT_A 0x60844 +#define _SRD_PERF_CNT_EDP 0x6f844 +#define EDP_PSR_PERF_CNT(tran) _MMIO(_PSR_ADJ(tran, _SRD_PERF_CNT_A)) #define EDP_PSR_PERF_CNT_MASK 0xffffff -#define EDP_PSR_DEBUG _MMIO(dev_priv->psr_mmio_base + 0x60) /* PSR_MASK on SKL+ */ +/* PSR_MASK on SKL+ */ +#define _SRD_DEBUG_A 0x60860 +#define _SRD_DEBUG_EDP 0x6f860 +#define EDP_PSR_DEBUG(tran) _MMIO(_PSR_ADJ(tran, _SRD_DEBUG_A)) #define EDP_PSR_DEBUG_MASK_MAX_SLEEP (1 << 28) #define EDP_PSR_DEBUG_MASK_LPSP (1 << 27) #define EDP_PSR_DEBUG_MASK_MEMUP (1 << 26) @@ -4271,7 +4289,9 @@ enum { #define EDP_PSR_DEBUG_MASK_DISP_REG_WRITE (1 << 16) /* Reserved in ICL+ */ #define EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */ -#define EDP_PSR2_CTL _MMIO(0x6f900) +#define _PSR2_CTL_A 0x60900 +#define _PSR2_CTL_EDP 0x6f900 +#define EDP_PSR2_CTL(tran) _MMIO_TRANS2(tran, _PSR2_CTL_A) #define EDP_PSR2_ENABLE (1 << 31) #define EDP_SU_TRACK_ENABLE (1 << 30) #define EDP_Y_COORDINATE_VALID (1 << 26) /* GLK and CNL+ */ @@ -4293,8 +4313,8 @@ enum { #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_TRANS_EDP 0x6f848 +#define PSR_EVENT(tran) _MMIO_TRANS2(tran, _PSR_EVENT_TRANS_A) #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) @@ -4312,15 +4332,16 @@ enum { #define PSR_EVENT_LPSP_MODE_EXIT (1 << 1) #define PSR_EVENT_PSR_DISABLE (1 << 0) -#define EDP_PSR2_STATUS _MMIO(0x6f940) +#define _PSR2_STATUS_A 0x60940 +#define _PSR2_STATUS_EDP 0x6f940 +#define EDP_PSR2_STATUS(tran) _MMIO_TRANS2(tran, _PSR2_STATUS_A) #define EDP_PSR2_STATUS_STATE_MASK (0xf << 28) #define EDP_PSR2_STATUS_STATE_SHIFT 28 -#define _PSR2_SU_STATUS_0 0x6F914 -#define _PSR2_SU_STATUS_1 0x6F918 -#define _PSR2_SU_STATUS_2 0x6F91C -#define _PSR2_SU_STATUS(index) _MMIO(_PICK_EVEN((index), _PSR2_SU_STATUS_0, _PSR2_SU_STATUS_1)) -#define PSR2_SU_STATUS(frame) (_PSR2_SU_STATUS((frame) / 3)) +#define _PSR2_SU_STATUS_A 0x60914 +#define _PSR2_SU_STATUS_EDP 0x6f914 +#define _PSR2_SU_STATUS(tran, index) _MMIO(_TRANS2(tran, _PSR2_SU_STATUS_A) + (index) * 4) +#define PSR2_SU_STATUS(tran, frame) (_PSR2_SU_STATUS(tran, (frame) / 3)) #define PSR2_SU_STATUS_SHIFT(frame) (((frame) % 3) * 10) #define PSR2_SU_STATUS_MASK(frame) (0x3ff << PSR2_SU_STATUS_SHIFT(frame)) #define PSR2_SU_STATUS_FRAMES 8