Re: [PATCH] drm/i915/fbc: Apply Wa_14028723391

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915/fbc: Wa_14028723391 in intel_fbc_check_plane() disables all FP16 FBC
--

commit 49c7819cacd48f3049b96afafb389df864ddc4bd
Author: Vinod Govindapillai <vinod.govindapillai@intel.com>

drm/i915/fbc: Apply Wa_14028723391

This commit applies Wa_14028723391 to disable FBC on Xe3p_LPD (DISPLAY_VER 35)
for FP16 formats with alpha.

> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 5694d20773f83..80a7ae671179a 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1687,6 +1687,18 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
>  		return 0;
>  	}
>  
> +	/*
> +	 * Wa_14028723391:
> +	 * Fixes: Screen corruption with FBC enabled on a plane with the FP16
> +	 * Workaround: Disable FBC for FP16+Alpha in xe3p_lpd
> +	 */
> +	if (intel_display_wa(display, INTEL_DISPLAY_WA_14028723391) &&
> +	    xe3p_lpd_fbc_fp16_format_is_valid(plane_state) &&
> +	    fb->format->has_alpha) {
> +		plane_state->no_fbc_reason = "Wa_14028723391";
> +		return 0;
> +	}

[Severity: Medium]
Does this check in intel_fbc_check_plane() end up disabling FBC for all FP16
formats on Xe3p_LPD rather than only FP16 formats with alpha, while leaving
the plane pixel normalizer enabled?

Looking at xe3p_lpd_fbc_fp16_format_is_valid() in
drivers/gpu/drm/i915/display/intel_fbc.c:

static bool
xe3p_lpd_fbc_fp16_format_is_valid(const struct intel_plane_state *plane_state)
{
	const struct drm_framebuffer *fb = plane_state->hw.fb;

	switch (fb->format->format) {
	case DRM_FORMAT_ARGB16161616F:
	case DRM_FORMAT_ABGR16161616F:
		return true;
	default:
		return false;
	}
}

Only DRM_FORMAT_ARGB16161616F and DRM_FORMAT_ABGR16161616F are matched, both
of which unconditionally have fb->format->has_alpha == true, while the
non-alpha FP16 formats (DRM_FORMAT_XRGB16161616F and DRM_FORMAT_XBGR16161616F,
which are exposed on HDR planes in icl_hdr_plane_formats[]) return false and
are rejected earlier by pixel_format_is_valid().

As a result, whenever xe3p_lpd_fbc_fp16_format_is_valid(plane_state) is true,
fb->format->has_alpha is always true, so every FP16 format is rejected for FBC
on DISPLAY_VER(display) == 35. In addition, unlike the per-pixel alpha check
immediately following this block:

	if (DISPLAY_VER(display) < 20 &&
	    plane_state->hw.pixel_blend_mode != DRM_MODE_BLEND_PIXEL_NONE &&
	    fb->format->has_alpha)

this check does not inspect plane_state->hw.pixel_blend_mode even when alpha
blending is disabled via DRM_MODE_BLEND_PIXEL_NONE.

At the same time, icl_plane_update_arm() -> pixel_normalizer_value() ->
intel_fbc_need_pixel_normalizer() checks:

bool intel_fbc_need_pixel_normalizer(const struct intel_plane_state *plane_state)
{
	struct intel_display *display = to_intel_display(plane_state);

	if (HAS_PIXEL_NORMALIZER(display) &&
	    xe3p_lpd_fbc_fp16_format_is_valid(plane_state))
		return true;

	return false;
}

Should xe3p_lpd_fbc_fp16_format_is_valid() and
intel_fbc_need_pixel_normalizer() also account for non-alpha FP16 formats and
Wa_14028723391 so that FBC can still be used with non-alpha FP16 formats (or
when pixel_blend_mode is DRM_MODE_BLEND_PIXEL_NONE) and
PLANE_PIXEL_NORMALIZE_ENABLE is not programmed when FBC is disabled by this
workaround?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023120653.194313-1-vinod.govindapillai@intel.com?part=1