Re: [PATCH v4 3/3] drm/i915/fbc: nuke CFB if Plane setting (except for surf addr) changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915/fbc: Severe performance regression due to overloaded FBC nuke checks
- [High] drm/i915/fbc: Missing state checks cause visual corruption with FBC dirty rect
--

commit 224f5233b9e570c9b8787cab16713f38059127bf
Author: Austin Hu <austin.hu@intel.com>

drm/i915/fbc: nuke CFB if Plane setting (except for surf addr) changes

When FBC Dirty Rectangle mode is enabled, any non-PLANE_SURF register update
requires fetching the full plane frame buffer from memory to re-compress and
update the CFB in stolen memory. This commit checks plane state attributes
during atomic commits and forces a full CFB nuke if any setting other than
the surface address changes.

> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 3915dea9edb1e..0d3b25e7a9fc1 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
[ ... ]
> @@ -1548,14 +1549,63 @@ static bool intel_fbc_can_flip_nuke(struct intel_atomic_state *state,
>  	    intel_fbc_override_cfb_stride(new_plane_state))
>  		return false;
>  
> +	if (!HAS_FBC_DIRTY_RECT(display))
> +		return true;
> +
> +	/*
> +	 * From BSpec about "FBC Dirty Rectangle", when Dirty Rectangle mode is
> +	 * active, partial updates only apply to surface address changes. Any other
> +	 * plane state modification requires Plane to fetch full-plane pixel data
> +	 * from memory to re-compress and update the entire CFB in stolen memory.
> +	 * Once fully re-compressed, subsequent atomic commits go ahead with FBC
> +	 * dirty rectangle updates for smooth visual updates.
> +	 *
> +	 * So check other Plane attribute changed except for its surface address by
> +	 * referring to intel_async_flip_check_hw() which also checks async flip.
> +	 */
> +
> +	/* Includes pixel_blend_mode, color_encoding & color_range checking. */
> +	if (old_plane_state->color_ctl != new_plane_state->color_ctl)
> +		return false;

[Severity: High]
Does this code miss checks for other plane properties, potentially causing
visual corruption?

The commit dictates that any non-PLANE_SURF register update requires a full CFB
nuke in FBC Dirty Rectangle mode. However, intel_fbc_can_flip_nuke() misses
checks for plane_state->ccval (Clear Color Value), plane_state->ctl (Plane
Control), and plane_state->scaler_id.

If an unprivileged DRM client updates these properties (e.g., changing the
clear color via fast clear), the function returns true, skipping the full
dirty rect expansion. The hardware would then fail to re-compress the new
pixel data, leaving artifacts on the display.

> +
> +	if (!drm_rect_equals(&old_plane_state->uapi.src,
> +			     &new_plane_state->uapi.src) ||
> +	    !drm_rect_equals(&old_plane_state->uapi.dst,
> +			     &new_plane_state->uapi.dst))
> +		return false;
[ ... ]
> @@ -1573,7 +1623,15 @@ __intel_fbc_prepare_dirty_rect(const struct intel_plane_state *plane_state,
>  		return;
>  	}
>  
> -	if (drm_rect_visible(damage)) {
> +	if (!intel_fbc_can_flip_nuke(state, crtc, plane)) {
> +		if (HAS_FBC_DIRTY_RECT(display))
> +			drm_dbg_kms(display->drm,
> +				    "[PLANE:%d:%s] Non surf addr changed for FBC DIRTY RECT\n",
> +				    plane->base.base.id, plane->base.name);
> +
> +		/* compress the entire region due to non PLANE_SURF updating. */
> +		*fbc_dirty_rect = DRM_RECT_INIT(0, y_offset, width, height);
> +	} else if (drm_rect_visible(damage)) {

[Severity: High]
Could reusing intel_fbc_can_flip_nuke() here cause a severe performance
degradation due to unnecessary vblank stalls during atomic plane updates?

When FBC Dirty Rect mode is enabled (Display Version 30+), changing plane
properties (like moving a window, modifying alpha, or color bounds) causes
intel_fbc_can_flip_nuke() to return false.

This will cause intel_fbc_deactivate() to be triggered in
__intel_fbc_pre_update() (which checks intel_fbc_can_flip_nuke()), which sets
need_vblank_wait = true. 

It seems the dirty rect expansion is inadvertently coupled with the FBC
deactivation logic. This is reachable by any userspace application performing
non-surface address plane updates (like window dragging), leading to constant
vblank stalls and visual stuttering.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925173658.1042048-1-austin.hu@intel.com?part=3