[PATCH 1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[PATCH 1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[José Roberto de Souza]

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.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 29 ++++++++++++++-----------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 480dadb1047b..8bcb33367d0d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3239,18 +3239,28 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
 	I915_WRITE(MBUS_ABOX_CTL, val);
 }
 
+static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv)
+{
+	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
+
+	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);
+	skl_pch_reset_handshake(dev_priv);
 
 	/* enable PG1 and Misc I/O */
 	mutex_lock(&power_domains->lock);
@@ -3306,7 +3316,6 @@ 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);
 
@@ -3316,9 +3325,7 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
 	 * 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);
+	skl_pch_reset_handshake(dev_priv);
 
 	/* Enable PG1 */
 	mutex_lock(&power_domains->lock);
@@ -3439,9 +3446,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);
+	skl_pch_reset_handshake(dev_priv);
 
 	/* 2. Enable Comp */
 	val = I915_READ(CHICKEN_MISC_2);
@@ -3524,9 +3529,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);
+	skl_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

[PATCH 2/4] drm/i915: Unset reset pch handshake when PCH is not present in one place

[José Roberto de Souza]

Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times inside
of intel_power_domains_init_hw() and if PCH is NOP it is unsed in
i915_gem_init_hw().
So making skl_pch_reset_handshake() handle both cases and calling
it for the missing gens in intel_power_domains_init_hw().
Ivybridge have a different register and bits but with the same
objective so moving it too.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 12 ------------
 drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89834ce19acd..b389e084c8c6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
 		I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
 			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
 
-	if (HAS_PCH_NOP(dev_priv)) {
-		if (IS_IVYBRIDGE(dev_priv)) {
-			u32 temp = I915_READ(GEN7_MSG_CTL);
-			temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
-			I915_WRITE(GEN7_MSG_CTL, temp);
-		} else if (INTEL_GEN(dev_priv) >= 7) {
-			u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT);
-			temp &= ~RESET_PCH_HANDSHAKE_ENABLE;
-			I915_WRITE(HSW_NDE_RSTWRN_OPT, temp);
-		}
-	}
-
 	intel_gt_workarounds_apply(dev_priv);
 
 	i915_gem_init_swizzling(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 8bcb33367d0d..369a292cafac 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
 	I915_WRITE(MBUS_ABOX_CTL, val);
 }
 
+/* Actually it is hsw+ but until skl it was not required to set it */
 static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv)
 {
 	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
 
-	if (HAS_PCH_SPLIT(dev_priv))
+	if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv))
 		val |= RESET_PCH_HANDSHAKE_ENABLE;
 	else
 		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
@@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
 		mutex_lock(&power_domains->lock);
 		vlv_cmnlane_wa(dev_priv);
 		mutex_unlock(&power_domains->lock);
+	} else if (IS_IVYBRIDGE(dev_priv)) {
+		if (HAS_PCH_NOP(dev_priv)) {
+			u32 val = I915_READ(GEN7_MSG_CTL);
+
+			val &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
+			I915_WRITE(GEN7_MSG_CTL, val);
+		}
+	} else if (INTEL_GEN(dev_priv) >= 7) {
+		skl_pch_reset_handshake(dev_priv);
 	}
 
 	/*
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/i915: Do not modifiy reserved bit in gens that do not have IPC

[José Roberto de Souza]

IPC was only added in SKL+(actually we don't even enable for SKL due
WA) so without this change, driver was writing to a reserved bit.

Also check for the WA in intel_init_ipc() to avoid further writes to
ipc_enabled.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index d99e5fabe93c..b2328f7d277d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6108,10 +6108,8 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
 	u32 val;
 
 	/* Display WA #0477 WaDisableIPC: skl */
-	if (IS_SKYLAKE(dev_priv)) {
-		dev_priv->ipc_enabled = false;
+	if (!HAS_IPC(dev_priv) || IS_SKYLAKE(dev_priv))
 		return;
-	}
 
 	val = I915_READ(DISP_ARB_CTL2);
 
