> Subject: [PATCH v2 04/10] drm/i915/alpm: Refactor Auxless wake time
> calculation
> 
> Divide the auxless wake time calculation in parts which will help later to add
> Xe3p related modification.
> 
> v1: Initial version.

No need for this you can start off with v2 directly
Same for all patches where this has occured

> v2: Refactor first existing calculation. [Jani]
> 

Add Bspec link

> Cc: Jouni Högander <[email protected]>
> Signed-off-by: Animesh Manna <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c | 37 ++++++++++++++++-------
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 779718d0c8dd..8d07455a62c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -85,6 +85,26 @@ static int get_lfps_half_cycle_clocks(const struct
> intel_crtc_state *crtc_state)
>               1000 / (2 * LFPS_CYCLE_COUNT);
>  }
> 
> +static int get_tphy2_p2_to_p0(struct intel_dp *intel_dp) {
> +     return 12 * 1000;
> +}
> +
> +static int get_establishment_period(struct intel_dp *intel_dp,
> +                                 const struct intel_crtc_state *crtc_state) {
> +     int port_clock = crtc_state->port_clock;
> +     int t1 = 50 * 1000;
> +     int tps4 = (252 * 10);

Where did this * 10 come from?

> +     long tml_phy_lock = 1000 * 1000 * tps4 / port_clock / 10;

Why the extra /10 required here also if you had not multiplied tps4 with 10 
then this wouldn't be required
You also removed the comment telling us portclock need to be in 10Kb/s

> +     int tcds, establishment_period;
> +
> +     tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) * tml_phy_lock;
> +     establishment_period = (SILENCE_PERIOD_TIME + t1 + tcds);
> +
> +     return establishment_period;
> +}
> +
>  /*
>   * AUX-Less Wake Time = CEILING( ((PHY P2 to P0) + tLFPS_Period, Max+
>   * tSilence, Max+ tPHY Establishment + tCDS) / tline) @@ -104,19 +124,14
> @@ static int get_lfps_half_cycle_clocks(const struct intel_crtc_state
> *crtc_state)
>   * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
>   * TPS4 Length = 252 Symbols
>   */
> -static int _lnl_compute_aux_less_wake_time(const struct intel_crtc_state
> *crtc_state)
> +static int _lnl_compute_aux_less_wake_time(struct intel_dp *intel_dp,
> +                                        const struct intel_crtc_state

I don’t see any justified reason to send intel_dp here

Regards,
Suraj Kandpal

> *crtc_state)
>  {
> -     int tphy2_p2_to_p0 = 12 * 1000;
> -     int t1 = 50 * 1000;
> -     int tps4 = 252;
> -     /* port_clock is link rate in 10kbit/s units */
> -     int tml_phy_lock = 1000 * 1000 * tps4 / crtc_state->port_clock;
> -     int num_ml_phy_lock = 7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1;
> -     int t2 = num_ml_phy_lock * tml_phy_lock;
> -     int tcds = 1 * t2;
> +     int tphy2_p2_to_p0 = get_tphy2_p2_to_p0(intel_dp);
> +     int establishment_period = get_establishment_period(intel_dp,
> +crtc_state);
> 
>       return DIV_ROUND_UP(tphy2_p2_to_p0 +
> get_lfps_cycle_time(crtc_state) +
> -                         SILENCE_PERIOD_TIME + t1 + tcds, 1000);
> +                         establishment_period, 1000);
>  }
> 
>  static int
> @@ -128,7 +143,7 @@ _lnl_compute_aux_less_alpm_params(struct intel_dp
> *intel_dp,
>               lfps_half_cycle;
> 
>       aux_less_wake_time =
> -             _lnl_compute_aux_less_wake_time(crtc_state);
> +             _lnl_compute_aux_less_wake_time(intel_dp, crtc_state);
>       aux_less_wake_lines = intel_usecs_to_scanlines(&crtc_state-
> >hw.adjusted_mode,
>                                                      aux_less_wake_time);
>       silence_period = get_silence_period_symbols(crtc_state);
> --
> 2.29.0

Reply via email to