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.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.
> 
> 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 <ch...@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

Reply via email to