| Message ID | 20240902050214.127352-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/psr: Implment WA to help reach PC10 | expand |
On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > 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] > > --v3 > -Use has_psr to check if psr can be enabled or not for dc5_entry cond > [Uma] > -Move the dc5 entry computation to psr_compute_config [Jouni] > -No need to change sequence of enabled and activate, > so dont make hsw_psr1_activate return anything [Jouni] > -Use has_psr to stop psr1 activation [Jouni] > -Use lineage no. in WA > -Add the display ver restrictions for WA > > WA: 22019444797 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 2 + > drivers/gpu/drm/i915/display/intel_psr.c | 96 > ++++++++++++++++++- > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1717,6 +1717,8 @@ struct intel_psr { > bool sink_support; > bool source_support; > bool enabled; > + bool is_dpkgc_configured; > + bool is_dc5_entry_possible; > 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 257526362b39..1faec76eac32 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -870,6 +870,69 @@ 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) You could add some context here in the name. This is somehow telling it's some generic delayed vblank limit while it is actually limit for this workaround. > +{ > + 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_state *crtc_state) > +{ > + struct intel_crtc *intel_crtc; > + > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > + return false; Currently this is not returning for DP2.1 PR. This would better match with comment above: if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) return false; Still "_all_ enabled encoders" is not handled... BR, Jouni Högander > + > + for_each_intel_crtc(&i915->drm, intel_crtc) { > + 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; > + } > + > + return true; > +} > + > static void hsw_activate_psr1(struct intel_dp *intel_dp) > { > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > @@ -980,7 +1043,11 @@ 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_22019444797 */ > + if (DISPLAY_VER(dev_priv) != 20 || > + (intel_dp->psr.is_dpkgc_configured && > + intel_dp->psr.is_dc5_entry_possible)) > + 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; > @@ -1595,6 +1662,32 @@ _panel_replay_compute_config(struct intel_dp > *intel_dp, > return true; > } > > +static void wa_22019444797(struct intel_dp *intel_dp, > + struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > + > + if (DISPLAY_VER(i915) != 20) > + return; > + > + intel_dp->psr.is_dpkgc_configured = > + intel_psr_is_dpkgc_configured(i915); > + intel_dp->psr.is_dc5_entry_possible = > + intel_psr_is_dc5_entry_possible(i915, crtc_state); > + > + /* PSR2 not handled here. Wa not needed for Panel Replay */ > + if (crtc_state->has_sel_update || crtc_state- > >has_panel_replay) > + return; > + > + if (intel_dp->psr.is_dpkgc_configured && > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > + intel_dp->psr.is_dc5_entry_possible)) { > + drm_dbg_kms(&i915->drm, > + "PSR1 not enabled as it doesn't meet > requirements of WA: 22019444797\n"); > + crtc_state->has_psr = false; > + } > +} > + > void intel_psr_compute_config(struct intel_dp *intel_dp, > struct intel_crtc_state *crtc_state, > struct drm_connector_state *conn_state) > @@ -1641,6 +1734,7 @@ 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); > + wa_22019444797(intel_dp, crtc_state); > } > > void intel_psr_get_config(struct intel_encoder *encoder,
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Monday, September 2, 2024 3:07 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > 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] > > > > --v3 > > -Use has_psr to check if psr can be enabled or not for dc5_entry cond > > [Uma] -Move the dc5 entry computation to psr_compute_config [Jouni] > > -No need to change sequence of enabled and activate, so dont make > > hsw_psr1_activate return anything [Jouni] -Use has_psr to stop psr1 > > activation [Jouni] -Use lineage no. in WA -Add the display ver > > restrictions for WA > > > > WA: 22019444797 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > .../drm/i915/display/intel_display_types.h | 2 + > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > ++++++++++++++++++- > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > bool sink_support; > > bool source_support; > > bool enabled; > > + bool is_dpkgc_configured; > > + bool is_dc5_entry_possible; > > 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 257526362b39..1faec76eac32 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -870,6 +870,69 @@ 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) > > You could add some context here in the name. This is somehow telling it's > some generic delayed vblank limit while it is actually limit for this > workaround. Sure will fix in next revision How about something like check_wa_delayed_vblank() > > > +{ > > + 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_state *crtc_state) { > > + struct intel_crtc *intel_crtc; > > + > > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > > + return false; > > Currently this is not returning for DP2.1 PR. This would better match with > comment above: > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > return false; > > Still "_all_ enabled encoders" is not handled... > How about in the below loop I add a encoder loop that checks if intel_dp_is_edp If the encoder is active and the psr check is taken care of by checking crtc_state->has_psr ? Previously I used to check if psr->enabled but since this is now called in psr compute config I only have has_psr to check If psr can be enabled on that encoder . Regards, Suraj Kandpal > BR, > > Jouni Högander > > > + > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > + 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; > > + } > > + > > + return true; > > +} > > + > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > { > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ > > -980,7 +1043,11 @@ 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_22019444797 */ > > + if (DISPLAY_VER(dev_priv) != 20 || > > + (intel_dp->psr.is_dpkgc_configured && > > + intel_dp->psr.is_dc5_entry_possible)) > > + 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; @@ -1595,6 +1662,32 @@ > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > return true; > > } > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > + struct intel_crtc_state *crtc_state) { > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + > > + if (DISPLAY_VER(i915) != 20) > > + return; > > + > > + intel_dp->psr.is_dpkgc_configured = > > + intel_psr_is_dpkgc_configured(i915); > > + intel_dp->psr.is_dc5_entry_possible = > > + intel_psr_is_dc5_entry_possible(i915, crtc_state); > > + > > + /* PSR2 not handled here. Wa not needed for Panel Replay */ > > + if (crtc_state->has_sel_update || crtc_state- > > >has_panel_replay) > > + return; > > + > > + if (intel_dp->psr.is_dpkgc_configured && > > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > > + intel_dp->psr.is_dc5_entry_possible)) { > > + drm_dbg_kms(&i915->drm, > > + "PSR1 not enabled as it doesn't meet > > requirements of WA: 22019444797\n"); > > + crtc_state->has_psr = false; > > + } > > +} > > + > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > struct intel_crtc_state *crtc_state, > > struct drm_connector_state *conn_state) > > @@ -1641,6 +1734,7 @@ 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); > > + wa_22019444797(intel_dp, crtc_state); > > } > > > > void intel_psr_get_config(struct intel_encoder *encoder,
On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > 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] > > > > --v3 > > -Use has_psr to check if psr can be enabled or not for dc5_entry > > cond > > [Uma] > > -Move the dc5 entry computation to psr_compute_config [Jouni] > > -No need to change sequence of enabled and activate, > > so dont make hsw_psr1_activate return anything [Jouni] > > -Use has_psr to stop psr1 activation [Jouni] > > -Use lineage no. in WA > > -Add the display ver restrictions for WA > > > > WA: 22019444797 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > .../drm/i915/display/intel_display_types.h | 2 + > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > ++++++++++++++++++- > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > bool sink_support; > > bool source_support; > > bool enabled; > > + bool is_dpkgc_configured; > > + bool is_dc5_entry_possible; > > 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 257526362b39..1faec76eac32 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -870,6 +870,69 @@ 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) > > You could add some context here in the name. This is somehow telling > it's some generic delayed vblank limit while it is actually limit for > this workaround. > > > +{ > > + 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_state > > *crtc_state) > > +{ > > + struct intel_crtc *intel_crtc; > > + > > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > > + return false; > > Currently this is not returning for DP2.1 PR. This would better match > with comment above: > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > return false; > > Still "_all_ enabled encoders" is not handled... > > BR, > > Jouni Högander > > > + > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > + 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; > > + } > > + > > + return true; > > +} > > + > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > { > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > @@ -980,7 +1043,11 @@ 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_22019444797 */ > > + if (DISPLAY_VER(dev_priv) != 20 || I think this is wrong. It will not configure idle frames for display version other than 20. BR, Jouni Högander > > + (intel_dp->psr.is_dpkgc_configured && > > + intel_dp->psr.is_dc5_entry_possible)) > > + 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; > > @@ -1595,6 +1662,32 @@ _panel_replay_compute_config(struct intel_dp > > *intel_dp, > > return true; > > } > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > + struct intel_crtc_state *crtc_state) > > +{ > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > + > > + if (DISPLAY_VER(i915) != 20) > > + return; > > + > > + intel_dp->psr.is_dpkgc_configured = > > + intel_psr_is_dpkgc_configured(i915); > > + intel_dp->psr.is_dc5_entry_possible = > > + intel_psr_is_dc5_entry_possible(i915, crtc_state); > > + > > + /* PSR2 not handled here. Wa not needed for Panel Replay */ > > + if (crtc_state->has_sel_update || crtc_state- > > > has_panel_replay) > > + return; > > + > > + if (intel_dp->psr.is_dpkgc_configured && > > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > > + intel_dp->psr.is_dc5_entry_possible)) { > > + drm_dbg_kms(&i915->drm, > > + "PSR1 not enabled as it doesn't meet > > requirements of WA: 22019444797\n"); > > + crtc_state->has_psr = false; > > + } > > +} > > + > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > struct intel_crtc_state *crtc_state, > > struct drm_connector_state > > *conn_state) > > @@ -1641,6 +1734,7 @@ 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); > > + wa_22019444797(intel_dp, crtc_state); > > } > > > > void intel_psr_get_config(struct intel_encoder *encoder, >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Monday, September 2, 2024 3:32 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > 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] > > > > > > --v3 > > > -Use has_psr to check if psr can be enabled or not for dc5_entry > > > cond [Uma] -Move the dc5 entry computation to psr_compute_config > > > [Jouni] -No need to change sequence of enabled and activate, so dont > > > make hsw_psr1_activate return anything [Jouni] -Use has_psr to stop > > > psr1 activation [Jouni] -Use lineage no. in WA -Add the display ver > > > restrictions for WA > > > > > > WA: 22019444797 > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > .../drm/i915/display/intel_display_types.h | 2 + > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > ++++++++++++++++++- > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > bool sink_support; > > > bool source_support; > > > bool enabled; > > > + bool is_dpkgc_configured; > > > + bool is_dc5_entry_possible; > > > 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 257526362b39..1faec76eac32 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -870,6 +870,69 @@ 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) > > > > You could add some context here in the name. This is somehow telling > > it's some generic delayed vblank limit while it is actually limit for > > this workaround. > > > > > +{ > > > + 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_state > > > *crtc_state) > > > +{ > > > + struct intel_crtc *intel_crtc; > > > + > > > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > > > + return false; > > > > Currently this is not returning for DP2.1 PR. This would better match > > with comment above: > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > return false; > > > > Still "_all_ enabled encoders" is not handled... > > > > BR, > > > > Jouni Högander > > > > > + > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > + 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; > > > + } > > > + > > > + return true; > > > +} > > > + > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > + if (DISPLAY_VER(dev_priv) != 20 || > > I think this is wrong. It will not configure idle frames for display version > other than 20. Wouldn’t this configure idle frames for all versions except 20 And if it is 20 then if dpkgc is configured and dc5 entry is possible Only then configure idle frames Regards, Suraj Kandpal > > BR, > > Jouni Högander > > > > + (intel_dp->psr.is_dpkgc_configured && > > > + intel_dp->psr.is_dc5_entry_possible)) > > > + 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; @@ -1595,6 +1662,32 @@ > > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > > return true; > > > } > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > + struct intel_crtc_state *crtc_state) { > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > + > > > + if (DISPLAY_VER(i915) != 20) > > > + return; > > > + > > > + intel_dp->psr.is_dpkgc_configured = > > > + intel_psr_is_dpkgc_configured(i915); > > > + intel_dp->psr.is_dc5_entry_possible = > > > + intel_psr_is_dc5_entry_possible(i915, crtc_state); > > > + > > > + /* PSR2 not handled here. Wa not needed for Panel Replay */ > > > + if (crtc_state->has_sel_update || crtc_state- > > > > has_panel_replay) > > > + return; > > > + > > > + if (intel_dp->psr.is_dpkgc_configured && > > > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > + drm_dbg_kms(&i915->drm, > > > + "PSR1 not enabled as it doesn't meet > > > requirements of WA: 22019444797\n"); > > > + crtc_state->has_psr = false; > > > + } > > > +} > > > + > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > struct intel_crtc_state *crtc_state, > > > struct drm_connector_state > > > *conn_state) > > > @@ -1641,6 +1734,7 @@ 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); > > > + wa_22019444797(intel_dp, crtc_state); > > > } > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > >
On Mon, 2024-09-02 at 10:14 +0000, Kandpal, Suraj wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Monday, September 2, 2024 3:32 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Shankar, Uma <uma.shankar@intel.com> > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > > 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] > > > > > > > > --v3 > > > > -Use has_psr to check if psr can be enabled or not for > > > > dc5_entry > > > > cond [Uma] -Move the dc5 entry computation to > > > > psr_compute_config > > > > [Jouni] -No need to change sequence of enabled and activate, so > > > > dont > > > > make hsw_psr1_activate return anything [Jouni] -Use has_psr to > > > > stop > > > > psr1 activation [Jouni] -Use lineage no. in WA -Add the display > > > > ver > > > > restrictions for WA > > > > > > > > WA: 22019444797 > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > > --- > > > > .../drm/i915/display/intel_display_types.h | 2 + > > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > > ++++++++++++++++++- > > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > > bool sink_support; > > > > bool source_support; > > > > bool enabled; > > > > + bool is_dpkgc_configured; > > > > + bool is_dc5_entry_possible; > > > > 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 257526362b39..1faec76eac32 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -870,6 +870,69 @@ 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) > > > > > > You could add some context here in the name. This is somehow > > > telling > > > it's some generic delayed vblank limit while it is actually limit > > > for > > > this workaround. > > > > > > > +{ > > > > + 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_state > > > > *crtc_state) > > > > +{ > > > > + struct intel_crtc *intel_crtc; > > > > + > > > > + if (!(crtc_state->has_psr || crtc_state- > > > > >has_sel_update)) > > > > + return false; > > > > > > Currently this is not returning for DP2.1 PR. This would better > > > match > > > with comment above: > > > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > > return false; > > > > > > Still "_all_ enabled encoders" is not handled... > > > > > > BR, > > > > > > Jouni Högander > > > > > > > + > > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > > + 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; > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > > { > > > > struct drm_i915_private *dev_priv = > > > > dp_to_i915(intel_dp); @@ > > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > I think this is wrong. It will not configure idle frames for > > display version > > other than 20. > > Wouldn’t this configure idle frames for all versions except 20 > And if it is 20 then if dpkgc is configured and dc5 entry is possible > Only then configure idle frames Yes. My bad. sorry for that. This should be ok for display version != 20. Another thing here. WA description is: "When PKG_C_LATENCY is configured (not all 1s), enable PSR2 deep sleep (PSR2_CTL Idle Frames != 0) only when DC5 can be entered." How about case where PKG_C_LATENCY is not configured? Do we need to care about it? BR, Jouni Högander > > Regards, > Suraj Kandpal > > > > BR, > > > > Jouni Högander > > > > > > + (intel_dp->psr.is_dpkgc_configured && > > > > + intel_dp->psr.is_dc5_entry_possible)) > > > > + 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; @@ -1595,6 +1662,32 > > > > @@ > > > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > > > return true; > > > > } > > > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > > + struct intel_crtc_state *crtc_state) > > > > { > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > + > > > > + if (DISPLAY_VER(i915) != 20) > > > > + return; > > > > + > > > > + intel_dp->psr.is_dpkgc_configured = > > > > + intel_psr_is_dpkgc_configured(i915); > > > > + intel_dp->psr.is_dc5_entry_possible = > > > > + intel_psr_is_dc5_entry_possible(i915, > > > > crtc_state); > > > > + > > > > + /* PSR2 not handled here. Wa not needed for Panel > > > > Replay */ > > > > + if (crtc_state->has_sel_update || crtc_state- > > > > > has_panel_replay) > > > > + return; > > > > + > > > > + if (intel_dp->psr.is_dpkgc_configured && > > > > + (intel_psr_check_delayed_vblank_limit(crtc_state) > > > > || > > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > > + drm_dbg_kms(&i915->drm, > > > > + "PSR1 not enabled as it doesn't > > > > meet > > > > requirements of WA: 22019444797\n"); > > > > + crtc_state->has_psr = false; > > > > + } > > > > +} > > > > + > > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > > struct intel_crtc_state > > > > *crtc_state, > > > > struct drm_connector_state > > > > *conn_state) > > > > @@ -1641,6 +1734,7 @@ 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); > > > > + wa_22019444797(intel_dp, crtc_state); > > > > } > > > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > > > >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Monday, September 2, 2024 4:32 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > On Mon, 2024-09-02 at 10:14 +0000, Kandpal, Suraj wrote: > > > > > > > -----Original Message----- > > > From: Hogander, Jouni <jouni.hogander@intel.com> > > > Sent: Monday, September 2, 2024 3:32 PM > > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Cc: Shankar, Uma <uma.shankar@intel.com> > > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > > > On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > > > 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] > > > > > > > > > > --v3 > > > > > -Use has_psr to check if psr can be enabled or not for dc5_entry > > > > > cond [Uma] -Move the dc5 entry computation to psr_compute_config > > > > > [Jouni] -No need to change sequence of enabled and activate, so > > > > > dont make hsw_psr1_activate return anything [Jouni] -Use has_psr > > > > > to stop > > > > > psr1 activation [Jouni] -Use lineage no. in WA -Add the display > > > > > ver restrictions for WA > > > > > > > > > > WA: 22019444797 > > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > > > --- > > > > > .../drm/i915/display/intel_display_types.h | 2 + > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > > > ++++++++++++++++++- > > > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > > > bool sink_support; > > > > > bool source_support; > > > > > bool enabled; > > > > > + bool is_dpkgc_configured; > > > > > + bool is_dc5_entry_possible; > > > > > 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 257526362b39..1faec76eac32 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > @@ -870,6 +870,69 @@ 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) > > > > > > > > You could add some context here in the name. This is somehow > > > > telling it's some generic delayed vblank limit while it is > > > > actually limit for this workaround. > > > > > > > > > +{ > > > > > + 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_state > > > > > *crtc_state) > > > > > +{ > > > > > + struct intel_crtc *intel_crtc; > > > > > + > > > > > + if (!(crtc_state->has_psr || crtc_state- > > > > > >has_sel_update)) > > > > > + return false; > > > > > > > > Currently this is not returning for DP2.1 PR. This would better > > > > match with comment above: > > > > > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > > > return false; > > > > > > > > Still "_all_ enabled encoders" is not handled... > > > > > > > > BR, > > > > > > > > Jouni Högander > > > > > > > > > + > > > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > > > + 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; > > > > > + } > > > > > + > > > > > + return true; > > > > > +} > > > > > + > > > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > > > { > > > > > struct drm_i915_private *dev_priv = > > > > > dp_to_i915(intel_dp); @@ > > > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > > > I think this is wrong. It will not configure idle frames for display > > > version other than 20. > > > > Wouldn’t this configure idle frames for all versions except 20 And if > > it is 20 then if dpkgc is configured and dc5 entry is possible Only > > then configure idle frames > > Yes. My bad. sorry for that. This should be ok for display version != 20. > Another thing here. WA description is: > > "When PKG_C_LATENCY is configured (not all 1s), enable PSR2 deep sleep > (PSR2_CTL Idle Frames != 0) only when DC5 can be entered." > > How about case where PKG_C_LATENCY is not configured? Do we need to > care about it? No we do not need to care about that the WA states this is only seen when dpkgc is configured On lnl Regards, Suraj Kandpal > > BR, > > Jouni Högander > > > > > Regards, > > Suraj Kandpal > > > > > > BR, > > > > > > Jouni Högander > > > > > > > > + (intel_dp->psr.is_dpkgc_configured && > > > > > + intel_dp->psr.is_dc5_entry_possible)) > > > > > + 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; @@ -1595,6 +1662,32 > > > > > @@ _panel_replay_compute_config(struct intel_dp *intel_dp, > > > > > return true; > > > > > } > > > > > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > > > + struct intel_crtc_state *crtc_state) > > > > > { > > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > > + > > > > > + if (DISPLAY_VER(i915) != 20) > > > > > + return; > > > > > + > > > > > + intel_dp->psr.is_dpkgc_configured = > > > > > + intel_psr_is_dpkgc_configured(i915); > > > > > + intel_dp->psr.is_dc5_entry_possible = > > > > > + intel_psr_is_dc5_entry_possible(i915, > > > > > crtc_state); > > > > > + > > > > > + /* PSR2 not handled here. Wa not needed for Panel > > > > > Replay */ > > > > > + if (crtc_state->has_sel_update || crtc_state- > > > > > > has_panel_replay) > > > > > + return; > > > > > + > > > > > + if (intel_dp->psr.is_dpkgc_configured && > > > > > + (intel_psr_check_delayed_vblank_limit(crtc_state) > > > > > || > > > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > > > + drm_dbg_kms(&i915->drm, > > > > > + "PSR1 not enabled as it doesn't > > > > > meet > > > > > requirements of WA: 22019444797\n"); > > > > > + crtc_state->has_psr = false; > > > > > + } > > > > > +} > > > > > + > > > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > > > struct intel_crtc_state > > > > > *crtc_state, > > > > > struct drm_connector_state > > > > > *conn_state) > > > > > @@ -1641,6 +1734,7 @@ 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); > > > > > + wa_22019444797(intel_dp, crtc_state); > > > > > } > > > > > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > > > > > >
On Mon, 2024-09-02 at 11:07 +0000, Kandpal, Suraj wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Monday, September 2, 2024 4:32 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Shankar, Uma <uma.shankar@intel.com> > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > On Mon, 2024-09-02 at 10:14 +0000, Kandpal, Suraj wrote: > > > > > > > > > > -----Original Message----- > > > > From: Hogander, Jouni <jouni.hogander@intel.com> > > > > Sent: Monday, September 2, 2024 3:32 PM > > > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > > > gfx@lists.freedesktop.org > > > > Cc: Shankar, Uma <uma.shankar@intel.com> > > > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach > > > > PC10 > > > > > > > > On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > > > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > > > > 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] > > > > > > > > > > > > --v3 > > > > > > -Use has_psr to check if psr can be enabled or not for > > > > > > dc5_entry > > > > > > cond [Uma] -Move the dc5 entry computation to > > > > > > psr_compute_config > > > > > > [Jouni] -No need to change sequence of enabled and > > > > > > activate, so > > > > > > dont make hsw_psr1_activate return anything [Jouni] -Use > > > > > > has_psr > > > > > > to stop > > > > > > psr1 activation [Jouni] -Use lineage no. in WA -Add the > > > > > > display > > > > > > ver restrictions for WA > > > > > > > > > > > > WA: 22019444797 > > > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > > > > --- > > > > > > .../drm/i915/display/intel_display_types.h | 2 + > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > > > > ++++++++++++++++++- > > > > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > > > > bool sink_support; > > > > > > bool source_support; > > > > > > bool enabled; > > > > > > + bool is_dpkgc_configured; > > > > > > + bool is_dc5_entry_possible; > > > > > > 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 257526362b39..1faec76eac32 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > @@ -870,6 +870,69 @@ 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) > > > > > > > > > > You could add some context here in the name. This is somehow > > > > > telling it's some generic delayed vblank limit while it is > > > > > actually limit for this workaround. > > > > > > > > > > > +{ > > > > > > + 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_state > > > > > > *crtc_state) > > > > > > +{ > > > > > > + struct intel_crtc *intel_crtc; > > > > > > + > > > > > > + if (!(crtc_state->has_psr || crtc_state- > > > > > > > has_sel_update)) > > > > > > + return false; > > > > > > > > > > Currently this is not returning for DP2.1 PR. This would > > > > > better > > > > > match with comment above: > > > > > > > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > > > > return false; > > > > > > > > > > Still "_all_ enabled encoders" is not handled... > > > > > > > > > > BR, > > > > > > > > > > Jouni Högander > > > > > > > > > > > + > > > > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > > > > + 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; > > > > > > + } > > > > > > + > > > > > > + return true; > > > > > > +} > > > > > > + > > > > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > > > > { > > > > > > struct drm_i915_private *dev_priv = > > > > > > dp_to_i915(intel_dp); @@ > > > > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > > > > > I think this is wrong. It will not configure idle frames for > > > > display > > > > version other than 20. > > > > > > Wouldn’t this configure idle frames for all versions except 20 > > > And if > > > it is 20 then if dpkgc is configured and dc5 entry is possible > > > Only > > > then configure idle frames > > > > Yes. My bad. sorry for that. This should be ok for display version > > != 20. > > Another thing here. WA description is: > > > > > "When PKG_C_LATENCY is configured (not all 1s), enable PSR2 deep > > sleep > > (PSR2_CTL Idle Frames != 0) only when DC5 can be entered." > > > > How about case where PKG_C_LATENCY is not configured? Do we need to > > care about it? > > No we do not need to care about that the WA states this is only seen > when dpkgc is configured > On lnl Ok, but you are still applying the WA (i.e. leaving PSR2_CTL[idle frames] as 0) for case where dpkc is not configured. BR, Jouni Högander > > Regards, > Suraj Kandpal > > > > > BR, > > > > Jouni Högander > > > > > > > > Regards, > > > Suraj Kandpal > > > > > > > > BR, > > > > > > > > Jouni Högander > > > > > > > > > > + (intel_dp->psr.is_dpkgc_configured && > > > > > > + intel_dp->psr.is_dc5_entry_possible)) > > > > > > + 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; @@ -1595,6 > > > > > > +1662,32 > > > > > > @@ _panel_replay_compute_config(struct intel_dp *intel_dp, > > > > > > return true; > > > > > > } > > > > > > > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > > > > + struct intel_crtc_state > > > > > > *crtc_state) > > > > > > { > > > > > > + struct drm_i915_private *i915 = > > > > > > dp_to_i915(intel_dp); > > > > > > + > > > > > > + if (DISPLAY_VER(i915) != 20) > > > > > > + return; > > > > > > + > > > > > > + intel_dp->psr.is_dpkgc_configured = > > > > > > + intel_psr_is_dpkgc_configured(i915); > > > > > > + intel_dp->psr.is_dc5_entry_possible = > > > > > > + intel_psr_is_dc5_entry_possible(i915, > > > > > > crtc_state); > > > > > > + > > > > > > + /* PSR2 not handled here. Wa not needed for Panel > > > > > > Replay */ > > > > > > + if (crtc_state->has_sel_update || crtc_state- > > > > > > > has_panel_replay) > > > > > > + return; > > > > > > + > > > > > > + if (intel_dp->psr.is_dpkgc_configured && > > > > > > + > > > > > > (intel_psr_check_delayed_vblank_limit(crtc_state) > > > > > > > > > > > > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > > > > + drm_dbg_kms(&i915->drm, > > > > > > + "PSR1 not enabled as it doesn't > > > > > > meet > > > > > > requirements of WA: 22019444797\n"); > > > > > > + crtc_state->has_psr = false; > > > > > > + } > > > > > > +} > > > > > > + > > > > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > > > > struct intel_crtc_state > > > > > > *crtc_state, > > > > > > struct drm_connector_state > > > > > > *conn_state) > > > > > > @@ -1641,6 +1734,7 @@ 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); > > > > > > + wa_22019444797(intel_dp, crtc_state); > > > > > > } > > > > > > > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > > > > > > > > >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Monday, September 2, 2024 4:44 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > On Mon, 2024-09-02 at 11:07 +0000, Kandpal, Suraj wrote: > > > > > > > -----Original Message----- > > > From: Hogander, Jouni <jouni.hogander@intel.com> > > > Sent: Monday, September 2, 2024 4:32 PM > > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Cc: Shankar, Uma <uma.shankar@intel.com> > > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > > > On Mon, 2024-09-02 at 10:14 +0000, Kandpal, Suraj wrote: > > > > > > > > > > > > > -----Original Message----- > > > > > From: Hogander, Jouni <jouni.hogander@intel.com> > > > > > Sent: Monday, September 2, 2024 3:32 PM > > > > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > > > > gfx@lists.freedesktop.org > > > > > Cc: Shankar, Uma <uma.shankar@intel.com> > > > > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach > > > > > PC10 > > > > > > > > > > On Mon, 2024-09-02 at 12:37 +0300, Hogander, Jouni wrote: > > > > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > > > > > 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] > > > > > > > > > > > > > > --v3 > > > > > > > -Use has_psr to check if psr can be enabled or not for > > > > > > > dc5_entry cond [Uma] -Move the dc5 entry computation to > > > > > > > psr_compute_config [Jouni] -No need to change sequence of > > > > > > > enabled and activate, so dont make hsw_psr1_activate return > > > > > > > anything [Jouni] -Use has_psr to stop > > > > > > > psr1 activation [Jouni] -Use lineage no. in WA -Add the > > > > > > > display ver restrictions for WA > > > > > > > > > > > > > > WA: 22019444797 > > > > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > > > > > --- > > > > > > > .../drm/i915/display/intel_display_types.h | 2 + > > > > > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > > > > > ++++++++++++++++++- > > > > > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > > > > > bool sink_support; > > > > > > > bool source_support; > > > > > > > bool enabled; > > > > > > > + bool is_dpkgc_configured; > > > > > > > + bool is_dc5_entry_possible; > > > > > > > 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 257526362b39..1faec76eac32 100644 > > > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > > > > @@ -870,6 +870,69 @@ 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) > > > > > > > > > > > > You could add some context here in the name. This is somehow > > > > > > telling it's some generic delayed vblank limit while it is > > > > > > actually limit for this workaround. > > > > > > > > > > > > > +{ > > > > > > > + 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_state > > > > > > > *crtc_state) > > > > > > > +{ > > > > > > > + struct intel_crtc *intel_crtc; > > > > > > > + > > > > > > > + if (!(crtc_state->has_psr || crtc_state- > > > > > > > > has_sel_update)) > > > > > > > + return false; > > > > > > > > > > > > Currently this is not returning for DP2.1 PR. This would > > > > > > better match with comment above: > > > > > > > > > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > > > > > return false; > > > > > > > > > > > > Still "_all_ enabled encoders" is not handled... > > > > > > > > > > > > BR, > > > > > > > > > > > > Jouni Högander > > > > > > > > > > > > > + > > > > > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > > > > > + 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; > > > > > > > + } > > > > > > > + > > > > > > > + return true; > > > > > > > +} > > > > > > > + > > > > > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > > > > > { > > > > > > > struct drm_i915_private *dev_priv = > > > > > > > dp_to_i915(intel_dp); @@ > > > > > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > > > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > > > > > > > I think this is wrong. It will not configure idle frames for > > > > > display version other than 20. > > > > > > > > Wouldn’t this configure idle frames for all versions except 20 And > > > > if it is 20 then if dpkgc is configured and dc5 entry is possible > > > > Only then configure idle frames > > > > > > Yes. My bad. sorry for that. This should be ok for display version > > > != 20. > > > Another thing here. WA description is: > > > > > > > > "When PKG_C_LATENCY is configured (not all 1s), enable PSR2 deep > > > sleep (PSR2_CTL Idle Frames != 0) only when DC5 can be entered." > > > > > > How about case where PKG_C_LATENCY is not configured? Do we need > to > > > care about it? > > > > No we do not need to care about that the WA states this is only seen > > when dpkgc is configured On lnl > > Ok, but you are still applying the WA (i.e. leaving PSR2_CTL[idle frames] as > 0) for case where dpkc is not configured. Ahh okay got it needed an extra condition which checks || !is_dpkgc_configured Regards, Suraj Kandpal > > BR, > > Jouni Högander > > > > > Regards, > > Suraj Kandpal > > > > > > > > BR, > > > > > > Jouni Högander > > > > > > > > > > > Regards, > > > > Suraj Kandpal > > > > > > > > > > BR, > > > > > > > > > > Jouni Högander > > > > > > > > > > > > + (intel_dp->psr.is_dpkgc_configured && > > > > > > > + intel_dp->psr.is_dc5_entry_possible)) > > > > > > > + 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; @@ -1595,6 > > > > > > > +1662,32 > > > > > > > @@ _panel_replay_compute_config(struct intel_dp *intel_dp, > > > > > > > return true; > > > > > > > } > > > > > > > > > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > > > > > + struct intel_crtc_state > > > > > > > *crtc_state) > > > > > > > { > > > > > > > + struct drm_i915_private *i915 = > > > > > > > dp_to_i915(intel_dp); > > > > > > > + > > > > > > > + if (DISPLAY_VER(i915) != 20) > > > > > > > + return; > > > > > > > + > > > > > > > + intel_dp->psr.is_dpkgc_configured = > > > > > > > + intel_psr_is_dpkgc_configured(i915); > > > > > > > + intel_dp->psr.is_dc5_entry_possible = > > > > > > > + intel_psr_is_dc5_entry_possible(i915, > > > > > > > crtc_state); > > > > > > > + > > > > > > > + /* PSR2 not handled here. Wa not needed for Panel > > > > > > > Replay */ > > > > > > > + if (crtc_state->has_sel_update || crtc_state- > > > > > > > > has_panel_replay) > > > > > > > + return; > > > > > > > + > > > > > > > + if (intel_dp->psr.is_dpkgc_configured && > > > > > > > + > > > > > > > (intel_psr_check_delayed_vblank_limit(crtc_state) > > > > > > > > > > > > > > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > > > > > + drm_dbg_kms(&i915->drm, > > > > > > > + "PSR1 not enabled as it doesn't > > > > > > > meet > > > > > > > requirements of WA: 22019444797\n"); > > > > > > > + crtc_state->has_psr = false; > > > > > > > + } > > > > > > > +} > > > > > > > + > > > > > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > > > > > struct intel_crtc_state > > > > > > > *crtc_state, > > > > > > > struct drm_connector_state > > > > > > > *conn_state) > > > > > > > @@ -1641,6 +1734,7 @@ 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); > > > > > > > + wa_22019444797(intel_dp, crtc_state); > > > > > > > } > > > > > > > > > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > > > > > > > > > > > >
On Mon, 2024-09-02 at 10:01 +0000, Kandpal, Suraj wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <jouni.hogander@intel.com> > > Sent: Monday, September 2, 2024 3:07 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > gfx@lists.freedesktop.org > > Cc: Shankar, Uma <uma.shankar@intel.com> > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > 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] > > > > > > --v3 > > > -Use has_psr to check if psr can be enabled or not for dc5_entry > > > cond > > > [Uma] -Move the dc5 entry computation to psr_compute_config > > > [Jouni] > > > -No need to change sequence of enabled and activate, so dont make > > > hsw_psr1_activate return anything [Jouni] -Use has_psr to stop > > > psr1 > > > activation [Jouni] -Use lineage no. in WA -Add the display ver > > > restrictions for WA > > > > > > WA: 22019444797 > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > .../drm/i915/display/intel_display_types.h | 2 + > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > ++++++++++++++++++- > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > bool sink_support; > > > bool source_support; > > > bool enabled; > > > + bool is_dpkgc_configured; > > > + bool is_dc5_entry_possible; > > > 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 257526362b39..1faec76eac32 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -870,6 +870,69 @@ 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) > > > > You could add some context here in the name. This is somehow > > telling it's > > some generic delayed vblank limit while it is actually limit for > > this > > workaround. > > Sure will fix in next revision > How about something like check_wa_delayed_vblank() > > > > > > +{ > > > + 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_state > > > *crtc_state) { > > > + struct intel_crtc *intel_crtc; > > > + > > > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > > > + return false; > > > > Currently this is not returning for DP2.1 PR. This would better > > match with > > comment above: > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > return false; > > > > Still "_all_ enabled encoders" is not handled... > > > > How about in the below loop I add a encoder loop that checks if > intel_dp_is_edp > If the encoder is active and the psr check is taken care of by > checking crtc_state->has_psr ? > Previously I used to check if psr->enabled but since this is now > called in psr compute config I only have has_psr to check > If psr can be enabled on that encoder . Loop idea is ok. Also crtc_state->has_psr. Shouldn't you also handle non-edp as well? If there is any non-edp encoder active I think dc5 will be blocked? BR, Jouni Högander > > Regards, > Suraj Kandpal > > BR, > > > > Jouni Högander > > > > > + > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > + 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; > > > + } > > > + > > > + return true; > > > +} > > > + > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > { > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > @@ > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > + (intel_dp->psr.is_dpkgc_configured && > > > + intel_dp->psr.is_dc5_entry_possible)) > > > + 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; @@ -1595,6 +1662,32 > > > @@ > > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > > return true; > > > } > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > + struct intel_crtc_state *crtc_state) { > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > + > > > + if (DISPLAY_VER(i915) != 20) > > > + return; > > > + > > > + intel_dp->psr.is_dpkgc_configured = > > > + intel_psr_is_dpkgc_configured(i915); > > > + intel_dp->psr.is_dc5_entry_possible = > > > + intel_psr_is_dc5_entry_possible(i915, > > > crtc_state); > > > + > > > + /* PSR2 not handled here. Wa not needed for Panel Replay > > > */ > > > + if (crtc_state->has_sel_update || crtc_state- > > > > has_panel_replay) > > > + return; > > > + > > > + if (intel_dp->psr.is_dpkgc_configured && > > > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > + drm_dbg_kms(&i915->drm, > > > + "PSR1 not enabled as it doesn't meet > > > requirements of WA: 22019444797\n"); > > > + crtc_state->has_psr = false; > > > + } > > > +} > > > + > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > struct intel_crtc_state > > > *crtc_state, > > > struct drm_connector_state > > > *conn_state) > > > @@ -1641,6 +1734,7 @@ 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); > > > + wa_22019444797(intel_dp, crtc_state); > > > } > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, >
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Tuesday, September 3, 2024 1:12 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > gfx@lists.freedesktop.org > Cc: Shankar, Uma <uma.shankar@intel.com> > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > On Mon, 2024-09-02 at 10:01 +0000, Kandpal, Suraj wrote: > > > > > > > -----Original Message----- > > > From: Hogander, Jouni <jouni.hogander@intel.com> > > > Sent: Monday, September 2, 2024 3:07 PM > > > To: Kandpal, Suraj <suraj.kandpal@intel.com>; intel- > > > gfx@lists.freedesktop.org > > > Cc: Shankar, Uma <uma.shankar@intel.com> > > > Subject: Re: [PATCH] drm/i915/psr: Implment WA to help reach PC10 > > > > > > On Mon, 2024-09-02 at 10:32 +0530, Suraj Kandpal wrote: > > > > 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] > > > > > > > > --v3 > > > > -Use has_psr to check if psr can be enabled or not for dc5_entry > > > > cond [Uma] -Move the dc5 entry computation to psr_compute_config > > > > [Jouni] -No need to change sequence of enabled and activate, so > > > > dont make hsw_psr1_activate return anything [Jouni] -Use has_psr > > > > to stop > > > > psr1 > > > > activation [Jouni] -Use lineage no. in WA -Add the display ver > > > > restrictions for WA > > > > > > > > WA: 22019444797 > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > > --- > > > > .../drm/i915/display/intel_display_types.h | 2 + > > > > drivers/gpu/drm/i915/display/intel_psr.c | 96 > > > > ++++++++++++++++++- > > > > 2 files changed, 97 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 868ff8976ed9..5395c1ecde7f 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > @@ -1717,6 +1717,8 @@ struct intel_psr { > > > > bool sink_support; > > > > bool source_support; > > > > bool enabled; > > > > + bool is_dpkgc_configured; > > > > + bool is_dc5_entry_possible; > > > > 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 257526362b39..1faec76eac32 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > > @@ -870,6 +870,69 @@ 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) > > > > > > You could add some context here in the name. This is somehow telling > > > it's some generic delayed vblank limit while it is actually limit > > > for this workaround. > > > > Sure will fix in next revision > > How about something like check_wa_delayed_vblank() > > > > > > > > > +{ > > > > + 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_state > > > > *crtc_state) { > > > > + struct intel_crtc *intel_crtc; > > > > + > > > > + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) > > > > + return false; > > > > > > Currently this is not returning for DP2.1 PR. This would better > > > match with comment above: > > > > > > if (!crtc_state->has_psr || !intel_dp_is_edp(intel_dp)) > > > return false; > > > > > > Still "_all_ enabled encoders" is not handled... > > > > > > > How about in the below loop I add a encoder loop that checks if > > intel_dp_is_edp If the encoder is active and the psr check is taken > > care of by checking crtc_state->has_psr ? > > Previously I used to check if psr->enabled but since this is now > > called in psr compute config I only have has_psr to check If psr can > > be enabled on that encoder . > > Loop idea is ok. Also crtc_state->has_psr. Shouldn't you also handle non-edp > as well? If there is any non-edp encoder active I think dc5 will be blocked? > Yes since the condition to enter dc5 is to have either psr1,psr2, or edp1.5 alpm pr Which would mean in the loop we need to check if encoder->type != INTEL_OUTPUT_EDP We return false. Regards, Suraj Kandpal > BR, > > Jouni Högander > > > > > Regards, > > Suraj Kandpal > > > BR, > > > > > > Jouni Högander > > > > > > > + > > > > + for_each_intel_crtc(&i915->drm, intel_crtc) { > > > > + 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; > > > > + } > > > > + > > > > + return true; > > > > +} > > > > + > > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > > { > > > > struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); > > > > @@ > > > > -980,7 +1043,11 @@ 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_22019444797 */ > > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > + (intel_dp->psr.is_dpkgc_configured && > > > > + intel_dp->psr.is_dc5_entry_possible)) > > > > + 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; @@ -1595,6 +1662,32 @@ > > > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > > > return true; > > > > } > > > > > > > > +static void wa_22019444797(struct intel_dp *intel_dp, > > > > + struct intel_crtc_state *crtc_state) { > > > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > + > > > > + if (DISPLAY_VER(i915) != 20) > > > > + return; > > > > + > > > > + intel_dp->psr.is_dpkgc_configured = > > > > + intel_psr_is_dpkgc_configured(i915); > > > > + intel_dp->psr.is_dc5_entry_possible = > > > > + intel_psr_is_dc5_entry_possible(i915, > > > > crtc_state); > > > > + > > > > + /* PSR2 not handled here. Wa not needed for Panel Replay > > > > */ > > > > + if (crtc_state->has_sel_update || crtc_state- > > > > > has_panel_replay) > > > > + return; > > > > + > > > > + if (intel_dp->psr.is_dpkgc_configured && > > > > + (intel_psr_check_delayed_vblank_limit(crtc_state) || > > > > + intel_dp->psr.is_dc5_entry_possible)) { > > > > + drm_dbg_kms(&i915->drm, > > > > + "PSR1 not enabled as it doesn't meet > > > > requirements of WA: 22019444797\n"); > > > > + crtc_state->has_psr = false; > > > > + } > > > > +} > > > > + > > > > void intel_psr_compute_config(struct intel_dp *intel_dp, > > > > struct intel_crtc_state *crtc_state, > > > > struct drm_connector_state > > > > *conn_state) > > > > @@ -1641,6 +1734,7 @@ 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); > > > > + wa_22019444797(intel_dp, crtc_state); > > > > } > > > > > > > > void intel_psr_get_config(struct intel_encoder *encoder, > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 868ff8976ed9..5395c1ecde7f 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1717,6 +1717,8 @@ struct intel_psr { bool sink_support; bool source_support; bool enabled; + bool is_dpkgc_configured; + bool is_dc5_entry_possible; 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 257526362b39..1faec76eac32 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -870,6 +870,69 @@ 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_state *crtc_state) +{ + struct intel_crtc *intel_crtc; + + if (!(crtc_state->has_psr || crtc_state->has_sel_update)) + return false; + + for_each_intel_crtc(&i915->drm, intel_crtc) { + 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; + } + + return true; +} + static void hsw_activate_psr1(struct intel_dp *intel_dp) { struct drm_i915_private *dev_priv = dp_to_i915(intel_dp); @@ -980,7 +1043,11 @@ 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_22019444797 */ + if (DISPLAY_VER(dev_priv) != 20 || + (intel_dp->psr.is_dpkgc_configured && + intel_dp->psr.is_dc5_entry_possible)) + 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; @@ -1595,6 +1662,32 @@ _panel_replay_compute_config(struct intel_dp *intel_dp, return true; } +static void wa_22019444797(struct intel_dp *intel_dp, + struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *i915 = dp_to_i915(intel_dp); + + if (DISPLAY_VER(i915) != 20) + return; + + intel_dp->psr.is_dpkgc_configured = + intel_psr_is_dpkgc_configured(i915); + intel_dp->psr.is_dc5_entry_possible = + intel_psr_is_dc5_entry_possible(i915, crtc_state); + + /* PSR2 not handled here. Wa not needed for Panel Replay */ + if (crtc_state->has_sel_update || crtc_state->has_panel_replay) + return; + + if (intel_dp->psr.is_dpkgc_configured && + (intel_psr_check_delayed_vblank_limit(crtc_state) || + intel_dp->psr.is_dc5_entry_possible)) { + drm_dbg_kms(&i915->drm, + "PSR1 not enabled as it doesn't meet requirements of WA: 22019444797\n"); + crtc_state->has_psr = false; + } +} + void intel_psr_compute_config(struct intel_dp *intel_dp, struct intel_crtc_state *crtc_state, struct drm_connector_state *conn_state) @@ -1641,6 +1734,7 @@ 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); + wa_22019444797(intel_dp, crtc_state); } void intel_psr_get_config(struct intel_encoder *encoder,
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] --v3 -Use has_psr to check if psr can be enabled or not for dc5_entry cond [Uma] -Move the dc5 entry computation to psr_compute_config [Jouni] -No need to change sequence of enabled and activate, so dont make hsw_psr1_activate return anything [Jouni] -Use has_psr to stop psr1 activation [Jouni] -Use lineage no. in WA -Add the display ver restrictions for WA WA: 22019444797 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- .../drm/i915/display/intel_display_types.h | 2 + drivers/gpu/drm/i915/display/intel_psr.c | 96 ++++++++++++++++++- 2 files changed, 97 insertions(+), 1 deletion(-)