Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] `get_lfps_half_cycle_clocks()` incorrectly uses the DP port clock (link rate) instead of the symbol clock, which inflates the cycle count and causes it to exceed the 5-bit register mask for DP 2.1 UHBR rates, completely breaking ALPM. -- --- Patch [8]: [PATCH v5 08/15] drm/i915/alpm: Half LFPS cycle calculation --- commit 3b44d7e9f6ce14c0e3f4bb3319784906c402099b Author: Animesh Manna <[email protected]> drm/i915/alpm: Half LFPS cycle calculation Add support for half LFPS cycle calculation for DP2.1 ALPM as dependent parameters got changed. > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c > b/drivers/gpu/drm/i915/display/intel_alpm.c > index 078e04bb69d29..98aa418a6ff77 100644 > --- a/drivers/gpu/drm/i915/display/intel_alpm.c > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c > @@ -80,12 +80,18 @@ static int get_silence_period_symbols(const struct > intel_crtc_state *crtc_state) [Severity: High] This isn't a bug introduced by this patch, but since this patch series enables DP 2.1 ALPM, it exposes an issue in get_lfps_half_cycle_clocks() (which is located nearby in this file). Does get_lfps_half_cycle_clocks() incorrectly use the DP port clock (link rate) instead of the symbol clock? For legacy 8b/10b encoding, the link rate in 10 kbps is numerically equal to the symbol clock in kHz. However, for 128b/132b (UHBR) rates introduced in DP 2.1, the link rate is 3.2x the symbol clock. This inflates the symbol count calculation by a factor of 3.2. When this inflated value is checked against the 5-bit mask (PORT_ALPM_LFPS_CTL_LAST_LFPS_HALF_CYCLE_DURATION_MASK) in _lnl_compute_aux_less_alpm_params(), it unconditionally fails because it exceeds the maximum value of 31. This causes ALPM configuration to abort for any DP 2.1 UHBR display. Other ALPM functions like get_silence_period_symbols() correctly use intel_dp_link_symbol_clock(crtc_state->port_clock). Should get_lfps_half_cycle_clocks() also be updated to use the symbol clock? > static void get_lfps_cycle_min_max_time(const struct intel_crtc_state > *crtc_state, > int *min, int *max) > { > - if (crtc_state->port_clock < 540000) { > - *min = 65 * LFPS_CYCLE_COUNT; > - *max = 75 * LFPS_CYCLE_COUNT; > - } else { > - *min = 140; > - *max = 800; > + struct intel_display *display = to_intel_display(crtc_state); > + > + *min = 320; > + *max = 1600; > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP)) { > + if (crtc_state->port_clock < 540000 && DISPLAY_VER(display) < > 35) { > + *min = 65 * LFPS_CYCLE_COUNT; > + *max = 75 * LFPS_CYCLE_COUNT; > + } else { > + *min = 140; > + *max = 800; > + } > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
