Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Accessor functions `drm_plane_get_damage_clips()` and `drm_plane_get_damage_clips_count()` return outdated partial clips instead of forcing a full update when `ignore_damage_clips` is set, causing drivers to skip full plane updates during modesets and `src` coordinate changes. --
commit 18363d38b23996b07f12d04823d811d72b0f24d4 Author: Thomas Zimmermann <[email protected]> drm/damage-helper: Test src coord in drm_atomic_helper_check_plane_damage() This commit updates drm_atomic_helper_check_plane_damage() to evaluate if the source coordinates change across atomic commits. If they do, it sets the ignore_damage_clips flag in the plane state to force a full update, and removes this check from drm_atomic_helper_damage_iter_init(). > diff --git a/drivers/gpu/drm/drm_damage_helper.c > b/drivers/gpu/drm/drm_damage_helper.c > index 945fac8dc27b6..f492a59edbeb5 100644 > --- a/drivers/gpu/drm/drm_damage_helper.c > +++ b/drivers/gpu/drm/drm_damage_helper.c [ ... ] > @@ -67,19 +68,23 @@ static void convert_clip_rect_to_rect(const struct > drm_clip_rect *src, > * &drm_plane_state.src as damage. > */ > void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state, > - struct drm_plane_state *plane_state) > + const struct drm_plane_state > *old_plane_state, > + struct drm_plane_state > *new_plane_state) > { > struct drm_crtc_state *crtc_state; > > - if (plane_state->crtc) { > + if (!drm_rect_equals(&new_plane_state->src, &old_plane_state->src)) > + new_plane_state->ignore_damage_clips = true; > + [Severity: High] Will this cause display corruption in drivers that manually read the damage clips? The accessors drm_plane_get_damage_clips_count() and __drm_plane_get_damage_clips() in drivers/gpu/drm/drm_plane.c do not check the ignore_damage_clips flag. If a driver calls them directly, such as AMDGPU in fill_dc_dirty_rects(): num_clips = drm_plane_get_damage_clips_count(new_plane_state); clips = drm_plane_get_damage_clips(new_plane_state); won't it receive the stale or partial user-provided damage clips instead of performing a full update when the src coordinates change or during a full modeset? > + if (new_plane_state->crtc) { > crtc_state = drm_atomic_get_new_crtc_state(state, > - plane_state->crtc); > + > new_plane_state->crtc); > > if (WARN_ON(!crtc_state)) > return; > > if (drm_atomic_crtc_needs_modeset(crtc_state)) > - plane_state->ignore_damage_clips = true; > + new_plane_state->ignore_damage_clips = true; > } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
