On Tue, 2025-10-07 at 09:28 +0000, Manna, Animesh wrote: > > > > -----Original Message----- > > From: Hogander, Jouni <[email protected]> > > Sent: Monday, September 29, 2025 6:30 PM > > To: [email protected]; [email protected] > > Cc: Manna, Animesh <[email protected]>; Hogander, Jouni > > <[email protected]> > > Subject: [PATCH v2 1/2] drm/i915/alpm: Compute ALPM parameters into > > crtc_state->alpm_state > > > > Currently ALPM parameters are computed directly into intel_dp- > > > alpm_parameters. This is a problem when compute config ends up to > > > not > > using the computed state. > > > > Fix this by adding ALPM parameters into intel_crtc_state and > > compute into > > there. Copy needed parameters (io_wake_lines and fast_wake_lines) > > from > > crtc_state->alpm_state into intel_dp->alpm.alpm_parameters > > (io_wake_lines > > and fast_wake_lines) when they are configured into HW. > > Good to add why copy needed for these two parameters in commit > description. > Other changes looks good to me. With the above fix. > > Reviewed-by: Animesh Manna <[email protected]>
Thank you Animesh for the review. These are both now pushed to drm- intel-next with you suggestion here included. BR, Jouni Högander > > > > > v2: > > - store io/fast wake lines into intel_dp->dp instead of > > intel_dp->alpm_parameters and do it in intel_psr_enable_locked > > - rename crtc_state->alpm_parameters -> crtc_state->alpm_state > > - clarify commit message > > > > Signed-off-by: Jouni Högander <[email protected]> > > --- > > drivers/gpu/drm/i915/display/intel_alpm.c | 36 +++++++++------ > > ---- > > drivers/gpu/drm/i915/display/intel_alpm.h | 2 +- > > .../drm/i915/display/intel_display_types.h | 21 +++++++---- > > drivers/gpu/drm/i915/display/intel_psr.c | 36 ++++++++++----- > > ---- > > 4 files changed, 53 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > > b/drivers/gpu/drm/i915/display/intel_alpm.c > > index 749119cc0b28..9f4a9d8b4dec 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > > @@ -122,7 +122,7 @@ static int > > _lnl_compute_aux_less_wake_time(const > > struct intel_crtc_state *crtc_s > > > > static int > > _lnl_compute_aux_less_alpm_params(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state) > > + struct intel_crtc_state > > *crtc_state) > > { > > struct intel_display *display = > > to_intel_display(intel_dp); > > int aux_less_wake_time, aux_less_wake_lines, > > silence_period, @@ - > > 144,15 +144,15 @@ _lnl_compute_aux_less_alpm_params(struct intel_dp > > *intel_dp, > > if (display->params.psr_safest_params) > > aux_less_wake_lines = > > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK; > > > > - intel_dp->alpm_parameters.aux_less_wake_lines = > > aux_less_wake_lines; > > - intel_dp->alpm_parameters.silence_period_sym_clocks = > > silence_period; > > - intel_dp->alpm_parameters.lfps_half_cycle_num_of_syms = > > lfps_half_cycle; > > + crtc_state->alpm_state.aux_less_wake_lines = > > aux_less_wake_lines; > > + crtc_state->alpm_state.silence_period_sym_clocks = > > silence_period; > > + crtc_state->alpm_state.lfps_half_cycle_num_of_syms = > > lfps_half_cycle; > > > > return true; > > } > > > > static bool _lnl_compute_alpm_params(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state) > > + struct intel_crtc_state > > *crtc_state) > > { > > struct intel_display *display = > > to_intel_display(intel_dp); > > int check_entry_lines; > > @@ -173,7 +173,7 @@ static bool _lnl_compute_alpm_params(struct > > intel_dp *intel_dp, > > if (display->params.psr_safest_params) > > check_entry_lines = 15; > > > > - intel_dp->alpm_parameters.check_entry_lines = > > check_entry_lines; > > + crtc_state->alpm_state.check_entry_lines = > > check_entry_lines; > > > > return true; > > } > > @@ -204,7 +204,7 @@ static int io_buffer_wake_time(const struct > > intel_crtc_state *crtc_state) } > > > > bool intel_alpm_compute_params(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state) > > + struct intel_crtc_state > > *crtc_state) > > { > > struct intel_display *display = > > to_intel_display(intel_dp); > > int io_wake_lines, io_wake_time, fast_wake_lines, > > fast_wake_time; > > @@ -242,8 +242,8 @@ bool intel_alpm_compute_params(struct intel_dp > > *intel_dp, > > io_wake_lines = fast_wake_lines = max_wake_lines; > > > > /* According to Bspec lower limit should be set as 7 > > lines. */ > > - intel_dp->alpm_parameters.io_wake_lines = > > max(io_wake_lines, 7); > > - intel_dp->alpm_parameters.fast_wake_lines = > > max(fast_wake_lines, > > 7); > > + crtc_state->alpm_state.io_wake_lines = max(io_wake_lines, > > 7); > > + crtc_state->alpm_state.fast_wake_lines = > > max(fast_wake_lines, 7); > > > > return true; > > } > > @@ -293,9 +293,9 @@ void intel_alpm_lobf_compute_config(struct > > intel_dp > > *intel_dp, > > adjusted_mode->crtc_vdisplay - > > context_latency; > > first_sdp_position = adjusted_mode->crtc_vtotal - > > adjusted_mode- > > > crtc_vsync_start; > > if (intel_alpm_aux_less_wake_supported(intel_dp)) > > - waketime_in_lines = intel_dp- > > > alpm_parameters.io_wake_lines; > > + waketime_in_lines = crtc_state- > > >alpm_state.io_wake_lines; > > else > > - waketime_in_lines = intel_dp- > > > alpm_parameters.aux_less_wake_lines; > > + waketime_in_lines = crtc_state- > > > alpm_state.aux_less_wake_lines; > > > > crtc_state->has_lobf = (context_latency + guardband) > > > (first_sdp_position + waketime_in_lines); @@ - > > 321,7 +321,7 > > @@ static void lnl_alpm_configure(struct intel_dp *intel_dp, > > alpm_ctl = ALPM_CTL_ALPM_ENABLE | > > ALPM_CTL_ALPM_AUX_LESS_ENABLE | > > > > ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS | > > - ALPM_CTL_AUX_LESS_WAKE_TIME(intel_dp- > > > alpm_parameters.aux_less_wake_lines); > > + > > +ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state- > > > alpm_state.aux_less_wake_lines) > > +; > > > > if (intel_dp->as_sdp_supported) { > > u32 pr_alpm_ctl = > > PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1; > > @@ -339,7 +339,7 @@ static void lnl_alpm_configure(struct intel_dp > > *intel_dp, > > > > } else { > > alpm_ctl = ALPM_CTL_EXTENDED_FAST_WAKE_ENABLE | > > - ALPM_CTL_EXTENDED_FAST_WAKE_TIME(intel_dp- > > > alpm_parameters.fast_wake_lines); > > + > > +ALPM_CTL_EXTENDED_FAST_WAKE_TIME(crtc_state- > > > alpm_state.fast_wake_lines > > +); > > } > > > > if (crtc_state->has_lobf) { > > @@ -347,7 +347,7 @@ static void lnl_alpm_configure(struct intel_dp > > *intel_dp, > > drm_dbg_kms(display->drm, "Link off between frames > > (LOBF) enabled\n"); > > } > > > > - alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp- > > > alpm_parameters.check_entry_lines); > > + alpm_ctl |= > > +ALPM_CTL_ALPM_ENTRY_CHECK(crtc_state- > > > alpm_state.check_entry_lines); > > > > intel_de_write(display, ALPM_CTL(display, cpu_transcoder), > > alpm_ctl); > > mutex_unlock(&intel_dp->alpm_parameters.lock); > > @@ -375,14 +375,14 @@ void intel_alpm_port_configure(struct > > intel_dp > > *intel_dp, > > PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) | > > PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) | > > PORT_ALPM_CTL_SILENCE_PERIOD( > > - intel_dp- > > > alpm_parameters.silence_period_sym_clocks); > > + crtc_state- > > > alpm_state.silence_period_sym_clocks); > > lfps_ctl_val = > > PORT_ALPM_LFPS_CTL_LFPS_CYCLE_COUNT(LFPS_CYCLE_COUNT) | > > > > PORT_ALPM_LFPS_CTL_LFPS_HALF_CYCLE_DURATION( > > - intel_dp- > > > alpm_parameters.lfps_half_cycle_num_of_syms) | > > + crtc_state- > > > alpm_state.lfps_half_cycle_num_of_syms) | > > > > PORT_ALPM_LFPS_CTL_FIRST_LFPS_HALF_CYCLE_DURATION( > > - intel_dp- > > > alpm_parameters.lfps_half_cycle_num_of_syms) | > > + crtc_state- > > > alpm_state.lfps_half_cycle_num_of_syms) | > > > > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION( > > - intel_dp- > > > alpm_parameters.lfps_half_cycle_num_of_syms); > > + crtc_state- > > > alpm_state.lfps_half_cycle_num_of_syms); > > } > > > > intel_de_write(display, PORT_ALPM_CTL(port), > > alpm_ctl_val); diff -- > > git a/drivers/gpu/drm/i915/display/intel_alpm.h > > b/drivers/gpu/drm/i915/display/intel_alpm.h > > index a861c20b5d79..53599b464dea 100644 > > --- a/drivers/gpu/drm/i915/display/intel_alpm.h > > +++ b/drivers/gpu/drm/i915/display/intel_alpm.h > > @@ -17,7 +17,7 @@ struct intel_crtc; > > > > void intel_alpm_init(struct intel_dp *intel_dp); bool > > intel_alpm_compute_params(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state); > > + struct intel_crtc_state > > *crtc_state); > > void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp, > > struct intel_crtc_state > > *crtc_state, > > struct drm_connector_state > > *conn_state); > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 029c47743f8b..57879cc5546e 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -1344,6 +1344,17 @@ struct intel_crtc_state { > > > > /* W2 window or 'set context latency' lines */ > > u16 set_context_latency; > > + > > + struct { > > + u8 io_wake_lines; > > + u8 fast_wake_lines; > > + > > + /* LNL and beyond */ > > + u8 check_entry_lines; > > + u8 aux_less_wake_lines; > > + u8 silence_period_sym_clocks; > > + u8 lfps_half_cycle_num_of_syms; > > + } alpm_state; > > }; > > > > enum intel_pipe_crc_source { > > @@ -1688,6 +1699,9 @@ struct intel_psr { > > struct delayed_work dc3co_work; > > u8 entry_setup_frames; > > > > + u8 io_wake_lines; > > + u8 fast_wake_lines; > > + > > bool link_ok; > > bool pkg_c_latency_used; > > > > @@ -1847,16 +1861,9 @@ struct intel_dp { > > bool colorimetry_support; > > > > struct { > > - u8 io_wake_lines; > > - u8 fast_wake_lines; > > enum transcoder transcoder; > > struct mutex lock; > > > > - /* LNL and beyond */ > > - u8 check_entry_lines; > > - u8 aux_less_wake_lines; > > - u8 silence_period_sym_clocks; > > - u8 lfps_half_cycle_num_of_syms; > > bool lobf_disable_debug; > > bool sink_alpm_error; > > } alpm_parameters; > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c > > b/drivers/gpu/drm/i915/display/intel_psr.c > > index f7115969b4c5..2131473cead6 100644 > > --- a/drivers/gpu/drm/i915/display/intel_psr.c > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c > > @@ -956,15 +956,16 @@ static u32 intel_psr2_get_tp_time(struct > > intel_dp > > *intel_dp) > > return val; > > } > > > > -static int psr2_block_count_lines(struct intel_dp *intel_dp) > > +static int > > +psr2_block_count_lines(u8 io_wake_lines, u8 fast_wake_lines) > > { > > - return intel_dp->alpm_parameters.io_wake_lines < 9 && > > - intel_dp->alpm_parameters.fast_wake_lines < 9 ? 8 > > : 12; > > + return io_wake_lines < 9 && fast_wake_lines < 9 ? 8 : 12; > > } > > > > static int psr2_block_count(struct intel_dp *intel_dp) { > > - return psr2_block_count_lines(intel_dp) / 4; > > + return psr2_block_count_lines(intel_dp->psr.io_wake_lines, > > + intel_dp- > > >psr.fast_wake_lines) / 4; > > } > > > > static u8 frames_before_su_entry(struct intel_dp *intel_dp) @@ - > > 1059,20 > > +1060,20 @@ static void hsw_activate_psr2(struct intel_dp > > *intel_dp) > > */ > > int tmp; > > > > - tmp = map[intel_dp->alpm_parameters.io_wake_lines > > - > > + tmp = map[intel_dp->psr.io_wake_lines - > > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES]; > > val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(tmp + > > TGL_EDP_PSR2_IO_BUFFER_WAKE_MIN_LINES); > > > > - tmp = map[intel_dp- > > >alpm_parameters.fast_wake_lines - > > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES]; > > + tmp = map[intel_dp->psr.fast_wake_lines - > > +TGL_EDP_PSR2_FAST_WAKE_MIN_LINES]; > > val |= TGL_EDP_PSR2_FAST_WAKE(tmp + > > TGL_EDP_PSR2_FAST_WAKE_MIN_LINES); > > } else if (DISPLAY_VER(display) >= 20) { > > - val |= LNL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > alpm_parameters.io_wake_lines); > > + val |= LNL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > psr.io_wake_lines); > > } else if (DISPLAY_VER(display) >= 12) { > > - val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > alpm_parameters.io_wake_lines); > > - val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp- > > > alpm_parameters.fast_wake_lines); > > + val |= TGL_EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > psr.io_wake_lines); > > + val |= TGL_EDP_PSR2_FAST_WAKE(intel_dp- > > > psr.fast_wake_lines); > > } else if (DISPLAY_VER(display) >= 9) { > > - val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > alpm_parameters.io_wake_lines); > > - val |= EDP_PSR2_FAST_WAKE(intel_dp- > > > alpm_parameters.fast_wake_lines); > > + val |= EDP_PSR2_IO_BUFFER_WAKE(intel_dp- > > > psr.io_wake_lines); > > + val |= EDP_PSR2_FAST_WAKE(intel_dp- > > > psr.fast_wake_lines); > > } > > > > if (intel_dp->psr.req_psr2_sdp_prior_scanline) > > @@ -1370,11 +1371,12 @@ static bool > > wake_lines_fit_into_vblank(struct > > intel_dp *intel_dp, > > int wake_lines; > > > > if (aux_less) > > - wake_lines = intel_dp- > > > alpm_parameters.aux_less_wake_lines; > > + wake_lines = crtc_state- > > >alpm_state.aux_less_wake_lines; > > else > > wake_lines = DISPLAY_VER(display) < 20 ? > > - psr2_block_count_lines(intel_dp) : > > - intel_dp->alpm_parameters.io_wake_lines; > > + psr2_block_count_lines(crtc_state- > > > alpm_state.io_wake_lines, > > + crtc_state- > > > alpm_state.fast_wake_lines) : > > + crtc_state->alpm_state.io_wake_lines; > > > > if (crtc_state->req_psr2_sdp_prior_scanline) > > vblank -= 1; > > @@ -1387,7 +1389,7 @@ static bool wake_lines_fit_into_vblank(struct > > intel_dp *intel_dp, } > > > > static bool alpm_config_valid(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state, > > + struct intel_crtc_state *crtc_state, > > bool aux_less) > > { > > struct intel_display *display = > > to_intel_display(intel_dp); @@ -1592,7 > > +1594,7 @@ static bool _psr_compute_config(struct intel_dp > > *intel_dp, > > > > static bool > > _panel_replay_compute_config(struct intel_dp *intel_dp, > > - const struct intel_crtc_state > > *crtc_state, > > + struct intel_crtc_state *crtc_state, > > const struct drm_connector_state > > *conn_state) { > > struct intel_display *display = > > to_intel_display(intel_dp); @@ -2022,6 > > +2024,8 @@ static void intel_psr_enable_locked(struct intel_dp > > *intel_dp, > > crtc_state->req_psr2_sdp_prior_scanline; > > intel_dp->psr.active_non_psr_pipes = crtc_state- > > > active_non_psr_pipes; > > intel_dp->psr.pkg_c_latency_used = crtc_state- > > >pkg_c_latency_used; > > + intel_dp->psr.io_wake_lines = crtc_state- > > >alpm_state.io_wake_lines; > > + intel_dp->psr.fast_wake_lines = > > +crtc_state->alpm_state.fast_wake_lines; > > > > if (!psr_interrupt_error_check(intel_dp)) > > return; > > -- > > 2.43.0 >