@@ -6126,7 +6124,9 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
 void intel_init_ipc(struct drm_i915_private *dev_priv)
 {
 	dev_priv->ipc_enabled = false;
-	if (!HAS_IPC(dev_priv))
+
+	/* Display WA #0477 WaDisableIPC: skl */
+	if (!HAS_IPC(dev_priv) || IS_SKYLAKE(dev_priv))
 		return;
 
 	dev_priv->ipc_enabled = true;
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/4] drm/i915: Remove duplicated definition of intel_update_rawclk

[José Roberto de Souza]

A few line above we have another definition of intel_update_rawclk()
keeping that one as the function is implemented in intel_cdclk.c.

Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bf1c38728a59..97e8241d5d36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1488,7 +1488,6 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
 void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
 enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
-void intel_update_rawclk(struct drm_i915_private *dev_priv);
 int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
 int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
 		      const char *name, u32 reg, int ref_freq);
-- 
2.19.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Rodrigo Vivi]

On Thu, Sep 13, 2018 at 02:22:48PM -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.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 29 ++++++++++++++-----------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 480dadb1047b..8bcb33367d0d 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3239,18 +3239,28 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
>  	I915_WRITE(MBUS_ABOX_CTL, val);
>  }
>  
> +static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv)
> +{
> +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +
> +	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);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
> @@ -3306,7 +3316,6 @@ 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);
>  
> @@ -3316,9 +3325,7 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
>  	 * Move the handshake programming to initialization sequence.
>  	 * Previously was left up to BIOS.

I think after this change this comment here gets far from the code
so it is not clear why when PCH not present you unset.

so maybe we should move with this patch, but only with the setting
path and leave this here as is since it seems a workaround exclusive
for bxt... 

>  	 */
> -	val = I915_READ(HSW_NDE_RSTWRN_OPT);
> -	val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* Enable PG1 */
>  	mutex_lock(&power_domains->lock);
> @@ -3439,9 +3446,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);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* 2. Enable Comp */
>  	val = I915_READ(CHICKEN_MISC_2);
> @@ -3524,9 +3529,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);
> +	skl_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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/4] drm/i915: Do not modifiy reserved bit in gens that do not have IPC

[Rodrigo Vivi]

On Thu, Sep 13, 2018 at 02:22:50PM -0700, José Roberto de Souza wrote:
> IPC was only added in SKL+(actually we don't even enable for SKL due
> WA) so without this change, driver was writing to a reserved bit.
> 
> Also check for the WA in intel_init_ipc() to avoid further writes to
> ipc_enabled.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index d99e5fabe93c..b2328f7d277d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6108,10 +6108,8 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
>  	u32 val;
>  
>  	/* Display WA #0477 WaDisableIPC: skl */
> -	if (IS_SKYLAKE(dev_priv)) {
> -		dev_priv->ipc_enabled = false;
> +	if (!HAS_IPC(dev_priv) || IS_SKYLAKE(dev_priv))
>  		return;
> -	}
>  
>  	val = I915_READ(DISP_ARB_CTL2);
>  
> @@ -6126,7 +6124,9 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
>  void intel_init_ipc(struct drm_i915_private *dev_priv)
>  {
>  	dev_priv->ipc_enabled = false;
> -	if (!HAS_IPC(dev_priv))
> +
> +	/* Display WA #0477 WaDisableIPC: skl */

What about move the WA 0477 inside HAS_IPC and avoid the
duplication?

> +	if (!HAS_IPC(dev_priv) || IS_SKYLAKE(dev_priv))
>  		return;
>  
>  	dev_priv->ipc_enabled = true;
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/4] drm/i915: Remove duplicated definition of intel_update_rawclk

[Rodrigo Vivi]

On Thu, Sep 13, 2018 at 02:22:51PM -0700, José Roberto de Souza wrote:
> A few line above we have another definition of intel_update_rawclk()
> keeping that one as the function is implemented in intel_cdclk.c.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bf1c38728a59..97e8241d5d36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1488,7 +1488,6 @@ void intel_dump_cdclk_state(const struct intel_cdclk_state *cdclk_state,
>  void i830_enable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
>  void i830_disable_pipe(struct drm_i915_private *dev_priv, enum pipe pipe);
>  enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc);
> -void intel_update_rawclk(struct drm_i915_private *dev_priv);
>  int vlv_get_hpll_vco(struct drm_i915_private *dev_priv);
>  int vlv_get_cck_clock(struct drm_i915_private *dev_priv,
>  		      const char *name, u32 reg, int ref_freq);
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Unset reset pch handshake when PCH is not present in one place

[Rodrigo Vivi]

On Thu, Sep 13, 2018 at 02:22:49PM -0700, José Roberto de Souza wrote:
> Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times inside
> of intel_power_domains_init_hw() and if PCH is NOP it is unsed in
> i915_gem_init_hw().
> So making skl_pch_reset_handshake() handle both cases and calling
> it for the missing gens in intel_power_domains_init_hw().
> Ivybridge have a different register and bits but with the same
> objective so moving it too.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c         | 12 ------------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++-
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 89834ce19acd..b389e084c8c6 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>  		I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ?
>  			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
>  
> -	if (HAS_PCH_NOP(dev_priv)) {
> -		if (IS_IVYBRIDGE(dev_priv)) {
> -			u32 temp = I915_READ(GEN7_MSG_CTL);
> -			temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
> -			I915_WRITE(GEN7_MSG_CTL, temp);
> -		} else if (INTEL_GEN(dev_priv) >= 7) {
> -			u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT);
> -			temp &= ~RESET_PCH_HANDSHAKE_ENABLE;
> -			I915_WRITE(HSW_NDE_RSTWRN_OPT, temp);
> -		}
> -	}
> -
>  	intel_gt_workarounds_apply(dev_priv);
>  
>  	i915_gem_init_swizzling(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 8bcb33367d0d..369a292cafac 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
>  	I915_WRITE(MBUS_ABOX_CTL, val);
>  }
>  
> +/* Actually it is hsw+ but until skl it was not required to set it */
>  static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv)
>  {
>  	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
>  
> -	if (HAS_PCH_SPLIT(dev_priv))
> +	if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv))

now I saw why you wanted the unset bit here as well ;)

