| Message ID | 20240920091203.1043308-2-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/psr: Implement WA to help reach PC10 | expand |
On Fri, 2024-09-20 at 14:42 +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. One comment below related to PantherLake. Otherwise patch looks ok to me. > > --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 > > --v4 > -use more appropriate name for check_vblank_limit() [Jouni] > -Cover the case for idle frames when dpkgc is not configured [Jouni] > -Check psr only for edp [Jouni] > > --v5 > -move psr1 handling to plane update [Jouni] > -add todo for cases when vblank is enabled when psr enabled [Jouni] > -use intel_display instead of drm_i915_private > > --v6 > -check target_dc_state [Jouni] > -fix condition in pre/post plane update [Jouni] > > --v7 > -fix has_psr condition [Uma] > -fix typo in commit subject [Uma] > -put psr1_wa check in its own helper [Uma] > -fix the dc_entry check [Jouni] > -use HAS_PSR() to cover two edp one with psr and one nonpsr [Jouni] > > WA: 22019444797 > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 3 + > drivers/gpu/drm/i915/display/intel_psr.c | 119 > +++++++++++++++++- > 2 files changed, 121 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 3e694c1204db..2d790abee76e 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -1577,6 +1577,9 @@ struct intel_psr { > #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE 0x40 > > u32 debug; > + bool is_dpkgc_configured; > + bool is_dc5_entry_possible; > + bool is_wa_delayed_vblank_limit; > bool sink_support; > bool source_support; > bool enabled; > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > b/drivers/gpu/drm/i915/display/intel_psr.c > index 5b355d0a3565..b882ff25fb92 100644 > --- a/drivers/gpu/drm/i915/display/intel_psr.c > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > @@ -26,6 +26,7 @@ > #include <drm/drm_atomic_helper.h> > #include <drm/drm_damage_helper.h> > #include <drm/drm_debugfs.h> > +#include <drm/drm_vblank.h> > > #include "i915_drv.h" > #include "i915_reg.h" > @@ -895,6 +896,89 @@ static u8 psr_compute_idle_frames(struct > intel_dp *intel_dp) > return idle_frames; > } > > +static bool > +intel_psr_check_wa_delayed_vblank(const struct drm_display_mode > *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 intel_display > *display, > + struct intel_atomic_state > *state) > +{ > + struct intel_crtc *intel_crtc; > + struct intel_crtc_state *crtc_state; > + int i; > + > + if (DISPLAY_VER(display) < 20) > + return false; > + > + for_each_new_intel_crtc_in_state(state, intel_crtc, > crtc_state, i) { > + if (!intel_crtc->active) > + continue; > + > + if (crtc_state->vrr.enable) > + return false; > + } > + > + return true; > +} > + > +static bool wa_22019444797_psr1_check(const struct intel_crtc_state > *crtc_state, > + struct intel_psr *psr) > +{ > + struct intel_display *display = to_intel_display(crtc_state); > + > + return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured > && > + (psr->is_wa_delayed_vblank_limit || !psr- > >is_dc5_entry_possible) && > + !crtc_state->has_sel_update && !crtc_state- > >has_panel_replay; > +} > + > +/* > + * 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 intel_display *display, > + struct intel_atomic_state *state) > +{ > + struct intel_crtc *intel_crtc; > + struct intel_crtc_state *crtc_state; > + int i; > + > + if ((display->power.domains.target_dc_state & > + DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0) > + return false; > + > + for_each_new_intel_crtc_in_state(state, intel_crtc, > crtc_state, i) { > + struct drm_crtc *crtc = &intel_crtc->base; > + struct drm_vblank_crtc *vblank; > + struct intel_encoder *encoder; > + > + if (!intel_crtc->active) > + continue; > + > + vblank = drm_crtc_vblank_crtc(crtc); > + > + if (vblank->enabled) > + return false; > + > + if (!crtc_state->has_psr) > + return false; > + > + for_each_encoder_on_crtc(display->drm, crtc, encoder) > + if (encoder->type != INTEL_OUTPUT_EDP || > + !CAN_PSR(enc_to_intel_dp(encoder))) > + return false; > + } > + > + return true; > +} > + > static void hsw_activate_psr1(struct intel_dp *intel_dp) > { > struct intel_display *display = to_intel_display(intel_dp); > @@ -1007,7 +1091,15 @@ 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 > + * TODO: Disable idle frames when vblank gets enabled while > + * PSR2 is enabled > + */ > + if (DISPLAY_VER(dev_priv) != 20 || I think same Workaround is needed by PantherLake but only if stepping is A0. You could have this here: (DISPLAY_VER(display) != 20 && !IS_DISPLAY_VER_STEP(display, IP_VER(30, 0), STEP_A0, STEP_B0)) 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(display) < 14 && !IS_ALDERLAKE_P(dev_priv)) > val |= EDP_SU_TRACK_ENABLE; > @@ -2692,10 +2784,20 @@ void intel_psr_pre_plane_update(struct > intel_atomic_state *state, > const struct intel_crtc_state *new_crtc_state = > intel_atomic_get_new_crtc_state(state, crtc); > struct intel_encoder *encoder; > + bool dpkgc_configured = false, dc5_entry_possible = false; > + bool wa_delayed_vblank_limit = false; > > if (!HAS_PSR(display)) > return; > > + if (DISPLAY_VER(display) == 20) { > + dpkgc_configured = > intel_psr_is_dpkgc_configured(display, state); > + dc5_entry_possible = > + intel_psr_is_dc5_entry_possible(display, > state); > + wa_delayed_vblank_limit = > + intel_psr_check_wa_delayed_vblank(&new_crtc_s > tate->hw.adjusted_mode); > + } > + > for_each_intel_encoder_mask_with_psr(state->base.dev, > encoder, > old_crtc_state- > >uapi.encoder_mask) { > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > @@ -2704,6 +2806,12 @@ void intel_psr_pre_plane_update(struct > intel_atomic_state *state, > > mutex_lock(&psr->lock); > > + if (DISPLAY_VER(i915) == 20) { > + psr->is_dpkgc_configured = dpkgc_configured; > + psr->is_dc5_entry_possible = > dc5_entry_possible; > + psr->is_wa_delayed_vblank_limit = > wa_delayed_vblank_limit; > + } > + > /* > * Reasons to disable: > * - PSR disabled in new state > @@ -2711,6 +2819,7 @@ void intel_psr_pre_plane_update(struct > intel_atomic_state *state, > * - Changing between PSR versions > * - Region Early Transport changing > * - Display WA #1136: skl, bxt > + * - Display WA_22019444797 > */ > needs_to_disable |= > intel_crtc_needs_modeset(new_crtc_state); > needs_to_disable |= !new_crtc_state->has_psr; > @@ -2720,6 +2829,8 @@ void intel_psr_pre_plane_update(struct > intel_atomic_state *state, > psr->su_region_et_enabled; > needs_to_disable |= DISPLAY_VER(i915) < 11 && > new_crtc_state->wm_level_disabled; > + /* TODO: Disable PSR1 when vblank gets enabled while > PSR1 is enabled */ > + needs_to_disable |= > wa_22019444797_psr1_check(new_crtc_state, psr); > > if (psr->enabled && needs_to_disable) > intel_psr_disable_locked(intel_dp); > @@ -2760,6 +2871,12 @@ void intel_psr_post_plane_update(struct > intel_atomic_state *state, > keep_disabled |= DISPLAY_VER(display) < 11 && > crtc_state->wm_level_disabled; > > + /* > + * Wa_22019444797 > + * TODO: Disable PSR1 when vblank gets enabled while > PSR1 is enabled > + */ > + keep_disabled |= > wa_22019444797_psr1_check(crtc_state, psr); > + > if (!psr->enabled && !keep_disabled) > intel_psr_enable_locked(intel_dp, > crtc_state); > else if (psr->enabled && !crtc_state- > >wm_level_disabled)
> -----Original Message----- > From: Hogander, Jouni <jouni.hogander@intel.com> > Sent: Friday, September 20, 2024 5:16 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: Implement WA to help reach PC10 > > On Fri, 2024-09-20 at 14:42 +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. > > One comment below related to PantherLake. Otherwise patch looks ok to > me. > > > > > > --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 > > > > --v4 > > -use more appropriate name for check_vblank_limit() [Jouni] -Cover the > > case for idle frames when dpkgc is not configured [Jouni] -Check psr > > only for edp [Jouni] > > > > --v5 > > -move psr1 handling to plane update [Jouni] -add todo for cases when > > vblank is enabled when psr enabled [Jouni] -use intel_display instead > > of drm_i915_private > > > > --v6 > > -check target_dc_state [Jouni] > > -fix condition in pre/post plane update [Jouni] > > > > --v7 > > -fix has_psr condition [Uma] > > -fix typo in commit subject [Uma] > > -put psr1_wa check in its own helper [Uma] -fix the dc_entry check > > [Jouni] -use HAS_PSR() to cover two edp one with psr and one nonpsr > > [Jouni] > > > > WA: 22019444797 > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > .../drm/i915/display/intel_display_types.h | 3 + > > drivers/gpu/drm/i915/display/intel_psr.c | 119 > > +++++++++++++++++- > > 2 files changed, 121 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 3e694c1204db..2d790abee76e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1577,6 +1577,9 @@ struct intel_psr { > > #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE 0x40 > > > > u32 debug; > > + bool is_dpkgc_configured; > > + bool is_dc5_entry_possible; > > + bool is_wa_delayed_vblank_limit; > > bool sink_support; > > bool source_support; > > bool enabled; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index 5b355d0a3565..b882ff25fb92 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -26,6 +26,7 @@ > > #include <drm/drm_atomic_helper.h> > > #include <drm/drm_damage_helper.h> > > #include <drm/drm_debugfs.h> > > +#include <drm/drm_vblank.h> > > > > #include "i915_drv.h" > > #include "i915_reg.h" > > @@ -895,6 +896,89 @@ static u8 psr_compute_idle_frames(struct > intel_dp > > *intel_dp) > > return idle_frames; > > } > > > > +static bool > > +intel_psr_check_wa_delayed_vblank(const struct drm_display_mode > > *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 intel_display > > *display, > > + struct intel_atomic_state > > *state) > > +{ > > + struct intel_crtc *intel_crtc; > > + struct intel_crtc_state *crtc_state; > > + int i; > > + > > + if (DISPLAY_VER(display) < 20) > > + return false; > > + > > + for_each_new_intel_crtc_in_state(state, intel_crtc, > > crtc_state, i) { > > + if (!intel_crtc->active) > > + continue; > > + > > + if (crtc_state->vrr.enable) > > + return false; > > + } > > + > > + return true; > > +} > > + > > +static bool wa_22019444797_psr1_check(const struct intel_crtc_state > > *crtc_state, > > + struct intel_psr *psr) { > > + struct intel_display *display = to_intel_display(crtc_state); > > + > > + return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured > > && > > + (psr->is_wa_delayed_vblank_limit || !psr- > > >is_dc5_entry_possible) && > > + !crtc_state->has_sel_update && !crtc_state- > > >has_panel_replay; > > +} > > + > > +/* > > + * 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 intel_display *display, > > + struct intel_atomic_state *state) { > > + struct intel_crtc *intel_crtc; > > + struct intel_crtc_state *crtc_state; > > + int i; > > + > > + if ((display->power.domains.target_dc_state & > > + DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0) > > + return false; > > + > > + for_each_new_intel_crtc_in_state(state, intel_crtc, > > crtc_state, i) { > > + struct drm_crtc *crtc = &intel_crtc->base; > > + struct drm_vblank_crtc *vblank; > > + struct intel_encoder *encoder; > > + > > + if (!intel_crtc->active) > > + continue; > > + > > + vblank = drm_crtc_vblank_crtc(crtc); > > + > > + if (vblank->enabled) > > + return false; > > + > > + if (!crtc_state->has_psr) > > + return false; > > + > > + for_each_encoder_on_crtc(display->drm, crtc, encoder) > > + if (encoder->type != INTEL_OUTPUT_EDP || > > + !CAN_PSR(enc_to_intel_dp(encoder))) > > + return false; > > + } > > + > > + return true; > > +} > > + > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > { > > struct intel_display *display = to_intel_display(intel_dp); @@ > > -1007,7 +1091,15 @@ 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 > > + * TODO: Disable idle frames when vblank gets enabled while > > + * PSR2 is enabled > > + */ > > + if (DISPLAY_VER(dev_priv) != 20 || > > > I think same Workaround is needed by PantherLake but only if stepping is > A0. You could have this here: > > (DISPLAY_VER(display) != 20 && !IS_DISPLAY_VER_STEP(display, IP_VER(30, > 0), STEP_A0, STEP_B0)) > I do see the WA mentioned in bspec 74219 but the I don’t see any individual HSD cloned for it. Moreover it is for one stepping and a driver temp workaround not sure if I need to implement this @Shankar, Uma what do you suggest should I float a version implementing this or do this later on requirement 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(display) < 14 && !IS_ALDERLAKE_P(dev_priv)) > > val |= EDP_SU_TRACK_ENABLE; @@ -2692,10 +2784,20 @@ > > void intel_psr_pre_plane_update(struct intel_atomic_state *state, > > const struct intel_crtc_state *new_crtc_state = > > intel_atomic_get_new_crtc_state(state, crtc); > > struct intel_encoder *encoder; > > + bool dpkgc_configured = false, dc5_entry_possible = false; > > + bool wa_delayed_vblank_limit = false; > > > > if (!HAS_PSR(display)) > > return; > > > > + if (DISPLAY_VER(display) == 20) { > > + dpkgc_configured = > > intel_psr_is_dpkgc_configured(display, state); > > + dc5_entry_possible = > > + intel_psr_is_dc5_entry_possible(display, > > state); > > + wa_delayed_vblank_limit = > > + intel_psr_check_wa_delayed_vblank(&new_crtc_s > > tate->hw.adjusted_mode); > > + } > > + > > for_each_intel_encoder_mask_with_psr(state->base.dev, > > encoder, > > old_crtc_state- > > >uapi.encoder_mask) { > > struct intel_dp *intel_dp = enc_to_intel_dp(encoder); > > @@ -2704,6 +2806,12 @@ void intel_psr_pre_plane_update(struct > > intel_atomic_state *state, > > > > mutex_lock(&psr->lock); > > > > + if (DISPLAY_VER(i915) == 20) { > > + psr->is_dpkgc_configured = dpkgc_configured; > > + psr->is_dc5_entry_possible = > > dc5_entry_possible; > > + psr->is_wa_delayed_vblank_limit = > > wa_delayed_vblank_limit; > > + } > > + > > /* > > * Reasons to disable: > > * - PSR disabled in new state @@ -2711,6 +2819,7 @@ > > void intel_psr_pre_plane_update(struct intel_atomic_state *state, > > * - Changing between PSR versions > > * - Region Early Transport changing > > * - Display WA #1136: skl, bxt > > + * - Display WA_22019444797 > > */ > > needs_to_disable |= > > intel_crtc_needs_modeset(new_crtc_state); > > needs_to_disable |= !new_crtc_state->has_psr; @@ > > -2720,6 +2829,8 @@ void intel_psr_pre_plane_update(struct > > intel_atomic_state *state, > > psr->su_region_et_enabled; > > needs_to_disable |= DISPLAY_VER(i915) < 11 && > > new_crtc_state->wm_level_disabled; > > + /* TODO: Disable PSR1 when vblank gets enabled while > > PSR1 is enabled */ > > + needs_to_disable |= > > wa_22019444797_psr1_check(new_crtc_state, psr); > > > > if (psr->enabled && needs_to_disable) > > intel_psr_disable_locked(intel_dp); > > @@ -2760,6 +2871,12 @@ void intel_psr_post_plane_update(struct > > intel_atomic_state *state, > > keep_disabled |= DISPLAY_VER(display) < 11 && > > crtc_state->wm_level_disabled; > > > > + /* > > + * Wa_22019444797 > > + * TODO: Disable PSR1 when vblank gets enabled while > > PSR1 is enabled > > + */ > > + keep_disabled |= > > wa_22019444797_psr1_check(crtc_state, psr); > > + > > if (!psr->enabled && !keep_disabled) > > intel_psr_enable_locked(intel_dp, crtc_state); > > else if (psr->enabled && !crtc_state- > > >wm_level_disabled)
> > On Fri, 2024-09-20 at 14:42 +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. > > > > One comment below related to PantherLake. Otherwise patch looks ok to > > me. > > > --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 > > > > > > --v4 > > > -use more appropriate name for check_vblank_limit() [Jouni] -Cover > > > the case for idle frames when dpkgc is not configured [Jouni] -Check > > > psr only for edp [Jouni] > > > > > > --v5 > > > -move psr1 handling to plane update [Jouni] -add todo for cases when > > > vblank is enabled when psr enabled [Jouni] -use intel_display > > > instead of drm_i915_private > > > > > > --v6 > > > -check target_dc_state [Jouni] > > > -fix condition in pre/post plane update [Jouni] > > > > > > --v7 > > > -fix has_psr condition [Uma] > > > -fix typo in commit subject [Uma] > > > -put psr1_wa check in its own helper [Uma] -fix the dc_entry check > > > [Jouni] -use HAS_PSR() to cover two edp one with psr and one nonpsr > > > [Jouni] > > > > > > WA: 22019444797 > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > .../drm/i915/display/intel_display_types.h | 3 + > > > drivers/gpu/drm/i915/display/intel_psr.c | 119 > > > +++++++++++++++++- > > > 2 files changed, 121 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 3e694c1204db..2d790abee76e 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -1577,6 +1577,9 @@ struct intel_psr { > > > #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE 0x40 > > > > > > u32 debug; > > > + bool is_dpkgc_configured; > > > + bool is_dc5_entry_possible; > > > + bool is_wa_delayed_vblank_limit; > > > bool sink_support; > > > bool source_support; > > > bool enabled; > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > > b/drivers/gpu/drm/i915/display/intel_psr.c > > > index 5b355d0a3565..b882ff25fb92 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > > @@ -26,6 +26,7 @@ > > > #include <drm/drm_atomic_helper.h> > > > #include <drm/drm_damage_helper.h> > > > #include <drm/drm_debugfs.h> > > > +#include <drm/drm_vblank.h> > > > > > > #include "i915_drv.h" > > > #include "i915_reg.h" > > > @@ -895,6 +896,89 @@ static u8 psr_compute_idle_frames(struct > > intel_dp > > > *intel_dp) > > > return idle_frames; > > > } > > > > > > +static bool > > > +intel_psr_check_wa_delayed_vblank(const struct drm_display_mode > > > *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 intel_display > > > *display, > > > + struct intel_atomic_state > > > *state) > > > +{ > > > + struct intel_crtc *intel_crtc; > > > + struct intel_crtc_state *crtc_state; > > > + int i; > > > + > > > + if (DISPLAY_VER(display) < 20) > > > + return false; > > > + > > > + for_each_new_intel_crtc_in_state(state, intel_crtc, > > > crtc_state, i) { > > > + if (!intel_crtc->active) > > > + continue; > > > + > > > + if (crtc_state->vrr.enable) > > > + return false; > > > + } > > > + > > > + return true; > > > +} > > > + > > > +static bool wa_22019444797_psr1_check(const struct intel_crtc_state > > > *crtc_state, > > > + struct intel_psr *psr) { > > > + struct intel_display *display = > > > +to_intel_display(crtc_state); > > > + > > > + return DISPLAY_VER(display) == 20 && > > > +psr->is_dpkgc_configured > > > && > > > + (psr->is_wa_delayed_vblank_limit || !psr- > > > >is_dc5_entry_possible) && > > > + !crtc_state->has_sel_update && !crtc_state- > > > >has_panel_replay; > > > +} > > > + > > > +/* > > > + * 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 intel_display *display, > > > + struct intel_atomic_state *state) { > > > + struct intel_crtc *intel_crtc; > > > + struct intel_crtc_state *crtc_state; > > > + int i; > > > + > > > + if ((display->power.domains.target_dc_state & > > > + DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0) > > > + return false; > > > + > > > + for_each_new_intel_crtc_in_state(state, intel_crtc, > > > crtc_state, i) { > > > + struct drm_crtc *crtc = &intel_crtc->base; > > > + struct drm_vblank_crtc *vblank; > > > + struct intel_encoder *encoder; > > > + > > > + if (!intel_crtc->active) > > > + continue; > > > + > > > + vblank = drm_crtc_vblank_crtc(crtc); > > > + > > > + if (vblank->enabled) > > > + return false; > > > + > > > + if (!crtc_state->has_psr) > > > + return false; > > > + > > > + for_each_encoder_on_crtc(display->drm, crtc, > > > +encoder) > > > + if (encoder->type != INTEL_OUTPUT_EDP || > > > + !CAN_PSR(enc_to_intel_dp(encoder))) > > > + return false; > > > + } > > > + > > > + return true; > > > +} > > > + > > > static void hsw_activate_psr1(struct intel_dp *intel_dp) > > > { > > > struct intel_display *display = to_intel_display(intel_dp); > > > @@ > > > -1007,7 +1091,15 @@ 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 > > > + * TODO: Disable idle frames when vblank gets enabled while > > > + * PSR2 is enabled > > > + */ > > > + if (DISPLAY_VER(dev_priv) != 20 || > > > > > > I think same Workaround is needed by PantherLake but only if stepping > > is A0. You could have this here: > > > > (DISPLAY_VER(display) != 20 && !IS_DISPLAY_VER_STEP(display, > > IP_VER(30, 0), STEP_A0, STEP_B0)) > > > > I do see the WA mentioned in bspec 74219 but the I don’t see any individual HSD > cloned for it. > Moreover it is for one stepping and a driver temp workaround not sure if I need to > implement this @Shankar, Uma what do you suggest should I float a version > implementing this or do this later on requirement Yeah, its needed for PTL but gets fixed later in hardware. I would suggest we can limit this to LNL for now and don't add a stepping check in upstream. Overall patch looks good to me. Reviewed-by: Uma Shankar <uma.shankar@intel.com> Regards, Uma Shankar > 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(display) < 14 && !IS_ALDERLAKE_P(dev_priv)) > > > val |= EDP_SU_TRACK_ENABLE; @@ -2692,10 +2784,20 @@ > > > void intel_psr_pre_plane_update(struct intel_atomic_state *state, > > > const struct intel_crtc_state *new_crtc_state = > > > intel_atomic_get_new_crtc_state(state, crtc); > > > struct intel_encoder *encoder; > > > + bool dpkgc_configured = false, dc5_entry_possible = false; > > > + bool wa_delayed_vblank_limit = false; > > > > > > if (!HAS_PSR(display)) > > > return; > > > > > > + if (DISPLAY_VER(display) == 20) { > > > + dpkgc_configured = > > > intel_psr_is_dpkgc_configured(display, state); > > > + dc5_entry_possible = > > > + intel_psr_is_dc5_entry_possible(display, > > > state); > > > + wa_delayed_vblank_limit = > > > + > > > +intel_psr_check_wa_delayed_vblank(&new_crtc_s > > > tate->hw.adjusted_mode); > > > + } > > > + > > > for_each_intel_encoder_mask_with_psr(state->base.dev, > > > encoder, > > > old_crtc_state- > > > >uapi.encoder_mask) { > > > struct intel_dp *intel_dp = > > > enc_to_intel_dp(encoder); @@ -2704,6 +2806,12 @@ void > > > intel_psr_pre_plane_update(struct intel_atomic_state *state, > > > > > > mutex_lock(&psr->lock); > > > > > > + if (DISPLAY_VER(i915) == 20) { > > > + psr->is_dpkgc_configured = dpkgc_configured; > > > + psr->is_dc5_entry_possible = > > > dc5_entry_possible; > > > + psr->is_wa_delayed_vblank_limit = > > > wa_delayed_vblank_limit; > > > + } > > > + > > > /* > > > * Reasons to disable: > > > * - PSR disabled in new state @@ -2711,6 +2819,7 @@ > > > void intel_psr_pre_plane_update(struct intel_atomic_state *state, > > > * - Changing between PSR versions > > > * - Region Early Transport changing > > > * - Display WA #1136: skl, bxt > > > + * - Display WA_22019444797 > > > */ > > > needs_to_disable |= > > > intel_crtc_needs_modeset(new_crtc_state); > > > needs_to_disable |= !new_crtc_state->has_psr; @@ > > > -2720,6 +2829,8 @@ void intel_psr_pre_plane_update(struct > > > intel_atomic_state *state, > > > psr->su_region_et_enabled; > > > needs_to_disable |= DISPLAY_VER(i915) < 11 && > > > new_crtc_state->wm_level_disabled; > > > + /* TODO: Disable PSR1 when vblank gets enabled while > > > PSR1 is enabled */ > > > + needs_to_disable |= > > > wa_22019444797_psr1_check(new_crtc_state, psr); > > > > > > if (psr->enabled && needs_to_disable) > > > intel_psr_disable_locked(intel_dp); > > > @@ -2760,6 +2871,12 @@ void intel_psr_post_plane_update(struct > > > intel_atomic_state *state, > > > keep_disabled |= DISPLAY_VER(display) < 11 && > > > crtc_state->wm_level_disabled; > > > > > > + /* > > > + * Wa_22019444797 > > > + * TODO: Disable PSR1 when vblank gets enabled while > > > PSR1 is enabled > > > + */ > > > + keep_disabled |= > > > wa_22019444797_psr1_check(crtc_state, psr); > > > + > > > if (!psr->enabled && !keep_disabled) > > > intel_psr_enable_locked(intel_dp, > > > crtc_state); > > > else if (psr->enabled && !crtc_state- > > > >wm_level_disabled)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 3e694c1204db..2d790abee76e 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -1577,6 +1577,9 @@ struct intel_psr { #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE 0x40 u32 debug; + bool is_dpkgc_configured; + bool is_dc5_entry_possible; + bool is_wa_delayed_vblank_limit; bool sink_support; bool source_support; bool enabled; diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c index 5b355d0a3565..b882ff25fb92 100644 --- a/drivers/gpu/drm/i915/display/intel_psr.c +++ b/drivers/gpu/drm/i915/display/intel_psr.c @@ -26,6 +26,7 @@ #include <drm/drm_atomic_helper.h> #include <drm/drm_damage_helper.h> #include <drm/drm_debugfs.h> +#include <drm/drm_vblank.h> #include "i915_drv.h" #include "i915_reg.h" @@ -895,6 +896,89 @@ static u8 psr_compute_idle_frames(struct intel_dp *intel_dp) return idle_frames; } +static bool +intel_psr_check_wa_delayed_vblank(const struct drm_display_mode *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 intel_display *display, + struct intel_atomic_state *state) +{ + struct intel_crtc *intel_crtc; + struct intel_crtc_state *crtc_state; + int i; + + if (DISPLAY_VER(display) < 20) + return false; + + for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) { + if (!intel_crtc->active) + continue; + + if (crtc_state->vrr.enable) + return false; + } + + return true; +} + +static bool wa_22019444797_psr1_check(const struct intel_crtc_state *crtc_state, + struct intel_psr *psr) +{ + struct intel_display *display = to_intel_display(crtc_state); + + return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured && + (psr->is_wa_delayed_vblank_limit || !psr->is_dc5_entry_possible) && + !crtc_state->has_sel_update && !crtc_state->has_panel_replay; +} + +/* + * 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 intel_display *display, + struct intel_atomic_state *state) +{ + struct intel_crtc *intel_crtc; + struct intel_crtc_state *crtc_state; + int i; + + if ((display->power.domains.target_dc_state & + DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0) + return false; + + for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) { + struct drm_crtc *crtc = &intel_crtc->base; + struct drm_vblank_crtc *vblank; + struct intel_encoder *encoder; + + if (!intel_crtc->active) + continue; + + vblank = drm_crtc_vblank_crtc(crtc); + + if (vblank->enabled) + return false; + + if (!crtc_state->has_psr) + return false; + + for_each_encoder_on_crtc(display->drm, crtc, encoder) + if (encoder->type != INTEL_OUTPUT_EDP || + !CAN_PSR(enc_to_intel_dp(encoder))) + return false; + } + + return true; +} + static void hsw_activate_psr1(struct intel_dp *intel_dp) { struct intel_display *display = to_intel_display(intel_dp); @@ -1007,7 +1091,15 @@ 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 + * TODO: Disable idle frames when vblank gets enabled while + * PSR2 is enabled + */ + 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(display) < 14 && !IS_ALDERLAKE_P(dev_priv)) val |= EDP_SU_TRACK_ENABLE; @@ -2692,10 +2784,20 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, const struct intel_crtc_state *new_crtc_state = intel_atomic_get_new_crtc_state(state, crtc); struct intel_encoder *encoder; + bool dpkgc_configured = false, dc5_entry_possible = false; + bool wa_delayed_vblank_limit = false; if (!HAS_PSR(display)) return; + if (DISPLAY_VER(display) == 20) { + dpkgc_configured = intel_psr_is_dpkgc_configured(display, state); + dc5_entry_possible = + intel_psr_is_dc5_entry_possible(display, state); + wa_delayed_vblank_limit = + intel_psr_check_wa_delayed_vblank(&new_crtc_state->hw.adjusted_mode); + } + for_each_intel_encoder_mask_with_psr(state->base.dev, encoder, old_crtc_state->uapi.encoder_mask) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -2704,6 +2806,12 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, mutex_lock(&psr->lock); + if (DISPLAY_VER(i915) == 20) { + psr->is_dpkgc_configured = dpkgc_configured; + psr->is_dc5_entry_possible = dc5_entry_possible; + psr->is_wa_delayed_vblank_limit = wa_delayed_vblank_limit; + } + /* * Reasons to disable: * - PSR disabled in new state @@ -2711,6 +2819,7 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, * - Changing between PSR versions * - Region Early Transport changing * - Display WA #1136: skl, bxt + * - Display WA_22019444797 */ needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state); needs_to_disable |= !new_crtc_state->has_psr; @@ -2720,6 +2829,8 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state, psr->su_region_et_enabled; needs_to_disable |= DISPLAY_VER(i915) < 11 && new_crtc_state->wm_level_disabled; + /* TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled */ + needs_to_disable |= wa_22019444797_psr1_check(new_crtc_state, psr); if (psr->enabled && needs_to_disable) intel_psr_disable_locked(intel_dp); @@ -2760,6 +2871,12 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state, keep_disabled |= DISPLAY_VER(display) < 11 && crtc_state->wm_level_disabled; + /* + * Wa_22019444797 + * TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled + */ + keep_disabled |= wa_22019444797_psr1_check(crtc_state, psr); + if (!psr->enabled && !keep_disabled) intel_psr_enable_locked(intel_dp, crtc_state); else if (psr->enabled && !crtc_state->wm_level_disabled)
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 --v4 -use more appropriate name for check_vblank_limit() [Jouni] -Cover the case for idle frames when dpkgc is not configured [Jouni] -Check psr only for edp [Jouni] --v5 -move psr1 handling to plane update [Jouni] -add todo for cases when vblank is enabled when psr enabled [Jouni] -use intel_display instead of drm_i915_private --v6 -check target_dc_state [Jouni] -fix condition in pre/post plane update [Jouni] --v7 -fix has_psr condition [Uma] -fix typo in commit subject [Uma] -put psr1_wa check in its own helper [Uma] -fix the dc_entry check [Jouni] -use HAS_PSR() to cover two edp one with psr and one nonpsr [Jouni] WA: 22019444797 Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- .../drm/i915/display/intel_display_types.h | 3 + drivers/gpu/drm/i915/display/intel_psr.c | 119 +++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-)