Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Missing CRTC active check in the plane's atomic_update callback causes the driver to send SPI framebuffer updates to a powered-off display panel. Pre-existing issues: - [High] The CRTC atomic enable/disable and plane atomic update callbacks execute sleeping functions and allocate memory with GFP_KERNEL, which is forbidden in atomic context. -- --- Patch [6]: [PATCH v2 6/8] drm/repaper: replace struct drm_simple_display_pipe with regular atomic helpers --- commit 3d9748839d37276a557c720301a5a848296e54b7 Author: Ze Huang <[email protected]> drm/repaper: replace struct drm_simple_display_pipe with regular atomic helpers Convert repaper to explicit primary plane, CRTC and encoder objects. Keep shadow-plane helpers, framebuffer access handling and no-scaling plane-state validation from simple-KMS path. > diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c > index c8270591afc7f..7057075161c4d 100644 > --- a/drivers/gpu/drm/tiny/repaper.c > +++ b/drivers/gpu/drm/tiny/repaper.c [ ... ] > -static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe, > - struct drm_crtc_state *crtc_state, > - struct drm_plane_state *plane_state) > +static void repaper_crtc_helper_atomic_enable(struct drm_crtc *crtc, > + struct drm_atomic_commit *commit) > { > - struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev); > + struct repaper_epd *epd = drm_to_epd(crtc->dev); [Severity: High] This is a pre-existing issue, but do the CRTC atomic enable/disable and plane atomic update callbacks execute sleeping functions in atomic context? The DRM subsystem explicitly forbids sleeping in CRTC enable/disable and plane update callbacks. However: - repaper_crtc_helper_atomic_enable() calls usleep_range() - repaper_crtc_helper_atomic_disable() calls msleep() - repaper_plane_helper_atomic_update() calls repaper_fb_dirty(), which allocates memory using GFP_KERNEL Since these operations are now wired directly to the standard atomic helpers which run in atomic context (for example, during non-blocking commits), can this cause system instability or deadlocks? [ ... ] > -static void repaper_pipe_update(struct drm_simple_display_pipe *pipe, > - struct drm_plane_state *old_state) > +static void repaper_plane_helper_atomic_update(struct drm_plane *plane, > + struct drm_atomic_commit *commit) > { > - struct drm_plane_state *state = pipe->plane.state; > - struct drm_shadow_plane_state *shadow_plane_state = > to_drm_shadow_plane_state(state); > + struct drm_plane_state *old_pstate = > drm_atomic_get_old_plane_state(commit, plane); > + struct drm_plane_state *pstate = drm_atomic_get_new_plane_state(commit, > plane); > + struct drm_shadow_plane_state *shadow_plane_state = > to_drm_shadow_plane_state(pstate); > struct drm_rect rect; > > - if (!pipe->crtc.state->active) > + if (!pstate->fb) > return; > > - if (drm_atomic_helper_damage_merged(old_state, state, &rect)) > - repaper_fb_dirty(state->fb, shadow_plane_state->data, > + if (drm_atomic_helper_damage_merged(old_pstate, pstate, &rect)) > + repaper_fb_dirty(pstate->fb, shadow_plane_state->data, > &shadow_plane_state->fmtcnv_state); [Severity: High] Can this cause the driver to send SPI framebuffer updates to a powered-off display panel? The previous code checked !pipe->crtc.state->active here to ensure we do not update inactive displays. During a DPMS OFF request, drm_atomic_helper_commit_modeset_disables() runs first and disables the panel by calling repaper_crtc_helper_atomic_disable(). Next, drm_atomic_helper_commit_planes() will still call this update function because active_only is not set by default and pstate->visible can remain true. Without the crtc_state->active check, doesn't this result in writing data to dead hardware via repaper_fb_dirty(), potentially causing SPI timeouts or driver hangs? > } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260716-drm-simple-kms-removal-v2-0-1133a8fc3...@oss.qualcomm.com?part=6
