| Message ID | 20170627173731.11566-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 27, 2017 at 06:37:31PM +0100, Chris Wilson wrote: > i915_gem_suspend() is called from all of our finalization paths > (suspend, hibernate, unload). i915_gem_drain_freed_objects() adds an > arbitrary delay as it uses an rcu_barrier() to ensure that there are no > more freed objects in flight, and this delay causes a large amount of > variability in suspend timings. For S3 suspend, we do not need to free > pages as doing so does not impact at all upon the system in its > suspended state, unlike S4 hibernation where we do want the hibernation > image to be as small as possible. Therefore we can forgo waiting inside > i915_gem_suspend(), so long as we ensure that we do cleanup before > unload (see i915_gem_load_cleanup()) and prefer to reap our objects > prior to hibernation (see i915_gem_freeze()). > > Removing the rcu_barrier() from i915_gem_suspend() improves S3 latency > by about 30ms on Skylake (ymmv). > > Reported-by: David Weinehall <david.weinehall@linux.intel.com> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: David Weinehall <david.weinehall@linux.intel.com> Tested-by: David Weinehall <david.weinehall@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 36d838677982..f38c84e485ab 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4597,8 +4597,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) > while (flush_delayed_work(&dev_priv->gt.idle_work)) > ; > > - i915_gem_drain_freed_objects(dev_priv); > - > /* Assert that we sucessfully flushed all the work and > * reset the GPU back to its idle, low power state. > */ > -- > 2.13.1 >
Quoting David Weinehall (2017-06-27 18:57:34) > On Tue, Jun 27, 2017 at 06:37:31PM +0100, Chris Wilson wrote: > > i915_gem_suspend() is called from all of our finalization paths > > (suspend, hibernate, unload). i915_gem_drain_freed_objects() adds an > > arbitrary delay as it uses an rcu_barrier() to ensure that there are no > > more freed objects in flight, and this delay causes a large amount of > > variability in suspend timings. For S3 suspend, we do not need to free > > pages as doing so does not impact at all upon the system in its > > suspended state, unlike S4 hibernation where we do want the hibernation > > image to be as small as possible. Therefore we can forgo waiting inside > > i915_gem_suspend(), so long as we ensure that we do cleanup before > > unload (see i915_gem_load_cleanup()) and prefer to reap our objects > > prior to hibernation (see i915_gem_freeze()). > > > > Removing the rcu_barrier() from i915_gem_suspend() improves S3 latency > > by about 30ms on Skylake (ymmv). > > > > Reported-by: David Weinehall <david.weinehall@linux.intel.com> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > Tested-by: David Weinehall <david.weinehall@linux.intel.com> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Thanks for the heads up and testing the patch, pushed. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 36d838677982..f38c84e485ab 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -4597,8 +4597,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv) while (flush_delayed_work(&dev_priv->gt.idle_work)) ; - i915_gem_drain_freed_objects(dev_priv); - /* Assert that we sucessfully flushed all the work and * reset the GPU back to its idle, low power state. */
i915_gem_suspend() is called from all of our finalization paths (suspend, hibernate, unload). i915_gem_drain_freed_objects() adds an arbitrary delay as it uses an rcu_barrier() to ensure that there are no more freed objects in flight, and this delay causes a large amount of variability in suspend timings. For S3 suspend, we do not need to free pages as doing so does not impact at all upon the system in its suspended state, unlike S4 hibernation where we do want the hibernation image to be as small as possible. Therefore we can forgo waiting inside i915_gem_suspend(), so long as we ensure that we do cleanup before unload (see i915_gem_load_cleanup()) and prefer to reap our objects prior to hibernation (see i915_gem_freeze()). Removing the rcu_barrier() from i915_gem_suspend() improves S3 latency by about 30ms on Skylake (ymmv). Reported-by: David Weinehall <david.weinehall@linux.intel.com> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 2 -- 1 file changed, 2 deletions(-)