diff mbox

[2/2] drm/i915: ignore forcewake get/put when using vgpu

Message ID 1485324296-14995-2-git-send-email-weinan.z.li@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Li, Weinan Z Jan. 25, 2017, 6:04 a.m. UTC 
Host maintian the hardware's forcewake state, guest don't need and also
can't control it. Although vgpu_read/write bypass forcewake_get/put in MMIO
read/write, but still have separate path called by
"intel_uncore_forcewake_get/put" and
"intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in guest
waste much CPU cost. Since we full virtualize the MMIO, just ignore the
forcewake get/put in low level.

Signed-off-by: Weinan Li <weinan.z.li@intel.com>
---
 drivers/gpu/drm/i915/intel_uncore.c | 78 ++++++++++---------------------------
 1 file changed, 20 insertions(+), 58 deletions(-)

Comments

Chris Wilson Jan. 25, 2017, 7:16 a.m. UTC | #1 
On Wed, Jan 25, 2017 at 02:04:56PM +0800, Weinan Li wrote:
> Host maintian the hardware's forcewake state, guest don't need and also
> can't control it. Although vgpu_read/write bypass forcewake_get/put in MMIO
> read/write, but still have separate path called by
> "intel_uncore_forcewake_get/put" and
> "intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in guest
> waste much CPU cost. Since we full virtualize the MMIO, just ignore the
> forcewake get/put in low level.

This patch is doing multiple things as you are altering a couple of
independent paths at the same time (fw control and mmio funcs).

>  static void
> +vgpu_fw_domains_get(struct drm_i915_private *dev_priv,
> +		    enum forcewake_domains fw_domains)
> +{
> +	/* Guest driver doesn't need to takes care forcewake. */;
> +}
> +
> +static void
> +vgpu_fw_domains_put(struct drm_i915_private *dev_priv,
> +		    enum forcewake_domains fw_domains)
> +{
> +	/* Guest driver doesn't need to takes care forcewake. */;
> +}

Or just 1 vgpu_fw_domains_nop()
-Chris
Li, Weinan Z Jan. 25, 2017, 12:37 p.m. UTC | #2 
> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Wednesday, January 25, 2017 3:17 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: ignore forcewake get/put
> when using vgpu
> 
> On Wed, Jan 25, 2017 at 02:04:56PM +0800, Weinan Li wrote:
> > Host maintian the hardware's forcewake state, guest don't need and
> > also can't control it. Although vgpu_read/write bypass
> > forcewake_get/put in MMIO read/write, but still have separate path
> > called by "intel_uncore_forcewake_get/put" and
> > "intel_uncore_forcewake_get/put__locked". Unnecessary MMIO access in
> > guest waste much CPU cost. Since we full virtualize the MMIO, just
> > ignore the forcewake get/put in low level.
> 
> This patch is doing multiple things as you are altering a couple of independent
> paths at the same time (fw control and mmio funcs).
> 
ok, let me separate 2 patches, 1 fw control, 2 remove vgpu_read/write funcs.
> >  static void
> > +vgpu_fw_domains_get(struct drm_i915_private *dev_priv,
> > +		    enum forcewake_domains fw_domains) {
> > +	/* Guest driver doesn't need to takes care forcewake. */; }
> > +
> > +static void
> > +vgpu_fw_domains_put(struct drm_i915_private *dev_priv,
> > +		    enum forcewake_domains fw_domains) {
> > +	/* Guest driver doesn't need to takes care forcewake. */; }
> 
> Or just 1 vgpu_fw_domains_nop()
vgpu_fw_domains_nop more simple and readable. 
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index abe0888..08e1b5f 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -133,6 +133,20 @@ 
 }
 
 static void
