>-----Original Message----- >From: Tvrtko Ursulin [mailto:tvrtko.ursu...@linux.intel.com] >Sent: Friday, March 17, 2017 5:30 PM >To: Zhenyu Wang <zhen...@linux.intel.com> >Cc: Chris Wilson <ch...@chris-wilson.co.uk>; Tvrtko Ursulin ><tursu...@ursulin.net>; Intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko ><tvrtko.ursu...@intel.com>; Li, Weinan Z <weinan.z...@intel.com>; Xu, >Terrence <terrence...@intel.com> >Subject: Re: [Intel-gfx] [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU >more thouroughly > > >Hi, > >On 13/03/2017 09:37, Zhenyu Wang wrote: >> On 2017.03.13 09:26:26 +0000, Tvrtko Ursulin wrote: >>> >>> On 10/03/2017 10:09, Chris Wilson wrote: >>>> On Fri, Mar 10, 2017 at 09:57:47AM +0000, Tvrtko Ursulin wrote: >>>>> From: Tvrtko Ursulin <tvrtko.ursu...@intel.com> >>>>> >>>>> If we avoid initializing forcewake domains when running as a guest, >>>>> and also use gen2 mmio accessors in that case, we can avoid the >>>>> timer traffic and any looping through the forcewake code which is >>>>> currently just so it can end up in the no-op forcewake >>>>> implementation. >>>>> >>>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@intel.com> >>>>> Cc: Weinan Li <weinan.z...@intel.com> >>>>> Cc: Chris Wilson <ch...@chris-wilson.co.uk> >>>>> --- >>>>> drivers/gpu/drm/i915/intel_uncore.c | 76 >>>>> +++++++++++++------------------------ >>>>> 1 file changed, 27 insertions(+), 49 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c >>>>> b/drivers/gpu/drm/i915/intel_uncore.c >>>>> index 71b9b387ad04..09f5f02d7901 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_uncore.c >>>>> +++ b/drivers/gpu/drm/i915/intel_uncore.c >>>>> @@ -138,13 +138,6 @@ fw_domains_put(struct drm_i915_private >>>>> *dev_priv, enum forcewake_domains fw_doma } >>>>> >>>>> static void >>>>> -vgpu_fw_domains_nop(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; @@ -1187,7 +1180,7 >@@ >>>>> static void fw_domain_init(struct drm_i915_private *dev_priv, >>>>> >>>>> static void intel_uncore_fw_domains_init(struct drm_i915_private >>>>> *dev_priv) { >>>>> - if (INTEL_INFO(dev_priv)->gen <= 5) >>>>> + if (INTEL_GEN(dev_priv) <= 5 || intel_vgpu_active(dev_priv)) >>>> >>>> Make these separate ifs, they aren't semantically related so be verbose. >>>> >>>>> return; >>>>> >>>>> if (IS_GEN9(dev_priv)) { >>>>> @@ -1273,11 +1266,6 @@ 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_nop; >>>>> - dev_priv->uncore.funcs.force_wake_put = >vgpu_fw_domains_nop; >>>>> - } >>>>> - >>>>> /* All future platforms are expected to require complex power gating >*/ >>>>> WARN_ON(dev_priv->uncore.fw_domains == 0); } @@ -1327,22 >>>>> +1315,22 @@ void intel_uncore_init(struct drm_i915_private >*dev_priv) >>>>> dev_priv->uncore.pmic_bus_access_nb.notifier_call = >>>>> i915_pmic_bus_access_notifier; >>>>> >>>>> - switch (INTEL_INFO(dev_priv)->gen) { >>>>> - default: >>>>> - case 9: >>>>> - ASSIGN_FW_DOMAINS_TABLE(__gen9_fw_ranges); >>>>> - ASSIGN_WRITE_MMIO_VFUNCS(fwtable); >>>>> - ASSIGN_READ_MMIO_VFUNCS(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; >>>>> + if (IS_GEN(dev_priv, 2, 4) || intel_vgpu_active(dev_priv)) { >>>> >>>> Ok, this doesn't look too bad. >>>> >>>> Do the gvt-g hosts in CI now provide coverage for us of vgpu paths? >>> >>> No idea. >>> >>> Adding Zhenyu. So this patch avoids burning CPU cycles in guests and >>> scheduling timers when all of that ends up in the dummy/no-op >>> forcewake implementation. >>> >>> If interesting to you, would it be easy for you to test it or how >>> should we proceed? >>> >> >> Patch looks fine to me. I can apply it for our QA testing if required. > >Were you perhaps able to smoke test this one?
Hi Ursulin, we have verified your patch in guest, no regression be found. Tested-by: Terrence Xu <terrence...@intel.com> >Regards, > >Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
