diff mbox series

[v4,2/2] squash! drm/i915/cmtg: Disable the CMTG

Message ID 20250104172937.64015-3-gustavo.sousa@intel.com (mailing list archive)
State New
Headers show
Series drm/i915/cmtg: Disable the CMTG | expand

Commit Message

Gustavo Sousa Jan. 4, 2025, 5:29 p.m. UTC
v4:
  - Do the disable sequence as part of the sanitization step after
    hardware readout instead of initial modeset commit. (Jani)

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---

Jani, I hope I have captured what you meant with doing handling the disabling
during sanitization here.

I sent this as a separate "squash" patch because I'm not sure if this is the
correct way of doing it.

One thing I don't like very much about this is that we are forcing pipe(s) to be
disabled for platforms that require a modeset for disabling the CMTG. The
previous solution caused a modeset during initial commit for this case, which
seems a bit better to me.

 drivers/gpu/drm/i915/display/intel_cmtg.c     | 165 +++++-------------
 drivers/gpu/drm/i915/display/intel_cmtg.h     |   7 +-
 drivers/gpu/drm/i915/display/intel_display.c  |  11 --
 .../drm/i915/display/intel_modeset_setup.c    |  17 +-
 4 files changed, 62 insertions(+), 138 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.c b/drivers/gpu/drm/i915/display/intel_cmtg.c
index 493bc5ad7c76..8a2e19a794c2 100644
--- a/drivers/gpu/drm/i915/display/intel_cmtg.c
+++ b/drivers/gpu/drm/i915/display/intel_cmtg.c
@@ -161,45 +161,6 @@  void intel_cmtg_readout_hw_state(struct intel_display *display)
 	intel_cmtg_dump_state(display, cmtg_state);
 }
 
-static struct intel_cmtg_state *
-intel_atomic_get_cmtg_state(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	struct intel_global_state *obj_state =
-		intel_atomic_get_global_obj_state(state, &display->cmtg.obj);
-
-	if (IS_ERR(obj_state))
-		return ERR_CAST(obj_state);
-
-	return to_intel_cmtg_state(obj_state);
-}
-
-static struct intel_cmtg_state *
-intel_atomic_get_old_cmtg_state(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	struct intel_global_state *obj_state =
-		intel_atomic_get_old_global_obj_state(state, &display->cmtg.obj);
-
-	if (!obj_state)
-		return NULL;
-
-	return to_intel_cmtg_state(obj_state);
-}
-
-static struct intel_cmtg_state *
-intel_atomic_get_new_cmtg_state(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	struct intel_global_state *obj_state =
-		intel_atomic_get_new_global_obj_state(state, &display->cmtg.obj);
-
-	if (!obj_state)
-		return NULL;
-
-	return to_intel_cmtg_state(obj_state);
-}
-
 static bool intel_cmtg_state_changed(struct intel_cmtg_state *old_cmtg_state,
 				     struct intel_cmtg_state *new_cmtg_state)
 {
@@ -212,89 +173,18 @@  static bool intel_cmtg_state_changed(struct intel_cmtg_state *old_cmtg_state,
 		old_cmtg_state->trans_b_secondary != new_cmtg_state->trans_b_secondary;
 }
 
-static int intel_cmtg_check_modeset(struct intel_atomic_state *state,
-				    struct intel_cmtg_state *old_cmtg_state,
-				    struct intel_cmtg_state *new_cmtg_state)
-{
-	struct intel_display *display = to_intel_display(state);
-	u8 pipe_mask;
-
-	if (!intel_cmtg_requires_modeset(display))
-		return 0;
-
-	pipe_mask = 0;
-
-	if (old_cmtg_state->trans_a_secondary != new_cmtg_state->trans_a_secondary)
-		pipe_mask |= BIT(PIPE_A);
-
-	if (old_cmtg_state->trans_b_secondary != new_cmtg_state->trans_b_secondary)
-		pipe_mask |= BIT(PIPE_B);
-
-	if (!pipe_mask)
-		return 0;
-
-	return intel_modeset_pipes_in_mask_early(state, "updating CMTG config", pipe_mask);
-}
-
-int intel_cmtg_force_disabled(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	struct intel_cmtg_state *new_cmtg_state;
-
-	if (!HAS_CMTG(display))
-		return 0;
-
-	new_cmtg_state = intel_atomic_get_cmtg_state(state);
-	if (IS_ERR(new_cmtg_state))
-		return PTR_ERR(new_cmtg_state);
-
-	new_cmtg_state->cmtg_a_enable = false;
-	new_cmtg_state->cmtg_b_enable = false;
-	new_cmtg_state->trans_a_secondary = false;
-	new_cmtg_state->trans_b_secondary = false;
-
-	return 0;
-}
-
-int intel_cmtg_atomic_check(struct intel_atomic_state *state)
+static void intel_cmtg_state_set_disabled(struct intel_cmtg_state *cmtg_state)
 {
-	struct intel_display *display = to_intel_display(state);
-	struct intel_cmtg_state *old_cmtg_state;
-	struct intel_cmtg_state *new_cmtg_state;
-	int ret;
-
-	if (!HAS_CMTG(display))
-		return 0;
-
-	old_cmtg_state = intel_atomic_get_old_cmtg_state(state);
-	new_cmtg_state = intel_atomic_get_new_cmtg_state(state);
-	if (!intel_cmtg_state_changed(old_cmtg_state, new_cmtg_state))
-		return 0;
-
-	ret = intel_cmtg_check_modeset(state, old_cmtg_state, new_cmtg_state);
-	if (ret)
-		return ret;
-
-	return intel_atomic_serialize_global_state(&new_cmtg_state->base);
+	cmtg_state->cmtg_a_enable = false;
+	cmtg_state->cmtg_b_enable = false;
+	cmtg_state->trans_a_secondary = false;
+	cmtg_state->trans_b_secondary = false;
 }
 
-/*
- * Access to CMTG registers require the PHY PLL that provides its clock to be
- * running (which is configured via CMTG_CLK_SEL). As such, this function needs
- * to be called before intel_commit_modeset_disables() to ensure that the PHY
- * PLL is still enabled when doing this.
- */
-void intel_cmtg_disable(struct intel_atomic_state *state)
+static void intel_cmtg_disable(struct intel_display *display,
+			       struct intel_cmtg_state *old_cmtg_state,
+			       struct intel_cmtg_state *new_cmtg_state)
 {
-	struct intel_display *display = to_intel_display(state);
-	struct intel_cmtg_state *old_cmtg_state;
-	struct intel_cmtg_state *new_cmtg_state;
-
-	if (!HAS_CMTG(display))
-		return;
-
-	old_cmtg_state = intel_atomic_get_old_cmtg_state(state);
-	new_cmtg_state = intel_atomic_get_new_cmtg_state(state);
 	if (!intel_cmtg_state_changed(old_cmtg_state, new_cmtg_state))
 		return;
 
@@ -320,3 +210,42 @@  void intel_cmtg_disable(struct intel_atomic_state *state)
 		intel_de_rmw(display, CMTG_CLK_SEL, clk_sel_clr, clk_sel_set);
 	}
 }
+
+static u32 intel_cmtg_modeset_crtc_mask(struct intel_display *display,
+					struct intel_cmtg_state *old_cmtg_state,
+					struct intel_cmtg_state *new_cmtg_state)
+{
+	u32 crtc_mask;
+
+	if (intel_cmtg_requires_modeset(display))
+		return 0;
+
+	crtc_mask = 0;
+
+	if (old_cmtg_state->trans_a_secondary != new_cmtg_state->trans_a_secondary)
+		crtc_mask |= drm_crtc_mask(&intel_crtc_for_pipe(display, PIPE_A)->base);
+
+	if (old_cmtg_state->trans_b_secondary != new_cmtg_state->trans_b_secondary)
+		crtc_mask |= drm_crtc_mask(&intel_crtc_for_pipe(display, PIPE_B)->base);
+
+	return crtc_mask;
+}
+
+/*
+ * Disable CMTG if enabled and return a mask of pipes that need to be disabled
+ * (for platforms where disabling the CMTG requires a modeset).
+ */
+u32 intel_cmtg_sanitize_state(struct intel_display *display)
+{
+	struct intel_cmtg_state *cmtg_state = to_intel_cmtg_state(display->cmtg.obj.state);
+	struct intel_cmtg_state old_cmtg_state;
+
+	if (!HAS_CMTG(display))
+		return 0;
+
+	old_cmtg_state = *cmtg_state;
+	intel_cmtg_state_set_disabled(cmtg_state);
+	intel_cmtg_disable(display, &old_cmtg_state, cmtg_state);
+
+	return intel_cmtg_modeset_crtc_mask(display, &old_cmtg_state, cmtg_state);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cmtg.h b/drivers/gpu/drm/i915/display/intel_cmtg.h
index 64c42e345665..3c51e144aa3f 100644
--- a/drivers/gpu/drm/i915/display/intel_cmtg.h
+++ b/drivers/gpu/drm/i915/display/intel_cmtg.h
@@ -6,14 +6,13 @@ 
 #ifndef __INTEL_CMTG_H__
 #define __INTEL_CMTG_H__
 
-struct intel_atomic_state;
+#include <linux/types.h>
+
 struct intel_display;
 struct intel_global_state;
 
 int intel_cmtg_init(struct intel_display *display);
 void intel_cmtg_readout_hw_state(struct intel_display *display);
-int intel_cmtg_force_disabled(struct intel_atomic_state *state);
-int intel_cmtg_atomic_check(struct intel_atomic_state *state);
-void intel_cmtg_disable(struct intel_atomic_state *state);
+u32 intel_cmtg_sanitize_state(struct intel_display *display);
 
 #endif /* __INTEL_CMTG_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 098985ad7ad4..4271da219b41 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -62,7 +62,6 @@ 
 #include "intel_bw.h"
 #include "intel_cdclk.h"
 #include "intel_clock_gating.h"
-#include "intel_cmtg.h"
 #include "intel_color.h"
 #include "intel_crt.h"
 #include "intel_crtc.h"
@@ -6829,10 +6828,6 @@  int intel_atomic_check(struct drm_device *dev,
 	if (ret)
 		goto fail;
 
-	ret = intel_cmtg_atomic_check(state);
-	if (ret)
-		goto fail;
-
 	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
 		if (!intel_crtc_needs_modeset(new_crtc_state))
 			continue;
@@ -7762,8 +7757,6 @@  static void intel_atomic_commit_tail(struct intel_atomic_state *state)
 			intel_modeset_get_crtc_power_domains(new_crtc_state, &put_domains[crtc->pipe]);
 	}
 
-	intel_cmtg_disable(state);
-
 	intel_commit_modeset_disables(state);
 
 	intel_dp_tunnel_atomic_alloc_bw(state);
@@ -8589,10 +8582,6 @@  int intel_initial_commit(struct drm_device *dev)
 		}
 	}
 
-	ret = intel_cmtg_force_disabled(to_intel_atomic_state(state));
-	if (ret)
-		goto out;
-
 	ret = drm_atomic_commit(state);
 
 out:
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index e0845ae21d82..c6eeb8a00a7b 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -474,10 +474,12 @@  static void intel_sanitize_fifo_underrun_reporting(const struct intel_crtc_state
 }
 
 static bool intel_sanitize_crtc(struct intel_crtc *crtc,
-				struct drm_modeset_acquire_ctx *ctx)
+				struct drm_modeset_acquire_ctx *ctx,
+				u32 force_off_crtc_mask)
 {
 	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
 	struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state);
+	u32 crtc_mask = drm_crtc_mask(&crtc->base);
 	bool needs_link_reset;
 
 	if (crtc_state->hw.active) {
@@ -508,7 +510,8 @@  static bool intel_sanitize_crtc(struct intel_crtc *crtc,
 	 * Adjust the state of the output pipe according to whether we have
 	 * active connectors/encoders.
 	 */
-	if (!needs_link_reset && intel_crtc_has_encoders(crtc))
+	if (!(crtc_mask & force_off_crtc_mask) &&
+	    !needs_link_reset && intel_crtc_has_encoders(crtc))
 		return false;
 
 	intel_crtc_disable_noatomic(crtc, ctx);
@@ -526,7 +529,8 @@  static bool intel_sanitize_crtc(struct intel_crtc *crtc,
 }
 
 static void intel_sanitize_all_crtcs(struct drm_i915_private *i915,
-				     struct drm_modeset_acquire_ctx *ctx)
+				     struct drm_modeset_acquire_ctx *ctx,
+				     u32 force_off_crtc_mask)
 {
 	struct intel_crtc *crtc;
 	u32 crtcs_forced_off = 0;
@@ -546,7 +550,7 @@  static void intel_sanitize_all_crtcs(struct drm_i915_private *i915,
 			if (crtcs_forced_off & crtc_mask)
 				continue;
 
-			if (intel_sanitize_crtc(crtc, ctx))
+			if (intel_sanitize_crtc(crtc, ctx, force_off_crtc_mask))
 				crtcs_forced_off |= crtc_mask;
 		}
 		if (crtcs_forced_off == old_mask)
@@ -967,6 +971,7 @@  void intel_modeset_setup_hw_state(struct drm_i915_private *i915,
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
 	intel_wakeref_t wakeref;
+	u32 force_off_crtc_mask;
 
 	wakeref = intel_display_power_get(i915, POWER_DOMAIN_INIT);
 
@@ -1009,7 +1014,9 @@  void intel_modeset_setup_hw_state(struct drm_i915_private *i915,
 	 */
 	intel_modeset_update_connector_atomic_state(i915);
 
-	intel_sanitize_all_crtcs(i915, ctx);
+	force_off_crtc_mask = intel_cmtg_sanitize_state(display);
+
+	intel_sanitize_all_crtcs(i915, ctx, force_off_crtc_mask);
 
 	intel_dpll_sanitize_state(i915);