[Intel-gfx] [PATCH 5/5] drm/i915: allow shared plls to be non-consecutive

Lucas De Marchi lucas.de.marchi at gmail.com
Thu Jan 17 22:06:38 UTC 2019


On Thu, Jan 17, 2019 at 12:21 PM Lucas De Marchi
<lucas.demarchi at intel.com> 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 at intel.com>
> ---
>  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Lucas De Marchi


More information about the Intel-gfx mailing list