[Intel-gfx] [RFC 2/2] [INCOMPLETE] drm/i915: Split up crtc_state into uapi and hw

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Fri Jun 14 20:21:15 UTC 2019


 Use uapi for drm, and keep the internal state in hw.
 
 This patch is incomplete, just want feedback on the direction I'm going.
---
 drivers/gpu/drm/i915/intel_atomic.c  | 13 ++--
 drivers/gpu/drm/i915/intel_display.c | 88 +++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 3 files changed, 57 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 1f891b554ead..36fbd408ba81 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -185,12 +185,14 @@ struct drm_crtc_state *
 intel_crtc_duplicate_state(struct drm_crtc *crtc)
 {
 	struct intel_crtc_state *crtc_state;
+	const struct intel_crtc_state *old_crtc_state = to_intel_crtc_state(crtc->state);
 
-	crtc_state = kmemdup(crtc->state, sizeof(*crtc_state), GFP_KERNEL);
+	crtc_state = kmemdup(old_crtc_state, sizeof(*crtc_state), GFP_KERNEL);
 	if (!crtc_state)
 		return NULL;
 
-	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->hw);
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->uapi);
+	memset(&crtc_state->hw, 0, sizeof(crtc_state->hw));
 
 	crtc_state->update_pipe = false;
 	crtc_state->disable_lp_wm = false;
@@ -203,7 +205,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	crtc_state->fb_bits = 0;
 	crtc_state->update_planes = 0;
 
-	return &crtc_state->hw;
+	return &crtc_state->uapi;
 }
 
 /**
@@ -218,7 +220,10 @@ void
 intel_crtc_destroy_state(struct drm_crtc *crtc,
 			 struct drm_crtc_state *state)
 {
-	drm_atomic_helper_crtc_destroy_state(crtc, state);
+	struct intel_crtc_state *crtc_state = to_intel_crtc_state(state);
+
+	__drm_atomic_helper_crtc_destroy_state(crtc, &crtc_state->uapi);
+	kfree(crtc_state);
 }
 
 static void intel_atomic_setup_scaler(struct intel_crtc_scaler_state *scaler_state,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9aa273fb5415..2de978ab8fc2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -514,9 +514,9 @@ icl_wa_scalerclkgating(struct drm_i915_private *dev_priv, enum pipe pipe,
 }
 
 static bool
-needs_modeset(const struct drm_crtc_state *state)
+needs_modeset(const struct intel_crtc_state *state)
 {
-	return drm_atomic_crtc_needs_modeset(state);
+	return drm_atomic_crtc_needs_modeset(&state->hw);
 }
 
 /*
@@ -5795,7 +5795,7 @@ static bool hsw_pre_update_disable_ips(const struct intel_crtc_state *old_crtc_s
 	if (!old_crtc_state->ips_enabled)
 		return false;
 
-	if (needs_modeset(&new_crtc_state->hw))
+	if (needs_modeset(new_crtc_state))
 		return true;
 
 	/*
@@ -5822,7 +5822,7 @@ static bool hsw_post_update_enable_ips(const struct intel_crtc_state *old_crtc_s
 	if (!new_crtc_state->ips_enabled)
 		return false;
 
-	if (needs_modeset(&new_crtc_state->hw))
+	if (needs_modeset(new_crtc_state))
 		return true;
 
 	/*
@@ -5899,7 +5899,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
 		intel_fbc_post_update(crtc);
 
 		if (new_primary_state->visible &&
-		    (needs_modeset(&pipe_config->hw) ||
+		    (needs_modeset(pipe_config) ||
 		     !old_primary_state->visible))
 			intel_post_enable_primary(&crtc->base, pipe_config);
 	}
@@ -5923,7 +5923,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 	struct drm_plane *primary = crtc->base.primary;
 	struct drm_plane_state *old_primary_state =
 		drm_atomic_get_old_plane_state(old_state, primary);
-	bool modeset = needs_modeset(&pipe_config->hw);
+	bool modeset = needs_modeset(pipe_config);
 	struct intel_atomic_state *old_intel_state =
 		to_intel_atomic_state(old_state);
 
@@ -5983,7 +5983,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
 	 * If we're doing a modeset, we're done.  No need to do any pre-vblank
 	 * watermark programming here.
 	 */
-	if (needs_modeset(&pipe_config->hw))
+	if (needs_modeset(pipe_config))
 		return;
 
 	/*
@@ -11338,7 +11338,7 @@ int intel_plane_atomic_calc_changes(const struct intel_crtc_state *old_crtc_stat
 	struct intel_plane *plane = to_intel_plane(plane_state->plane);
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	bool mode_changed = needs_modeset(crtc_state);
+	bool mode_changed = needs_modeset(pipe_config);
 	bool was_crtc_enabled = old_crtc_state->hw.active;
 	bool is_crtc_enabled = crtc_state->active;
 	bool turn_off, turn_on, visible, was_visible;
@@ -11607,7 +11607,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc_state);
 	int ret;
-	bool mode_changed = needs_modeset(crtc_state);
+	bool mode_changed = needs_modeset(pipe_config);
 
 	if (INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv) &&
 	    mode_changed && !crtc_state->active)
@@ -13095,7 +13095,7 @@ intel_modeset_verify_crtc(struct drm_crtc *crtc,
 			  struct drm_crtc_state *old_state,
 			  struct drm_crtc_state *new_state)
 {
-	if (!needs_modeset(new_state) &&
+	if (!needs_modeset(to_intel_crtc_state(new_state)) &&
 	    !to_intel_crtc_state(new_state)->update_pipe)
 		return;
 
@@ -13187,7 +13187,7 @@ static void intel_modeset_clear_plls(struct intel_atomic_state *state)
 		struct intel_shared_dpll *old_dpll =
 			old_crtc_state->shared_dpll;
 
-		if (!needs_modeset(&new_crtc_state->hw))
+		if (!needs_modeset(new_crtc_state))
 			continue;
 
 		new_crtc_state->shared_dpll = NULL;
@@ -13217,7 +13217,7 @@ static int haswell_mode_set_planes_workaround(struct intel_atomic_state *state)
 	/* look at all crtc's that are going to be enabled in during modeset */
 	for_each_new_intel_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!crtc_state->hw.active ||
-		    !needs_modeset(&crtc_state->hw))
+		    !needs_modeset(crtc_state))
 			continue;
 
 		if (first_crtc_state) {
@@ -13242,7 +13242,7 @@ static int haswell_mode_set_planes_workaround(struct intel_atomic_state *state)
 		crtc_state->hsw_workaround_pipe = INVALID_PIPE;
 
 		if (!crtc_state->hw.active ||
-		    needs_modeset(&crtc_state->hw))
+		    needs_modeset(crtc_state))
 			continue;
 
 		/* 2 or more enabled crtcs means no need for w/a */
@@ -13276,32 +13276,32 @@ static int intel_lock_all_pipes(struct drm_atomic_state *state)
 	return 0;
 }
 
