Re: [Intel-gfx] [PATCH] drm/i915/psr: Use calculated io and fast wake lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> > 





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux