Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Lockless access to `crtc->base.state` in `intel_display_power_dc3co_compute()` for CRTCs not in the current atomic state can lead to Use-After-Free. -- commit 8c209591fdca806eaeb08fb945c1a557f13cd189 Author: Uma Shankar <[email protected]> drm/i915/display: Force disable DC3co when AS SDP skip frames is enabled Periodic AS SDP (skip frames) relies on the AS SDP still being transmitted while Panel Replay is active. DC3co uses the idle protocol which suppresses AS SDP transmission entirely, so the two are mutually exclusive: leaving DC3co enabled while skip frames is programmed breaks the periodic AS SDP and the panel never sees the slower refresh. > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > b/drivers/gpu/drm/i915/display/intel_display_power.c > index 0ebec6e0c2400..1b60ce2dd00c4 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c [ ... ] > @@ -488,6 +489,14 @@ void intel_display_power_dc3co_compute(struct > intel_atomic_state *state) > if (crtc_state->has_sel_update) > trigger |= DC3CO_TRIGGER_PSR2; > > + /* > + * Periodic AS SDP (skip frames) needs the AS SDP to keep flowing during > + * PR active, which is incompatible with DC3co. Keep DC3co disabled > while > + * skip frames is enabled. > + */ > + if (intel_alpm_pr_as_sdp_skip_frames_enabled(intel_dp, crtc_state)) [Severity: High] This is a pre-existing issue, but this patch exacerbates it by adding more reads to the locklessly accessed state. Can this lockless access to crtc_state lead to a use-after-free? Looking at intel_display_power_dc3co_compute(), it retrieves the state of a CRTC without locking it: pipe = ffs(active_pipes) - 1; crtc = intel_crtc_for_pipe(display, pipe); crtc_state = to_intel_crtc_state(crtc->base.state); If a system has multiple active CRTCs (e.g., A and B) and a non-blocking atomic commit is issued to disable CRTC A, intel_display_power_dc3co_compute() runs in the commit tail. It calculates that exactly one pipe (CRTC B) will remain active and retrieves its state. Because CRTC B is not part of the current atomic commit, no locks are held for it. A concurrent non-blocking atomic commit on CRTC B can swap and free its state while the first commit is reading fields like crtc_state->has_panel_replay or crtc_state->vrr.enable (via intel_alpm_pr_as_sdp_skip_frames_enabled()), potentially causing memory corruption or crashes. > + trigger = DC3CO_TRIGGER_NONE; > + > done: > intel_display_power_dc3co_update(display, trigger); > } -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
