Re: [PATCH v5 2/5] drm/i915/gen11: Program DPCLKA_CFGCR0_ICL according to PHY

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

 



On Wed, 2019-07-03 at 18:06 -0700, Matt Roper wrote:
> Although the register name implies that it operates on DDI's,
> DPCLKA_CFGCR0_ICL actually needs to be programmed according to the
> PHY
> that's in use.  I.e., when using EHL's DDI-D on combo PHY A, the bits
> described as "port A" in the bspec are what we need to set.  The
> bspec
> clarifies:
> 
>         "[For EHL] DDID clock tied to DDIA clock, so DPCLKA_CFGCR0
> DDIA
>         Clock Select chooses the PLL for both DDIA and DDID and
> drives
>         port A in all cases."
> 
> Also, since the CNL DPCLKA_CFGCR0 bit defines are still port-based,
> we
> create separate ICL-specific defines that accept the PHY rather than
> trying to share the same bit definitions between CNL and ICL.
> 

Nit: Why not already rename DPCLKA_CFGCR0_ICL to ICL_DPCLKA_CFGCR0? The
bits have the new name, so you are already touching in everyplace that
uses DPCLKA_CFGCR0_ICL.

> v5: Make icl_dpclka_cfgcr0_clk_off() take phy rather than port.  When
>     splitting the original patch the hunk to handle this wound up too
>     late in the series.  (Sparse)

Reviewed-by: José Roberto de Souza <jose.souza@xxxxxxxxx>

