Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The logic in `should_reset_plane` fails to trigger a reset for all planes on a CRTC when another plane's color management changes via the new DRM Color Pipeline API, leading to plane z-order corruption. --
commit 52aefc9e9f5291e7e9724eb87eff29b790339396 Author: Melissa Wen <[email protected]> drm/amd/display: use plane color_mgmt_changed to track colorop changes Ensure the driver tracks changes in any colorop property of a plane color pipeline by using the same mechanism of CRTC color management and update plane color blocks when any colorop property changes. > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index 8f80420e9f0b8..56ba9bf40ed9f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c [ ... ] > @@ -12208,6 +12208,10 @@ static bool should_reset_plane(struct > drm_atomic_state *state, > if (new_crtc_state->color_mgmt_changed) > return true; > > + /* Plane color pipeline or its colorop changes. */ > + if (new_plane_state->color_mgmt_changed) > + return true; > + > /* > * On zpos change, planes need to be reordered by removing and re-adding [Severity: High] Does this code need to check color_mgmt_changed for other planes on the CRTC as well? Looking further down in should_reset_plane(), there is a loop over other planes that checks legacy AMD color properties, but it misses checking new_other_state->color_mgmt_changed: for_each_oldnew_plane_in_state(state, other, old_other_state, new_other_state, i) { ... /* HDR/Transfer Function changes. */ if (dm_old_other_state->degamma_tf != dm_new_other_state->degamma_tf || ... If Plane A has color_mgmt_changed == true, should_reset_plane() correctly returns true and Plane A gets reset. However, when should_reset_plane() is evaluated for Plane B on the same CRTC, it iterates over the other planes (including Plane A). Since the loop does not check Plane A's color_mgmt_changed state, it could return false for Plane B. Could this cause only some planes on the CRTC to be removed and re-added, resulting in a scrambled hardware z-order or pipeline acquisition errors? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
