> 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); > /* 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) > { > 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
