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

[PATCH v3 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..4e050dc3ebde 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 have_pch)
+{
+	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
+
+	if (have_pch)
+		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 v3 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()

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 | 24 ++++++++++++++++++------
 2 files changed, 18 insertions(+), 18 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 4e050dc3ebde..6b724209264a 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 have_pch)
 {
-	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
+	i915_reg_t reg;
+	u32 reset_bits, val;
 
-	if (have_pch)
-		val |= RESET_PCH_HANDSHAKE_ENABLE;
+	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 (have_pch && !HAS_PCH_NOP(dev_priv))
+		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,
@@ -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, true);
 
 	/*
 	 * 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 v3 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 v3 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().

Cc: 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 v3 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.

Cc: 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 v3 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 v3 1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Ville Syrjälä]

On Tue, Sep 18, 2018 at 11:10:10AM -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
> 
> 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..4e050dc3ebde 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 have_pch)

You seem very intent on moving the HAS_PCH_NOP() into this function.
I still don't like that approach. I think it's clearner to let the
caller 100% decide whether to enable or disable the reset handshake.

> +{
> +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> +
> +	if (have_pch)
> +		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

-- 
Ville Syrjälä
Intel
_______________________________________________
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 [v3,1/6] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake

[Patchwork]

== Series Details ==

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

== Summary ==

= CI Bug Log - changes from CI_DRM_4840 -> Patchwork_10217 =

== Summary - FAILURE ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-cfl-8109u:       PASS -> INCOMPLETE

    igt@drv_selftest@live_objects:
      fi-cfl-8109u:       PASS -> DMESG-WARN

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@amdgpu/amd_prime@amd-to-i915:
      fi-kbl-8809g:       NOTRUN -> FAIL (fdo#107341)

    igt@kms_setmode@basic-clone-single-crtc:
      fi-ilk-650:         PASS -> DMESG-WARN (fdo#106387)

    
  fdo#106387 https://bugs.freedesktop.org/show_bug.cgi?id=106387
  fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341


== Participating hosts (50 -> 45) ==

  Missing    (5): fi-byt-j1900 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4840 -> Patchwork_10217

  CI_DRM_4840: eb5915292ad60c16f895c77967a1353c7ef87fef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4646: d409cc6f234fbc0122c64be27ba85b5603658de5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10217: 59f28afc2c0d562134612c6b047d37eaacd09fd3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

59f28afc2c0d drm/i915: Remove duplicated definition of intel_update_rawclk
a26617be3caa drm/i915: Move IPC WA #1141 to init_ipc()
4feaa1c038d7 drm/i915: Move SKL IPC WA to HAS_IPC()
a0b81b7af683 drm/i915: Do not modifiy reserved bit in gens that do not have IPC
b90f2df66e70 drm/i915: Unset reset pch handshake when PCH is not present in one place
23eebc4be038 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_10217/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Souza, Jose]

On Tue, 2018-09-18 at 22:04 +0300, Ville Syrjälä wrote:
> On Tue, Sep 18, 2018 at 11:10:10AM -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
> > 
> > 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..4e050dc3ebde 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 have_pch)
> 
> You seem very intent on moving the HAS_PCH_NOP() into this function.
> I still don't like that approach. I think it's clearner to let the
> caller 100% decide whether to enable or disable the reset handshake.

Because it would just move HAS_PCH_NOP() to all the callers expect for
BXT, also we are doing the same thing right now but in a wrong place.
But if you and Rodrigo really wants it, I can change that.


> 
> > +{
> > +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > +
> > +	if (have_pch)
> > +		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 v3 4/6] drm/i915: Move SKL IPC WA to HAS_IPC()

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 11:10:13AM -0700, José Roberto de Souza wrote:
> 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().
> 
> 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/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

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

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 11:10:14AM -0700, José Roberto de Souza wrote:
> symmetric_memory do not change after initialization so lets just set
> ipc_enabled once for this WA.
> 
> 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_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

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

[Rodrigo Vivi]

On Tue, Sep 18, 2018 at 12:40:21PM -0700, Souza, Jose wrote:
> On Tue, 2018-09-18 at 22:04 +0300, Ville Syrjälä wrote:
> > On Tue, Sep 18, 2018 at 11:10:10AM -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
> > > 
> > > 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..4e050dc3ebde 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 have_pch)
> > 
> > You seem very intent on moving the HAS_PCH_NOP() into this function.
> > I still don't like that approach. I think it's clearner to let the
> > caller 100% decide whether to enable or disable the reset handshake.
> 
> Because it would just move HAS_PCH_NOP() to all the callers expect for
> BXT, also we are doing the same thing right now but in a wrong place.
> But if you and Rodrigo really wants it, I can change that.

:)

Thanks for changing to the bool approach already.

Checking the other patch I agree with ville that leaving the enable/disable
decision outside is better.

> 
> 
> > 
> > > +{
> > > +	u32 val = I915_READ(HSW_NDE_RSTWRN_OPT);
> > > +
> > > +	if (have_pch)
> > > +		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