[PATCH 17/23] drm/i915: Sanitize the shared DPLL reserve/release interface
Imre Deak
imre.deak at intel.com
Mon Jun 3 03:25:32 UTC 2019
For consistency s/intel_get_shared_dpll()/intel_reserve_shared_dplls()/
to better match intel_release_shared_dplls(). Also, pass to them the
CRTC object instead of its state. Using the object they can retrieve the
state they need.
To make the PLL clearing from the atomic modeset state and CRTC's state
in a more onion-like way do the clearing in new
put_dplls()/intel_unreference_shared_dpll() functions (matching the
existing get_dplls()/intel_reference_shared_dpll()).
Since nothing uses the PLL returned by intel_reserve_shared_dplls(),
make it return only a bool.
This refactoring is also a preparation for a follow-up change that needs
to reserve multiple DPLLs.
No functional changes.
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Signed-off-by: Imre Deak <imre.deak at intel.com>
---
drivers/gpu/drm/i915/intel_display.c | 22 ++--
drivers/gpu/drm/i915/intel_dpll_mgr.c | 181 ++++++++++++++++----------
drivers/gpu/drm/i915/intel_dpll_mgr.h | 13 +-
3 files changed, 128 insertions(+), 88 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0162758e46d8..c2ba3dd72321 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9425,6 +9425,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;
@@ -9466,7 +9468,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;
@@ -9846,7 +9848,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;
@@ -13117,26 +13119,18 @@ static void intel_modeset_clear_plls(struct drm_atomic_state *state)
struct drm_device *dev = state->dev;
struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_crtc *crtc;
- struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+ struct drm_crtc_state *new_crtc_state;
int i;
if (!dev_priv->display.crtc_compute_clock)
return;
- for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
- struct intel_shared_dpll *old_dpll =
- to_intel_crtc_state(old_crtc_state)->shared_dpll;
-
+ for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
if (!needs_modeset(new_crtc_state))
continue;
- to_intel_crtc_state(new_crtc_state)->shared_dpll = NULL;
-
- if (!old_dpll)
- continue;
-
- intel_release_shared_dpll(old_dpll, intel_crtc, state);
+ intel_release_shared_dplls(to_intel_atomic_state(state),
+ to_intel_crtc(crtc));
}
}
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 487fe92b7f10..68306710da4c 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/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().
+ * modest 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,25 @@ 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_shared_dpll *pll,
+ struct intel_crtc_state *crtc_state)
+{
+ struct intel_shared_dpll_state *shared_dpll;
+ enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+
+ shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
+ shared_dpll[pll->info->id].crtc_mask &= ~(1 << pipe);
+}
+
+static void intel_put_dpll(struct intel_crtc_state *crtc_state)
+{
+ if (!crtc_state->shared_dpll)
+ return;
+
+ intel_unreference_shared_dpll(crtc_state->shared_dpll, crtc_state);
+ crtc_state->shared_dpll = NULL;
+}
+
/**
* intel_shared_dpll_swap_state - make atomic DPLL configuration effective
* @state: atomic state
@@ -421,7 +441,7 @@ static void ibx_pch_dpll_disable(struct drm_i915_private *dev_priv,
udelay(200);
}
-static struct intel_shared_dpll *
+static bool
ibx_get_dpll(struct intel_crtc_state *crtc_state,
struct intel_encoder *encoder)
{
@@ -446,7 +466,7 @@ 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->dpll_hw_state,
@@ -454,7 +474,7 @@ ibx_get_dpll(struct intel_crtc_state *crtc_state,
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static void ibx_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -826,7 +846,7 @@ hsw_ddi_dp_get_dpll(struct intel_crtc_state *crtc_state)
return pll;
}
-static struct intel_shared_dpll *
+static bool
hsw_get_dpll(struct intel_crtc_state *crtc_state,
struct intel_encoder *encoder)
{
@@ -841,7 +861,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_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
@@ -850,18 +870,18 @@ hsw_get_dpll(struct intel_crtc_state *crtc_state,
&crtc_state->dpll_hw_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->dpll_hw_state,
crtc_state);
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static void hsw_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1394,7 +1414,7 @@ skl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
return true;
}
-static struct intel_shared_dpll *
+static bool
skl_get_dpll(struct intel_crtc_state *crtc_state,
struct intel_encoder *encoder)
{
@@ -1405,16 +1425,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))
@@ -1428,14 +1448,14 @@ 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->dpll_hw_state,
crtc_state);
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static void skl_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1841,7 +1861,7 @@ 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 *
+static bool
bxt_get_dpll(struct intel_crtc_state *crtc_state,
struct intel_encoder *encoder)
{
@@ -1852,11 +1872,11 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
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;
@@ -1870,7 +1890,7 @@ bxt_get_dpll(struct intel_crtc_state *crtc_state,
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static void bxt_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -1901,8 +1921,9 @@ 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_crtc_state *crtc_state,
+ struct intel_encoder *encoder);
+ void (*put_dplls)(struct intel_crtc_state *crtc_state);
void (*dump_hw_state)(struct drm_i915_private *dev_priv,
struct intel_dpll_hw_state *hw_state);
@@ -1916,7 +1937,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,
};
@@ -1932,7 +1954,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,
};
@@ -1946,7 +1969,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,
};
@@ -1959,7 +1983,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,
};
@@ -2349,7 +2374,7 @@ cnl_ddi_dp_set_dpll_hw_state(struct intel_crtc_state *crtc_state)
return true;
}
-static struct intel_shared_dpll *
+static bool
cnl_get_dpll(struct intel_crtc_state *crtc_state,
struct intel_encoder *encoder)
{
@@ -2360,18 +2385,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,
@@ -2380,7 +2405,7 @@ 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->dpll_hw_state,
@@ -2388,7 +2413,7 @@ cnl_get_dpll(struct intel_crtc_state *crtc_state,
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static void cnl_dump_hw_state(struct drm_i915_private *dev_priv,
@@ -2415,7 +2440,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,
};
@@ -2813,9 +2839,9 @@ 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_crtc_state *crtc_state,
+ struct intel_encoder *encoder)
{
struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
struct intel_digital_port *intel_dig_port;
@@ -2855,12 +2881,12 @@ 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;
}
@@ -2868,7 +2894,7 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
min, max);
if (!pll) {
DRM_DEBUG_KMS("No PLL selected\n");
- return NULL;
+ return false;
}
intel_reference_shared_dpll(pll, &crtc_state->dpll_hw_state,
@@ -2876,7 +2902,7 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
crtc_state->shared_dpll = pll;
- return pll;
+ return true;
}
static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
@@ -3251,7 +3277,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,
};
@@ -3263,7 +3290,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,
};
@@ -3315,50 +3343,67 @@ 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().
+ * Reserve all required DPLLs for the given CRTC and encoder combination. The
+ * DPLLs along with their new state are stored into @crtc's new state and they
+ * are reserved in the atomic state.
+ *
+ * The configuration is made effective by calling
+ * intel_shared_dpll_swap_state(). The DPLLs reserved by this function 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 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(crtc->base.dev);
const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
+ struct intel_crtc_state *crtc_state =
+ to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base,
+ &crtc->base));
if (WARN_ON(!dpll_mgr))
- return NULL;
+ return false;
- return dpll_mgr->get_dpll(crtc_state, encoder);
+ return dpll_mgr->get_dplls(crtc_state, 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 @crtc's new state and from the atomic @state.
*
- * This function releases the reference from @crtc to @dpll from the
- * atomic @state. The new configuration is made effective by calling
+ * The new configuration 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(crtc->base.dev);
+ const struct intel_dpll_mgr *dpll_mgr = dev_priv->dpll_mgr;
+ struct intel_crtc_state *crtc_state =
+ to_intel_crtc_state(drm_atomic_get_new_crtc_state(&state->base,
+ &crtc->base));
+
+ /*
+ * FIXME: intel_release_shared_dplls is called for every platform with
+ * a compute_clock hook, even though the platform didn't call
+ * intel_reserve_shared_dplls.
+ */
+ 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(crtc_state);
}
/**
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 8835dd20f1d2..3ce305b06ff0 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -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,
- struct intel_crtc *crtc,
- struct drm_atomic_state *state);
+bool intel_reserve_shared_dplls(struct intel_atomic_state *state,
+ struct intel_crtc *crtc,
+ 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);
--
2.17.1
More information about the Intel-gfx-trybot
mailing list