[Intel-gfx] [PATCH 1/3] drm/i915: unify the x_modeset_calc_cdclk() functions

Paulo Zanoni paulo.r.zanoni at intel.com
Mon Feb 20 20:00:40 UTC 2017


There's a lot of duplicated platform-independent logic in the current
modeset_calc_cdclk() functions. Adding cdclk support for more
platforms will only add more copies of this code.

To solve this problem, in this patch we create a new function called
intel_modeset_calc_cdclk() which unifies the platform-independent
logic, and we also create a new vfunc called calc_cdclk_state(), which
is responsible to contain only the platform-dependent code.

The code is now smaller and support for new platforms should be easier
and not require duplicated platform-independent code.

v2: Previously I had 2 different patches addressing these problems,
but wiht Ville's suggestion I think it makes more sense to keep
everything in a single patch (Ville).

Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   4 +-
 drivers/gpu/drm/i915/intel_cdclk.c   | 187 ++++++++++-------------------------
 drivers/gpu/drm/i915/intel_display.c |   6 +-
 drivers/gpu/drm/i915/intel_drv.h     |   1 +
 4 files changed, 60 insertions(+), 138 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e7046d..f1c35c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -644,7 +644,9 @@ struct drm_i915_display_funcs {
 				    struct intel_crtc_state *cstate);
 	int (*compute_global_watermarks)(struct drm_atomic_state *state);
 	void (*update_wm)(struct intel_crtc *crtc);
-	int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
+	void (*calc_cdclk_state)(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state);
 	/* Returns the active state of the crtc, and if the crtc is active,
 	 * fills out the pipe-config with the hw state. */
 	bool (*get_pipe_config)(struct intel_crtc *,
diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c
index d643c0c..dd6fe25 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -1496,148 +1496,69 @@ static int intel_max_pixel_rate(struct drm_atomic_state *state)
 	return max_pixel_rate;
 }
 
-static int vlv_modeset_calc_cdclk(struct drm_atomic_state *state)
+static void vlv_calc_cdclk_state(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk;
-
-	cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
-
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		cdclk = vlv_calc_cdclk(dev_priv, 0);
-
-		intel_state->cdclk.actual.cdclk = cdclk;
-	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
-	}
-
-	return 0;
+	cdclk_state->cdclk = vlv_calc_cdclk(dev_priv, max_pixclk);
 }
 
-static int bdw_modeset_calc_cdclk(struct drm_atomic_state *state)
+static void bdw_calc_cdclk_state(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	int max_pixclk = intel_max_pixel_rate(state);
-	int cdclk;
-
-	/*
-	 * FIXME should also account for plane ratio
-	 * once 64bpp pixel formats are supported.
-	 */
-	cdclk = bdw_calc_cdclk(max_pixclk);
-
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		cdclk = bdw_calc_cdclk(0);
-
-		intel_state->cdclk.actual.cdclk = cdclk;
-	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
-	}
-
-	return 0;
+	cdclk_state->cdclk = bdw_calc_cdclk(max_pixclk);
 }
 
-static int skl_modeset_calc_cdclk(struct drm_atomic_state *state)
+static void skl_calc_cdclk_state(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state)
 {
-	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	const int max_pixclk = intel_max_pixel_rate(state);
-	int cdclk, vco;
+	if (!cdclk_state->vco)
+		cdclk_state->vco = dev_priv->skl_preferred_vco_freq;
 
-	vco = intel_state->cdclk.logical.vco;
-	if (!vco)
-		vco = dev_priv->skl_preferred_vco_freq;
-
-	/*
-	 * FIXME should also account for plane ratio
-	 * once 64bpp pixel formats are supported.
-	 */
-	cdclk = skl_calc_cdclk(max_pixclk, vco);
-
-	if (cdclk > dev_priv->max_cdclk_freq) {
-		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
-		return -EINVAL;
-	}
-
-	intel_state->cdclk.logical.vco = vco;
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		cdclk = skl_calc_cdclk(0, vco);
+	cdclk_state->cdclk = skl_calc_cdclk(max_pixclk, cdclk_state->vco);
+}
 
-		intel_state->cdclk.actual.vco = vco;
-		intel_state->cdclk.actual.cdclk = cdclk;
-	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
-	}
+static void bxt_calc_cdclk_state(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state)
+{
+	cdclk_state->cdclk = bxt_calc_cdclk(max_pixclk);
+	cdclk_state->vco = bxt_de_pll_vco(dev_priv, cdclk_state->cdclk);
+}
 
-	return 0;
+static void glk_calc_cdclk_state(struct drm_i915_private *dev_priv,
+				 int max_pixclk,
+				 struct intel_cdclk_state *cdclk_state)
+{
+	cdclk_state->cdclk = glk_calc_cdclk(max_pixclk);
+	cdclk_state->vco = glk_de_pll_vco(dev_priv, cdclk_state->cdclk);
 }
 
