Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Build Breakage / Function Prototype Mismatch - [High] Logic Error overwriting correctly calculated color_space with SRGB during commit_tail --
--- Patch [9]: [PATCH v4 09/11] drm/amd/display: Check actual state during commit_tail --- commit 7fa51e59a15d532394f2e3959287098fefb48b8d Author: Harry Wentland <[email protected]> drm/amd/display: Check actual state during commit_tail Previously we checked the plane_state->state's plane_color_pipeline client config when determining whether fill_plane_color_attributes should run, i.e., whether we had a plane color pipeline and should use that code for handling fixed matrix properties, or whether we should look at legacy plane encoding and range. > 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 e7080880b2211..4f07fe5688262 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3008,7 +3008,8 @@ STATIC_IFN_KUNIT bool modereset_required(struct > drm_crtc_state *crtc_state) > EXPORT_IF_KUNIT(modereset_required); > > STATIC_IFN_KUNIT int > -fill_plane_color_attributes(const struct drm_plane_state *plane_state, > +fill_plane_color_attributes(struct drm_atomic_commit *state, > + const struct drm_plane_state *plane_state, > const enum surface_pixel_format format, > enum dc_color_space *color_space) > { [Severity: High] Will this break the build when CONFIG_DRM_AMD_DC_KUNIT_TEST is enabled? The old prototype remains in drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h: int fill_plane_color_attributes(const struct drm_plane_state *plane_state, const enum surface_pixel_format format, enum dc_color_space *color_space); And the KUnit tests still call the function with 3 arguments: drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_test.c:dm_test_fill_color_attr_rgb_format() { ... ret = fill_plane_color_attributes(&plane_state, SURFACE_PIXEL_FORMAT_GRPH_ARGB8888, &color_space); ... } > @@ -3017,7 +3018,7 @@ fill_plane_color_attributes(const struct > drm_plane_state *plane_state, > *color_space = COLOR_SPACE_SRGB; > > /* Ignore properties when DRM_CLIENT_CAP_PLANE_COLOR_PIPELINE is set */ > - if (plane_state->state && plane_state->state->plane_color_pipeline) > + if (state && state->plane_color_pipeline) > return 0; [Severity: High] Could this unintentionally overwrite the correctly calculated color_space with COLOR_SPACE_SRGB during commit_tail? Because *color_space is initialized to COLOR_SPACE_SRGB before the early return, when the color pipeline is active, fill_plane_color_attributes() sets plane_info->color_space to COLOR_SPACE_SRGB. Later, DC core's copy_surface_update_to_plane() unconditionally copies this value to surface->color_space: drivers/gpu/drm/amd/display/dc/core/dc.c:copy_surface_update_to_plane() { ... if (srf_update->plane_info) { surface->color_space = srf_update->plane_info->color_space; ... } Can this clobber the fixed matrix value calculated during atomic check, causing YUV-to-RGB conversion to be bypassed for YUV framebuffers? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
