On Fri, 25 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Start splitting the .compute_crtc_clock() into two parts; one
> part does the computation, the second part does the shared dpll
> assignment. I want to move the actual computation part much earlier
> into the compute_config() phase.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 4 ++
> drivers/gpu/drm/i915/display/intel_dpll.c | 54 +++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_dpll.h | 2 +
> 3 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7c68bc07c925..1b7bc764498c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5003,6 +5003,10 @@ static int intel_crtc_atomic_check(struct intel_atomic_state *state,
> ret = intel_dpll_crtc_compute_clock(state, crtc);
> if (ret)
> return ret;
> +
> + ret = intel_dpll_crtc_get_shared_dpll(state, crtc);
> + if (ret)
> + return ret;
> }
>
> /*
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.c b/drivers/gpu/drm/i915/display/intel_dpll.c
> index bc59efe18e89..2ee7255f3c36 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.c
> @@ -20,6 +20,8 @@
> struct intel_dpll_funcs {
> int (*crtc_compute_clock)(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> + int (*crtc_get_shared_dpll)(struct intel_atomic_state *state,
> + struct intel_crtc *crtc);
> };
>
> struct intel_limit {
> @@ -930,6 +932,12 @@ static void i8xx_compute_dpll(struct intel_crtc_state *crtc_state,
>
> static int hsw_crtc_compute_clock(struct intel_atomic_state *state,
> struct intel_crtc *crtc)
> +{
> + return 0;
> +}
> +
> +static int hsw_crtc_get_shared_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> {
> struct drm_i915_private *dev_priv = to_i915(state->base.dev);
> struct intel_crtc_state *crtc_state =
> @@ -964,6 +972,12 @@ static int dg2_crtc_compute_clock(struct intel_atomic_state *state,
> return intel_mpllb_calc_state(crtc_state, encoder);
> }
>
> +static int dg2_crtc_get_shared_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + return 0;
> +}
> +
This seems superfluous at this time because
intel_dpll_crtc_get_shared_dpll() checks for NULL
.crtc_get_shared_dpll() and does exactly that.
Not a biggie.
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> static bool ilk_needs_fb_cb_tune(const struct dpll *dpll, int factor)
> {
> return dpll->m < factor * dpll->n;
> @@ -1087,7 +1101,6 @@ static int ilk_crtc_compute_clock(struct intel_atomic_state *state,
> intel_atomic_get_new_crtc_state(state, crtc);
> const struct intel_limit *limit;
> int refclk = 120000;
> - int ret;
>
> /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> if (!crtc_state->has_pch_encoder)
> @@ -1127,6 +1140,21 @@ static int ilk_crtc_compute_clock(struct intel_atomic_state *state,
> ilk_compute_dpll(crtc_state, &crtc_state->dpll,
> &crtc_state->dpll);
>
> + return 0;
> +}
> +
> +static int ilk_crtc_get_shared_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + 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);
> + int ret;
> +
> + /* CPU eDP is the only output that doesn't need a PCH PLL of its own. */
> + if (!crtc_state->has_pch_encoder)
> + return 0;
> +
> ret = intel_reserve_shared_dplls(state, crtc, NULL);
> if (ret) {
> drm_dbg_kms(&dev_priv->drm,
> @@ -1372,14 +1400,17 @@ static int i8xx_crtc_compute_clock(struct intel_atomic_state *state,
>
> static const struct intel_dpll_funcs dg2_dpll_funcs = {
> .crtc_compute_clock = dg2_crtc_compute_clock,
> + .crtc_get_shared_dpll = dg2_crtc_get_shared_dpll,
> };
>
> static const struct intel_dpll_funcs hsw_dpll_funcs = {
> .crtc_compute_clock = hsw_crtc_compute_clock,
> + .crtc_get_shared_dpll = hsw_crtc_get_shared_dpll,
> };
>
> static const struct intel_dpll_funcs ilk_dpll_funcs = {
> .crtc_compute_clock = ilk_crtc_compute_clock,
> + .crtc_get_shared_dpll = ilk_crtc_get_shared_dpll,
> };
>
> static const struct intel_dpll_funcs chv_dpll_funcs = {
> @@ -1427,6 +1458,27 @@ int intel_dpll_crtc_compute_clock(struct intel_atomic_state *state,
> return i915->dpll_funcs->crtc_compute_clock(state, crtc);
> }
>
> +int intel_dpll_crtc_get_shared_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc)
> +{
> + struct drm_i915_private *i915 = to_i915(state->base.dev);
> + struct intel_crtc_state *crtc_state =
> + intel_atomic_get_new_crtc_state(state, crtc);
> +
> + drm_WARN_ON(&i915->drm, !intel_crtc_needs_modeset(crtc_state));
> +
> + if (drm_WARN_ON(&i915->drm, crtc_state->shared_dpll))
> + return 0;
> +
> + if (!crtc_state->hw.enable)
> + return 0;
> +
> + if (!i915->dpll_funcs->crtc_get_shared_dpll)
> + return 0;
> +
> + return i915->dpll_funcs->crtc_get_shared_dpll(state, crtc);
> +}
> +
> void
> intel_dpll_init_clock_hook(struct drm_i915_private *dev_priv)
> {
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll.h b/drivers/gpu/drm/i915/display/intel_dpll.h
> index e9731b2dd01c..bbc30542f29f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll.h
> @@ -18,6 +18,8 @@ enum pipe;
> void intel_dpll_init_clock_hook(struct drm_i915_private *dev_priv);
> int intel_dpll_crtc_compute_clock(struct intel_atomic_state *state,
> struct intel_crtc *crtc);
> +int intel_dpll_crtc_get_shared_dpll(struct intel_atomic_state *state,
> + struct intel_crtc *crtc);
> int vlv_calc_dpll_params(int refclk, struct dpll *clock);
> int pnv_calc_dpll_params(int refclk, struct dpll *clock);
> int i9xx_calc_dpll_params(int refclk, struct dpll *clock);
--
Jani Nikula, Intel Open Source Graphics Center
