On Tue, 2025-08-19 at 04:48 +0000, Shankar, Uma wrote:
>
>
> > -----Original Message-----
> > From: Intel-xe <intel-xe-boun...@lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Monday, May 19, 2025 1:22 PM
> > To: intel-gfx@lists.freedesktop.org; intel...@lists.freedesktop.org
> > Cc: ville.syrj...@linux.intel.com; Hogander, Jouni
> > <jouni.hogan...@intel.com>
> > Subject: [PATCH] drm/i915/psr: Underrun on idle PSR wa only when
> > pkgc latency
> > > delayed vblank
> >
> > Underrun on idle PSR workaround (Wa_16025596647) is supposed to be
> > applied
> > only when pkg c latency > delayed vblank. Currently we are applying
> > it always
> > when other criterias are met.
> >
> > Fix this by adding new boolean flag which is supposed to be set
> > when calculating
> > watermark levels and pkgc latency > delayed vblank is detected.
> > currently this
> > scenario is blocked but might be added later. Due to this add also
> > TODO comment
> > into skl_max_wm_level_for_vblank.
> >
> > Bspec: 74151
> > Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> > ---
> > .../gpu/drm/i915/display/intel_display_types.h | 2 ++
> > drivers/gpu/drm/i915/display/intel_psr.c | 16 ++++++++++--
> > ----
> > drivers/gpu/drm/i915/display/skl_watermark.c | 5 +++++
> > 3 files changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 056219272c36..209ead520660 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1116,6 +1116,7 @@ struct intel_crtc_state {
> > bool req_psr2_sdp_prior_scanline;
> > bool has_panel_replay;
> > bool wm_level_disabled;
> > + bool pkg_c_latency_used;
> > u32 dc3co_exitline;
> > u16 su_y_granularity;
> > u8 active_non_psr_pipes;
> > @@ -1655,6 +1656,7 @@ struct intel_psr {
> > u8 entry_setup_frames;
> >
> > bool link_ok;
> > + bool pkg_c_latency_used;
> >
> > u8 active_non_psr_pipes;
> > };
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 430ad4ef7146..a75ef515d016 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -931,7 +931,7 @@ static void hsw_activate_psr1(struct intel_dp
> > *intel_dp)
> > /* Wa_16025596647 */
> > if ((DISPLAY_VER(display) == 20 ||
> > IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0,
> > STEP_B0))
> > &&
> > - is_dc5_dc6_blocked(intel_dp))
> > + is_dc5_dc6_blocked(intel_dp) && intel_dp-
> > >psr.pkg_c_latency_used)
> > intel_dmc_start_pkgc_exit_at_start_of_undelayed_vb
> > lank(display,
> >
> > intel_dp-
> > > psr.pipe,
> >
> > true);
> > @@ -1021,7 +1021,7 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > /* Wa_16025596647 */
> > if ((DISPLAY_VER(display) == 20 ||
> > IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0,
> > STEP_B0))
> > &&
> > - is_dc5_dc6_blocked(intel_dp))
> > + is_dc5_dc6_blocked(intel_dp) && intel_dp-
> > >psr.pkg_c_latency_used)
> > idle_frames = 0;
> > else
> > idle_frames = psr_compute_idle_frames(intel_dp);
> > @@ -2022,6 +2022,7 @@ static void intel_psr_enable_locked(struct
> > intel_dp
> > *intel_dp,
> > intel_dp->psr.req_psr2_sdp_prior_scanline =
> > crtc_state->req_psr2_sdp_prior_scanline;
> > intel_dp->psr.active_non_psr_pipes = crtc_state-
> > >active_non_psr_pipes;
> > + intel_dp->psr.pkg_c_latency_used = crtc_state-
> > >pkg_c_latency_used;
> >
> > if (!psr_interrupt_error_check(intel_dp))
> > return;
> > @@ -2202,6 +2203,7 @@ static void intel_psr_disable_locked(struct
> > intel_dp
> > *intel_dp)
> > intel_dp->psr.su_region_et_enabled = false;
> > intel_dp->psr.psr2_sel_fetch_cff_enabled = false;
> > intel_dp->psr.active_non_psr_pipes = 0;
> > + intel_dp->psr.pkg_c_latency_used = 0;
> > }
> >
> > /**
> > @@ -3696,7 +3698,7 @@ static void
> > intel_psr_apply_underrun_on_idle_wa_locked(struct intel_dp
> > *intel_dp
> > struct intel_display *display =
> > to_intel_display(intel_dp);
> > bool dc5_dc6_blocked;
> >
> > - if (!intel_dp->psr.active)
> > + if (!intel_dp->psr.active || !intel_dp-
> > >psr.pkg_c_latency_used)
> > return;
> >
> > dc5_dc6_blocked = is_dc5_dc6_blocked(intel_dp); @@ -3721,7
> > +3723,8
> > @@ static void psr_dc5_dc6_wa_work(struct work_struct *work)
> >
> > mutex_lock(&intel_dp->psr.lock);
> >
> > - if (intel_dp->psr.enabled && !intel_dp-
> > >psr.panel_replay_enabled)
> > + if (intel_dp->psr.enabled && !intel_dp-
> > >psr.panel_replay_enabled
> > &&
> > + !intel_dp->psr.pkg_c_latency_used)
> > intel_psr_apply_underrun_on_idle_wa_locked
> > (intel_dp);
> >
> > mutex_unlock(&intel_dp->psr.lock);
> > @@ -3799,7 +3802,8 @@ void intel_psr_notify_pipe_change(struct
> > intel_atomic_state *state,
> > goto unlock;
> >
> > if ((enable && intel_dp->psr.active_non_psr_pipes)
> > ||
> > - (!enable && !intel_dp-
> > >psr.active_non_psr_pipes)) {
> > + (!enable && !intel_dp-
> > >psr.active_non_psr_pipes) ||
> > + !intel_dp->psr.pkg_c_latency_used) {
> > intel_dp->psr.active_non_psr_pipes =
> > active_non_psr_pipes;
> > goto unlock;
> > }
> > @@ -3834,7 +3838,7 @@ void
> > intel_psr_notify_vblank_enable_disable(struct
> > intel_display *display,
> > break;
> > }
> >
> > - if (intel_dp->psr.enabled)
> > + if (intel_dp->psr.enabled && intel_dp-
> > >psr.pkg_c_latency_used)
> > intel_psr_apply_underrun_on_idle_wa_locked
> > (intel_dp);
> >
> > mutex_unlock(&intel_dp->psr.lock);
> > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> > b/drivers/gpu/drm/i915/display/skl_watermark.c
> > index 8080f777910a..ccde151fa9fd 100644
> > --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> > @@ -2333,6 +2333,11 @@ static int skl_max_wm0_lines(const struct
> > intel_crtc_state *crtc_state)
> > return wm0_lines;
> > }
> >
> > +/*
> > + * TODO: In case we use PKG_C_LATENCY to allow C-states when the
> > +delayed vblank
> > + * size is too small for the package C exit latency we need to
> > notify
> > +PSR about
> > + * the scenario to apply Wa_16025596647.
> > + */
>
> Looks Good to me, we can enable it once the package C latency is
> programmed to
> a non FF value.
>
> Would suggest to keep a ticket to track this.
>
> Reviewed-by: Uma Shankar <uma.shan...@intel.com>
Thank you Uma for the review. This is now pushed to drm-intel-next.
BR,
Jouni Högander
>
> > static int skl_max_wm_level_for_vblank(struct intel_crtc_state
> > *crtc_state,
> > int wm0_lines)
> > {
> > --
> > 2.43.0
>
