On 12/24/2015 8:02 PM, Chris Wilson wrote:
On Thu, Dec 24, 2015 at 07:54:09PM +0530, Goel, Akash wrote:
On 12/24/2015 5:52 PM, Chris Wilson wrote:
On Thu, Dec 24, 2015 at 04:16:08PM +0530, Praveen Paneri wrote:
When the system is running low on memory, gem shrinker is invoked.
In this process objects will be unbounded from GTT and unbinding process
will require access to GTT(GTTADR) and also to fence register potentially.
That requires a resume of gfx device, if suspended, in the shrinker path.
Considering the power leakage due to intermediate resume, perform unbinding
operation only if device is already runtime active.
Signed-off-by: Akash Goel <akash.goel@xxxxxxxxx>
Signed-off-by: Praveen Paneri <praveen.paneri@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Lgtm, the only complication is that we over report the number of
shrinkable objects. But that isn't such a big issue with the current
incarnation of the shrinker.
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index f7df54a..89350f4 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -89,6 +89,15 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
i915_gem_retire_requests(dev_priv->dev);
/*
+ * Unbinding of objects will require HW access. Lets not wake
+ * up gfx device just for this. Do the unbinding only if gfx
+ * device is already active.
+ */
+ if ((flags & I915_SHRINK_BOUND) &&
+ !intel_runtime_pm_get_noidle(dev_priv))
Please line up contnuation lines with the opening bracking, hint cino=:0,(0 for vim.
With the whitespace fixed,
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
/* Unbinding of objects will require HW access; let us not wake up
* the device just to recover a little memory. If absolutely necessary,
* we will force the wake during oom-notifier.
*/
Sorry not fully sure but do we need to cover
i915_gem_retire_requests() also ?
No. That is covered by the dev_priv->mm.busy wakeref.
Actually retire_requests could also lead to a potential unbinding,
if the last reference of a context goes away in that.
Indeed, also last object unreference could trigger an unbinding, and
even last vma use. All covered by the dev_priv->mm.busy wakeref held
whilst there are any requests in flight.
Thank you so much for the clarification.
So if the device is in a runtime suspended state, the call to
i915_gem_retire_requests() should almost be a NOOP.
Best regards
Akash
There is a runtime_pm_get protection in i915_gem_free_object, so
should not be a problem for ringbuffer & context image objects and
most probably the i915_gem_context_clean would get completed before
the device again goes into runtime suspend state.
No the one in i915_gem_free_object is actually wrong (granularity), and
hopefully will be fixed in the near future.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
