On Fri, 2023-02-24 at 11:32 +0200, Lisovskiy, Stanislav wrote:
> On Tue, Feb 21, 2023 at 10:53:04AM +0200, Jouni Högander wrote:
> > Currently we are using hardcoded 7 for io and fast wake lines.
> >
> > According to Bspec io and fast wake times are both 42us for
> > DISPLAY_VER >= 12 and 50us and 32us for older platforms.
> >
> > Calculate line counts for these and configure them into PSR2_CTL
> > accordingly
> >
> > Use 45 us for the fast wake calculation as 42 seems to be too
> > tight based on testing.
> >
> > Bspec: 49274, 4289
>
> Checked, can't see any obvious flaws or issue, looks good to me.
Thank you for the review. Adding fixes tag:
Fixes: 64cf40a125ff ("drm/i915/psr: Program default IO buffer Wake and
Fast Wake")
>
> Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
>
> >
> > Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
> > Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7725
> > Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx>
> > ---
> > .../drm/i915/display/intel_display_types.h | 2 +
> > drivers/gpu/drm/i915/display/intel_psr.c | 78
> > +++++++++++++++----
> > 2 files changed, 63 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 9ccae7a46020..fc15f4e5d49f 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1634,6 +1634,8 @@ struct intel_psr {
> > bool psr2_sel_fetch_cff_enabled;
> > bool req_psr2_sdp_prior_scanline;
> > u8 sink_sync_latency;
> > + u8 io_wake_lines;
> > + u8 fast_wake_lines;
> > ktime_t last_entry_attempt;
> > ktime_t last_exit;
> > bool sink_not_reliable;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 2954759e9d12..059e220144ce 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -542,6 +542,14 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > val |= EDP_PSR2_FRAME_BEFORE_SU(max_t(u8, intel_dp-
> > >psr.sink_sync_latency + 1, 2));
> > val |= intel_psr2_get_tp_time(intel_dp);
> >
> > + if (DISPLAY_VER(dev_priv) >= 12) {
> > + if (intel_dp->psr.io_wake_lines < 9 &&
> > + intel_dp->psr.fast_wake_lines < 9)
> > + val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> > + else
> > + val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_3;
> > + }
> > +
> > /* Wa_22012278275:adl-p */
> > if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_E0)) {
> > static const u8 map[] = {
> > @@ -558,31 +566,21 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > * Still using the default IO_BUFFER_WAKE and
> > FAST_WAKE, see
> > * comments bellow for more information
> > */
> > - u32 tmp, lines = 7;
> > -
> > - val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> > + u32 tmp;
> >
> > - tmp = map[lines -
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > + tmp = map[intel_dp->psr.io_wake_lines -
> > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES];
> > tmp = tmp << TGL_EDP_PSR2_IO_BUFFER_WAKE_SHIFT;
> > val |= tmp;
> >
> > - tmp = map[lines -
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > + tmp = map[intel_dp->psr.fast_wake_lines -
> > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES];
> > tmp = tmp << TGL_EDP_PSR2_FAST_WAKE_MIN_SHIFT;
> > val |= tmp;
> > } else if (DISPLAY_VER(dev_priv) >= 12) {
> > - /*
> > - * TODO: 7 lines of IO_BUFFER_WAKE and FAST_WAKE
> > are default
> > - * values from BSpec. In order to setting an
> > optimal power
> > - * consumption, lower than 4k resolution mode needs
> > to decrease
> > - * IO_BUFFER_WAKE and FAST_WAKE. And higher than 4K
> > resolution
> > - * mode needs to increase IO_BUFFER_WAKE and
> > FAST_WAKE.
> > - */
> > - val |= TGL_EDP_PSR2_BLOCK_COUNT_NUM_2;
> > - val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(7);
> > - val |= TGL_EDP_PSR2_FAST_WAKE(7);
> > + val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > >psr.io_wake_lines);
> > + val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp-
> > >psr.fast_wake_lines);
> > } else if (DISPLAY_VER(dev_priv) >= 9) {
> > - val |= EDP_PSR2_IO_BUFFER_WAKE(7);
> > - val |= EDP_PSR2_FAST_WAKE(7);
> > + val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp-
> > >psr.io_wake_lines);
> > + val |= EDP_PSR2_FAST_WAKE(intel_dp-
> > >psr.fast_wake_lines);
> > }
> >
> > if (intel_dp->psr.req_psr2_sdp_prior_scanline)
> > @@ -829,6 +827,46 @@ static bool
> > _compute_psr2_sdp_prior_scanline_indication(struct intel_dp
> > *intel_d
> > return true;
> > }
> >
> > +static bool _compute_psr2_wake_times(struct intel_dp *intel_dp,
> > + struct intel_crtc_state
> > *crtc_state)
> > +{
> > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > + int io_wake_lines, io_wake_time, fast_wake_lines,
> > fast_wake_time;
> > + u8 max_wake_lines;
> > +
> > + if (DISPLAY_VER(i915) >= 12) {
> > + io_wake_time = 42;
> > + /*
> > + * According to Bspec it's 42us, but based on
> > testing
> > + * it is not enough -> use 45 us.
> > + */
> > + fast_wake_time = 45;
> > + max_wake_lines = 12;
> > + } else {
> > + io_wake_time = 50;
> > + fast_wake_time = 32;
> > + max_wake_lines = 8;
> > + }
> > +
> > + io_wake_lines = intel_usecs_to_scanlines(
> > + &crtc_state->uapi.adjusted_mode, io_wake_time);
> > + fast_wake_lines = intel_usecs_to_scanlines(
> > + &crtc_state->uapi.adjusted_mode, fast_wake_time);
> > +
> > + if (io_wake_lines > max_wake_lines ||
> > + fast_wake_lines > max_wake_lines)
> > + return false;
> > +
> > + if (i915->params.psr_safest_params)
> > + io_wake_lines = fast_wake_lines = max_wake_lines;
> > +
> > + /* According to Bspec lower limit should be set as 7 lines.
> > */
> > + intel_dp->psr.io_wake_lines = max(io_wake_lines, 7);
> > + intel_dp->psr.fast_wake_lines = max(fast_wake_lines, 7);
> > +
> > + return true;
> > +}
> > +
> > static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
> > struct intel_crtc_state
> > *crtc_state)
> > {
> > @@ -923,6 +961,12 @@ static bool intel_psr2_config_valid(struct
> > intel_dp *intel_dp,
> > return false;
> > }
> >
> > + if (!_compute_psr2_wake_times(intel_dp, crtc_state)) {
> > + drm_dbg_kms(&dev_priv->drm,
> > + "PSR2 not enabled, Unable to use long
> > enough wake times\n");
> > + return false;
> > + }
> > +
> > if (HAS_PSR2_SEL_FETCH(dev_priv)) {
> > if (!intel_psr2_sel_fetch_config_valid(intel_dp,
> > crtc_state) &&
> > !HAS_PSR_HW_TRACKING(dev_priv)) {
> > --
> > 2.34.1
> >
