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

[PATCH v4 1/6] 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.

v2(Rodrigo):
- renamed to intel_pch_reset_handshake()
- added comment about why BXT need the bit to be unset

v3(Rodrigo and Ville):
- added bool have_pch to intel_pch_reset_handshake()
- added back BXT comment

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
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 | 30 ++++++++++++++-----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 9bebec389de1..aa0ff4c08bad 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,
+				      bool enable)
+{
+	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
+
+	if (enable)
+		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, true);
 
 	/* enable PG1 and Misc I/O */
 	mutex_lock(&power_domains->lock);
@@ -3306,7 +3317,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 +3326,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);
+	intel_pch_reset_handshake(dev_priv, false);
 
 	/* Enable PG1 */
 	mutex_lock(&power_domains->lock);
@@ -3439,9 +3447,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, true);
 
 	/* 2. Enable Comp */
 	val = I915_READ(CHICKEN_MISC_2);
@@ -3524,9 +3530,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, true);
 
 	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 v4 2/6] 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.

v2(Rodrigo):
- handling IVYBRIDGE case inside intel_pch_reset_handshake()

v4(Rodrigo and Ville):
- moving the enable/disable decision to callers

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
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 | 28 ++++++++++++++++++-------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a94d5a308c4d..3fe5d4f058ee 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5299,18 +5299,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 aa0ff4c08bad..308d06971435 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3242,14 +3242,25 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
 static void intel_pch_reset_handshake(struct drm_i915_private *dev_priv,
 				      bool enable)
 {
-	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
+	i915_reg_t reg;
+	u32 reset_bits, val;
+
+	if (IS_IVYBRIDGE(dev_priv)) {
+		reg = GEN7_MSG_CTL;
+		reset_bits = WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK;
+	} else {
+		reg = HSW_NDE_RSTWRN_OPT;
+		reset_bits = RESET_PCH_HANDSHAKE_ENABLE;
+	}
+
+	val = I915_READ(reg);
 
 	if (enable)
-		val |= RESET_PCH_HANDSHAKE_ENABLE;
+		val |= reset_bits;
 	else
-		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
+		val &= ~reset_bits;
 
-	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
+	I915_WRITE(reg, val);
 }
 
 static void skl_display_core_init(struct drm_i915_private *dev_priv,
@@ -3261,7 +3272,7 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
 	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
 
 	/* enable PCH reset handshake */
-	intel_pch_reset_handshake(dev_priv, true);
+	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
 	/* enable PG1 and Misc I/O */
 	mutex_lock(&power_domains->lock);
@@ -3447,7 +3458,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 */
-	intel_pch_reset_handshake(dev_priv, true);
+	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
 	/* 2. Enable Comp */
 	val = I915_READ(CHICKEN_MISC_2);
@@ -3530,7 +3541,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. */
-	intel_pch_reset_handshake(dev_priv, true);
+	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
 	for (port = PORT_A; port <= PORT_B; port++) {
 		/* 2. Enable DDI combo PHY comp. */
@@ -3762,7 +3773,8 @@ 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) || INTEL_GEN(dev_priv) >= 7)
+		intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
 
 	/*
 	 * Keep all power wells enabled for any dependent HW access during
-- 
2.19.0

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

[PATCH v4 3/6] 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 removing the uncessary dev_priv->ipc_enabled = false; as now
gens without IPC will not have IPC enabled.

v2(Rodrigo):
- moved the new handling of WA #0477 to the next patch

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1db9b8328275..e2ca04534e23 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6117,6 +6117,9 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
 {
 	u32 val;
 
+	if (!HAS_IPC(dev_priv))
+		return;
+
 	/* Display WA #0477 WaDisableIPC: skl */
 	if (IS_SKYLAKE(dev_priv))
 		dev_priv->ipc_enabled = false;
@@ -6138,7 +6141,6 @@ 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))
 		return;
 
-- 
2.19.0

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

[PATCH v4 4/6] drm/i915: Move SKL IPC WA to HAS_IPC()

[José Roberto de Souza]

