Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Changes to DRM plane color pipeline properties (drm_colorop) do not trigger a DC plane state reset, causing the hardware to use stale color space and colorop properties. --
--- Patch [10]: [PATCH v4 10/11] drm/amd/display: Set color_space to plane_infos --- commit d69c24f2afbe9ab20f934e3e3173c2c3d2c7b892 Author: Harry Wentland <[email protected]> drm/amd/display: Set color_space to plane_infos This commit assigns the color space from the dc_plane directly to the plane_infos array in the update bundle during plane commits. This replaces the legacy behavior that handled COLOR_ENCODING and COLOR_RANGE inside fill_plane_color_attributes. > 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 4f07fe5688262..779f98e8d2d7d 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4288,6 +4288,8 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_commit *state, > &bundle->flip_addrs[planes_count].address, > afb->tmz_surface); > > + bundle->plane_infos[planes_count].color_space = > dc_plane->color_space; > + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [Severity: High] Could this assignment use a stale color_space value? When userspace dynamically updates a property on an active drm_colorop pipeline, DRM core sets new_plane_state->color_mgmt_changed = true. However, should_reset_plane() incorrectly omits checking new_other_state->color_mgmt_changed or iterating over drm_colorop_state property changes: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c:should_reset_plane() { ... /* Colorspace changes. */ if (old_other_state->color_range != new_other_state->color_range || old_other_state->color_encoding != new_other_state->color_encoding) return true; ... } Because should_reset_plane() returns false when only colorop pipeline properties are modified, dm_update_plane_state() skips recreating the dc_plane_state and bypasses amdgpu_dm_update_plane_color_mgmt(). As a result, stale color space parameters are passed to DC here. Should should_reset_plane() be updated to properly check for color management changes so the hardware color pipeline does not retain stale configuration values? > drm_dbg_state(state->dev, "plane: id=%d dcc_en=%d\n", > new_plane_state->plane->index, > bundle->plane_infos[planes_count].dcc.enable); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
