[PATCH 00/16] drm/i915/display: make all global state opaque

[PATCH 00/16] drm/i915/display: make all global state opaque

[Jani Nikula]

Hide all the structs that "derive" from struct intel_global_state inside
their respective implementation files.

Jani Nikula (16):
  drm/i915/wm: abstract intel_dbuf_pmdemand_needs_update()
  drm/i915/wm: add more accessors to dbuf state
  drm/i915/wm: make struct intel_dbuf_state opaque type
  drm/i915/bw: abstract intel_bw_pmdemand_needs_update()
  drm/i915/bw: relocate intel_can_enable_sagv() and rename to
    intel_bw_can_enable_sagv()
  drm/i915: move icl_sagv_{pre,post}_plane_update() to intel_bw.c
  drm/i915/bw: abstract intel_bw_qgv_point_peakbw()
  drm/i915/bw: make struct intel_bw_state opaque
  drm/i915/cdclk: abstract intel_cdclk_logical()
  drm/i915/cdclk: abstract intel_cdclk_min_cdclk()
  drm/i915/cdclk: abstract intel_cdclk_bw_min_cdclk()
  drm/i915/cdclk: abstract intel_cdclk_pmdemand_needs_update()
  drm/i915/cdclk: abstract intel_cdclk_force_min_cdclk()
  drm/i915/cdclk: abstract intel_cdclk_read_hw()
  drm/i915/cdclk: abstract intel_cdclk_actual() and
    intel_cdclk_actual_voltage_level()
  drm/i915/cdclk: make struct intel_cdclk_state opaque

 drivers/gpu/drm/i915/display/hsw_ips.c        |   2 +-
 .../gpu/drm/i915/display/intel_atomic_plane.c |   4 +-
 drivers/gpu/drm/i915/display/intel_audio.c    |   2 +-
 drivers/gpu/drm/i915/display/intel_bw.c       | 153 ++++++++++++++++--
 drivers/gpu/drm/i915/display/intel_bw.h       |  53 ++----
 drivers/gpu/drm/i915/display/intel_cdclk.c    |  93 +++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  50 ++----
 drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
 .../drm/i915/display/intel_display_driver.c   |   8 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      |   2 +-
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  41 ++---
 drivers/gpu/drm/i915/display/skl_watermark.c  | 134 +++++++--------
 drivers/gpu/drm/i915/display/skl_watermark.h  |  33 +---
 13 files changed, 336 insertions(+), 241 deletions(-)

-- 
2.39.5

[PATCH 01/16] drm/i915/wm: abstract intel_dbuf_pmdemand_needs_update()

[Jani Nikula]

Add intel_dbuf_pmdemand_needs_update() helper to avoid looking at struct
intel_dbuf_state internals outside of skl_watermark.c.

With this, we can also move to_intel_dbuf_state(),
intel_atomic_get_old_dbuf_state(), and intel_atomic_get_new_dbuf_state()
inside skl_watermark.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_pmdemand.c | 14 +--------
 drivers/gpu/drm/i915/display/skl_watermark.c  | 30 +++++++++++++++++++
 drivers/gpu/drm/i915/display/skl_watermark.h  | 10 ++-----
 3 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index 93d5ee36fff1..0f1501c456df 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -294,11 +294,9 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
 
 static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 {
-	struct intel_display *display = to_intel_display(state);
 	const struct intel_bw_state *new_bw_state, *old_bw_state;
 	const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
 	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
-	const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state;
 	struct intel_crtc *crtc;
 	int i;
 
@@ -308,19 +306,9 @@ static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 	    old_bw_state->qgv_point_peakbw)
 		return true;
 
-	new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
-	old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
-	if (new_dbuf_state &&
-	    new_dbuf_state->active_pipes != old_dbuf_state->active_pipes)
+	if (intel_dbuf_pmdemand_needs_update(state))
 		return true;
 
-	if (DISPLAY_VER(display) < 30) {
-		if (new_dbuf_state &&
-		    new_dbuf_state->enabled_slices !=
-		    old_dbuf_state->enabled_slices)
-			return true;
-	}
-
 	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
 	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
 	if (new_cdclk_state &&
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 2c2371574d6f..55280d16f9f7 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -38,6 +38,14 @@
  */
 #define DSB_EXE_TIME 100
 
+#define to_intel_dbuf_state(global_state) \
+	container_of_const((global_state), struct intel_dbuf_state, base)
+
+#define intel_atomic_get_old_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state, &to_intel_display(state)->dbuf.obj))
+#define intel_atomic_get_new_dbuf_state(state) \
+	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_intel_display(state)->dbuf.obj))
+
 static void skl_sagv_disable(struct intel_display *display);
 
 /* Stores plane specific WM parameters */
@@ -3693,6 +3701,28 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
 	gen9_dbuf_slices_update(display, new_slices);
 }
 
+bool intel_dbuf_pmdemand_needs_update(struct intel_atomic_state *state)
+{
+	struct intel_display *display = to_intel_display(state);
+	const struct intel_dbuf_state *new_dbuf_state, *old_dbuf_state;
+
+	new_dbuf_state = intel_atomic_get_new_dbuf_state(state);
+	old_dbuf_state = intel_atomic_get_old_dbuf_state(state);
+
+	if (new_dbuf_state &&
+	    new_dbuf_state->active_pipes != old_dbuf_state->active_pipes)
+		return true;
+
+	if (DISPLAY_VER(display) < 30) {
+		if (new_dbuf_state &&
+		    new_dbuf_state->enabled_slices !=
+		    old_dbuf_state->enabled_slices)
+			return true;
+	}
+
+	return false;
+}
+
 static void skl_mbus_sanitize(struct intel_display *display)
 {
 	struct intel_dbuf_state *dbuf_state =
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index 95b0b599d5c3..3b9a0b254cff 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -78,14 +78,6 @@ struct intel_dbuf_state {
 struct intel_dbuf_state *
 intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 
-#define to_intel_dbuf_state(global_state) \
-	container_of_const((global_state), struct intel_dbuf_state, base)
-
-#define intel_atomic_get_old_dbuf_state(state) \
-	to_intel_dbuf_state(intel_atomic_get_old_global_obj_state(state, &to_intel_display(state)->dbuf.obj))
-#define intel_atomic_get_new_dbuf_state(state) \
-	to_intel_dbuf_state(intel_atomic_get_new_global_obj_state(state, &to_intel_display(state)->dbuf.obj))
-
 int intel_dbuf_init(struct intel_display *display);
 int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state,
 					   int ratio);
@@ -98,5 +90,7 @@ void intel_dbuf_mbus_pre_ddb_update(struct intel_atomic_state *state);
 void intel_dbuf_mbus_post_ddb_update(struct intel_atomic_state *state);
 void intel_program_dpkgc_latency(struct intel_atomic_state *state);
 
+bool intel_dbuf_pmdemand_needs_update(struct intel_atomic_state *state);
+
 #endif /* __SKL_WATERMARK_H__ */
 
-- 
2.39.5

[PATCH 02/16] drm/i915/wm: add more accessors to dbuf state

[Jani Nikula]

Add intel_dbuf_num_enabled_slices() and intel_dbuf_num_active_pipes()
helpers to avoid looking at struct intel_dbuf_state internals outside of
skl_watermark.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  6 +++---
 drivers/gpu/drm/i915/display/skl_watermark.c  | 10 ++++++++++
 drivers/gpu/drm/i915/display/skl_watermark.h  |  3 +++
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index 0f1501c456df..eeb88f9fc92d 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -358,12 +358,12 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
 
 	if (DISPLAY_VER(display) < 30) {
 		new_pmdemand_state->params.active_dbufs =
-			min_t(u8, hweight8(new_dbuf_state->enabled_slices), 3);
+			min_t(u8, intel_dbuf_num_enabled_slices(new_dbuf_state), 3);
 		new_pmdemand_state->params.active_pipes =
-			min_t(u8, hweight8(new_dbuf_state->active_pipes), 3);
+			min_t(u8, intel_dbuf_num_active_pipes(new_dbuf_state), 3);
 	} else {
 		new_pmdemand_state->params.active_pipes =
-			min_t(u8, hweight8(new_dbuf_state->active_pipes), INTEL_NUM_PIPES(display));
+			min_t(u8, intel_dbuf_num_active_pipes(new_dbuf_state), INTEL_NUM_PIPES(display));
 	}
 
 	new_cdclk_state = intel_atomic_get_cdclk_state(state);
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 55280d16f9f7..f35f2603d543 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -3701,6 +3701,16 @@ void intel_dbuf_post_plane_update(struct intel_atomic_state *state)
 	gen9_dbuf_slices_update(display, new_slices);
 }
 
+int intel_dbuf_num_enabled_slices(const struct intel_dbuf_state *dbuf_state)
+{
+	return hweight8(dbuf_state->enabled_slices);
+}
+
+int intel_dbuf_num_active_pipes(const struct intel_dbuf_state *dbuf_state)
+{
+	return hweight8(dbuf_state->active_pipes);
+}
+
 bool intel_dbuf_pmdemand_needs_update(struct intel_atomic_state *state)
 {
 	struct intel_display *display = to_intel_display(state);
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index 3b9a0b254cff..a1993ded034a 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -78,6 +78,9 @@ struct intel_dbuf_state {
 struct intel_dbuf_state *
 intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 
+int intel_dbuf_num_enabled_slices(const struct intel_dbuf_state *dbuf_state);
+int intel_dbuf_num_active_pipes(const struct intel_dbuf_state *dbuf_state);
+
 int intel_dbuf_init(struct intel_display *display);
 int intel_dbuf_state_set_mdclk_cdclk_ratio(struct intel_atomic_state *state,
 					   int ratio);
-- 
2.39.5

[PATCH 03/16] drm/i915/wm: make struct intel_dbuf_state opaque type

[Jani Nikula]

With all the code touching struct intel_dbuf_state moved inside
skl_watermark.c, we move the struct definition there too, and make the
type opaque. This nicely reduces includes from skl_watermark.h.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 12 ++++++++++++
 drivers/gpu/drm/i915/display/skl_watermark.h | 19 +++----------------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index f35f2603d543..34726895075b 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -38,6 +38,18 @@
  */
 #define DSB_EXE_TIME 100
 
+struct intel_dbuf_state {
+	struct intel_global_state base;
+
+	struct skl_ddb_entry ddb[I915_MAX_PIPES];
+	unsigned int weight[I915_MAX_PIPES];
+	u8 slices[I915_MAX_PIPES];
+	u8 enabled_slices;
+	u8 active_pipes;
+	u8 mdclk_cdclk_ratio;
+	bool joined_mbus;
+};
+
 #define to_intel_dbuf_state(global_state) \
 	container_of_const((global_state), struct intel_dbuf_state, base)
 
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index a1993ded034a..87d052b640b3 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -8,17 +8,16 @@
 
 #include <linux/types.h>
 
-#include "intel_display_limits.h"
-#include "intel_global_state.h"
-#include "intel_wm_types.h"
-
+enum plane_id;
 struct intel_atomic_state;
 struct intel_bw_state;
 struct intel_crtc;
 struct intel_crtc_state;
+struct intel_dbuf_state;
 struct intel_display;
 struct intel_plane;
 struct intel_plane_state;
+struct skl_ddb_entry;
 struct skl_pipe_wm;
 struct skl_wm_level;
 
@@ -63,18 +62,6 @@ unsigned int skl_plane_relative_data_rate(const struct intel_crtc_state *crtc_st
 					  struct intel_plane *plane, int width,
 					  int height, int cpp);
 
-struct intel_dbuf_state {
-	struct intel_global_state base;
-
-	struct skl_ddb_entry ddb[I915_MAX_PIPES];
-	unsigned int weight[I915_MAX_PIPES];
-	u8 slices[I915_MAX_PIPES];
-	u8 enabled_slices;
-	u8 active_pipes;
-	u8 mdclk_cdclk_ratio;
-	bool joined_mbus;
-};
-
 struct intel_dbuf_state *
 intel_atomic_get_dbuf_state(struct intel_atomic_state *state);
 
-- 
2.39.5

[PATCH 04/16] drm/i915/bw: abstract intel_bw_pmdemand_needs_update()

[Jani Nikula]

Add intel_bw_pmdemand_needs_update() helper to avoid looking at struct
intel_bw_state internals outside of intel_bw.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c       | 14 ++++++++++++++
 drivers/gpu/drm/i915/display/intel_bw.h       |  2 ++
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  6 +-----
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 6c2ab2e0dc91..c077ab05eb61 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -1644,3 +1644,17 @@ int intel_bw_init(struct intel_display *display)
 
 	return 0;
 }
+
+bool intel_bw_pmdemand_needs_update(struct intel_atomic_state *state)
+{
+	const struct intel_bw_state *new_bw_state, *old_bw_state;
+
+	new_bw_state = intel_atomic_get_new_bw_state(state);
+	old_bw_state = intel_atomic_get_old_bw_state(state);
+
+	if (new_bw_state &&
+	    new_bw_state->qgv_point_peakbw != old_bw_state->qgv_point_peakbw)
+		return true;
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index eb2cc883e9c1..0acc6f19c981 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -76,4 +76,6 @@ int intel_bw_min_cdclk(struct intel_display *display,
 void intel_bw_update_hw_state(struct intel_display *display);
 void intel_bw_crtc_disable_noatomic(struct intel_crtc *crtc);
 
+bool intel_bw_pmdemand_needs_update(struct intel_atomic_state *state);
+
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index eeb88f9fc92d..8334744a2e23 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -294,16 +294,12 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
 
 static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 {
-	const struct intel_bw_state *new_bw_state, *old_bw_state;
 	const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
 	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	int i;
 
-	new_bw_state = intel_atomic_get_new_bw_state(state);
-	old_bw_state = intel_atomic_get_old_bw_state(state);
-	if (new_bw_state && new_bw_state->qgv_point_peakbw !=
-	    old_bw_state->qgv_point_peakbw)
+	if (intel_bw_pmdemand_needs_update(state))
 		return true;
 
 	if (intel_dbuf_pmdemand_needs_update(state))
-- 
2.39.5

[PATCH 05/16] drm/i915/bw: relocate intel_can_enable_sagv() and rename to intel_bw_can_enable_sagv()

[Jani Nikula]

Prefer only looking at struct intel_bw_state internals inside
intel_bw.c. To that effect, move intel_can_enable_sagv() there, and
rename to intel_bw_can_enable_sagv() to have consistent naming.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c      | 22 ++++++++++++++------
 drivers/gpu/drm/i915/display/intel_bw.h      |  2 ++
 drivers/gpu/drm/i915/display/skl_watermark.c | 16 +++-----------
 drivers/gpu/drm/i915/display/skl_watermark.h |  3 ---
 4 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index c077ab05eb61..2e801ef313c8 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -994,7 +994,7 @@ static int mtl_find_qgv_points(struct intel_display *display,
 	 * for qgv peak bw in PM Demand request. So assign UINT_MAX if SAGV is
 	 * not enabled. PM Demand code will clamp the value for the register
 	 */
-	if (!intel_can_enable_sagv(display, new_bw_state)) {
+	if (!intel_bw_can_enable_sagv(display, new_bw_state)) {
 		new_bw_state->qgv_point_peakbw = U16_MAX;
 		drm_dbg_kms(display->drm, "No SAGV, use UINT_MAX as peak bw.");
 		return 0;
@@ -1107,7 +1107,7 @@ static int icl_find_qgv_points(struct intel_display *display,
 	 * we can't enable SAGV due to the increased memory latency it may
 	 * cause.
 	 */
-	if (!intel_can_enable_sagv(display, new_bw_state)) {
+	if (!intel_bw_can_enable_sagv(display, new_bw_state)) {
 		qgv_points = icl_max_bw_qgv_point_mask(display, num_active_planes);
 		drm_dbg_kms(display->drm, "No SAGV, using single QGV point mask 0x%x\n",
 			    qgv_points);
@@ -1474,8 +1474,8 @@ static int intel_bw_check_sagv_mask(struct intel_atomic_state *state)
 	if (!new_bw_state)
 		return 0;
 
-	if (intel_can_enable_sagv(display, new_bw_state) !=
-	    intel_can_enable_sagv(display, old_bw_state)) {
+	if (intel_bw_can_enable_sagv(display, new_bw_state) !=
+	    intel_bw_can_enable_sagv(display, old_bw_state)) {
 		ret = intel_atomic_serialize_global_state(&new_bw_state->base);
 		if (ret)
 			return ret;
@@ -1521,8 +1521,8 @@ int intel_bw_atomic_check(struct intel_atomic_state *state, bool any_ms)
 	new_bw_state = intel_atomic_get_new_bw_state(state);
 
 	if (new_bw_state &&
-	    intel_can_enable_sagv(display, old_bw_state) !=
-	    intel_can_enable_sagv(display, new_bw_state))
+	    intel_bw_can_enable_sagv(display, old_bw_state) !=
+	    intel_bw_can_enable_sagv(display, new_bw_state))
 		changed = true;
 
 	/*
@@ -1658,3 +1658,13 @@ bool intel_bw_pmdemand_needs_update(struct intel_atomic_state *state)
 
 	return false;
 }
+
+bool intel_bw_can_enable_sagv(struct intel_display *display,
+			      const struct intel_bw_state *bw_state)
+{
+	if (DISPLAY_VER(display) < 11 &&
+	    bw_state->active_pipes && !is_power_of_2(bw_state->active_pipes))
+		return false;
+
+	return bw_state->pipe_sagv_reject == 0;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index 0acc6f19c981..ee6e4a7ac89d 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -77,5 +77,7 @@ void intel_bw_update_hw_state(struct intel_display *display);
 void intel_bw_crtc_disable_noatomic(struct intel_crtc *crtc);
 
 bool intel_bw_pmdemand_needs_update(struct intel_atomic_state *state);
+bool intel_bw_can_enable_sagv(struct intel_display *display,
+			      const struct intel_bw_state *bw_state);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 34726895075b..ec2838d641fb 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -252,7 +252,7 @@ static void skl_sagv_pre_plane_update(struct intel_atomic_state *state)
 	if (!new_bw_state)
 		return;
 
-	if (!intel_can_enable_sagv(display, new_bw_state))
+	if (!intel_bw_can_enable_sagv(display, new_bw_state))
 		skl_sagv_disable(display);
 }
 
@@ -265,7 +265,7 @@ static void skl_sagv_post_plane_update(struct intel_atomic_state *state)
 	if (!new_bw_state)
 		return;
 
-	if (intel_can_enable_sagv(display, new_bw_state))
+	if (intel_bw_can_enable_sagv(display, new_bw_state))
 		skl_sagv_enable(display);
 }
 
@@ -466,16 +466,6 @@ bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state)
 		return skl_crtc_can_enable_sagv(crtc_state);
 }
 
-bool intel_can_enable_sagv(struct intel_display *display,
-			   const struct intel_bw_state *bw_state)
-{
-	if (DISPLAY_VER(display) < 11 &&
-	    bw_state->active_pipes && !is_power_of_2(bw_state->active_pipes))
-		return false;
-
-	return bw_state->pipe_sagv_reject == 0;
-}
-
 static u16 skl_ddb_entry_init(struct skl_ddb_entry *entry,
 			      u16 start, u16 end)
 {
@@ -3031,7 +3021,7 @@ skl_compute_wm(struct intel_atomic_state *state)
 		 * drm_atomic_check_only() gets upset if we pull more crtcs
 		 * into the state, so we have to calculate this based on the
 		 * individual intel_crtc_can_enable_sagv() rather than
-		 * the overall intel_can_enable_sagv(). Otherwise the
+		 * the overall intel_bw_can_enable_sagv(). Otherwise the
 		 * crtcs not included in the commit would not switch to the
 		 * SAGV watermarks when we are about to enable SAGV, and that
 		 * would lead to underruns. This does mean extra power draw
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.h b/drivers/gpu/drm/i915/display/skl_watermark.h
index 87d052b640b3..62790816f030 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.h
+++ b/drivers/gpu/drm/i915/display/skl_watermark.h
@@ -10,7 +10,6 @@
 
 enum plane_id;
 struct intel_atomic_state;
-struct intel_bw_state;
 struct intel_crtc;
 struct intel_crtc_state;
 struct intel_dbuf_state;
@@ -26,8 +25,6 @@ u8 intel_enabled_dbuf_slices_mask(struct intel_display *display);
 void intel_sagv_pre_plane_update(struct intel_atomic_state *state);
 void intel_sagv_post_plane_update(struct intel_atomic_state *state);
 bool intel_crtc_can_enable_sagv(const struct intel_crtc_state *crtc_state);
-bool intel_can_enable_sagv(struct intel_display *display,
-			   const struct intel_bw_state *bw_state);
 bool intel_has_sagv(struct intel_display *display);
 
 u32 skl_ddb_dbuf_slice_mask(struct intel_display *display,
-- 
2.39.5

[PATCH 06/16] drm/i915: move icl_sagv_{pre, post}_plane_update() to intel_bw.c

[Jani Nikula]

Prefer only looking at struct intel_bw_state internals inside
intel_bw.c. To that effect, move icl_sagv_{pre,post}_plane_update()
there.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c      | 68 +++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_bw.h      |  4 +-
 drivers/gpu/drm/i915/display/skl_watermark.c | 64 ------------------
 3 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 2e801ef313c8..05cb1bd65ee0 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -154,8 +154,8 @@ static bool is_sagv_enabled(struct intel_display *display, u16 points_mask)
 			      ICL_PCODE_REQ_QGV_PT_MASK);
 }
 
-int icl_pcode_restrict_qgv_points(struct intel_display *display,
-				  u32 points_mask)
+static int icl_pcode_restrict_qgv_points(struct intel_display *display,
+					 u32 points_mask)
 {
 	struct drm_i915_private *i915 = to_i915(display->drm);
 	int ret;
@@ -974,6 +974,70 @@ static void icl_force_disable_sagv(struct intel_display *display,
 	icl_pcode_restrict_qgv_points(display, bw_state->qgv_points_mask);
 }
 
+void icl_sagv_pre_plane_update(struct intel_atomic_state *state)
+{
+	struct intel_display *display = to_intel_display(state);
+	const struct intel_bw_state *old_bw_state =
+		intel_atomic_get_old_bw_state(state);
+	const struct intel_bw_state *new_bw_state =
+		intel_atomic_get_new_bw_state(state);
+	u16 old_mask, new_mask;
+
+	if (!new_bw_state)
+		return;
+
+	old_mask = old_bw_state->qgv_points_mask;
+	new_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
+
+	if (old_mask == new_mask)
+		return;
+
+	WARN_ON(!new_bw_state->base.changed);
+
+	drm_dbg_kms(display->drm, "Restricting QGV points: 0x%x -> 0x%x\n",
+		    old_mask, new_mask);
+
+	/*
+	 * Restrict required qgv points before updating the configuration.
+	 * According to BSpec we can't mask and unmask qgv points at the same
+	 * time. Also masking should be done before updating the configuration
+	 * and unmasking afterwards.
+	 */
+	icl_pcode_restrict_qgv_points(display, new_mask);
+}
+
+void icl_sagv_post_plane_update(struct intel_atomic_state *state)
+{
+	struct intel_display *display = to_intel_display(state);
+	const struct intel_bw_state *old_bw_state =
+		intel_atomic_get_old_bw_state(state);
+	const struct intel_bw_state *new_bw_state =
+		intel_atomic_get_new_bw_state(state);
+	u16 old_mask, new_mask;
+
+	if (!new_bw_state)
+		return;
+
+	old_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
+	new_mask = new_bw_state->qgv_points_mask;
+
+	if (old_mask == new_mask)
+		return;
+
+	WARN_ON(!new_bw_state->base.changed);
+
+	drm_dbg_kms(display->drm, "Relaxing QGV points: 0x%x -> 0x%x\n",
+		    old_mask, new_mask);
+
+	/*
+	 * Allow required qgv points after updating the configuration.
+	 * According to BSpec we can't mask and unmask qgv points at the same
+	 * time. Also masking should be done before updating the configuration
+	 * and unmasking afterwards.
+	 */
+	icl_pcode_restrict_qgv_points(display, new_mask);
+}
+
 static int mtl_find_qgv_points(struct intel_display *display,
 			       unsigned int data_rate,
 			       unsigned int num_active_planes,
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index ee6e4a7ac89d..68b95c2a0cb9 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -67,8 +67,6 @@ intel_atomic_get_bw_state(struct intel_atomic_state *state);
 void intel_bw_init_hw(struct intel_display *display);
 int intel_bw_init(struct intel_display *display);
 int intel_bw_atomic_check(struct intel_atomic_state *state, bool any_ms);
-int icl_pcode_restrict_qgv_points(struct intel_display *display,
-				  u32 points_mask);
 int intel_bw_calc_min_cdclk(struct intel_atomic_state *state,
 			    bool *need_cdclk_calc);
 int intel_bw_min_cdclk(struct intel_display *display,
@@ -79,5 +77,7 @@ void intel_bw_crtc_disable_noatomic(struct intel_crtc *crtc);
 bool intel_bw_pmdemand_needs_update(struct intel_atomic_state *state);
 bool intel_bw_can_enable_sagv(struct intel_display *display,
 			      const struct intel_bw_state *bw_state);
+void icl_sagv_pre_plane_update(struct intel_atomic_state *state);
+void icl_sagv_post_plane_update(struct intel_atomic_state *state);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index ec2838d641fb..95515d69ad68 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -269,70 +269,6 @@ static void skl_sagv_post_plane_update(struct intel_atomic_state *state)
 		skl_sagv_enable(display);
 }
 
-static void icl_sagv_pre_plane_update(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	const struct intel_bw_state *old_bw_state =
-		intel_atomic_get_old_bw_state(state);
-	const struct intel_bw_state *new_bw_state =
-		intel_atomic_get_new_bw_state(state);
-	u16 old_mask, new_mask;
-
-	if (!new_bw_state)
-		return;
-
-	old_mask = old_bw_state->qgv_points_mask;
-	new_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
-
-	if (old_mask == new_mask)
-		return;
-
-	WARN_ON(!new_bw_state->base.changed);
-
-	drm_dbg_kms(display->drm, "Restricting QGV points: 0x%x -> 0x%x\n",
-		    old_mask, new_mask);
-
-	/*
-	 * Restrict required qgv points before updating the configuration.
-	 * According to BSpec we can't mask and unmask qgv points at the same
-	 * time. Also masking should be done before updating the configuration
-	 * and unmasking afterwards.
-	 */
-	icl_pcode_restrict_qgv_points(display, new_mask);
-}
-
-static void icl_sagv_post_plane_update(struct intel_atomic_state *state)
-{
-	struct intel_display *display = to_intel_display(state);
-	const struct intel_bw_state *old_bw_state =
-		intel_atomic_get_old_bw_state(state);
-	const struct intel_bw_state *new_bw_state =
-		intel_atomic_get_new_bw_state(state);
-	u16 old_mask, new_mask;
-
-	if (!new_bw_state)
-		return;
-
-	old_mask = old_bw_state->qgv_points_mask | new_bw_state->qgv_points_mask;
-	new_mask = new_bw_state->qgv_points_mask;
-
-	if (old_mask == new_mask)
-		return;
-
-	WARN_ON(!new_bw_state->base.changed);
-
-	drm_dbg_kms(display->drm, "Relaxing QGV points: 0x%x -> 0x%x\n",
-		    old_mask, new_mask);
-
-	/*
-	 * Allow required qgv points after updating the configuration.
-	 * According to BSpec we can't mask and unmask qgv points at the same
-	 * time. Also masking should be done before updating the configuration
-	 * and unmasking afterwards.
-	 */
-	icl_pcode_restrict_qgv_points(display, new_mask);
-}
-
 void intel_sagv_pre_plane_update(struct intel_atomic_state *state)
 {
 	struct intel_display *display = to_intel_display(state);
-- 
2.39.5

[PATCH 07/16] drm/i915/bw: abstract intel_bw_qgv_point_peakbw()

[Jani Nikula]

Add intel_bw_qgv_point_peakbw() helper to avoid looking at struct
intel_bw_state internals outside of intel_bw.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c       | 5 +++++
 drivers/gpu/drm/i915/display/intel_bw.h       | 1 +
 drivers/gpu/drm/i915/display/intel_pmdemand.c | 2 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 05cb1bd65ee0..65718e6b5333 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -1732,3 +1732,8 @@ bool intel_bw_can_enable_sagv(struct intel_display *display,
 
 	return bw_state->pipe_sagv_reject == 0;
 }
+
+int intel_bw_qgv_point_peakbw(const struct intel_bw_state *bw_state)
+{
+	return bw_state->qgv_point_peakbw;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index 68b95c2a0cb9..7728dc86a31a 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -79,5 +79,6 @@ bool intel_bw_can_enable_sagv(struct intel_display *display,
 			      const struct intel_bw_state *bw_state);
 void icl_sagv_pre_plane_update(struct intel_atomic_state *state);
 void icl_sagv_post_plane_update(struct intel_atomic_state *state);
+int intel_bw_qgv_point_peakbw(const struct intel_bw_state *bw_state);
 
 #endif /* __INTEL_BW_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index 8334744a2e23..a4d53fd94489 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -346,7 +346,7 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
 
 	/* firmware will calculate the qclk_gv_index, requirement is set to 0 */
 	new_pmdemand_state->params.qclk_gv_index = 0;
-	new_pmdemand_state->params.qclk_gv_bw = new_bw_state->qgv_point_peakbw;
+	new_pmdemand_state->params.qclk_gv_bw = intel_bw_qgv_point_peakbw(new_bw_state);
 
 	new_dbuf_state = intel_atomic_get_dbuf_state(state);
 	if (IS_ERR(new_dbuf_state))
-- 
2.39.5

[PATCH 08/16] drm/i915/bw: make struct intel_bw_state opaque

[Jani Nikula]

With all the code touching struct intel_bw_state moved inside
intel_bw.c, we move the struct definition there too, and make the type
opaque. to_intel_bw_state() needs to be turned into a proper
function. All of this nicely reduces includes from intel_bw.h.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 40 ++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_bw.h | 44 ++-----------------------
 2 files changed, 43 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 65718e6b5333..67c54c144274 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -20,6 +20,41 @@
 #include "intel_pcode.h"
 #include "skl_watermark.h"
 
+struct intel_dbuf_bw {
+	unsigned int max_bw[I915_MAX_DBUF_SLICES];
+	u8 active_planes[I915_MAX_DBUF_SLICES];
+};
+
+struct intel_bw_state {
+	struct intel_global_state base;
+	struct intel_dbuf_bw dbuf_bw[I915_MAX_PIPES];
+
+	/*
+	 * Contains a bit mask, used to determine, whether correspondent
+	 * pipe allows SAGV or not.
+	 */
+	u8 pipe_sagv_reject;
+
+	/* bitmask of active pipes */
+	u8 active_pipes;
+
+	/*
+	 * From MTL onwards, to lock a QGV point, punit expects the peak BW of
+	 * the selected QGV point as the parameter in multiples of 100MB/s
+	 */
+	u16 qgv_point_peakbw;
+
+	/*
+	 * Current QGV points mask, which restricts
+	 * some particular SAGV states, not to confuse
+	 * with pipe_sagv_mask.
+	 */
+	u16 qgv_points_mask;
+
+	unsigned int data_rate[I915_MAX_PIPES];
+	u8 num_active_planes[I915_MAX_PIPES];
+};
+
 /* Parameters for Qclk Geyserville (QGV) */
 struct intel_qgv_point {
 	u16 dclk, t_rp, t_rdpre, t_rc, t_ras, t_rcd;
@@ -865,6 +900,11 @@ static unsigned int intel_bw_data_rate(struct intel_display *display,
 	return data_rate;
 }
 
+struct intel_bw_state *to_intel_bw_state(struct intel_global_state *obj_state)
+{
+	return container_of(obj_state, struct intel_bw_state, base);
+}
+
 struct intel_bw_state *
 intel_atomic_get_old_bw_state(struct intel_atomic_state *state)
 {
diff --git a/drivers/gpu/drm/i915/display/intel_bw.h b/drivers/gpu/drm/i915/display/intel_bw.h
index 7728dc86a31a..d51f50c9d302 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.h
+++ b/drivers/gpu/drm/i915/display/intel_bw.h
@@ -8,52 +8,14 @@
 
 #include <drm/drm_atomic.h>
 
-#include "intel_display_limits.h"
-#include "intel_display_power.h"
-#include "intel_global_state.h"
-
 struct intel_atomic_state;
+struct intel_bw_state;
 struct intel_crtc;
 struct intel_crtc_state;
 struct intel_display;
+struct intel_global_state;
 
-struct intel_dbuf_bw {
-	unsigned int max_bw[I915_MAX_DBUF_SLICES];
-	u8 active_planes[I915_MAX_DBUF_SLICES];
-};
-
-struct intel_bw_state {
-	struct intel_global_state base;
-	struct intel_dbuf_bw dbuf_bw[I915_MAX_PIPES];
-
-	/*
-	 * Contains a bit mask, used to determine, whether correspondent
-	 * pipe allows SAGV or not.
-	 */
-	u8 pipe_sagv_reject;
-
-	/* bitmask of active pipes */
-	u8 active_pipes;
-
-	/*
-	 * From MTL onwards, to lock a QGV point, punit expects the peak BW of
-	 * the selected QGV point as the parameter in multiples of 100MB/s
-	 */
-	u16 qgv_point_peakbw;
-
-	/*
-	 * Current QGV points mask, which restricts
-	 * some particular SAGV states, not to confuse
-	 * with pipe_sagv_mask.
-	 */
-	u16 qgv_points_mask;
-
-	unsigned int data_rate[I915_MAX_PIPES];
-	u8 num_active_planes[I915_MAX_PIPES];
-};
-
-#define to_intel_bw_state(global_state) \
-	container_of_const((global_state), struct intel_bw_state, base)
+struct intel_bw_state *to_intel_bw_state(struct intel_global_state *obj_state);
 
 struct intel_bw_state *
 intel_atomic_get_old_bw_state(struct intel_atomic_state *state);
-- 
2.39.5

[PATCH 09/16] drm/i915/cdclk: abstract intel_cdclk_logical()

[Jani Nikula]

Add intel_cdclk_logical() helper to avoid looking at struct
intel_cdclk_state internals outside of intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/hsw_ips.c       | 2 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c   | 5 +++++
 drivers/gpu/drm/i915/display/intel_cdclk.h   | 2 ++
 drivers/gpu/drm/i915/display/intel_display.c | 2 +-
 drivers/gpu/drm/i915/display/intel_fbc.c     | 2 +-
 drivers/gpu/drm/i915/display/skl_watermark.c | 2 +-
 6 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
index 0d33782f11be..35a1e17ec82b 100644
--- a/drivers/gpu/drm/i915/display/hsw_ips.c
+++ b/drivers/gpu/drm/i915/display/hsw_ips.c
@@ -268,7 +268,7 @@ int hsw_ips_compute_config(struct intel_atomic_state *state,
 			return PTR_ERR(cdclk_state);
 
 		/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
-		if (crtc_state->pixel_rate > cdclk_state->logical.cdclk * 95 / 100)
+		if (crtc_state->pixel_rate > intel_cdclk_logical(cdclk_state) * 95 / 100)
 			return 0;
 	}
 
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 38b3094b37d7..5082d2b64ce5 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3837,3 +3837,8 @@ void intel_init_cdclk_hooks(struct intel_display *display)
 		     "Unknown platform. Assuming i830\n"))
 		display->funcs.cdclk = &i830_cdclk_funcs;
 }
+
+int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state)
+{
+	return 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 a1cefd455d92..20a66f613072 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -97,4 +97,6 @@ void intel_cdclk_crtc_disable_noatomic(struct intel_crtc *crtc);
 int intel_cdclk_init(struct intel_display *display);
 void intel_cdclk_debugfs_register(struct intel_display *display);
 
+int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state);
+
 #endif /* __INTEL_CDCLK_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b0c7c46ffbe2..aa01e48b23f5 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4160,7 +4160,7 @@ static u16 hsw_ips_linetime_wm(const struct intel_crtc_state *crtc_state,
 		return 0;
 
 	linetime_wm = DIV_ROUND_CLOSEST(pipe_mode->crtc_htotal * 1000 * 8,
-					cdclk_state->logical.cdclk);
+					intel_cdclk_logical(cdclk_state));
 
 	return min(linetime_wm, 0x1ff);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index ec1ef8694c35..5d28a6062db1 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1576,7 +1576,7 @@ static int intel_fbc_check_plane(struct intel_atomic_state *state,
 		if (IS_ERR(cdclk_state))
 			return PTR_ERR(cdclk_state);
 
-		if (crtc_state->pixel_rate >= cdclk_state->logical.cdclk * 95 / 100) {
+		if (crtc_state->pixel_rate >= intel_cdclk_logical(cdclk_state) * 95 / 100) {
 			plane_state->no_fbc_reason = "pixel rate too high";
 			return 0;
 		}
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 95515d69ad68..e1e23247d2be 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -2182,7 +2182,7 @@ cdclk_prefill_adjustment(const struct intel_crtc_state *crtc_state)
 	}
 
 	return min(1, DIV_ROUND_UP(crtc_state->pixel_rate,
-				   2 * cdclk_state->logical.cdclk));
+				   2 * intel_cdclk_logical(cdclk_state)));
 }
 
 static int
-- 
2.39.5

[PATCH 10/16] drm/i915/cdclk: abstract intel_cdclk_min_cdclk()

[Jani Nikula]

Add intel_cdclk_min_cdclk() helper to avoid looking at struct
intel_cdclk_state internals outside of intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic_plane.c | 4 ++--
 drivers/gpu/drm/i915/display/intel_cdclk.c        | 5 +++++
 drivers/gpu/drm/i915/display/intel_cdclk.h        | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 15ede7678636..1e9b906c3e44 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -333,7 +333,7 @@ int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
 	 * display blinking due to constant cdclk changes.
 	 */
 	if (new_crtc_state->min_cdclk[plane->id] <=
-	    cdclk_state->min_cdclk[crtc->pipe])
+	    intel_cdclk_min_cdclk(cdclk_state, crtc->pipe))
 		return 0;
 
 	drm_dbg_kms(display->drm,
@@ -341,7 +341,7 @@ int intel_plane_calc_min_cdclk(struct intel_atomic_state *state,
 		    plane->base.base.id, plane->base.name,
 		    new_crtc_state->min_cdclk[plane->id],
 		    crtc->base.base.id, crtc->base.name,
-		    cdclk_state->min_cdclk[crtc->pipe]);
+		    intel_cdclk_min_cdclk(cdclk_state, crtc->pipe));
 	*need_cdclk_calc = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 5082d2b64ce5..05e94fcd8326 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3842,3 +3842,8 @@ int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state)
 {
 	return cdclk_state->logical.cdclk;
 }
+
+int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe pipe)
+{
+	return cdclk_state->min_cdclk[pipe];
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 20a66f613072..ef6ad9d04c20 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -98,5 +98,6 @@ int intel_cdclk_init(struct intel_display *display);
 void intel_cdclk_debugfs_register(struct intel_display *display);
 
 int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state);
+int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe pipe);
 
 #endif /* __INTEL_CDCLK_H__ */
-- 
2.39.5

[PATCH 11/16] drm/i915/cdclk: abstract intel_cdclk_bw_min_cdclk()

[Jani Nikula]

Add intel_cdclk_bw_min_cdclk() helper to avoid looking at struct
intel_cdclk_state internals outside of intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c    | 4 ++--
 drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_cdclk.h | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 67c54c144274..306a13ef8b35 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -1461,12 +1461,12 @@ int intel_bw_calc_min_cdclk(struct intel_atomic_state *state,
 	 * requirements. This can reduce back and forth
 	 * display blinking due to constant cdclk changes.
 	 */
-	if (new_min_cdclk <= cdclk_state->bw_min_cdclk)
+	if (new_min_cdclk <= intel_cdclk_bw_min_cdclk(cdclk_state))
 		return 0;
 
 	drm_dbg_kms(display->drm,
 		    "new bandwidth min cdclk (%d kHz) > old min cdclk (%d kHz)\n",
-		    new_min_cdclk, cdclk_state->bw_min_cdclk);
+		    new_min_cdclk, intel_cdclk_bw_min_cdclk(cdclk_state));
 	*need_cdclk_calc = true;
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 05e94fcd8326..59d126e1b12a 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3847,3 +3847,8 @@ int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe
 {
 	return cdclk_state->min_cdclk[pipe];
 }
+
+int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state)
+{
+	return cdclk_state->bw_min_cdclk;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index ef6ad9d04c20..fe1a1f1c1900 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -99,5 +99,6 @@ void intel_cdclk_debugfs_register(struct intel_display *display);
 
 int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state);
 int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe pipe);
+int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
 
 #endif /* __INTEL_CDCLK_H__ */
-- 
2.39.5

[PATCH 12/16] drm/i915/cdclk: abstract intel_cdclk_pmdemand_needs_update()

[Jani Nikula]

Add intel_cdclk_pmdemand_needs_update() helper to avoid looking at
struct intel_cdclk_state internals outside of intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 15 +++++++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  1 +
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  9 +--------
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 59d126e1b12a..ed6c407f66c7 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3852,3 +3852,18 @@ int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state)
 {
 	return cdclk_state->bw_min_cdclk;
 }
+
+bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state)
+{
+	const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
+
+	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
+	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
+
+	if (new_cdclk_state &&
+	    (new_cdclk_state->actual.cdclk != old_cdclk_state->actual.cdclk ||
+	     new_cdclk_state->actual.voltage_level != old_cdclk_state->actual.voltage_level))
+		return true;
+
+	return false;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index fe1a1f1c1900..8527a6e44ee5 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -100,5 +100,6 @@ void intel_cdclk_debugfs_register(struct intel_display *display);
 int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state);
 int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe pipe);
 int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
+bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
 
 #endif /* __INTEL_CDCLK_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index a4d53fd94489..16ef68ef4041 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -294,7 +294,6 @@ intel_pmdemand_connector_needs_update(struct intel_atomic_state *state)
 
 static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 {
-	const struct intel_cdclk_state *new_cdclk_state, *old_cdclk_state;
 	const struct intel_crtc_state *new_crtc_state, *old_crtc_state;
 	struct intel_crtc *crtc;
 	int i;
@@ -305,13 +304,7 @@ static bool intel_pmdemand_needs_update(struct intel_atomic_state *state)
 	if (intel_dbuf_pmdemand_needs_update(state))
 		return true;
 
-	new_cdclk_state = intel_atomic_get_new_cdclk_state(state);
-	old_cdclk_state = intel_atomic_get_old_cdclk_state(state);
-	if (new_cdclk_state &&
-	    (new_cdclk_state->actual.cdclk !=
-	     old_cdclk_state->actual.cdclk ||
-	     new_cdclk_state->actual.voltage_level !=
-	     old_cdclk_state->actual.voltage_level))
+	if (intel_cdclk_pmdemand_needs_update(state))
 		return true;
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
-- 
2.39.5

[PATCH 13/16] drm/i915/cdclk: abstract intel_cdclk_force_min_cdclk()

[Jani Nikula]

Add intel_cdclk_force_min_cdclk() helper to avoid modifying struct
intel_cdclk_state internals outside of intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_audio.c | 2 +-
 drivers/gpu/drm/i915/display/intel_cdclk.c | 5 +++++
 drivers/gpu/drm/i915/display/intel_cdclk.h | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_audio.c b/drivers/gpu/drm/i915/display/intel_audio.c
index 55af3a553c58..5bdaef38f13d 100644
--- a/drivers/gpu/drm/i915/display/intel_audio.c
+++ b/drivers/gpu/drm/i915/display/intel_audio.c
@@ -951,7 +951,7 @@ static int glk_force_audio_cdclk_commit(struct intel_atomic_state *state,
 	if (IS_ERR(cdclk_state))
 		return PTR_ERR(cdclk_state);
 
-	cdclk_state->force_min_cdclk = enable ? 2 * 96000 : 0;
+	intel_cdclk_force_min_cdclk(cdclk_state, enable ? 2 * 96000 : 0);
 
 	return drm_atomic_commit(&state->base);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index ed6c407f66c7..f63b6b3b5476 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3867,3 +3867,8 @@ bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state)
 
 	return false;
 }
+
+void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk)
+{
+	cdclk_state->force_min_cdclk = force_min_cdclk;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 8527a6e44ee5..ff10ed526bd4 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -101,5 +101,6 @@ int intel_cdclk_logical(const struct intel_cdclk_state *cdclk_state);
 int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe pipe);
 int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
 bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
+void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
 
 #endif /* __INTEL_CDCLK_H__ */
-- 
2.39.5

[PATCH 14/16] drm/i915/cdclk: abstract intel_cdclk_read_hw()

[Jani Nikula]

Add intel_cdclk_read_hw() function to avoid looking at struct
intel_cdclk_state internals outside of intel_cdclk.c.

intel_cdclk_init_hw() would be a better name, but we already have that.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c          | 12 ++++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h          |  1 +
 drivers/gpu/drm/i915/display/intel_display_driver.c |  8 +-------
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index f63b6b3b5476..994be1d0e20c 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3872,3 +3872,15 @@ void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int forc
 {
 	cdclk_state->force_min_cdclk = force_min_cdclk;
 }
+
+void intel_cdclk_read_hw(struct intel_display *display)
+{
+	struct intel_cdclk_state *cdclk_state;
+
+	cdclk_state = to_intel_cdclk_state(display->cdclk.obj.state);
+
+	intel_update_cdclk(display);
+	intel_cdclk_dump_config(display, &display->cdclk.hw, "Current CDCLK");
+	cdclk_state->actual = display->cdclk.hw;
+	cdclk_state->logical = display->cdclk.hw;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index ff10ed526bd4..0d5ee1826168 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -102,5 +102,6 @@ int intel_cdclk_min_cdclk(const struct intel_cdclk_state *cdclk_state, enum pipe
 int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
 bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
 void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
+void intel_cdclk_read_hw(struct intel_display *display);
 
 #endif /* __INTEL_CDCLK_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c
index ec799a1773e4..9058c23dd487 100644
--- a/drivers/gpu/drm/i915/display/intel_display_driver.c
+++ b/drivers/gpu/drm/i915/display/intel_display_driver.c
@@ -84,16 +84,10 @@ bool intel_display_driver_probe_defer(struct pci_dev *pdev)
 
 void intel_display_driver_init_hw(struct intel_display *display)
 {
-	struct intel_cdclk_state *cdclk_state;
-
 	if (!HAS_DISPLAY(display))
 		return;
 
-	cdclk_state = to_intel_cdclk_state(display->cdclk.obj.state);
-
-	intel_update_cdclk(display);
-	intel_cdclk_dump_config(display, &display->cdclk.hw, "Current CDCLK");
-	cdclk_state->logical = cdclk_state->actual = display->cdclk.hw;
+	intel_cdclk_read_hw(display);
 
 	intel_display_wa_apply(display);
 }
-- 
2.39.5

[PATCH 15/16] drm/i915/cdclk: abstract intel_cdclk_actual() and intel_cdclk_actual_voltage_level()

[Jani Nikula]

Add intel_cdclk_actual() and intel_cdclk_actual_voltage_level() helpers
to avoid looking at struct intel_cdclk_state internals outside of
intel_cdclk.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c    | 10 ++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h    |  2 ++
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  4 ++--
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 994be1d0e20c..2e8abf237bd1 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3884,3 +3884,13 @@ void intel_cdclk_read_hw(struct intel_display *display)
 	cdclk_state->actual = display->cdclk.hw;
 	cdclk_state->logical = display->cdclk.hw;
 }
+
+int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state)
+{
+	return cdclk_state->actual.cdclk;
+}
+
+int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state)
+{
+	return cdclk_state->actual.voltage_level;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index 0d5ee1826168..f38605c6ab72 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -103,5 +103,7 @@ int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
 bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
 void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
 void intel_cdclk_read_hw(struct intel_display *display);
+int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state);
+int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state);
 
 #endif /* __INTEL_CDCLK_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index 16ef68ef4041..d806c15db7ce 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -360,9 +360,9 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
 		return PTR_ERR(new_cdclk_state);
 
 	new_pmdemand_state->params.voltage_index =