>  		val |= RESET_PCH_HANDSHAKE_ENABLE;
>  	else
>  		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> @@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
>  		mutex_lock(&power_domains->lock);
>  		vlv_cmnlane_wa(dev_priv);
>  		mutex_unlock(&power_domains->lock);
> +	} else if (IS_IVYBRIDGE(dev_priv)) {
> +		if (HAS_PCH_NOP(dev_priv)) {
> +			u32 val = I915_READ(GEN7_MSG_CTL);
> +
> +			val &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK);
> +			I915_WRITE(GEN7_MSG_CTL, val);
> +		}
> +	} else if (INTEL_GEN(dev_priv) >= 7) {
> +		skl_pch_reset_handshake(dev_priv);

well... skl_ doesn't make sense anymore...
maybe s/skl/hsw ?

but what about the remaining block for Ivybridge?
I feel it got little unbalanced here...

maybe we move everything inside to a intel_pch_reset_handshake(dev_priv, bool set) ?

>  	}
>  
>  	/*
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake
URL   : https://patchwork.freedesktop.org/series/49671/
State : failure

== Summary ==

Applying: drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake
Applying: drm/i915: Unset reset pch handshake when PCH is not present in one place
Applying: drm/i915: Do not modifiy reserved bit in gens that do not have IPC
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/intel_pm.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_pm.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/intel_pm.c
error: Failed to merge in the changes.
Patch failed at 0003 drm/i915: Do not modifiy reserved bit in gens that do not have IPC
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/4] drm/i915: Unset reset pch handshake when PCH is not present in one place

[Souza, Jose]

On Thu, 2018-09-13 at 14:51 -0700, Rodrigo Vivi wrote:
> On Thu, Sep 13, 2018 at 02:22:49PM -0700, José Roberto de Souza
> wrote:
> > Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times
> > inside
> > of intel_power_domains_init_hw() and if PCH is NOP it is unsed in
> > i915_gem_init_hw().
> > So making skl_pch_reset_handshake() handle both cases and calling
> > it for the missing gens in intel_power_domains_init_hw().
> > Ivybridge have a different register and bits but with the same
> > objective so moving it too.
> > 
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c         | 12 ------------
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++-
> >  2 files changed, 11 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 89834ce19acd..b389e084c8c6 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private
> > *dev_priv)
> >  		I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv)
> > ?
> >  			   LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED);
> >  
> > -	if (HAS_PCH_NOP(dev_priv)) {
> > -		if (IS_IVYBRIDGE(dev_priv)) {
> > -			u32 temp = I915_READ(GEN7_MSG_CTL);
> > -			temp &= ~(WAIT_FOR_PCH_FLR_ACK |
> > WAIT_FOR_PCH_RESET_ACK);
> > -			I915_WRITE(GEN7_MSG_CTL, temp);
> > -		} else if (INTEL_GEN(dev_priv) >= 7) {
> > -			u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT);
> > -			temp &= ~RESET_PCH_HANDSHAKE_ENABLE;
> > -			I915_WRITE(HSW_NDE_RSTWRN_OPT, temp);
> > -		}
> > -	}
> > -
> >  	intel_gt_workarounds_apply(dev_priv);
> >  
> >  	i915_gem_init_swizzling(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 8bcb33367d0d..369a292cafac 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct
> > drm_i915_private *dev_priv)
> >  	I915_WRITE(MBUS_ABOX_CTL, val);
> >  }
> >  
> > +/* Actually it is hsw+ but until skl it was not required to set it
> > */
> >  static void skl_pch_reset_handshake(struct drm_i915_private
> > *dev_priv)
> >  {
> >  	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> >  
> > -	if (HAS_PCH_SPLIT(dev_priv))
> > +	if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv))
> 
> now I saw why you wanted the unset bit here as well ;)
> 
> >  		val |= RESET_PCH_HANDSHAKE_ENABLE;
> >  	else
> >  		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> > @@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct
> > drm_i915_private *dev_priv, bool resume)
> >  		mutex_lock(&power_domains->lock);
> >  		vlv_cmnlane_wa(dev_priv);
> >  		mutex_unlock(&power_domains->lock);
> > +	} else if (IS_IVYBRIDGE(dev_priv)) {
> > +		if (HAS_PCH_NOP(dev_priv)) {
> > +			u32 val = I915_READ(GEN7_MSG_CTL);
> > +
> > +			val &= ~(WAIT_FOR_PCH_FLR_ACK |
> > WAIT_FOR_PCH_RESET_ACK);
> > +			I915_WRITE(GEN7_MSG_CTL, val);
> > +		}
> > +	} else if (INTEL_GEN(dev_priv) >= 7) {
> > +		skl_pch_reset_handshake(dev_priv);
> 
> well... skl_ doesn't make sense anymore...
> maybe s/skl/hsw ?
> 
> but what about the remaining block for Ivybridge?
> I feel it got little unbalanced here...
> 
> maybe we move everything inside to a
> intel_pch_reset_handshake(dev_priv, bool set) ?

I guess move everything to intel_pch_reset_handshake() sounds better
but without the bool set, as the HAS_PCH_NOP can be set to all those
platforms depending if display is on or off.



> 
> >  	}
> >  
> >  	/*
> > -- 
> > 2.19.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Ville Syrjälä]

On Thu, Sep 13, 2018 at 02:22:48PM -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.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 29 ++++++++++++++-----------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 480dadb1047b..8bcb33367d0d 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3239,18 +3239,28 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
>  	I915_WRITE(MBUS_ABOX_CTL, val);
>  }
>  
> +static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv)
> +{
> +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +
> +	if (HAS_PCH_SPLIT(dev_priv))
> +		val |= RESET_PCH_HANDSHAKE_ENABLE;
> +	else
> +		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> +
> +	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> +}

hsw has this too. And ivb has a slightly different version. Could unify
it all. And maybe pass the enable/disable as a paramter to make it clear
from reading the calling code what it's doing?

> +
>  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);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
> @@ -3306,7 +3316,6 @@ 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);
>  
> @@ -3316,9 +3325,7 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
>  	 * 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);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* Enable PG1 */
>  	mutex_lock(&power_domains->lock);
> @@ -3439,9 +3446,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);
> +	skl_pch_reset_handshake(dev_priv);
>  
>  	/* 2. Enable Comp */
>  	val = I915_READ(CHICKEN_MISC_2);
> @@ -3524,9 +3529,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);
> +	skl_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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx