Re: [PATCH v5 3/5] drm/i915/psr: Calculate IO wake and fast wake lines for DISPLAY_VER < 12

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Mar 08, 2024 at 01:00:37PM +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.
> 
> v3:
>   - do not handle < 9 separately
>   - add own helper for skl and tgl io buffer wake times
> v2:
>   - initialize io/fast_wake_time for display version < 9
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 46 ++++++++++++++++--------
>  1 file changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 7736bdcad82d..747761efa4be 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1150,36 +1150,52 @@ static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp,
>  	return true;
>  }
>  
> -static int io_buffer_wake_time(void)
> +/*
> + * IO wake time for DISPLAY_VER < 12 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 skl_io_buffer_wake_time(void)
> +{
> +	return 18;
> +}
> +
> +static int tgl_io_buffer_wake_time(void)
>  {
>  	return 10;
>  }
>  
> +static int io_buffer_wake_time(const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> +	if (DISPLAY_VER(i915) >= 12)
> +		return tgl_io_buffer_wake_time();
> +	else
> +		return skl_io_buffer_wake_time();
> +}
> +
>  static bool _compute_alpm_params(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;
> +	int tfw_exit_latency = 20; /* eDP spec */
> +	int phy_wake = 4;	   /* eDP spec */
> +	int preamble = 8;	   /* eDP spec */
> +	int precharge = intel_dp_aux_fw_sync_len() - preamble;
>  	u8 max_wake_lines;
>  
> -	if (DISPLAY_VER(i915) >= 12) {
> -		int tfw_exit_latency = 20; /* eDP spec */
> -		int phy_wake = 4;	   /* eDP spec */
> -		int preamble = 8;	   /* eDP spec */
> -		int precharge = intel_dp_aux_fw_sync_len() - preamble;
> -
> -		io_wake_time = max(precharge, io_buffer_wake_time()) + preamble +
> -			phy_wake + tfw_exit_latency;
> -		fast_wake_time = precharge + preamble + phy_wake +
> -			tfw_exit_latency;
> +	io_wake_time = max(precharge, io_buffer_wake_time(crtc_state)) +
> +		preamble + phy_wake + tfw_exit_latency;
> +	fast_wake_time = precharge + preamble + phy_wake +
> +		tfw_exit_latency;
>  
> +	if (DISPLAY_VER(i915) >= 12)
>  		/* 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;
> +	else
>  		max_wake_lines = 8;
> -	}
>  
>  	io_wake_lines = intel_usecs_to_scanlines(
>  		&crtc_state->hw.adjusted_mode, io_wake_time);
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel