Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Potential kernel panic due to division by zero if `refresh_hz` or `denom` evaluates to 0. -- commit 66e20e6af2a54e1f051a2802b102f884a27f0948 Author: Francis Marlou Pacaro <[email protected]> drm/amd/display: fix MALL hysteresis timer underflow at high refresh rates This commit fixes an issue where high refresh rates cause the MALL frame cache hysteresis timer calculation to wrap instead of going negative. It addresses this by explicitly calculating the timer ticks and clamping the result to 0 to prevent the underflow. > diff --git a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c > b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c > index 8b0a667dc59b7..76aa7a82e24e7 100644 > --- a/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c > +++ b/drivers/gpu/drm/amd/display/dc/hwss/dcn30/dcn30_hwseq.c > @@ -1065,10 +1065,12 @@ bool dcn30_apply_idle_power_optimizations(struct dc > *dc, bool enable) > */ > unsigned int denom = refresh_hz * 6528; > unsigned int stutter_period = > dc->current_state->perf_params.stutter_period_us; > + u64 num = (1000000LL + 2 * stutter_period * > refresh_hz) * > + (100LL + > dc->debug.mall_additional_timer_percent); > + u64 tmr_ticks; > > - tmr_delay = (uint32_t)(div_u64(((1000000LL + 2 > * stutter_period * refresh_hz) * > - (100LL + > dc->debug.mall_additional_timer_percent) + denom - 1), > - denom) - 64LL); > + tmr_ticks = div_u64(num + denom - 1, denom); [Severity: High] This is a pre-existing issue, but could this calculation trigger a division by zero kernel panic if denom is 0? Looking at how refresh_hz is calculated earlier in dcn30_apply_idle_power_optimizations(): refresh_hz = (unsigned int)div_u64( (unsigned long long)stream->timing.pix_clk_100hz * 100LL, (v_total * stream->timing.h_total)); If a compositor provides a malformed display mode with very low clocks and high totals such that pix_clk_100hz * 100 < v_total * h_total, refresh_hz will evaluate to 0. This makes denom = 0, leading to a division by zero in the div_u64() call above. Additionally, this same condition natively panics a few lines down in the unpatched context where we check the stutter period: if (stutter_period > 100000/refresh_hz) return false; Would it make sense to add an early return to check if refresh_hz evaluates to 0 before proceeding with these calculations? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
