Re: [PATCH 02/22] drm/i915/tgl: select correct bit for port select

["Atwood, Matthew S" <matthew.s.atwood@intel.com>]

On Fri, 2019-07-12 at 18:09 -0700, Lucas De Marchi wrote:
> From: Mahesh Kumar <mahesh1.kumar@intel.com>
> 
> Bit definitions for port-select got changed for TRANS_CLK_SEL &
> TRANS_DDI_FUNC_CTL registers in TGL.
> 
> v2 (Lucas):
>   - Nuke TRANS_DDI_PORT_NONE since it's 0: we are already clearing
>     {TGL_,}TRANS_DDI_PORT_MASK (suggested by Ville)
>   - Also cover haswell_get_ddi_port_state() in intel_display.c that
> was
>     missing
>   - Define macros using the _SHIFT macros so we don't lose other
> users
> 
Reviewed-by: Matt Atwood <matthew.s.atwood@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c     | 47 +++++++++++++++---
> --
>  drivers/gpu/drm/i915/display/intel_display.c |  6 ++-
>  drivers/gpu/drm/i915/i915_reg.h              | 11 +++--
>  3 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 8445244aa593..339c01e567ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -1773,7 +1773,10 @@ void intel_ddi_enable_transcoder_func(const
> struct intel_crtc_state *crtc_state)
>  
>  	/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode
> */
>  	temp = TRANS_DDI_FUNC_ENABLE;
> -	temp |= TRANS_DDI_SELECT_PORT(port);
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		temp |= TGL_TRANS_DDI_SELECT_PORT(port);
> +	else
> +		temp |= TRANS_DDI_SELECT_PORT(port);
>  
>  	switch (crtc_state->pipe_bpp) {
>  	case 18:
> @@ -1853,8 +1856,13 @@ void intel_ddi_disable_transcoder_func(const
> struct intel_crtc_state *crtc_state
>  	i915_reg_t reg = TRANS_DDI_FUNC_CTL(cpu_transcoder);
>  	u32 val = I915_READ(reg);
>  
> -	val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> -	val |= TRANS_DDI_PORT_NONE;
> +	if (INTEL_GEN(dev_priv) >= 12) {
> +		val &= ~(TRANS_DDI_FUNC_ENABLE |
> TGL_TRANS_DDI_PORT_MASK |
> +			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> +	} else {
> +		val &= ~(TRANS_DDI_FUNC_ENABLE | TRANS_DDI_PORT_MASK |
> +			 TRANS_DDI_DP_VC_PAYLOAD_ALLOC);
> +	}
>  	I915_WRITE(reg, val);
>  
>  	if (dev_priv->quirks & QUIRK_INCREASE_DDI_DISABLED_TIME &&
> @@ -2006,10 +2014,19 @@ static void
> intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
>  	mst_pipe_mask = 0;
>  	for_each_pipe(dev_priv, p) {
>  		enum transcoder cpu_transcoder = (enum transcoder)p;
> +		unsigned int port_mask, ddi_select;
> +
> +		if (INTEL_GEN(dev_priv) >= 12) {
> +			port_mask = TGL_TRANS_DDI_PORT_MASK;
> +			ddi_select = TGL_TRANS_DDI_SELECT_PORT(port);
> +		} else {
> +			port_mask = TRANS_DDI_PORT_MASK;
> +			ddi_select = TRANS_DDI_SELECT_PORT(port);
> +		}
>  
>  		tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  
> -		if ((tmp & TRANS_DDI_PORT_MASK) !=
> TRANS_DDI_SELECT_PORT(port))
> +		if ((tmp & port_mask) != ddi_select)
>  			continue;
>  
>  		if ((tmp & TRANS_DDI_MODE_SELECT_MASK) ==
> @@ -2126,9 +2143,14 @@ void intel_ddi_enable_pipe_clock(const struct
> intel_crtc_state *crtc_state)
>  	enum port port = encoder->port;
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> -	if (cpu_transcoder != TRANSCODER_EDP)
> -		I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> -			   TRANS_CLK_SEL_PORT(port));
> +	if (cpu_transcoder != TRANSCODER_EDP) {
> +		if (INTEL_GEN(dev_priv) >= 12)
> +			I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> +				   TGL_TRANS_CLK_SEL_PORT(port));
> +		else
> +			I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> +				   TRANS_CLK_SEL_PORT(port));
> +	}
>  }
>  
>  void intel_ddi_disable_pipe_clock(const struct intel_crtc_state
> *crtc_state)
> @@ -2136,9 +2158,14 @@ void intel_ddi_disable_pipe_clock(const struct
> intel_crtc_state *crtc_state)
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state-
> >base.crtc->dev);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  
> -	if (cpu_transcoder != TRANSCODER_EDP)
> -		I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> -			   TRANS_CLK_SEL_DISABLED);
> +	if (cpu_transcoder != TRANSCODER_EDP) {
> +		if (INTEL_GEN(dev_priv) >= 12)
> +			I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> +				   TGL_TRANS_CLK_SEL_DISABLED);
> +		else
> +			I915_WRITE(TRANS_CLK_SEL(cpu_transcoder),
> +				   TRANS_CLK_SEL_DISABLED);
> +	}
>  }
>  
>  static void _skl_ddi_set_iboost(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index e25b82d07d4f..51e4f6798a6b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10353,7 +10353,11 @@ static void
> haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  
>  	tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe_config-
> >cpu_transcoder));
>  
> -	port = (tmp & TRANS_DDI_PORT_MASK) >> TRANS_DDI_PORT_SHIFT;
> +	if (INTEL_GEN(dev_priv) >= 12)
> +		port = (tmp & TGL_TRANS_DDI_PORT_MASK) >>
> +			TGL_TRANS_DDI_PORT_SHIFT;
> +	else
> +		port = (tmp & TRANS_DDI_PORT_MASK) >>
> TRANS_DDI_PORT_SHIFT;
>  
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		icelake_get_ddi_pll(dev_priv, port, pipe_config);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 0dd4506323f2..def71fd2e4d1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9391,10 +9391,12 @@ enum skl_power_gate {
>  
>  #define  TRANS_DDI_FUNC_ENABLE		(1 << 31)
>  /* Those bits are ignored by pipe EDP since it can only connect to
> DDI A */
> -#define  TRANS_DDI_PORT_MASK		(7 << 28)
>  #define  TRANS_DDI_PORT_SHIFT		28
> -#define  TRANS_DDI_SELECT_PORT(x)	((x) << 28)
> -#define  TRANS_DDI_PORT_NONE		(0 << 28)
> +#define  TGL_TRANS_DDI_PORT_SHIFT	27
> +#define  TRANS_DDI_PORT_MASK		(7 << TRANS_DDI_PORT_SHIFT)
> +#define  TGL_TRANS_DDI_PORT_MASK	(0xf <<
> TGL_TRANS_DDI_PORT_SHIFT)
> +#define  TRANS_DDI_SELECT_PORT(x)	((x) << TRANS_DDI_PORT_SHIFT)
> +#define  TGL_TRANS_DDI_SELECT_PORT(x)	(((x) + 1) <<
> TGL_TRANS_DDI_PORT_SHIFT)
>  #define  TRANS_DDI_MODE_SELECT_MASK	(7 << 24)
>  #define  TRANS_DDI_MODE_SELECT_HDMI	(0 << 24)
>  #define  TRANS_DDI_MODE_SELECT_DVI	(1 << 24)
> @@ -9604,6 +9606,9 @@ enum skl_power_gate {
>  /* For each transcoder, we need to select the corresponding port
> clock */
>  #define  TRANS_CLK_SEL_DISABLED		(0x0 << 29)
>  #define  TRANS_CLK_SEL_PORT(x)		(((x) + 1) << 29)
> +#define  TGL_TRANS_CLK_SEL_DISABLED	(0x0 << 28)
> +#define  TGL_TRANS_CLK_SEL_PORT(x)	(((x) + 1) << 28)
> +
>  
>  #define CDCLK_FREQ			_MMIO(0x46200)
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx