Re: [v5 2/4] drm/i915/display: Force disable DC3co when AS SDP skip frames is enabled

[sashiko-bot@kernel.org]

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 <uma.shankar@intel.com>

    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/20260909080810.2202879-1-uma.shankar@intel.com?part=2