drm/i915: Sanitize the shared DPLL reserve/release interface

For consistency s/intel_get_shared_dpll()/intel_reserve_shared_dplls()/
to better match intel_release_shared_dplls(). Also, pass to the
reserve/release and get_dplls/put_dplls hooks the intel_atomic_state and
CRTC object, that way these functions can look up the old or new state
as needed.

Also release the PLLs from the atomic state via a new
put_dplls->intel_unreference_shared_dpll() call chain for better
symmetry with the reservation via the
get_dplls->intel_reference_shared_dpll() call chain.

Since nothing uses the PLL returned by intel_reserve_shared_dplls(),
make it return only a bool.

While at it also clarify the reserve/release function docbook headers
making it clear that multiple DPLLs will be reserved/released and
whether the new or old atomic CRTC state is affected.

This refactoring is also a preparation for a follow-up change that needs
to reserve multiple DPLLs.

Kudos to Ville for the idea to pass intel_atomic_state around, to make
things clearer locally where an object's old/new atomic state is
required.

No functional changes.

v2:
- Fix checkpatch issue: typo in code comment.
v3:
- Rebase on drm-tip.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190628143635.22066-17-imre.deak@intel.com

1 files changed, 139 insertions, 82 deletions

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index bf66261c8bf0..3fbc975851fa 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -36,9 +36,10 @@
  * This file provides an abstraction over display PLLs. The function
  * intel_shared_dpll_init() initializes the PLLs for the given platform.  The
  * users of a PLL are tracked and that tracking is integrated with the atomic
- * modest interface. During an atomic operation, a PLL can be requested for a
- * given CRTC and encoder configuration by calling intel_get_shared_dpll() and
- * a previously used PLL can be released with intel_release_shared_dpll().
+ * modset interface. During an atomic operation, required PLLs can be reserved
+ * for a given CRTC and encoder configuration by calling
+ * intel_reserve_shared_dplls() and previously reserved PLLs can be released
+ * with intel_release_shared_dplls().
  * Changes to the users are first staged in the atomic state, and then made
  * effective by calling intel_shared_dpll_swap_state() during the atomic
  * commit phase.
@@ -309,6 +310,28 @@ intel_reference_shared_dpll(struct intel_shared_dpll *pll,
 	shared_dpll[id].crtc_mask |= 1 << crtc->pipe;
 }
 
+static void intel_unreference_shared_dpll(struct intel_atomic_state *state,
+					  const struct intel_crtc *crtc,
+					  const struct intel_shared_dpll *pll)
+{
+	struct intel_shared_dpll_state *shared_dpll;
+
+	shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
+	shared_dpll[pll->info->id].crtc_mask &= ~(1 << crtc->pipe);
+}
+
+static void intel_put_dpll(struct intel_atomic_state *state,
+			   struct intel_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_old_crtc_state(state, crtc);
+
+	if (!crtc_state->shared_dpll)
+		return;
+
+	intel_unreference_shared_dpll(state, crtc, crtc_state->shared_dpll);
+}
+
 /**
  * intel_shared_dpll_swap_state - make atomic DPLL configuration effective
  * @state: atomic state
@@ -421,11 +444,12 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
 	udelay(200);
 }
 
-static struct intel_shared_dpll *
-ibx_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool ibx_get_dpll(struct intel_atomic_state *state,
+			 struct intel_crtc *crtc,
+			 struct intel_encoder *encoder)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id i;
@@ -445,12 +469,12 @@ ibx_get_dpll(struct intel_crtc_state *crtc_state,
 	}
 
 	if (!pll)
-		return NULL;
+		return false;
 
 	/* reference the pll */
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static void ibx_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -821,10 +845,12 @@ hsw_ddi_dp_get_dpll(struct intel_crtc_state *crtc_state)
 	return pll;
 }
 
-static struct intel_shared_dpll *
-hsw_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool hsw_get_dpll(struct intel_atomic_state *state,
+			 struct intel_crtc *crtc,
+			 struct intel_encoder *encoder)
 {
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_shared_dpll *pll;
 
 	memset(&crtc_state->dpll_hw_state, 0,
@@ -836,7 +862,7 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
 		pll = hsw_ddi_dp_get_dpll(crtc_state);
 	} else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG)) {
 		if (WARN_ON(crtc_state->port_clock / 2 != 135000))
-			return NULL;
+			return false;
 
 		crtc_state->dpll_hw_state.spll =
 			SPLL_PLL_ENABLE | SPLL_FREQ_1350MHz | SPLL_REF_MUXED_SSC;
@@ -844,15 +870,15 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
 		pll = intel_find_shared_dpll(crtc_state,
 					     DPLL_ID_SPLL, DPLL_ID_SPLL);
 	} else {
-		return NULL;
+		return false;
 	}
 
 	if (!pll)
-		return NULL;
+		return false;
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static void hsw_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1385,10 +1411,12 @@ skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
-static struct intel_shared_dpll *
-skl_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool skl_get_dpll(struct intel_atomic_state *state,
+			 struct intel_crtc *crtc,
+			 struct intel_encoder *encoder)
 {
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_shared_dpll *pll;
 	bool bret;
 
@@ -1396,16 +1424,16 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
 		bret = skl_ddi_hdmi_pll_dividers(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
-			return NULL;
+			return false;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		bret = skl_ddi_dp_set_dpll_hw_state(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
-			return NULL;
+			return false;
 		}
 	} else {
-		return NULL;
+		return false;
 	}
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
@@ -1417,11 +1445,11 @@ skl_get_dpll(struct intel_crtc_state *crtc_state,
 					     DPLL_ID_SKL_DPLL1,
 					     DPLL_ID_SKL_DPLL3);
 	if (!pll)
-		return NULL;
+		return false;
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static void skl_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1827,22 +1855,23 @@ bxt_ddi_hdmi_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 	return bxt_ddi_set_dpll_hw_state(crtc_state, &clk_div);
 }
 
-static struct intel_shared_dpll *
-bxt_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool bxt_get_dpll(struct intel_atomic_state *state,
+			 struct intel_crtc *crtc,
+			 struct intel_encoder *encoder)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id id;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
 	    !bxt_ddi_hdmi_set_dpll_hw_state(crtc_state))
-		return NULL;
+		return false;
 
 	if (intel_crtc_has_dp_encoder(crtc_state) &&
 	    !bxt_ddi_dp_set_dpll_hw_state(crtc_state))
-		return NULL;
+		return false;
 
 	/* 1:1 mapping between ports and PLLs */
 	id = (enum intel_dpll_id) encoder->port;
