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.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovs...@intel.com>

> 
> Cc: Mika Kahola <mika.kah...@intel.com>
> Cc: José Roberto de Souza <jose.so...@intel.com>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7725
> Signed-off-by: Jouni Högander <jouni.hogan...@intel.com>
> ---
>  .../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
> 

Reply via email to