Re: [PATCH v2 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps

From: Daniel Vetter <daniel@ffwll.ch>
To: Yu Zhang <yu.c.zhang@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps
Date: Tue, 21 Oct 2014 18:40:22 +0200	[thread overview]
Message-ID: <20141021164022.GC26941@phenom.ffwll.local> (raw)
In-Reply-To: <1413440668-12158-8-git-send-email-yu.c.zhang@linux.intel.com>

On Thu, Oct 16, 2014 at 02:24:27PM +0800, Yu Zhang wrote:
> In the virtualized environment, forcewake operations are not
> necessory for the driver, because mmio accesses will be trapped
> and emulated by the host side, and real forcewake operations are
> also done in the host. New mmio write handlers are added to directly
> call the __raw_i915_write, therefore will reduce many traps and
> increase the overall performance for drivers runing in the VM
> with Intel GVT-g enhancement.
> 
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Signed-off-by: Jike Song <jike.song@intel.com>
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index d5f39f3..ec6d5ce 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -719,6 +719,14 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>  	REG_WRITE_FOOTER; \
>  }
>  
> +#define __vgpu_write(x) \
> +static void \
> +vgpu_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
> +	REG_WRITE_HEADER; \
> +	__raw_i915_write##x(dev_priv, reg, val); \
> +	REG_WRITE_FOOTER; \
> +}
> +
>  static const u32 gen8_shadowed_regs[] = {
>  	FORCEWAKE_MT,
>  	GEN6_RPNSWREQ,
> @@ -813,6 +821,10 @@ __gen4_write(8)
>  __gen4_write(16)
>  __gen4_write(32)
>  __gen4_write(64)
> +__vgpu_write(8)
> +__vgpu_write(16)
> +__vgpu_write(32)
> +__vgpu_write(64)
>  
>  #undef __chv_write
>  #undef __gen8_write
> @@ -820,6 +832,7 @@ __gen4_write(64)
>  #undef __gen6_write
>  #undef __gen5_write
>  #undef __gen4_write
> +#undef __vgpu_write
>  #undef REG_WRITE_FOOTER
>  #undef REG_WRITE_HEADER
>  
> @@ -950,6 +963,13 @@ void intel_uncore_init(struct drm_device *dev)
>  		dev_priv->uncore.funcs.mmio_readq  = gen4_read64;
>  		break;
>  	}
> +
> +	if (intel_vgpu_active(dev)) {
> +		dev_priv->uncore.funcs.mmio_writeb = vgpu_write8;
> +		dev_priv->uncore.funcs.mmio_writew = vgpu_write16;
> +		dev_priv->uncore.funcs.mmio_writel = vgpu_write32;
> +		dev_priv->uncore.funcs.mmio_writeq = vgpu_write64;

Someone should write a cool macro which uses prepocessor string
concatenation so that we can compress this all to

	ASSIGN_WRITE_MMIO_VFUNCS(vgpu)

Then throw in an ASSIGN_READ_MMIO_VFUNC which looks similarly and this
might actually be pretty. Just an idea for some follow-up cleanup.
-Daniel

> +	}
>  }
>  
>  void intel_uncore_fini(struct drm_device *dev)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch