[PATCH 0/2] cdclk mismatch with sw/hw

[PATCH 0/2] cdclk mismatch with sw/hw

[Arun R Murthy]

With 4k eDP panel and an external DP, cdclk sw/hw state mismatch
warnings are observed along with cdclk mismatch on DC6 exit is also
observed and eventually FIFO underrun.

On boot, skl_cdclk_init_hw()->skl_sanitize_cdclk() decides to use the
pre-OS cdclk leading to programming a very low cdclk that doesnt satisfy
the pipes requirement. This leads to FIFO underrun.
Also this programmed and the sw calculated cdclk mismatches and on every
dc state exit warnings are thrown.

In this series the cdclk is programmed to max instead of 0, and then the
first modeset will bring it back to the required/calculated.

Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
---
Arun R Murthy (2):
      drm/i915/display: Seed skl_cdclk_init_hw() with SKU-max, not zero
      drm/i915/display: Reconcile cdclk SW state with HW on DC-off exit

 drivers/gpu/drm/i915/display/intel_cdclk.c         | 65 +++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_cdclk.h         |  2 +
 .../drm/i915/display/intel_display_power_well.c    |  1 +
 3 files changed, 67 insertions(+), 1 deletion(-)
---
base-commit: 0377b35cbeef071dfeed06ccadc0d7607eccaa64
change-id: 20260804-cdclk-9e7ffc71f82a

Best regards,
-- 
Arun R Murthy <arun.r.murthy@intel.com>

[PATCH 1/2] drm/i915/display: Seed skl_cdclk_init_hw() with SKU-max, not zero

[Arun R Murthy]

skl_cdclk_init_hw() has a recovery path that runs when skl_sanitize_cdclk()
has decided the pre-OS CDCLK state is not usable and zeroes
display->cdclk.hw.

Seed with the SKU-permitted maximum instead (as read from SKL_DFSM), so
the recovery path lands on a cdclk that satisfies every pipe on the SKU.
The very first atomic commit that follows will dial cdclk down to what
active pipes actually need via the normal skl_modeset_calc_cdclk() path.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16595
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Tested-by: Polo-François Poli <polo.code.dev@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 65 +++++++++++++++++++++++++++++-
 1 file changed, 64 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index a53d887271778193fd400b9199ad841d5d9730fe..19f3ef8adfcded378428924539db9336834278ea 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1288,6 +1288,31 @@ static void skl_sanitize_cdclk(struct intel_display *display)
 	display->cdclk.hw.vco = ~0;
 }
 
+/*
+ * Seed cdclk from the SKU-permitted maximum. Used by skl_cdclk_init_hw()'s
+ * recovery path when skl_sanitize_cdclk() has decided the pre-OS CDCLK state
+ * is not usable and has zeroed display->cdclk.hw.
+ * Seed with the SKU top instead: it is guaranteed to satisfy every pipe on
+ * the platform, and the very first atomic commit will dial cdclk down via
+ * skl_modeset_calc_cdclk() once per-pipe min_cdclk is known.
+ */
+static int skl_cdclk_init_hw_seed(struct intel_display *display, int vco)
+{
+	u32 limit = intel_de_read(display, SKL_DFSM) & SKL_DFSM_CDCLK_LIMIT_MASK;
+	int max_cdclk;
+
+	if (limit == SKL_DFSM_CDCLK_LIMIT_675)
+		max_cdclk = 617143;
+	else if (limit == SKL_DFSM_CDCLK_LIMIT_540)
+		max_cdclk = 540000;
+	else if (limit == SKL_DFSM_CDCLK_LIMIT_450)
+		max_cdclk = 432000;
+	else
+		max_cdclk = 308571;
+
+	return skl_calc_cdclk(max_cdclk, vco);
+}
+
 static void skl_cdclk_init_hw(struct intel_display *display)
 {
 	struct intel_cdclk_config cdclk_config;
@@ -1311,9 +1336,14 @@ static void skl_cdclk_init_hw(struct intel_display *display)
 	cdclk_config.vco = display->cdclk.skl_preferred_vco_freq;
 	if (cdclk_config.vco == 0)
 		cdclk_config.vco = 8100000;
-	cdclk_config.cdclk = skl_calc_cdclk(0, cdclk_config.vco);
+
+	cdclk_config.cdclk = skl_cdclk_init_hw_seed(display, cdclk_config.vco);
 	cdclk_config.voltage_level = skl_calc_voltage_level(cdclk_config.cdclk);
 
+	drm_dbg_kms(display->drm,
+		    "cdclk: init seed cdclk=%d vco=%d volt=%d (SKU-max, no per-pipe min_cdclk yet)\n",
+		    cdclk_config.cdclk, cdclk_config.vco, cdclk_config.voltage_level);
+
 	skl_set_cdclk(display, &cdclk_config, INVALID_PIPE);
 }
 
@@ -4231,6 +4261,39 @@ void intel_cdclk_read_hw(struct intel_display *display)
 	cdclk_state->logical = display->cdclk.hw;
 }
 
+/**
+ * intel_cdclk_sync_hw_state - Adopt the HW-asserted cdclk into SW state.
+ * @display: display instance
+ * @hw_config: the cdclk configuration as currently read back from HW
+ *
+ * This helper reconciles the SW state to match what HW is currently asserting.
+ * It updates display->cdclk.hw and the committed cdclk_state's actual/logical
+ * so that any following atomic commit sees a truthful baseline and will
+ * naturally attach a cdclk_state (via intel_cdclk_atomic_check()) to raise
+ * cdclk when the workload demands it.
+ */
+void intel_cdclk_sync_hw_state(struct intel_display *display,
+			       const struct intel_cdclk_config *hw_config)
+{
+	struct intel_cdclk_state *cdclk_state =
+		to_intel_cdclk_state(display->cdclk.obj.state);
+
+	if (!intel_cdclk_clock_changed(&display->cdclk.hw, hw_config))
+		return;
+
+	drm_dbg_kms(display->drm,
+		    "cdclk: syncing SW state to HW after drift (sw %d/vco %d/ref %d -> hw %d/vco %d/ref %d)\n",
+		    display->cdclk.hw.cdclk, display->cdclk.hw.vco, display->cdclk.hw.ref,
+		    hw_config->cdclk, hw_config->vco, hw_config->ref);
+
+	display->cdclk.hw.cdclk = hw_config->cdclk;
+	display->cdclk.hw.vco = hw_config->vco;
+	display->cdclk.hw.ref = hw_config->ref;
+
+	cdclk_state->actual = display->cdclk.hw;
+	cdclk_state->logical = display->cdclk.hw;
+}
+
 static int calc_cdclk(const struct intel_crtc_state *crtc_state, int min_cdclk)
 {
 	struct intel_display *display = to_intel_display(crtc_state);

-- 
2.25.1

[PATCH 2/2] drm/i915/display: Reconcile cdclk SW state with HW on DC-off exit

[Arun R Murthy]

Add intel_cdclk_sync_hw_state() which adopts the HW-asserted cdclk into
display->cdclk.hw and the committed cdclk_state (actual/logical) so that
future atomic checks operate on a truthful baseline. Any pipe that needs
more cdclk than HW is currently delivering will then naturally attach a
cdclk_state via intel_cdclk_atomic_check() and raise cdclk through the
normal modeset path.

voltage_level cannot be reliably read back on all gens, so the SW value
is preserved.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16595
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Tested-by: Polo-François Poli <polo.code.dev@gmail.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.h              | 2 ++
 drivers/gpu/drm/i915/display/intel_display_power_well.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index a60cbf745ee2638a092dbdce98b994b17d7009ef..ed78c855320ad4e92364989fd3f9248748a9de8a 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -69,6 +69,8 @@ int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe
 bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
 void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
 void intel_cdclk_read_hw(struct intel_display *display);
+void intel_cdclk_sync_hw_state(struct intel_display *display,
+			       const struct intel_cdclk_config *hw_config);
 
 unsigned int intel_cdclk_prefill_adjustment(const struct intel_crtc_state *crtc_state);
 unsigned int intel_cdclk_prefill_adjustment_worst(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power_well.c b/drivers/gpu/drm/i915/display/intel_display_power_well.c
index 02cb4d800e2389fc2a7d958ed78339a74f5c057e..11b8521d9d0c8b5381d3b90b5cbf24b97deacf65 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power_well.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power_well.c
@@ -1105,6 +1105,7 @@ void gen9_disable_dc_states(struct intel_display *display)
 
 	intel_cdclk_get_cdclk(display, &cdclk_config);
 	/* Can't read out voltage_level so can't use intel_cdclk_changed() */
+	intel_cdclk_sync_hw_state(display, &cdclk_config);
 	drm_WARN_ON(display->drm,
 		    intel_cdclk_clock_changed(&display->cdclk.hw,
 					      &cdclk_config));

-- 
2.25.1