diff mbox series

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

Message ID 20190117202113.5190-6-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series icl: Misc PLL patches | expand

Commit Message

Lucas De Marchi Jan. 17, 2019, 8:21 p.m. UTC 
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.

Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 41 ++++++++++++---------------
 1 file changed, 18 insertions(+), 23 deletions(-)

Comments

Lucas De Marchi Jan. 17, 2019, 10:06 p.m. UTC | #1 
On Thu, Jan 17, 2019 at 12:21 PM Lucas De Marchi
<lucas.demarchi@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

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;