On Thu, Jan 17, 2019 at 12:21 PM Lucas De Marchi
<lucas.demarchi@xxxxxxxxx> wrote:
>
> Right now when searching for shared plls we mandate that they have
> consecutive IDs since we just pass the min and max id and loop over the
> range. However the IDs can't be arbitrarily defined since they are used
> as index to the MMIO address, hence dependent on what the hardware
> implements.
>
> This allows us to use PLLs that are not consecutive (although we don't
> currently have any case) while clarifying the code paths in which only
> one PLL is supposed to be used.
Other possible approach for if/when we need this would be to use a different
macro that doesn't use the id as the index for the registers... Not
sure which one is better.
Lucas De Marchi
>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 ++++++++++++---------------
> 1 file changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 8f70838ac7d8..103e42cfa2e3 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -243,8 +243,7 @@ void intel_disable_shared_dpll(const struct intel_crtc_state *crtc_state)
> static struct intel_shared_dpll *
> intel_find_shared_dpll(struct intel_crtc *crtc,
> struct intel_crtc_state *crtc_state,
> - enum intel_dpll_id range_min,
> - enum intel_dpll_id range_max)
> + unsigned long pll_mask)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> struct intel_shared_dpll *pll, *unused_pll = NULL;
> @@ -253,7 +252,7 @@ intel_find_shared_dpll(struct intel_crtc *crtc,
>
> shared_dpll = intel_atomic_get_shared_dpll_state(crtc_state->base.state);
>
> - for (i = range_min; i <= range_max; i++) {
> + for_each_set_bit(i, &pll_mask, I915_NUM_PLLS) {
> pll = &dev_priv->shared_dplls[i];
>
> /* Only want to check enabled timings first */
> @@ -436,8 +435,8 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> pll->info->name);
> } else {
> pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_PCH_PLL_A,
> - DPLL_ID_PCH_PLL_B);
> + GENMASK(DPLL_ID_PCH_PLL_B,
> + DPLL_ID_PCH_PLL_A));
> }
>
> if (!pll)
> @@ -780,7 +779,7 @@ static struct intel_shared_dpll *hsw_ddi_hdmi_get_dpll(int clock,
> crtc_state->dpll_hw_state.wrpll = val;
>
> pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
> + GENMASK(DPLL_ID_WRPLL2, DPLL_ID_WRPLL1));
>
> if (!pll)
> return NULL;
> @@ -840,7 +839,7 @@ hsw_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> SPLL_PLL_ENABLE | SPLL_PLL_FREQ_1350MHz | SPLL_PLL_SSC;
>
> pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_SPLL, DPLL_ID_SPLL);
> + BIT(DPLL_ID_SPLL));
> } else {
> return NULL;
> }
> @@ -1389,6 +1388,7 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> int clock = crtc_state->port_clock;
> bool bret;
> struct intel_dpll_hw_state dpll_hw_state;
> + unsigned long pll_mask;
>
> memset(&dpll_hw_state, 0, sizeof(dpll_hw_state));
>
> @@ -1410,13 +1410,11 @@ skl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> }
>
> if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> - pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_SKL_DPLL0,
> - DPLL_ID_SKL_DPLL0);
> + pll_mask = BIT(DPLL_ID_SKL_DPLL0);
> else
> - pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_SKL_DPLL1,
> - DPLL_ID_SKL_DPLL3);
> + pll_mask = GENMASK(DPLL_ID_SKL_DPLL3, DPLL_ID_SKL_DPLL1);
> +
> + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask);
> if (!pll)
> return NULL;
>
> @@ -2390,8 +2388,8 @@ cnl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> }
>
> pll = intel_find_shared_dpll(crtc, crtc_state,
> - DPLL_ID_SKL_DPLL0,
> - DPLL_ID_SKL_DPLL2);
> + GENMASK(DPLL_ID_SKL_DPLL2,
> + DPLL_ID_SKL_DPLL0));
> if (!pll) {
> DRM_DEBUG_KMS("No PLL selected\n");
> return NULL;
> @@ -2899,13 +2897,12 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> struct intel_shared_dpll *pll;
> struct intel_dpll_hw_state pll_state = {};
> enum port port = encoder->port;
> - enum intel_dpll_id min, max;
> + unsigned long pll_mask;
> int clock = crtc_state->port_clock;
> bool ret;
>
> if (intel_port_is_combophy(dev_priv, port)) {
> - min = DPLL_ID_ICL_DPLL0;
> - max = DPLL_ID_ICL_DPLL1;
> + pll_mask = GENMASK(DPLL_ID_ICL_DPLL1, DPLL_ID_ICL_DPLL0);
> ret = icl_calc_dpll_state(crtc_state, encoder, clock,
> &pll_state);
> } else if (intel_port_is_tc(dev_priv, port)) {
> @@ -2919,16 +2916,14 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
> }
>
> if (intel_dig_port->tc_type == TC_PORT_TBT) {
> - min = DPLL_ID_ICL_TBTPLL;
> - max = min;
> + pll_mask = BIT(DPLL_ID_ICL_TBTPLL);
> ret = icl_calc_dpll_state(crtc_state, encoder, clock,
> &pll_state);
> } else {
> enum tc_port tc_port;
>
> tc_port = intel_port_to_tc(dev_priv, port);
> - min = icl_tc_port_to_pll_id(tc_port);
> - max = min;
> + pll_mask = BIT(icl_tc_port_to_pll_id(tc_port));
> ret = icl_calc_mg_pll_state(crtc_state, encoder, clock,
> &pll_state);
> }
> @@ -2944,7 +2939,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
>
> crtc_state->dpll_hw_state = pll_state;
>
> - pll = intel_find_shared_dpll(crtc, crtc_state, min, max);
> + pll = intel_find_shared_dpll(crtc, crtc_state, pll_mask);
> if (!pll) {
> DRM_DEBUG_KMS("No PLL selected\n");
> return NULL;
> --
> 2.20.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