SKL has IPC but it should not be set according to the WA, so lets
just mark as it don't have it to simply the code and avoid
unnecessary MMIO writes at every call to intel_enable_ipc().

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 2 ++
 drivers/gpu/drm/i915/intel_pm.c | 4 ----
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index d6f7b9fe1d26..adac75e5d5f7 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -472,6 +472,8 @@ static const struct intel_device_info intel_cherryview_info = {
 
 #define SKL_PLATFORM \
 	GEN9_FEATURES, \
+	/* Display WA #0477 WaDisableIPC: skl */ \
+	.has_ipc = 0, \
 	PLATFORM(INTEL_SKYLAKE)
 
 static const struct intel_device_info intel_skylake_gt1_info = {
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index e2ca04534e23..538bcde0bf7d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6120,10 +6120,6 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
 	if (!HAS_IPC(dev_priv))
 		return;
 
-	/* Display WA #0477 WaDisableIPC: skl */
-	if (IS_SKYLAKE(dev_priv))
-		dev_priv->ipc_enabled = false;
-
 	/* Display WA #1141: SKL:all KBL:all CFL */
 	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
 	    !dev_priv->dram_info.symmetric_memory)
-- 
2.19.0

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

[PATCH v4 5/6] drm/i915: Move IPC WA #1141 to init_ipc()

[José Roberto de Souza]

symmetric_memory do not change after initialization so lets just set
ipc_enabled once for this WA.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 538bcde0bf7d..1392aa56a55a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6120,11 +6120,6 @@ void intel_enable_ipc(struct drm_i915_private *dev_priv)
 	if (!HAS_IPC(dev_priv))
 		return;
 
-	/* Display WA #1141: SKL:all KBL:all CFL */
-	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
-	    !dev_priv->dram_info.symmetric_memory)
-		dev_priv->ipc_enabled = false;
-
 	val = I915_READ(DISP_ARB_CTL2);
 
 	if (dev_priv->ipc_enabled)
@@ -6140,7 +6135,12 @@ void intel_init_ipc(struct drm_i915_private *dev_priv)
 	if (!HAS_IPC(dev_priv))
 		return;
 
-	dev_priv->ipc_enabled = true;
+	/* Display WA #1141: SKL:all KBL:all CFL */
+	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
+		dev_priv->ipc_enabled = dev_priv->dram_info.symmetric_memory;
+	else
+		dev_priv->ipc_enabled = true;
+
 	intel_enable_ipc(dev_priv);
 }
 
-- 
2.19.0

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

[PATCH v4 6/6] 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.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
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 v4 1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 01:47:09PM -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
> 
> v3(Rodrigo and Ville):
> - added bool have_pch to intel_pch_reset_handshake()
> - added back BXT comment

thanks!

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 30 ++++++++++++++-----------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 9bebec389de1..aa0ff4c08bad 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,
> +				      bool enable)
> +{
> +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +
> +	if (enable)
> +		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, true);
>  
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
> @@ -3306,7 +3317,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 +3326,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);
> +	intel_pch_reset_handshake(dev_priv, false);
>  
>  	/* Enable PG1 */
>  	mutex_lock(&power_domains->lock);
> @@ -3439,9 +3447,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, true);
>  
>  	/* 2. Enable Comp */
>  	val = I915_READ(CHICKEN_MISC_2);
> @@ -3524,9 +3530,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, true);
>  
>  	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

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

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 01:47:10PM -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.
> 
> v2(Rodrigo):
> - handling IVYBRIDGE case inside intel_pch_reset_handshake()
> 
> v4(Rodrigo and Ville):
> - moving the enable/disable decision to callers
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>

I was going to cowardly wait for CI, but I will end up
forgetting the review and code looks right to my human eyes,
so before I forget:

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

> ---
>  drivers/gpu/drm/i915/i915_gem.c         | 12 -----------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 28 ++++++++++++++++++-------
>  2 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index a94d5a308c4d..3fe5d4f058ee 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5299,18 +5299,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 aa0ff4c08bad..308d06971435 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -3242,14 +3242,25 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
>  static void intel_pch_reset_handshake(struct drm_i915_private *dev_priv,
>  				      bool enable)
>  {
> -	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +	i915_reg_t reg;
> +	u32 reset_bits, val;
> +
> +	if (IS_IVYBRIDGE(dev_priv)) {
> +		reg = GEN7_MSG_CTL;
> +		reset_bits = WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK;
> +	} else {
> +		reg = HSW_NDE_RSTWRN_OPT;
> +		reset_bits = RESET_PCH_HANDSHAKE_ENABLE;
> +	}
> +
> +	val = I915_READ(reg);
>  
>  	if (enable)
> -		val |= RESET_PCH_HANDSHAKE_ENABLE;
> +		val |= reset_bits;
>  	else
> -		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> +		val &= ~reset_bits;
>  
> -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> +	I915_WRITE(reg, val);
>  }
>  
>  static void skl_display_core_init(struct drm_i915_private *dev_priv,
> @@ -3261,7 +3272,7 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
>  
>  	/* enable PCH reset handshake */
> -	intel_pch_reset_handshake(dev_priv, true);
> +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
>  	/* enable PG1 and Misc I/O */
>  	mutex_lock(&power_domains->lock);
> @@ -3447,7 +3458,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 */
> -	intel_pch_reset_handshake(dev_priv, true);
> +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
>  	/* 2. Enable Comp */
>  	val = I915_READ(CHICKEN_MISC_2);
> @@ -3530,7 +3541,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. */
> -	intel_pch_reset_handshake(dev_priv, true);
> +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
>  	for (port = PORT_A; port <= PORT_B; port++) {
>  		/* 2. Enable DDI combo PHY comp. */
> @@ -3762,7 +3773,8 @@ 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) || INTEL_GEN(dev_priv) >= 7)
> +		intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
>  
>  	/*
>  	 * Keep all power wells enabled for any dependent HW access during
> -- 
> 2.19.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [v4,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake
URL   : https://patchwork.freedesktop.org/series/49874/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4841 -> Patchwork_10219 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49874/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10219 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           PASS -> FAIL (fdo#107336)

    igt@pm_rpm@module-reload:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_coherency:
      fi-gdg-551:         DMESG-FAIL (fdo#107164) -> PASS

    igt@drv_selftest@mock_hugepages:
      fi-bwr-2160:        DMESG-FAIL (fdo#107930) -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       INCOMPLETE (fdo#107773) -> PASS

    
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#107164 https://bugs.freedesktop.org/show_bug.cgi?id=107164
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#107930 https://bugs.freedesktop.org/show_bug.cgi?id=107930


== Participating hosts (52 -> 49) ==

  Additional (2): fi-hsw-4770r fi-icl-u 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4841 -> Patchwork_10219

  CI_DRM_4841: 4a03aefd83495146beca5f593558343aca40eb51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4646: d409cc6f234fbc0122c64be27ba85b5603658de5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10219: eca0c2923bd45520b041848c8e504190a428e031 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

eca0c2923bd4 drm/i915: Remove duplicated definition of intel_update_rawclk
729c0fa84833 drm/i915: Move IPC WA #1141 to init_ipc()
cf8d2b7866a5 drm/i915: Move SKL IPC WA to HAS_IPC()
e85573b247c9 drm/i915: Do not modifiy reserved bit in gens that do not have IPC
663141ece98b drm/i915: Unset reset pch handshake when PCH is not present in one place
63bdff69f093 drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10219/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [v4,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake
URL   : https://patchwork.freedesktop.org/series/49874/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4841_full -> Patchwork_10219_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10219_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10219_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10219_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_vblank@pipe-a-query-idle:
      shard-snb:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_10219_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_busy@extended-pageflip-hang-newfb-render-a:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_cursor_legacy@cursorb-vs-flipb-toggle:
      shard-glk:          PASS -> DMESG-WARN (fdo#106538, fdo#105763)

    
    ==== Possible fixes ====

    igt@kms_busy@extended-modeset-hang-newfb-render-c:
      shard-hsw:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
      shard-apl:          DMESG-WARN (fdo#107956) -> PASS

    igt@kms_cursor_crc@cursor-128x128-onscreen:
      shard-apl:          DMESG-WARN (fdo#103558, fdo#105602) -> PASS +3

    igt@kms_cursor_crc@cursor-128x128-suspend:
      shard-apl:          FAIL (fdo#103191, fdo#103232) -> PASS

    igt@kms_draw_crc@draw-method-rgb565-render-untiled:
      shard-apl:          DMESG-FAIL (fdo#103558, fdo#105602) -> SKIP

    
    ==== Warnings ====

    igt@syncobj_wait@multi-wait-all-for-submit-submitted:
      shard-snb:          DMESG-WARN (fdo#107469) -> INCOMPLETE (fdo#105411)

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4841 -> Patchwork_10219

  CI_DRM_4841: 4a03aefd83495146beca5f593558343aca40eb51 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4646: d409cc6f234fbc0122c64be27ba85b5603658de5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10219: eca0c2923bd45520b041848c8e504190a428e031 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10219/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 02:48:13PM -0700, Rodrigo Vivi wrote:
> On Tue, Sep 18, 2018 at 01:47:10PM -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.
> > 
> > v2(Rodrigo):
> > - handling IVYBRIDGE case inside intel_pch_reset_handshake()
> > 
> > v4(Rodrigo and Ville):
> > - moving the enable/disable decision to callers
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> 
> I was going to cowardly wait for CI, but I will end up
> forgetting the review and code looks right to my human eyes,
> so before I forget:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

pushed to dinq. Thanks for the patches.

> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c         | 12 -----------
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 28 ++++++++++++++++++-------
> >  2 files changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index a94d5a308c4d..3fe5d4f058ee 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -5299,18 +5299,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 aa0ff4c08bad..308d06971435 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -3242,14 +3242,25 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv)
> >  static void intel_pch_reset_handshake(struct drm_i915_private *dev_priv,
> >  				      bool enable)
> >  {
> > -	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > +	i915_reg_t reg;
> > +	u32 reset_bits, val;
> > +
> > +	if (IS_IVYBRIDGE(dev_priv)) {
> > +		reg = GEN7_MSG_CTL;
> > +		reset_bits = WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK;
> > +	} else {
> > +		reg = HSW_NDE_RSTWRN_OPT;
> > +		reset_bits = RESET_PCH_HANDSHAKE_ENABLE;
> > +	}
> > +
> > +	val = I915_READ(reg);
> >  
> >  	if (enable)
> > -		val |= RESET_PCH_HANDSHAKE_ENABLE;
> > +		val |= reset_bits;
> >  	else
> > -		val &= ~RESET_PCH_HANDSHAKE_ENABLE;
> > +		val &= ~reset_bits;
> >  
> > -	I915_WRITE(HSW_NDE_RSTWRN_OPT, val);
> > +	I915_WRITE(reg, val);
> >  }
> >  
> >  static void skl_display_core_init(struct drm_i915_private *dev_priv,
> > @@ -3261,7 +3272,7 @@ static void skl_display_core_init(struct drm_i915_private *dev_priv,
> >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> >  
> >  	/* enable PCH reset handshake */
> > -	intel_pch_reset_handshake(dev_priv, true);
> > +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> >  	/* enable PG1 and Misc I/O */
> >  	mutex_lock(&power_domains->lock);
> > @@ -3447,7 +3458,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 */
> > -	intel_pch_reset_handshake(dev_priv, true);
> > +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> >  	/* 2. Enable Comp */
> >  	val = I915_READ(CHICKEN_MISC_2);
> > @@ -3530,7 +3541,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. */
> > -	intel_pch_reset_handshake(dev_priv, true);
> > +	intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> >  	for (port = PORT_A; port <= PORT_B; port++) {
> >  		/* 2. Enable DDI combo PHY comp. */
> > @@ -3762,7 +3773,8 @@ 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) || INTEL_GEN(dev_priv) >= 7)
> > +		intel_pch_reset_handshake(dev_priv, !HAS_PCH_NOP(dev_priv));
> >  
> >  	/*
> >  	 * Keep all power wells enabled for any dependent HW access during
> > -- 
> > 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