Thomas Zimmermann <[email protected]> writes: > Hi Javier > > Am 11.06.26 um 12:10 schrieb Javier Martinez Canillas: >> Thomas Zimmermann <[email protected]> writes: >> >> Hello Thomas, >> >>> The mode-setting pipeline can disabled damage clippings for a commit >>> by setting ignore_damage_clips in struct drm_plane_state. The commit >>> will then do a full display update. >>> >>> Test the flag in DCN code and do a full update in DCN code if it has >>> been set. >>> >>> Commit 35ed38d58257 ("drm: Allow drivers to indicate the damage helpers >>> to ignore damage clips") introduced ignore_damage_clips to selectively >>> ignore damage clipping in certain framebuffer changes. This driver does >>> not do that, but DRM's damage iterator will soon rely on the flag. >>> Therefore supporting it here as well make sense for consistency. >>> >>> Signed-off-by: Thomas Zimmermann <[email protected]> >>> Fixes: 35ed38d58257 ("drm: Allow drivers to indicate the damage helpers to >>> ignore damage clips") >> I don't think that a Fixes tag is correct here? Your patch series >> is changing the 'struct drm_plane_state.ignore_damage_clips' and >> the changes make sense, but definitely isn't a fix in my opinion. > > But shouldn't we have added this test in amdgpu and the other drivers as > part of commit 35ed38d58257 ? Sure, these drivers don't use > ignore_damage_clips, but it's still an inconsistency wrt damage
I don't think so, since the original scope of ignore_damage_clips was for DRM driver of virtual devices (namely virtio-gpu and vmwgfx). These do per-buffer uploads instead of per-plane uploads, and so there was a need to force a full plane update if the framebuffer attached to the plane was changed. Your series are now extending the scope of ignore_damage_clips to be used by core helpers and force a full plane update when doing a modeset. This makes sense to me but it wasn't the original intention of the propery and that is why I don't think that should be considered a fix. The only patch that IMO is really a fix for commit 35ed38d58257 is patch #6. Because is true that the plane state ignore_damage_clips was carried over when the state was duplicated. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
