[PATCH 2/3] drm/i915/display: Restrict flipq to simple single-plane updates

Mika Kahola <[email protected]>
Newsgroups org.freedesktop.lists.intel-xe,org.freedesktop.lists.intel-gfx
Message-ID <[email protected]>
Our flipq eligibility check is too loose: it still lets through
commits that change the active/enabled plane mask, or need extra
pipe/M-N/LRR/watermark programming. Those aren't simple queued plane
updates and have no business going through flipq.

Found this chasing a CRC mismatch in kms_cursor_crc's
cursor-alpha-opaque: a plane enable/disable commit slipped through
flipq ahead of the plane-content commit the CRC actually checks.

Reject flipq for anything touching more than one plane, changing the
active/enabled plane masks, or needing async flip, VRR, PSR, color,
pipe, M/N, LRR or watermark updates. More conservative than strictly
necessary in some cases, but better safe than racy.

Assisted-by: Copilot:claude-sonnet-5
Signed-off-by: Mika Kahola <[email protected]>
---
 drivers/gpu/drm/i915/display/intel_display.c | 53 +++++++++++++++++---
 1 file changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 5cdaa59f005e..2483c031631c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7323,6 +7323,51 @@ static void intel_atomic_prepare_plane_clear_colors(struct intel_atomic_state *s
 	}
 }
 
+static bool intel_flipq_commit_is_eligible(struct intel_atomic_state *state,
+					   struct intel_crtc *crtc)
+{
+	struct intel_display *display = to_intel_display(state);
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(state, crtc);
+	const struct intel_crtc_state *new_crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
+
+	if (!intel_flipq_supported(display))
+		return false;
+
+	if (intel_crtc_needs_modeset(new_crtc_state) ||
+	    intel_crtc_needs_fastset(new_crtc_state) ||
+	    intel_crtc_needs_color_update(new_crtc_state))
+		return false;
+
+	if (!new_crtc_state->update_planes ||
+	    !is_power_of_2(new_crtc_state->update_planes))
+		return false;
+
+	if (new_crtc_state->do_async_flip ||
+	    new_crtc_state->vrr.enable ||
+	    new_crtc_state->has_psr)
+		return false;
+
+	if (old_crtc_state->active_planes != new_crtc_state->active_planes ||
+	    old_crtc_state->enabled_planes != new_crtc_state->enabled_planes)
+		return false;
+
+	/*
+	 * Flipq is only suitable for simple queued single-plane updates
+	 * that do not change plane topology and do not require additional
+	 * pipe/timing/watermark programming.
+	 */
+	if (new_crtc_state->update_pipe ||
+	    new_crtc_state->update_m_n ||
+	    new_crtc_state->update_lrr ||
+	    new_crtc_state->update_wm_pre ||
+	    new_crtc_state->update_wm_post)
+		return false;
+
+	return true;
+}
+
 static void intel_atomic_dsb_prepare(struct intel_atomic_state *state,
 				     struct intel_crtc *crtc)
 {
@@ -7338,13 +7383,7 @@ static void intel_atomic_dsb_prepare(struct intel_atomic_state *state,
 
 	/* FIXME deal with everything */
 	new_crtc_state->use_flipq =
-		intel_flipq_supported(display) &&
-		!new_crtc_state->do_async_flip &&
-		!new_crtc_state->vrr.enable &&
-		!new_crtc_state->has_psr &&
-		!intel_crtc_needs_modeset(new_crtc_state) &&
-		!intel_crtc_needs_fastset(new_crtc_state) &&
-		!intel_crtc_needs_color_update(new_crtc_state);
+		intel_flipq_commit_is_eligible(state, crtc);
 
 	new_crtc_state->use_dsb =
 		!new_crtc_state->use_flipq &&
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.