> -----Original Message----- > From: [email protected] <[email protected]> > Sent: Wednesday, September 9, 2026 1:32 PM > To: Shankar, Uma <[email protected]> > Cc: [email protected]; [email protected] > Subject: Re: [v5 2/4] drm/i915/display: Force disable DC3co when AS SDP skip > frames is enabled > > 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.
This is a pre-existing condition and is unlikely scenario. This change is not modifying it and is just using the existing flows. In case any real usecase arises, a separate fix can be sent out. > > + trigger = DC3CO_TRIGGER_NONE; > > + > > done: > > intel_display_power_dc3co_update(display, trigger); } > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260909080810.2202879-1- > [email protected]?part=2
