From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yu, Zhang" Subject: Re: [PATCH v2 7/8] drm/i915: Create vgpu specific write MMIO to reduce traps Date: Thu, 23 Oct 2014 15:10:12 +0800 Message-ID: <5448A9D4.4060503@linux.intel.com> References: <1413440668-12158-1-git-send-email-yu.c.zhang@linux.intel.com> <1413440668-12158-8-git-send-email-yu.c.zhang@linux.intel.com> <20141021164022.GC26941@phenom.ffwll.local> <5447A2C6.4000701@linux.intel.com> <20141022153329.GX26941@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 089DB6E3DA for ; Thu, 23 Oct 2014 00:16:34 -0700 (PDT) In-Reply-To: <20141022153329.GX26941@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 10/22/2014 11:33 PM, Daniel Vetter wrote: > On Wed, Oct 22, 2014 at 08:27:50PM +0800, Yu, Zhang wrote: >> >> >> On 10/22/2014 12:40 AM, Daniel Vetter wrote: >>> 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 >>>> Signed-off-by: Jike Song >>>> Signed-off-by: Kevin Tian >>>> --- >>>> 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 >>> >> Thanks Daniel. >> Do you mean something like this: >> #define ASSIGN_WRITE_MMIO_VFUNCS(x) \ >> do { \ >> dev_priv->uncore.funcs.mmio_writeb = x##_write8; \ >> dev_priv->uncore.funcs.mmio_writew = x##_write16; \ >> dev_priv->uncore.funcs.mmio_writel = x##_write32; \ >> dev_priv->uncore.funcs.mmio_writeq = x##_write64; \ >> } while (0) >> >> and then we can use ASSIGN_WRITE_MMIO_VFUNCS(hsw) for hsw and >> ASSIGN_WRITE_MMIO_VFUNCS(vgpu) for vgpu, etc? > > Yup. Plus the version for assigning READ vfuncs (on many platforms they > don't match up). Probably best if you do this conversion as a prep patch > before the vgt series so that I can merge it right away. > -Daniel Sure, thanks. I 'll send this patch later, before our v3 vgpu patch series. :) Yu >