Re: [PATCH] drm/i915: Select DPLL's via mask

[Lucas De Marchi <lucas.demarchi@xxxxxxxxx>]

On Tue, Oct 08, 2019 at 09:12:52AM -0700, Matt Roper wrote:
> This slightly simplifies the EHL DPLL4 handling and also gives us more
> flexibility in the future in case we need to skip the use of specific
> PLL's (e.g., due to hardware workarounds and such).
>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>


Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>

Lucas De Marchi

> ---
> drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 42 +++++++++----------
> 1 file changed, 20 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 5e9e84c94a15..14e040658b12 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -247,8 +247,7 @@ static struct intel_shared_dpll *
> intel_find_shared_dpll(struct intel_atomic_state *state,
> 		       const struct intel_crtc *crtc,
> 		       const struct intel_dpll_hw_state *pll_state,
> -		       enum intel_dpll_id range_min,
> -		       enum intel_dpll_id range_max)
> +		       unsigned long dpll_mask)
> {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 	struct intel_shared_dpll *pll, *unused_pll = NULL;
> @@ -257,7 +256,9 @@ intel_find_shared_dpll(struct intel_atomic_state *state,
>
> 	shared_dpll = intel_atomic_get_shared_dpll_state(&state->base);
>
> -	for (i = range_min; i <= range_max; i++) {
> +	WARN_ON(dpll_mask & ~(BIT(I915_NUM_PLLS) - 1));
> +
> +	for_each_set_bit(i, &dpll_mask, I915_NUM_PLLS) {
> 		pll = &dev_priv->shared_dplls[i];
>
> 		/* Only want to check enabled timings first */
> @@ -464,8 +465,8 @@ static bool ibx_get_dpll(struct intel_atomic_state *state,
> 	} else {
> 		pll = intel_find_shared_dpll(state, crtc,
> 					     &crtc_state->dpll_hw_state,
> -					     DPLL_ID_PCH_PLL_A,
> -					     DPLL_ID_PCH_PLL_B);
> +					     GENMASK(DPLL_ID_PCH_PLL_B,
> +						     DPLL_ID_PCH_PLL_A));
> 	}
>
> 	if (!pll)
> @@ -814,7 +815,7 @@ hsw_ddi_hdmi_get_dpll(struct intel_atomic_state *state,
>
> 	pll = intel_find_shared_dpll(state, crtc,
> 				     &crtc_state->dpll_hw_state,
> -				     DPLL_ID_WRPLL1, DPLL_ID_WRPLL2);
> +				     GENMASK(DPLL_ID_WRPLL2, DPLL_ID_WRPLL1));
>
> 	if (!pll)
> 		return NULL;
> @@ -877,7 +878,7 @@ static bool hsw_get_dpll(struct intel_atomic_state *state,
>
> 		pll = intel_find_shared_dpll(state, crtc,
> 					     &crtc_state->dpll_hw_state,
> -					     DPLL_ID_SPLL, DPLL_ID_SPLL);
> +					     BIT(DPLL_ID_SPLL));
> 	} else {
> 		return false;
> 	}
> @@ -1447,13 +1448,12 @@ static bool skl_get_dpll(struct intel_atomic_state *state,
> 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> 		pll = intel_find_shared_dpll(state, crtc,
> 					     &crtc_state->dpll_hw_state,
> -					     DPLL_ID_SKL_DPLL0,
> -					     DPLL_ID_SKL_DPLL0);
> +					     BIT(DPLL_ID_SKL_DPLL0));
> 	else
> 		pll = intel_find_shared_dpll(state, crtc,
> 					     &crtc_state->dpll_hw_state,
> -					     DPLL_ID_SKL_DPLL1,
> -					     DPLL_ID_SKL_DPLL3);
> +					     GENMASK(DPLL_ID_SKL_DPLL3,
> +						      DPLL_ID_SKL_DPLL1));
> 	if (!pll)
> 		return false;
>
> @@ -2401,8 +2401,8 @@ static bool cnl_get_dpll(struct intel_atomic_state *state,
>
> 	pll = intel_find_shared_dpll(state, crtc,
> 				     &crtc_state->dpll_hw_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 false;
> @@ -2975,7 +2975,7 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> 		&crtc_state->icl_port_dplls[ICL_PORT_DPLL_DEFAULT];
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> 	enum port port = encoder->port;
> -	bool has_dpll4 = false;
> +	unsigned dpll_mask;
>
> 	if (!icl_calc_dpll_state(crtc_state, encoder, &port_dpll->hw_state)) {
> 		DRM_DEBUG_KMS("Could not calculate combo PHY PLL state.\n");
> @@ -2984,13 +2984,13 @@ static bool icl_get_combo_phy_dpll(struct intel_atomic_state *state,
> 	}
>
> 	if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
> -		has_dpll4 = true;
> +		dpll_mask = GENMASK(DPLL_ID_EHL_DPLL4, DPLL_ID_ICL_DPLL0);
> +	else
> +		dpll_mask = GENMASK(DPLL_ID_ICL_DPLL1, DPLL_ID_ICL_DPLL0);
>
> 	port_dpll->pll = intel_find_shared_dpll(state, crtc,
> 						&port_dpll->hw_state,
> -						DPLL_ID_ICL_DPLL0,
> -						has_dpll4 ? DPLL_ID_EHL_DPLL4
> -							  : DPLL_ID_ICL_DPLL1);
> +						dpll_mask);
> 	if (!port_dpll->pll) {
> 		DRM_DEBUG_KMS("No combo PHY PLL found for [ENCODER:%d:%s]\n",
> 			      encoder->base.base.id, encoder->base.name);
> @@ -3023,8 +3023,7 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
>
> 	port_dpll->pll = intel_find_shared_dpll(state, crtc,
> 						&port_dpll->hw_state,
> -						DPLL_ID_ICL_TBTPLL,
> -						DPLL_ID_ICL_TBTPLL);
> +						BIT(DPLL_ID_ICL_TBTPLL));
> 	if (!port_dpll->pll) {
> 		DRM_DEBUG_KMS("No TBT-ALT PLL found\n");
> 		return false;
> @@ -3043,8 +3042,7 @@ static bool icl_get_tc_phy_dplls(struct intel_atomic_state *state,
> 							 encoder->port));
> 	port_dpll->pll = intel_find_shared_dpll(state, crtc,
> 						&port_dpll->hw_state,
> -						dpll_id,
> -						dpll_id);
> +						BIT(dpll_id));
> 	if (!port_dpll->pll) {
> 		DRM_DEBUG_KMS("No MG PHY PLL found\n");
> 		goto err_unreference_tbt_pll;
> --
> 2.21.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx