Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fix Cx0 Suspend Resume issue
@ 2026-01-14  3:42 Suraj Kandpal
  2026-01-14  3:42 ` [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts Suraj Kandpal
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Suraj Kandpal @ 2026-01-14  3:42 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: ankit.k.nautiyal, Suraj Kandpal

CX0 PHY currently has two issues which cause a hang when we try
to suspend resume machine with a delay of 15mins and 1+ hour.
This happens due to two reasons:
1) We do not follow the Enablement sequence where we need to
enable our clock after PPS Enablement cycle
2) We do not make sure response ready and error bit are cleared
in P2M_MSGBUS_STATUS before writing the transaction pending bit.
This series aims to solve this.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

Mika Kahola (1):
  drm/i915/cx0: Split PLL enabling/disabling in two parts

Suraj Kandpal (2):
  drm/i915/cx0: Clear response ready & error bit
  drm/i915/cx0: Rename intel_clear_response_ready flag

 drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 134 +++++++++++-------
 drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   4 +-
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
 drivers/gpu/drm/i915/display/intel_lt_phy.c   |   2 +-
 4 files changed, 92 insertions(+), 55 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts
  2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
@ 2026-01-14  3:42 ` Suraj Kandpal
  2026-01-14 15:12   ` Imre Deak
  2026-01-14  3:42 ` [PATCH v2 2/3] drm/i915/cx0: Clear response ready & error bit Suraj Kandpal
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Suraj Kandpal @ 2026-01-14  3:42 UTC (permalink / raw)
  To: intel-xe, intel-gfx
  Cc: ankit.k.nautiyal, Mika Kahola, Suraj Kandpal, Michał Grzelak

From: Mika Kahola <mika.kahola@intel.com>

Split PLL enabling/disabling in two parts - one for pll setting
pll dividers and second one to enable/disable pll clock. PLL
clock enabling/disbling happens via encoder->enable_clock/disable_clock
function hook.

PLL state verification happens now earlier than the clock is enabled
which causes a drm warn to be thrown. Silence this warning by
allowing this check for only earlier platforms than MeteorLake.

While at it also add the necessary argument to cx0_enable_clock
so that we can move step 12 of the enable sequence.

v2:
- Move state verification to enable_clock() function for
MTL+ platforms
- Squash patch 1 & 2 (Gustavo)
- Use correct Bspec references (Gustavo)
- Fix build error (Michal)

Bspec: 65448, 68849
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 120 +++++++++++-------
 drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
 2 files changed, 80 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 7288065d2461..3418a3ed28fd 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -3225,11 +3225,8 @@ static void intel_cx0pll_enable(struct intel_encoder *encoder,
 {
 	int port_clock = pll_state->use_c10 ? pll_state->c10.clock : pll_state->c20.clock;
 	struct intel_display *display = to_intel_display(encoder);
-	enum phy phy = intel_encoder_to_phy(encoder);
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	bool lane_reversal = dig_port->lane_reversal;
-	u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 :
-					  INTEL_CX0_LANE0;
 	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
 
 	/*
@@ -3284,42 +3281,6 @@ static void intel_cx0pll_enable(struct intel_encoder *encoder,
 	 */
 	intel_de_write(display, DDI_CLK_VALFREQ(encoder->port), port_clock);
 
-	/*
-	 * 9. Set PORT_CLOCK_CTL register PCLK PLL Request
-	 * LN<Lane for maxPCLK> to "1" to enable PLL.
-	 */
-	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
-		     intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES),
-		     intel_cx0_get_pclk_pll_request(maxpclk_lane));
-
-	/* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == "1". */
-	if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
-			     intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES),
-			     intel_cx0_get_pclk_pll_ack(maxpclk_lane),
-			     XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL))
-		drm_warn(display->drm, "Port %c PLL not locked\n",
-			 phy_name(phy));
-
-	/*
-	 * 11. Follow the Display Voltage Frequency Switching Sequence After
-	 * Frequency Change. We handle this step in bxt_set_cdclk().
-	 */
-
-	/*
-	 * 12. Toggle powerdown if HDMI is enabled on C10 PHY.
-	 *
-	 * Wa_13013502646:
-	 * Fixes: HDMI lane to lane skew violations on C10 display PHYs.
-	 * Workaround: Toggle powerdown value by setting first to P0 and then to P2, for both
-	 * PHY lanes.
-	 */
-	if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) {
-		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
-						    XELPDP_P0_STATE_ACTIVE);
-		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
-						    XELPDP_P2_STATE_READY);
-	}
-
 	intel_cx0_phy_transaction_end(encoder, wakeref);
 }
 
@@ -3403,6 +3364,56 @@ static int intel_mtl_tbt_clock_select(struct intel_display *display,
 	}
 }
 
+static void intel_cx0pll_enable_clock(struct intel_encoder *encoder,
+				      const struct intel_cx0pll_state *pll_state)
+{
+	struct intel_display *display = to_intel_display(encoder);
+	enum phy phy = intel_encoder_to_phy(encoder);
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+	bool lane_reversal = dig_port->lane_reversal;
+	u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 :
+					  INTEL_CX0_LANE0;
+	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
+
+	/*
+	 * 9. Set PORT_CLOCK_CTL register PCLK PLL Request
+	 * LN<Lane for maxPCLK> to "1" to enable PLL.
+	 */
+	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
+		     intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES),
+		     intel_cx0_get_pclk_pll_request(maxpclk_lane));
+
+	/* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == "1". */
+	if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
+			     intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES),
+			     intel_cx0_get_pclk_pll_ack(maxpclk_lane),
+			     XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL))
+		drm_warn(display->drm, "Port %c PLL not locked\n",
+			 phy_name(phy));
+
+	/*
+	 * 11. Follow the Display Voltage Frequency Switching Sequence After
+	 * Frequency Change. We handle this step in bxt_set_cdclk().
+	 */
+
+	/*
+	 * 12. Toggle powerdown if HDMI is enabled on C10 PHY.
+	 *
+	 * Wa_13013502646:
+	 * Fixes: HDMI lane to lane skew violations on C10 display PHYs.
+	 * Workaround: Toggle powerdown value by setting first to P0 and then to P2, for both
+	 * PHY lanes.
+	 */
+	if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) {
+		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
+						    XELPDP_P0_STATE_ACTIVE);
+		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
+						    XELPDP_P2_STATE_READY);
+	}
+
+	intel_cx0_phy_transaction_end(encoder, wakeref);
+}
+
 void intel_mtl_tbt_pll_enable_clock(struct intel_encoder *encoder, int port_clock)
 {
 	struct intel_display *display = to_intel_display(encoder);
@@ -3468,10 +3479,16 @@ void intel_mtl_pll_enable(struct intel_encoder *encoder,
 void intel_mtl_pll_enable_clock(struct intel_encoder *encoder,
 				const struct intel_crtc_state *crtc_state)
 {
+	struct intel_display *display = to_intel_display(encoder);
+	struct intel_dpll *pll = crtc_state->intel_dpll;
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 
 	if (intel_tc_port_in_tbt_alt_mode(dig_port))
 		intel_mtl_tbt_pll_enable_clock(encoder, crtc_state->port_clock);
+	else
+		intel_cx0pll_enable_clock(encoder, &crtc_state->dpll_hw_state.cx0pll);
+
+	assert_dpll_enabled(display, pll);
 }
 
 /*
@@ -3567,12 +3584,6 @@ static void intel_cx0pll_disable(struct intel_encoder *encoder)
 	 * Frequency Change. We handle this step in bxt_set_cdclk().
 	 */
 
-	/* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */
-	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
-		     XELPDP_DDI_CLOCK_SELECT_MASK(display), 0);
-	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
-		     XELPDP_FORWARD_CLOCK_UNGATE, 0);
-
 	intel_cx0_phy_transaction_end(encoder, wakeref);
 }
 
@@ -3586,6 +3597,20 @@ static bool intel_cx0_pll_is_enabled(struct intel_encoder *encoder)
 			     intel_cx0_get_pclk_pll_request(lane);
 }
 
+static void intel_cx0pll_disable_clock(struct intel_encoder *encoder)
+{
+	struct intel_display *display = to_intel_display(encoder);
+	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
+
+	/* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */
+	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
+		     XELPDP_DDI_CLOCK_SELECT_MASK(display), 0);
+	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
+		     XELPDP_FORWARD_CLOCK_UNGATE, 0);
+
+	intel_cx0_phy_transaction_end(encoder, wakeref);
+}
+
 void intel_mtl_tbt_pll_disable_clock(struct intel_encoder *encoder)
 {
 	struct intel_display *display = to_intel_display(encoder);
@@ -3635,6 +3660,9 @@ void intel_mtl_pll_disable_clock(struct intel_encoder *encoder)
 
 	if (intel_tc_port_in_tbt_alt_mode(dig_port))
 		intel_mtl_tbt_pll_disable_clock(encoder);
+	else
+		intel_cx0pll_disable_clock(encoder);
+
 }
 
 enum icl_port_dpll_id
@@ -3783,6 +3811,8 @@ void intel_cx0_pll_power_save_wa(struct intel_display *display)
 			    encoder->base.base.id, encoder->base.name);
 
 		intel_cx0pll_enable(encoder, &pll_state);
+		intel_cx0pll_enable_clock(encoder, &pll_state);
 		intel_cx0pll_disable(encoder);
+		intel_cx0pll_disable_clock(encoder);
 	}
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 9aa84a430f09..040c97d81302 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -298,7 +298,8 @@ void intel_dpll_enable(const struct intel_crtc_state *crtc_state)
 
 	if (old_mask) {
 		drm_WARN_ON(display->drm, !pll->on);
-		assert_dpll_enabled(display, pll);
+		if (DISPLAY_VER(display) < 14)
+			assert_dpll_enabled(display, pll);
 		goto out;
 	}
 	drm_WARN_ON(display->drm, pll->on);
@@ -342,7 +343,9 @@ void intel_dpll_disable(const struct intel_crtc_state *crtc_state)
 		    pll->info->name, pll->active_mask, pll->on,
 		    crtc->base.base.id, crtc->base.name);
 
-	assert_dpll_enabled(display, pll);
+	if (DISPLAY_VER(display) < 14)
+		assert_dpll_enabled(display, pll);
+
 	drm_WARN_ON(display->drm, !pll->on);
 
 	pll->active_mask &= ~pipe_mask;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/3] drm/i915/cx0: Clear response ready & error bit
  2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
  2026-01-14  3:42 ` [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts Suraj Kandpal
@ 2026-01-14  3:42 ` Suraj Kandpal
  2026-01-14  3:42 ` [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag Suraj Kandpal
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Suraj Kandpal @ 2026-01-14  3:42 UTC (permalink / raw)
  To: intel-xe, intel-gfx
  Cc: ankit.k.nautiyal, Suraj Kandpal, Gustavo Sousa,
	Michał Grzelak

Clear the response ready and error bit of PORT_P2M_MESSAGE_BUS_STATUS
before writing the transaction pending bit of
PORT_M2P_MSGBUS_CTL as that is a hard requirement. If not done
we find that the PHY hangs since it ends up in a weird state if left
idle for more than 1 hour.

Bspec: 65101
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 3418a3ed28fd..00c7fa9040ee 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -222,6 +222,8 @@ static int __intel_cx0_read_once(struct intel_encoder *encoder,
 		return -ETIMEDOUT;
 	}
 
+	intel_clear_response_ready_flag(encoder, lane);
+
 	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, port, lane),
 		       XELPDP_PORT_M2P_TRANSACTION_PENDING |
 		       XELPDP_PORT_M2P_COMMAND_READ |
@@ -293,6 +295,8 @@ static int __intel_cx0_write_once(struct intel_encoder *encoder,
 		return -ETIMEDOUT;
 	}
 
+	intel_clear_response_ready_flag(encoder, lane);
+
 	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, port, lane),
 		       XELPDP_PORT_M2P_TRANSACTION_PENDING |
 		       (committed ? XELPDP_PORT_M2P_COMMAND_WRITE_COMMITTED :
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag
  2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
  2026-01-14  3:42 ` [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts Suraj Kandpal
  2026-01-14  3:42 ` [PATCH v2 2/3] drm/i915/cx0: Clear response ready & error bit Suraj Kandpal
@ 2026-01-14  3:42 ` Suraj Kandpal
  2026-01-14  6:03   ` Garg, Nemesa
  2026-01-14  5:22 ` ✗ i915.CI.BAT: failure for Fix Cx0 Suspend Resume issue (rev2) Patchwork
  2026-01-14 15:10 ` [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Rodrigo Vivi
  4 siblings, 1 reply; 9+ messages in thread
From: Suraj Kandpal @ 2026-01-14  3:42 UTC (permalink / raw)
  To: intel-xe, intel-gfx; +Cc: ankit.k.nautiyal, Suraj Kandpal

Rename the non static intel_clear_response_ready_flag to
intel_cx0_clear_response_ready_flag so that we follow the
naming standards of non static function.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 14 +++++++-------
 drivers/gpu/drm/i915/display/intel_cx0_phy.h |  4 ++--
 drivers/gpu/drm/i915/display/intel_lt_phy.c  |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index 00c7fa9040ee..716b5108b4c4 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -127,8 +127,8 @@ static void intel_cx0_phy_transaction_end(struct intel_encoder *encoder, struct
 	intel_display_power_put(display, POWER_DOMAIN_DC_OFF, wakeref);
 }
 
-void intel_clear_response_ready_flag(struct intel_encoder *encoder,
-				     int lane)
+void intel_cx0_clear_response_ready_flag(struct intel_encoder *encoder,
+					 int lane)
 {
 	struct intel_display *display = to_intel_display(encoder);
 
@@ -155,7 +155,7 @@ void intel_cx0_bus_reset(struct intel_encoder *encoder, int lane)
 		return;
 	}
 
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 }
 
 int intel_cx0_wait_for_ack(struct intel_encoder *encoder,
@@ -222,7 +222,7 @@ static int __intel_cx0_read_once(struct intel_encoder *encoder,
 		return -ETIMEDOUT;
 	}
 
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 
 	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, port, lane),
 		       XELPDP_PORT_M2P_TRANSACTION_PENDING |
@@ -233,7 +233,7 @@ static int __intel_cx0_read_once(struct intel_encoder *encoder,
 	if (ack < 0)
 		return ack;
 
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 
 	/*
 	 * FIXME: Workaround to let HW to settle
@@ -295,7 +295,7 @@ static int __intel_cx0_write_once(struct intel_encoder *encoder,
 		return -ETIMEDOUT;
 	}
 
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 
 	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display, port, lane),
 		       XELPDP_PORT_M2P_TRANSACTION_PENDING |
@@ -325,7 +325,7 @@ static int __intel_cx0_write_once(struct intel_encoder *encoder,
 		return -EINVAL;
 	}
 
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 
 	/*
 	 * FIXME: Workaround to let HW to settle
diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.h b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
index ae98ac23ea22..87d3bdaca3ec 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
@@ -25,8 +25,8 @@ struct intel_dpll_hw_state;
 struct intel_encoder;
 struct intel_hdmi;
 
-void intel_clear_response_ready_flag(struct intel_encoder *encoder,
-				     int lane);
+void intel_cx0_clear_response_ready_flag(struct intel_encoder *encoder,
+					 int lane);
 bool intel_encoder_is_c10phy(struct intel_encoder *encoder);
 void intel_mtl_pll_enable(struct intel_encoder *encoder,
 			  struct intel_dpll *pll,
diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c b/drivers/gpu/drm/i915/display/intel_lt_phy.c
index 6cdae03ee172..e174ca011d50 100644
--- a/drivers/gpu/drm/i915/display/intel_lt_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c
@@ -1106,7 +1106,7 @@ static int __intel_lt_phy_p2p_write_once(struct intel_encoder *encoder,
 	 * This is the time PHY takes to settle down after programming the PHY.
 	 */
 	udelay(150);
-	intel_clear_response_ready_flag(encoder, lane);
+	intel_cx0_clear_response_ready_flag(encoder, lane);
 	intel_lt_phy_clear_status_p2p(encoder, lane);
 
 	return 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✗ i915.CI.BAT: failure for Fix Cx0 Suspend Resume issue (rev2)
  2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
                   ` (2 preceding siblings ...)
  2026-01-14  3:42 ` [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag Suraj Kandpal
@ 2026-01-14  5:22 ` Patchwork
  2026-01-14 15:10 ` [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Rodrigo Vivi
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2026-01-14  5:22 UTC (permalink / raw)
  To: Kandpal, Suraj; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 3326 bytes --]

== Series Details ==

Series: Fix Cx0 Suspend Resume issue (rev2)
URL   : https://patchwork.freedesktop.org/series/159540/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_17817 -> Patchwork_159540v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_159540v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_159540v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/index.html

Participating hosts (43 -> 41)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_159540v2:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - bat-arls-6:         [PASS][1] -> [DMESG-WARN][2] +71 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17817/bat-arls-6/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/bat-arls-6/igt@i915_module_load@reload.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3:
    - bat-arls-5:         [PASS][3] -> [DMESG-WARN][4] +71 other tests dmesg-warn
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17817/bat-arls-5/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/bat-arls-5/igt@kms_flip@basic-flip-vs-wf_vblank@c-dp3.html

  
Known issues
------------

  Here are the changes found in Patchwork_159540v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_hdmi_inject@inject-audio:
    - fi-tgl-1115g4:      [PASS][5] -> [FAIL][6] ([i915#14867])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17817/fi-tgl-1115g4/igt@kms_hdmi_inject@inject-audio.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/fi-tgl-1115g4/igt@kms_hdmi_inject@inject-audio.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-9:         [DMESG-FAIL][7] ([i915#12061]) -> [PASS][8] +1 other test pass
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17817/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#14867]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14867


Build changes
-------------

  * Linux: CI_DRM_17817 -> Patchwork_159540v2

  CI-20190529: 20190529
  CI_DRM_17817: 1580579412915ac76344699e53124b580060302d @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8699: 0b67ab25f2eb58b296872c8c34474b79353727d5 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_159540v2: 1580579412915ac76344699e53124b580060302d @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_159540v2/index.html

[-- Attachment #2: Type: text/html, Size: 4018 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag
  2026-01-14  3:42 ` [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag Suraj Kandpal
@ 2026-01-14  6:03   ` Garg, Nemesa
  0 siblings, 0 replies; 9+ messages in thread
From: Garg, Nemesa @ 2026-01-14  6:03 UTC (permalink / raw)
  To: Kandpal, Suraj, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
  Cc: Nautiyal, Ankit K, Kandpal, Suraj



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Suraj
> Kandpal
> Sent: Wednesday, January 14, 2026 9:13 AM
> To: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Kandpal, Suraj
> <suraj.kandpal@intel.com>
> Subject: [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready
> flag
> 
> Rename the non static intel_clear_response_ready_flag to
> intel_cx0_clear_response_ready_flag so that we follow the naming standards
> of non static function.
> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c | 14 +++++++-------
> drivers/gpu/drm/i915/display/intel_cx0_phy.h |  4 ++--
> drivers/gpu/drm/i915/display/intel_lt_phy.c  |  2 +-
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 00c7fa9040ee..716b5108b4c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -127,8 +127,8 @@ static void intel_cx0_phy_transaction_end(struct
> intel_encoder *encoder, struct
>  	intel_display_power_put(display, POWER_DOMAIN_DC_OFF,
> wakeref);  }
> 
> -void intel_clear_response_ready_flag(struct intel_encoder *encoder,
> -				     int lane)
> +void intel_cx0_clear_response_ready_flag(struct intel_encoder *encoder,
> +					 int lane)
>  {
>  	struct intel_display *display = to_intel_display(encoder);
> 
> @@ -155,7 +155,7 @@ void intel_cx0_bus_reset(struct intel_encoder
> *encoder, int lane)
>  		return;
>  	}
> 
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
>  }
> 
>  int intel_cx0_wait_for_ack(struct intel_encoder *encoder, @@ -222,7 +222,7
> @@ static int __intel_cx0_read_once(struct intel_encoder *encoder,
>  		return -ETIMEDOUT;
>  	}
> 
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
> 
>  	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display,
> port, lane),
>  		       XELPDP_PORT_M2P_TRANSACTION_PENDING | @@ -
> 233,7 +233,7 @@ static int __intel_cx0_read_once(struct intel_encoder
> *encoder,
>  	if (ack < 0)
>  		return ack;
> 
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
> 
>  	/*
>  	 * FIXME: Workaround to let HW to settle @@ -295,7 +295,7 @@
> static int __intel_cx0_write_once(struct intel_encoder *encoder,
>  		return -ETIMEDOUT;
>  	}
> 
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
> 
>  	intel_de_write(display, XELPDP_PORT_M2P_MSGBUS_CTL(display,
> port, lane),
>  		       XELPDP_PORT_M2P_TRANSACTION_PENDING | @@ -
> 325,7 +325,7 @@ static int __intel_cx0_write_once(struct intel_encoder
> *encoder,
>  		return -EINVAL;
>  	}
> 
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
> 
>  	/*
>  	 * FIXME: Workaround to let HW to settle diff --git
> a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> index ae98ac23ea22..87d3bdaca3ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.h
> @@ -25,8 +25,8 @@ struct intel_dpll_hw_state;  struct intel_encoder;  struct
> intel_hdmi;
> 
> -void intel_clear_response_ready_flag(struct intel_encoder *encoder,
> -				     int lane);
> +void intel_cx0_clear_response_ready_flag(struct intel_encoder *encoder,
> +					 int lane);
>  bool intel_encoder_is_c10phy(struct intel_encoder *encoder);  void
> intel_mtl_pll_enable(struct intel_encoder *encoder,
>  			  struct intel_dpll *pll,
> diff --git a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> index 6cdae03ee172..e174ca011d50 100644
> --- a/drivers/gpu/drm/i915/display/intel_lt_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_lt_phy.c
> @@ -1106,7 +1106,7 @@ static int __intel_lt_phy_p2p_write_once(struct
> intel_encoder *encoder,
>  	 * This is the time PHY takes to settle down after programming the
> PHY.
>  	 */
>  	udelay(150);
> -	intel_clear_response_ready_flag(encoder, lane);
> +	intel_cx0_clear_response_ready_flag(encoder, lane);
>  	intel_lt_phy_clear_status_p2p(encoder, lane);
> 
>  	return 0;
> --
LGTM,
Reviewed-by: Nemesa Garg <nemesa.garg@intel.com>
> 2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/3] Fix Cx0 Suspend Resume issue
  2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
                   ` (3 preceding siblings ...)
  2026-01-14  5:22 ` ✗ i915.CI.BAT: failure for Fix Cx0 Suspend Resume issue (rev2) Patchwork
@ 2026-01-14 15:10 ` Rodrigo Vivi
  2026-01-14 15:42   ` Saarinen, Jani
  4 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2026-01-14 15:10 UTC (permalink / raw)
  To: Suraj Kandpal; +Cc: intel-xe, intel-gfx, ankit.k.nautiyal

On Wed, Jan 14, 2026 at 09:12:56AM +0530, Suraj Kandpal wrote:
> CX0 PHY currently has two issues which cause a hang when we try
> to suspend resume machine with a delay of 15mins and 1+ hour.
> This happens due to two reasons:
> 1) We do not follow the Enablement sequence where we need to
> enable our clock after PPS Enablement cycle
> 2) We do not make sure response ready and error bit are cleared
> in P2M_MSGBUS_STATUS before writing the transaction pending bit.
> This series aims to solve this.

Is there any Fixes: tag that we should add to any of the commits
in this series?

Also, next time, consider a fix as the first patch for easy backport
and the refactor on top.

Thanks,
Rodrigo.

> 
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> 
> Mika Kahola (1):
>   drm/i915/cx0: Split PLL enabling/disabling in two parts
> 
> Suraj Kandpal (2):
>   drm/i915/cx0: Clear response ready & error bit
>   drm/i915/cx0: Rename intel_clear_response_ready flag
> 
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 134 +++++++++++-------
>  drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   4 +-
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
>  drivers/gpu/drm/i915/display/intel_lt_phy.c   |   2 +-
>  4 files changed, 92 insertions(+), 55 deletions(-)
> 
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts
  2026-01-14  3:42 ` [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts Suraj Kandpal
@ 2026-01-14 15:12   ` Imre Deak
  0 siblings, 0 replies; 9+ messages in thread
From: Imre Deak @ 2026-01-14 15:12 UTC (permalink / raw)
  To: Suraj Kandpal
  Cc: intel-xe, intel-gfx, ankit.k.nautiyal, Mika Kahola,
	Michał Grzelak

On Wed, Jan 14, 2026 at 09:12:57AM +0530, Suraj Kandpal wrote:
> From: Mika Kahola <mika.kahola@intel.com>
> 
> Split PLL enabling/disabling in two parts - one for pll setting
> pll dividers and second one to enable/disable pll clock. PLL
> clock enabling/disbling happens via encoder->enable_clock/disable_clock
> function hook.

This is missing the rationale for the change. As I understand it
appeares to fix a PLL enabling/disabling timeout, if so that should be
described here in the commit log and also explain how the change fixes
the timeout, IOW wrt. what are the enabling/disabling steps gets
reordered?

Also, what about CMTG? It may require an output/DDI's PLL for another
ouput/DDI even if the former output/DDI is disabled.

Also, there is an ordering issue in the change, see below.

> PLL state verification happens now earlier than the clock is enabled
> which causes a drm warn to be thrown. Silence this warning by
> allowing this check for only earlier platforms than MeteorLake.
> 
> While at it also add the necessary argument to cx0_enable_clock
> so that we can move step 12 of the enable sequence.
> 
> v2:
> - Move state verification to enable_clock() function for
> MTL+ platforms
> - Squash patch 1 & 2 (Gustavo)
> - Use correct Bspec references (Gustavo)
> - Fix build error (Michal)
> 
> Bspec: 65448, 68849
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Reviewed-by: Michał Grzelak <michal.grzelak@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 120 +++++++++++-------
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
>  2 files changed, 80 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> index 7288065d2461..3418a3ed28fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> @@ -3225,11 +3225,8 @@ static void intel_cx0pll_enable(struct intel_encoder *encoder,
>  {
>  	int port_clock = pll_state->use_c10 ? pll_state->c10.clock : pll_state->c20.clock;
>  	struct intel_display *display = to_intel_display(encoder);
> -	enum phy phy = intel_encoder_to_phy(encoder);
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	bool lane_reversal = dig_port->lane_reversal;
> -	u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 :
> -					  INTEL_CX0_LANE0;
>  	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
>  
>  	/*
> @@ -3284,42 +3281,6 @@ static void intel_cx0pll_enable(struct intel_encoder *encoder,
>  	 */
>  	intel_de_write(display, DDI_CLK_VALFREQ(encoder->port), port_clock);
>  
> -	/*
> -	 * 9. Set PORT_CLOCK_CTL register PCLK PLL Request
> -	 * LN<Lane for maxPCLK> to "1" to enable PLL.
> -	 */
> -	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> -		     intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES),
> -		     intel_cx0_get_pclk_pll_request(maxpclk_lane));
> -
> -	/* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == "1". */
> -	if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> -			     intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES),
> -			     intel_cx0_get_pclk_pll_ack(maxpclk_lane),
> -			     XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL))
> -		drm_warn(display->drm, "Port %c PLL not locked\n",
> -			 phy_name(phy));
> -
> -	/*
> -	 * 11. Follow the Display Voltage Frequency Switching Sequence After
> -	 * Frequency Change. We handle this step in bxt_set_cdclk().
> -	 */
> -
> -	/*
> -	 * 12. Toggle powerdown if HDMI is enabled on C10 PHY.
> -	 *
> -	 * Wa_13013502646:
> -	 * Fixes: HDMI lane to lane skew violations on C10 display PHYs.
> -	 * Workaround: Toggle powerdown value by setting first to P0 and then to P2, for both
> -	 * PHY lanes.
> -	 */
> -	if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) {
> -		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
> -						    XELPDP_P0_STATE_ACTIVE);
> -		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
> -						    XELPDP_P2_STATE_READY);
> -	}
> -
>  	intel_cx0_phy_transaction_end(encoder, wakeref);
>  }
>  
> @@ -3403,6 +3364,56 @@ static int intel_mtl_tbt_clock_select(struct intel_display *display,
>  	}
>  }
>  
> +static void intel_cx0pll_enable_clock(struct intel_encoder *encoder,
> +				      const struct intel_cx0pll_state *pll_state)
> +{
> +	struct intel_display *display = to_intel_display(encoder);
> +	enum phy phy = intel_encoder_to_phy(encoder);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	bool lane_reversal = dig_port->lane_reversal;
> +	u8 maxpclk_lane = lane_reversal ? INTEL_CX0_LANE1 :
> +					  INTEL_CX0_LANE0;
> +	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
> +
> +	/*
> +	 * 9. Set PORT_CLOCK_CTL register PCLK PLL Request
> +	 * LN<Lane for maxPCLK> to "1" to enable PLL.
> +	 */
> +	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> +		     intel_cx0_get_pclk_pll_request(INTEL_CX0_BOTH_LANES),
> +		     intel_cx0_get_pclk_pll_request(maxpclk_lane));
> +
> +	/* 10. Poll on PORT_CLOCK_CTL PCLK PLL Ack LN<Lane for maxPCLK> == "1". */
> +	if (intel_de_wait_us(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> +			     intel_cx0_get_pclk_pll_ack(INTEL_CX0_BOTH_LANES),
> +			     intel_cx0_get_pclk_pll_ack(maxpclk_lane),
> +			     XELPDP_PCLK_PLL_ENABLE_TIMEOUT_US, NULL))
> +		drm_warn(display->drm, "Port %c PLL not locked\n",
> +			 phy_name(phy));
> +
> +	/*
> +	 * 11. Follow the Display Voltage Frequency Switching Sequence After
> +	 * Frequency Change. We handle this step in bxt_set_cdclk().
> +	 */
> +
> +	/*
> +	 * 12. Toggle powerdown if HDMI is enabled on C10 PHY.
> +	 *
> +	 * Wa_13013502646:
> +	 * Fixes: HDMI lane to lane skew violations on C10 display PHYs.
> +	 * Workaround: Toggle powerdown value by setting first to P0 and then to P2, for both
> +	 * PHY lanes.
> +	 */
> +	if (!cx0pll_state_is_dp(pll_state) && pll_state->use_c10) {
> +		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
> +						    XELPDP_P0_STATE_ACTIVE);
> +		intel_cx0_powerdown_change_sequence(encoder, INTEL_CX0_BOTH_LANES,
> +						    XELPDP_P2_STATE_READY);
> +	}
> +
> +	intel_cx0_phy_transaction_end(encoder, wakeref);
> +}
> +
>  void intel_mtl_tbt_pll_enable_clock(struct intel_encoder *encoder, int port_clock)
>  {
>  	struct intel_display *display = to_intel_display(encoder);
> @@ -3468,10 +3479,16 @@ void intel_mtl_pll_enable(struct intel_encoder *encoder,
>  void intel_mtl_pll_enable_clock(struct intel_encoder *encoder,
>  				const struct intel_crtc_state *crtc_state)
>  {
> +	struct intel_display *display = to_intel_display(encoder);
> +	struct intel_dpll *pll = crtc_state->intel_dpll;
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  
>  	if (intel_tc_port_in_tbt_alt_mode(dig_port))
>  		intel_mtl_tbt_pll_enable_clock(encoder, crtc_state->port_clock);
> +	else
> +		intel_cx0pll_enable_clock(encoder, &crtc_state->dpll_hw_state.cx0pll);
> +
> +	assert_dpll_enabled(display, pll);
>  }
>  
>  /*
> @@ -3567,12 +3584,6 @@ static void intel_cx0pll_disable(struct intel_encoder *encoder)
>  	 * Frequency Change. We handle this step in bxt_set_cdclk().
>  	 */
>  
> -	/* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */
> -	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> -		     XELPDP_DDI_CLOCK_SELECT_MASK(display), 0);
> -	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> -		     XELPDP_FORWARD_CLOCK_UNGATE, 0);
> -
>  	intel_cx0_phy_transaction_end(encoder, wakeref);
>  }
>  
> @@ -3586,6 +3597,20 @@ static bool intel_cx0_pll_is_enabled(struct intel_encoder *encoder)
>  			     intel_cx0_get_pclk_pll_request(lane);
>  }
>  
> +static void intel_cx0pll_disable_clock(struct intel_encoder *encoder)
> +{
> +	struct intel_display *display = to_intel_display(encoder);
> +	struct ref_tracker *wakeref = intel_cx0_phy_transaction_begin(encoder);
> +
> +	/* 7. Program PORT_CLOCK_CTL register to disable and gate clocks. */
> +	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> +		     XELPDP_DDI_CLOCK_SELECT_MASK(display), 0);
> +	intel_de_rmw(display, XELPDP_PORT_CLOCK_CTL(display, encoder->port),
> +		     XELPDP_FORWARD_CLOCK_UNGATE, 0);
> +
> +	intel_cx0_phy_transaction_end(encoder, wakeref);

During a modeset disable sequence encoder::disable_clock() is called
first and only afterwards intel_dpll_funcs::disable() is called. So the
above will incorrectly reorder the clearing of the clock-select and
forward-clock-ungate flags wrt. the rest of the steps
intel_cx0pll_disable(), i.e. not matching now bspec at all.

> +}
> +
>  void intel_mtl_tbt_pll_disable_clock(struct intel_encoder *encoder)
>  {
>  	struct intel_display *display = to_intel_display(encoder);
> @@ -3635,6 +3660,9 @@ void intel_mtl_pll_disable_clock(struct intel_encoder *encoder)
>  
>  	if (intel_tc_port_in_tbt_alt_mode(dig_port))
>  		intel_mtl_tbt_pll_disable_clock(encoder);
> +	else
> +		intel_cx0pll_disable_clock(encoder);
> +
>  }
>  
>  enum icl_port_dpll_id
> @@ -3783,6 +3811,8 @@ void intel_cx0_pll_power_save_wa(struct intel_display *display)
>  			    encoder->base.base.id, encoder->base.name);
>  
>  		intel_cx0pll_enable(encoder, &pll_state);
> +		intel_cx0pll_enable_clock(encoder, &pll_state);
>  		intel_cx0pll_disable(encoder);
> +		intel_cx0pll_disable_clock(encoder);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 9aa84a430f09..040c97d81302 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -298,7 +298,8 @@ void intel_dpll_enable(const struct intel_crtc_state *crtc_state)
>  
>  	if (old_mask) {
>  		drm_WARN_ON(display->drm, !pll->on);
> -		assert_dpll_enabled(display, pll);
> +		if (DISPLAY_VER(display) < 14)
> +			assert_dpll_enabled(display, pll);
>  		goto out;
>  	}
>  	drm_WARN_ON(display->drm, pll->on);
> @@ -342,7 +343,9 @@ void intel_dpll_disable(const struct intel_crtc_state *crtc_state)
>  		    pll->info->name, pll->active_mask, pll->on,
>  		    crtc->base.base.id, crtc->base.name);
>  
> -	assert_dpll_enabled(display, pll);
> +	if (DISPLAY_VER(display) < 14)
> +		assert_dpll_enabled(display, pll);
> +
>  	drm_WARN_ON(display->drm, !pll->on);
>  
>  	pll->active_mask &= ~pipe_mask;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v2 0/3] Fix Cx0 Suspend Resume issue
  2026-01-14 15:10 ` [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Rodrigo Vivi
@ 2026-01-14 15:42   ` Saarinen, Jani
  0 siblings, 0 replies; 9+ messages in thread
From: Saarinen, Jani @ 2026-01-14 15:42 UTC (permalink / raw)
  To: Vivi, Rodrigo, Kandpal, Suraj
  Cc: intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	Nautiyal, Ankit K

Hi, 
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> Rodrigo Vivi
> Sent: Wednesday, 14 January 2026 17.10
> To: Kandpal, Suraj <suraj.kandpal@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Nautiyal,
> Ankit K <ankit.k.nautiyal@intel.com>
> Subject: Re: [PATCH v2 0/3] Fix Cx0 Suspend Resume issue
> 
> On Wed, Jan 14, 2026 at 09:12:56AM +0530, Suraj Kandpal wrote:
> > CX0 PHY currently has two issues which cause a hang when we try to
> > suspend resume machine with a delay of 15mins and 1+ hour.
> > This happens due to two reasons:
> > 1) We do not follow the Enablement sequence where we need to enable
> > our clock after PPS Enablement cycle
> > 2) We do not make sure response ready and error bit are cleared in
> > P2M_MSGBUS_STATUS before writing the transaction pending bit.
> > This series aims to solve this.
> 
> Is there any Fixes: tag that we should add to any of the commits in this series?
> 
> Also, next time, consider a fix as the first patch for easy backport and the
> refactor on top.
Looking at both i915 and xe CI results this is not really ready as is. 

Br,
Jani
> 
> Thanks,
> Rodrigo.
> 
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > Mika Kahola (1):
> >   drm/i915/cx0: Split PLL enabling/disabling in two parts
> >
> > Suraj Kandpal (2):
> >   drm/i915/cx0: Clear response ready & error bit
> >   drm/i915/cx0: Rename intel_clear_response_ready flag
> >
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.c  | 134 +++++++++++-------
> >  drivers/gpu/drm/i915/display/intel_cx0_phy.h  |   4 +-
> >  drivers/gpu/drm/i915/display/intel_dpll_mgr.c |   7 +-
> >  drivers/gpu/drm/i915/display/intel_lt_phy.c   |   2 +-
> >  4 files changed, 92 insertions(+), 55 deletions(-)
> >
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-01-14 15:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14  3:42 [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Suraj Kandpal
2026-01-14  3:42 ` [PATCH v2 1/3] drm/i915/cx0: Split PLL enabling/disabling in two parts Suraj Kandpal
2026-01-14 15:12   ` Imre Deak
2026-01-14  3:42 ` [PATCH v2 2/3] drm/i915/cx0: Clear response ready & error bit Suraj Kandpal
2026-01-14  3:42 ` [PATCH v2 3/3] drm/i915/cx0: Rename intel_clear_response_ready flag Suraj Kandpal
2026-01-14  6:03   ` Garg, Nemesa
2026-01-14  5:22 ` ✗ i915.CI.BAT: failure for Fix Cx0 Suspend Resume issue (rev2) Patchwork
2026-01-14 15:10 ` [PATCH v2 0/3] Fix Cx0 Suspend Resume issue Rodrigo Vivi
2026-01-14 15:42   ` Saarinen, Jani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox