Re: [PATCH v3 2/3] drm/i915/fbc: fbc_dirty_rect restrictions and logging

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/i915/fbc: invalid FBC dirty rect for 180-degree rotation when y_offset > 0
- [Medium] drm/i915/fbc: spurious debug log spam on 180-degree rotated planes
--

commit e741ad7c090c4e479fc0c403a1f27d2b43fe1e44
Author: Charlton Lin <charlton.lin@intel.com>

drm/i915/fbc: fbc_dirty_rect restrictions and logging

Clamp the FBC dirty rectangle coordinates to the valid frame buffer
range [y_offset, y_end] to prevent out-of-bounds coordinates, and
log a debug message when clamping occurs.

> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 0f841e5b42826..e8583092de663 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c

[ ... ]

> @@ -1580,11 +1583,41 @@ __intel_fbc_prepare_dirty_rect(const struct intel_plane_state *plane_state,
>  		return;
>  	}
>  
> -	if (drm_rect_visible(damage))
> -		*fbc_dirty_rect = *damage;
> -	else
> +	if (drm_rect_visible(damage)) {
> +		int y1, y2;
> +
> +		if (plane_state->hw.rotation & DRM_MODE_ROTATE_180) {
> +			/* Under 180 degree rotation, coordinate system is inverted */
> +			int inv_y1 = height - damage->y2;
> +			int inv_y2 = height - damage->y1;
> +
> +			y1 = clamp(y_offset + inv_y1, y_offset, y_end);
> +			y2 = clamp(y_offset + inv_y2, y_offset, y_end);

[Severity: High]
Does this math work correctly when y_offset is greater than 0?

Since damage coordinates are in absolute framebuffer space and bounded by
[y_offset, y_end], damage->y2 can be larger than height when y_offset > 0.
This would cause inv_y1 to become negative. 

Adding this negative value to y_offset makes the result less than y_offset,
causing the clamp() to always collapse the dirty rect coordinate to y_offset.
Will this cause screen update failures for rotated planes with a source crop?

> +		} else {
> +			y1 = clamp(damage->y1, y_offset, y_end);
> +			y2 = clamp(damage->y2, y_offset, y_end);
> +		}
> +
> +		/*
> +		 * Clamp dirty rect to the valid FB range [y_offset, y_end].
> +		 * Per Bspec:
> +		 *   start_line >= y_offset
> +		 *   end_line <= y_offset + plane_height
> +		 */
> +		if (y1 != damage->y1 || y2 != damage->y2)

[Severity: Medium]
Will this condition generate false positive log spam when the plane has
180-degree rotation?

For a rotated plane, y1 and y2 are computed as inverted coordinates, but
this check compares them directly against the original non-inverted damage
coordinates. Since inversion naturally changes the coordinate values,
won't this almost always evaluate to true and falsely log that clamping
occurred?

> +			drm_dbg_kms(display->drm,
> +				    "[PLANE:%d:%s] FBC dirty rect out of range: y1=%d y2=%d clamped to y1=%d y2=%d (y_offset=%d y_end=%d)\n",
> +				    plane->base.base.id, plane->base.name,
> +				    damage->y1, damage->y2, y1, y2, y_offset, y_end);

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