diff mbox series

[v2,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

Message ID 20180914141849.2046-1-jose.souza@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake | expand

Commit Message

Souza, Jose Sept. 14, 2018, 2:18 p.m. UTC 
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@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 36 ++++++++++++-------------
 1 file changed, 17 insertions(+), 19 deletions(-)

Comments

Ville Syrjälä Sept. 14, 2018, 2:52 p.m. UTC | #1 
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@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@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.

> +		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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Sept. 14, 2018, 3:37 p.m. UTC | #2 
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@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@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...

> 
> > +		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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
Souza, Jose Sept. 17, 2018, 9:28 p.m. UTC | #3 
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@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
diff mbox series

Patch

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))
+		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. */