Re: [PATCH 04/10] drm/intel/display: Convert vblank event handling to 2-stage arming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Moving vblank event preparation and counter sampling before vblank evasion causes events and workers to fire one frame early if an evasion wait occurs.

Pre-existing issues:
- [High] drm_crtc_vblank_put() is called within a local_irq_disable() critical section, which can acquire a sleepable rt_mutex on PREEMPT_RT kernels, causing a lockdep splat and potential system crash.
--

commit 90a4948ef379d3e7a2ebc312abe7069b65f38a68
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/intel/display: Convert vblank event handling to 2-stage arming

This converts the vblank functions to be called with interrupts
disabled, even on PREEMPT_RT kernels. Instead of using the normal
vblank functions inside the critical section, it prepares the vblank
at the start and enables the work after hardware programming completes.

> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 805645318747f..34a159f7c9a43 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
[ ... ]
> @@ -607,6 +626,8 @@ void intel_pipe_update_start(struct intel_atomic_state *state,
>  
>  	if (intel_crtc_needs_vblank_work(new_crtc_state))
>  		intel_crtc_vblank_work_init(new_crtc_state);
> +	else
> +		intel_crtc_arm_vblank_event(new_crtc_state);

[Severity: High]
Does moving the vblank event preparation and counter sampling before the
evasion wait cause the event to fire a frame early if the wait actually
spins?

If intel_pipe_update_start() locks the target vblank sequence to the
current vblank count + 1, and the CPU is delayed so that the evasion
wait spins:

intel_pipe_update_start() {
    ...
    scanline = intel_vblank_evade(&evade);
    ...
}

The hardware latching point will have passed, meaning the registers will
apply at N+2. However, because the event sequence is already locked to N+1
prior to the evasion, the event will fire a frame early in
intel_pipe_update_end() when un-postponed:

intel_pipe_update_end() {
    ...
    drm_crtc_arm_prepared_vblank_event(new_crtc_state->uapi.event);
    ...
}

Could this lead to screen tearing or premature compositor synchronization?

[Severity: High]
This is a pre-existing issue, but does calling drm_crtc_vblank_put() inside
the local_irq_disable() block later in this function cause issues on
PREEMPT_RT kernels?

intel_pipe_update_start() {
    ...
    local_irq_disable();
    ...
    scanline = intel_vblank_evade(&evade);
    drm_crtc_vblank_put(&crtc->base);
    ...
}

If this call drops the last vblank reference and immediate vblank disable is
configured, vblank_disable_fn() executes synchronously and acquires
dev->vbl_lock.

Since this spinlock is converted to a sleepable rt_mutex on PREEMPT_RT
kernels, will this cause a lockdep splat or panic by sleeping in atomic
context?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702072154.171324-1-dev@lankhorst.se?part=4