> -----Original Message-----
> From: Shankar, Uma <uma.shankar@xxxxxxxxx>
> Sent: Thursday, August 22, 2024 10:50 AM
> To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; intel-
> gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Manna, Animesh <animesh.manna@xxxxxxxxx>; Murthy, Arun R
> <arun.r.murthy@xxxxxxxxx>; Hogander, Jouni <jouni.hogander@xxxxxxxxx>;
> jani.nikula@xxxxxxxxxxxxxxx; Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> Subject: RE: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
>
>
>
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Suraj Kandpal
> > Sent: Wednesday, June 19, 2024 10:08 AM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Manna, Animesh <animesh.manna@xxxxxxxxx>; Murthy, Arun R
> > <arun.r.murthy@xxxxxxxxx>; Hogander, Jouni
> <jouni.hogander@xxxxxxxxx>;
> > jani.nikula@xxxxxxxxxxxxxxx; Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> > Subject: [PATCH 2/2] drm/i915/psr: Implment WA to help reach PC10
>
> Nit: Typo in Implement
>
Wil fix.
> > To reach PC10 when PKG_C_LATENCY is configure we must do the
> following
> > things
> > 1) Enter PSR1 only when delayed_vblank < 6 lines and DC5 can be
> > entered
> > 2) Allow PSR2 deep sleep when DC5 can be entered
> > 3) DC5 can be entered when all transocoder have either PSR1, PSR2 or
> > eDP 1.5 PR ALPM enabled and VBI is disabled and flips and pushes are not
> happening.
> >
> > --v2
> > -Switch condition and do an early return [Jani] -Do some checks in
> > compute_config [Jani] -Do not use register reads as a method of
> > checking states for DPKGC or delayed vblank [Jani] -Use another way to
> > see is vblank interrupts are disabled or not [Jani]
> >
> > WA: 16023497226
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> > ---
> > .../drm/i915/display/intel_display_types.h | 2 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 82 ++++++++++++++++++-
> > 2 files changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 46b3cbeb4a82..031f8e889b65 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1708,6 +1708,8 @@ struct intel_psr {
> > bool sink_support;
> > bool source_support;
> > bool enabled;
> > + bool delayed_vblank;
> > + bool is_dpkgc_configured;
> > bool paused;
> > enum pipe pipe;
> > enum transcoder transcoder;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 080bf5e51148..4ddea6737386 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -808,6 +808,73 @@ static u8 psr_compute_idle_frames(struct
> intel_dp
> > *intel_dp)
> > return idle_frames;
> > }
> >
> > +static bool intel_psr_check_delayed_vblank_limit(struct
> > +intel_crtc_state *crtc_state) {
> > + struct drm_display_mode *adjusted_mode =
> > +&crtc_state->hw.adjusted_mode;
> > +
> > + return (adjusted_mode->crtc_vblank_start -
> > +adjusted_mode->crtc_vdisplay) >= 6; }
> > +
> > +/*
> > + * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
> > + * VRR is not enabled
> > + */
> > +static bool intel_psr_is_dpkgc_configured(struct drm_i915_private
> > +*i915) {
> > + struct intel_crtc *intel_crtc;
> > +
> > + if (DISPLAY_VER(i915) < 20)
> > + return false;
> > +
> > + for_each_intel_crtc(&i915->drm, intel_crtc) {
> > + struct intel_crtc_state *crtc_state;
> > +
> > + if (!intel_crtc->active)
> > + continue;
> > +
> > + crtc_state = intel_crtc->config;
> > +
> > + if (crtc_state->vrr.enable)
> > + return false;
> > + }
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * DC5 entry is only possible if vblank interrupt is disabled
> > + * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
> > + * enabled encoders.
> > + */
> > +static bool intel_psr_is_dc5_entry_possible(struct drm_i915_private
> > +*i915) {
> > + struct intel_crtc *intel_crtc;
> > +
> > + for_each_intel_crtc(&i915->drm, intel_crtc) {
> > + struct intel_encoder *encoder;
> > + struct drm_crtc *crtc = &intel_crtc->base;
> > + struct drm_vblank_crtc *vblank;
> > +
> > + if (!intel_crtc->active)
> > + continue;
> > +
> > + vblank = drm_crtc_vblank_crtc(crtc);
> > +
> > + if (vblank->enabled)
> > + return false;
> > +
> > + for_each_encoder_on_crtc(&i915->drm, crtc, encoder) {
> > + struct intel_dp *intel_dp =
> enc_to_intel_dp(encoder);
>
> All encoders on crtc may not be of type DP, need to be handled here.
>
Ahh ohkay will add a null check for intel_dp here and continue based on that
> > + struct intel_psr *psr = &intel_dp->psr;
> > +
> > + if (!psr->enabled)
> > + return false;
> > + }
> > + }
> > +
> > + return true;
> > +}
> > +
> > static bool hsw_activate_psr1(struct intel_dp *intel_dp) {
> > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -
> 815,6
> > +882,14 @@ static bool hsw_activate_psr1(struct intel_dp *intel_dp)
> > u32 max_sleep_time = 0x1f;
> > u32 val = EDP_PSR_ENABLE;
> >
> > + /* WA: 16023497226*/
> > + if (intel_dp->psr.is_dpkgc_configured &&
> > + (intel_dp->psr.delayed_vblank ||
> > intel_psr_is_dc5_entry_possible(dev_priv))) {
>
> In intel_psr_is_dc5_entry_possible function we use psr->enabled and based
> on that deciding to return from PSR1 activate, logic looks suspicious. Can
> you re-check once.
So this is so we can see if psr can be enabled on that encoder or not which will indicate we
We can enter dc5 or not and if we can then not to activate psr1 incase dpkgc is configured
Regards,
Suraj Kandpal
>
> > + drm_dbg_kms(&dev_priv->drm,
> > + "PSR1 not activated as it doesn't meet
> requirements
> > of WA:16023497226\n");
> > + return false;
> > + }
> > +
> > val |= EDP_PSR_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> >
> > if (DISPLAY_VER(dev_priv) < 20)
> > @@ -907,7 +982,10 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
> > u32 val = EDP_PSR2_ENABLE;
> > u32 psr_val = 0;
> >
> > - val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> > + /* WA: 16023497226*/
> > + if (intel_dp->psr.is_dpkgc_configured &&
> > + intel_psr_is_dc5_entry_possible(dev_priv))
> > + val |=
> > EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> >
> > if (DISPLAY_VER(dev_priv) < 14 && !IS_ALDERLAKE_P(dev_priv))
> > val |= EDP_SU_TRACK_ENABLE;
> > @@ -1502,6 +1580,8 @@ void intel_psr_compute_config(struct intel_dp
> > *intel_dp,
> > return;
> >
> > crtc_state->has_sel_update = intel_sel_update_config_valid(intel_dp,
> > crtc_state);
> > + intel_dp->psr.delayed_vblank =
> > intel_psr_check_delayed_vblank_limit(crtc_state);
> > + intel_dp->psr.is_dpkgc_configured =
> > +intel_psr_is_dpkgc_configured(dev_priv);
> > }
> >
> > void intel_psr_get_config(struct intel_encoder *encoder,
> > --
> > 2.43.2
