Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The atomic_disable callback uses the new plane state to determine the screen area to clear, which may be zeroed out and fail to clear the screen. -- commit cdbf0de82b5bf761696ff2cc3c658a3d090a02c8 Author: Chen-Yu Tsai <[email protected]> drm/sysfb: Use iosys_map_memset() to clear buffer This patch updates the sysfb atomic disable routine to use iosys_map_memset() with an offset, rather than directly extracting the virtual address for memory clearing. > diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > index d2de29caf89eb..99b13af4e391c 100644 > --- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > +++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > @@ -375,13 +375,11 @@ void drm_sysfb_plane_helper_atomic_disable(struct > drm_plane *plane, > { > struct drm_device *dev = plane->dev; > struct drm_sysfb_device *sysfb = to_drm_sysfb_device(dev); > - struct iosys_map dst = sysfb->fb_addr; > struct drm_plane_state *plane_state = > drm_atomic_get_new_plane_state(state, plane); > - void __iomem *dst_vmap = dst.vaddr_iomem; /* TODO: Use mapping > abstraction */ > unsigned int dst_pitch = sysfb->fb_pitch; > const struct drm_format_info *dst_format = sysfb->fb_format; > struct drm_rect dst_clip; > - unsigned long lines, linepixels, i; > + unsigned long lines, linepixels, i, offset; > int idx; > > drm_rect_init(&dst_clip, > plane_state->src_x >> 16, plane_state->src_y >> 16, > plane_state->src_w >> 16, plane_state->src_h >> 16); > > lines = drm_rect_height(&dst_clip); > linepixels = drm_rect_width(&dst_clip); [Severity: High] This isn't a bug introduced by this patch, but does using the new plane state to calculate the clear area result in a failure to clear the screen? Because drm_sysfb_plane_helper_atomic_disable() fetches the new plane state above, it uses the state properties after the plane is disabled. If a userspace display server zeroes out SRC_W and SRC_H when setting FB_ID = 0 to disable the plane, both lines and linepixels will evaluate to 0 here. Could this cause the clearing loop to be completely skipped, leaving stale framebuffer data visible on the screen? Should this code use drm_atomic_get_old_plane_state() instead to ensure it always correctly clears the area previously occupied by the plane? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