-static int bxt_modeset_calc_cdclk(struct drm_atomic_state *state)
+int intel_modeset_calc_cdclk(struct intel_atomic_state *state)
 {
-	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	int max_pixclk = intel_max_pixel_rate(state);
-	struct intel_atomic_state *intel_state =
-		to_intel_atomic_state(state);
-	int cdclk, vco;
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_cdclk_state *logical = &state->cdclk.logical;
+	struct intel_cdclk_state *actual = &state->cdclk.actual;
+	int max_pixclk = intel_max_pixel_rate(&state->base);
 
-	if (IS_GEMINILAKE(dev_priv)) {
-		cdclk = glk_calc_cdclk(max_pixclk);
-		vco = glk_de_pll_vco(dev_priv, cdclk);
-	} else {
-		cdclk = bxt_calc_cdclk(max_pixclk);
-		vco = bxt_de_pll_vco(dev_priv, cdclk);
-	}
+	/*
+	 * FIXME: should also account for plane ratio once 64bpp pixel formats
+	 * are supported.
+	 */
+	dev_priv->display.calc_cdclk_state(dev_priv, max_pixclk, logical);
 
-	if (cdclk > dev_priv->max_cdclk_freq) {
+	if (logical->cdclk > dev_priv->max_cdclk_freq) {
 		DRM_DEBUG_KMS("requested cdclk (%d kHz) exceeds max (%d kHz)\n",
-			      cdclk, dev_priv->max_cdclk_freq);
+			      logical->cdclk, dev_priv->max_cdclk_freq);
 		return -EINVAL;
 	}
 
-	intel_state->cdclk.logical.vco = vco;
-	intel_state->cdclk.logical.cdclk = cdclk;
-
-	if (!intel_state->active_crtcs) {
-		if (IS_GEMINILAKE(dev_priv)) {
-			cdclk = glk_calc_cdclk(0);
-			vco = glk_de_pll_vco(dev_priv, cdclk);
-		} else {
-			cdclk = bxt_calc_cdclk(0);
-			vco = bxt_de_pll_vco(dev_priv, cdclk);
-		}
-
-		intel_state->cdclk.actual.vco = vco;
-		intel_state->cdclk.actual.cdclk = cdclk;
-	} else {
-		intel_state->cdclk.actual =
-			intel_state->cdclk.logical;
-	}
+	if (!state->active_crtcs)
+		dev_priv->display.calc_cdclk_state(dev_priv, 0, actual);
+	else
+		*actual = *logical;
 
 	return 0;
 }
@@ -1823,24 +1744,22 @@ void intel_init_cdclk_hooks(struct drm_i915_private *dev_priv)
 {
 	if (IS_CHERRYVIEW(dev_priv)) {
 		dev_priv->display.set_cdclk = chv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
+		dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state;
 	} else if (IS_VALLEYVIEW(dev_priv)) {
 		dev_priv->display.set_cdclk = vlv_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			vlv_modeset_calc_cdclk;
+		dev_priv->display.calc_cdclk_state = vlv_calc_cdclk_state;
 	} else if (IS_BROADWELL(dev_priv)) {
 		dev_priv->display.set_cdclk = bdw_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bdw_modeset_calc_cdclk;
-	} else if (IS_GEN9_LP(dev_priv)) {
+		dev_priv->display.calc_cdclk_state = bdw_calc_cdclk_state;
+	} else if (IS_BROXTON(dev_priv)) {
+		dev_priv->display.set_cdclk = bxt_set_cdclk;
+		dev_priv->display.calc_cdclk_state = bxt_calc_cdclk_state;
+	} else if (IS_GEMINILAKE(dev_priv)) {
 		dev_priv->display.set_cdclk = bxt_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			bxt_modeset_calc_cdclk;
+		dev_priv->display.calc_cdclk_state = glk_calc_cdclk_state;
 	} else if (IS_GEN9_BC(dev_priv)) {
 		dev_priv->display.set_cdclk = skl_set_cdclk;
-		dev_priv->display.modeset_calc_cdclk =
-			skl_modeset_calc_cdclk;
+		dev_priv->display.calc_cdclk_state = skl_calc_cdclk_state;
 	}
 
 	if (IS_GEN9_BC(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f6feb93..1d2cb491 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12414,8 +12414,8 @@ static int intel_modeset_checks(struct drm_atomic_state *state)
 	 * mode set on this crtc.  For other crtcs we need to use the
 	 * adjusted_mode bits in the crtc directly.
 	 */
-	if (dev_priv->display.modeset_calc_cdclk) {
-		ret = dev_priv->display.modeset_calc_cdclk(state);
+	if (dev_priv->display.calc_cdclk_state) {
+		ret = intel_modeset_calc_cdclk(intel_state);
 		if (ret < 0)
 			return ret;
 
@@ -15468,7 +15468,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
 				pixclk = crtc_state->pixel_rate;
 			else
-				WARN_ON(dev_priv->display.modeset_calc_cdclk);
+				WARN_ON(dev_priv->display.calc_cdclk_state);
 
 			/* pixel rate mustn't exceed 95% of cdclk with IPS on BDW */
 			if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 821c57c..3e112fe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1260,6 +1260,7 @@ bool intel_cdclk_state_compare(const struct intel_cdclk_state *a,
 			       const struct intel_cdclk_state *b);
 void intel_set_cdclk(struct drm_i915_private *dev_priv,
 		     const struct intel_cdclk_state *cdclk_state);
+int intel_modeset_calc_cdclk(struct intel_atomic_state *state);
 
 /* intel_display.c */
 enum transcoder intel_crtc_pch_transcoder(struct intel_crtc *crtc);
-- 
2.7.4



More information about the Intel-gfx mailing list