Re: [PATCH 4/4] drm/amd/display: Don't use tiling flags anymore

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. július 15., szerda 13:42:38 közép-európai nyári idő sashiko-
bot@kernel.org 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 <timur.kristof@gmail.com>
> 
> 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?