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

Zhenyu Wang zhenyuw at linux.intel.com
Wed Jan 25 02:36:02 UTC 2017


On 2017.01.25 02:37:08 +0000, Li, Weinan Z wrote:
> 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".
> The original problem comes from 4.4 kenel guest, it will access forcewake_get/put frequently when have heavy workload, unnecessary MMIO access in guest waste much CPU cost.
> So if we full virtualize the MMIO, shall we ignore the forcewake get/put in low level.
> 

yeah, that makes more sense. Pls update the description and send to intel-gfx.

thanks

> > 
> > On 2017.01.23 01:42:48 +0000, Tian, Kevin wrote:
> > > does it make sense to make below configurable?
> > >
> > 
> > VGPU_READ/WRITE is supposed to ignore forcewake setting for guest right?
> > Although this change can also do that, what's the real point of this?
> > 
> > And anyway for any i915 change, need to send to intel-gfx and cc gvt.
> > 
> > > > Subject: [PATCH 2/2] drm/i915: ignore forcewake get/put when using
> > > > vgpu
> > > >
> > > > Host maintian the hardware's forcewake state, guest don't need and
> > > > also can't control it, just ignore. Remove vgpu_read/write.
> > > >
> > > > Signed-off-by: Weinan Li <weinan.z.li at intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_uncore.c | 78
> > > > ++++++++++---------------------------
> > > >  1 file changed, 20 insertions(+), 58 deletions(-)
> > > >
> > > > 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
> > > > --
> > > > 1.9.1
> > > >
> > > > _______________________________________________
> > > > intel-gvt-dev mailing list
> > > > intel-gvt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> > > _______________________________________________
> > > intel-gvt-dev mailing list
> > > intel-gvt-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> > 
> > --
> > Open Source Technology Center, Intel ltd.
> > 
> > $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 163 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170125/ae25d8e6/attachment-0001.sig>


More information about the intel-gvt-dev mailing list