On Wed, Mar 06, 2024 at 10:34:25AM +0200, Jouni Högander wrote:
> Bspec mentions 50 us for IO wake time and 32 us for fast wake time. 32 us
> is most probably wrong as it doesn't meet the specification as fast wake
> time is calculated in Bspec like this:
> 
> 10..16 us (precharge) + 8 us (preamble) + 4 us (phy_wake) + 20 us
> (tfw_exit_latency)
> 
> Instead of using these constants calculate IO wake and fast wake for
> DISPLAY_VER < 12 as well.
> 
> v2:
>   - initialize io/fast_wake_time for display version < 9
> 
> Signed-off-by: Jouni Högander <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 34 ++++++++++++++++++------
>  1 file changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c 
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 905208e1c771..e1df0ece9fa3 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1150,9 +1150,25 @@ static bool _lnl_compute_alpm_params(struct intel_dp 
> *intel_dp,
>       return true;
>  }
>  
> -static int get_io_buffer_wake_time(void)
> +/*
> + * From Bspec:
> + *
> + * For DISPLAY_VER >= 12
> + * 10 us
> + *
> + * For DISPLAY_VER < 12
> + * This is not directly mentioned in Bspec. There are 50 us io wake time and 
> 32
> + * us fast wake time. Clearly preharge pulses are not (improperly) included 
> in
> + * 32 us fast wake time. 50 us - 32 us = 18 us.
> + */
> +static int get_io_buffer_wake_time(const struct intel_crtc_state *crtc_state)
>  {
> -     return 10;
> +     struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> +     if (DISPLAY_VER(i915) < 12)
> +             return 18;
> +     else
> +             return 10;
>  }
>  
>  static bool _compute_alpm_params(struct intel_dp *intel_dp,
> @@ -1162,8 +1178,8 @@ static bool _compute_alpm_params(struct intel_dp 
> *intel_dp,
>       int io_wake_lines, io_wake_time, fast_wake_lines, fast_wake_time;
>       u8 max_wake_lines;
>  
> -     if (DISPLAY_VER(i915) >= 12) {
> -             int io_buffer_wake_time = get_io_buffer_wake_time();
> +     if (DISPLAY_VER(i915) >= 9) {
> +             int io_buffer_wake_time = get_io_buffer_wake_time(crtc_state);
>               int tfw_exit_latency = 20; /* eDP spec */
>               int phy_wake = 4;          /* eDP spec */
>               int preamble = 8;          /* eDP spec */
> @@ -1173,15 +1189,17 @@ static bool _compute_alpm_params(struct intel_dp 
> *intel_dp,
>                       phy_wake + tfw_exit_latency;
>               fast_wake_time = precharge + preamble + phy_wake +
>                       tfw_exit_latency;
> -
> -             /* TODO: Check how we can use ALPM_CTL fast wake extended field 
> */
> -             max_wake_lines = 12;
>       } else {
>               io_wake_time = 50;
>               fast_wake_time = 32;
> -             max_wake_lines = 8;

Isn't the whole else branch dead code now?

>       }
>  
> +     if (DISPLAY_VER(i915) >= 12)
> +             /* TODO: Check how we can use ALPM_CTL fast wake extended field 
> */
> +             max_wake_lines = 12;
> +     else
> +             max_wake_lines = 8;
> +
>       io_wake_lines = intel_usecs_to_scanlines(
>               &crtc_state->hw.adjusted_mode, io_wake_time);
>       fast_wake_lines = intel_usecs_to_scanlines(
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

Reply via email to