+vgpu_fw_domains_get(struct drm_i915_private *dev_priv,
+		    enum forcewake_domains fw_domains)
+{
+	/* Guest driver doesn't need to takes care forcewake. */;
+}
+
+static void
+vgpu_fw_domains_put(struct drm_i915_private *dev_priv,
+		    enum forcewake_domains fw_domains)
+{
+	/* Guest driver doesn't need to takes care forcewake. */;
+}
+
+static void
 fw_domains_posting_read(struct drm_i915_private *dev_priv)
 {
 	struct intel_uncore_forcewake_domain *d;
@@ -1045,34 +1059,6 @@  static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
 #undef GEN6_READ_FOOTER
 #undef GEN6_READ_HEADER
 
-#define VGPU_READ_HEADER(x) \
-	unsigned long irqflags; \
-	u##x val = 0; \
-	assert_rpm_device_not_suspended(dev_priv); \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
-
-#define VGPU_READ_FOOTER \
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags); \
-	trace_i915_reg_rw(false, reg, val, sizeof(val), trace); \
-	return val
-
-#define __vgpu_read(x) \
-static u##x \
-vgpu_read##x(struct drm_i915_private *dev_priv, i915_reg_t reg, bool trace) { \
-	VGPU_READ_HEADER(x); \
-	val = __raw_i915_read##x(dev_priv, reg); \
-	VGPU_READ_FOOTER; \
-}
-
-__vgpu_read(8)
-__vgpu_read(16)
-__vgpu_read(32)
-__vgpu_read(64)
-
-#undef __vgpu_read
-#undef VGPU_READ_FOOTER
-#undef VGPU_READ_HEADER
-
 #define GEN2_WRITE_HEADER \
 	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
 	assert_rpm_wakelock_held(dev_priv); \
@@ -1195,31 +1181,6 @@  static inline void __force_wake_auto(struct drm_i915_private *dev_priv,
 #undef GEN6_WRITE_FOOTER
 #undef GEN6_WRITE_HEADER
 
-#define VGPU_WRITE_HEADER \
-	unsigned long irqflags; \
-	trace_i915_reg_rw(true, reg, val, sizeof(val), trace); \
-	assert_rpm_device_not_suspended(dev_priv); \
-	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags)
-
-#define VGPU_WRITE_FOOTER \
-	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags)
-
-#define __vgpu_write(x) \
-static void vgpu_write##x(struct drm_i915_private *dev_priv, \
-			  i915_reg_t reg, u##x val, bool trace) { \
-	VGPU_WRITE_HEADER; \
-	__raw_i915_write##x(dev_priv, reg, val); \
-	VGPU_WRITE_FOOTER; \
-}
-
-__vgpu_write(8)
-__vgpu_write(16)
-__vgpu_write(32)
-
-#undef __vgpu_write
-#undef VGPU_WRITE_FOOTER
-#undef VGPU_WRITE_HEADER
-
 #define ASSIGN_WRITE_MMIO_VFUNCS(x) \
 do { \
 	dev_priv->uncore.funcs.mmio_writeb = x##_write8; \
@@ -1375,6 +1336,12 @@  static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
 			       FORCEWAKE, FORCEWAKE_ACK);
 	}
 
+	if (intel_vgpu_active(dev_priv)) {
+		dev_priv->uncore.funcs.force_wake_get =
+			vgpu_fw_domains_get;
+		dev_priv->uncore.funcs.force_wake_put =
+			vgpu_fw_domains_put;
+	}
 	/* All future platforms are expected to require complex power gating */
 	WARN_ON(dev_priv->uncore.fw_domains == 0);
 }
@@ -1449,11 +1416,6 @@  void intel_uncore_init(struct drm_i915_private *dev_priv)
 	if (INTEL_GEN(dev_priv) >= 8)
 		intel_shadow_table_check();
 
-	if (intel_vgpu_active(dev_priv)) {
-		ASSIGN_WRITE_MMIO_VFUNCS(vgpu);
-		ASSIGN_READ_MMIO_VFUNCS(vgpu);
-	}
-
 	i915_check_and_clear_faults(dev_priv);
 }
 #undef ASSIGN_WRITE_MMIO_VFUNCS