Re: [PATCH 2/2] drm/sysfb: Use drm_format_info_min_pitch() to calculate line clear length

[sashiko-bot@kernel.org]

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 <wenst@chromium.org>

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/20260903060651.3724163-1-wenst@chromium.org?part=2