[Intel-gfx] [PATCH] drm/i915: Unbind objects in shrinker only if device is runtime active

[Chris Wilson]

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 at intel.com>
> Signed-off-by: Praveen Paneri <praveen.paneri at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>

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 at chris-wilson.co.uk>

/* 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.
 */

Gives a better rationale, I think.

And can you, whilst you are here, please put the
intel_runtime_pm_get() into i915_gem_shrinker_oom()
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre