Re: [PATCH 31/70] drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] appletbdrm_flush_damage constructs malformed USB packets and uses mismatched coordinate spaces for clipping.
--

--- Patch [31]: [PATCH 31/70] drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state ---
commit c5a19cb22088517bba7b18adab2a528f848a790f
Author: Maxime Ripard <[email protected]>

drm/tiny: appletbdrm: Switch to drm_atomic_helper_crtc_create_state

The appletbdrm crtc implementation uses the deprecated
drm_atomic_helper_crtc_reset() as its reset hook.

Switch to drm_atomic_helper_crtc_create_state() instead.

> diff --git a/drivers/gpu/drm/tiny/appletbdrm.c b/drivers/gpu/drm/tiny/appletbdrm.c
> index cdd35af498920..94bad6dc720ef 100644
> --- a/drivers/gpu/drm/tiny/appletbdrm.c
> +++ b/drivers/gpu/drm/tiny/appletbdrm.c
> @@ -598,7 +598,7 @@ static const struct drm_crtc_helper_funcs appletbdrm_crtc_helper_funcs = {
>  };
>  
>  static const struct drm_crtc_funcs appletbdrm_crtc_funcs = {
> -	.reset = drm_atomic_helper_crtc_reset,
> +	.atomic_create_state = drm_atomic_helper_crtc_create_state,
>  	.destroy = drm_crtc_cleanup,
>  	.set_config = drm_atomic_helper_set_config,
>  	.page_flip = drm_atomic_helper_page_flip,

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
appletbdrm_flush_damage() in this same file.

When damage (in framebuffer coordinates) is intersected with state->dst (in
CRTC coordinates), could this produce incorrect clipping results if the plane
is not placed at (0,0)?

Also, because buf_size is calculated before the intersection, but the frame
dimensions use the mutated damage:

drivers/gpu/drm/tiny/appletbdrm.c:appletbdrm_flush_damage() {
	...
		u32 buf_size = rect_size(&damage);

		if (!drm_rect_intersect(&dst_clip, &damage))
			continue;

		/* ... */
		frame->width = cpu_to_le16(drm_rect_height(&damage));
		frame->height = cpu_to_le16(drm_rect_width(&damage));
		frame->buf_size = cpu_to_le32(buf_size);
	...
}

If the damage rectangle is partially outside the destination bounds, will
this cause buf_size to be larger than the actual frame dimensions? Does this
lead to malformed USB frames with uninitialized memory gaps before the footer,
violating the hardware USB protocol?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=31