[PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[kai.chen]

From: Kai Chen <kai.chen@intel.com>

The decoupled mmio feature doesn't work as intended by HW team. Enabling
it with forcewake will only make debugging efforts more difficult, so
let's just simply remove it.

v2:
- Remove dead code related to GEN9LP decoupled mmio.
- Change backgrounds: In theory, decoupled mmio should require less cycles
  for single read/write operation by avoiding frequent software forcewake.
  However, it turns out this design not to be true on HW practice and not to
  provide any decoupling benefit. It also introduces problems which cause
  failures in intel-gpu-tools (gem), and also cause driver code and debugging
  more complex.
- This change therefore reverts:

  commit 85ee17ebeedd1af0dccd98f82ab4e644e29d84c0
  Author: Praveen Paneri <praveen.paneri@intel.com>
  Date: Tue, 15 Nov 2016 22:49:20 +0530

      drm/i915/bxt: Broxton decoupled MMIO

  coomit a3f79ca63b9bcf5a527b886953092bfd65e78940
  Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
  Date: Thu, 24 Nov 2016 15:23:27 +0200

      drm/i915: Don't sanitize has_decoupled_mmio if platform is not broxton

Signed-off-by: Kai Chen <kai.chen@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c     |   1 -
 drivers/gpu/drm/i915/i915_reg.h     |   7 --
 drivers/gpu/drm/i915/intel_uncore.c | 126 ------------------------------------
 3 files changed, 134 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f80db2c..cf43dc1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -385,7 +385,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
 	.has_gmbus_irq = 1, \
 	.has_logical_ring_contexts = 1, \
 	.has_guc = 1, \
-	.has_decoupled_mmio = 1, \
 	.has_aliasing_ppgtt = 1, \
 	.has_full_ppgtt = 1, \
 	.has_full_48bit_ppgtt = 1, \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee144ec..78872f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7792,13 +7792,6 @@ enum {
 #define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
 #define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
 
-/* Decoupled MMIO register pair for kernel driver */
-#define GEN9_DECOUPLED_REG0_DW0			_MMIO(0xF00)
-#define GEN9_DECOUPLED_REG0_DW1			_MMIO(0xF04)
-#define GEN9_DECOUPLED_DW1_GO			(1<<31)
-#define GEN9_DECOUPLED_PD_SHIFT			28
-#define GEN9_DECOUPLED_OP_SHIFT			24
-
 /* Per-pipe DDI Function Control */
 #define _TRANS_DDI_FUNC_CTL_A		0x60400
 #define _TRANS_DDI_FUNC_CTL_B		0x61400
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a9a6933..3901800 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,8 +400,6 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
 static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 					  bool restore_forcewake)
 {
-	struct intel_device_info *info = mkwrite_device_info(dev_priv);
-
 	/* clear out unclaimed reg detection bit */
 	if (check_for_unclaimed_mmio(dev_priv))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
@@ -414,9 +412,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 				   GT_FIFO_CTL_RC6_POLICY_STALL);
 	}
 
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
-		info->has_decoupled_mmio = false;
-
 	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
 }
 
@@ -801,78 +796,6 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
 	__unclaimed_reg_debug(dev_priv, reg, read, before);
 }
 
-enum decoupled_power_domain {
-	GEN9_DECOUPLED_PD_BLITTER = 0,
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-enum decoupled_ops {
-	GEN9_DECOUPLED_OP_WRITE = 0,
-	GEN9_DECOUPLED_OP_READ
-};
-
-static const enum decoupled_power_domain fw2dpd_domain[] = {
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_BLITTER,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-/*
- * Decoupled MMIO access for only 1 DWORD
- */
-static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
-					 u32 reg,
-					 enum forcewake_domains fw_domain,
-					 enum decoupled_ops operation)
-{
-	enum decoupled_power_domain dp_domain;
-	u32 ctrl_reg_data = 0;
-
-	dp_domain = fw2dpd_domain[fw_domain - 1];
-
-	ctrl_reg_data |= reg;
-	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
-	ctrl_reg_data |= (dp_domain << GEN9_DECOUPLED_PD_SHIFT);
-	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
-
-	if (wait_for_atomic((__raw_i915_read32(dev_priv,
-			    GEN9_DECOUPLED_REG0_DW1) &
-			    GEN9_DECOUPLED_DW1_GO) == 0,
-			    FORCEWAKE_ACK_TIMEOUT_MS))
-		DRM_ERROR("Decoupled MMIO wait timed out\n");
-}
-
-static inline u32
-__gen9_decoupled_mmio_read32(struct drm_i915_private *dev_priv,
-			     u32 reg,
-			     enum forcewake_domains fw_domain)
-{
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_READ);
-
-	return __raw_i915_read32(dev_priv, GEN9_DECOUPLED_REG0_DW0);
-}
-
-static inline void
-__gen9_decoupled_mmio_write(struct drm_i915_private *dev_priv,
-			    u32 reg, u32 data,
-			    enum forcewake_domains fw_domain)
-{
-
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW0, data);
-
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_WRITE);
-}
-
-
 #define GEN2_READ_HEADER(x) \
 	u##x val = 0; \
 	assert_rpm_wakelock_held(dev_priv);
@@ -969,28 +892,6 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
 #define __gen6_read(x) __gen_read(gen6, x)
 #define __fwtable_read(x) __gen_read(fwtable, x)
 
-#define __gen9_decoupled_read(x) \
-static u##x \
-gen9_decoupled_read##x(struct drm_i915_private *dev_priv, \
-		       i915_reg_t reg, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_READ_HEADER(x); \
-	fw_engine = __fwtable_reg_read_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) { \
-		unsigned i; \
-		u32 *ptr_data = (u32 *) &val; \
-		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
-			*ptr_data = __gen9_decoupled_mmio_read32(dev_priv, \
-								 offset, \
-								 fw_engine); \
-	} else { \
-		val = __raw_i915_read##x(dev_priv, reg); \
-	} \
-	GEN6_READ_FOOTER; \
-}
-
-__gen9_decoupled_read(32)
-__gen9_decoupled_read(64)
 __fwtable_read(8)
 __fwtable_read(16)
 __fwtable_read(32)
