Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unconsumed CRTC VBLANK event when disabling the CRTC causes a memory leak and userspace hang. --
commit f35a0721a36bbf78ed41d1be8bb186efb88c9092 Author: Ze Huang <[email protected]> Subject: drm/pl111: replace struct drm_simple_display_pipe with regular atomic helpers This commit replaces the PL111 simple display pipe with explicit plane, CRTC, and encoder objects. It preserves existing hardware programming and vblank behavior while using the GEM framebuffer prepare helper. > diff --git a/drivers/gpu/drm/pl111/pl111_display.c > b/drivers/gpu/drm/pl111/pl111_display.c > index 5d10bc5fdf1f6..b1bdd4c9dbe68 100644 > --- a/drivers/gpu/drm/pl111/pl111_display.c > +++ b/drivers/gpu/drm/pl111/pl111_display.c [ ... ] > -static void pl111_display_disable(struct drm_simple_display_pipe *pipe) > +static void pl111_crtc_helper_atomic_disable(struct drm_crtc *crtc, > + struct drm_atomic_commit *state) > { > - struct drm_crtc *crtc = &pipe->crtc; > struct drm_device *drm = crtc->dev; > struct pl111_drm_dev_private *priv = drm->dev_private; > u32 cntl; [ ... ] > -static void pl111_display_update(struct drm_simple_display_pipe *pipe, > - struct drm_plane_state *old_pstate) > +static void pl111_plane_helper_atomic_update(struct drm_plane *plane, > + struct drm_atomic_commit *state) > { > - struct drm_crtc *crtc = &pipe->crtc; > - struct drm_device *drm = crtc->dev; > - struct pl111_drm_dev_private *priv = drm->dev_private; > - struct drm_pending_vblank_event *event = crtc->state->event; > - struct drm_plane *plane = &pipe->plane; > + struct drm_crtc *crtc = plane->state->crtc; > + struct drm_device *drm; > + struct pl111_drm_dev_private *priv; > + struct drm_pending_vblank_event *event; > struct drm_plane_state *pstate = plane->state; > struct drm_framebuffer *fb = pstate->fb; > > + if (!crtc) > + return; > + [Severity: High] When disabling the CRTC, does this code leak the vblank event? If an atomic commit disables the CRTC and requests a completion event, this early return in pl111_plane_helper_atomic_update() triggers because plane->state->crtc is NULL when the plane is disabled. Since the event consumption logic previously provided by the drm_simple_display_pipe framework has been removed, and pl111_crtc_helper_atomic_disable() also lacks logic to consume crtc->state->event, won't the event remain attached and unconsumed? This can trigger a kernel warning in drm_atomic_helper_commit_hw_done() (which warns if the event is still attached) and cause userspace compositors to hang indefinitely waiting for the page flip event. > + drm = crtc->dev; > + priv = drm->dev_private; > + event = crtc->state->event; > + > if (fb) { > u32 addr = drm_fb_dma_get_gem_addr(fb, pstate, 0); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260705-drm-simple-kms-removal-v1-0-b4e1ca053...@oss.qualcomm.com?part=5
