Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Incomplete implementation of block format support, contradicting the commit message's claim of making the logic "future proof with respect to format block size additions". --
commit 530a334e99c89cda78ed6306c7a7d6188cc1bf65 Author: Chen-Yu Tsai <[email protected]> drm/sysfb: Use drm_format_info_min_pitch() to calculate line clear length This commit updates drm_sysfb_plane_helper_atomic_disable() to use drm_format_info_min_pitch() instead of format->cpp[] to calculate the length of each line in the buffer to clear. This change is intended to transparently handle the migration to format->chars_per_block[] and be future-proof for format block size additions. > diff --git a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > --- a/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > +++ b/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c > @@ -396,7 +396,7 @@ void drm_sysfb_plane_helper_atomic_disable(struct > drm_plane *plane, > offset = drm_fb_clip_offset(dst_pitch, dst_format, &dst_clip); > for (i = 0; i < lines; ++i) > iosys_map_memset(&sysfb->fb_addr, offset + dst_pitch * i, 0, > - linepixels * dst_format->cpp[0]); > + drm_format_info_min_pitch(dst_format, 0, > linepixels)); [Severity: Medium] The commit message claims this change makes the logic "future proof with respect to format block size additions", but is this implementation complete for block formats? For block formats where block_height > 1, lines represents the number of pixel rows, but dst_pitch represents bytes per block row. Because the loop iterates lines times and advances the pointer by dst_pitch each iteration, wouldn't this advance the memory pointer block_height times faster than the physical layout? Additionally, it appears drm_fb_clip_offset() still multiplies by format->cpp[0] internally, which is 0 for modern block formats, potentially leading to an incorrect starting offset. Although sysfb currently only exposes linear formats from firmware (meaning block formats cannot reach this path today), could this cause an out-of-bounds memory write and incorrect X-axis offset calculations if block formats were ever used with sysfb in the future? > > drm_dev_exit(idx); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
