Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Race condition in atomic commit causing display hang in oneshot mode due to premature hardware start. --
--- Patch [3]: [PATCH v2 3/8] drm/mcde: replace struct drm_simple_display_pipe with regular atomic helpers --- commit 24a767bb6a2c4adfefe9e0a5de7e9c8bbe7b2a64 Author: Ze Huang <[email protected]> drm/mcde: replace struct drm_simple_display_pipe with regular atomic helpers Convert MCDE to explicit plane, CRTC and encoder objects. Handle pending page-flip events from the CRTC atomic flush path using the commit-local CRTC state. > diff --git a/drivers/gpu/drm/mcde/mcde_display.c > b/drivers/gpu/drm/mcde/mcde_display.c > index 257a6e84dd583..86edf94c52372 100644 > --- a/drivers/gpu/drm/mcde/mcde_display.c > +++ b/drivers/gpu/drm/mcde/mcde_display.c [ ... ] > @@ -1381,44 +1398,13 @@ static void mcde_set_extsrc(struct mcde *mcde, u32 > buffer_address) > -static void mcde_display_update(struct drm_simple_display_pipe *pipe, > - struct drm_plane_state *old_pstate) > +static void mcde_plane_helper_atomic_update(struct drm_plane *plane, > + struct drm_atomic_commit *commit) > { > - struct drm_crtc *crtc = &pipe->crtc; > - struct drm_device *drm = crtc->dev; > - struct mcde *mcde = to_mcde(drm); > - struct drm_pending_vblank_event *event = crtc->state->event; > - struct drm_plane *plane = &pipe->plane; > - struct drm_plane_state *pstate = plane->state; > + struct drm_plane_state *pstate = drm_atomic_get_new_plane_state(commit, > plane); > + struct mcde *mcde = to_mcde(plane->dev); > struct drm_framebuffer *fb = pstate->fb; > > - /* > - * Handle any pending event first, we need to arm the vblank > - * interrupt before sending any update to the display so we don't > - * miss the interrupt. > - */ > - if (event) { [ ... ] > - spin_unlock_irq(&crtc->dev->event_lock); > - } > - > /* > * We do not start sending framebuffer updates before the > * display is enabled. Update events will however be dispatched [ ... ] > -static int mcde_display_enable_vblank(struct drm_simple_display_pipe *pipe) > +static void mcde_crtc_helper_atomic_flush(struct drm_crtc *crtc, > + struct drm_atomic_commit *commit) > +{ > + struct drm_crtc_state *crtc_state = > drm_atomic_get_new_crtc_state(commit, crtc); > + struct drm_pending_vblank_event *event = crtc_state->event; > + > + /* > + * Handle any pending event first, we need to arm the vblank > + * interrupt before sending any update to the display so we don't > + * miss the interrupt. > + */ > + if (!event) > + return; [Severity: High] Does this split introduce a race condition that could hang the pipeline in oneshot mode? Because the DRM core executes the plane's atomic_update callback before the CRTC's atomic_flush callback, the hardware flow is now started before the vblank event is armed. If mcde_start_flow() is triggered during mcde_plane_helper_atomic_update() (which happens just below the quoted block), it could complete the hardware transfer and fire its interrupt before the vblank event gets armed later in mcde_crtc_helper_atomic_flush(). The relocated comment in atomic_flush still notes this exact requirement: "we need to arm the vblank interrupt before sending any update to the display so we don't miss the interrupt." If the interrupt fires early in oneshot mode, it will be missed, and the atomic commit could wait indefinitely for an event that will never complete. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260716-drm-simple-kms-removal-v2-0-1133a8fc3...@oss.qualcomm.com?part=3
