Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] drm: sleeping in atomic context during plane atomic update - [High] drm: VBLANK IRQ storm due to unhandled interrupt on driver unbind --
commit 32feffcdec94761d97377d2756a8face5aaf6281 Author: Leander Kieweg <[email protected]> drm/glanda: Add initial DRM driver for GlandaGPU Introduce the core DRM/KMS driver for GlandaGPU. This driver supports basic modesetting, atomic updates via shadow plane helpers, and optional QEMU PCI probing alongside the platform driver. > diff --git a/drivers/gpu/drm/tiny/glandagpu.c > b/drivers/gpu/drm/tiny/glandagpu.c > new file mode 100644 > index 0000000000000..38fa5809f31e0 > --- /dev/null > +++ b/drivers/gpu/drm/tiny/glandagpu.c [ ... ] > +static void glanda_blit_rect(struct glanda_device *gdev, > + const struct drm_rect *dst_clip, > + const struct iosys_map *src, > + struct drm_framebuffer *fb, > + int dst_off_x, int dst_off_y, > + struct drm_format_conv_state *fmtcnv_state) > +{ > + unsigned int src_pitch = fb->pitches[0]; > + unsigned int width = drm_rect_width(dst_clip); > + unsigned int height = drm_rect_height(dst_clip); > + unsigned int x, y; > + size_t len = width * sizeof(u32); > + u32 *sbuf; > + > + sbuf = drm_format_conv_state_reserve(fmtcnv_state, len, GFP_KERNEL); [Severity: High] Does this code sleep in atomic context? The glanda_blit_rect() function is called from the glanda_plane_atomic_update() callback, which runs in atomic context where sleeping is forbidden. Using GFP_KERNEL here can cause kernel panics or deadlocks. [ ... ] > +static void glanda_plane_atomic_update(struct drm_plane *plane, > + struct drm_atomic_commit *state) > +{ > + struct drm_plane_state *old_state = > drm_atomic_get_old_plane_state(state, plane); > + struct drm_plane_state *new_state = > drm_atomic_get_new_plane_state(state, plane); > + struct drm_shadow_plane_state *shadow_state = > to_drm_shadow_plane_state(new_state); > + struct drm_rect vram_clip = DRM_RECT_INIT(0, 0, GLANDA_WIDTH, > GLANDA_HEIGHT); > + struct glanda_device *gdev = to_glanda(plane->dev); > + struct drm_framebuffer *fb = new_state->fb; > + struct drm_atomic_helper_damage_iter iter; > + struct drm_rect damage; > + int dst_off_x, dst_off_y; > + int ret, idx; > + > + ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE); [Severity: High] Can this sleep while waiting for hardware fences? Since glanda_plane_atomic_update() is executed in atomic context, calling drm_gem_fb_begin_cpu_access() (and the corresponding end_cpu_access) is unsafe here. [ ... ] > +static void glanda_drm_fini(struct glanda_device *gdev) > +{ > + drm_dev_unplug(&gdev->drm); > + drm_atomic_helper_shutdown(&gdev->drm); > +} [Severity: High] Does this teardown order cause an unhandled IRQ storm on unbind? If drm_dev_unplug() is called before drm_atomic_helper_shutdown(), the device is marked as unplugged. When the shutdown helper tries to disable the CRTC, it calls glanda_crtc_disable_vblank(): static void glanda_crtc_disable_vblank(struct drm_crtc *crtc) { ... if (!drm_dev_enter(crtc->dev, &idx)) return; ... } The drm_dev_enter() check will fail because the device is already unplugged, causing the hardware write to disable the VBLANK interrupt to be skipped. The interrupt would remain active in hardware while the driver frees the IRQ handler. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
