[PATCH 0/8] Enable LNL display

[PATCH 0/8] Enable LNL display

[Gustavo Sousa]

This series aims at providing the remaining patches for enabling display
on Lunar Lake, which used Xe2LPD display IP.

The first set of patches contains fixes and extra stuff required for
supporting CDCLK on Xe2LPD:

    drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table
    drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()
    drm/i915/cdclk: Only compute squash waveform when necessary
    drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()
    drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state
    drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

Then we have a single patch that enables loading Xe2LPD DMC, which is
already available in linux-firmware:

    drm/i915/xe2lpd: Load DMC

Finally, we have the last patch, which enables display for LNL:

    drm/xe/lnl: Enable display support

I believe this last one is supposed to be merged via drm-xe-next.

--
Gustavo Sousa

Balasubramani Vivekanandan (2):
  drm/i915/xe2lpd: Load DMC
  drm/xe/lnl: Enable display support

Gustavo Sousa (6):
  drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table
  drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()
  drm/i915/cdclk: Only compute squash waveform when necessary
  drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()
  drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state
  drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

 drivers/gpu/drm/i915/display/intel_cdclk.c    | 83 +++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  5 ++
 drivers/gpu/drm/i915/display/intel_dmc.c      |  9 +-
 drivers/gpu/drm/i915/display/skl_watermark.c  | 74 ++++++++++++++---
 drivers/gpu/drm/i915/display/skl_watermark.h  |  4 +
 .../gpu/drm/i915/display/skl_watermark_regs.h | 18 ++--
 drivers/gpu/drm/i915/i915_reg.h               |  4 +-
 drivers/gpu/drm/xe/xe_pci.c                   |  1 +
 8 files changed, 170 insertions(+), 28 deletions(-)

-- 
2.44.0

[PATCH 1/8] drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table

[Gustavo Sousa]

The CDCLK table is tied to Xe2LPD display and not to the platform. Let's
rename lnl_cdclk_table to xe2lpd_cdclk_table in order to reflect that.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 22473c55b899..407bd541eb46 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1417,7 +1417,7 @@ static const struct intel_cdclk_vals mtl_cdclk_table[] = {
 	{}
 };
 
-static const struct intel_cdclk_vals lnl_cdclk_table[] = {
+static const struct intel_cdclk_vals xe2lpd_cdclk_table[] = {
 	{ .refclk = 38400, .cdclk = 153600, .ratio = 16, .waveform = 0xaaaa },
 	{ .refclk = 38400, .cdclk = 172800, .ratio = 16, .waveform = 0xad5a },
 	{ .refclk = 38400, .cdclk = 192000, .ratio = 16, .waveform = 0xb6b6 },
@@ -3710,7 +3710,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
 	if (DISPLAY_VER(dev_priv) >= 20) {
 		dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs;
-		dev_priv->display.cdclk.table = lnl_cdclk_table;
+		dev_priv->display.cdclk.table = xe2lpd_cdclk_table;
 	} else if (DISPLAY_VER(dev_priv) >= 14) {
 		dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs;
 		dev_priv->display.cdclk.table = mtl_cdclk_table;
-- 
2.44.0

[PATCH 2/8] drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()

[Gustavo Sousa]

There will be future changes that rely on the source of the MDCLK. Let's
have xe2lpd_mdclk_source_sel() as the function responsible for reporting
that information.

Bspec: 69090
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 17 ++++++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h            |  4 +++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 407bd541eb46..bf84bf27213f 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -1876,6 +1876,21 @@ static bool cdclk_pll_is_unknown(unsigned int vco)
 	return vco == ~0;
 }
 
+static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
+{
+	if (DISPLAY_VER(i915) >= 20)
+		return MDCLK_SOURCE_SEL_CDCLK_PLL;
+
+	/*
+	 * Earlier display IPs do not provide means of selecting the
+	 * MDCLK source, but MDCLK_SOURCE_SEL_CD2XCLK is a nice default,
+	 * since it reflects the source used for those and allows
+	 * xe2lpd_mdclk_source_sel() to be used in logic that depends on
+	 * it.
+	 */
+	return MDCLK_SOURCE_SEL_CD2XCLK;
+}
+
 static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
 						    const struct intel_cdclk_config *old_cdclk_config,
 						    const struct intel_cdclk_config *new_cdclk_config,
@@ -1980,7 +1995,7 @@ static u32 bxt_cdclk_ctl(struct drm_i915_private *i915,
 		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
 
 	if (DISPLAY_VER(i915) >= 20)
-		val |= MDCLK_SOURCE_SEL_CDCLK_PLL;
+		val |= xe2lpd_mdclk_source_sel(i915);
 	else
 		val |= skl_cdclk_decimal(cdclk);
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e00557e1a57f..eb953ed1f113 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5900,7 +5900,9 @@ enum skl_power_gate {
 #define  CDCLK_FREQ_540		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
 #define  CDCLK_FREQ_337_308		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
 #define  CDCLK_FREQ_675_617		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
-#define  MDCLK_SOURCE_SEL_CDCLK_PLL	REG_BIT(25)
+#define  MDCLK_SOURCE_SEL_MASK		REG_GENMASK(25, 25)
+#define  MDCLK_SOURCE_SEL_CD2XCLK	REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 0)
+#define  MDCLK_SOURCE_SEL_CDCLK_PLL	REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 1)
 #define  BXT_CDCLK_CD2X_DIV_SEL_MASK	REG_GENMASK(23, 22)
 #define  BXT_CDCLK_CD2X_DIV_SEL_1	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
 #define  BXT_CDCLK_CD2X_DIV_SEL_1_5	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
-- 
2.44.0

[PATCH 3/8] drm/i915/cdclk: Only compute squash waveform when necessary

[Gustavo Sousa]

It is no use computing the squash waveform if we are not going to use
it. Move the call to cdclk_squash_waveform() inside the block guarded by
HAS_CDCLK_SQUASH(dev_priv).

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index bf84bf27213f..cdf3ae766f9e 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -2023,10 +2023,11 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
 	} else
 		bxt_cdclk_pll_update(dev_priv, vco);
 
-	waveform = cdclk_squash_waveform(dev_priv, cdclk);
+	if (HAS_CDCLK_SQUASH(dev_priv)) {
+		waveform = cdclk_squash_waveform(dev_priv, cdclk);
 
-	if (HAS_CDCLK_SQUASH(dev_priv))
 		dg2_cdclk_squash_program(dev_priv, waveform);
+	}
 
 	intel_de_write(dev_priv, CDCLK_CTL, bxt_cdclk_ctl(dev_priv, cdclk_config, pipe));
 
-- 
2.44.0

[PATCH 4/8] drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()

[Gustavo Sousa]

As of Xe2LPD, it is now possible to select the source of the MDCLK
as either the CD2XCLK or the CDCLK PLL.

Previous display IPs were hardcoded to use the CD2XCLK. For those, the
ratio between MDCLK and CDCLK remained constant, namely 2. For Xe2LPD,
when we select the CDCLK PLL as the source, the ratio will vary
according to the squashing configuration (since the cd2x divisor is
fixed for all supported configurations).

To help the transition to supporting changes in the ratio, extract the
function intel_dbuf_mdclk_cdclk_ratio_update() from the existing logic
and call it using 2 as hardcoded ratio. Upcoming changes will use that
function for updates in the ratio due to CDCLK changes.

Bspec: 50057, 69445, 49213, 68868
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 30 +++++++++++++-------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index c6b9be80d83c..d9e49cd60d3a 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -3530,6 +3530,21 @@ int intel_dbuf_init(struct drm_i915_private *i915)
 	return 0;
 }
 
+static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
+						u8 ratio,
+						bool joined_mbus)
+{
+	enum dbuf_slice slice;
+
+	if (joined_mbus)
+		ratio *= 2;
+
+	for_each_dbuf_slice(i915, slice)
+		intel_de_rmw(i915, DBUF_CTL_S(slice),
+			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
+			     DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1));
+}
+
 /*
  * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before
  * update the request state of all DBUS slices.
@@ -3537,8 +3552,7 @@ int intel_dbuf_init(struct drm_i915_private *i915)
 static void update_mbus_pre_enable(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
-	u32 mbus_ctl, dbuf_min_tracker_val;
-	enum dbuf_slice slice;
+	u32 mbus_ctl;
 	const struct intel_dbuf_state *dbuf_state =
 		intel_atomic_get_new_dbuf_state(state);
 
@@ -3549,24 +3563,18 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 	 * TODO: Implement vblank synchronized MBUS joining changes.
 	 * Must be properly coordinated with dbuf reprogramming.
 	 */
-	if (dbuf_state->joined_mbus) {
+	if (dbuf_state->joined_mbus)
 		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
 			MBUS_JOIN_PIPE_SELECT_NONE;
-		dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(3);
-	} else {
+	else
 		mbus_ctl = MBUS_HASHING_MODE_2x2 |
 			MBUS_JOIN_PIPE_SELECT_NONE;
-		dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(1);
-	}
 
 	intel_de_rmw(i915, MBUS_CTL,
 		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
 		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
 
-	for_each_dbuf_slice(i915, slice)
-		intel_de_rmw(i915, DBUF_CTL_S(slice),
-			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
-			     dbuf_min_tracker_val);
+	intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus);
 }
 
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
-- 
2.44.0

[PATCH 5/8] drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state

[Gustavo Sousa]

CDCLK programming Xe2LPD always selects the CDCLK PLL as source for the
MDCLK. Because of that, the ratio between MDCLK and CDCLK is not be
constant anymore. As such, make sure to have the current ratio available
in intel_dbuf_state so that it can be used during dbuf programming.

Note that we write-lock the global state instead of serializing to a
hardware commit because a change in the ratio should be rather handled
in the CDCLK change sequence, which will need to take care of updating
the necessary registers in that case. We will implement that in upcoming
changes.

That said, changes in the MBus joining state should be handled by the
DBUF/MBUS logic, just like it is already done, but the logic will need
to know the ratio to properly update the registers.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c   | 26 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h   |  2 ++
 drivers/gpu/drm/i915/display/skl_watermark.c | 18 +++++++++++++-
 drivers/gpu/drm/i915/display/skl_watermark.h |  3 +++
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index cdf3ae766f9e..04a6e9806254 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -39,6 +39,7 @@
 #include "intel_pcode.h"
 #include "intel_psr.h"
 #include "intel_vdsc.h"
