Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/i915/alpm: ALPM AUX-less wake time incorrectly restricted to legacy 6-bit maximum on xe3lpd --
commit fefc7fdf839fcf4154d50e19eac0f82242cca5ad Author: Animesh Manna <[email protected]> Subject: drm/i915/alpm: Modify AUX_LESS_WAKE_TIME bitfield for xe3lpd This commit widens the ALPM_CTL_AUX_LESS_WAKE_TIME bitfield from 6 bits to 8 bits for xe3lpd (display version 35) and later. It introduces new register macros and updates lnl_alpm_configure() to program the wake time using the appropriate macro based on the display version. > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > b/drivers/gpu/drm/i915/display/intel_alpm.c > index 924fa8dd4638f..dc025058a6266 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > @@ -610,8 +610,12 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp, > if (intel_alpm_is_alpm_aux_less(intel_dp, crtc_state)) { > 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(crtc_state->alpm_state.aux_less_wake_lines); > + ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS; > + > + if (DISPLAY_VER(display) < 35) > + alpm_ctl |= > ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state->alpm_state.aux_less_wake_lines); > + else > + alpm_ctl |= > ALPM_CTL_AUX_LESS_WAKE_TIME_XE3LPD(crtc_state->alpm_state.aux_less_wake_lines); [Severity: Medium] While this successfully programs the wider 8-bit wake time on xe3lpd hardware, does _lnl_compute_aux_less_alpm_params() in the same file also need to be updated to use this new mask? Looking at _lnl_compute_aux_less_alpm_params(), the bounds check is still hardcoded to the 6-bit legacy mask ALPM_CTL_AUX_LESS_WAKE_TIME_MASK on all platforms: if (aux_less_wake_lines > ALPM_CTL_AUX_LESS_WAKE_TIME_MASK || silence_period > PORT_ALPM_CTL_SILENCE_PERIOD_MASK || lfps_half_cycle > PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK) return false; Will this restrict aux_less_wake_lines to a maximum of 63, causing validation to fail and aborting ALPM if the computed wake time is larger on xe3lpd? Additionally, the safest params fallback is also restricted to the legacy mask: if (display->params.psr_safest_params) aux_less_wake_lines = ALPM_CTL_AUX_LESS_WAKE_TIME_MASK; Will this cap the safest params to 63 instead of 255 on xe3lpd? > > if (intel_dp->as_sdp_supported) > intel_alpm_configure_pr_as_sdp(intel_dp, crtc_state); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