-static int intel_modeset_all_pipes(struct drm_atomic_state *state)
+static int intel_modeset_all_pipes(struct intel_atomic_state *state)
 {
-	struct drm_crtc *crtc;
+	struct intel_crtc *crtc;
 
 	/*
 	 * Add all pipes to the state, and force
 	 * a modeset on all the active ones.
 	 */
 	for_each_crtc(state->dev, crtc) {
-		struct drm_crtc_state *crtc_state;
+		struct intel_crtc_state *crtc_state;
 		int ret;
 
-		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		crtc_state = intel_atomic_get_crtc_state(state, crtc);
 		if (IS_ERR(crtc_state))
 			return PTR_ERR(crtc_state);
 
-		if (!crtc_state->active || needs_modeset(crtc_state))
+		if (!crtc_state->hw.active || needs_modeset(crtc_state))
 			continue;
 
 		crtc_state->mode_changed = true;
 
-		ret = drm_atomic_add_affected_connectors(state, crtc);
+		ret = drm_atomic_add_affected_connectors(&state->base, &crtc->base);
 		if (ret)
 			return ret;
 
-		ret = drm_atomic_add_affected_planes(state, crtc);
+		ret = drm_atomic_add_affected_planes(&state->base, &crtc->base);
 		if (ret)
 			return ret;
 	}
@@ -13369,12 +13369,12 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 		}
 
 		if (is_power_of_2(state->active_crtcs)) {
-			struct drm_crtc *crtc;
-			struct drm_crtc_state *crtc_state;
+			struct intel_crtc *crtc;
+			struct intel_crtc_state *crtc_state;
 
 			pipe = ilog2(state->active_crtcs);
-			crtc = &intel_get_crtc_for_pipe(dev_priv, pipe)->base;
-			crtc_state = drm_atomic_get_new_crtc_state(&state->base, crtc);
+			crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+			crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
 			if (crtc_state && needs_modeset(crtc_state))
 				pipe = INVALID_PIPE;
 		} else {
@@ -13393,7 +13393,7 @@ static int intel_modeset_checks(struct intel_atomic_state *state)
 			state->cdclk.pipe = pipe;
 		} else if (intel_cdclk_needs_modeset(&dev_priv->cdclk.actual,
 						     &state->cdclk.actual)) {
-			ret = intel_modeset_all_pipes(&state->base);
+			ret = intel_modeset_all_pipes(state);
 			if (ret < 0)
 				return ret;
 
@@ -13451,6 +13451,8 @@ static int intel_atomic_check(struct drm_device *dev,
 	/* Catch I915_MODE_FLAG_INHERITED */
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
+		new_intel_crtc_state->hw = new_intel_crtc_state->uapi;
+
 		if (new_crtc_state->hw.mode.private_flags !=
 		    old_crtc_state->hw.mode.private_flags)
 			new_crtc_state->hw.mode_changed = true;
@@ -13462,7 +13464,7 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (!needs_modeset(&new_crtc_state->hw))
+		if (!needs_modeset(new_crtc_state))
 			continue;
 
 		if (!new_crtc_state->hw.enable) {
@@ -13480,7 +13482,7 @@ static int intel_atomic_check(struct drm_device *dev,
 			new_crtc_state->update_pipe = true;
 		}
 
-		if (needs_modeset(&new_crtc_state->hw))
+		if (needs_modeset(new_crtc_state))
 			any_ms = true;
 	}
 
@@ -13515,12 +13517,12 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
-		if (!needs_modeset(&new_crtc_state->hw) &&
+		if (!needs_modeset(new_crtc_state) &&
 		    !new_crtc_state->update_pipe)
 			continue;
 
 		intel_dump_pipe_config(new_crtc_state, state,
-				       needs_modeset(&new_crtc_state->hw) ?
+				       needs_modeset(new_crtc_state) ?
 				       "[modeset]" : "[fastset]");
 	}
 
@@ -13567,7 +13569,7 @@ static void intel_update_crtc(struct drm_crtc *crtc,
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_crtc_state *pipe_config = to_intel_crtc_state(new_crtc_state);
-	bool modeset = needs_modeset(new_crtc_state);
+	bool modeset = needs_modeset(pipe_config);
 	struct intel_plane_state *new_plane_state =
 		intel_atomic_get_new_plane_state(to_intel_atomic_state(state),
 						 to_intel_plane(crtc->primary));
@@ -13776,20 +13778,23 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 		new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
 		intel_crtc = to_intel_crtc(crtc);
 
-		if (needs_modeset(new_crtc_state) ||
-		    to_intel_crtc_state(new_crtc_state)->update_pipe) {
+		/* Copy for drm_atomic_helper_setup_commit() */
+		new_intel_crtc_state->hw = new_intel_crtc_state->uapi;
+
+		if (needs_modeset(new_intel_crtc_state) ||
+		    new_intel_crtc_state->update_pipe) {
 
 			put_domains[intel_crtc->pipe] =
 				modeset_get_crtc_power_domains(crtc,
 					new_intel_crtc_state);
 		}
 
-		if (!needs_modeset(new_crtc_state))
+		if (!needs_modeset(new_intel_crtc_state))
 			continue;
 
 		intel_pre_plane_update(old_intel_crtc_state, new_intel_crtc_state);
 
-		if (old_crtc_state->active) {
+		if (new_intel_crtc_state->hw.active) {
 			intel_crtc_disable_planes(intel_state, intel_crtc);
 
 			/*
@@ -13878,8 +13883,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		new_intel_crtc_state = to_intel_crtc_state(new_crtc_state);
 
-		if (new_crtc_state->active &&
-		    !needs_modeset(new_crtc_state) &&
+		if (new_intel_crtc_state->hw.active &&
+		    !needs_modeset(new_intel_crtc_state) &&
 		    (new_intel_crtc_state->hw.color_mgmt_changed ||
 		     new_intel_crtc_state->update_pipe))
 			intel_color_load_luts(new_intel_crtc_state);
@@ -14238,9 +14243,9 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	int ret;
 
 	if (old_obj) {
-		struct drm_crtc_state *crtc_state =
-			drm_atomic_get_new_crtc_state(new_state->state,
-						      plane->state->crtc);
+		struct intel_crtc_state *crtc_state =
+			intel_atomic_get_new_crtc_state(intel_state,
+							plane->state->crtc);
 
 		/* Big Hammer, we also need to ensure that any pending
 		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
@@ -14401,7 +14406,7 @@ static void intel_begin_crtc_commit(struct intel_atomic_state *state,
 		intel_atomic_get_old_crtc_state(state, crtc);
 	struct intel_crtc_state *new_crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
-	bool modeset = needs_modeset(&new_crtc_state->hw);
+	bool modeset = needs_modeset(new_crtc_state);
 
 	/* Perform vblank evasion around commit operation */
 	intel_pipe_update_start(new_crtc_state);
@@ -14454,7 +14459,7 @@ static void intel_finish_crtc_commit(struct intel_atomic_state *state,
 	intel_pipe_update_end(new_crtc_state);
 
 	if (new_crtc_state->update_pipe &&
-	    !needs_modeset(&new_crtc_state->hw) &&
+	    !needs_modeset(new_crtc_state) &&
 	    old_crtc_state->hw.mode.private_flags & I915_MODE_FLAG_INHERITED)
 		intel_crtc_arm_fifo_underrun(crtc, new_crtc_state);
 }
@@ -14608,6 +14613,7 @@ intel_legacy_cursor_update(struct drm_plane *plane,
 		ret = -ENOMEM;
 		goto out_free;
 	}
+	new_crtc_state->hw = new_crtc_state->uapi;
 
 	drm_atomic_set_fb_for_plane(new_plane_state, fb);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ec50f9167161..d39ef2443ce9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -713,7 +713,7 @@ enum intel_output_format {
 };
 
 struct intel_crtc_state {
-	struct drm_crtc_state hw;
+	struct drm_crtc_state uapi, hw;
 
 	/**
 	 * quirks - bitfield with hw state readout quirks
-- 
2.20.1



More information about the Intel-gfx mailing list