@@ -1077,25 +978,6 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
 #define __gen8_write(x) __gen_write(gen8, x)
 #define __fwtable_write(x) __gen_write(fwtable, x)
 
-#define __gen9_decoupled_write(x) \
-static void \
-gen9_decoupled_write##x(struct drm_i915_private *dev_priv, \
-			i915_reg_t reg, u##x val, \
-		bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_WRITE_HEADER; \
-	fw_engine = __fwtable_reg_write_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) \
-		__gen9_decoupled_mmio_write(dev_priv, \
-					    offset, \
-					    val, \
-					    fw_engine); \
-	else \
-		__raw_i915_write##x(dev_priv, reg, val); \
-	GEN6_WRITE_FOOTER; \
-}
-
-__gen9_decoupled_write(32)
 __fwtable_write(8)
 __fwtable_write(16)
 __fwtable_write(32)
@@ -1332,14 +1214,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
 		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
 		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
-		if (HAS_DECOUPLED_MMIO(dev_priv)) {
-			dev_priv->uncore.funcs.mmio_readl =
-						gen9_decoupled_read32;
-			dev_priv->uncore.funcs.mmio_readq =
-						gen9_decoupled_read64;
-			dev_priv->uncore.funcs.mmio_writel =
-						gen9_decoupled_write32;
-		}
 	}
 
 	iosf_mbi_register_pmic_bus_access_notifier(
-- 
2.9.3

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

✓ Fi.CI.BAT: success for drm/i915: Disable decoupled mmio for GEN9LP (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable decoupled mmio for GEN9LP (rev2)
URL   : https://patchwork.freedesktop.org/series/24470/
State : success

== Summary ==

Series 24470v2 drm/i915: Disable decoupled mmio for GEN9LP
https://patchwork.freedesktop.org/api/1.0/series/24470/revisions/2/mbox/

fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:494s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:429s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:436s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:409s
fi-skl-6700k failed to connect after reboot

713f8ec675d5a793326b94d9a0cc484608eff75e drm-tip: 2017y-05m-16d-15h-22m-08s UTC integration manifest
60cfda0 drm/i915: Disable decoupled mmio for GEN9LP

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4714/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Disable decoupled mmio for GEN9LP (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Disable decoupled mmio for GEN9LP (rev2)
URL   : https://patchwork.freedesktop.org/series/24470/
State : success

== Summary ==

Series 24470v2 drm/i915: Disable decoupled mmio for GEN9LP
https://patchwork.freedesktop.org/api/1.0/series/24470/revisions/2/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:446s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:582s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:496s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:485s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:501s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:468s
fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:462s
fi-kbl-7560u     total:278  pass:263  dwarn:5   dfail:0   fail:0   skip:10  time:572s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:455s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:582s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:500s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:411s

713f8ec675d5a793326b94d9a0cc484608eff75e drm-tip: 2017y-05m-16d-15h-22m-08s UTC integration manifest
a0485f8 drm/i915: Disable decoupled mmio for GEN9LP

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4718/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[Tvrtko Ursulin]

On 17/05/2017 02:07, kai.chen@intel.com wrote:
> From: Kai Chen <kai.chen@intel.com>
>
> The decoupled mmio feature doesn't work as intended by HW team. Enabling
> it with forcewake will only make debugging efforts more difficult, so
> let's just simply remove it.
>
> v2:
> - Remove dead code related to GEN9LP decoupled mmio.
> - Change backgrounds: In theory, decoupled mmio should require less cycles
>   for single read/write operation by avoiding frequent software forcewake.
>   However, it turns out this design not to be true on HW practice and not to
>   provide any decoupling benefit. It also introduces problems which cause
>   failures in intel-gpu-tools (gem), and also cause driver code and debugging
>   more complex.
> - This change therefore reverts:
>
>   commit 85ee17ebeedd1af0dccd98f82ab4e644e29d84c0
>   Author: Praveen Paneri <praveen.paneri@intel.com>
>   Date: Tue, 15 Nov 2016 22:49:20 +0530
>
>       drm/i915/bxt: Broxton decoupled MMIO
>
>   coomit a3f79ca63b9bcf5a527b886953092bfd65e78940
>   Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>   Date: Thu, 24 Nov 2016 15:23:27 +0200
>
>       drm/i915: Don't sanitize has_decoupled_mmio if platform is not broxton

Revert looks incomplete since it is leaving behind the field in the 
device info and HAS_DECOUPLED_MMIO macro.

But revert in principle is fine by me.

Regards,

Tvrtko

> Signed-off-by: Kai Chen <kai.chen@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c     |   1 -
>  drivers/gpu/drm/i915/i915_reg.h     |   7 --
>  drivers/gpu/drm/i915/intel_uncore.c | 126 ------------------------------------
>  3 files changed, 134 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index f80db2c..cf43dc1 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -385,7 +385,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
>  	.has_gmbus_irq = 1, \
>  	.has_logical_ring_contexts = 1, \
>  	.has_guc = 1, \
> -	.has_decoupled_mmio = 1, \
>  	.has_aliasing_ppgtt = 1, \
>  	.has_full_ppgtt = 1, \
>  	.has_full_48bit_ppgtt = 1, \
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index ee144ec..78872f9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7792,13 +7792,6 @@ enum {
>  #define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
>  #define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
>
> -/* Decoupled MMIO register pair for kernel driver */
> -#define GEN9_DECOUPLED_REG0_DW0			_MMIO(0xF00)
> -#define GEN9_DECOUPLED_REG0_DW1			_MMIO(0xF04)
> -#define GEN9_DECOUPLED_DW1_GO			(1<<31)
> -#define GEN9_DECOUPLED_PD_SHIFT			28
> -#define GEN9_DECOUPLED_OP_SHIFT			24
> -
>  /* Per-pipe DDI Function Control */
>  #define _TRANS_DDI_FUNC_CTL_A		0x60400
>  #define _TRANS_DDI_FUNC_CTL_B		0x61400
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a9a6933..3901800 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -400,8 +400,6 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
>  static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>  					  bool restore_forcewake)
>  {
> -	struct intel_device_info *info = mkwrite_device_info(dev_priv);
> -
>  	/* clear out unclaimed reg detection bit */
>  	if (check_for_unclaimed_mmio(dev_priv))
>  		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
> @@ -414,9 +412,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>  				   GT_FIFO_CTL_RC6_POLICY_STALL);
>  	}
>
> -	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
> -		info->has_decoupled_mmio = false;
> -
>  	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
>  }
>
> @@ -801,78 +796,6 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>  	__unclaimed_reg_debug(dev_priv, reg, read, before);
>  }
>
> -enum decoupled_power_domain {
> -	GEN9_DECOUPLED_PD_BLITTER = 0,
> -	GEN9_DECOUPLED_PD_RENDER,
> -	GEN9_DECOUPLED_PD_MEDIA,
> -	GEN9_DECOUPLED_PD_ALL
> -};
> -
> -enum decoupled_ops {
> -	GEN9_DECOUPLED_OP_WRITE = 0,
> -	GEN9_DECOUPLED_OP_READ
> -};
> -
> -static const enum decoupled_power_domain fw2dpd_domain[] = {
> -	GEN9_DECOUPLED_PD_RENDER,
> -	GEN9_DECOUPLED_PD_BLITTER,
> -	GEN9_DECOUPLED_PD_ALL,
> -	GEN9_DECOUPLED_PD_MEDIA,
> -	GEN9_DECOUPLED_PD_ALL,
> -	GEN9_DECOUPLED_PD_ALL,
> -	GEN9_DECOUPLED_PD_ALL
> -};
> -
> -/*
> - * Decoupled MMIO access for only 1 DWORD
> - */
> -static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
> -					 u32 reg,
> -					 enum forcewake_domains fw_domain,
> -					 enum decoupled_ops operation)
> -{
> -	enum decoupled_power_domain dp_domain;
> -	u32 ctrl_reg_data = 0;
> -
> -	dp_domain = fw2dpd_domain[fw_domain - 1];
> -
> -	ctrl_reg_data |= reg;
> -	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
> -	ctrl_reg_data |= (dp_domain << GEN9_DECOUPLED_PD_SHIFT);
> -	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
> -	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
> -
> -	if (wait_for_atomic((__raw_i915_read32(dev_priv,
> -			    GEN9_DECOUPLED_REG0_DW1) &
> -			    GEN9_DECOUPLED_DW1_GO) == 0,
> -			    FORCEWAKE_ACK_TIMEOUT_MS))
> -		DRM_ERROR("Decoupled MMIO wait timed out\n");
> -}
> -
> -static inline u32
> -__gen9_decoupled_mmio_read32(struct drm_i915_private *dev_priv,
> -			     u32 reg,
> -			     enum forcewake_domains fw_domain)
> -{
> -	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
> -				     GEN9_DECOUPLED_OP_READ);
> -
> -	return __raw_i915_read32(dev_priv, GEN9_DECOUPLED_REG0_DW0);
> -}
> -
> -static inline void
> -__gen9_decoupled_mmio_write(struct drm_i915_private *dev_priv,
> -			    u32 reg, u32 data,
> -			    enum forcewake_domains fw_domain)
> -{
> -
> -	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW0, data);
> -
> -	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
> -				     GEN9_DECOUPLED_OP_WRITE);
> -}
> -
> -
>  #define GEN2_READ_HEADER(x) \
>  	u##x val = 0; \
>  	assert_rpm_wakelock_held(dev_priv);
> @@ -969,28 +892,6 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
>  #define __gen6_read(x) __gen_read(gen6, x)
>  #define __fwtable_read(x) __gen_read(fwtable, x)
>
> -#define __gen9_decoupled_read(x) \
> -static u##x \
> -gen9_decoupled_read##x(struct drm_i915_private *dev_priv, \
> -		       i915_reg_t reg, bool trace) { \
> -	enum forcewake_domains fw_engine; \
> -	GEN6_READ_HEADER(x); \
> -	fw_engine = __fwtable_reg_read_fw_domains(offset); \
> -	if (fw_engine & ~dev_priv->uncore.fw_domains_active) { \
> -		unsigned i; \
> -		u32 *ptr_data = (u32 *) &val; \
> -		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
> -			*ptr_data = __gen9_decoupled_mmio_read32(dev_priv, \
> -								 offset, \
> -								 fw_engine); \
> -	} else { \
> -		val = __raw_i915_read##x(dev_priv, reg); \
> -	} \
> -	GEN6_READ_FOOTER; \
> -}
> -
> -__gen9_decoupled_read(32)
> -__gen9_decoupled_read(64)
>  __fwtable_read(8)
>  __fwtable_read(16)
>  __fwtable_read(32)
> @@ -1077,25 +978,6 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
>  #define __gen8_write(x) __gen_write(gen8, x)
>  #define __fwtable_write(x) __gen_write(fwtable, x)
>
> -#define __gen9_decoupled_write(x) \
> -static void \
> -gen9_decoupled_write##x(struct drm_i915_private *dev_priv, \
> -			i915_reg_t reg, u##x val, \
> -		bool trace) { \
> -	enum forcewake_domains fw_engine; \
> -	GEN6_WRITE_HEADER; \
> -	fw_engine = __fwtable_reg_write_fw_domains(offset); \
> -	if (fw_engine & ~dev_priv->uncore.fw_domains_active) \
> -		__gen9_decoupled_mmio_write(dev_priv, \
> -					    offset, \
> -					    val, \
> -					    fw_engine); \
> -	else \
> -		__raw_i915_write##x(dev_priv, reg, val); \
> -	GEN6_WRITE_FOOTER; \
> -}
> -
> -__gen9_decoupled_write(32)
>  __fwtable_write(8)
>  __fwtable_write(16)
>  __fwtable_write(32)
> @@ -1332,14 +1214,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>  		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
>  		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
>  		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
> -		if (HAS_DECOUPLED_MMIO(dev_priv)) {
> -			dev_priv->uncore.funcs.mmio_readl =
> -						gen9_decoupled_read32;
> -			dev_priv->uncore.funcs.mmio_readq =
> -						gen9_decoupled_read64;
> -			dev_priv->uncore.funcs.mmio_writel =
> -						gen9_decoupled_write32;
> -		}
>  	}
>
>  	iosf_mbi_register_pmic_bus_access_notifier(
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[Jani Nikula]

On Wed, 17 May 2017, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 17/05/2017 02:07, kai.chen@intel.com wrote:
>> From: Kai Chen <kai.chen@intel.com>
>>
>> The decoupled mmio feature doesn't work as intended by HW team. Enabling
>> it with forcewake will only make debugging efforts more difficult, so
>> let's just simply remove it.
>>
>> v2:
>> - Remove dead code related to GEN9LP decoupled mmio.
>> - Change backgrounds: In theory, decoupled mmio should require less cycles
>>   for single read/write operation by avoiding frequent software forcewake.
>>   However, it turns out this design not to be true on HW practice and not to
>>   provide any decoupling benefit. It also introduces problems which cause
>>   failures in intel-gpu-tools (gem), and also cause driver code and debugging
>>   more complex.
>> - This change therefore reverts:
>>
>>   commit 85ee17ebeedd1af0dccd98f82ab4e644e29d84c0
>>   Author: Praveen Paneri <praveen.paneri@intel.com>
>>   Date: Tue, 15 Nov 2016 22:49:20 +0530
>>
>>       drm/i915/bxt: Broxton decoupled MMIO
>>
>>   coomit a3f79ca63b9bcf5a527b886953092bfd65e78940
>>   Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>>   Date: Thu, 24 Nov 2016 15:23:27 +0200
>>
>>       drm/i915: Don't sanitize has_decoupled_mmio if platform is not broxton
>
> Revert looks incomplete since it is leaving behind the field in the 
> device info and HAS_DECOUPLED_MMIO macro.

We've shipped kernels with decoupled mmio enabled, so we need to
backport the fix. IOW, we'll need the simple patch [1] with proper
Fixes: annotations, which will be backported, and another patch on top
with the rest that's not going to be backported.

BR,
Jani.

[1] http://patchwork.freedesktop.org/patch/msgid/20170515231827.17902-1-kai.chen@intel.com


>
> But revert in principle is fine by me.
>
> Regards,
>
> Tvrtko
>
>> Signed-off-by: Kai Chen <kai.chen@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_pci.c     |   1 -
>>  drivers/gpu/drm/i915/i915_reg.h     |   7 --
>>  drivers/gpu/drm/i915/intel_uncore.c | 126 ------------------------------------
>>  3 files changed, 134 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index f80db2c..cf43dc1 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -385,7 +385,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
>>  	.has_gmbus_irq = 1, \
>>  	.has_logical_ring_contexts = 1, \
>>  	.has_guc = 1, \
>> -	.has_decoupled_mmio = 1, \
>>  	.has_aliasing_ppgtt = 1, \
>>  	.has_full_ppgtt = 1, \
>>  	.has_full_48bit_ppgtt = 1, \
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index ee144ec..78872f9 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7792,13 +7792,6 @@ enum {
>>  #define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
>>  #define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
>>
>> -/* Decoupled MMIO register pair for kernel driver */
>> -#define GEN9_DECOUPLED_REG0_DW0			_MMIO(0xF00)
>> -#define GEN9_DECOUPLED_REG0_DW1			_MMIO(0xF04)
>> -#define GEN9_DECOUPLED_DW1_GO			(1<<31)
>> -#define GEN9_DECOUPLED_PD_SHIFT			28
>> -#define GEN9_DECOUPLED_OP_SHIFT			24
>> -
>>  /* Per-pipe DDI Function Control */
>>  #define _TRANS_DDI_FUNC_CTL_A		0x60400
>>  #define _TRANS_DDI_FUNC_CTL_B		0x61400
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index a9a6933..3901800 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -400,8 +400,6 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
>>  static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>>  					  bool restore_forcewake)
>>  {
>> -	struct intel_device_info *info = mkwrite_device_info(dev_priv);
>> -
>>  	/* clear out unclaimed reg detection bit */
>>  	if (check_for_unclaimed_mmio(dev_priv))
>>  		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
>> @@ -414,9 +412,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
>>  				   GT_FIFO_CTL_RC6_POLICY_STALL);
>>  	}
>>
>> -	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
>> -		info->has_decoupled_mmio = false;
>> -
>>  	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
>>  }
>>
>> @@ -801,78 +796,6 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
>>  	__unclaimed_reg_debug(dev_priv, reg, read, before);
>>  }
>>
>> -enum decoupled_power_domain {
>> -	GEN9_DECOUPLED_PD_BLITTER = 0,
>> -	GEN9_DECOUPLED_PD_RENDER,
>> -	GEN9_DECOUPLED_PD_MEDIA,
>> -	GEN9_DECOUPLED_PD_ALL
>> -};
>> -
>> -enum decoupled_ops {
>> -	GEN9_DECOUPLED_OP_WRITE = 0,
>> -	GEN9_DECOUPLED_OP_READ
>> -};
>> -
>> -static const enum decoupled_power_domain fw2dpd_domain[] = {
>> -	GEN9_DECOUPLED_PD_RENDER,
>> -	GEN9_DECOUPLED_PD_BLITTER,
>> -	GEN9_DECOUPLED_PD_ALL,
>> -	GEN9_DECOUPLED_PD_MEDIA,
>> -	GEN9_DECOUPLED_PD_ALL,
>> -	GEN9_DECOUPLED_PD_ALL,
>> -	GEN9_DECOUPLED_PD_ALL
>> -};
>> -
>> -/*
>> - * Decoupled MMIO access for only 1 DWORD
>> - */
>> -static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
>> -					 u32 reg,
>> -					 enum forcewake_domains fw_domain,
>> -					 enum decoupled_ops operation)
>> -{
>> -	enum decoupled_power_domain dp_domain;
>> -	u32 ctrl_reg_data = 0;
>> -
>> -	dp_domain = fw2dpd_domain[fw_domain - 1];
>> -
>> -	ctrl_reg_data |= reg;
>> -	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
>> -	ctrl_reg_data |= (dp_domain << GEN9_DECOUPLED_PD_SHIFT);
>> -	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
>> -	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
>> -
>> -	if (wait_for_atomic((__raw_i915_read32(dev_priv,
>> -			    GEN9_DECOUPLED_REG0_DW1) &
>> -			    GEN9_DECOUPLED_DW1_GO) == 0,
>> -			    FORCEWAKE_ACK_TIMEOUT_MS))
>> -		DRM_ERROR("Decoupled MMIO wait timed out\n");
>> -}
>> -
>> -static inline u32
>> -__gen9_decoupled_mmio_read32(struct drm_i915_private *dev_priv,
>> -			     u32 reg,
>> -			     enum forcewake_domains fw_domain)
>> -{
>> -	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
>> -				     GEN9_DECOUPLED_OP_READ);
>> -
>> -	return __raw_i915_read32(dev_priv, GEN9_DECOUPLED_REG0_DW0);
>> -}
>> -
>> -static inline void
>> -__gen9_decoupled_mmio_write(struct drm_i915_private *dev_priv,
>> -			    u32 reg, u32 data,
>> -			    enum forcewake_domains fw_domain)
>> -{
>> -
>> -	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW0, data);
>> -
>> -	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
>> -				     GEN9_DECOUPLED_OP_WRITE);
>> -}
>> -
>> -
>>  #define GEN2_READ_HEADER(x) \
>>  	u##x val = 0; \
>>  	assert_rpm_wakelock_held(dev_priv);
>> @@ -969,28 +892,6 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
>>  #define __gen6_read(x) __gen_read(gen6, x)
>>  #define __fwtable_read(x) __gen_read(fwtable, x)
>>
>> -#define __gen9_decoupled_read(x) \
>> -static u##x \
>> -gen9_decoupled_read##x(struct drm_i915_private *dev_priv, \
>> -		       i915_reg_t reg, bool trace) { \
>> -	enum forcewake_domains fw_engine; \
>> -	GEN6_READ_HEADER(x); \
>> -	fw_engine = __fwtable_reg_read_fw_domains(offset); \
>> -	if (fw_engine & ~dev_priv->uncore.fw_domains_active) { \
>> -		unsigned i; \
>> -		u32 *ptr_data = (u32 *) &val; \
>> -		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
>> -			*ptr_data = __gen9_decoupled_mmio_read32(dev_priv, \
>> -								 offset, \
>> -								 fw_engine); \
>> -	} else { \
>> -		val = __raw_i915_read##x(dev_priv, reg); \
>> -	} \
>> -	GEN6_READ_FOOTER; \
>> -}
>> -
>> -__gen9_decoupled_read(32)
>> -__gen9_decoupled_read(64)
>>  __fwtable_read(8)
>>  __fwtable_read(16)
>>  __fwtable_read(32)
>> @@ -1077,25 +978,6 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
>>  #define __gen8_write(x) __gen_write(gen8, x)
>>  #define __fwtable_write(x) __gen_write(fwtable, x)
>>
>> -#define __gen9_decoupled_write(x) \
>> -static void \
>> -gen9_decoupled_write##x(struct drm_i915_private *dev_priv, \
>> -			i915_reg_t reg, u##x val, \
>> -		bool trace) { \
>> -	enum forcewake_domains fw_engine; \
>> -	GEN6_WRITE_HEADER; \
>> -	fw_engine = __fwtable_reg_write_fw_domains(offset); \
>> -	if (fw_engine & ~dev_priv->uncore.fw_domains_active) \
>> -		__gen9_decoupled_mmio_write(dev_priv, \
>> -					    offset, \
>> -					    val, \
>> -					    fw_engine); \
>> -	else \
>> -		__raw_i915_write##x(dev_priv, reg, val); \
>> -	GEN6_WRITE_FOOTER; \
>> -}
>> -
>> -__gen9_decoupled_write(32)
>>  __fwtable_write(8)
>>  __fwtable_write(16)
>>  __fwtable_write(32)
>> @@ -1332,14 +1214,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
>>  		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
>>  		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
>>  		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
>> -		if (HAS_DECOUPLED_MMIO(dev_priv)) {
>> -			dev_priv->uncore.funcs.mmio_readl =
>> -						gen9_decoupled_read32;
>> -			dev_priv->uncore.funcs.mmio_readq =
>> -						gen9_decoupled_read64;
>> -			dev_priv->uncore.funcs.mmio_writel =
>> -						gen9_decoupled_write32;
>> -		}
>>  	}
>>
>>  	iosf_mbi_register_pmic_bus_access_notifier(
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[kai.chen]

From: Kai Chen <kai.chen@intel.com>

The decoupled mmio feature doesn't work as intended by HW team. Enabling
it with forcewake will only make debugging efforts more difficult, so
let's just simply remove it.

v2:
- Remove dead code related to GEN9LP decoupled mmio.
- Change backgrounds: In theory, decoupled mmio should require less cycles
  for single read/write operation by avoiding frequent software forcewake.
  However, it turns out this design not to be true on HW practice and not to
  provide any decoupling benefit. It also introduces problems which cause
  failures in intel-gpu-tools (gem), and also cause driver code and debugging
  more complex.
- This change therefore reverts:

  commit 85ee17ebeedd1af0dccd98f82ab4e644e29d84c0
  Author: Praveen Paneri <praveen.paneri@intel.com>
  Date:   Tue Nov 15 22:49:20 2016 +0530

      drm/i915/bxt: Broxton decoupled MMIO

V3:
- Remove HAS_DECOUPLED_MMIO macro and has_decoupled_mmio flag, as they are not
  needed, either.
- This patch will be on top with drm-intel.

V4:
- Add signed-off-by.

Signed-off-by: Kai Chen <kai.chen@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h     |   3 -
 drivers/gpu/drm/i915/i915_pci.c     |   1 -
 drivers/gpu/drm/i915/i915_reg.h     |   7 --
 drivers/gpu/drm/i915/intel_uncore.c | 126 ------------------------------------
 4 files changed, 137 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a6f2047..41ff031 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -703,7 +703,6 @@ struct intel_csr {
 	func(has_aliasing_ppgtt); \
 	func(has_csr); \
 	func(has_ddi); \
-	func(has_decoupled_mmio); \
 	func(has_dp_mst); \
 	func(has_fbc); \
 	func(has_fpga_dbg); \
@@ -2944,8 +2943,6 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define GT_FREQUENCY_MULTIPLIER 50
 #define GEN9_FREQ_SCALER 3
 
-#define HAS_DECOUPLED_MMIO(dev_priv) (INTEL_INFO(dev_priv)->has_decoupled_mmio)
-
 #include "i915_trace.h"
 
 static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f80db2c..cf43dc1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -385,7 +385,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
 	.has_gmbus_irq = 1, \
 	.has_logical_ring_contexts = 1, \
 	.has_guc = 1, \
-	.has_decoupled_mmio = 1, \
 	.has_aliasing_ppgtt = 1, \
 	.has_full_ppgtt = 1, \
 	.has_full_48bit_ppgtt = 1, \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee144ec..78872f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7792,13 +7792,6 @@ enum {
 #define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
 #define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
 
-/* Decoupled MMIO register pair for kernel driver */
-#define GEN9_DECOUPLED_REG0_DW0			_MMIO(0xF00)
-#define GEN9_DECOUPLED_REG0_DW1			_MMIO(0xF04)
-#define GEN9_DECOUPLED_DW1_GO			(1<<31)
-#define GEN9_DECOUPLED_PD_SHIFT			28
-#define GEN9_DECOUPLED_OP_SHIFT			24
-
 /* Per-pipe DDI Function Control */
 #define _TRANS_DDI_FUNC_CTL_A		0x60400
 #define _TRANS_DDI_FUNC_CTL_B		0x61400
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a9a6933..3901800 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,8 +400,6 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
 static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 					  bool restore_forcewake)
 {
-	struct intel_device_info *info = mkwrite_device_info(dev_priv);
-
 	/* clear out unclaimed reg detection bit */
 	if (check_for_unclaimed_mmio(dev_priv))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
@@ -414,9 +412,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 				   GT_FIFO_CTL_RC6_POLICY_STALL);
 	}
 
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
-		info->has_decoupled_mmio = false;
-
 	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
 }
 
@@ -801,78 +796,6 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
 	__unclaimed_reg_debug(dev_priv, reg, read, before);
 }
 
-enum decoupled_power_domain {
-	GEN9_DECOUPLED_PD_BLITTER = 0,
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-enum decoupled_ops {
-	GEN9_DECOUPLED_OP_WRITE = 0,
-	GEN9_DECOUPLED_OP_READ
-};
-
-static const enum decoupled_power_domain fw2dpd_domain[] = {
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_BLITTER,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-/*
- * Decoupled MMIO access for only 1 DWORD
- */
-static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
-					 u32 reg,
-					 enum forcewake_domains fw_domain,
-					 enum decoupled_ops operation)
-{
-	enum decoupled_power_domain dp_domain;
-	u32 ctrl_reg_data = 0;
-
-	dp_domain = fw2dpd_domain[fw_domain - 1];
-
-	ctrl_reg_data |= reg;
-	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
-	ctrl_reg_data |= (dp_domain << GEN9_DECOUPLED_PD_SHIFT);
-	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
-
-	if (wait_for_atomic((__raw_i915_read32(dev_priv,
-			    GEN9_DECOUPLED_REG0_DW1) &
-			    GEN9_DECOUPLED_DW1_GO) == 0,
-			    FORCEWAKE_ACK_TIMEOUT_MS))
-		DRM_ERROR("Decoupled MMIO wait timed out\n");
-}
-
-static inline u32
-__gen9_decoupled_mmio_read32(struct drm_i915_private *dev_priv,
-			     u32 reg,
-			     enum forcewake_domains fw_domain)
-{
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_READ);
-
-	return __raw_i915_read32(dev_priv, GEN9_DECOUPLED_REG0_DW0);
-}
-
-static inline void
-__gen9_decoupled_mmio_write(struct drm_i915_private *dev_priv,
-			    u32 reg, u32 data,
-			    enum forcewake_domains fw_domain)
-{
-
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW0, data);
-
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_WRITE);
-}
-
-
 #define GEN2_READ_HEADER(x) \
 	u##x val = 0; \
 	assert_rpm_wakelock_held(dev_priv);
