Re: [PATCH 26/32] drm/i915/xe3p_lpd: Enable pixel normalizer for fp16 formats for FBC

[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>
>
> There is a hw restriction that we could enable the FBC for FP16
> formats only if the pixel normalization block is enabled. Hence
> enable the pixel normalizer block with normalzation factor as
> 1.0 for the supported FP16 formats to get the FBC enabled. Two
> existing helper function definitions are moved up to avoid the
> forward declarations as part of this patch as well.
>
> Bspec: 69863, 68881
> 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/skl_universal_plane.c | 50 ++++++++++++++--------
>  .../drm/i915/display/skl_universal_plane_regs.h    |  1 +
>  2 files changed, 33 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> index 16a9c141281b..ae1bf6beac95 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
> @@ -486,6 +486,23 @@ static int skl_plane_max_height(const struct drm_framebuffer *fb,
>  	return 4096;
>  }
>  
> +static enum intel_fbc_id skl_fbc_id_for_pipe(enum pipe pipe)
> +{
> +	return pipe - PIPE_A + INTEL_FBC_A;
> +}
> +
> +static bool skl_plane_has_fbc(struct intel_display *display,
> +			      enum intel_fbc_id fbc_id, enum plane_id plane_id)
> +{
> +	if ((DISPLAY_RUNTIME_INFO(display)->fbc_mask & BIT(fbc_id)) == 0)
> +		return false;
> +
> +	if (DISPLAY_VER(display) >= 20)
> +		return icl_is_hdr_plane(display, plane_id);
> +	else
> +		return plane_id == PLANE_1;
> +}
> +
>  static int icl_plane_max_height(const struct drm_framebuffer *fb,
>  				int color_plane,
>  				unsigned int rotation)
> @@ -896,7 +913,21 @@ static void skl_write_plane_wm(struct intel_dsb *dsb,
>  static void
>  xe3p_lpd_plane_check_pixel_normalizer(struct intel_plane_state *plane_state)
>  {
> -	plane_state->pixel_normalizer = 0;
> +	struct intel_display *display = to_intel_display(plane_state);
> +	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
> +	enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(plane->pipe);
> +	u32 reg = 0;
> +
> +	/*
> +	 * To enable FBC for FP16 formats, enable pixel normalizer with
> +	 * normalization factor as 1.0
> +	 */
> +	if (skl_plane_has_fbc(display, fbc_id, plane->id) &&
> +	    intel_fbc_is_fp16_format_supported(plane_state))
> +		reg = PLANE_PIXEL_NORMALIZE_NORM_FACTOR(PLANE_PIXEL_NORMALIZE_NORM_FACTOR_1_0) |
> +		      PLANE_PIXEL_NORMALIZE_ENABLE;

Again, this functions should be about software state, and shouldn't have
to concern itself with the register macros.

The function name still doesn't make sense.

> +
> +	plane_state->pixel_normalizer = reg;
>  }
>  
>  static void
> @@ -2449,23 +2480,6 @@ void icl_link_nv12_planes(struct intel_plane_state *uv_plane_state,
>  	}
>  }
>  
> -static enum intel_fbc_id skl_fbc_id_for_pipe(enum pipe pipe)
> -{
> -	return pipe - PIPE_A + INTEL_FBC_A;
> -}
> -
> -static bool skl_plane_has_fbc(struct intel_display *display,
> -			      enum intel_fbc_id fbc_id, enum plane_id plane_id)
> -{
> -	if ((DISPLAY_RUNTIME_INFO(display)->fbc_mask & BIT(fbc_id)) == 0)
> -		return false;
> -
> -	if (DISPLAY_VER(display) >= 20)
> -		return icl_is_hdr_plane(display, plane_id);
> -	else
> -		return plane_id == PLANE_1;
> -}
> -
>  static struct intel_fbc *skl_plane_fbc(struct intel_display *display,
>  				       enum pipe pipe, enum plane_id plane_id)
>  {
> diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h b/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> index 11c713f9b237..eb25de5d1778 100644
> --- a/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> +++ b/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
> @@ -466,5 +466,6 @@
>  #define   PLANE_PIXEL_NORMALIZE_ENABLE			REG_BIT(31)
>  #define   PLANE_PIXEL_NORMALIZE_NORM_FACTOR_MASK	REG_GENMASK(15, 0)
>  #define   PLANE_PIXEL_NORMALIZE_NORM_FACTOR(val)	REG_FIELD_PREP(PLANE_PIXEL_NORMALIZE_NORM_FACTOR_MASK, (val))
> +#define   PLANE_PIXEL_NORMALIZE_NORM_FACTOR_1_0		0x3c00
>  
>  #endif /* __SKL_UNIVERSAL_PLANE_REGS_H__ */

-- 
Jani Nikula, Intel