On 6/18/19 3:22 AM, Chris Wilson wrote:
Quoting Daniele Ceraolo Spurio (2019-06-17 19:09:33)
We always call some of the setup/cleanup functions for forcewake, even
if the feature is not actually available. Skipping these operations if
forcewake is not available saves us some operations on older gens and
prepares us for having a forcewake-less display uncore.
The suspend/resume functions have also been renamed to clearly indicate
that they only operate on forcewake status.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.c | 15 +--
drivers/gpu/drm/i915/intel_uncore.c | 147 +++++++++++++++++-----------
drivers/gpu/drm/i915/intel_uncore.h | 8 +-
3 files changed, 101 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d113296cbe34..95b36fe17f99 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -996,7 +996,7 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
intel_device_info_init_mmio(dev_priv);
- intel_uncore_prune_mmio_domains(&dev_priv->uncore);
+ intel_uncore_prune_forcewake_domains(&dev_priv->uncore);
The _prune_ was the exceptional case...
intel_uc_init_mmio(dev_priv);
@@ -2152,7 +2152,7 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
i915_gem_suspend_late(dev_priv);
- intel_uncore_suspend(&dev_priv->uncore);
+ intel_uncore_forcewake_suspend(&dev_priv->uncore);
But are you sure you want to delegate all the fw control to i915_drv.c,
and not keep this as a call into intel_uncore_suspend() ? It is meant to
be telling the uncore functionality that it is time to suspend, and it
has to work out what to save.
I am not sold on this yet. (One day this will just be a bunch of async
tasks plugged into signals with ordering determined by their
self-declared dependency tree. One day.)
So sell me on why we want an uncore_forcewake object, as currently you
are proposing a intel_uncore_suspend_forcewake().
-Chris
My aim was to make it clear that this call will not be required for
display_uncore since there is nothing to do on suspend/resume if there
is no forcewake (and you're right, intel_uncore_suspend_forcewake is a
better naming). Ultimately I'd like to transition the GT uncore inside
intel_gt and call this inside intel_gt_suspend(). However, I don't mind
keeping the current naming and calling it for display uncore as well to
be more generic, there are checks everywhere so the overhead should me
minimal. What's your preference?
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
