[Intel-gfx] [PATCH v2 1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Souza, Jose]

On Fri, 2018-09-14 at 08:37 -0700, Rodrigo Vivi wrote:
> On Fri, Sep 14, 2018 at 05:52:39PM +0300, Ville Syrjälä wrote:
> > On Fri, Sep 14, 2018 at 07:18:44AM -0700, José Roberto de Souza
> > wrote:
> > > Instead of have the same code spread into 4 platforms lets share
> > > it.
> > > BXT do not have a PCH so here also handling this case by unseting
> > > RESET_PCH_HANDSHAKE_ENABLE.
> > > 
> > > v2(Rodrigo):
> > > - renamed to intel_pch_reset_handshake()
> > > - added comment about why BXT need the bit to be unset
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 36 ++++++++++++-------
> > > ------
> > >  1 file changed, 17 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 9bebec389de1..1bcd0e51fca1 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -3239,18 +3239,29 @@ static void icl_mbus_init(struct
> > > drm_i915_private *dev_priv)
> > >  	I915_WRITE(MBUS_ABOX_CTL, val);
> > >  }
> > >  
> > > +static void intel_pch_reset_handshake(struct drm_i915_private
> > > *dev_priv)
> > > +{
> > > +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > +
> > > +	/* BXT don't have PCH and it requires that this bit is always
> > > unset */
> > > +	if (HAS_PCH_SPLIT(dev_priv))
> > 
> > Still would prefer 'bool enable' etc. rather than this magic
> > inside.
> 
> I agree. So we could keep the original BXT comment where it is...

Several other places also uses HAS_PCH_SPLIT() to differentiate BXT of
other platforms so it is not something new, also adding the 'bool
enable' would move part of the "magic" to the callers as we need to
check if HAS_PCH_NOP(dev_priv) is true in the next patch and unset the
handshake.

> 
> > 
> > > +		val |= RESET_PCH_HANDSHAKE_ENABLE;
> > > +	else
> > > +		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> > > +
> > > +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> > > +}
> > > +
> > >  static void skl_display_core_init(struct drm_i915_private
> > > *dev_priv,
> > >  				   bool resume)
> > >  {
> > >  	struct i915_power_domains *power_domains = &dev_priv-
> > > >power_domains;
> > >  	struct i915_power_well *well;
> > > -	uint32_t val;
> > >  
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  
> > >  	/* enable PCH reset handshake */
> > > -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val |
> > > RESET_PCH_HANDSHAKE_ENABLE);
> > > +	intel_pch_reset_handshake(dev_priv);
> > >  
> > >  	/* enable PG1 and Misc I/O */
> > >  	mutex_lock(&power_domains->lock);
> > > @@ -3306,19 +3317,10 @@ void bxt_display_core_init(struct
> > > drm_i915_private *dev_priv,
> > >  {
> > >  	struct i915_power_domains *power_domains = &dev_priv-
> > > >power_domains;
> > >  	struct i915_power_well *well;
> > > -	uint32_t val;
> > >  
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  
> > > -	/*
> > > -	 * NDE_RSTWRN_OPT RST PCH Handshake En must always be 0b on BXT
> > > -	 * or else the reset will hang because there is no PCH to
> > > respond.
> > > -	 * Move the handshake programming to initialization sequence.
> > > -	 * Previously was left up to BIOS.
> > > -	 */
> > > -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > -	val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> > > -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> > > +	intel_pch_reset_handshake(dev_priv);
> > >  
> > >  	/* Enable PG1 */
> > >  	mutex_lock(&power_domains->lock);
> > > @@ -3439,9 +3441,7 @@ static void cnl_display_core_init(struct
> > > drm_i915_private *dev_priv, bool resume
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  
> > >  	/* 1. Enable PCH Reset Handshake */
> > > -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > -	val |= RESET_PCH_HANDSHAKE_ENABLE;
> > > -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> > > +	intel_pch_reset_handshake(dev_priv);
> > >  
> > >  	/* 2. Enable Comp */
> > >  	val = I915_READ(CHICKEN_MISC_2);
> > > @@ -3524,9 +3524,7 @@ static void icl_display_core_init(struct
> > > drm_i915_private *dev_priv,
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  
> > >  	/* 1. Enable PCH reset handshake. */
> > > -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > -	val |= RESET_PCH_HANDSHAKE_ENABLE;
> > > -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> > > +	intel_pch_reset_handshake(dev_priv);
> > >  
> > >  	for (port = PORT_A; port <= PORT_B; port++) {
> > >  		/* 2. Enable DDI combo PHY comp. */
> > > -- 
> > > 2.19.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel