On 2026. július 15., szerda 13:42:38 közép-európai nyári idő sashiko- [email protected] wrote: > 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. --
This is not an issue. DCC is not supported at all on GFX6-7 and is not supported by the display engine on GFX8. The code does not advertise any modifiers with DCC on GFX6-8 either. > > --- 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? DCC is not supported by the display engine on GFX6-8 so there is no DCC validation. > > 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? No, it couldn't. > > [ ... ] > > > @@ -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? The tiling flags are no longer used because they are converted to modifiers. Therefore, removing the tiling flag check is not changing the behavior of the code. As I explained above, DCC is not supported by this HW. Additionally, "identical modifiers but with different DCC pitches" would be illegal for DRM format modifiers. > > 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. How? > > Could this lead to DC hardware being programmed using stale Display Mode > Logger (DML) calculations and watermarks, resulting in display corruption? How?
