> -----Original Message-----
> From: Intel-xe <[email protected]> On Behalf Of
> Kandpal, Suraj
> Sent: Monday, November 17, 2025 2:50 PM
> To: Manna, Animesh <[email protected]>; intel-
> [email protected]; [email protected]; dri-
> [email protected]
> Cc: Nikula, Jani <[email protected]>; Hogander, Jouni
> <[email protected]>
> Subject: RE: [PATCH v4 05/10] drm/i915/alpm: Auxless wake time calculation
> for Xe3p
>
> > Subject: [PATCH v4 05/10] drm/i915/alpm: Auxless wake time calculation
> > for Xe3p
> >
> > Add support for auxless waketime calculation for DP2.1 ALPM as
> > dependent parameter got changed.
> >
> > v1: Initial version.
> > v2: Use intel_dp_is_uhbr(). [Jani]
> >
>
> Add Bspec no. here
>
> > Cc: Jouni Högander <[email protected]>
> > Signed-off-by: Animesh Manna <[email protected]>
> > ---
> > drivers/gpu/drm/i915/display/intel_alpm.c | 63
> > +++++++++++++++++++----
> > 1 file changed, 53 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index b4b874dd3725..81472254ab73 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -100,23 +100,64 @@ static int get_lfps_half_cycle_clocks(const
> > struct intel_crtc_state *crtc_state)
> >
> > static int get_tphy2_p2_to_p0(const struct intel_crtc_state *crtc_state) {
> > - return 12 * 1000;
> > + struct intel_display *display = to_intel_display(crtc_state);
> > +
> > + return DISPLAY_VER(display) >= 35 ? (40 * 1000) : (12 * 1000);
> > }
> >
> > -static int get_establishment_period(const struct intel_crtc_state
> > *crtc_state)
> > +static int get_establishment_period(struct intel_dp *intel_dp,
> > + const struct intel_crtc_state *crtc_state)
> > {
> > int t1 = 50 * 1000;
> > - int tps4 = 252;
> > + int tps4 = intel_dp_is_uhbr(crtc_state) ? (396 * 32) : (252 * 10);
Also I see 396 is used at more places than 1 so maybe we can have a define for
it
ML_PHY_LOCK_LEN
> > /* port_clock is link rate in 10kbit/s units */
> > - int tml_phy_lock = 1000 * 1000 * tps4 / crtc_state->port_clock;
> > + int tml_phy_lock = 1000 * 1000 * tps4 / crtc_state->port_clock / 10;
> > + int lttpr_count = 0;
> > int tcds, establishment_period;
> >
> > - tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) * tml_phy_lock;
> > - establishment_period = (SILENCE_PERIOD_TIME + t1 + tcds);
> > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) {
> > + tcds = (7 + DIV_ROUND_UP(6500, tml_phy_lock) + 1) *
> > tml_phy_lock;
> > + } else {
> > + tcds = 7 * tml_phy_lock;
> > + lttpr_count = drm_dp_lttpr_count(intel_dp-
> > >lttpr_common_caps);
> > + }
> > +
> > + if (lttpr_count) {
> > + int tlw = 13000;
> > + int tcs = 10000;
> > + int tlfps_period = get_lfps_cycle_time(crtc_state);
> > + int tdcs = (SILENCE_PERIOD_TIME + t1 + tcs +
> > + (lttpr_count - 1) * (tlw + tlfps_period));
> > + int tacds = 70000;
> > + int tds = (lttpr_count - 1) * 7 * tml_phy_lock;
> > +
> > + /* tdrl is same as tcds*/
> > + establishment_period = tlw + tlfps_period + tdcs + tacds +
> > tds + tcds;
> > + } else {
> > + /* TODO: Add a check for data realign by DPCD 0x116[3] */
> > +
> > + establishment_period = (SILENCE_PERIOD_TIME + t1 + tcds);
> > + }
> >
> > return establishment_period;
> > }
> >
> > +static int get_switch_to_active(const struct intel_crtc_state
> > +*crtc_state) {
> > + int port_clock = crtc_state->port_clock;
> > + int switch_to_active;
>
> Make this switch_to_active = 0 that way the else block later is not required
>
> > +
> > + if (intel_dp_is_uhbr(crtc_state)) {
> > + int symbol_clock = port_clock /
> > +intel_dp_link_symbol_size(port_clock);
> > +
> > + switch_to_active = 32 * DIV_ROUND_UP((396 + 3 + 64),
>
> I would like if you had 396 assigned as a variable ml_phy_lock_len
>
> > symbol_clock);
> > + } else {
> > + switch_to_active = 0;
> > + }
>
> Should you not take care of the mst use case.
>
> So the switch to active latency seems wrong here what you are calculating
> here is T_switch_to_active = 32 * DIV_ROUND_UP((396 + 3 + 64),flink)
> Switch_to_active_latency= CEIL(t_switch_to_active/t_line) And this need to
> be written to ALPM_CTL2 which you are not doing see below comment.
> Also you are not taking into account the fact that f_link is in MHZ and
> intel_dp_link_symbol_clock give value in kHZ so probably look into that too.
> Still not sure how f_link is the intel_dp_link_symbol_clock can you explain
> that
> too?
>
> > +
> > + return switch_to_active;
> > +}
> > +
> > /*
> > * AUX-Less Wake Time = CEILING( ((PHY P2 to P0) + tLFPS_Period, Max+
> > * tSilence, Max+ tPHY Establishment + tCDS) / tline) @@ -136,13
> > +177,15 @@ static int get_establishment_period(const struct
> > intel_crtc_state
> > *crtc_state)
> > * tML_PHY_LOCK = TPS4 Length * ( 10 / (Link Rate in MHz) )
> > * TPS4 Length = 252 Symbols
> > */
>
> Lot of changes in this function and how we calculate data so the comment also
> needs to change here To reflect those changes.
>
>
> > -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
> > *crtc_state)
You also seem to have missed the addition of register bits auxless wake time
extension in ALPM_CLT2 and
Updation of Auxless wake time in APLM_CTL and the corresponding checks that you
need to do to determine what needs
To be written.
There are also some other register bit in ALPM_CTL that need updation or
redefinition of how the value in them is written.
Have a look
AUX_LESS_SLEEP_HOLD_TIME
ALPM_ENTRY_CHECK
AUX_LESS_WAKE_TIME
Regards,
Suraj Kandpal
> > {
> > int tphy2_p2_to_p0 = get_tphy2_p2_to_p0(crtc_state);
> > - int establishment_period = get_establishment_period(crtc_state);
> > + int establishment_period = get_establishment_period(intel_dp,
> > crtc_state);
> > + int switch_to_active = get_switch_to_active(crtc_state);
> >
> > return DIV_ROUND_UP(tphy2_p2_to_p0 +
> > get_lfps_cycle_time(crtc_state) +
> > - establishment_period, 1000);
> > + establishment_period + switch_to_active, 1000);
>
> I do not think you have to add the switch to active latency here, Switch to
> active latency a separate field in ALPM_CTL2 so you will have to write it
> Which means you need to create another variable that you fill with switch to
> active latency.
> H/w will internally maintain Aux Wake time + Switch to active latency.
> I don't see you writing to ALPM_CTL2[Switch to Active time] here or
> anywhere else did you miss it?
>
> Regards,
> Suraj Kandpal
>
> > }
> >
> > static int
> > @@ -154,7 +197,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
