* [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly @ 2017-03-10 9:57 Tvrtko Ursulin 2017-03-10 10:09 ` Chris Wilson 2017-03-10 12:48 ` ✓ Fi.CI.BAT: success for " Patchwork 0 siblings, 2 replies; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-10 9:57 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@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.ursulin@intel.com> Cc: Weinan Li <weinan.z.li@intel.com> Cc: Chris Wilson <chris@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)) 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)) { + ASSIGN_WRITE_MMIO_VFUNCS(gen2); + ASSIGN_READ_MMIO_VFUNCS(gen2); + } else if (IS_GEN5(dev_priv)) { + ASSIGN_WRITE_MMIO_VFUNCS(gen5); + ASSIGN_READ_MMIO_VFUNCS(gen5); + } else if (IS_GEN(dev_priv, 6, 7)) { + ASSIGN_WRITE_MMIO_VFUNCS(gen6); + + if (IS_VALLEYVIEW(dev_priv)) { + ASSIGN_FW_DOMAINS_TABLE(__vlv_fw_ranges); + ASSIGN_READ_MMIO_VFUNCS(fwtable); + } else { + ASSIGN_READ_MMIO_VFUNCS(gen6); } - break; - case 8: + } else if (IS_GEN8(dev_priv)) { if (IS_CHERRYVIEW(dev_priv)) { ASSIGN_FW_DOMAINS_TABLE(__chv_fw_ranges); ASSIGN_WRITE_MMIO_VFUNCS(fwtable); @@ -1352,28 +1340,18 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) ASSIGN_WRITE_MMIO_VFUNCS(gen8); ASSIGN_READ_MMIO_VFUNCS(gen6); } - break; - case 7: - case 6: - ASSIGN_WRITE_MMIO_VFUNCS(gen6); - - if (IS_VALLEYVIEW(dev_priv)) { - ASSIGN_FW_DOMAINS_TABLE(__vlv_fw_ranges); - ASSIGN_READ_MMIO_VFUNCS(fwtable); - } else { - ASSIGN_READ_MMIO_VFUNCS(gen6); + } else { + 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; } - break; - case 5: - ASSIGN_WRITE_MMIO_VFUNCS(gen5); - ASSIGN_READ_MMIO_VFUNCS(gen5); - break; - case 4: - case 3: - case 2: - ASSIGN_WRITE_MMIO_VFUNCS(gen2); - ASSIGN_READ_MMIO_VFUNCS(gen2); - break; } iosf_mbi_register_pmic_bus_access_notifier( -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-10 9:57 [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly Tvrtko Ursulin @ 2017-03-10 10:09 ` Chris Wilson 2017-03-10 13:05 ` Mika Kuoppala 2017-03-13 9:26 ` Tvrtko Ursulin 2017-03-10 12:48 ` ✓ Fi.CI.BAT: success for " Patchwork 1 sibling, 2 replies; 12+ messages in thread From: Chris Wilson @ 2017-03-10 10:09 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Fri, Mar 10, 2017 at 09:57:47AM +0000, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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.ursulin@intel.com> > Cc: Weinan Li <weinan.z.li@intel.com> > Cc: Chris Wilson <chris@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? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-10 10:09 ` Chris Wilson @ 2017-03-10 13:05 ` Mika Kuoppala 2017-03-13 9:26 ` Tvrtko Ursulin 1 sibling, 0 replies; 12+ messages in thread From: Mika Kuoppala @ 2017-03-10 13:05 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin; +Cc: Intel-gfx Chris Wilson <chris@chris-wilson.co.uk> writes: > On Fri, Mar 10, 2017 at 09:57:47AM +0000, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@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.ursulin@intel.com> >> Cc: Weinan Li <weinan.z.li@intel.com> >> Cc: Chris Wilson <chris@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. First I thought that he hates switches :) But yup, not bad. -Mika > > Do the gvt-g hosts in CI now provide coverage for us of vgpu paths? > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-10 10:09 ` Chris Wilson 2017-03-10 13:05 ` Mika Kuoppala @ 2017-03-13 9:26 ` Tvrtko Ursulin 2017-03-13 9:37 ` Zhenyu Wang 1 sibling, 1 reply; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-13 9:26 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Weinan Li, Zhenyu Wang 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.ursulin@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.ursulin@intel.com> >> Cc: Weinan Li <weinan.z.li@intel.com> >> Cc: Chris Wilson <chris@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? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-13 9:26 ` Tvrtko Ursulin @ 2017-03-13 9:37 ` Zhenyu Wang 2017-03-13 9:47 ` Tvrtko Ursulin 2017-03-17 9:29 ` Tvrtko Ursulin 0 siblings, 2 replies; 12+ messages in thread From: Zhenyu Wang @ 2017-03-13 9:37 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx, Xu, Terrence [-- Attachment #1.1: Type: text/plain, Size: 4056 bytes --] 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.ursulin@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.ursulin@intel.com> > > > Cc: Weinan Li <weinan.z.li@intel.com> > > > Cc: Chris Wilson <chris@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. About CI for gvt, I think Terrence is still working but don't know how far it goes now. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 163 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-13 9:37 ` Zhenyu Wang @ 2017-03-13 9:47 ` Tvrtko Ursulin 2017-03-13 9:59 ` Chris Wilson 2017-03-17 9:29 ` Tvrtko Ursulin 1 sibling, 1 reply; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-13 9:47 UTC (permalink / raw) To: Zhenyu Wang; +Cc: Intel-gfx, Xu, Terrence 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.ursulin@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.ursulin@intel.com> >>>> Cc: Weinan Li <weinan.z.li@intel.com> >>>> Cc: Chris Wilson <chris@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. That would be good I think, thank you. When it has been cleared that it actually works and doesn't break anything we can then merge it. Regards, Tvrtko > About CI for gvt, I think Terrence is still working but don't know how far it goes now. > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-13 9:47 ` Tvrtko Ursulin @ 2017-03-13 9:59 ` Chris Wilson 0 siblings, 0 replies; 12+ messages in thread From: Chris Wilson @ 2017-03-13 9:59 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx, Xu, Terrence On Mon, Mar 13, 2017 at 09:47:15AM +0000, Tvrtko Ursulin wrote: > > 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.ursulin@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.ursulin@intel.com> > >>>>Cc: Weinan Li <weinan.z.li@intel.com> > >>>>Cc: Chris Wilson <chris@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. > > That would be good I think, thank you. When it has been cleared that > it actually works and doesn't break anything we can then merge it. For the record, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-13 9:37 ` Zhenyu Wang 2017-03-13 9:47 ` Tvrtko Ursulin @ 2017-03-17 9:29 ` Tvrtko Ursulin 2017-03-17 9:42 ` Xu, Terrence 1 sibling, 1 reply; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-17 9:29 UTC (permalink / raw) To: Zhenyu Wang; +Cc: Intel-gfx, Xu, Terrence 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.ursulin@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.ursulin@intel.com> >>>> Cc: Weinan Li <weinan.z.li@intel.com> >>>> Cc: Chris Wilson <chris@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? Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-17 9:29 ` Tvrtko Ursulin @ 2017-03-17 9:42 ` Xu, Terrence 2017-03-17 9:54 ` Tvrtko Ursulin 0 siblings, 1 reply; 12+ messages in thread From: Xu, Terrence @ 2017-03-17 9:42 UTC (permalink / raw) To: Tvrtko Ursulin, Zhenyu Wang; +Cc: Intel-gfx >-----Original Message----- >From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] >Sent: Friday, March 17, 2017 5:30 PM >To: Zhenyu Wang <zhenyuw@linux.intel.com> >Cc: Chris Wilson <chris@chris-wilson.co.uk>; Tvrtko Ursulin ><tursulin@ursulin.net>; Intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko ><tvrtko.ursulin@intel.com>; Li, Weinan Z <weinan.z.li@intel.com>; Xu, >Terrence <terrence.xu@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.ursulin@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.ursulin@intel.com> >>>>> Cc: Weinan Li <weinan.z.li@intel.com> >>>>> Cc: Chris Wilson <chris@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.xu@intel.com> >Regards, > >Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-17 9:42 ` Xu, Terrence @ 2017-03-17 9:54 ` Tvrtko Ursulin 0 siblings, 0 replies; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-17 9:54 UTC (permalink / raw) To: Xu, Terrence, Zhenyu Wang; +Cc: Intel-gfx On 17/03/2017 09:42, Xu, Terrence wrote: >> -----Original Message----- >> From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] >> Sent: Friday, March 17, 2017 5:30 PM >> To: Zhenyu Wang <zhenyuw@linux.intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>; Tvrtko Ursulin >> <tursulin@ursulin.net>; Intel-gfx@lists.freedesktop.org; Ursulin, Tvrtko >> <tvrtko.ursulin@intel.com>; Li, Weinan Z <weinan.z.li@intel.com>; Xu, >> Terrence <terrence.xu@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.ursulin@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.ursulin@intel.com> >>>>>> Cc: Weinan Li <weinan.z.li@intel.com> >>>>>> Cc: Chris Wilson <chris@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.xu@intel.com> Thanks! Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-10 9:57 [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly Tvrtko Ursulin 2017-03-10 10:09 ` Chris Wilson @ 2017-03-10 12:48 ` Patchwork 2017-03-17 9:55 ` Tvrtko Ursulin 1 sibling, 1 reply; 12+ messages in thread From: Patchwork @ 2017-03-10 12:48 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly URL : https://patchwork.freedesktop.org/series/21047/ State : success == Summary == Series 21047v1 drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly https://patchwork.freedesktop.org/api/1.0/series/21047/revisions/1/mbox/ fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 467s fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 605s fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 536s fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 609s fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 500s fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 441s fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 434s fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 441s fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 507s fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 493s fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 476s fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 503s fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 608s fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 507s fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 559s fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 561s fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 427s d8d69f76555feef19e3e4b601378446604d90da5 drm-tip: 2017y-03m-10d-11h-50m-04s UTC integration manifest 212fdd9 drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4133/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: ✓ Fi.CI.BAT: success for drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly 2017-03-10 12:48 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2017-03-17 9:55 ` Tvrtko Ursulin 0 siblings, 0 replies; 12+ messages in thread From: Tvrtko Ursulin @ 2017-03-17 9:55 UTC (permalink / raw) To: intel-gfx, Tvrtko Ursulin On 10/03/2017 12:48, Patchwork wrote: > == Series Details == > > Series: drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly > URL : https://patchwork.freedesktop.org/series/21047/ > State : success > > == Summary == > > Series 21047v1 drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly > https://patchwork.freedesktop.org/api/1.0/series/21047/revisions/1/mbox/ > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 467s > fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 605s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 536s > fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 609s > fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 506s > fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 500s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 441s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 434s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 441s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 507s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 493s > fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 476s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 503s > fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 608s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 507s > fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 559s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 561s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 427s > > d8d69f76555feef19e3e4b601378446604d90da5 drm-tip: 2017y-03m-10d-11h-50m-04s UTC integration manifest > 212fdd9 drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly Pushed, thanks for review and testing! Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-03-17 9:55 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-10 9:57 [PATCH] drm/i915/vgpu: Neuter forcewakes for VGPU more thouroughly Tvrtko Ursulin 2017-03-10 10:09 ` Chris Wilson 2017-03-10 13:05 ` Mika Kuoppala 2017-03-13 9:26 ` Tvrtko Ursulin 2017-03-13 9:37 ` Zhenyu Wang 2017-03-13 9:47 ` Tvrtko Ursulin 2017-03-13 9:59 ` Chris Wilson 2017-03-17 9:29 ` Tvrtko Ursulin 2017-03-17 9:42 ` Xu, Terrence 2017-03-17 9:54 ` Tvrtko Ursulin 2017-03-10 12:48 ` ✓ Fi.CI.BAT: success for " Patchwork 2017-03-17 9:55 ` Tvrtko Ursulin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.