@@ -1853,7 +1882,7 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static void bxt_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1884,8 +1913,11 @@ static const struct intel_shared_dpll_funcs bxt_ddi_pll_funcs = {
 struct intel_dpll_mgr {
 	const struct dpll_info *dpll_info;
 
-	struct intel_shared_dpll *(*get_dpll)(struct intel_crtc_state *crtc_state,
-					      struct intel_encoder *encoder);
+	bool (*get_dplls)(struct intel_atomic_state *state,
+			  struct intel_crtc *crtc,
+			  struct intel_encoder *encoder);
+	void (*put_dplls)(struct intel_atomic_state *state,
+			  struct intel_crtc *crtc);
 
 	void (*dump_hw_state)(struct drm_i915_private *dev_priv,
 			      const struct intel_dpll_hw_state *hw_state);
@@ -1899,7 +1931,8 @@ static const struct dpll_info pch_plls[] = {
 
 static const struct intel_dpll_mgr pch_pll_mgr = {
 	.dpll_info = pch_plls,
-	.get_dpll = ibx_get_dpll,
+	.get_dplls = ibx_get_dpll,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = ibx_dump_hw_state,
 };
 
@@ -1915,7 +1948,8 @@ static const struct dpll_info hsw_plls[] = {
 
 static const struct intel_dpll_mgr hsw_pll_mgr = {
 	.dpll_info = hsw_plls,
-	.get_dpll = hsw_get_dpll,
+	.get_dplls = hsw_get_dpll,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = hsw_dump_hw_state,
 };
 
@@ -1929,7 +1963,8 @@ static const struct dpll_info skl_plls[] = {
 
 static const struct intel_dpll_mgr skl_pll_mgr = {
 	.dpll_info = skl_plls,
-	.get_dpll = skl_get_dpll,
+	.get_dplls = skl_get_dpll,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = skl_dump_hw_state,
 };
 
@@ -1942,7 +1977,8 @@ static const struct dpll_info bxt_plls[] = {
 
 static const struct intel_dpll_mgr bxt_pll_mgr = {
 	.dpll_info = bxt_plls,
-	.get_dpll = bxt_get_dpll,
+	.get_dplls = bxt_get_dpll,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = bxt_dump_hw_state,
 };
 
@@ -2332,10 +2368,12 @@ cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
-static struct intel_shared_dpll *
-cnl_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool cnl_get_dpll(struct intel_atomic_state *state,
+			 struct intel_crtc *crtc,
+			 struct intel_encoder *encoder)
 {
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_shared_dpll *pll;
 	bool bret;
 
@@ -2343,18 +2381,18 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
 		bret = cnl_ddi_hdmi_pll_dividers(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not get HDMI pll dividers.\n");
-			return NULL;
+			return false;
 		}
 	} else if (intel_crtc_has_dp_encoder(crtc_state)) {
 		bret = cnl_ddi_dp_set_dpll_hw_state(crtc_state);
 		if (!bret) {
 			DRM_DEBUG_KMS("Could not set DP dpll HW state.\n");
-			return NULL;
+			return false;
 		}
 	} else {
 		DRM_DEBUG_KMS("Skip DPLL setup for output_types 0x%x\n",
 			      crtc_state->output_types);
-		return NULL;
+		return false;
 	}
 
 	pll = intel_find_shared_dpll(crtc_state,
@@ -2362,12 +2400,12 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
 				     DPLL_ID_SKL_DPLL2);
 	if (!pll) {
 		DRM_DEBUG_KMS("No PLL selected\n");
-		return NULL;
+		return false;
 	}
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static void cnl_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -2394,7 +2432,8 @@ static const struct dpll_info cnl_plls[] = {
 
 static const struct intel_dpll_mgr cnl_pll_mgr = {
 	.dpll_info = cnl_plls,
-	.get_dpll = cnl_get_dpll,
+	.get_dplls = cnl_get_dpll,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = cnl_dump_hw_state,
 };
 
@@ -2792,11 +2831,13 @@ static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
 	return true;
 }
 
-static struct intel_shared_dpll *
-icl_get_dpll(struct intel_crtc_state *crtc_state,
-	     struct intel_encoder *encoder)
+static bool icl_get_dplls(struct intel_atomic_state *state,
+			  struct intel_crtc *crtc,
+			  struct intel_encoder *encoder)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	struct intel_crtc_state *crtc_state =
+		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_digital_port *intel_dig_port;
 	struct intel_shared_dpll *pll;
 	enum port port = encoder->port;
@@ -2831,24 +2872,24 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 		}
 	} else {
 		MISSING_CASE(port);
-		return NULL;
+		return false;
 	}
 
 	if (!ret) {
 		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
-		return NULL;
+		return false;
 	}
 
 
 	pll = intel_find_shared_dpll(crtc_state, min, max);
 	if (!pll) {
 		DRM_DEBUG_KMS("No PLL selected\n");
-		return NULL;
+		return false;
 	}
 
 	intel_reference_shared_dpll(pll, crtc_state);
 
-	return pll;
+	return true;
 }
 
 static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
@@ -3223,7 +3264,8 @@ static const struct dpll_info icl_plls[] = {
 
 static const struct intel_dpll_mgr icl_pll_mgr = {
 	.dpll_info = icl_plls,
-	.get_dpll = icl_get_dpll,
+	.get_dplls = icl_get_dplls,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = icl_dump_hw_state,
 };
 
@@ -3235,7 +3277,8 @@ static const struct dpll_info ehl_plls[] = {
 
 static const struct intel_dpll_mgr ehl_pll_mgr = {
 	.dpll_info = ehl_plls,
-	.get_dpll = icl_get_dpll,
+	.get_dplls = icl_get_dplls,
+	.put_dplls = intel_put_dpll,
 	.dump_hw_state = icl_dump_hw_state,
 };
 
@@ -3287,50 +3330,64 @@ void intel_shared_dpll_init(struct drm_device *dev)
 }
 
 /**
- * intel_get_shared_dpll - get a shared DPLL for CRTC and encoder combination
- * @crtc_state: atomic state for the crtc
+ * intel_reserve_shared_dplls - reserve DPLLs for CRTC and encoder combination
+ * @state: atomic state
+ * @crtc: CRTC to reserve DPLLs for
  * @encoder: encoder
  *
- * Find an appropriate DPLL for the given CRTC and encoder combination. A
- * reference from the @crtc_state to the returned pll is registered in the
- * atomic state. That configuration is made effective by calling
- * intel_shared_dpll_swap_state(). The reference should be released by calling
- * intel_release_shared_dpll().
+ * This function reserves all required DPLLs for the given CRTC and encoder
+ * combination in the current atomic commit @state and the new @crtc atomic
+ * state.
+ *
+ * The new configuration in the atomic commit @state is made effective by
+ * calling intel_shared_dpll_swap_state().
+ *
+ * The reserved DPLLs should be released by calling
+ * intel_release_shared_dplls().
  *
  * Returns:
- * A shared DPLL to be used by @crtc_state and @encoder.
+ * True if all required DPLLs were successfully reserved.
  */
-struct intel_shared_dpll *
-intel_get_shared_dpll(struct intel_crtc_state *crtc_state,
-		      struct intel_encoder *encoder)
+bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
+				struct intel_crtc *crtc,
+				struct intel_encoder *encoder)
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
 	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
 
 	if (WARN_ON(!dpll_mgr))
-		return NULL;
+		return false;
 
-	return dpll_mgr->get_dpll(crtc_state, encoder);
+	return dpll_mgr->get_dplls(state, crtc, encoder);
 }
 
 /**
- * intel_release_shared_dpll - end use of DPLL by CRTC in atomic state
- * @dpll: dpll in use by @crtc
- * @crtc: crtc
+ * intel_release_shared_dplls - end use of DPLLs by CRTC in atomic state
  * @state: atomic state
+ * @crtc: crtc from which the DPLLs are to be released
  *
- * This function releases the reference from @crtc to @dpll from the
- * atomic @state. The new configuration is made effective by calling
- * intel_shared_dpll_swap_state().
+ * This function releases all DPLLs reserved by intel_reserve_shared_dplls()
+ * from the current atomic commit @state and the old @crtc atomic state.
+ *
+ * The new configuration in the atomic commit @state is made effective by
+ * calling intel_shared_dpll_swap_state().
  */
-void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
-			       struct intel_crtc *crtc,
-			       struct drm_atomic_state *state)
+void intel_release_shared_dplls(struct intel_atomic_state *state,
+				struct intel_crtc *crtc)
 {
-	struct intel_shared_dpll_state *shared_dpll_state;
+	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
+	const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
+
+	/*
+	 * FIXME: this function is called for every platform having a
+	 * compute_clock hook, even though the platform doesn't yet support
+	 * the shared DPLL framework and intel_reserve_shared_dplls() is not
+	 * called on those.
+	 */
+	if (!dpll_mgr)
+		return;
 
-	shared_dpll_state = intel_atomic_get_shared_dpll_state(state);
-	shared_dpll_state[dpll->info->id].crtc_mask &= ~(1 << crtc->pipe);
+	dpll_mgr->put_dplls(state, crtc);
 }
 
 /**