Re: [PATCH v4 01/10] drm/damage-helper: Do not alter damage clips on modeset, but ignore them

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Drivers using raw damage clip getters bypass the newly introduced `ignore_damage_clips` flag, leading to partial screen updates during full modesets.
--

commit 8480930644dc907381d0145f944fe144e2010b34
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/damage-helper: Do not alter damage clips on modeset, but ignore them

This commit changes the behavior of drm_atomic_helper_check_plane_damage()
to preserve fb_damage_clips during a modeset. Instead of clearing the clips,
it now sets a new ignore_damage_clips flag, which the damage iterator
respects.

> diff --git a/drivers/gpu/drm/drm_damage_helper.c b/drivers/gpu/drm/drm_damage_helper.c
> index 74a7f4252ecff..945fac8dc27b6 100644
> --- a/drivers/gpu/drm/drm_damage_helper.c
> +++ b/drivers/gpu/drm/drm_damage_helper.c
[ ... ]
> @@ -78,10 +78,8 @@ void drm_atomic_helper_check_plane_damage(struct drm_atomic_commit *state,
>  		if (WARN_ON(!crtc_state))
>  			return;
>  
> -		if (drm_atomic_crtc_needs_modeset(crtc_state)) {
> -			drm_property_blob_put(plane_state->fb_damage_clips);
> -			plane_state->fb_damage_clips = NULL;
> -		}
> +		if (drm_atomic_crtc_needs_modeset(crtc_state))
> +			plane_state->ignore_damage_clips = true;
>  	}
>  }

[Severity: High]
Does this change cause a regression for drivers that use the raw damage clip
getters instead of the iterator?

Several drivers call drm_plane_get_damage_clips() and
drm_plane_get_damage_clips_count() directly. These raw getters return the
fb_damage_clips without evaluating the new ignore_damage_clips flag:

drivers/gpu/drm/drm_plane.c:drm_plane_get_damage_clips() {
    ...
    return __drm_plane_get_damage_clips(state);
}

Before this patch, fb_damage_clips was forced to NULL on modeset, which
caused drivers to correctly fall back to a full frame update. Now they will
see num_clips > 0 and attempt a partial screen update during a full modeset.

For example, in amdgpu, the full frame update fallback is skipped:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:fill_dc_dirty_rects() {
    ...
    num_clips = drm_plane_get_damage_clips_count(new_plane_state);
    clips = drm_plane_get_damage_clips(new_plane_state);
    ...
}

Similarly in vboxvideo:

drivers/gpu/drm/vboxvideo/vbox_mode.c:vbox_primary_atomic_update() {
    ...
    clips = drm_plane_get_damage_clips(new_state);
    num_clips = drm_plane_get_damage_clips_count(new_state);
    ...
}

And in vmwgfx:

drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c:vmw_ldu_primary_plane_atomic_update() {
    ...
    struct drm_mode_rect *damage_rects = drm_plane_get_damage_clips(new_state);
    u32 rect_count = drm_plane_get_damage_clips_count(new_state);
    ...
}

Should the raw getters be updated to check ignore_damage_clips, or should
these drivers be transitioned to use the damage iterator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530185716.65688-1-tzimmermann@suse.de?part=1