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 <[email protected]> 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/[email protected]?part=4
