public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH v3] drm/i915/display: fix the pixel normalization handling for xe3p_lpd
@ 2026-01-27 11:13 Vinod Govindapillai
  2026-01-27 12:18 ` Ville Syrjälä
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Vinod Govindapillai @ 2026-01-27 11:13 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: vinod.govindapillai, uma.shankar, juha-pekka.heikkila

Pixel normalizer is enabled with normalization factor as 1.0 for
FP16 formats in order to support FBC for those formats in xe3p_lpd.
Previously pixel normalizer gets disabled during the plane disable
routine. But there could be plane format settings without explicitly
calling the plane disable in-between and we could endup keeping the
pixel normalizer enabled for formats which we don't require that.
This is causing crc mismatches in yuv formats and FIFO underruns in
planar formats like NV12.

Fix this by updating the pixel normalizer configuration based on the
pixel formats explicitly during the plane settings arm calls itself
- enable it for FP16 and disable it for other formats in HDR capable
planes. To avoid redundancies in these updates, normalization factor
between old and new plane states are compared before the update. The
function to check validity of the fp16 formats for fbc is now updated
to return the normalization factor as 1.0 in case of fp16 formats and
0 in other cases.

v2: avoid redundant pixel normalization setting updates

v3: moved the normalization factor definition to intel_fbc.c and some
    updates to comments

Fixes: 5298eea7ed20 ("drm/i915/xe3p_lpd: use pixel normalizer for fp16 formats for FBC")
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 .../drm/i915/display/intel_display_device.h   |  1 +
 .../drm/i915/display/intel_display_types.h    |  8 ++
 drivers/gpu/drm/i915/display/intel_fbc.c      | 19 ++++-
 drivers/gpu/drm/i915/display/intel_fbc.h      |  4 +-
 .../drm/i915/display/skl_universal_plane.c    | 82 +++++++++++++++----
 .../i915/display/skl_universal_plane_regs.h   |  1 -
 6 files changed, 92 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 6c74d6b0cc48..126aa1eeeb6d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -175,6 +175,7 @@ struct intel_display_platforms {
 #define HAS_DSC_MST(__display)		(DISPLAY_VER(__display) >= 12 && HAS_DSC(__display))
 #define HAS_FBC(__display)		(DISPLAY_RUNTIME_INFO(__display)->fbc_mask != 0)
 #define HAS_FBC_DIRTY_RECT(__display)	(DISPLAY_VER(__display) >= 30)
+#define HAS_FBC_FP16_FORMATS(__display)	(DISPLAY_VER(__display) >= 35)
 #define HAS_FBC_SYS_CACHE(__display)	(DISPLAY_VER(__display) >= 35 && !(__display)->platform.dgfx)
 #define HAS_FPGA_DBG_UNCLAIMED(__display)	(DISPLAY_INFO(__display)->has_fpga_dbg)
 #define HAS_FW_BLC(__display)		(DISPLAY_VER(__display) >= 3)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e6298279dc89..92bce232b2c5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -686,6 +686,14 @@ struct intel_plane_state {
 	unsigned long flags;
 #define PLANE_HAS_FENCE BIT(0)
 
+	/* xe3p_lpd+ */
+	struct {
+		/* In half-precision floating-point format. 0x3c00 (1.0) for fp16 formats */
+		unsigned int factor;
+		/* update is needed if factor differs between old and new plane states */
+		bool needs_update;
+	} pixel_normalizer;
+
 	struct intel_fb_view view;
 
 	/* for legacy cursor fb unpin */
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 1f3f5237a1c2..f9474e7741c8 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -71,6 +71,9 @@
 
 #define FBC_SYS_CACHE_ID_NONE	I915_MAX_FBCS
 
+/* Pixel normalization factor 1.0 in half-precision floating-point format */
+#define NORM_FACTOR_1_0_IN_HALF_PRECISION_FP		0x3c00
+
 struct intel_fbc_funcs {
 	void (*activate)(struct intel_fbc *fbc);
 	void (*deactivate)(struct intel_fbc *fbc);
@@ -1215,13 +1218,21 @@ static bool xe3p_lpd_fbc_pixel_format_is_valid(const struct intel_plane_state *p
 	}
 }
 
-bool
-intel_fbc_is_enable_pixel_normalizer(const struct intel_plane_state *plane_state)
+unsigned int
+intel_fbc_normalization_factor(const struct intel_plane_state *plane_state)
 {
 	struct intel_display *display = to_intel_display(plane_state);
 
-	return DISPLAY_VER(display) >= 35 &&
-	       xe3p_lpd_fbc_fp16_format_is_valid(plane_state);
+	/*
+	 * In order to have FBC for fp16 formats pixel normalizer block must be
+	 * active. For FP16 formats, use normalization factor as 1.0 and enable
+	 * the block.
+	 */
+	if (HAS_FBC_FP16_FORMATS(display) &&
+	    xe3p_lpd_fbc_fp16_format_is_valid(plane_state))
+		return NORM_FACTOR_1_0_IN_HALF_PRECISION_FP;
+
+	return 0;
 }
 
 static bool pixel_format_is_valid(const struct intel_plane_state *plane_state)
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.h b/drivers/gpu/drm/i915/display/intel_fbc.h
index f0255ddae2b6..b5888e98a659 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.h
+++ b/drivers/gpu/drm/i915/display/intel_fbc.h
@@ -56,7 +56,7 @@ 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_enable_pixel_normalizer(const struct intel_plane_state *plane_state);
+unsigned int
+intel_fbc_normalization_factor(const struct intel_plane_state *plane_state);
 
 #endif /* __INTEL_FBC_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index b3d41705448a..05c227913b8d 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -891,20 +891,49 @@ static void icl_plane_disable_sel_fetch_arm(struct intel_dsb *dsb,
 	intel_de_write_dsb(display, dsb, SEL_FETCH_PLANE_CTL(pipe, plane->id), 0);
 }
 
-static void x3p_lpd_plane_update_pixel_normalizer(struct intel_dsb *dsb,
-						  struct intel_plane *plane,
-						  bool enable)
+static void xe3p_lpd_plane_disable_pixel_normalizer(struct intel_dsb *dsb,
+						    struct intel_plane *plane)
 {
 	struct intel_display *display = to_intel_display(plane);
 	enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(plane->pipe);
-	u32 val;
+	const struct intel_plane_state *plane_state =
+		to_intel_plane_state(plane->base.state);
+
+	if (!HAS_FBC_FP16_FORMATS(display))
+		return;
+
+	if (!skl_plane_has_fbc(display, fbc_id, plane->id))
+		return;
+
+	if (!plane_state->pixel_normalizer.factor)
+		return;
+
+	intel_de_write_dsb(display, dsb,
+			   PLANE_PIXEL_NORMALIZE(plane->pipe, plane->id), 0);
+}
+
+static void xe3p_lpd_plane_update_pixel_normalizer(struct intel_dsb *dsb,
+						   struct intel_plane *plane)
+{
+	struct intel_display *display = to_intel_display(plane);
+	enum intel_fbc_id fbc_id = skl_fbc_id_for_pipe(plane->pipe);
+	const struct intel_plane_state *plane_state =
+		to_intel_plane_state(plane->base.state);
+	u32 val = 0;
+
+	if (!HAS_FBC_FP16_FORMATS(display))
+		return;
 
-	/* Only HDR planes have pixel normalizer and don't matter if no FBC */
+	/* Only HDR planes have pixel normalizer and don't matter if FBC is fused off */
 	if (!skl_plane_has_fbc(display, fbc_id, plane->id))
 		return;
 
-	val = enable ? PLANE_PIXEL_NORMALIZE_NORM_FACTOR(PLANE_PIXEL_NORMALIZE_NORM_FACTOR_1_0) |
-		       PLANE_PIXEL_NORMALIZE_ENABLE : 0;
+	if (!plane_state->pixel_normalizer.needs_update)
+		return;
+
+	if (plane_state->pixel_normalizer.factor)
+		val = PLANE_PIXEL_NORMALIZE_NORM_FACTOR(plane_state->pixel_normalizer.factor) |
+		      PLANE_PIXEL_NORMALIZE_ENABLE;
 
 	intel_de_write_dsb(display, dsb,
 			   PLANE_PIXEL_NORMALIZE(plane->pipe, plane->id), val);
@@ -926,8 +955,7 @@ icl_plane_disable_arm(struct intel_dsb *dsb,
 
 	icl_plane_disable_sel_fetch_arm(dsb, plane, crtc_state);
 
-	if (DISPLAY_VER(display) >= 35)
-		x3p_lpd_plane_update_pixel_normalizer(dsb, plane, false);
+	xe3p_lpd_plane_disable_pixel_normalizer(dsb, plane);
 
 	intel_de_write_dsb(display, dsb, PLANE_CTL(pipe, plane_id), 0);
 	intel_de_write_dsb(display, dsb, PLANE_SURF(pipe, plane_id), 0);
@@ -1674,13 +1702,7 @@ icl_plane_update_arm(struct intel_dsb *dsb,
 
 	intel_color_plane_commit_arm(dsb, plane_state);
 
-	/*
-	 * In order to have FBC for fp16 formats pixel normalizer block must be
-	 * active. Check if pixel normalizer block need to be enabled for FBC.
-	 * If needed, use normalization factor as 1.0 and enable the block.
-	 */
-	if (intel_fbc_is_enable_pixel_normalizer(plane_state))
-		x3p_lpd_plane_update_pixel_normalizer(dsb, plane, true);
+	xe3p_lpd_plane_update_pixel_normalizer(dsb, plane);
 
 	/*
 	 * The control register self-arms if the plane was previously
@@ -2350,6 +2372,32 @@ static void clip_damage(struct intel_plane_state *plane_state)
 	drm_rect_intersect(damage, &src);
 }
 
+static void check_pixel_normalizer(struct intel_plane_state *plane_state)
+{
+	struct intel_display *display = to_intel_display(plane_state);
+	struct intel_plane *plane = to_intel_plane(plane_state->uapi.plane);
+	struct intel_atomic_state *state =
+		to_intel_atomic_state(plane_state->uapi.state);
+	const struct intel_plane_state *old_plane_state =
+		intel_atomic_get_old_plane_state(state, plane);
+
+	if (!HAS_FBC_FP16_FORMATS(display))
+		return;
+
+	plane_state->pixel_normalizer.factor =
+		intel_fbc_normalization_factor(plane_state);
+
+	/*
+	 * In case of no old state to compare, better to force update the pixel
+	 * normalizer settings.
+	 */
+	plane_state->pixel_normalizer.needs_update = true;
+	if (old_plane_state && old_plane_state->hw.fb)
+		plane_state->pixel_normalizer.needs_update =
+			plane_state->pixel_normalizer.factor !=
+			intel_fbc_normalization_factor(old_plane_state);
+}
+
 static int skl_plane_check(struct intel_crtc_state *crtc_state,
 			   struct intel_plane_state *plane_state)
 {
@@ -2400,6 +2448,8 @@ static int skl_plane_check(struct intel_crtc_state *crtc_state,
 
 	check_protection(plane_state);
 
+	check_pixel_normalizer(plane_state);
+
 	/* HW only has 8 bits pixel precision, disable plane if invisible */
 	if (!(plane_state->hw.alpha >> 8)) {
 		plane_state->uapi.visible = false;
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 6fd4da9f63cf..651f3557b576 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane_regs.h
@@ -580,6 +580,5 @@
 #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__ */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-01-28 15:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-27 11:13 [PATCH v3] drm/i915/display: fix the pixel normalization handling for xe3p_lpd Vinod Govindapillai
2026-01-27 12:18 ` Ville Syrjälä
2026-01-28  6:06   ` Shankar, Uma
2026-01-28  7:38   ` Govindapillai, Vinod
2026-01-28 15:28     ` Ville Syrjälä
2026-01-27 12:21 ` ✓ i915.CI.BAT: success for drm/i915/display: fix the pixel normalization handling for xe3p_lpd (rev3) Patchwork
2026-01-27 18:04 ` ✗ i915.CI.Full: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox