On Wed, 15 Oct 2025, Gustavo Sousa <[email protected]> wrote:
> From: Vinod Govindapillai <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Vinod Govindapillai <[email protected]>
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
> 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