Re: [PATCH v3 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 1 potential issue(s) to consider:
- [Low] drm/i915/fbc: redundant full dirty rect assignment is dead code
--

commit b3f8f671dc6c5ad11aa10ae1d17ed6ba529a91c9
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 e8583092de663..1ab583eadfcb3 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c

[ ... ]

> @@ -1583,7 +1633,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);

[Severity: Low]
Is this fallback assignment to *fbc_dirty_rect dead code?

If intel_fbc_can_flip_nuke() returns false here, that same condition causes
__intel_fbc_pre_update() to unconditionally call intel_fbc_deactivate(),
disabling FBC in hardware.

When FBC is subsequently reactivated in __intel_fbc_post_update(), the
hardware naturally fetches the entire plane from scratch to build the initial
compressed framebuffer.

Doesn't this disable/enable cycle bypass the programmed XE3_FBC_DIRTY_RECT
register on the first frame, meaning this assignment never takes effect?

> +	} else if (drm_rect_visible(damage)) {
>  		int y1, y2;
>  
>  		if (plane_state->hw.rotation & DRM_MODE_ROTATE_180) {

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