Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v8)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 21, 2019 at 07:00:26PM -0700, Vivek Kasireddy wrote:
> This patch adds support for DPLL4 on EHL that include the
> following restrictions:
> 
> - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
>   DPLL4 can be used with other DDIs, including DDID
>   (combo port A external usage).
> 
> - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> 
> - The DPLL4 enable, lock, power enabled, and power state are connected
>   to the MGPLL1_ENABLE register.
> 
> v2: (suggestions from Bob Paauwe)
> - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
>   iterate twice: once for Combo plls and once for MG plls.
> 
> - Use MG pll funcs for DPLL4 instead of creating new ones and modify
>   mg_pll_enable to include the restrictions for EHL.
> 
> v3: Fix compilation error
> 
> v4: (suggestions from Lucas and Ville)
> - Treat DPLL4 as a combo phy PLL and not as MG PLL
> - Disable DC states when this DPLL is being enabled
> - Reuse icl_get_dpll instead of creating a separate one for EHL
> 
> v5: (suggestion from Ville)
> - Refcount the DC OFF power domains during the enabling and disabling
>   of this DPLL.
> 
> v6: rebase
> 
> v7: (suggestion from Imre)
> - Add a new power domain instead of iterating over the domains
>   assoicated with DC OFF power well.
> 
> v8: (Ville and Imre)
> - Rename POWER_DOMAIN_DPLL4 TO POWER_DOMAIN_DPLL_DC_OFF
> - Grab a reference in intel_modeset_setup_hw_state() if this
>   DPLL was already enabled perhaps by BIOS.
> - Check for the port type instead of the encoder
> 
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  6 +++
>  .../drm/i915/display/intel_display_power.c    |  3 ++
>  .../drm/i915/display/intel_display_power.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 42 +++++++++++++++++--
>  drivers/gpu/drm/i915/display/intel_dpll_mgr.h |  6 +++
>  5 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8592a7d422de..a5f387e486ee 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -16778,6 +16778,12 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	for (i = 0; i < dev_priv->num_shared_dpll; i++) {
>  		struct intel_shared_dpll *pll = &dev_priv->shared_dplls[i];
>  
> +		if (IS_ELKHARTLAKE(dev_priv) && pll->on &&
> +		    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +			pll->wakeref = intel_display_power_get(dev_priv,
> +							       POWER_DOMAIN_DPLL_DC_OFF);
> +		}

I think this a bit too late. We should do it before
intel_sanitize_crtc() as that may already call
intel_disable_shared_dpll().

> +
>  		if (!pll->on || pll->active_mask)
>  			continue;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index c93ad512014c..1c101a842331 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -117,6 +117,8 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "MODESET";
>  	case POWER_DOMAIN_GT_IRQ:
>  		return "GT_IRQ";
> +	case POWER_DOMAIN_DPLL_DC_OFF:
> +		return "DPLL_DC_OFF";
>  	default:
>  		MISSING_CASE(domain);
>  		return "?";
> @@ -2361,6 +2363,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
>  	ICL_PW_2_POWER_DOMAINS |			\
>  	BIT_ULL(POWER_DOMAIN_MODESET) |			\
>  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> +	BIT_ULL(POWER_DOMAIN_DPLL_DC_OFF) |			\
>  	BIT_ULL(POWER_DOMAIN_INIT))
>  
>  #define ICL_DDI_IO_A_POWER_DOMAINS (			\
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> index ff57b0a7fe59..8f43f7051a16 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> @@ -59,6 +59,7 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_GMBUS,
>  	POWER_DOMAIN_MODESET,
>  	POWER_DOMAIN_GT_IRQ,
> +	POWER_DOMAIN_DPLL_DC_OFF,
>  	POWER_DOMAIN_INIT,
>  
>  	POWER_DOMAIN_NUM,
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> index 2d4e7b9a7b9d..81e1443cb583 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
> @@ -2806,6 +2806,10 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
>  	if (intel_port_is_combophy(dev_priv, port)) {
>  		min = DPLL_ID_ICL_DPLL0;
>  		max = DPLL_ID_ICL_DPLL1;
> +
> +		if (IS_ELKHARTLAKE(dev_priv) && port != PORT_A)
> +			max = DPLL_ID_EHL_DPLL4;
> +
>  		ret = icl_calc_dpll_state(crtc_state, encoder);
>  	} else if (intel_port_is_tc(dev_priv, port)) {
>  		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> @@ -2945,8 +2949,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> -				    CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +	}
> +
> +	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
>  
>  static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
> @@ -3057,6 +3067,19 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  {
>  	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>  
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +
> +		/*
> +		 * We need to disable DC states when this DPLL is enabled.
> +		 * This can be done by taking a reference on DPLL4 power
> +		 * domain.
> +		 */
> +		pll->wakeref = intel_display_power_get(dev_priv,
> +						       POWER_DOMAIN_DPLL_DC_OFF);
> +	}
> +
>  	icl_pll_power_enable(dev_priv, pll, enable_reg);
>  
>  	icl_dpll_write(dev_priv, pll);
> @@ -3152,7 +3175,19 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +		icl_pll_disable(dev_priv, pll, enable_reg);
> +
> +		intel_display_power_put(dev_priv, POWER_DOMAIN_DPLL_DC_OFF,
> +					pll->wakeref);
> +		return;
> +	}
> +
> +	icl_pll_disable(dev_priv, pll, enable_reg);
>  }
>  
>  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> @@ -3230,6 +3265,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
>  static const struct dpll_info ehl_plls[] = {
>  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> +	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
>  	{ },
>  };
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> index d0570414f3d1..3e2aa1099562 100644
> --- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
> @@ -28,6 +28,7 @@
>  #include <linux/types.h>
>  
>  #include "intel_display.h"
> +#include "intel_wakeref.h"
>  
>  /*FIXME: Move this to a more appropriate place. */
>  #define abs_diff(a, b) ({			\
> @@ -117,6 +118,10 @@ enum intel_dpll_id {
>  	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
>  	 */
>  	DPLL_ID_ICL_DPLL1 = 1,
> +	/**
> +	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
> +	 */
> +	DPLL_ID_EHL_DPLL4 = 2,
>  	/**
>  	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
>  	 */
> @@ -312,6 +317,7 @@ struct intel_shared_dpll {
>  	 * @info: platform specific info
>  	 */
>  	const struct dpll_info *info;
> +	intel_wakeref_t wakeref;
>  };
>  
>  #define SKL_DPLL0 0
> -- 
> 2.21.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux