Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Removing the `tiling_flags` check in `should_reset_plane` allows framebuffers with different DCC pitches or dimensions (but identical modifiers) to bypass DRM atomic check validation, leading to hardware corruption and stale DML calculations during commit. --
--- Patch [4]: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore --- commit b71fc193d0b1254508d4ef4e5b1e90decbcba27a Author: Timur Kristóf <[email protected]> drm/amd/display: Don't use tiling flags anymore This commit removes all code from amdgpu_dm that dealt with tiling flags, as all supported GPU generations now support DRM format modifiers. The legacy non-DC display code still relies on tiling flags. > 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 c643cc5428577..2f7388c47d02a 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c [ ... ] > @@ -4279,7 +4277,6 @@ static void amdgpu_dm_commit_planes(struct > drm_atomic_state *state, > > fill_dc_plane_info_and_addr( > dm->adev, new_plane_state, > - afb->tiling_flags, > &bundle->plane_infos[planes_count], > &bundle->flip_addrs[planes_count].address, > afb->tmz_surface); [Severity: High] Since the return value of fill_dc_plane_info_and_addr() is ignored here, what happens if DCC validation fails due to missed dimension or pitch changes? Could this result in a zero-filled plane_info structure being sent to the hardware (for example, setting global_alpha to 0) and causing a black screen? [ ... ] > @@ -6248,8 +6245,7 @@ static bool should_reset_plane(struct drm_atomic_state > *state, > new_afb = (struct amdgpu_framebuffer *)new_other_state->fb; > > /* Tiling and DCC changes also require bandwidth updates. */ > - if (old_afb->tiling_flags != new_afb->tiling_flags || > - old_afb->base.modifier != new_afb->base.modifier) > + if (old_afb->base.modifier != new_afb->base.modifier) > return true; > } [Severity: High] By removing the tiling_flags check here in should_reset_plane(), how do we ensure that framebuffers with identical modifiers but different DCC pitches or dimensions still trigger a validation reset? If a user-space client flips between two framebuffers with the same format and source rectangle, but different backing buffer dimensions, it seems this might bypass DRM atomic check validation. Could this lead to DC hardware being programmed using stale Display Mode Logger (DML) calculations and watermarks, resulting in display corruption? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
