Re: [PATCH 25/32] drm/i915/xe3p_lpd: Add FBC support for FP16 formats

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 15 Oct 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> From: Vinod Govindapillai <vinod.govindapillai@intel.com>
>
> Add supported FP16 formats for FBC. FBC can be enabled with FP16 formats
> only when plane pixel normalizer block is enabled.
>
> Bspec: 6881, 69863, 68904
> Cc: Shekhar Chauhan <shekhar.chauhan@intel.com>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 37 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_fbc.h |  1 +
>  2 files changed, 38 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 75c78bef54f2..715a9acabe89 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -64,6 +64,7 @@
>  #include "intel_fbc.h"
>  #include "intel_fbc_regs.h"
>  #include "intel_frontbuffer.h"
> +#include "skl_universal_plane_regs.h"
>  
>  #define for_each_fbc_id(__display, __fbc_id) \
>  	for ((__fbc_id) = INTEL_FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) \
> @@ -154,6 +155,8 @@ static unsigned int intel_fbc_cfb_cpp(const struct intel_plane_state *plane_stat
>  	case DRM_FORMAT_XBGR16161616:
>  	case DRM_FORMAT_ARGB16161616:
>  	case DRM_FORMAT_ABGR16161616:
> +	case DRM_FORMAT_ARGB16161616F:
> +	case DRM_FORMAT_ABGR16161616F:
>  		return 8;
>  	default:
>  		return 4;
> @@ -696,6 +699,30 @@ static void skl_fbc_program_cfb_stride(struct intel_fbc *fbc)
>  		     CHICKEN_FBC_STRIDE_MASK, val);
>  }
>  
> +static bool
> +xe3p_lpd_fbc_is_fp16_format(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;
> +	}
> +}
> +
> +bool
> +intel_fbc_is_fp16_format_supported(const struct intel_plane_state *plane_state)
> +{
> +	struct intel_display *display = to_intel_display(plane_state);
> +
> +	if (DISPLAY_VER(display) >= 35)
> +		return xe3p_lpd_fbc_is_fp16_format(plane_state);
> +
> +	return false;
> +}
>  static u32 ivb_dpfc_ctl(struct intel_fbc *fbc)
>  {
>  	struct intel_display *display = fbc->display;
> @@ -811,6 +838,8 @@ static void intel_fbc_nuke(struct intel_fbc *fbc)
>  static void intel_fbc_activate(struct intel_fbc *fbc)
>  {
>  	struct intel_display *display = fbc->display;
> +	struct intel_plane *plane = fbc->state.plane;
> +	struct intel_plane_state *plane_state = to_intel_plane_state(plane->base.state);
>  
>  	lockdep_assert_held(&fbc->lock);
>  
> @@ -823,6 +852,11 @@ static void intel_fbc_activate(struct intel_fbc *fbc)
>  	 */
>  	drm_WARN_ON(display->drm, fbc->active && HAS_FBC_DIRTY_RECT(display));
>  
> +	drm_WARN_ON(display->drm,
> +		    DISPLAY_VER(display) >= 35 &&
> +		    xe3p_lpd_fbc_is_fp16_format(plane_state) &&
> +		    (plane_state->pixel_normalizer & PLANE_PIXEL_NORMALIZE_ENABLE) == 0);

With the software state being logical, this part here wouldn't have to
do *register* level decoding on the software state. Now the physical and
logical states are conflated.

> +
>  	intel_fbc_hw_activate(fbc);
>  	intel_fbc_nuke(fbc);
>  
> @@ -1140,6 +1174,9 @@ static bool xe3p_lpd_fbc_pixel_format_is_valid(const struct intel_plane_state *p
>  {
>  	const struct drm_framebuffer *fb = plane_state->hw.fb;
>  
> +	if (xe3p_lpd_fbc_is_fp16_format(plane_state))
> +		return true;
> +
>  	switch (fb->format->format) {
>  	case DRM_FORMAT_XRGB8888:
>  	case DRM_FORMAT_XBGR8888:
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
> index 0e715cb6b4e6..e14dc359ecf5 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.h
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.h
> @@ -52,5 +52,6 @@ void intel_fbc_prepare_dirty_rect(struct intel_atomic_state *state,
>  				  struct intel_crtc *crtc);
>  void intel_fbc_dirty_rect_update_noarm(struct intel_dsb *dsb,
>  				       struct intel_plane *plane);
> +bool intel_fbc_is_fp16_format_supported(const struct intel_plane_state *plane_state);
>  
>  #endif /* __INTEL_FBC_H__ */

-- 
Jani Nikula, Intel