> 
> Bspec: 33148
> Cc: José Roberto de Souza <jose.souza@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c   | 17 ++++++---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 47 +++++++++++++++-------
> --
>  drivers/gpu/drm/i915/i915_reg.h          | 12 ++++--
>  3 files changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index b8673debf932..f574af62888c 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -560,11 +560,13 @@ static void gen11_dsi_gate_clocks(struct
> intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	u32 tmp;
>  	enum port port;
> +	enum phy phy;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> +		phy = intel_port_to_phy(dev_priv, port);
> +		tmp |= ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
>  	}
>  
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
> @@ -577,11 +579,13 @@ static void gen11_dsi_ungate_clocks(struct
> intel_encoder *encoder)
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	u32 tmp;
>  	enum port port;
> +	enum phy phy;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		tmp &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> +		phy = intel_port_to_phy(dev_priv, port);
> +		tmp &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
>  	}
>  
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
> @@ -595,19 +599,22 @@ static void gen11_dsi_map_pll(struct
> intel_encoder *encoder,
>  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
>  	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>  	enum port port;
> +	enum phy phy;
>  	u32 val;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  
>  	val = I915_READ(DPCLKA_CFGCR0_ICL);
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
> -		val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port);
> +		phy = intel_port_to_phy(dev_priv, port);
> +		val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> +		val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id,
> phy);
>  	}
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  
>  	for_each_dsi_port(port, intel_dsi->ports) {
> -		val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> +		phy = intel_port_to_phy(dev_priv, port);
> +		val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
>  	}
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a4172595c8d8..065feb917db4 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2729,12 +2729,13 @@ u32 ddi_signal_levels(struct intel_dp
> *intel_dp)
>  
>  static inline
>  u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
> -			      enum port port)
> +			      enum phy phy)
>  {
> -	if (intel_port_is_combophy(dev_priv, port)) {
> -		return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> -	} else if (intel_port_is_tc(dev_priv, port)) {
> -		enum tc_port tc_port = intel_port_to_tc(dev_priv,
> port);
> +	if (intel_phy_is_combo(dev_priv, phy)) {
> +		return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy);
> +	} else if (intel_phy_is_tc(dev_priv, phy)) {
> +		enum tc_port tc_port = intel_port_to_tc(dev_priv,
> +							(enum
> port)phy);
>  
>  		return ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port);
>  	}
> @@ -2747,22 +2748,32 @@ static void icl_map_plls_to_ports(struct
> intel_encoder *encoder,
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	struct intel_shared_dpll *pll = crtc_state->shared_dpll;
> -	enum port port = encoder->port;
> +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>  	u32 val;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  
>  	val = I915_READ(DPCLKA_CFGCR0_ICL);
> -	WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)) ==
> 0);
> +	WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0);
>  
> -	if (intel_port_is_combophy(dev_priv, port)) {
> -		val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port);
> -		val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port);
> +	if (intel_phy_is_combo(dev_priv, phy)) {
> +		/*
> +		 * Even though this register references DDIs, note that
> we
> +		 * want to pass the PHY rather than the port
> (DDI).  For
> +		 * ICL, port=phy in all cases so it doesn't matter, but
> for
> +		 * EHL the bspec notes the following:
> +		 *
> +		 *   "DDID clock tied to DDIA clock, so DPCLKA_CFGCR0
> DDIA
> +		 *   Clock Select chooses the PLL for both DDIA and
> DDID and
> +		 *   drives port A in all cases."
> +		 */
> +		val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
> +		val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id,
> phy);
>  		I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  		POSTING_READ(DPCLKA_CFGCR0_ICL);
>  	}
>  
> -	val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, port);
> +	val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, phy);
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  
>  	mutex_unlock(&dev_priv->dpll_lock);
> @@ -2771,13 +2782,13 @@ static void icl_map_plls_to_ports(struct
> intel_encoder *encoder,
>  static void icl_unmap_plls_to_ports(struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum port port = encoder->port;
> +	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>  	u32 val;
>  
>  	mutex_lock(&dev_priv->dpll_lock);
>  
>  	val = I915_READ(DPCLKA_CFGCR0_ICL);
> -	val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port);
> +	val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy);
>  	I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  
>  	mutex_unlock(&dev_priv->dpll_lock);
> @@ -2838,9 +2849,11 @@ void icl_sanitize_encoder_pll_mapping(struct
> intel_encoder *encoder)
>  
>  	val = I915_READ(DPCLKA_CFGCR0_ICL);
>  	for_each_port_masked(port, port_mask) {
> +		enum phy phy = intel_port_to_phy(dev_priv, port);
> +
>  		bool ddi_clk_ungated = !(val &
>  					 icl_dpclka_cfgcr0_clk_off(dev_
> priv,
> -								   port
> ));
> +								   phy)
> );
>  
>  		if (ddi_clk_needed == ddi_clk_ungated)
>  			continue;
> @@ -2852,9 +2865,9 @@ void icl_sanitize_encoder_pll_mapping(struct
> intel_encoder *encoder)
>  		if (WARN_ON(ddi_clk_needed))
>  			continue;
>  
> -		DRM_NOTE("Port %c is disabled/in DSI mode with an
> ungated DDI clock, gate it\n",
> -			 port_name(port));
> -		val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port);
> +		DRM_NOTE("PHY %c is disabled/in DSI mode with an
> ungated DDI clock, gate it\n",
> +			 phy_name(port));
> +		val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy);
>  		I915_WRITE(DPCLKA_CFGCR0_ICL, val);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index c814cc1b3ae5..c9e2e09b6f01 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9680,17 +9680,21 @@ enum skl_power_gate {
>   * CNL Clocks
>   */
>  #define DPCLKA_CFGCR0				_MMIO(0x6C200)
> -#define DPCLKA_CFGCR0_ICL			_MMIO(0x164280)
>  #define  DPCLKA_CFGCR0_DDI_CLK_OFF(port)	(1 << ((port)
> ==  PORT_F ? 23 : \
>  						      (port) + 10))
> -#define  ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port)   (1 << ((port) + 10))
> -#define  ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) ==
> PORT_TC4 ? \
> -						      21 : (tc_port) +
> 12))
>  #define  DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)	((port) ==
> PORT_F ? 21 : \
>  						(port) * 2)
>  #define  DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port)	(3 <<
> DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port))
>  #define  DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port)	((pll) <<
> DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port))
>  
> +#define DPCLKA_CFGCR0_ICL			_MMIO(0x164280)
> +#define  ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy)	(1 << _PICK(phy, 10,
> 11, 24))
> +#define  ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) ==
> PORT_TC4 ? \
> +						      21 : (tc_port) +
> 12))
> +#define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)	((phy) * 2)
> +#define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy)	(3 <<
> ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> +#define  ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy)	((pll) <<
> ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
> +
>  /* CNL PLL */
>  #define DPLL0_ENABLE		0x46010
>  #define DPLL1_ENABLE		0x46014
_______________________________________________
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