+#include "skl_watermark.h"
 #include "vlv_sideband.h"
 
 /**
@@ -1891,6 +1892,22 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
 	return MDCLK_SOURCE_SEL_CD2XCLK;
 }
 
+u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
+			   const struct intel_cdclk_config *cdclk_config)
+{
+	u32 source_sel = xe2lpd_mdclk_source_sel(i915);
+
+	switch (source_sel) {
+	case MDCLK_SOURCE_SEL_CD2XCLK:
+		return 2;
+	case MDCLK_SOURCE_SEL_CDCLK_PLL:
+		return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk);
+	default:
+		MISSING_CASE(source_sel);
+		return 2;
+	}
+}
+
 static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
 						    const struct intel_cdclk_config *old_cdclk_config,
 						    const struct intel_cdclk_config *new_cdclk_config,
@@ -3281,6 +3298,15 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 			    "Modeset required for cdclk change\n");
 	}
 
+	if (intel_mdclk_cdclk_ratio(dev_priv, &old_cdclk_state->actual) !=
+	    intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual)) {
+		u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual);
+
+		ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio);
+		if (ret)
+			return ret;
+	}
+
 	drm_dbg_kms(&dev_priv->drm,
 		    "New cdclk calculated to be logical %u kHz, actual %u kHz\n",
 		    new_cdclk_state->logical.cdclk,
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index fa301495e7f1..8e6e302bd599 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -62,6 +62,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv);
 u32 intel_read_rawclk(struct drm_i915_private *dev_priv);
 bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a,
 			       const struct intel_cdclk_config *b);
+u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
+			   const struct intel_cdclk_config *cdclk_config);
 void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state);
 void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state);
 void intel_cdclk_dump_config(struct drm_i915_private *i915,
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index d9e49cd60d3a..4410e21888ad 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -3057,6 +3057,8 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
 	if (HAS_MBUS_JOINING(i915))
 		dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
 
+	dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(i915, &i915->display.cdclk.hw);
+
 	for_each_intel_crtc(&i915->drm, crtc) {
 		struct intel_crtc_state *crtc_state =
 			to_intel_crtc_state(crtc->base.state);
@@ -3530,6 +3532,19 @@ int intel_dbuf_init(struct drm_i915_private *i915)
 	return 0;
 }
 
+int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio)
+{
+	struct intel_dbuf_state *dbuf_state;
+
+	dbuf_state = intel_atomic_get_dbuf_state(state);
+	if (IS_ERR(dbuf_state))
+		return PTR_ERR(dbuf_state);
+
+	dbuf_state->mdclk_cdclk_ratio = ratio;
+
+	return intel_atomic_lock_global_state(&dbuf_state->base);
+}
+
 static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
 						u8 ratio,
 						bool joined_mbus)
@@ -3574,7 +3589,8 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
 		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
 
-	intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus);
+	intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
+					    dbuf_state->joined_mbus);
 }
 
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index e3d1d74a7b17..fed4d12df584 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -58,6 +58,7 @@ struct intel_dbuf_state {
 	u8 slices[I915_MAX_PIPES];
 	u8 enabled_slices;
 	u8 active_pipes;
+	u8 mdclk_cdclk_ratio;
 	bool joined_mbus;
 };
 
@@ -71,6 +72,8 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->display.dbuf.obj))
 
 int intel_dbuf_init(struct drm_i915_private *i915);
+int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio);
+
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
 void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
 void intel_mbus_dbox_update(struct intel_atomic_state *state);
-- 
2.44.0

[PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

[Gustavo Sousa]

Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit
3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started
adding support for CDCLK programming support for Xe2LPD. One final piece
is missing, which is the programming necessary for changed in the ratio
between MDCLK and CDCLK. Let's do that now.

BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the
ratio between MDCLK and CDCLK changes. The updates must be done before
changing the CDCLK when decreasing the frequency; or after it when
increasing the frequency.

Ratio-related updates to MBUS_CTL also depend on the state of MBus
joining, so they are performed by either CDCLK change sequence or by
changes in MBus joining. Since one might happen independently of the
other, we need to make sure that both logics see the necessary state
values when programming that register. MBus joining logic needs to know
the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio
field of struct intel_dbuf_state.

For the CDCLK logic, we need to have something similar: we need to
propagate the status of MBus joining to struct intel_cdclk_state. Do
that by adding the field joined_mbus to struct intel_cdclk_config.
(Preferably, that field would be added to intel_cdclk_state, however
currently only intel_cdclk_config is passed down to the functions that
do the register programming. We might revisit this decision if we find
that refactoring the code to pass the whole intel_cdclk_state is worth
it.)

Bspec: 68864, 68868, 69090, 69482
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 31 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  3 ++
 drivers/gpu/drm/i915/display/skl_watermark.c  | 40 +++++++++++++++----
 drivers/gpu/drm/i915/display/skl_watermark.h  |  1 +
 .../gpu/drm/i915/display/skl_watermark_regs.h | 18 +++++----
 5 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 04a6e9806254..12753589072d 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -40,6 +40,7 @@
 #include "intel_psr.h"
 #include "intel_vdsc.h"
 #include "skl_watermark.h"
+#include "skl_watermark_regs.h"
 #include "vlv_sideband.h"
 
 /**
@@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
 	}
 
  out:
+	if (DISPLAY_VER(dev_priv) >= 20)
+		cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
 	/*
 	 * Can't read this out :( Let's assume it's
 	 * at least what the CDCLK frequency requires.
@@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
 	}
 }
 
+static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915,
+					     const struct intel_cdclk_config *cdclk_config)
+{
+	intel_dbuf_mdclk_cdclk_ratio_update(i915,
+					    intel_mdclk_cdclk_ratio(i915, cdclk_config),
+					    cdclk_config->joined_mbus);
+}
+
 static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
 						    const struct intel_cdclk_config *old_cdclk_config,
 						    const struct intel_cdclk_config *new_cdclk_config,
@@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		return;
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 20 && cdclk < dev_priv->display.cdclk.hw.cdclk)
+		xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
+
 	if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, &dev_priv->display.cdclk.hw,
 						    cdclk_config, &mid_cdclk_config)) {
 		_bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe);
@@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
 		_bxt_set_cdclk(dev_priv, cdclk_config, pipe);
 	}
 
+	if (DISPLAY_VER(dev_priv) >= 20 && cdclk > dev_priv->display.cdclk.hw.cdclk)
+		xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
+
 	if (DISPLAY_VER(dev_priv) >= 14)
 		/*
 		 * NOOP - No Pcode communication needed for
@@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
 	return 0;
 }
 
+int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus)
+{
+	struct intel_cdclk_state *cdclk_state;
+
+	cdclk_state = intel_atomic_get_cdclk_state(state);
+	if (IS_ERR(cdclk_state))
+		return PTR_ERR(cdclk_state);
+
+	cdclk_state->actual.joined_mbus = joined_mbus;
+	cdclk_state->logical.joined_mbus = joined_mbus;
+
+	return intel_atomic_lock_global_state(&cdclk_state->base);
+}
+
 int intel_cdclk_init(struct drm_i915_private *dev_priv)
 {
 	struct intel_cdclk_state *cdclk_state;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 8e6e302bd599..e358234bfe26 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -18,6 +18,8 @@ struct intel_crtc_state;
 struct intel_cdclk_config {
 	unsigned int cdclk, vco, ref, bypass;
 	u8 voltage_level;
+	/* This field is only valid for Xe2LPD and above. */
+	bool joined_mbus;
 };
 
 struct intel_cdclk_state {
@@ -74,6 +76,7 @@ void intel_cdclk_get_cdclk(struct drm_i915_private *dev_priv,
 			   struct intel_cdclk_config *cdclk_config);
 int intel_cdclk_atomic_check(struct intel_atomic_state *state,
 			     bool *need_cdclk_calc);
+int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus);
 struct intel_cdclk_state *
 intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
 
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 4410e21888ad..bc341abcab2f 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -12,6 +12,7 @@
 #include "intel_atomic.h"
 #include "intel_atomic_plane.h"
 #include "intel_bw.h"
+#include "intel_cdclk.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
 #include "intel_display.h"
@@ -2601,10 +2602,17 @@ skl_compute_ddb(struct intel_atomic_state *state)
 			return ret;
 	}
 
-	if (HAS_MBUS_JOINING(i915))
+	if (HAS_MBUS_JOINING(i915)) {
 		new_dbuf_state->joined_mbus =
 			adlp_check_mbus_joined(new_dbuf_state->active_pipes);
 
+		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
+			ret = intel_cdclk_state_set_joined_mbus(state, new_dbuf_state->joined_mbus);
+			if (ret)
+				return ret;
+		}
+	}
+
 	for_each_intel_crtc(&i915->drm, crtc) {
 		enum pipe pipe = crtc->pipe;
 
@@ -3545,12 +3553,14 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
 	return intel_atomic_lock_global_state(&dbuf_state->base);
 }
 
-static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
-						u8 ratio,
-						bool joined_mbus)
+void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus)
 {
 	enum dbuf_slice slice;
 
+	if (DISPLAY_VER(i915) >= 20)
+		intel_de_rmw(i915, MBUS_CTL, MBUS_TRANSLATION_THROTTLE_MIN_MASK,
+			     MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1));
+
 	if (joined_mbus)
 		ratio *= 2;
 
@@ -3568,7 +3578,9 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 {
 	struct drm_i915_private *i915 = to_i915(state->base.dev);
 	u32 mbus_ctl;
-	const struct intel_dbuf_state *dbuf_state =
+	const struct intel_dbuf_state *old_dbuf_state =
+		intel_atomic_get_old_dbuf_state(state);
+	const struct intel_dbuf_state *new_dbuf_state =
 		intel_atomic_get_new_dbuf_state(state);
 
 	if (!HAS_MBUS_JOINING(i915))
@@ -3578,7 +3590,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 	 * TODO: Implement vblank synchronized MBUS joining changes.
 	 * Must be properly coordinated with dbuf reprogramming.
 	 */
-	if (dbuf_state->joined_mbus)
+	if (new_dbuf_state->joined_mbus)
 		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
 			MBUS_JOIN_PIPE_SELECT_NONE;
 	else
@@ -3589,8 +3601,20 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
 		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
 		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
 
-	intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
-					    dbuf_state->joined_mbus);
+	if (DISPLAY_VER(i915) >= 20 &&
+	    old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) {
+		/*
+		 * For Xe2LPD and beyond, when there is a change in the ratio
+		 * between MDCLK and CDCLK, updates to related registers need to
+		 * happen at a specific point in the CDCLK change sequence. In
+		 * that case, we defer to the call to
+		 * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic.
+		 */
+		return;
+	}
+
+	intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio,
+					    new_dbuf_state->joined_mbus);
 }
 
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index fed4d12df584..3a90741cab06 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -76,6 +76,7 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
 
 void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
 void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
+void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus);
 void intel_mbus_dbox_update(struct intel_atomic_state *state);
 
 #endif /* __SKL_WATERMARK_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
index 20b30c9a6613..269163fa3350 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
@@ -32,14 +32,16 @@
 #define MBUS_BBOX_CTL_S1		_MMIO(0x45040)
 #define MBUS_BBOX_CTL_S2		_MMIO(0x45044)
 
-#define MBUS_CTL			_MMIO(0x4438C)
-#define MBUS_JOIN			REG_BIT(31)
-#define MBUS_HASHING_MODE_MASK		REG_BIT(30)
-#define MBUS_HASHING_MODE_2x2		REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
-#define MBUS_HASHING_MODE_1x4		REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
-#define MBUS_JOIN_PIPE_SELECT_MASK	REG_GENMASK(28, 26)
-#define MBUS_JOIN_PIPE_SELECT(pipe)	REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
-#define MBUS_JOIN_PIPE_SELECT_NONE	MBUS_JOIN_PIPE_SELECT(7)
+#define MBUS_CTL				_MMIO(0x4438C)
+#define   MBUS_JOIN				REG_BIT(31)
+#define   MBUS_HASHING_MODE_MASK		REG_BIT(30)
+#define   MBUS_HASHING_MODE_2x2			REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
+#define   MBUS_HASHING_MODE_1x4			REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
+#define   MBUS_JOIN_PIPE_SELECT_MASK		REG_GENMASK(28, 26)
+#define   MBUS_JOIN_PIPE_SELECT(pipe)		REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
+#define   MBUS_JOIN_PIPE_SELECT_NONE		MBUS_JOIN_PIPE_SELECT(7)
+#define   MBUS_TRANSLATION_THROTTLE_MIN_MASK	REG_GENMASK(15, 13)
+#define   MBUS_TRANSLATION_THROTTLE_MIN(val)	REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val)
 
 /* Watermark register definitions for SKL */
 #define _CUR_WM_A_0		0x70140
-- 
2.44.0

[PATCH 7/8] drm/i915/xe2lpd: Load DMC

[Gustavo Sousa]

From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Load DMC for Xe2LPD. The value 0x8000 is the maximum payload size for
any Xe2LPD DMC firmware.

Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dmc.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
index 835781624482..54c5909de293 100644
--- a/drivers/gpu/drm/i915/display/intel_dmc.c
+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
@@ -89,10 +89,14 @@ static struct intel_dmc *i915_to_dmc(struct drm_i915_private *i915)
 	__stringify(major) "_"			\
 	__stringify(minor) ".bin"
 
+#define XE2LPD_MAX_FW_SIZE		0x8000
 #define XELPDP_DMC_MAX_FW_SIZE		0x7000
 #define DISPLAY_VER13_DMC_MAX_FW_SIZE	0x20000
 #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
 
+#define XE2LPD_DMC_PATH			DMC_PATH(xe2lpd)
+MODULE_FIRMWARE(XE2LPD_DMC_PATH);
+
 #define MTL_DMC_PATH			DMC_PATH(mtl)
 MODULE_FIRMWARE(MTL_DMC_PATH);
 
@@ -987,7 +991,10 @@ void intel_dmc_init(struct drm_i915_private *i915)
 
 	INIT_WORK(&dmc->work, dmc_load_work_fn);
 
-	if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
+	if (DISPLAY_VER_FULL(i915) == IP_VER(20, 0)) {
+		dmc->fw_path = XE2LPD_DMC_PATH;
+		dmc->max_fw_size = XE2LPD_MAX_FW_SIZE;
+	} else if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
 		dmc->fw_path = MTL_DMC_PATH;
 		dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE;
 	} else if (IS_DG2(i915)) {
-- 
2.44.0

[PATCH 8/8] drm/xe/lnl: Enable display support

[Gustavo Sousa]

From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>

Enable display support for Lunar Lake.

Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/xe/xe_pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
index 5b5c29761c5d..42ba2ea62c1e 100644
--- a/drivers/gpu/drm/xe/xe_pci.c
+++ b/drivers/gpu/drm/xe/xe_pci.c
@@ -333,6 +333,7 @@ static const struct xe_device_desc mtl_desc = {
 
 static const struct xe_device_desc lnl_desc = {
 	PLATFORM(XE_LUNARLAKE),
+	.has_display = true,
 	.require_force_probe = true,
 };
 
-- 
2.44.0

Re: [PATCH 7/8] drm/i915/xe2lpd: Load DMC

[Lucas De Marchi]

On Mon, Mar 04, 2024 at 03:30:26PM -0300, Gustavo Sousa wrote:
>From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>
>Load DMC for Xe2LPD. The value 0x8000 is the maximum payload size for
>any Xe2LPD DMC firmware.
>
>Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
>Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_dmc.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>index 835781624482..54c5909de293 100644
>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>@@ -89,10 +89,14 @@ static struct intel_dmc *i915_to_dmc(struct drm_i915_private *i915)
> 	__stringify(major) "_"			\
> 	__stringify(minor) ".bin"
>
>+#define XE2LPD_MAX_FW_SIZE		0x8000

		 ^ missing DMC_ here to be like the other macros below?

other than that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Lucas De Marchi

> #define XELPDP_DMC_MAX_FW_SIZE		0x7000
> #define DISPLAY_VER13_DMC_MAX_FW_SIZE	0x20000
> #define DISPLAY_VER12_DMC_MAX_FW_SIZE	ICL_DMC_MAX_FW_SIZE
>
>+#define XE2LPD_DMC_PATH			DMC_PATH(xe2lpd)
>+MODULE_FIRMWARE(XE2LPD_DMC_PATH);
>+
> #define MTL_DMC_PATH			DMC_PATH(mtl)
> MODULE_FIRMWARE(MTL_DMC_PATH);
>
>@@ -987,7 +991,10 @@ void intel_dmc_init(struct drm_i915_private *i915)
>
> 	INIT_WORK(&dmc->work, dmc_load_work_fn);
>
>-	if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
>+	if (DISPLAY_VER_FULL(i915) == IP_VER(20, 0)) {
>+		dmc->fw_path = XE2LPD_DMC_PATH;
>+		dmc->max_fw_size = XE2LPD_MAX_FW_SIZE;
>+	} else if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
> 		dmc->fw_path = MTL_DMC_PATH;
> 		dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE;
> 	} else if (IS_DG2(i915)) {
>-- 
>2.44.0
>

Re: [PATCH 8/8] drm/xe/lnl: Enable display support

[Lucas De Marchi]

On Mon, Mar 04, 2024 at 03:30:27PM -0300, Gustavo Sousa wrote:
>From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>
>Enable display support for Lunar Lake.
>
>Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

No need to merge this through drm-xe. Since we require all the other
patches in this series on the i915 side, let's just make our lives
easier and merge this one through drm-intel.

Lucas De Marchi

>---
> drivers/gpu/drm/xe/xe_pci.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
>index 5b5c29761c5d..42ba2ea62c1e 100644
>--- a/drivers/gpu/drm/xe/xe_pci.c
>+++ b/drivers/gpu/drm/xe/xe_pci.c
>@@ -333,6 +333,7 @@ static const struct xe_device_desc mtl_desc = {
>
> static const struct xe_device_desc lnl_desc = {
> 	PLATFORM(XE_LUNARLAKE),
>+	.has_display = true,
> 	.require_force_probe = true,
> };
>
>-- 
>2.44.0
>

Re: [PATCH 7/8] drm/i915/xe2lpd: Load DMC

[Gustavo Sousa]

Quoting Lucas De Marchi (2024-03-04 16:50:49-03:00)
>On Mon, Mar 04, 2024 at 03:30:26PM -0300, Gustavo Sousa wrote:
>>From: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>>
>>Load DMC for Xe2LPD. The value 0x8000 is the maximum payload size for
>>any Xe2LPD DMC firmware.
>>
>>Signed-off-by: Balasubramani Vivekanandan <balasubramani.vivekanandan@intel.com>
>>Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@intel.com>
>>Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>---
>> drivers/gpu/drm/i915/display/intel_dmc.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_dmc.c b/drivers/gpu/drm/i915/display/intel_dmc.c
>>index 835781624482..54c5909de293 100644
>>--- a/drivers/gpu/drm/i915/display/intel_dmc.c
>>+++ b/drivers/gpu/drm/i915/display/intel_dmc.c
>>@@ -89,10 +89,14 @@ static struct intel_dmc *i915_to_dmc(struct drm_i915_private *i915)
>>         __stringify(major) "_"                        \
>>         __stringify(minor) ".bin"
>>
>>+#define XE2LPD_MAX_FW_SIZE                0x8000
>
>                 ^ missing DMC_ here to be like the other macros below?

Oh, yeah. That went unnoticed. Thanks!

--
Gustavo

>
>other than that, Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
>Lucas De Marchi
>
>> #define XELPDP_DMC_MAX_FW_SIZE                0x7000
>> #define DISPLAY_VER13_DMC_MAX_FW_SIZE        0x20000
>> #define DISPLAY_VER12_DMC_MAX_FW_SIZE        ICL_DMC_MAX_FW_SIZE
>>
>>+#define XE2LPD_DMC_PATH                        DMC_PATH(xe2lpd)
>>+MODULE_FIRMWARE(XE2LPD_DMC_PATH);
>>+
>> #define MTL_DMC_PATH                        DMC_PATH(mtl)
>> MODULE_FIRMWARE(MTL_DMC_PATH);
>>
>>@@ -987,7 +991,10 @@ void intel_dmc_init(struct drm_i915_private *i915)
>>
>>         INIT_WORK(&dmc->work, dmc_load_work_fn);
>>
>>-        if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
>>+        if (DISPLAY_VER_FULL(i915) == IP_VER(20, 0)) {
>>+                dmc->fw_path = XE2LPD_DMC_PATH;
>>+                dmc->max_fw_size = XE2LPD_MAX_FW_SIZE;
>>+        } else if (DISPLAY_VER_FULL(i915) == IP_VER(14, 0)) {
>>                 dmc->fw_path = MTL_DMC_PATH;
>>                 dmc->max_fw_size = XELPDP_DMC_MAX_FW_SIZE;
>>         } else if (IS_DG2(i915)) {
>>-- 
>>2.44.0
>>

Re: [PATCH 1/8] drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table

[Matt Roper]

On Mon, Mar 04, 2024 at 03:30:20PM -0300, Gustavo Sousa wrote:
> The CDCLK table is tied to Xe2LPD display and not to the platform. Let's
> rename lnl_cdclk_table to xe2lpd_cdclk_table in order to reflect that.
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 22473c55b899..407bd541eb46 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1417,7 +1417,7 @@ static const struct intel_cdclk_vals mtl_cdclk_table[] = {
>  	{}
>  };
>  
> -static const struct intel_cdclk_vals lnl_cdclk_table[] = {
> +static const struct intel_cdclk_vals xe2lpd_cdclk_table[] = {
>  	{ .refclk = 38400, .cdclk = 153600, .ratio = 16, .waveform = 0xaaaa },
>  	{ .refclk = 38400, .cdclk = 172800, .ratio = 16, .waveform = 0xad5a },
>  	{ .refclk = 38400, .cdclk = 192000, .ratio = 16, .waveform = 0xb6b6 },
> @@ -3710,7 +3710,7 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
>  {
>  	if (DISPLAY_VER(dev_priv) >= 20) {
>  		dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs;
> -		dev_priv->display.cdclk.table = lnl_cdclk_table;
> +		dev_priv->display.cdclk.table = xe2lpd_cdclk_table;
>  	} else if (DISPLAY_VER(dev_priv) >= 14) {
>  		dev_priv->display.funcs.cdclk = &mtl_cdclk_funcs;
>  		dev_priv->display.cdclk.table = mtl_cdclk_table;
> -- 
> 2.44.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 2/8] drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()

[Matt Roper]

On Mon, Mar 04, 2024 at 03:30:21PM -0300, Gustavo Sousa wrote:
> There will be future changes that rely on the source of the MDCLK. Let's
> have xe2lpd_mdclk_source_sel() as the function responsible for reporting
> that information.
> 
> Bspec: 69090
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 ++++++++++++++++-
>  drivers/gpu/drm/i915/i915_reg.h            |  4 +++-
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 407bd541eb46..bf84bf27213f 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -1876,6 +1876,21 @@ static bool cdclk_pll_is_unknown(unsigned int vco)
>  	return vco == ~0;
>  }
>  
> +static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
> +{
> +	if (DISPLAY_VER(i915) >= 20)
> +		return MDCLK_SOURCE_SEL_CDCLK_PLL;
> +
> +	/*
> +	 * Earlier display IPs do not provide means of selecting the
> +	 * MDCLK source, but MDCLK_SOURCE_SEL_CD2XCLK is a nice default,
> +	 * since it reflects the source used for those and allows
> +	 * xe2lpd_mdclk_source_sel() to be used in logic that depends on
> +	 * it.
> +	 */
> +	return MDCLK_SOURCE_SEL_CD2XCLK;

At the moment this function only gets called on Xe2 and beyond where the
register field exists; if that's going to change soon, then wouldn't it
be more natural to just use an early exit to highlight that there's
nothing we need to OR into the CDCLK_CTL for earlier platforms?  

    /* Not configurable for older platforms; they always use CD2XCLK */
    if (DISPLAY_VER(i915) < 20)
            return 0;

Functionally it's the same, but it feels more intuitive to me.

If we aren't expecting to call this from common codepaths that aren't
already protected by a display version check, then we could make this a
drm_WARN_ON() to assert that we haven't deviated from expected behavior.


Matt

> +}
> +
>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>  						    const struct intel_cdclk_config *old_cdclk_config,
>  						    const struct intel_cdclk_config *new_cdclk_config,
> @@ -1980,7 +1995,7 @@ static u32 bxt_cdclk_ctl(struct drm_i915_private *i915,
>  		val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>  
>  	if (DISPLAY_VER(i915) >= 20)
> -		val |= MDCLK_SOURCE_SEL_CDCLK_PLL;
> +		val |= xe2lpd_mdclk_source_sel(i915);
>  	else
>  		val |= skl_cdclk_decimal(cdclk);
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e00557e1a57f..eb953ed1f113 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5900,7 +5900,9 @@ enum skl_power_gate {
>  #define  CDCLK_FREQ_540		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
>  #define  CDCLK_FREQ_337_308		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
>  #define  CDCLK_FREQ_675_617		REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
> -#define  MDCLK_SOURCE_SEL_CDCLK_PLL	REG_BIT(25)
> +#define  MDCLK_SOURCE_SEL_MASK		REG_GENMASK(25, 25)
> +#define  MDCLK_SOURCE_SEL_CD2XCLK	REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 0)
> +#define  MDCLK_SOURCE_SEL_CDCLK_PLL	REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 1)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_MASK	REG_GENMASK(23, 22)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
>  #define  BXT_CDCLK_CD2X_DIV_SEL_1_5	REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
> -- 
> 2.44.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 3/8] drm/i915/cdclk: Only compute squash waveform when necessary

[Matt Roper]

On Mon, Mar 04, 2024 at 03:30:22PM -0300, Gustavo Sousa wrote:
> It is no use computing the squash waveform if we are not going to use
> it. Move the call to cdclk_squash_waveform() inside the block guarded by
> HAS_CDCLK_SQUASH(dev_priv).
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

You could also move the 'u32 waveform' declaration from the top of the
function inside the block too to help prevent any future mistakes of
using it unitialized.

Either way,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index bf84bf27213f..cdf3ae766f9e 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -2023,10 +2023,11 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  	} else
>  		bxt_cdclk_pll_update(dev_priv, vco);
>  
> -	waveform = cdclk_squash_waveform(dev_priv, cdclk);
> +	if (HAS_CDCLK_SQUASH(dev_priv)) {
> +		waveform = cdclk_squash_waveform(dev_priv, cdclk);
>  
> -	if (HAS_CDCLK_SQUASH(dev_priv))
>  		dg2_cdclk_squash_program(dev_priv, waveform);
> +	}
>  
>  	intel_de_write(dev_priv, CDCLK_CTL, bxt_cdclk_ctl(dev_priv, cdclk_config, pipe));
>  
> -- 
> 2.44.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 4/8] drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()

[Matt Roper]

On Mon, Mar 04, 2024 at 03:30:23PM -0300, Gustavo Sousa wrote:
> As of Xe2LPD, it is now possible to select the source of the MDCLK
> as either the CD2XCLK or the CDCLK PLL.
> 
> Previous display IPs were hardcoded to use the CD2XCLK. For those, the
> ratio between MDCLK and CDCLK remained constant, namely 2. For Xe2LPD,
> when we select the CDCLK PLL as the source, the ratio will vary
> according to the squashing configuration (since the cd2x divisor is
> fixed for all supported configurations).
> 
> To help the transition to supporting changes in the ratio, extract the
> function intel_dbuf_mdclk_cdclk_ratio_update() from the existing logic
> and call it using 2 as hardcoded ratio. Upcoming changes will use that
> function for updates in the ratio due to CDCLK changes.
> 
> Bspec: 50057, 69445, 49213, 68868
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 30 +++++++++++++-------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index c6b9be80d83c..d9e49cd60d3a 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3530,6 +3530,21 @@ int intel_dbuf_init(struct drm_i915_private *i915)
>  	return 0;
>  }
>  
> +static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
> +						u8 ratio,
> +						bool joined_mbus)
> +{
> +	enum dbuf_slice slice;
> +
> +	if (joined_mbus)
> +		ratio *= 2;
> +
> +	for_each_dbuf_slice(i915, slice)
> +		intel_de_rmw(i915, DBUF_CTL_S(slice),
> +			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
> +			     DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1));
> +}
> +
>  /*
>   * Configure MBUS_CTL and all DBUF_CTL_S of each slice to join_mbus state before
>   * update the request state of all DBUS slices.
> @@ -3537,8 +3552,7 @@ int intel_dbuf_init(struct drm_i915_private *i915)
>  static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
> -	u32 mbus_ctl, dbuf_min_tracker_val;
> -	enum dbuf_slice slice;
> +	u32 mbus_ctl;
>  	const struct intel_dbuf_state *dbuf_state =
>  		intel_atomic_get_new_dbuf_state(state);
>  
> @@ -3549,24 +3563,18 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  	 * TODO: Implement vblank synchronized MBUS joining changes.
>  	 * Must be properly coordinated with dbuf reprogramming.
>  	 */
> -	if (dbuf_state->joined_mbus) {
> +	if (dbuf_state->joined_mbus)
>  		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
>  			MBUS_JOIN_PIPE_SELECT_NONE;
> -		dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(3);
> -	} else {
> +	else
>  		mbus_ctl = MBUS_HASHING_MODE_2x2 |
>  			MBUS_JOIN_PIPE_SELECT_NONE;
> -		dbuf_min_tracker_val = DBUF_MIN_TRACKER_STATE_SERVICE(1);
> -	}
>  
>  	intel_de_rmw(i915, MBUS_CTL,
>  		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
>  		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
>  
> -	for_each_dbuf_slice(i915, slice)
> -		intel_de_rmw(i915, DBUF_CTL_S(slice),
> -			     DBUF_MIN_TRACKER_STATE_SERVICE_MASK,
> -			     dbuf_min_tracker_val);
> +	intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus);
>  }
>  
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
> -- 
> 2.44.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH 5/8] drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state

[Matt Roper]

On Mon, Mar 04, 2024 at 03:30:24PM -0300, Gustavo Sousa wrote:
> CDCLK programming Xe2LPD always selects the CDCLK PLL as source for the

I think something got a bit muddled while rewriting this sentence.
Maybe the first two words were supposed to be dropped?

Otherwise,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> MDCLK. Because of that, the ratio between MDCLK and CDCLK is not be
> constant anymore. As such, make sure to have the current ratio available
> in intel_dbuf_state so that it can be used during dbuf programming.
> 
> Note that we write-lock the global state instead of serializing to a
> hardware commit because a change in the ratio should be rather handled
> in the CDCLK change sequence, which will need to take care of updating
> the necessary registers in that case. We will implement that in upcoming
> changes.
> 
> That said, changes in the MBus joining state should be handled by the
> DBUF/MBUS logic, just like it is already done, but the logic will need
> to know the ratio to properly update the registers.
> 
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c   | 26 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_cdclk.h   |  2 ++
>  drivers/gpu/drm/i915/display/skl_watermark.c | 18 +++++++++++++-
>  drivers/gpu/drm/i915/display/skl_watermark.h |  3 +++
>  4 files changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index cdf3ae766f9e..04a6e9806254 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -39,6 +39,7 @@
>  #include "intel_pcode.h"
>  #include "intel_psr.h"
>  #include "intel_vdsc.h"
> +#include "skl_watermark.h"
>  #include "vlv_sideband.h"
>  
>  /**
> @@ -1891,6 +1892,22 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
>  	return MDCLK_SOURCE_SEL_CD2XCLK;
>  }
>  
> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
> +			   const struct intel_cdclk_config *cdclk_config)
> +{
> +	u32 source_sel = xe2lpd_mdclk_source_sel(i915);
> +
> +	switch (source_sel) {
> +	case MDCLK_SOURCE_SEL_CD2XCLK:
> +		return 2;
> +	case MDCLK_SOURCE_SEL_CDCLK_PLL:
> +		return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk);
> +	default:
> +		MISSING_CASE(source_sel);
> +		return 2;
> +	}
> +}
> +
>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>  						    const struct intel_cdclk_config *old_cdclk_config,
>  						    const struct intel_cdclk_config *new_cdclk_config,
> @@ -3281,6 +3298,15 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>  			    "Modeset required for cdclk change\n");
>  	}
>  
> +	if (intel_mdclk_cdclk_ratio(dev_priv, &old_cdclk_state->actual) !=
> +	    intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual)) {
> +		u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual);
> +
> +		ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	drm_dbg_kms(&dev_priv->drm,
>  		    "New cdclk calculated to be logical %u kHz, actual %u kHz\n",
>  		    new_cdclk_state->logical.cdclk,
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index fa301495e7f1..8e6e302bd599 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -62,6 +62,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv);
>  u32 intel_read_rawclk(struct drm_i915_private *dev_priv);
>  bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a,
>  			       const struct intel_cdclk_config *b);
> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
> +			   const struct intel_cdclk_config *cdclk_config);
>  void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state);
>  void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state);
>  void intel_cdclk_dump_config(struct drm_i915_private *i915,
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index d9e49cd60d3a..4410e21888ad 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3057,6 +3057,8 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>  	if (HAS_MBUS_JOINING(i915))
>  		dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
>  
> +	dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(i915, &i915->display.cdclk.hw);
> +
>  	for_each_intel_crtc(&i915->drm, crtc) {
>  		struct intel_crtc_state *crtc_state =
>  			to_intel_crtc_state(crtc->base.state);
> @@ -3530,6 +3532,19 @@ int intel_dbuf_init(struct drm_i915_private *i915)
>  	return 0;
>  }
>  
> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio)
> +{
> +	struct intel_dbuf_state *dbuf_state;
> +
> +	dbuf_state = intel_atomic_get_dbuf_state(state);
> +	if (IS_ERR(dbuf_state))
> +		return PTR_ERR(dbuf_state);
> +
> +	dbuf_state->mdclk_cdclk_ratio = ratio;
> +
> +	return intel_atomic_lock_global_state(&dbuf_state->base);
> +}
> +
>  static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
>  						u8 ratio,
>  						bool joined_mbus)
> @@ -3574,7 +3589,8 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
>  		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
>  
> -	intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus);
> +	intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
> +					    dbuf_state->joined_mbus);
>  }
>  
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
> index e3d1d74a7b17..fed4d12df584 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> @@ -58,6 +58,7 @@ struct intel_dbuf_state {
>  	u8 slices[I915_MAX_PIPES];
>  	u8 enabled_slices;
>  	u8 active_pipes;
> +	u8 mdclk_cdclk_ratio;
>  	bool joined_mbus;
>  };
>  
> @@ -71,6 +72,8 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
>  	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->display.dbuf.obj))
>  
>  int intel_dbuf_init(struct drm_i915_private *i915);
> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio);
> +
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
>  void intel_mbus_dbox_update(struct intel_atomic_state *state);
> -- 
> 2.44.0
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

✗ Fi.CI.CHECKPATCH: warning for Enable LNL display

[Patchwork]

== Series Details ==

Series: Enable LNL display
URL   : https://patchwork.freedesktop.org/series/130689/
State : warning

== Summary ==

Error: dim checkpatch failed
52ce7e3a93e7 drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table
3f9ed6439f1f drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()
7071a46e9b9d drm/i915/cdclk: Only compute squash waveform when necessary
f17882210bdb drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()
60c6197de41a drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state
d26865441ec6 drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
-:264: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#264: FILE: drivers/gpu/drm/i915/display/skl_watermark_regs.h:44:
+#define   MBUS_TRANSLATION_THROTTLE_MIN(val)	REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val)

total: 0 errors, 1 warnings, 0 checks, 195 lines checked
f4c6b4816724 drm/i915/xe2lpd: Load DMC
b3ff3350eb2d drm/xe/lnl: Enable display support

✗ Fi.CI.SPARSE: warning for Enable LNL display

[Patchwork]

== Series Details ==

Series: Enable LNL display
URL   : https://patchwork.freedesktop.org/series/130689/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ Fi.CI.BAT: failure for Enable LNL display

[Patchwork]

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

== Series Details ==

Series: Enable LNL display
URL   : https://patchwork.freedesktop.org/series/130689/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_14383 -> Patchwork_130689v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_130689v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_130689v1, 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_130689v1/index.html

Participating hosts (41 -> 40)
------------------------------

  Additional (1): fi-glk-j4005 
  Missing    (2): bat-arls-4 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem_contexts:
    - bat-arls-2:         NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@i915_selftest@live@gem_contexts.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-arls-2:         NOTRUN -> [SKIP][2] ([i915#9318])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-glk-j4005:       NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/fi-glk-j4005/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-glk-j4005:       NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/fi-glk-j4005/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-arls-2:         NOTRUN -> [SKIP][5] ([i915#10213]) +3 other tests skip
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_mmap@basic:
    - bat-arls-2:         NOTRUN -> [SKIP][6] ([i915#4083])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@gem_mmap@basic.html

  * igt@gem_mmap_gtt@basic:
    - bat-arls-2:         NOTRUN -> [SKIP][7] ([i915#10196] / [i915#4077]) +2 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@gem_mmap_gtt@basic.html

  * igt@gem_render_tiled_blits@basic:
    - bat-arls-2:         NOTRUN -> [SKIP][8] ([i915#10197] / [i915#10211] / [i915#4079])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@gem_render_tiled_blits@basic.html

  * igt@gem_tiled_pread_basic:
    - bat-arls-2:         NOTRUN -> [SKIP][9] ([i915#10206] / [i915#4079])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_rps@basic-api:
    - bat-arls-2:         NOTRUN -> [SKIP][10] ([i915#10209])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@i915_pm_rps@basic-api.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-arls-2:         NOTRUN -> [SKIP][11] ([i915#10200]) +9 other tests skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-glk-j4005:       NOTRUN -> [SKIP][12] ([fdo#109271]) +10 other tests skip
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/fi-glk-j4005/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - bat-arls-2:         NOTRUN -> [SKIP][13] ([i915#10202]) +1 other test skip
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_dsc@dsc-basic:
    - bat-arls-2:         NOTRUN -> [SKIP][14] ([i915#9886])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_dsc@dsc-basic.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-arls-2:         NOTRUN -> [SKIP][15] ([i915#10207])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@psr-primary-mmap-gtt@edp-1:
    - bat-arls-2:         NOTRUN -> [SKIP][16] ([i915#10196] / [i915#4077] / [i915#9688])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_psr@psr-primary-mmap-gtt@edp-1.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-arls-2:         NOTRUN -> [SKIP][17] ([i915#10208] / [i915#8809])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-mmap:
    - bat-arls-2:         NOTRUN -> [SKIP][18] ([i915#10196] / [i915#3708] / [i915#4077]) +1 other test skip
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@prime_vgem@basic-fence-mmap.html

  * igt@prime_vgem@basic-fence-read:
    - bat-arls-2:         NOTRUN -> [SKIP][19] ([i915#10212] / [i915#3708])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@prime_vgem@basic-fence-read.html

  * igt@prime_vgem@basic-read:
    - bat-arls-2:         NOTRUN -> [SKIP][20] ([i915#10214] / [i915#3708])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-write:
    - bat-arls-2:         NOTRUN -> [SKIP][21] ([i915#10216] / [i915#3708])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@i915_module_load@load:
    - bat-arls-2:         [ABORT][22] -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14383/bat-arls-2/igt@i915_module_load@load.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-arls-2/igt@i915_module_load@load.html

  * igt@i915_selftest@live@gt_pm:
    - bat-dg2-9:          [ABORT][24] -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14383/bat-dg2-9/igt@i915_selftest@live@gt_pm.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_130689v1/bat-dg2-9/igt@i915_selftest@live@gt_pm.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#10196]: https://gitlab.freedesktop.org/drm/intel/issues/10196
  [i915#10197]: https://gitlab.freedesktop.org/drm/intel/issues/10197
  [i915#10200]: https://gitlab.freedesktop.org/drm/intel/issues/10200
  [i915#10202]: https://gitlab.freedesktop.org/drm/intel/issues/10202
  [i915#10206]: https://gitlab.freedesktop.org/drm/intel/issues/10206
  [i915#10207]: https://gitlab.freedesktop.org/drm/intel/issues/10207
  [i915#10208]: https://gitlab.freedesktop.org/drm/intel/issues/10208
  [i915#10209]: https://gitlab.freedesktop.org/drm/intel/issues/10209
  [i915#10211]: https://gitlab.freedesktop.org/drm/intel/issues/10211
  [i915#10212]: https://gitlab.freedesktop.org/drm/intel/issues/10212
  [i915#10213]: https://gitlab.freedesktop.org/drm/intel/issues/10213
  [i915#10214]: https://gitlab.freedesktop.org/drm/intel/issues/10214
  [i915#10216]: https://gitlab.freedesktop.org/drm/intel/issues/10216
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#8809]: https://gitlab.freedesktop.org/drm/intel/issues/8809
  [i915#9318]: https://gitlab.freedesktop.org/drm/intel/issues/9318
  [i915#9688]: https://gitlab.freedesktop.org/drm/intel/issues/9688
  [i915#9886]: https://gitlab.freedesktop.org/drm/intel/issues/9886


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

  * Linux: CI_DRM_14383 -> Patchwork_130689v1

  CI-20190529: 20190529
  CI_DRM_14383: e90b5046fce6ed727427eec0d52e4f9626609ed0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7745: 838ad56bf511c1caf56851c661fb5828c0440e3b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_130689v1: e90b5046fce6ed727427eec0d52e4f9626609ed0 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

e6f1bf8a2796 drm/xe/lnl: Enable display support
7824f542c322 drm/i915/xe2lpd: Load DMC
388ecdd3ec9e drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes
2d8dd6c582e5 drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state
369f572e7c88 drm/i915: Extract intel_dbuf_mdclk_cdclk_ratio_update()
e17f7a0bc07f drm/i915/cdclk: Only compute squash waveform when necessary
f79bb3f1c6fb drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()
be30aa4ac24d drm/i915/cdclk: Rename lnl_cdclk_table to xe2lpd_cdclk_table

== Logs ==

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

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

Re: [PATCH 2/8] drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()

[Gustavo Sousa]

Quoting Matt Roper (2024-03-04 18:58:34-03:00)
>On Mon, Mar 04, 2024 at 03:30:21PM -0300, Gustavo Sousa wrote:
>> There will be future changes that rely on the source of the MDCLK. Let's
>> have xe2lpd_mdclk_source_sel() as the function responsible for reporting
>> that information.
>> 
>> Bspec: 69090
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 ++++++++++++++++-
>>  drivers/gpu/drm/i915/i915_reg.h            |  4 +++-
>>  2 files changed, 19 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index 407bd541eb46..bf84bf27213f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -1876,6 +1876,21 @@ static bool cdclk_pll_is_unknown(unsigned int vco)
>>          return vco == ~0;
>>  }
>>  
>> +static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
>> +{
>> +        if (DISPLAY_VER(i915) >= 20)
>> +                return MDCLK_SOURCE_SEL_CDCLK_PLL;
>> +
>> +        /*
>> +         * Earlier display IPs do not provide means of selecting the
>> +         * MDCLK source, but MDCLK_SOURCE_SEL_CD2XCLK is a nice default,
>> +         * since it reflects the source used for those and allows
>> +         * xe2lpd_mdclk_source_sel() to be used in logic that depends on
>> +         * it.
>> +         */
>> +        return MDCLK_SOURCE_SEL_CD2XCLK;
>
>At the moment this function only gets called on Xe2 and beyond where the
>register field exists; if that's going to change soon, then wouldn't it
>be more natural to just use an early exit to highlight that there's
>nothing we need to OR into the CDCLK_CTL for earlier platforms?  
>
>    /* Not configurable for older platforms; they always use CD2XCLK */
>    if (DISPLAY_VER(i915) < 20)
>            return 0;
>
>Functionally it's the same, but it feels more intuitive to me.
>
>If we aren't expecting to call this from common codepaths that aren't
>already protected by a display version check, then we could make this a
>drm_WARN_ON() to assert that we haven't deviated from expected behavior.

Well, the intention here was for this function to serve 2 purposes: (i)
to give the value of the "MDCLK Source Select" field of CDCLK_CTL and
also (ii) tell which was the source of the MDCLK for displays pre and
post (including) Xe2LPD, because we will need that information in
"drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state".

I was hoping to do that instead of creating a new enum, but maybe it
will just cause confusion?

Should we have one function to tell us the source and another for
giving the value of the register field?

--
Gustavo Sousa

>
>
>Matt
>
>> +}
>> +
>>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>>                                                      const struct intel_cdclk_config *old_cdclk_config,
>>                                                      const struct intel_cdclk_config *new_cdclk_config,
>> @@ -1980,7 +1995,7 @@ static u32 bxt_cdclk_ctl(struct drm_i915_private *i915,
>>                  val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>>  
>>          if (DISPLAY_VER(i915) >= 20)
>> -                val |= MDCLK_SOURCE_SEL_CDCLK_PLL;
>> +                val |= xe2lpd_mdclk_source_sel(i915);
>>          else
>>                  val |= skl_cdclk_decimal(cdclk);
>>  
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index e00557e1a57f..eb953ed1f113 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5900,7 +5900,9 @@ enum skl_power_gate {
>>  #define  CDCLK_FREQ_540                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
>>  #define  CDCLK_FREQ_337_308                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
>>  #define  CDCLK_FREQ_675_617                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
>> -#define  MDCLK_SOURCE_SEL_CDCLK_PLL        REG_BIT(25)
>> +#define  MDCLK_SOURCE_SEL_MASK                REG_GENMASK(25, 25)
>> +#define  MDCLK_SOURCE_SEL_CD2XCLK        REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 0)
>> +#define  MDCLK_SOURCE_SEL_CDCLK_PLL        REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 1)
>>  #define  BXT_CDCLK_CD2X_DIV_SEL_MASK        REG_GENMASK(23, 22)
>>  #define  BXT_CDCLK_CD2X_DIV_SEL_1        REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
>>  #define  BXT_CDCLK_CD2X_DIV_SEL_1_5        REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
>> -- 
>> 2.44.0
>> 
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation

Re: [PATCH 3/8] drm/i915/cdclk: Only compute squash waveform when necessary

[Gustavo Sousa]

Quoting Matt Roper (2024-03-04 19:04:19-03:00)
>On Mon, Mar 04, 2024 at 03:30:22PM -0300, Gustavo Sousa wrote:
>> It is no use computing the squash waveform if we are not going to use
>> it. Move the call to cdclk_squash_waveform() inside the block guarded by
>> HAS_CDCLK_SQUASH(dev_priv).
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>
>You could also move the 'u32 waveform' declaration from the top of the
>function inside the block too to help prevent any future mistakes of
>using it unitialized.

Yep, makes sense. Will do that in v2.

>
>Either way,
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks!

--
Gustavo Sousa

>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index bf84bf27213f..cdf3ae766f9e 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -2023,10 +2023,11 @@ static void _bxt_set_cdclk(struct drm_i915_private *dev_priv,
>>          } else
>>                  bxt_cdclk_pll_update(dev_priv, vco);
>>  
>> -        waveform = cdclk_squash_waveform(dev_priv, cdclk);
>> +        if (HAS_CDCLK_SQUASH(dev_priv)) {
>> +                waveform = cdclk_squash_waveform(dev_priv, cdclk);
>>  
>> -        if (HAS_CDCLK_SQUASH(dev_priv))
>>                  dg2_cdclk_squash_program(dev_priv, waveform);
>> +        }
>>  
>>          intel_de_write(dev_priv, CDCLK_CTL, bxt_cdclk_ctl(dev_priv, cdclk_config, pipe));
>>  
>> -- 
>> 2.44.0
>> 
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation

Re: [PATCH 5/8] drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state

[Gustavo Sousa]

Quoting Matt Roper (2024-03-04 20:25:31-03:00)
>On Mon, Mar 04, 2024 at 03:30:24PM -0300, Gustavo Sousa wrote:
>> CDCLK programming Xe2LPD always selects the CDCLK PLL as source for the
>
>I think something got a bit muddled while rewriting this sentence.
>Maybe the first two words were supposed to be dropped?

Yeah. The original intention here was "CDCLK programming for Xe2LPD
...". Dropping the first two words also works :-)

Will reword this in v2.

>
>Otherwise,
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks!

--
Gustavo Sousa

>
>> MDCLK. Because of that, the ratio between MDCLK and CDCLK is not be
>> constant anymore. As such, make sure to have the current ratio available
>> in intel_dbuf_state so that it can be used during dbuf programming.
>> 
>> Note that we write-lock the global state instead of serializing to a
>> hardware commit because a change in the ratio should be rather handled
>> in the CDCLK change sequence, which will need to take care of updating
>> the necessary registers in that case. We will implement that in upcoming
>> changes.
>> 
>> That said, changes in the MBus joining state should be handled by the
>> DBUF/MBUS logic, just like it is already done, but the logic will need
>> to know the ratio to properly update the registers.
>> 
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c   | 26 ++++++++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_cdclk.h   |  2 ++
>>  drivers/gpu/drm/i915/display/skl_watermark.c | 18 +++++++++++++-
>>  drivers/gpu/drm/i915/display/skl_watermark.h |  3 +++
>>  4 files changed, 48 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index cdf3ae766f9e..04a6e9806254 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -39,6 +39,7 @@
>>  #include "intel_pcode.h"
>>  #include "intel_psr.h"
>>  #include "intel_vdsc.h"
>> +#include "skl_watermark.h"
>>  #include "vlv_sideband.h"
>>  
>>  /**
>> @@ -1891,6 +1892,22 @@ static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
>>          return MDCLK_SOURCE_SEL_CD2XCLK;
>>  }
>>  
>> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>> +                           const struct intel_cdclk_config *cdclk_config)
>> +{
>> +        u32 source_sel = xe2lpd_mdclk_source_sel(i915);
>> +
>> +        switch (source_sel) {
>> +        case MDCLK_SOURCE_SEL_CD2XCLK:
>> +                return 2;
>> +        case MDCLK_SOURCE_SEL_CDCLK_PLL:
>> +                return DIV_ROUND_UP(cdclk_config->vco, cdclk_config->cdclk);
>> +        default:
>> +                MISSING_CASE(source_sel);
>> +                return 2;
>> +        }
>> +}
>> +
>>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>>                                                      const struct intel_cdclk_config *old_cdclk_config,
>>                                                      const struct intel_cdclk_config *new_cdclk_config,
>> @@ -3281,6 +3298,15 @@ int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
>>                              "Modeset required for cdclk change\n");
>>          }
>>  
>> +        if (intel_mdclk_cdclk_ratio(dev_priv, &old_cdclk_state->actual) !=
>> +            intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual)) {
>> +                u8 ratio = intel_mdclk_cdclk_ratio(dev_priv, &new_cdclk_state->actual);
>> +
>> +                ret = intel_dbuf_state_set_mdclk_cdclk_ratio(state, ratio);
>> +                if (ret)
>> +                        return ret;
>> +        }
>> +
>>          drm_dbg_kms(&dev_priv->drm,
>>                      "New cdclk calculated to be logical %u kHz, actual %u kHz\n",
>>                      new_cdclk_state->logical.cdclk,
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> index fa301495e7f1..8e6e302bd599 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> @@ -62,6 +62,8 @@ void intel_update_cdclk(struct drm_i915_private *dev_priv);
>>  u32 intel_read_rawclk(struct drm_i915_private *dev_priv);
>>  bool intel_cdclk_clock_changed(const struct intel_cdclk_config *a,
>>                                 const struct intel_cdclk_config *b);
>> +u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>> +                           const struct intel_cdclk_config *cdclk_config);
>>  void intel_set_cdclk_pre_plane_update(struct intel_atomic_state *state);
>>  void intel_set_cdclk_post_plane_update(struct intel_atomic_state *state);
>>  void intel_cdclk_dump_config(struct drm_i915_private *i915,
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
>> index d9e49cd60d3a..4410e21888ad 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
>> @@ -3057,6 +3057,8 @@ static void skl_wm_get_hw_state(struct drm_i915_private *i915)
>>          if (HAS_MBUS_JOINING(i915))
>>                  dbuf_state->joined_mbus = intel_de_read(i915, MBUS_CTL) & MBUS_JOIN;
>>  
>> +        dbuf_state->mdclk_cdclk_ratio = intel_mdclk_cdclk_ratio(i915, &i915->display.cdclk.hw);
>> +
>>          for_each_intel_crtc(&i915->drm, crtc) {
>>                  struct intel_crtc_state *crtc_state =
>>                          to_intel_crtc_state(crtc->base.state);
>> @@ -3530,6 +3532,19 @@ int intel_dbuf_init(struct drm_i915_private *i915)
>>          return 0;
>>  }
>>  
>> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio)
>> +{
>> +        struct intel_dbuf_state *dbuf_state;
>> +
>> +        dbuf_state = intel_atomic_get_dbuf_state(state);
>> +        if (IS_ERR(dbuf_state))
>> +                return PTR_ERR(dbuf_state);
>> +
>> +        dbuf_state->mdclk_cdclk_ratio = ratio;
>> +
>> +        return intel_atomic_lock_global_state(&dbuf_state->base);
>> +}
>> +
>>  static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
>>                                                  u8 ratio,
>>                                                  bool joined_mbus)
>> @@ -3574,7 +3589,8 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>>                       MBUS_HASHING_MODE_MASK | MBUS_JOIN |
>>                       MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
>>  
>> -        intel_dbuf_mdclk_cdclk_ratio_update(i915, 2, dbuf_state->joined_mbus);
>> +        intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
>> +                                            dbuf_state->joined_mbus);
>>  }
>>  
>>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
>> index e3d1d74a7b17..fed4d12df584 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
>> @@ -58,6 +58,7 @@ struct intel_dbuf_state {
>>          u8 slices[I915_MAX_PIPES];
>>          u8 enabled_slices;
>>          u8 active_pipes;
>> +        u8 mdclk_cdclk_ratio;
>>          bool joined_mbus;
>>  };
>>  
>> @@ -71,6 +72,8 @@ intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
>>          to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_i915(state->base.dev)->display.dbuf.obj))
>>  
>>  int intel_dbuf_init(struct drm_i915_private *i915);
>> +int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8 ratio);
>> +
>>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
>>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
>>  void intel_mbus_dbox_update(struct intel_atomic_state *state);
>> -- 
>> 2.44.0
>> 
>
>-- 
>Matt Roper
>Graphics Software Engineer
>Linux GPU Platform Enablement
>Intel Corporation

Re: [PATCH 2/8] drm/i915/cdclk: Add and use xe2lpd_mdclk_source_sel()

[Gustavo Sousa]

Quoting Gustavo Sousa (2024-03-05 11:40:48-03:00)
>Quoting Matt Roper (2024-03-04 18:58:34-03:00)
>>On Mon, Mar 04, 2024 at 03:30:21PM -0300, Gustavo Sousa wrote:
>>> There will be future changes that rely on the source of the MDCLK. Let's
>>> have xe2lpd_mdclk_source_sel() as the function responsible for reporting
>>> that information.
>>> 
>>> Bspec: 69090
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_cdclk.c | 17 ++++++++++++++++-
>>>  drivers/gpu/drm/i915/i915_reg.h            |  4 +++-
>>>  2 files changed, 19 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> index 407bd541eb46..bf84bf27213f 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>>> @@ -1876,6 +1876,21 @@ static bool cdclk_pll_is_unknown(unsigned int vco)
>>>          return vco == ~0;
>>>  }
>>>  
>>> +static u32 xe2lpd_mdclk_source_sel(struct drm_i915_private *i915)
>>> +{
>>> +        if (DISPLAY_VER(i915) >= 20)
>>> +                return MDCLK_SOURCE_SEL_CDCLK_PLL;
>>> +
>>> +        /*
>>> +         * Earlier display IPs do not provide means of selecting the
>>> +         * MDCLK source, but MDCLK_SOURCE_SEL_CD2XCLK is a nice default,
>>> +         * since it reflects the source used for those and allows
>>> +         * xe2lpd_mdclk_source_sel() to be used in logic that depends on
>>> +         * it.
>>> +         */
>>> +        return MDCLK_SOURCE_SEL_CD2XCLK;
>>
>>At the moment this function only gets called on Xe2 and beyond where the
>>register field exists; if that's going to change soon, then wouldn't it
>>be more natural to just use an early exit to highlight that there's
>>nothing we need to OR into the CDCLK_CTL for earlier platforms?  
>>
>>    /* Not configurable for older platforms; they always use CD2XCLK */
>>    if (DISPLAY_VER(i915) < 20)
>>            return 0;
>>
>>Functionally it's the same, but it feels more intuitive to me.
>>
>>If we aren't expecting to call this from common codepaths that aren't
>>already protected by a display version check, then we could make this a
>>drm_WARN_ON() to assert that we haven't deviated from expected behavior.
>
>Well, the intention here was for this function to serve 2 purposes: (i)
>to give the value of the "MDCLK Source Select" field of CDCLK_CTL and
>also (ii) tell which was the source of the MDCLK for displays pre and
>post (including) Xe2LPD, because we will need that information in
>"drm/i915: Add mdclk_cdclk_ratio to intel_dbuf_state".
>
>I was hoping to do that instead of creating a new enum, but maybe it
>will just cause confusion?
>
>Should we have one function to tell us the source and another for
>giving the value of the register field?

Unless there are objections here, I plan on doing something along these
lines for v2.

--
Gustavo Sousa

>
>--
>Gustavo Sousa
>
>>
>>
>>Matt
>>
>>> +}
>>> +
>>>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>>>                                                      const struct intel_cdclk_config *old_cdclk_config,
>>>                                                      const struct intel_cdclk_config *new_cdclk_config,
>>> @@ -1980,7 +1995,7 @@ static u32 bxt_cdclk_ctl(struct drm_i915_private *i915,
>>>                  val |= BXT_CDCLK_SSA_PRECHARGE_ENABLE;
>>>  
>>>          if (DISPLAY_VER(i915) >= 20)
>>> -                val |= MDCLK_SOURCE_SEL_CDCLK_PLL;
>>> +                val |= xe2lpd_mdclk_source_sel(i915);
>>>          else
>>>                  val |= skl_cdclk_decimal(cdclk);
>>>  
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index e00557e1a57f..eb953ed1f113 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -5900,7 +5900,9 @@ enum skl_power_gate {
>>>  #define  CDCLK_FREQ_540                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 1)
>>>  #define  CDCLK_FREQ_337_308                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 2)
>>>  #define  CDCLK_FREQ_675_617                REG_FIELD_PREP(CDCLK_FREQ_SEL_MASK, 3)
>>> -#define  MDCLK_SOURCE_SEL_CDCLK_PLL        REG_BIT(25)
>>> +#define  MDCLK_SOURCE_SEL_MASK                REG_GENMASK(25, 25)
>>> +#define  MDCLK_SOURCE_SEL_CD2XCLK        REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 0)
>>> +#define  MDCLK_SOURCE_SEL_CDCLK_PLL        REG_FIELD_PREP(MDCLK_SOURCE_SEL_MASK, 1)
>>>  #define  BXT_CDCLK_CD2X_DIV_SEL_MASK        REG_GENMASK(23, 22)
>>>  #define  BXT_CDCLK_CD2X_DIV_SEL_1        REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 0)
>>>  #define  BXT_CDCLK_CD2X_DIV_SEL_1_5        REG_FIELD_PREP(BXT_CDCLK_CD2X_DIV_SEL_MASK, 1)
>>> -- 
>>> 2.44.0
>>> 
>>
>>-- 
>>Matt Roper
>>Graphics Software Engineer
>>Linux GPU Platform Enablement
>>Intel Corporation

Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

[Lisovskiy, Stanislav]

On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote:
> Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit
> 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started
> adding support for CDCLK programming support for Xe2LPD. One final piece
> is missing, which is the programming necessary for changed in the ratio
> between MDCLK and CDCLK. Let's do that now.
> 
> BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the
> ratio between MDCLK and CDCLK changes. The updates must be done before
> changing the CDCLK when decreasing the frequency; or after it when
> increasing the frequency.
> 
> Ratio-related updates to MBUS_CTL also depend on the state of MBus
> joining, so they are performed by either CDCLK change sequence or by
> changes in MBus joining. Since one might happen independently of the
> other, we need to make sure that both logics see the necessary state
> values when programming that register. MBus joining logic needs to know
> the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio
> field of struct intel_dbuf_state.
> 
> For the CDCLK logic, we need to have something similar: we need to
> propagate the status of MBus joining to struct intel_cdclk_state. Do
> that by adding the field joined_mbus to struct intel_cdclk_config.
> (Preferably, that field would be added to intel_cdclk_state, however
> currently only intel_cdclk_config is passed down to the functions that
> do the register programming. We might revisit this decision if we find
> that refactoring the code to pass the whole intel_cdclk_state is worth
> it.)
> 
> Bspec: 68864, 68868, 69090, 69482
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 31 ++++++++++++++
>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  3 ++
>  drivers/gpu/drm/i915/display/skl_watermark.c  | 40 +++++++++++++++----
>  drivers/gpu/drm/i915/display/skl_watermark.h  |  1 +
>  .../gpu/drm/i915/display/skl_watermark_regs.h | 18 +++++----
>  5 files changed, 77 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 04a6e9806254..12753589072d 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -40,6 +40,7 @@
>  #include "intel_psr.h"
>  #include "intel_vdsc.h"
>  #include "skl_watermark.h"
> +#include "skl_watermark_regs.h"
>  #include "vlv_sideband.h"
>  
>  /**
> @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
>  	}
>  
>   out:
> +	if (DISPLAY_VER(dev_priv) >= 20)
> +		cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
>  	/*
>  	 * Can't read this out :( Let's assume it's
>  	 * at least what the CDCLK frequency requires.
> @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>  	}
>  }
>  
> +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915,
> +					     const struct intel_cdclk_config *cdclk_config)
> +{
> +	intel_dbuf_mdclk_cdclk_ratio_update(i915,
> +					    intel_mdclk_cdclk_ratio(i915, cdclk_config),
> +					    cdclk_config->joined_mbus);
> +}
> +
>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>  						    const struct intel_cdclk_config *old_cdclk_config,
>  						    const struct intel_cdclk_config *new_cdclk_config,
> @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		return;
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 20 && cdclk < dev_priv->display.cdclk.hw.cdclk)
> +		xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
> +
>  	if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, &dev_priv->display.cdclk.hw,
>  						    cdclk_config, &mid_cdclk_config)) {
>  		_bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe);
> @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>  		_bxt_set_cdclk(dev_priv, cdclk_config, pipe);
>  	}
>  
> +	if (DISPLAY_VER(dev_priv) >= 20 && cdclk > dev_priv->display.cdclk.hw.cdclk)
> +		xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
> +
>  	if (DISPLAY_VER(dev_priv) >= 14)
>  		/*
>  		 * NOOP - No Pcode communication needed for
> @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
>  	return 0;
>  }
>  
> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus)
> +{
> +	struct intel_cdclk_state *cdclk_state;
> +
> +	cdclk_state = intel_atomic_get_cdclk_state(state);
> +	if (IS_ERR(cdclk_state))
> +		return PTR_ERR(cdclk_state);
> +
> +	cdclk_state->actual.joined_mbus = joined_mbus;
> +	cdclk_state->logical.joined_mbus = joined_mbus;
> +
> +	return intel_atomic_lock_global_state(&cdclk_state->base);

Shouldn't we actually lock global state already before we modify
the cdclk_state?


Stan

> +}
> +
>  int intel_cdclk_init(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_cdclk_state *cdclk_state;
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 8e6e302bd599..e358234bfe26 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -18,6 +18,8 @@ struct intel_crtc_state;
>  struct intel_cdclk_config {
>  	unsigned int cdclk, vco, ref, bypass;
>  	u8 voltage_level;
> +	/* This field is only valid for Xe2LPD and above. */
> +	bool joined_mbus;
>  };
>  
>  struct intel_cdclk_state {
> @@ -74,6 +76,7 @@ void intel_cdclk_get_cdclk(struct drm_i915_private *dev_priv,
>  			   struct intel_cdclk_config *cdclk_config);
>  int intel_cdclk_atomic_check(struct intel_atomic_state *state,
>  			     bool *need_cdclk_calc);
> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus);
>  struct intel_cdclk_state *
>  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>  
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 4410e21888ad..bc341abcab2f 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -12,6 +12,7 @@
>  #include "intel_atomic.h"
>  #include "intel_atomic_plane.h"
>  #include "intel_bw.h"
> +#include "intel_cdclk.h"
>  #include "intel_crtc.h"
>  #include "intel_de.h"
>  #include "intel_display.h"
> @@ -2601,10 +2602,17 @@ skl_compute_ddb(struct intel_atomic_state *state)
>  			return ret;
>  	}
>  
> -	if (HAS_MBUS_JOINING(i915))
> +	if (HAS_MBUS_JOINING(i915)) {
>  		new_dbuf_state->joined_mbus =
>  			adlp_check_mbus_joined(new_dbuf_state->active_pipes);
>  
> +		if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> +			ret = intel_cdclk_state_set_joined_mbus(state, new_dbuf_state->joined_mbus);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
>  	for_each_intel_crtc(&i915->drm, crtc) {
>  		enum pipe pipe = crtc->pipe;
>  
> @@ -3545,12 +3553,14 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
>  	return intel_atomic_lock_global_state(&dbuf_state->base);
>  }
>  
> -static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
> -						u8 ratio,
> -						bool joined_mbus)
> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus)
>  {
>  	enum dbuf_slice slice;
>  
> +	if (DISPLAY_VER(i915) >= 20)
> +		intel_de_rmw(i915, MBUS_CTL, MBUS_TRANSLATION_THROTTLE_MIN_MASK,
> +			     MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1));
> +
>  	if (joined_mbus)
>  		ratio *= 2;
>  
> @@ -3568,7 +3578,9 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  {
>  	struct drm_i915_private *i915 = to_i915(state->base.dev);
>  	u32 mbus_ctl;
> -	const struct intel_dbuf_state *dbuf_state =
> +	const struct intel_dbuf_state *old_dbuf_state =
> +		intel_atomic_get_old_dbuf_state(state);
> +	const struct intel_dbuf_state *new_dbuf_state =
>  		intel_atomic_get_new_dbuf_state(state);
>  
>  	if (!HAS_MBUS_JOINING(i915))
> @@ -3578,7 +3590,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  	 * TODO: Implement vblank synchronized MBUS joining changes.
>  	 * Must be properly coordinated with dbuf reprogramming.
>  	 */
> -	if (dbuf_state->joined_mbus)
> +	if (new_dbuf_state->joined_mbus)
>  		mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
>  			MBUS_JOIN_PIPE_SELECT_NONE;
>  	else
> @@ -3589,8 +3601,20 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>  		     MBUS_HASHING_MODE_MASK | MBUS_JOIN |
>  		     MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
>  
> -	intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
> -					    dbuf_state->joined_mbus);
> +	if (DISPLAY_VER(i915) >= 20 &&
> +	    old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) {
> +		/*
> +		 * For Xe2LPD and beyond, when there is a change in the ratio
> +		 * between MDCLK and CDCLK, updates to related registers need to
> +		 * happen at a specific point in the CDCLK change sequence. In
> +		 * that case, we defer to the call to
> +		 * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic.
> +		 */
> +		return;
> +	}
> +
> +	intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio,
> +					    new_dbuf_state->joined_mbus);
>  }
>  
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
> index fed4d12df584..3a90741cab06 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> @@ -76,6 +76,7 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
>  
>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus);
>  void intel_mbus_dbox_update(struct intel_atomic_state *state);
>  
>  #endif /* __SKL_WATERMARK_H__ */
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> index 20b30c9a6613..269163fa3350 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> @@ -32,14 +32,16 @@
>  #define MBUS_BBOX_CTL_S1		_MMIO(0x45040)
>  #define MBUS_BBOX_CTL_S2		_MMIO(0x45044)
>  
> -#define MBUS_CTL			_MMIO(0x4438C)
> -#define MBUS_JOIN			REG_BIT(31)
> -#define MBUS_HASHING_MODE_MASK		REG_BIT(30)
> -#define MBUS_HASHING_MODE_2x2		REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
> -#define MBUS_HASHING_MODE_1x4		REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
> -#define MBUS_JOIN_PIPE_SELECT_MASK	REG_GENMASK(28, 26)
> -#define MBUS_JOIN_PIPE_SELECT(pipe)	REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
> -#define MBUS_JOIN_PIPE_SELECT_NONE	MBUS_JOIN_PIPE_SELECT(7)
> +#define MBUS_CTL				_MMIO(0x4438C)
> +#define   MBUS_JOIN				REG_BIT(31)
> +#define   MBUS_HASHING_MODE_MASK		REG_BIT(30)
> +#define   MBUS_HASHING_MODE_2x2			REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
> +#define   MBUS_HASHING_MODE_1x4			REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
> +#define   MBUS_JOIN_PIPE_SELECT_MASK		REG_GENMASK(28, 26)
> +#define   MBUS_JOIN_PIPE_SELECT(pipe)		REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
> +#define   MBUS_JOIN_PIPE_SELECT_NONE		MBUS_JOIN_PIPE_SELECT(7)
> +#define   MBUS_TRANSLATION_THROTTLE_MIN_MASK	REG_GENMASK(15, 13)
> +#define   MBUS_TRANSLATION_THROTTLE_MIN(val)	REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val)
>  
>  /* Watermark register definitions for SKL */
>  #define _CUR_WM_A_0		0x70140
> -- 
> 2.44.0
> 

Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

[Gustavo Sousa]

Quoting Lisovskiy, Stanislav (2024-03-11 18:01:04-03:00)
>On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote:
>> Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit
>> 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started
>> adding support for CDCLK programming support for Xe2LPD. One final piece
>> is missing, which is the programming necessary for changed in the ratio
>> between MDCLK and CDCLK. Let's do that now.
>> 
>> BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the
>> ratio between MDCLK and CDCLK changes. The updates must be done before
>> changing the CDCLK when decreasing the frequency; or after it when
>> increasing the frequency.
>> 
>> Ratio-related updates to MBUS_CTL also depend on the state of MBus
>> joining, so they are performed by either CDCLK change sequence or by
>> changes in MBus joining. Since one might happen independently of the
>> other, we need to make sure that both logics see the necessary state
>> values when programming that register. MBus joining logic needs to know
>> the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio
>> field of struct intel_dbuf_state.
>> 
>> For the CDCLK logic, we need to have something similar: we need to
>> propagate the status of MBus joining to struct intel_cdclk_state. Do
>> that by adding the field joined_mbus to struct intel_cdclk_config.
>> (Preferably, that field would be added to intel_cdclk_state, however
>> currently only intel_cdclk_config is passed down to the functions that
>> do the register programming. We might revisit this decision if we find
>> that refactoring the code to pass the whole intel_cdclk_state is worth
>> it.)
>> 
>> Bspec: 68864, 68868, 69090, 69482
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 31 ++++++++++++++
>>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  3 ++
>>  drivers/gpu/drm/i915/display/skl_watermark.c  | 40 +++++++++++++++----
>>  drivers/gpu/drm/i915/display/skl_watermark.h  |  1 +
>>  .../gpu/drm/i915/display/skl_watermark_regs.h | 18 +++++----
>>  5 files changed, 77 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index 04a6e9806254..12753589072d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -40,6 +40,7 @@
>>  #include "intel_psr.h"
>>  #include "intel_vdsc.h"
>>  #include "skl_watermark.h"
>> +#include "skl_watermark_regs.h"
>>  #include "vlv_sideband.h"
>>  
>>  /**
>> @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
>>          }
>>  
>>   out:
>> +        if (DISPLAY_VER(dev_priv) >= 20)
>> +                cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
>>          /*
>>           * Can't read this out :( Let's assume it's
>>           * at least what the CDCLK frequency requires.
>> @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
>>          }
>>  }
>>  
>> +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915,
>> +                                             const struct intel_cdclk_config *cdclk_config)
>> +{
>> +        intel_dbuf_mdclk_cdclk_ratio_update(i915,
>> +                                            intel_mdclk_cdclk_ratio(i915, cdclk_config),
>> +                                            cdclk_config->joined_mbus);
>> +}
>> +
>>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
>>                                                      const struct intel_cdclk_config *old_cdclk_config,
>>                                                      const struct intel_cdclk_config *new_cdclk_config,
>> @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>>                  return;
>>          }
>>  
>> +        if (DISPLAY_VER(dev_priv) >= 20 && cdclk < dev_priv->display.cdclk.hw.cdclk)
>> +                xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
>> +
>>          if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, &dev_priv->display.cdclk.hw,
>>                                                      cdclk_config, &mid_cdclk_config)) {
>>                  _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe);
>> @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
>>                  _bxt_set_cdclk(dev_priv, cdclk_config, pipe);
>>          }
>>  
>> +        if (DISPLAY_VER(dev_priv) >= 20 && cdclk > dev_priv->display.cdclk.hw.cdclk)
>> +                xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
>> +
>>          if (DISPLAY_VER(dev_priv) >= 14)
>>                  /*
>>                   * NOOP - No Pcode communication needed for
>> @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
>>          return 0;
>>  }
>>  
>> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus)
>> +{
>> +        struct intel_cdclk_state *cdclk_state;
>> +
>> +        cdclk_state = intel_atomic_get_cdclk_state(state);
>> +        if (IS_ERR(cdclk_state))
>> +                return PTR_ERR(cdclk_state);
>> +
>> +        cdclk_state->actual.joined_mbus = joined_mbus;
>> +        cdclk_state->logical.joined_mbus = joined_mbus;
>> +
>> +        return intel_atomic_lock_global_state(&cdclk_state->base);
>
>Shouldn't we actually lock global state already before we modify
>the cdclk_state?

I don't think so. The cdclk_state that we have here is already a copy
that belongs only to this commit. I think it is actually critical that
we have the write locked at the swap time, which is done only after the
atomic checks.

As such, I think locking after we modify the copy should be fine. In
fact, we already have places in the code using the same pattern.

--
Gustavo Sousa

>
>
>Stan
>
>> +}
>> +
>>  int intel_cdclk_init(struct drm_i915_private *dev_priv)
>>  {
>>          struct intel_cdclk_state *cdclk_state;
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> index 8e6e302bd599..e358234bfe26 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> @@ -18,6 +18,8 @@ struct intel_crtc_state;
>>  struct intel_cdclk_config {
>>          unsigned int cdclk, vco, ref, bypass;
>>          u8 voltage_level;
>> +        /* This field is only valid for Xe2LPD and above. */
>> +        bool joined_mbus;
>>  };
>>  
>>  struct intel_cdclk_state {
>> @@ -74,6 +76,7 @@ void intel_cdclk_get_cdclk(struct drm_i915_private *dev_priv,
>>                             struct intel_cdclk_config *cdclk_config);
>>  int intel_cdclk_atomic_check(struct intel_atomic_state *state,
>>                               bool *need_cdclk_calc);
>> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus);
>>  struct intel_cdclk_state *
>>  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
>>  
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
>> index 4410e21888ad..bc341abcab2f 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
>> @@ -12,6 +12,7 @@
>>  #include "intel_atomic.h"
>>  #include "intel_atomic_plane.h"
>>  #include "intel_bw.h"
>> +#include "intel_cdclk.h"
>>  #include "intel_crtc.h"
>>  #include "intel_de.h"
>>  #include "intel_display.h"
>> @@ -2601,10 +2602,17 @@ skl_compute_ddb(struct intel_atomic_state *state)
>>                          return ret;
>>          }
>>  
>> -        if (HAS_MBUS_JOINING(i915))
>> +        if (HAS_MBUS_JOINING(i915)) {
>>                  new_dbuf_state->joined_mbus =
>>                          adlp_check_mbus_joined(new_dbuf_state->active_pipes);
>>  
>> +                if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
>> +                        ret = intel_cdclk_state_set_joined_mbus(state, new_dbuf_state->joined_mbus);
>> +                        if (ret)
>> +                                return ret;
>> +                }
>> +        }
>> +
>>          for_each_intel_crtc(&i915->drm, crtc) {
>>                  enum pipe pipe = crtc->pipe;
>>  
>> @@ -3545,12 +3553,14 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
>>          return intel_atomic_lock_global_state(&dbuf_state->base);
>>  }
>>  
>> -static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
>> -                                                u8 ratio,
>> -                                                bool joined_mbus)
>> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus)
>>  {
>>          enum dbuf_slice slice;
>>  
>> +        if (DISPLAY_VER(i915) >= 20)
>> +                intel_de_rmw(i915, MBUS_CTL, MBUS_TRANSLATION_THROTTLE_MIN_MASK,
>> +                             MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1));
>> +
>>          if (joined_mbus)
>>                  ratio *= 2;
>>  
>> @@ -3568,7 +3578,9 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>>  {
>>          struct drm_i915_private *i915 = to_i915(state->base.dev);
>>          u32 mbus_ctl;
>> -        const struct intel_dbuf_state *dbuf_state =
>> +        const struct intel_dbuf_state *old_dbuf_state =
>> +                intel_atomic_get_old_dbuf_state(state);
>> +        const struct intel_dbuf_state *new_dbuf_state =
>>                  intel_atomic_get_new_dbuf_state(state);
>>  
>>          if (!HAS_MBUS_JOINING(i915))
>> @@ -3578,7 +3590,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>>           * TODO: Implement vblank synchronized MBUS joining changes.
>>           * Must be properly coordinated with dbuf reprogramming.
>>           */
>> -        if (dbuf_state->joined_mbus)
>> +        if (new_dbuf_state->joined_mbus)
>>                  mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
>>                          MBUS_JOIN_PIPE_SELECT_NONE;
>>          else
>> @@ -3589,8 +3601,20 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
>>                       MBUS_HASHING_MODE_MASK | MBUS_JOIN |
>>                       MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
>>  
>> -        intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
>> -                                            dbuf_state->joined_mbus);
>> +        if (DISPLAY_VER(i915) >= 20 &&
>> +            old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) {
>> +                /*
>> +                 * For Xe2LPD and beyond, when there is a change in the ratio
>> +                 * between MDCLK and CDCLK, updates to related registers need to
>> +                 * happen at a specific point in the CDCLK change sequence. In
>> +                 * that case, we defer to the call to
>> +                 * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic.
>> +                 */
>> +                return;
>> +        }
>> +
>> +        intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio,
>> +                                            new_dbuf_state->joined_mbus);
>>  }
>>  
>>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
>> index fed4d12df584..3a90741cab06 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
>> @@ -76,6 +76,7 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
>>  
>>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
>>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
>> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus);
>>  void intel_mbus_dbox_update(struct intel_atomic_state *state);
>>  
>>  #endif /* __SKL_WATERMARK_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
>> index 20b30c9a6613..269163fa3350 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
>> @@ -32,14 +32,16 @@
>>  #define MBUS_BBOX_CTL_S1                _MMIO(0x45040)
>>  #define MBUS_BBOX_CTL_S2                _MMIO(0x45044)
>>  
>> -#define MBUS_CTL                        _MMIO(0x4438C)
>> -#define MBUS_JOIN                        REG_BIT(31)
>> -#define MBUS_HASHING_MODE_MASK                REG_BIT(30)
>> -#define MBUS_HASHING_MODE_2x2                REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
>> -#define MBUS_HASHING_MODE_1x4                REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
>> -#define MBUS_JOIN_PIPE_SELECT_MASK        REG_GENMASK(28, 26)
>> -#define MBUS_JOIN_PIPE_SELECT(pipe)        REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
>> -#define MBUS_JOIN_PIPE_SELECT_NONE        MBUS_JOIN_PIPE_SELECT(7)
>> +#define MBUS_CTL                                _MMIO(0x4438C)
>> +#define   MBUS_JOIN                                REG_BIT(31)
>> +#define   MBUS_HASHING_MODE_MASK                REG_BIT(30)
>> +#define   MBUS_HASHING_MODE_2x2                        REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
>> +#define   MBUS_HASHING_MODE_1x4                        REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
>> +#define   MBUS_JOIN_PIPE_SELECT_MASK                REG_GENMASK(28, 26)
>> +#define   MBUS_JOIN_PIPE_SELECT(pipe)                REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
>> +#define   MBUS_JOIN_PIPE_SELECT_NONE                MBUS_JOIN_PIPE_SELECT(7)
>> +#define   MBUS_TRANSLATION_THROTTLE_MIN_MASK        REG_GENMASK(15, 13)
>> +#define   MBUS_TRANSLATION_THROTTLE_MIN(val)        REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val)
>>  
>>  /* Watermark register definitions for SKL */
>>  #define _CUR_WM_A_0                0x70140
>> -- 
>> 2.44.0
>>

