Blob Blame History Raw
From 6484e21e5f2557f773a62ad9e7825896d26afce5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
Date: Mon, 14 Feb 2022 12:55:31 +0200
Subject: drm/i915: Fix the async flip wm0/ddb optimization
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Alt-commit: 2e08437160d1e8f2cd3f0d56d59e74423602116e
Git-commit: 5c8107dc9a9f33a88a380aea79be564597d00663
Patch-mainline: v5.18-rc1
References: jsc#PED-1166 jsc#PED-1168 jsc#PED-1170 jsc#PED-1218 jsc#PED-1220 jsc#PED-1222 jsc#PED-1223 jsc#PED-1225

The current implementation of the async flip wm0/ddb optimization
does not work at all. The biggest problem is that we skip the
whole intel_pipe_update_{start,end}() dance and thus never actually
complete the commit that is trying to do the wm/ddb change.

To fix this we need to move the do_async_flip flag to the crtc
state since we handle commits per-pipe, not per-plane.

Also since all planes can now be included in the first/last
"async flip" (which gets converted to a sync flip to do the
wm/ddb mangling) we need to be more careful when checking if
the plane state is async flip comptatible. Only planes doing
the async flip should be checked and other planes are perfectly
fine not adhereing to any async flip related limitations.

However for subsequent commits which are actually going do the
async flip in hardware we want to make sure no other planes
are in the state. That should never happen assuming we did our
job correctly, so we'll toss in a WARN to make sure we catch
any bugs here.

Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Fixes: c3639f3be480 ("drm/i915: Use wm0 only during async flips for DG2")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20220214105532.13049-4-ville.syrjala@linux.intel.com
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
(cherry picked from commit 2e08437160d1e8f2cd3f0d56d59e74423602116e)
Signed-off-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Acked-by: Patrik Jakobsson <pjakobsson@suse.de>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |  1 +
 .../gpu/drm/i915/display/intel_atomic_plane.c |  7 ++--
 drivers/gpu/drm/i915/display/intel_crtc.c     |  4 +-
 drivers/gpu/drm/i915/display/intel_display.c  | 38 +++++++++++--------
 .../drm/i915/display/intel_display_types.h    |  6 +--
 5 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index e0667d163266..40da7910f845 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -262,6 +262,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->preload_luts = false;
 	crtc_state->inherited = false;
 	crtc_state->wm.need_postvbl_update = false;
+	crtc_state->do_async_flip = false;
 	crtc_state->fb_bits = 0;
 	crtc_state->update_planes = 0;
 	crtc_state->dsb = NULL;
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index c53aa6a4c7a0..5712688232fb 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -110,7 +110,6 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 	intel_state->ggtt_vma = NULL;
 	intel_state->dpt_vma = NULL;
 	intel_state->flags = 0;
-	intel_state->do_async_flip = false;
 
 	/* add reference to fb */
 	if (intel_state->hw.fb)
@@ -506,7 +505,7 @@ static int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_cr
 		new_crtc_state->disable_lp_wm = true;
 
 	if (intel_plane_do_async_flip(plane, old_crtc_state, new_crtc_state))
-		new_plane_state->do_async_flip = true;
+		new_crtc_state->do_async_flip = true;
 
 	return 0;
 }
@@ -678,7 +677,7 @@ void intel_plane_update_arm(struct intel_plane *plane,
 
 	trace_intel_plane_update_arm(&plane->base, crtc);
 
-	if (plane_state->do_async_flip)
+	if (crtc_state->do_async_flip && plane->async_flip)
 		plane->async_flip(plane, crtc_state, plane_state, true);
 	else
 		plane->update_arm(plane, crtc_state, plane_state);
@@ -703,7 +702,7 @@ void intel_crtc_planes_update_noarm(struct intel_atomic_state *state,
 	struct intel_plane *plane;
 	int i;
 
-	if (new_crtc_state->uapi.async_flip)
+	if (new_crtc_state->do_async_flip)
 		return;
 
 	/*
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 08ee3e17ee5c..65827481c1b1 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -485,7 +485,7 @@ void intel_pipe_update_start(struct intel_crtc_state *new_crtc_state)
 		intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
 	DEFINE_WAIT(wait);
 
-	if (new_crtc_state->uapi.async_flip)
+	if (new_crtc_state->do_async_flip)
 		return;
 
 	if (intel_crtc_needs_vblank_work(new_crtc_state))
@@ -630,7 +630,7 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 	ktime_t end_vbl_time = ktime_get();
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 
-	if (new_crtc_state->uapi.async_flip)
+	if (new_crtc_state->do_async_flip)
 		return;
 
 	trace_intel_pipe_update_end(crtc, end_vbl_count, scanline_end);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7099c8aaf4fb..7dfeb458aa65 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1263,10 +1263,8 @@ static void intel_crtc_enable_flip_done(struct intel_atomic_state *state,
 	int i;
 
 	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
-		if (plane->enable_flip_done &&
-		    plane->pipe == crtc->pipe &&
-		    update_planes & BIT(plane->id) &&
-		    plane_state->do_async_flip)
+		if (plane->pipe == crtc->pipe &&
+		    update_planes & BIT(plane->id))
 			plane->enable_flip_done(plane);
 	}
 }
@@ -1282,10 +1280,8 @@ static void intel_crtc_disable_flip_done(struct intel_atomic_state *state,
 	int i;
 
 	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
-		if (plane->disable_flip_done &&
-		    plane->pipe == crtc->pipe &&
-		    update_planes & BIT(plane->id) &&
-		    plane_state->do_async_flip)
+		if (plane->pipe == crtc->pipe &&
+		    update_planes & BIT(plane->id))
 			plane->disable_flip_done(plane);
 	}
 }
@@ -7504,15 +7500,25 @@ static int intel_async_flip_check_hw(struct intel_atomic_state *state, struct in
 			continue;
 
 		/*
-		 * TODO: Async flip is only supported through the page flip IOCTL
-		 * as of now. So support currently added for primary plane only.
-		 * Support for other planes on platforms on which supports
-		 * this(vlv/chv and icl+) should be added when async flip is
-		 * enabled in the atomic IOCTL path.
+		 * Only async flip capable planes should be in the state
+		 * if we're really about to ask the hardware to perform
+		 * an async flip. We should never get this far otherwise.
 		 */
-		if (!plane->async_flip)
+		if (drm_WARN_ON(&i915->drm,
+				new_crtc_state->do_async_flip && !plane->async_flip))
 			return -EINVAL;
 
+		/*
+		 * Only check async flip capable planes other planes
+		 * may be involved in the initial commit due to
+		 * the wm0/ddb optimization.
+		 *
+		 * TODO maybe should track which planes actually
+		 * were requested to do the async flip...
+		 */
+		if (!plane->async_flip)
+			continue;
+
 		/*
 		 * FIXME: This check is kept generic for all platforms.
 		 * Need to verify this for all gen9 platforms to enable
@@ -8463,7 +8469,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	intel_dbuf_pre_plane_update(state);
 
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
-		if (new_crtc_state->uapi.async_flip)
+		if (new_crtc_state->do_async_flip)
 			intel_crtc_enable_flip_done(state, crtc);
 	}
 
@@ -8489,7 +8495,7 @@ static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 	drm_atomic_helper_wait_for_flip_done(dev, &state->base);
 
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
-		if (new_crtc_state->uapi.async_flip)
+		if (new_crtc_state->do_async_flip)
 			intel_crtc_disable_flip_done(state, crtc);
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index b50d0e6efe21..776b3e6662f2 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -613,9 +613,6 @@ struct intel_plane_state {
 
 	struct intel_fb_view view;
 
-	/* Indicates if async flip is required */
-	bool do_async_flip;
-
 	/* Plane pxp decryption state */
 	bool decrypt;
 
@@ -951,6 +948,9 @@ struct intel_crtc_state {
 	bool preload_luts;
 	bool inherited; /* state inherited from BIOS? */
 
+	/* Ask the hardware to actually async flip? */
+	bool do_async_flip;
+
 	/* Pipe source size (ie. panel fitter input size)
 	 * All planes will be positioned inside this space,
 	 * and get clipped at the edges. */
-- 
2.38.1