From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/amd/display: use plane color_mgmt_changed to track colorop changes Date: Tue, 26 May 2026 07:18:47 +1000 Message-ID: In-Reply-To: <20260525100524.304263-5-mwen@igalia.com> References: <20260525100524.304263-1-mwen@igalia.com> <20260525100524.304263-5-mwen@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review The AMD driver consumer of the new flag. Two changes: 1. In `amdgpu_dm_commit_planes` (line ~10201): ```c if (new_pcrtc_state->color_mgmt_changed || new_plane_state->color_mgmt_chan= ged) { ``` Previously only CRTC color_mgmt_changed triggered updating the surface colo= r blocks (gamma, transfer func, gamut remap, shaper LUT, 3D LUT). Now plane= -level colorop changes also trigger this. This is the actual bug fix for ga= mescope night mode. 2. In `should_reset_plane` (line ~12027): ```c if (new_plane_state->color_mgmt_changed) return true; ``` Forces a plane reset (remove and re-add to DC state) when plane color manag= ement changes. This parallels the existing CRTC `color_mgmt_changed` check = just above it. **Question worth considering**: `should_reset_plane` returning `true` trigg= ers a full plane remove/re-add in DC. For pure colorop property changes (e.= g., just changing an interpolation mode), this may be heavier than necessar= y =E2=80=94 the `amdgpu_dm_commit_planes` surface update path might be suff= icient without a full reset. However, this matches the existing behavior fo= r CRTC color management changes and is the safe approach. If performance is= a concern for high-frequency colorop updates, this could be revisited late= r, but correctness is ensured. No bugs found. --- **Summary**: The series is correct and ready to merge. The change tracking = logic properly detects actual value changes (not just property sets), the s= tate management for interpolation fields follows established patterns, and = the AMD driver correctly consumes the new flag. All patches have appropriat= e reviewed-by tags. --- Generated by Claude Code Patch Reviewer