Commit 866955fa authored by Imre Deak's avatar Imre Deak
Browse files

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: default avatarImre Deak <imre.deak@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190628143635.22066-17-imre.deak@intel.com
parent 4f36afb2
Loading
Loading
Loading
Loading
+7 −12
Original line number Diff line number Diff line
@@ -9493,6 +9493,8 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,
				       struct intel_crtc_state *crtc_state)
{
	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
	struct intel_atomic_state *state =
		to_intel_atomic_state(crtc_state->base.state);
	const struct intel_limit *limit;
	int refclk = 120000;

@@ -9534,7 +9536,7 @@ static int ironlake_crtc_compute_clock(struct intel_crtc *crtc,

	ironlake_compute_dpll(crtc, crtc_state, NULL);

	if (!intel_get_shared_dpll(crtc_state, NULL)) {
	if (!intel_reserve_shared_dplls(state, crtc, NULL)) {
		DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
			      pipe_name(crtc->pipe));
		return -EINVAL;
@@ -9915,7 +9917,7 @@ static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
		struct intel_encoder *encoder =
			intel_get_crtc_new_encoder(state, crtc_state);

		if (!intel_get_shared_dpll(crtc_state, encoder)) {
		if (!intel_reserve_shared_dplls(state, crtc, encoder)) {
			DRM_DEBUG_KMS("failed to find PLL for pipe %c\n",
				      pipe_name(crtc->pipe));
			return -EINVAL;
@@ -13171,27 +13173,20 @@ static void update_scanline_offset(const struct intel_crtc_state *crtc_state)
static void intel_modeset_clear_plls(struct intel_atomic_state *state)
{
	struct drm_i915_private *dev_priv = to_i915(state->base.dev);
	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
	struct intel_crtc_state *new_crtc_state;
	struct intel_crtc *crtc;
	int i;

	if (!dev_priv->display.crtc_compute_clock)
		return;

	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
					    new_crtc_state, i) {
		struct intel_shared_dpll *old_dpll =
			old_crtc_state->shared_dpll;

	for_each_new_intel_crtc_in_state(state, crtc, new_crtc_state, i) {
		if (!needs_modeset(new_crtc_state))
			continue;

		new_crtc_state->shared_dpll = NULL;

		if (!old_dpll)
			continue;

		intel_release_shared_dpll(old_dpll, crtc, &state->base);
		intel_release_shared_dplls(state, crtc);
	}
}

+139 −82
Original line number Diff line number Diff line
@@ -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,
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,
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,
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,
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,
	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,
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,
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,
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 all DPLLs reserved by intel_reserve_shared_dplls()
 * from the current atomic commit @state and the old @crtc atomic state.
 *
 * 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().
 * 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);
}

/**
+7 −6
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@
struct drm_atomic_state;
struct drm_device;
struct drm_i915_private;
struct intel_atomic_state;
struct intel_crtc;
struct intel_crtc_state;
struct intel_encoder;
@@ -195,7 +196,7 @@ struct intel_dpll_hw_state {
 * future state which would be applied by an atomic mode set (stored in
 * a struct &intel_atomic_state).
 *
 * See also intel_get_shared_dpll() and intel_release_shared_dpll().
 * See also intel_reserve_shared_dplls() and intel_release_shared_dplls().
 */
struct intel_shared_dpll_state {
	/**
@@ -331,11 +332,11 @@ void assert_shared_dpll(struct drm_i915_private *dev_priv,
			bool state);
#define assert_shared_dpll_enabled(d, p) assert_shared_dpll(d, p, true)
#define assert_shared_dpll_disabled(d, p) assert_shared_dpll(d, p, false)
struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc_state *state,
						struct intel_encoder *encoder);
void intel_release_shared_dpll(struct intel_shared_dpll *dpll,
bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
				struct intel_crtc *crtc,
			       struct drm_atomic_state *state);
				struct intel_encoder *encoder);
void intel_release_shared_dplls(struct intel_atomic_state *state,
				struct intel_crtc *crtc);
void intel_prepare_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_enable_shared_dpll(const struct intel_crtc_state *crtc_state);
void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state);