Re: [v3 3/4] drm/i915/display: Reprogram AS SDP skip frames on seamless VRR transitions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Sleeping function mutex_lock() is called from atomic context during seamless VRR enable.
--

commit 130854c15003f92b491f6090e85bf6bc4afe74ed
Author: Uma Shankar <uma.shankar@intel.com>

drm/i915/display: Reprogram AS SDP skip frames on seamless VRR transitions

This commit factors out AS SDP skip-frame programming and exposes
intel_alpm_pr_as_sdp_update() to recompute fields for the current VRR
state. It calls this function from seamless VRR enable and disable sites
so the skip counter matches whether VRR is actively driving the refresh rate.

> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 5f635f14a5880..03d5bf5c526f6 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> [ ... ]
> +void intel_alpm_pr_as_sdp_update(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct intel_encoder *encoder;
> +
> +	/* AS SDP skip frames field only exists on Xe3LPD+ */
> +	if (DISPLAY_VER(display) < 35)
> +		return;
> +
> +	for_each_intel_encoder_mask(display->drm, encoder,
> +				    crtc_state->uapi.encoder_mask) {
> +		struct intel_dp *intel_dp;
> +
> +		if (!intel_encoder_is_dp(encoder))
> +			continue;
> +
> +		intel_dp = enc_to_intel_dp(encoder);
> +
> +		if (!intel_dp->as_sdp_supported ||
> +		    !intel_alpm_is_alpm_aux_less(intel_dp, crtc_state))
> +			continue;
> +
> +		mutex_lock(&intel_dp->alpm.lock);
                ^^^^^^^^^^

[Severity: High]
Is it safe to acquire a mutex here? This function is called from
commit_pipe_post_planes() and intel_pre_plane_update() during seamless VRR
transitions.

Since these callers execute within the vblank evasion critical section,
local interrupts are disabled and we are in an atomic context:

intel_update_crtc() {
        intel_pipe_update_start(state, crtc); /* Disables local interrupts */
        ...
        commit_pipe_post_planes(state, crtc);
        ...
}

Calling a sleeping lock like mutex_lock() from this atomic context will
trigger a scheduling while atomic oops and can lead to a hard system
deadlock when dynamically toggling VRR on a supported display.

> +		intel_alpm_configure_pr_as_sdp(intel_dp, crtc_state);
> +		mutex_unlock(&intel_dp->alpm.lock);
> +	}
> +}
> [ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 9dfd095f325f7..a6b776da14b3a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> [ ... ]
> @@ -6796,8 +6804,17 @@ static void commit_pipe_post_planes(struct intel_atomic_state *state,
>  	    HAS_DOUBLE_BUFFERED_LUT(display))
>  		intel_color_load_luts(new_crtc_state);
>  
> -	if (intel_crtc_vrr_enabling(state, crtc))
> +	if (intel_crtc_vrr_enabling(state, crtc)) {
>  		intel_vrr_enable(new_crtc_state);
> +
> +		/*
> +		 * VRR is being enabled seamlessly (no modeset, Panel Replay
> +		 * stays enabled), so re-apply the AS SDP skip-frame programming
> +		 * for the new (VRR on) state.
> +		 */
> +		if (!modeset)
> +			intel_alpm_pr_as_sdp_update(new_crtc_state);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831125914.1736501-1-uma.shankar@intel.com?part=3