-		new_cdclk_state->actual.voltage_level;
+		intel_cdclk_actual_voltage_level(new_cdclk_state);
 	new_pmdemand_state->params.cdclk_freq_mhz =
-		DIV_ROUND_UP(new_cdclk_state->actual.cdclk, 1000);
+		DIV_ROUND_UP(intel_cdclk_actual(new_cdclk_state), 1000);
 
 	intel_pmdemand_update_max_ddiclk(display, state, new_pmdemand_state);
 
-- 
2.39.5

[PATCH 16/16] drm/i915/cdclk: make struct intel_cdclk_state opaque

[Jani Nikula]

With all the code touching struct intel_cdclk_state moved inside
intel_cdclk.c, we move the struct definition there too, and make the
type opaque. This nicely reduces includes from intel_cdclk.h.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c | 36 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_cdclk.h | 41 ++--------------------
 2 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 2e8abf237bd1..ad7a6a53a082 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -114,6 +114,42 @@
  * dividers can be programmed correctly.
  */
 
+struct intel_cdclk_state {
+	struct intel_global_state base;
+
+	/*
+	 * Logical configuration of cdclk (used for all scaling,
+	 * watermark, etc. calculations and checks). This is
+	 * computed as if all enabled crtcs were active.
+	 */
+	struct intel_cdclk_config logical;
+
+	/*
+	 * Actual configuration of cdclk, can be different from the
+	 * logical configuration only when all crtc's are DPMS off.
+	 */
+	struct intel_cdclk_config actual;
+
+	/* minimum acceptable cdclk to satisfy bandwidth requirements */
+	int bw_min_cdclk;
+	/* minimum acceptable cdclk for each pipe */
+	int min_cdclk[I915_MAX_PIPES];
+	/* minimum acceptable voltage level for each pipe */
+	u8 min_voltage_level[I915_MAX_PIPES];
+
+	/* pipe to which cd2x update is synchronized */
+	enum pipe pipe;
+
+	/* forced minimum cdclk for glk+ audio w/a */
+	int force_min_cdclk;
+
+	/* bitmask of active pipes */
+	u8 active_pipes;
+
+	/* update cdclk with pipes disabled */
+	bool disable_pipes;
+};
+
 struct intel_cdclk_funcs {
 	void (*get_cdclk)(struct intel_display *display,
 			  struct intel_cdclk_config *cdclk_config);
diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
index f38605c6ab72..4a2821bf6c65 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.h
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
@@ -8,10 +8,9 @@
 
 #include <linux/types.h>
 
-#include "intel_display_limits.h"
-#include "intel_global_state.h"
-
+enum pipe;
 struct intel_atomic_state;
+struct intel_cdclk_state;
 struct intel_crtc;
 struct intel_crtc_state;
 struct intel_display;
@@ -23,42 +22,6 @@ struct intel_cdclk_config {
 	bool joined_mbus;
 };
 
-struct intel_cdclk_state {
-	struct intel_global_state base;
-
-	/*
-	 * Logical configuration of cdclk (used for all scaling,
-	 * watermark, etc. calculations and checks). This is
-	 * computed as if all enabled crtcs were active.
-	 */
-	struct intel_cdclk_config logical;
-
-	/*
-	 * Actual configuration of cdclk, can be different from the
-	 * logical configuration only when all crtc's are DPMS off.
-	 */
-	struct intel_cdclk_config actual;
-
-	/* minimum acceptable cdclk to satisfy bandwidth requirements */
-	int bw_min_cdclk;
-	/* minimum acceptable cdclk for each pipe */
-	int min_cdclk[I915_MAX_PIPES];
-	/* minimum acceptable voltage level for each pipe */
-	u8 min_voltage_level[I915_MAX_PIPES];
-
-	/* pipe to which cd2x update is synchronized */
-	enum pipe pipe;
-
-	/* forced minimum cdclk for glk+ audio w/a */
-	int force_min_cdclk;
-
-	/* bitmask of active pipes */
-	u8 active_pipes;
-
-	/* update cdclk with pipes disabled */
-	bool disable_pipes;
-};
-
 void intel_cdclk_init_hw(struct intel_display *display);
 void intel_cdclk_uninit_hw(struct intel_display *display);
 void intel_init_cdclk_hooks(struct intel_display *display);
-- 
2.39.5

✗ i915.CI.BAT: failure for drm/i915/display: make all global state opaque

[Patchwork]

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

== Series Details ==

Series: drm/i915/display: make all global state opaque
URL   : https://patchwork.freedesktop.org/series/150158/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_16692 -> Patchwork_150158v1
====================================================

Summary
-------

  **FAILURE**

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

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

  Additional (1): bat-arlh-2 
  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_fence@basic-await@bcs0:
    - bat-twl-1:          [PASS][1] -> [FAIL][2] +1 other test fail
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16692/bat-twl-1/igt@gem_exec_fence@basic-await@bcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-twl-1/igt@gem_exec_fence@basic-await@bcs0.html

  * igt@prime_self_import@basic-with_one_bo:
    - bat-arlh-2:         NOTRUN -> [ABORT][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@prime_self_import@basic-with_one_bo.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@fbdev@eof:
    - bat-arlh-2:         NOTRUN -> [SKIP][4] ([i915#11345] / [i915#11346]) +3 other tests skip
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@fbdev@eof.html

  * igt@fbdev@info:
    - bat-arlh-2:         NOTRUN -> [SKIP][5] ([i915#11346] / [i915#1849])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@fbdev@info.html

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

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

  * igt@gem_tiled_fence_blits@basic:
    - bat-arlh-2:         NOTRUN -> [SKIP][8] ([i915#11346] / [i915#12637]) +2 other tests skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@gem_tiled_fence_blits@basic.html

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

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

  * igt@intel_hwmon@hwmon-read:
    - bat-arlh-2:         NOTRUN -> [SKIP][11] ([i915#11346] / [i915#11680] / [i915#7707]) +1 other test skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@intel_hwmon@hwmon-read.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - bat-arlh-2:         NOTRUN -> [SKIP][12] ([i915#10200] / [i915#11346] / [i915#11666] / [i915#12203])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_addfb_basic@basic-x-tiled-legacy:
    - bat-arlh-2:         NOTRUN -> [SKIP][13] ([i915#10200] / [i915#11346] / [i915#11666]) +8 other tests skip
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@kms_addfb_basic@basic-x-tiled-legacy.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence:
    - bat-arlh-2:         NOTRUN -> [SKIP][14] ([i915#11190] / [i915#11346]) +16 other tests skip
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@kms_pipe_crc_basic@read-crc-frame-sequence.html

  * igt@kms_pm_backlight@basic-brightness:
    - bat-arlh-2:         NOTRUN -> [SKIP][15] ([i915#11346]) +14 other tests skip
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_150158v1/bat-arlh-2/igt@kms_pm_backlight@basic-brightness.html

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

  
  [i915#10197]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10197
  [i915#10200]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10200
  [i915#10206]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10206
  [i915#10208]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10208
  [i915#10209]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10209
  [i915#10211]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10211
  [i915#11190]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11190
  [i915#11343]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11343
  [i915#11345]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11345
  [i915#11346]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11346
  [i915#11666]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11666
  [i915#11680]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11680
  [i915#11681]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11681
  [i915#11724]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11724
  [i915#11725]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11725
  [i915#12203]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12203
  [i915#12637]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12637
  [i915#1849]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1849
  [i915#4079]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/4079
  [i915#7707]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/7707
  [i915#8809]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/8809


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

  * Linux: CI_DRM_16692 -> Patchwork_150158v1

  CI-20190529: 20190529
  CI_DRM_16692: b5ef50f5d944d569d6eda26728bffb78c4ef6fa2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8406: 12d7c99650c85e479571b6db2c392408be474c88 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_150158v1: b5ef50f5d944d569d6eda26728bffb78c4ef6fa2 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

Re: [PATCH 00/16] drm/i915/display: make all global state opaque

[Imre Deak]

On Thu, Jun 12, 2025 at 03:11:55PM +0300, Jani Nikula wrote:
> Hide all the structs that "derive" from struct intel_global_state inside
> their respective implementation files.

On the patchset:
Reviewed-by: Imre Deak <imre.deak@intel.com>

A nit later about naming of functions.

> Jani Nikula (16):
>   drm/i915/wm: abstract intel_dbuf_pmdemand_needs_update()
>   drm/i915/wm: add more accessors to dbuf state
>   drm/i915/wm: make struct intel_dbuf_state opaque type
>   drm/i915/bw: abstract intel_bw_pmdemand_needs_update()
>   drm/i915/bw: relocate intel_can_enable_sagv() and rename to
>     intel_bw_can_enable_sagv()
>   drm/i915: move icl_sagv_{pre,post}_plane_update() to intel_bw.c
>   drm/i915/bw: abstract intel_bw_qgv_point_peakbw()
>   drm/i915/bw: make struct intel_bw_state opaque
>   drm/i915/cdclk: abstract intel_cdclk_logical()
>   drm/i915/cdclk: abstract intel_cdclk_min_cdclk()
>   drm/i915/cdclk: abstract intel_cdclk_bw_min_cdclk()
>   drm/i915/cdclk: abstract intel_cdclk_pmdemand_needs_update()
>   drm/i915/cdclk: abstract intel_cdclk_force_min_cdclk()
>   drm/i915/cdclk: abstract intel_cdclk_read_hw()
>   drm/i915/cdclk: abstract intel_cdclk_actual() and
>     intel_cdclk_actual_voltage_level()
>   drm/i915/cdclk: make struct intel_cdclk_state opaque
> 
>  drivers/gpu/drm/i915/display/hsw_ips.c        |   2 +-
>  .../gpu/drm/i915/display/intel_atomic_plane.c |   4 +-
>  drivers/gpu/drm/i915/display/intel_audio.c    |   2 +-
>  drivers/gpu/drm/i915/display/intel_bw.c       | 153 ++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_bw.h       |  53 ++----
>  drivers/gpu/drm/i915/display/intel_cdclk.c    |  93 +++++++++++
>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  50 ++----
>  drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
>  .../drm/i915/display/intel_display_driver.c   |   8 +-
>  drivers/gpu/drm/i915/display/intel_fbc.c      |   2 +-
>  drivers/gpu/drm/i915/display/intel_pmdemand.c |  41 ++---
>  drivers/gpu/drm/i915/display/skl_watermark.c  | 134 +++++++--------
>  drivers/gpu/drm/i915/display/skl_watermark.h  |  33 +---
>  13 files changed, 336 insertions(+), 241 deletions(-)
> 
> -- 
> 2.39.5
> 

Re: [PATCH 15/16] drm/i915/cdclk: abstract intel_cdclk_actual() and intel_cdclk_actual_voltage_level()

[Imre Deak]

On Thu, Jun 12, 2025 at 03:12:10PM +0300, Jani Nikula wrote:
> Add intel_cdclk_actual() and intel_cdclk_actual_voltage_level() helpers
> to avoid looking at struct intel_cdclk_state internals outside of
> intel_cdclk.c.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 10 ++++++++++
>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_pmdemand.c |  4 ++--
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 994be1d0e20c..2e8abf237bd1 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -3884,3 +3884,13 @@ void intel_cdclk_read_hw(struct intel_display *display)
>  	cdclk_state->actual = display->cdclk.hw;
>  	cdclk_state->logical = display->cdclk.hw;
>  }
> +
> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state)
> +{
> +	return cdclk_state->actual.cdclk;
> +}
> +
> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state)
> +{
> +	return cdclk_state->actual.voltage_level;
> +}

These could've been grouped better after intel_cdclk_logical().

I wondered if it'd make sense to use
intel_cdclk_{logical,actual}_cdclk() instead of 
intel_cdclk_{logical,actual}().

Or *_clock() instead of *_cdclk() in the above and other helpers.

> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> index 0d5ee1826168..f38605c6ab72 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> @@ -103,5 +103,7 @@ int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
>  bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
>  void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
>  void intel_cdclk_read_hw(struct intel_display *display);
> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state);
> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state);
>  
>  #endif /* __INTEL_CDCLK_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> index 16ef68ef4041..d806c15db7ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> @@ -360,9 +360,9 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
>  		return PTR_ERR(new_cdclk_state);
>  
>  	new_pmdemand_state->params.voltage_index =
> -		new_cdclk_state->actual.voltage_level;
> +		intel_cdclk_actual_voltage_level(new_cdclk_state);
>  	new_pmdemand_state->params.cdclk_freq_mhz =
> -		DIV_ROUND_UP(new_cdclk_state->actual.cdclk, 1000);
> +		DIV_ROUND_UP(intel_cdclk_actual(new_cdclk_state), 1000);
>  
>  	intel_pmdemand_update_max_ddiclk(display, state, new_pmdemand_state);
>  
> -- 
> 2.39.5
> 

Re: [PATCH 15/16] drm/i915/cdclk: abstract intel_cdclk_actual() and intel_cdclk_actual_voltage_level()

[Jani Nikula]

On Wed, 18 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> On Thu, Jun 12, 2025 at 03:12:10PM +0300, Jani Nikula wrote:
>> Add intel_cdclk_actual() and intel_cdclk_actual_voltage_level() helpers
>> to avoid looking at struct intel_cdclk_state internals outside of
>> intel_cdclk.c.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 10 ++++++++++
>>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  2 ++
>>  drivers/gpu/drm/i915/display/intel_pmdemand.c |  4 ++--
>>  3 files changed, 14 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> index 994be1d0e20c..2e8abf237bd1 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
>> @@ -3884,3 +3884,13 @@ void intel_cdclk_read_hw(struct intel_display *display)
>>  	cdclk_state->actual = display->cdclk.hw;
>>  	cdclk_state->logical = display->cdclk.hw;
>>  }
>> +
>> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state)
>> +{
>> +	return cdclk_state->actual.cdclk;
>> +}
>> +
>> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state)
>> +{
>> +	return cdclk_state->actual.voltage_level;
>> +}
>
> These could've been grouped better after intel_cdclk_logical().

Yes, changing that.

> I wondered if it'd make sense to use
> intel_cdclk_{logical,actual}_cdclk() instead of 
> intel_cdclk_{logical,actual}().

Mmh. I dislike the repetition, "cdclk logical cdclk"...

> Or *_clock() instead of *_cdclk() in the above and other helpers.

...so I set out to consistently use "clock", but then it didn't feel
right for things like "intel_cdclk_min_cdclk" because it's then compared
against min_cdclk in a number of places.

I don't know, leave it as it is now in the patches?

BR,
Jani.



>
>> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> index 0d5ee1826168..f38605c6ab72 100644
>> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
>> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
>> @@ -103,5 +103,7 @@ int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
>>  bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
>>  void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
>>  void intel_cdclk_read_hw(struct intel_display *display);
>> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state);
>> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state);
>>  
>>  #endif /* __INTEL_CDCLK_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> index 16ef68ef4041..d806c15db7ce 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
>> @@ -360,9 +360,9 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
>>  		return PTR_ERR(new_cdclk_state);
>>  
>>  	new_pmdemand_state->params.voltage_index =
>> -		new_cdclk_state->actual.voltage_level;
>> +		intel_cdclk_actual_voltage_level(new_cdclk_state);
>>  	new_pmdemand_state->params.cdclk_freq_mhz =
>> -		DIV_ROUND_UP(new_cdclk_state->actual.cdclk, 1000);
>> +		DIV_ROUND_UP(intel_cdclk_actual(new_cdclk_state), 1000);
>>  
>>  	intel_pmdemand_update_max_ddiclk(display, state, new_pmdemand_state);
>>  
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel

Re: [PATCH 15/16] drm/i915/cdclk: abstract intel_cdclk_actual() and intel_cdclk_actual_voltage_level()

[Imre Deak]

On Thu, Jun 19, 2025 at 01:11:32PM +0300, Jani Nikula wrote:
> On Wed, 18 Jun 2025, Imre Deak <imre.deak@intel.com> wrote:
> > On Thu, Jun 12, 2025 at 03:12:10PM +0300, Jani Nikula wrote:
> >> Add intel_cdclk_actual() and intel_cdclk_actual_voltage_level() helpers
> >> to avoid looking at struct intel_cdclk_state internals outside of
> >> intel_cdclk.c.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_cdclk.c    | 10 ++++++++++
> >>  drivers/gpu/drm/i915/display/intel_cdclk.h    |  2 ++
> >>  drivers/gpu/drm/i915/display/intel_pmdemand.c |  4 ++--
> >>  3 files changed, 14 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> index 994be1d0e20c..2e8abf237bd1 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> >> @@ -3884,3 +3884,13 @@ void intel_cdclk_read_hw(struct intel_display *display)
> >>  	cdclk_state->actual = display->cdclk.hw;
> >>  	cdclk_state->logical = display->cdclk.hw;
> >>  }
> >> +
> >> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state)
> >> +{
> >> +	return cdclk_state->actual.cdclk;
> >> +}
> >> +
> >> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state)
> >> +{
> >> +	return cdclk_state->actual.voltage_level;
> >> +}
> >
> > These could've been grouped better after intel_cdclk_logical().
> 
> Yes, changing that.
> 
> > I wondered if it'd make sense to use
> > intel_cdclk_{logical,actual}_cdclk() instead of 
> > intel_cdclk_{logical,actual}().
> 
> Mmh. I dislike the repetition, "cdclk logical cdclk"...

Yes, though there's already intel_cdclk_min_cdclk() anyway.

> > Or *_clock() instead of *_cdclk() in the above and other helpers.
> 
> ...so I set out to consistently use "clock", but then it didn't feel
> right for things like "intel_cdclk_min_cdclk" because it's then compared
> against min_cdclk in a number of places.
> 
> I don't know, leave it as it is now in the patches?

I only pointed this out since intel_cdclk_actual() is strange wrt.
intel_cdclk_actual_voltage_level() for instace. But sure, this is not a
big deal.

> 
> BR,
> Jani.
> 
> 
> 
> >
> >> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.h b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> index 0d5ee1826168..f38605c6ab72 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.h
> >> @@ -103,5 +103,7 @@ int intel_cdclk_bw_min_cdclk(const struct intel_cdclk_state *cdclk_state);
> >>  bool intel_cdclk_pmdemand_needs_update(struct intel_atomic_state *state);
> >>  void intel_cdclk_force_min_cdclk(struct intel_cdclk_state *cdclk_state, int force_min_cdclk);
> >>  void intel_cdclk_read_hw(struct intel_display *display);
> >> +int intel_cdclk_actual(const struct intel_cdclk_state *cdclk_state);
> >> +int intel_cdclk_actual_voltage_level(const struct intel_cdclk_state *cdclk_state);
> >>  
> >>  #endif /* __INTEL_CDCLK_H__ */
> >> diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> >> index 16ef68ef4041..d806c15db7ce 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
> >> @@ -360,9 +360,9 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
> >>  		return PTR_ERR(new_cdclk_state);
> >>  
> >>  	new_pmdemand_state->params.voltage_index =
> >> -		new_cdclk_state->actual.voltage_level;
> >> +		intel_cdclk_actual_voltage_level(new_cdclk_state);
> >>  	new_pmdemand_state->params.cdclk_freq_mhz =
> >> -		DIV_ROUND_UP(new_cdclk_state->actual.cdclk, 1000);
> >> +		DIV_ROUND_UP(intel_cdclk_actual(new_cdclk_state), 1000);
> >>  
> >>  	intel_pmdemand_update_max_ddiclk(display, state, new_pmdemand_state);
> >>  
> >> -- 
> >> 2.39.5
> >> 
> 
> -- 
> Jani Nikula, Intel