[Intel-gfx] [RFC 3/3] drm/i915: Update modeset programming to use intermediate state

[Matt Roper]

As suggested by Ville, the general flow should now roughly follow:

        // whatever the user wanted
        compute_final_atomic_state()

        // include all crtcs in the intermediate state which are
        // getting disabled (even temporarily to perform a modeset)
        compute_intermediate_atomic_state()

        ret = check_state_change(old, intermediate)
        ret = check_state_change(intermediate, new)

        // commit all planes in one go to make them pop out as
        // atomically as possible
        for_each_crtc_in(intermediate) {
                commit_planes()
        }

        for_each_crtc_in(intermediate) {
                disable_crtc()
        }

        for_each_crtc_in(new) {
                if (!currently_active)
                        crtc_enable()
        }

        // commit all planes in one go to make them pop in as atomically
        // as possible
        for_each_crtc_in(new) {
                commit_planes()
        }

Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
---
Ville, is this what you had in mind for the intermediate states for modeset?  I
think there might still be a bug or two here that I need to track down, but
figured I should post this now to make sure this matches what you were asking for.

 drivers/gpu/drm/i915/intel_display.c | 63 +++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 76f3727..cbc2c69 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4747,10 +4747,11 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 }
 
-static void intel_post_plane_update(struct intel_crtc *crtc)
+static void intel_post_plane_update(struct drm_crtc_state *s)
 {
-	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
-	struct drm_device *dev = crtc->base.dev;
+	struct intel_crtc_state *cstate = to_intel_crtc_state(s);
+	struct intel_crtc *crtc = to_intel_crtc(s->crtc);
+	struct drm_device *dev = s->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_plane *plane;
 
@@ -4776,10 +4777,11 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
 					       0, 0, 0, false, false);
 }
 
-static void intel_pre_plane_update(struct intel_crtc *crtc)
+static void intel_pre_plane_update(struct drm_crtc_state *s)
 {
-	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->base.state);
-	struct drm_device *dev = crtc->base.dev;
+	struct intel_crtc_state *cstate = to_intel_crtc_state(s);
+	struct intel_crtc *crtc = to_intel_crtc(s->crtc);
+	struct drm_device *dev = s->crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_plane *p;
 
@@ -13076,8 +13078,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 			       bool async)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_atomic_state *istate = to_intel_atomic_state(state);
 	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_state *old_crtc_state;
 	int ret = 0;
 	int i;
 	bool any_ms = false;
@@ -13091,19 +13094,36 @@ static int intel_atomic_commit(struct drm_device *dev,
 	if (ret)
 		return ret;
 
+	/*
+	 * Commit final state to the underlying crtc/plane objects; the final
+	 * state should now be accessed via crtc->state, plane->state, etc.
+	 * However the intermediate state continues to reside in the top-level
+	 * state structure (istate->int_{crtc,plane}_states[]).
+	 */
 	drm_atomic_helper_swap_state(dev, state);
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	/*
+	 * Commit all planes in one go to make them pop out as atomically
+	 * as possible.
+	 */
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		struct drm_crtc_state *int_cstate;
 
 		if (!needs_modeset(crtc->state))
 			continue;
+		if (WARN_ON(!(int_cstate = istate->int_crtc_states[i])))
+			continue;
 
 		any_ms = true;
-		intel_pre_plane_update(intel_crtc);
+		intel_pre_plane_update(int_cstate);
+		drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
+		intel_post_plane_update(int_cstate);
+	}
 
-		if (crtc_state->active) {
-			intel_crtc_disable_planes(crtc, crtc_state->plane_mask);
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+		if (needs_modeset(crtc->state) && old_crtc_state->active) {
 			dev_priv->display.crtc_disable(crtc);
 			intel_crtc->active = false;
 			intel_disable_shared_dpll(intel_crtc);
@@ -13121,21 +13141,26 @@ static int intel_atomic_commit(struct drm_device *dev,
 		modeset_update_crtc_power_domains(state);
 	}
 
-	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	/* Now enable the clocks, pipe, and connectors that we set up. */
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
 		bool modeset = needs_modeset(crtc->state);
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
 		}
+	}
 
-		if (!modeset)
-			intel_pre_plane_update(intel_crtc);
+	/*
+	 * Commit all planes in one go to make them pop in as atomically as
+	 * possible.
+	 */
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		old_crtc_state = istate->int_crtc_states[i] ?: old_crtc_state;
 
-		drm_atomic_helper_commit_planes_on_crtc(crtc_state);
-		intel_post_plane_update(intel_crtc);
+		intel_pre_plane_update(crtc->state);
+		drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
+		intel_post_plane_update(crtc->state);
 	}
 
 	/* FIXME: add subpixel order */
-- 
2.1.4