[06/23] drm/i915: Fix the TBT AUX power well enabling

Submitted by Imre Deak on June 4, 2019, 2:58 p.m.

Details

Message ID 20190604145826.16424-7-imre.deak@intel.com
State New
Headers show
Series "drm/i915: Fix TypeC port mode switching" ( rev: 2 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Imre Deak June 4, 2019, 2:58 p.m.
Fix the mapping from a TBT AUX power well index to the DP_AUX_CH_CTL
register.

Fixes: c7375d9542f1 ("drm/i915: Configure AUX_CH_CTL when enabling the AUX power domain")
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display_power.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/intel_display_power.c b/drivers/gpu/drm/i915/intel_display_power.c
index 249a5fa55df6..14cf04bf0bf4 100644
--- a/drivers/gpu/drm/i915/intel_display_power.c
+++ b/drivers/gpu/drm/i915/intel_display_power.c
@@ -442,16 +442,23 @@  icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
 #define ICL_AUX_PW_TO_CH(pw_idx)	\
 	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
 
+#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
+	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
+
 static void
 icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
 				 struct i915_power_well *power_well)
 {
-	enum aux_ch aux_ch = ICL_AUX_PW_TO_CH(power_well->desc->hsw.idx);
+	int pw_idx = power_well->desc->hsw.idx;
+	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
+	enum aux_ch aux_ch;
 	u32 val;
 
+	aux_ch = is_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
+			  ICL_AUX_PW_TO_CH(pw_idx);
 	val = I915_READ(DP_AUX_CH_CTL(aux_ch));
 	val &= ~DP_AUX_CH_CTL_TBT_IO;
-	if (power_well->desc->hsw.is_tc_tbt)
+	if (is_tbt)
 		val |= DP_AUX_CH_CTL_TBT_IO;
 	I915_WRITE(DP_AUX_CH_CTL(aux_ch), val);
 

Comments

On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> Fix the mapping from a TBT AUX power well index to the DP_AUX_CH_CTL

> register.

> 

> Fixes: c7375d9542f1 ("drm/i915: Configure AUX_CH_CTL when enabling

> the AUX power domain")

> Cc: José Roberto de Souza <jose.souza@intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Imre Deak <imre.deak@intel.com>

> ---

>  drivers/gpu/drm/i915/intel_display_power.c | 11 +++++++++--

>  1 file changed, 9 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/gpu/drm/i915/intel_display_power.c

> b/drivers/gpu/drm/i915/intel_display_power.c

> index 249a5fa55df6..14cf04bf0bf4 100644

> --- a/drivers/gpu/drm/i915/intel_display_power.c

> +++ b/drivers/gpu/drm/i915/intel_display_power.c

> @@ -442,16 +442,23 @@ icl_combo_phy_aux_power_well_disable(struct

> drm_i915_private *dev_priv,

>  #define ICL_AUX_PW_TO_CH(pw_idx)	\

>  	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)

>  

> +#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\

> +	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)

> +

>  static void

>  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,

>  				 struct i915_power_well *power_well)

>  {

> -	enum aux_ch aux_ch = ICL_AUX_PW_TO_CH(power_well->desc-

> >hsw.idx);

> +	int pw_idx = power_well->desc->hsw.idx;

> +	bool is_tbt = power_well->desc->hsw.is_tc_tbt;

> +	enum aux_ch aux_ch;

>  	u32 val;

>  

> +	aux_ch = is_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :

> +			  ICL_AUX_PW_TO_CH(pw_idx);


Matches

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


>  	val = I915_READ(DP_AUX_CH_CTL(aux_ch));

>  	val &= ~DP_AUX_CH_CTL_TBT_IO;

> -	if (power_well->desc->hsw.is_tc_tbt)

> +	if (is_tbt)

>  		val |= DP_AUX_CH_CTL_TBT_IO;


So this register needs to be set before the aux transaction?

>  	I915_WRITE(DP_AUX_CH_CTL(aux_ch), val);

>
On Fri, Jun 07, 2019 at 10:58:41PM +0300, Souza, Jose wrote:
> On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > Fix the mapping from a TBT AUX power well index to the DP_AUX_CH_CTL
> > register.
> > 
> > Fixes: c7375d9542f1 ("drm/i915: Configure AUX_CH_CTL when enabling
> > the AUX power domain")
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display_power.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display_power.c
> > b/drivers/gpu/drm/i915/intel_display_power.c
> > index 249a5fa55df6..14cf04bf0bf4 100644
> > --- a/drivers/gpu/drm/i915/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/intel_display_power.c
> > @@ -442,16 +442,23 @@ icl_combo_phy_aux_power_well_disable(struct
> > drm_i915_private *dev_priv,
> >  #define ICL_AUX_PW_TO_CH(pw_idx)	\
> >  	((pw_idx) - ICL_PW_CTL_IDX_AUX_A + AUX_CH_A)
> >  
> > +#define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
> > +	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
> > +
> >  static void
> >  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
> >  				 struct i915_power_well *power_well)
> >  {
> > -	enum aux_ch aux_ch = ICL_AUX_PW_TO_CH(power_well->desc-
> > >hsw.idx);
> > +	int pw_idx = power_well->desc->hsw.idx;
> > +	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> > +	enum aux_ch aux_ch;
> >  	u32 val;
> >  
> > +	aux_ch = is_tbt ? ICL_TBT_AUX_PW_TO_CH(pw_idx) :
> > +			  ICL_AUX_PW_TO_CH(pw_idx);
> 
> Matches
> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> >  	val = I915_READ(DP_AUX_CH_CTL(aux_ch));
> >  	val &= ~DP_AUX_CH_CTL_TBT_IO;
> > -	if (power_well->desc->hsw.is_tc_tbt)
> > +	if (is_tbt)
> >  		val |= DP_AUX_CH_CTL_TBT_IO;
> 
> So this register needs to be set before the aux transaction?

Not only, based on bspec TypeC Programming (Index/21750) it needs to be
set as part of the connect sequence; so whenever you need to enable this
power well.

> 
> >  	I915_WRITE(DP_AUX_CH_CTL(aux_ch), val);
> >