@@ -969,28 +892,6 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
 #define __gen6_read(x) __gen_read(gen6, x)
 #define __fwtable_read(x) __gen_read(fwtable, x)
 
-#define __gen9_decoupled_read(x) \
-static u##x \
-gen9_decoupled_read##x(struct drm_i915_private *dev_priv, \
-		       i915_reg_t reg, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_READ_HEADER(x); \
-	fw_engine = __fwtable_reg_read_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) { \
-		unsigned i; \
-		u32 *ptr_data = (u32 *) &val; \
-		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
-			*ptr_data = __gen9_decoupled_mmio_read32(dev_priv, \
-								 offset, \
-								 fw_engine); \
-	} else { \
-		val = __raw_i915_read##x(dev_priv, reg); \
-	} \
-	GEN6_READ_FOOTER; \
-}
-
-__gen9_decoupled_read(32)
-__gen9_decoupled_read(64)
 __fwtable_read(8)
 __fwtable_read(16)
 __fwtable_read(32)
@@ -1077,25 +978,6 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
 #define __gen8_write(x) __gen_write(gen8, x)
 #define __fwtable_write(x) __gen_write(fwtable, x)
 
-#define __gen9_decoupled_write(x) \
-static void \
-gen9_decoupled_write##x(struct drm_i915_private *dev_priv, \
-			i915_reg_t reg, u##x val, \
-		bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_WRITE_HEADER; \
-	fw_engine = __fwtable_reg_write_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) \
-		__gen9_decoupled_mmio_write(dev_priv, \
-					    offset, \
-					    val, \
-					    fw_engine); \
-	else \
-		__raw_i915_write##x(dev_priv, reg, val); \
-	GEN6_WRITE_FOOTER; \
-}
-
-__gen9_decoupled_write(32)
 __fwtable_write(8)
 __fwtable_write(16)
 __fwtable_write(32)
@@ -1332,14 +1214,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
 		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
 		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
-		if (HAS_DECOUPLED_MMIO(dev_priv)) {
-			dev_priv->uncore.funcs.mmio_readl =
-						gen9_decoupled_read32;
-			dev_priv->uncore.funcs.mmio_readq =
-						gen9_decoupled_read64;
-			dev_priv->uncore.funcs.mmio_writel =
-						gen9_decoupled_write32;
-		}
 	}
 
 	iosf_mbi_register_pmic_bus_access_notifier(
-- 
2.9.3

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

[PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[kai.chen]

From: Kai Chen <kai.chen@intel.com>

The decoupled mmio feature doesn't work as intended by HW team. Enabling
it with forcewake will only make debugging efforts more difficult, so
let's just simply remove it.

v2:
- Remove dead code related to GEN9LP decoupled mmio.
- Change backgrounds: In theory, decoupled mmio should require less cycles
  for single read/write operation by avoiding frequent software forcewake.
  However, it turns out this design not to be true on HW practice and not to
  provide any decoupling benefit. It also introduces problems which cause
  failures in intel-gpu-tools (gem), and also cause driver code and debugging
  more complex.
- This change therefore reverts:

  commit 85ee17ebeedd1af0dccd98f82ab4e644e29d84c0
  Author: Praveen Paneri <praveen.paneri@intel.com>
  Date:   Tue Nov 15 22:49:20 2016 +0530

      drm/i915/bxt: Broxton decoupled MMIO

V3:
- Remove HAS_DECOUPLED_MMIO macro and has_decoupled_mmio flag, as they are not
  needed, either.
- This patch will be on top with drm-intel.
---
 drivers/gpu/drm/i915/i915_drv.h     |   3 -
 drivers/gpu/drm/i915/i915_pci.c     |   1 -
 drivers/gpu/drm/i915/i915_reg.h     |   7 --
 drivers/gpu/drm/i915/intel_uncore.c | 126 ------------------------------------
 4 files changed, 137 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a6f2047..41ff031 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -703,7 +703,6 @@ struct intel_csr {
 	func(has_aliasing_ppgtt); \
 	func(has_csr); \
 	func(has_ddi); \
-	func(has_decoupled_mmio); \
 	func(has_dp_mst); \
 	func(has_fbc); \
 	func(has_fpga_dbg); \
@@ -2944,8 +2943,6 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define GT_FREQUENCY_MULTIPLIER 50
 #define GEN9_FREQ_SCALER 3
 
-#define HAS_DECOUPLED_MMIO(dev_priv) (INTEL_INFO(dev_priv)->has_decoupled_mmio)
-
 #include "i915_trace.h"
 
 static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f80db2c..cf43dc1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -385,7 +385,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
 	.has_gmbus_irq = 1, \
 	.has_logical_ring_contexts = 1, \
 	.has_guc = 1, \
-	.has_decoupled_mmio = 1, \
 	.has_aliasing_ppgtt = 1, \
 	.has_full_ppgtt = 1, \
 	.has_full_48bit_ppgtt = 1, \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index ee144ec..78872f9 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7792,13 +7792,6 @@ enum {
 #define  SKL_FUSE_PG1_DIST_STATUS              (1<<26)
 #define  SKL_FUSE_PG2_DIST_STATUS              (1<<25)
 
-/* Decoupled MMIO register pair for kernel driver */
-#define GEN9_DECOUPLED_REG0_DW0			_MMIO(0xF00)
-#define GEN9_DECOUPLED_REG0_DW1			_MMIO(0xF04)
-#define GEN9_DECOUPLED_DW1_GO			(1<<31)
-#define GEN9_DECOUPLED_PD_SHIFT			28
-#define GEN9_DECOUPLED_OP_SHIFT			24
-
 /* Per-pipe DDI Function Control */
 #define _TRANS_DDI_FUNC_CTL_A		0x60400
 #define _TRANS_DDI_FUNC_CTL_B		0x61400
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index a9a6933..3901800 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -400,8 +400,6 @@ check_for_unclaimed_mmio(struct drm_i915_private *dev_priv)
 static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 					  bool restore_forcewake)
 {
-	struct intel_device_info *info = mkwrite_device_info(dev_priv);
-
 	/* clear out unclaimed reg detection bit */
 	if (check_for_unclaimed_mmio(dev_priv))
 		DRM_DEBUG("unclaimed mmio detected on uncore init, clearing\n");
@@ -414,9 +412,6 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
 				   GT_FIFO_CTL_RC6_POLICY_STALL);
 	}
 
-	if (IS_BXT_REVID(dev_priv, 0, BXT_REVID_B_LAST))
-		info->has_decoupled_mmio = false;
-
 	intel_uncore_forcewake_reset(dev_priv, restore_forcewake);
 }
 
@@ -801,78 +796,6 @@ unclaimed_reg_debug(struct drm_i915_private *dev_priv,
 	__unclaimed_reg_debug(dev_priv, reg, read, before);
 }
 
-enum decoupled_power_domain {
-	GEN9_DECOUPLED_PD_BLITTER = 0,
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-enum decoupled_ops {
-	GEN9_DECOUPLED_OP_WRITE = 0,
-	GEN9_DECOUPLED_OP_READ
-};
-
-static const enum decoupled_power_domain fw2dpd_domain[] = {
-	GEN9_DECOUPLED_PD_RENDER,
-	GEN9_DECOUPLED_PD_BLITTER,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_MEDIA,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL,
-	GEN9_DECOUPLED_PD_ALL
-};
-
-/*
- * Decoupled MMIO access for only 1 DWORD
- */
-static void __gen9_decoupled_mmio_access(struct drm_i915_private *dev_priv,
-					 u32 reg,
-					 enum forcewake_domains fw_domain,
-					 enum decoupled_ops operation)
-{
-	enum decoupled_power_domain dp_domain;
-	u32 ctrl_reg_data = 0;
-
-	dp_domain = fw2dpd_domain[fw_domain - 1];
-
-	ctrl_reg_data |= reg;
-	ctrl_reg_data |= (operation << GEN9_DECOUPLED_OP_SHIFT);
-	ctrl_reg_data |= (dp_domain << GEN9_DECOUPLED_PD_SHIFT);
-	ctrl_reg_data |= GEN9_DECOUPLED_DW1_GO;
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW1, ctrl_reg_data);
-
-	if (wait_for_atomic((__raw_i915_read32(dev_priv,
-			    GEN9_DECOUPLED_REG0_DW1) &
-			    GEN9_DECOUPLED_DW1_GO) == 0,
-			    FORCEWAKE_ACK_TIMEOUT_MS))
-		DRM_ERROR("Decoupled MMIO wait timed out\n");
-}
-
-static inline u32
-__gen9_decoupled_mmio_read32(struct drm_i915_private *dev_priv,
-			     u32 reg,
-			     enum forcewake_domains fw_domain)
-{
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_READ);
-
-	return __raw_i915_read32(dev_priv, GEN9_DECOUPLED_REG0_DW0);
-}
-
-static inline void
-__gen9_decoupled_mmio_write(struct drm_i915_private *dev_priv,
-			    u32 reg, u32 data,
-			    enum forcewake_domains fw_domain)
-{
-
-	__raw_i915_write32(dev_priv, GEN9_DECOUPLED_REG0_DW0, data);
-
-	__gen9_decoupled_mmio_access(dev_priv, reg, fw_domain,
-				     GEN9_DECOUPLED_OP_WRITE);
-}
-
-
 #define GEN2_READ_HEADER(x) \
 	u##x val = 0; \
 	assert_rpm_wakelock_held(dev_priv);
@@ -969,28 +892,6 @@ func##_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) {
 #define __gen6_read(x) __gen_read(gen6, x)
 #define __fwtable_read(x) __gen_read(fwtable, x)
 
-#define __gen9_decoupled_read(x) \
-static u##x \
-gen9_decoupled_read##x(struct drm_i915_private *dev_priv, \
-		       i915_reg_t reg, bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_READ_HEADER(x); \
-	fw_engine = __fwtable_reg_read_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) { \
-		unsigned i; \
-		u32 *ptr_data = (u32 *) &val; \
-		for (i = 0; i < x/32; i++, offset += sizeof(u32), ptr_data++) \
-			*ptr_data = __gen9_decoupled_mmio_read32(dev_priv, \
-								 offset, \
-								 fw_engine); \
-	} else { \
-		val = __raw_i915_read##x(dev_priv, reg); \
-	} \
-	GEN6_READ_FOOTER; \
-}
-
-__gen9_decoupled_read(32)
-__gen9_decoupled_read(64)
 __fwtable_read(8)
 __fwtable_read(16)
 __fwtable_read(32)
@@ -1077,25 +978,6 @@ func##_write##x(struct drm_i915_private *dev_priv, i915_reg_t reg, u##x val, boo
 #define __gen8_write(x) __gen_write(gen8, x)
 #define __fwtable_write(x) __gen_write(fwtable, x)
 
-#define __gen9_decoupled_write(x) \
-static void \
-gen9_decoupled_write##x(struct drm_i915_private *dev_priv, \
-			i915_reg_t reg, u##x val, \
-		bool trace) { \
-	enum forcewake_domains fw_engine; \
-	GEN6_WRITE_HEADER; \
-	fw_engine = __fwtable_reg_write_fw_domains(offset); \
-	if (fw_engine & ~dev_priv->uncore.fw_domains_active) \
-		__gen9_decoupled_mmio_write(dev_priv, \
-					    offset, \
-					    val, \
-					    fw_engine); \
-	else \
-		__raw_i915_write##x(dev_priv, reg, val); \
-	GEN6_WRITE_FOOTER; \
-}
-
-__gen9_decoupled_write(32)
 __fwtable_write(8)
 __fwtable_write(16)
 __fwtable_write(32)
@@ -1332,14 +1214,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
 		ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges);
 		ASSIGN_WRITE_MMIO_VFUNCS(dev_priv, fwtable);
 		ASSIGN_READ_MMIO_VFUNCS(dev_priv, fwtable);
-		if (HAS_DECOUPLED_MMIO(dev_priv)) {
-			dev_priv->uncore.funcs.mmio_readl =
-						gen9_decoupled_read32;
-			dev_priv->uncore.funcs.mmio_readq =
-						gen9_decoupled_read64;
-			dev_priv->uncore.funcs.mmio_writel =
-						gen9_decoupled_write32;
-		}
 	}
 
 	iosf_mbi_register_pmic_bus_access_notifier(
-- 
2.9.3

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

Re: [PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[Chris Wilson]

On Mon, May 15, 2017 at 04:18:27PM -0700, kai.chen@intel.com wrote:
> From: Kai Chen <kai.chen@intel.com>
> 
> The decoupled mmio feature doesn't work as intended by HW team. Enabling
> it with forcewake will only make debugging efforts more difficult, so
> let's just simply remove it.

Is the code going to be reused in the near future, i.e. why not remove
the dead code as well?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Disable decoupled mmio for GEN9LP

[kai.chen]

From: Kai Chen <kai.chen@intel.com>

The decoupled mmio feature doesn't work as intended by HW team. Enabling
it with forcewake will only make debugging efforts more difficult, so
let's just simply remove it.

Signed-off-by: Kai Chen <kai.chen@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index f9cfcc2..449de19 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -384,7 +384,6 @@ static const struct intel_device_info intel_skylake_gt3_info = {
 	.has_hw_contexts = 1, \
 	.has_logical_ring_contexts = 1, \
 	.has_guc = 1, \
-	.has_decoupled_mmio = 1, \
 	.has_aliasing_ppgtt = 1, \
 	.has_full_ppgtt = 1, \
 	.has_full_48bit_ppgtt = 1, \
-- 
2.9.3

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