Re: [PATCH 6/8] drm/i915/xe2lpd: Support MDCLK:CDCLK ratio changes

[Lisovskiy, Stanislav]

On Mon, Mar 11, 2024 at 06:13:29PM -0300, Gustavo Sousa wrote:
> Quoting Lisovskiy, Stanislav (2024-03-11 18:01:04-03:00)
> >On Mon, Mar 04, 2024 at 03:30:25PM -0300, Gustavo Sousa wrote:
> >> Commit 394b4b7df9f7 ("drm/i915/lnl: Add CDCLK table") and commit
> >> 3d3696c0fed1 ("drm/i915/lnl: Start using CDCLK through PLL") started
> >> adding support for CDCLK programming support for Xe2LPD. One final piece
> >> is missing, which is the programming necessary for changed in the ratio
> >> between MDCLK and CDCLK. Let's do that now.
> >> 
> >> BSpec instructs us to update MBUS_CTL and DBUF_CTL_S* registers when the
> >> ratio between MDCLK and CDCLK changes. The updates must be done before
> >> changing the CDCLK when decreasing the frequency; or after it when
> >> increasing the frequency.
> >> 
> >> Ratio-related updates to MBUS_CTL also depend on the state of MBus
> >> joining, so they are performed by either CDCLK change sequence or by
> >> changes in MBus joining. Since one might happen independently of the
> >> other, we need to make sure that both logics see the necessary state
> >> values when programming that register. MBus joining logic needs to know
> >> the MDCLK:CDCLK ratio and that's already provided via mdclk_cdclk_ratio
> >> field of struct intel_dbuf_state.
> >> 
> >> For the CDCLK logic, we need to have something similar: we need to
> >> propagate the status of MBus joining to struct intel_cdclk_state. Do
> >> that by adding the field joined_mbus to struct intel_cdclk_config.
> >> (Preferably, that field would be added to intel_cdclk_state, however
> >> currently only intel_cdclk_config is passed down to the functions that
> >> do the register programming. We might revisit this decision if we find
> >> that refactoring the code to pass the whole intel_cdclk_state is worth
> >> it.)
> >> 
> >> Bspec: 68864, 68868, 69090, 69482
> >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 31 ++++++++++++++
> >>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  3 ++
> >>  drivers/gpu/drm/i915/display/skl_watermark.c  | 40 +++++++++++++++----
> >>  drivers/gpu/drm/i915/display/skl_watermark.h  |  1 +
> >>  .../gpu/drm/i915/display/skl_watermark_regs.h | 18 +++++----
> >>  5 files changed, 77 insertions(+), 16 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> index 04a6e9806254..12753589072d 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> @@ -40,6 +40,7 @@
> >>  #include "intel_psr.h"
> >>  #include "intel_vdsc.h"
> >>  #include "skl_watermark.h"
> >> +#include "skl_watermark_regs.h"
> >>  #include "vlv_sideband.h"
> >>  
> >>  /**
> >> @@ -1683,6 +1684,8 @@ static void bxt_get_cdclk(struct drm_i915_private *dev_priv,
> >>          }
> >>  
> >>   out:
> >> +        if (DISPLAY_VER(dev_priv) >= 20)
> >> +                cdclk_config->joined_mbus = intel_de_read(dev_priv, MBUS_CTL) & MBUS_JOIN;
> >>          /*
> >>           * Can't read this out :( Let's assume it's
> >>           * at least what the CDCLK frequency requires.
> >> @@ -1908,6 +1911,14 @@ u8 intel_mdclk_cdclk_ratio(struct drm_i915_private *i915,
> >>          }
> >>  }
> >>  
> >> +static void xe2lpd_mdclk_cdclk_ratio_program(struct drm_i915_private *i915,
> >> +                                             const struct intel_cdclk_config *cdclk_config)
> >> +{
> >> +        intel_dbuf_mdclk_cdclk_ratio_update(i915,
> >> +                                            intel_mdclk_cdclk_ratio(i915, cdclk_config),
> >> +                                            cdclk_config->joined_mbus);
> >> +}
> >> +
> >>  static bool cdclk_compute_crawl_and_squash_midpoint(struct drm_i915_private *i915,
> >>                                                      const struct intel_cdclk_config *old_cdclk_config,
> >>                                                      const struct intel_cdclk_config *new_cdclk_config,
> >> @@ -2089,6 +2100,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> >>                  return;
> >>          }
> >>  
> >> +        if (DISPLAY_VER(dev_priv) >= 20 && cdclk < dev_priv->display.cdclk.hw.cdclk)
> >> +                xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
> >> +
> >>          if (cdclk_compute_crawl_and_squash_midpoint(dev_priv, &dev_priv->display.cdclk.hw,
> >>                                                      cdclk_config, &mid_cdclk_config)) {
> >>                  _bxt_set_cdclk(dev_priv, &mid_cdclk_config, pipe);
> >> @@ -2097,6 +2111,9 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv,
> >>                  _bxt_set_cdclk(dev_priv, cdclk_config, pipe);
> >>          }
> >>  
> >> +        if (DISPLAY_VER(dev_priv) >= 20 && cdclk > dev_priv->display.cdclk.hw.cdclk)
> >> +                xe2lpd_mdclk_cdclk_ratio_program(dev_priv, cdclk_config);
> >> +
> >>          if (DISPLAY_VER(dev_priv) >= 14)
> >>                  /*
> >>                   * NOOP - No Pcode communication needed for
> >> @@ -3179,6 +3196,20 @@ int intel_cdclk_atomic_check(struct intel_atomic_state *state,
> >>          return 0;
> >>  }
> >>  
> >> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus)
> >> +{
> >> +        struct intel_cdclk_state *cdclk_state;
> >> +
> >> +        cdclk_state = intel_atomic_get_cdclk_state(state);
> >> +        if (IS_ERR(cdclk_state))
> >> +                return PTR_ERR(cdclk_state);
> >> +
> >> +        cdclk_state->actual.joined_mbus = joined_mbus;
> >> +        cdclk_state->logical.joined_mbus = joined_mbus;
> >> +
> >> +        return intel_atomic_lock_global_state(&cdclk_state->base);
> >
> >Shouldn't we actually lock global state already before we modify
> >the cdclk_state?
> 
> I don't think so. The cdclk_state that we have here is already a copy
> that belongs only to this commit. I think it is actually critical that
> we have the write locked at the swap time, which is done only after the
> atomic checks.
> 
> As such, I think locking after we modify the copy should be fine. In
> fact, we already have places in the code using the same pattern.
> 
> --
> Gustavo Sousa

Yes, right. We lock global state if we modify it and/or serialize commit
if we want to write the hardware.

Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>


> 
> >
> >
> >Stan
> >
> >> +}
> >> +
> >>  int intel_cdclk_init(struct drm_i915_private *dev_priv)
> >>  {
> >>          struct intel_cdclk_state *cdclk_state;
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> index 8e6e302bd599..e358234bfe26 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> @@ -18,6 +18,8 @@ struct intel_crtc_state;
> >>  struct intel_cdclk_config {
> >>          unsigned int cdclk, vco, ref, bypass;
> >>          u8 voltage_level;
> >> +        /* This field is only valid for Xe2LPD and above. */
> >> +        bool joined_mbus;
> >>  };
> >>  
> >>  struct intel_cdclk_state {
> >> @@ -74,6 +76,7 @@ void intel_cdclk_get_cdclk(struct drm_i915_private *dev_priv,
> >>                             struct intel_cdclk_config *cdclk_config);
> >>  int intel_cdclk_atomic_check(struct intel_atomic_state *state,
> >>                               bool *need_cdclk_calc);
> >> +int intel_cdclk_state_set_joined_mbus(struct intel_atomic_state *state, bool joined_mbus);
> >>  struct intel_cdclk_state *
> >>  intel_atomic_get_cdclk_state(struct intel_atomic_state *state);
> >>  
> >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> >> index 4410e21888ad..bc341abcab2f 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> >> @@ -12,6 +12,7 @@
> >>  #include "intel_atomic.h"
> >>  #include "intel_atomic_plane.h"
> >>  #include "intel_bw.h"
> >> +#include "intel_cdclk.h"
> >>  #include "intel_crtc.h"
> >>  #include "intel_de.h"
> >>  #include "intel_display.h"
> >> @@ -2601,10 +2602,17 @@ skl_compute_ddb(struct intel_atomic_state *state)
> >>                          return ret;
> >>          }
> >>  
> >> -        if (HAS_MBUS_JOINING(i915))
> >> +        if (HAS_MBUS_JOINING(i915)) {
> >>                  new_dbuf_state->joined_mbus =
> >>                          adlp_check_mbus_joined(new_dbuf_state->active_pipes);
> >>  
> >> +                if (old_dbuf_state->joined_mbus != new_dbuf_state->joined_mbus) {
> >> +                        ret = intel_cdclk_state_set_joined_mbus(state, new_dbuf_state->joined_mbus);
> >> +                        if (ret)
> >> +                                return ret;
> >> +                }
> >> +        }
> >> +
> >>          for_each_intel_crtc(&i915->drm, crtc) {
> >>                  enum pipe pipe = crtc->pipe;
> >>  
> >> @@ -3545,12 +3553,14 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
> >>          return intel_atomic_lock_global_state(&dbuf_state->base);
> >>  }
> >>  
> >> -static void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915,
> >> -                                                u8 ratio,
> >> -                                                bool joined_mbus)
> >> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus)
> >>  {
> >>          enum dbuf_slice slice;
> >>  
> >> +        if (DISPLAY_VER(i915) >= 20)
> >> +                intel_de_rmw(i915, MBUS_CTL, MBUS_TRANSLATION_THROTTLE_MIN_MASK,
> >> +                             MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1));
> >> +
> >>          if (joined_mbus)
> >>                  ratio *= 2;
> >>  
> >> @@ -3568,7 +3578,9 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
> >>  {
> >>          struct drm_i915_private *i915 = to_i915(state->base.dev);
> >>          u32 mbus_ctl;
> >> -        const struct intel_dbuf_state *dbuf_state =
> >> +        const struct intel_dbuf_state *old_dbuf_state =
> >> +                intel_atomic_get_old_dbuf_state(state);
> >> +        const struct intel_dbuf_state *new_dbuf_state =
> >>                  intel_atomic_get_new_dbuf_state(state);
> >>  
> >>          if (!HAS_MBUS_JOINING(i915))
> >> @@ -3578,7 +3590,7 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
> >>           * TODO: Implement vblank synchronized MBUS joining changes.
> >>           * Must be properly coordinated with dbuf reprogramming.
> >>           */
> >> -        if (dbuf_state->joined_mbus)
> >> +        if (new_dbuf_state->joined_mbus)
> >>                  mbus_ctl = MBUS_HASHING_MODE_1x4 | MBUS_JOIN |
> >>                          MBUS_JOIN_PIPE_SELECT_NONE;
> >>          else
> >> @@ -3589,8 +3601,20 @@ static void update_mbus_pre_enable(struct intel_atomic_state *state)
> >>                       MBUS_HASHING_MODE_MASK | MBUS_JOIN |
> >>                       MBUS_JOIN_PIPE_SELECT_MASK, mbus_ctl);
> >>  
> >> -        intel_dbuf_mdclk_cdclk_ratio_update(i915, dbuf_state->mdclk_cdclk_ratio,
> >> -                                            dbuf_state->joined_mbus);
> >> +        if (DISPLAY_VER(i915) >= 20 &&
> >> +            old_dbuf_state->mdclk_cdclk_ratio != new_dbuf_state->mdclk_cdclk_ratio) {
> >> +                /*
> >> +                 * For Xe2LPD and beyond, when there is a change in the ratio
> >> +                 * between MDCLK and CDCLK, updates to related registers need to
> >> +                 * happen at a specific point in the CDCLK change sequence. In
> >> +                 * that case, we defer to the call to
> >> +                 * intel_dbuf_mdclk_cdclk_ratio_update() to the CDCLK logic.
> >> +                 */
> >> +                return;
> >> +        }
> >> +
> >> +        intel_dbuf_mdclk_cdclk_ratio_update(i915, new_dbuf_state->mdclk_cdclk_ratio,
> >> +                                            new_dbuf_state->joined_mbus);
> >>  }
> >>  
> >>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state)
> >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
> >> index fed4d12df584..3a90741cab06 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_watermark.h
> >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.h
> >> @@ -76,6 +76,7 @@ int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state, u8
> >>  
> >>  void intel_dbuf_pre_plane_update(struct intel_atomic_state *state);
> >>  void intel_dbuf_post_plane_update(struct intel_atomic_state *state);
> >> +void intel_dbuf_mdclk_cdclk_ratio_update(struct drm_i915_private *i915, u8 ratio, bool joined_mbus);
> >>  void intel_mbus_dbox_update(struct intel_atomic_state *state);
> >>  
> >>  #endif /* __SKL_WATERMARK_H__ */
> >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> >> index 20b30c9a6613..269163fa3350 100644
> >> --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> >> +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h
> >> @@ -32,14 +32,16 @@
> >>  #define MBUS_BBOX_CTL_S1                _MMIO(0x45040)
> >>  #define MBUS_BBOX_CTL_S2                _MMIO(0x45044)
> >>  
> >> -#define MBUS_CTL                        _MMIO(0x4438C)
> >> -#define MBUS_JOIN                        REG_BIT(31)
> >> -#define MBUS_HASHING_MODE_MASK                REG_BIT(30)
> >> -#define MBUS_HASHING_MODE_2x2                REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
> >> -#define MBUS_HASHING_MODE_1x4                REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
> >> -#define MBUS_JOIN_PIPE_SELECT_MASK        REG_GENMASK(28, 26)
> >> -#define MBUS_JOIN_PIPE_SELECT(pipe)        REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
> >> -#define MBUS_JOIN_PIPE_SELECT_NONE        MBUS_JOIN_PIPE_SELECT(7)
> >> +#define MBUS_CTL                                _MMIO(0x4438C)
> >> +#define   MBUS_JOIN                                REG_BIT(31)
> >> +#define   MBUS_HASHING_MODE_MASK                REG_BIT(30)
> >> +#define   MBUS_HASHING_MODE_2x2                        REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0)
> >> +#define   MBUS_HASHING_MODE_1x4                        REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1)
> >> +#define   MBUS_JOIN_PIPE_SELECT_MASK                REG_GENMASK(28, 26)
> >> +#define   MBUS_JOIN_PIPE_SELECT(pipe)                REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe)
> >> +#define   MBUS_JOIN_PIPE_SELECT_NONE                MBUS_JOIN_PIPE_SELECT(7)
> >> +#define   MBUS_TRANSLATION_THROTTLE_MIN_MASK        REG_GENMASK(15, 13)
> >> +#define   MBUS_TRANSLATION_THROTTLE_MIN(val)        REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val)
> >>  
> >>  /* Watermark register definitions for SKL */
> >>  #define _CUR_WM_A_0                0x70140
> >> -- 
> >> 2.44.0
> >>