[02/23] drm/i915: Tune down WARNs about TBT AUX power well enabling

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

Details

Message ID 20190604145826.16424-3-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.
The HW completion flag for the TBT AUX power well enabling/disabling
gets stuck if the firmware tears down the TBT DP tunnel before the
completion.

We shouldn't complain about the timeout, since it's expected to happen
and doesn't cause further issues. We suppress the disabling timeout
already, do the same for enabling.

Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display_power.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 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 278a7edc94f5..249a5fa55df6 100644
--- a/drivers/gpu/drm/i915/intel_display_power.c
+++ b/drivers/gpu/drm/i915/intel_display_power.c
@@ -268,11 +268,16 @@  static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
 	int pw_idx = power_well->desc->hsw.idx;
 
 	/* Timeout for PW1:10 us, AUX:not specified, other PWs:20 us. */
-	WARN_ON(intel_wait_for_register(&dev_priv->uncore,
-					regs->driver,
-					HSW_PWR_WELL_CTL_STATE(pw_idx),
-					HSW_PWR_WELL_CTL_STATE(pw_idx),
-					1));
+	if (intel_wait_for_register(&dev_priv->uncore,
+				    regs->driver,
+				    HSW_PWR_WELL_CTL_STATE(pw_idx),
+				    HSW_PWR_WELL_CTL_STATE(pw_idx),
+				    1)) {
+		DRM_DEBUG_KMS("%s forced off\n", power_well->desc->name);
+
+		/* An AUX timeout is expected if the TBT DP tunnel is down. */
+		WARN_ON(!power_well->desc->hsw.is_tc_tbt);
+	}
 }
 
 static u32 hsw_power_well_requesters(struct drm_i915_private *dev_priv,

Comments

On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> The HW completion flag for the TBT AUX power well enabling/disabling

> gets stuck if the firmware tears down the TBT DP tunnel before the

> completion.

> 

> We shouldn't complain about the timeout, since it's expected to

> happen

> and doesn't cause further issues. We suppress the disabling timeout

> already, do the same for enabling.


This was documented in spec?

> 

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

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

> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

> ---

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

>  1 file changed, 10 insertions(+), 5 deletions(-)

> 

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

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

> index 278a7edc94f5..249a5fa55df6 100644

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

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

> @@ -268,11 +268,16 @@ static void

> hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,

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

>  

>  	/* Timeout for PW1:10 us, AUX:not specified, other PWs:20 us.

> */

> -	WARN_ON(intel_wait_for_register(&dev_priv->uncore,

> -					regs->driver,

> -					HSW_PWR_WELL_CTL_STATE(pw_idx),

> -					HSW_PWR_WELL_CTL_STATE(pw_idx),

> -					1));

> +	if (intel_wait_for_register(&dev_priv->uncore,

> +				    regs->driver,

> +				    HSW_PWR_WELL_CTL_STATE(pw_idx),

> +				    HSW_PWR_WELL_CTL_STATE(pw_idx),

> +				    1)) {

> +		DRM_DEBUG_KMS("%s forced off\n", power_well->desc-

> >name);


Maybe "%s power well enable timeout"?
Anyways we can fix that latter.

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


> +

> +		/* An AUX timeout is expected if the TBT DP tunnel is

> down. */

> +		WARN_ON(!power_well->desc->hsw.is_tc_tbt);

> +	}

>  }

>  

>  static u32 hsw_power_well_requesters(struct drm_i915_private

> *dev_priv,
On Fri, Jun 07, 2019 at 08:50:34PM +0300, Souza, Jose wrote:
> On Tue, 2019-06-04 at 17:58 +0300, Imre Deak wrote:
> > The HW completion flag for the TBT AUX power well enabling/disabling
> > gets stuck if the firmware tears down the TBT DP tunnel before the
> > completion.
> > 
> > We shouldn't complain about the timeout, since it's expected to
> > happen
> > and doesn't cause further issues. We suppress the disabling timeout
> > already, do the same for enabling.
> 
> This was documented in spec?
> 
> > 
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display_power.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display_power.c
> > b/drivers/gpu/drm/i915/intel_display_power.c
> > index 278a7edc94f5..249a5fa55df6 100644
> > --- a/drivers/gpu/drm/i915/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/intel_display_power.c
> > @@ -268,11 +268,16 @@ static void
> > hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
> >  	int pw_idx = power_well->desc->hsw.idx;
> >  
> >  	/* Timeout for PW1:10 us, AUX:not specified, other PWs:20 us.
> > */
> > -	WARN_ON(intel_wait_for_register(&dev_priv->uncore,
> > -					regs->driver,
> > -					HSW_PWR_WELL_CTL_STATE(pw_idx),
> > -					HSW_PWR_WELL_CTL_STATE(pw_idx),
> > -					1));
> > +	if (intel_wait_for_register(&dev_priv->uncore,
> > +				    regs->driver,
> > +				    HSW_PWR_WELL_CTL_STATE(pw_idx),
> > +				    HSW_PWR_WELL_CTL_STATE(pw_idx),
> > +				    1)) {
> > +		DRM_DEBUG_KMS("%s forced off\n", power_well->desc-
> > >name);
> 
> Maybe "%s power well enable timeout"?

Yep, makes sense, will change this.

> Anyways we can fix that latter.
> 
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> 
> > +
> > +		/* An AUX timeout is expected if the TBT DP tunnel is
> > down. */
> > +		WARN_ON(!power_well->desc->hsw.is_tc_tbt);
> > +	}
> >  }
> >  
> >  static u32 hsw_power_well_requesters(struct drm_i915_private
> > *dev_priv,