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@xxxxxxxxx>
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > > > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > > > > Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> > > > > > Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > > > > > ---
> > > > > > 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@xxxxxxxxxxxxxxxxxxxxx
> > > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
