[PATCH] drm/i915: i915_gem_shrink_all() needs an awake device

[PATCH] drm/i915: i915_gem_shrink_all() needs an awake device

[Chris Wilson]

Since to unbind an object, we may need a powered up device to access the
GTT entries, we only shrink bound objects if awake. Callers to
i915_gem_shrink_all() had to take this into account and take the rpm
wakeref, but we can move this wakeref into the shrink_all itself for
convenience and making the function live up to its name.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c          | 4 ----
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 5 +++--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b7632bbbafd8..d8d107fe6ca7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4646,14 +4646,10 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
 
 int i915_gem_freeze(struct drm_i915_private *dev_priv)
 {
-	intel_runtime_pm_get(dev_priv);
-
 	mutex_lock(&dev_priv->drm.struct_mutex);
 	i915_gem_shrink_all(dev_priv);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
-	intel_runtime_pm_put(dev_priv);
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 401006b4c6a3..f249a1eb46ac 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -259,10 +259,13 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
 {
 	unsigned long freed;
 
+	intel_runtime_pm_get(dev_priv);
 	freed = i915_gem_shrink(dev_priv, -1UL,
 				I915_SHRINK_BOUND |
 				I915_SHRINK_UNBOUND |
 				I915_SHRINK_ACTIVE);
+	intel_runtime_pm_put(dev_priv);
+
 	rcu_barrier(); /* wait until our RCU delayed slab frees are completed */
 
 	return freed;
@@ -380,9 +383,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
 		return NOTIFY_DONE;
 
-	intel_runtime_pm_get(dev_priv);
 	freed_pages = i915_gem_shrink_all(dev_priv);
-	intel_runtime_pm_put(dev_priv);
 
 	/* Because we may be allocating inside our own driver, we cannot
 	 * assert that there are no objects with pinned pages that are not
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915: i915_gem_shrink_all() needs an awake device

[Patchwork]

== Series Details ==

Series: drm/i915: i915_gem_shrink_all() needs an awake device
URL   : https://patchwork.freedesktop.org/series/19297/
State : failure

== Summary ==

Series 19297v1 drm/i915: i915_gem_shrink_all() needs an awake device
https://patchwork.freedesktop.org/api/1.0/series/19297/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup force-connector-state:
                dmesg-warn -> PASS       (fi-snb-2520m)
        Subgroup force-edid:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                fail       -> PASS       (fi-skl-6770hq)
        Subgroup read-crc-pipe-a-frame-sequence:
                fail       -> PASS       (fi-skl-6770hq)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> INCOMPLETE (fi-byt-n2820)

fi-bdw-5557u     total:252  pass:238  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:252  pass:213  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:252  pass:230  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:252  pass:225  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:215  pass:184  dwarn:0   dfail:0   fail:0   skip:30 
fi-hsw-4770      total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:252  pass:233  dwarn:0   dfail:0   fail:0   skip:19 
fi-ilk-650       total:252  pass:199  dwarn:0   dfail:0   fail:0   skip:53 
fi-ivb-3520m     total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:252  pass:231  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:252  pass:229  dwarn:0   dfail:0   fail:2   skip:21 
fi-skl-6260u     total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:252  pass:232  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:252  pass:227  dwarn:4   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:252  pass:239  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:252  pass:220  dwarn:1   dfail:0   fail:0   skip:31 
fi-snb-2600      total:252  pass:220  dwarn:0   dfail:0   fail:0   skip:32 

80b02983d80ad38722a8bb6c86ca001ed57b9755 drm-tip: 2017y-02m-08d-09h-27m-34s UTC integration manifest
e7d55c9 drm/i915: i915_gem_shrink_all() needs an awake device

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3734/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: i915_gem_shrink_all() needs an awake device

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Since to unbind an object, we may need a powered up device to access the
> GTT entries, we only shrink bound objects if awake. Callers to
> i915_gem_shrink_all() had to take this into account and take the rpm
> wakeref, but we can move this wakeref into the shrink_all itself for
> convenience and making the function live up to its name.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem.c          | 4 ----
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 5 +++--
>  2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index b7632bbbafd8..d8d107fe6ca7 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4646,14 +4646,10 @@ void i915_gem_load_cleanup(struct drm_i915_private *dev_priv)
>  
>  int i915_gem_freeze(struct drm_i915_private *dev_priv)
>  {
> -	intel_runtime_pm_get(dev_priv);
> -
>  	mutex_lock(&dev_priv->drm.struct_mutex);
>  	i915_gem_shrink_all(dev_priv);
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  
> -	intel_runtime_pm_put(dev_priv);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 401006b4c6a3..f249a1eb46ac 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -259,10 +259,13 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
>  {
>  	unsigned long freed;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	freed = i915_gem_shrink(dev_priv, -1UL,
>  				I915_SHRINK_BOUND |
>  				I915_SHRINK_UNBOUND |
>  				I915_SHRINK_ACTIVE);
> +	intel_runtime_pm_put(dev_priv);
> +
>  	rcu_barrier(); /* wait until our RCU delayed slab frees are completed */
>  
>  	return freed;
> @@ -380,9 +383,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
>  		return NOTIFY_DONE;
>  
> -	intel_runtime_pm_get(dev_priv);
>  	freed_pages = i915_gem_shrink_all(dev_priv);
> -	intel_runtime_pm_put(dev_priv);
>  
>  	/* Because we may be allocating inside our own driver, we cannot
>  	 * assert that there are no objects with pinned pages that are not
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: i915_gem_shrink_all() needs an awake device

[Chris Wilson]

On Wed, Feb 08, 2017 at 03:26:26PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Since to unbind an object, we may need a powered up device to access the
> > GTT entries, we only shrink bound objects if awake. Callers to
> > i915_gem_shrink_all() had to take this into account and take the rpm
> > wakeref, but we can move this wakeref into the shrink_all itself for
> > convenience and making the function live up to its name.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Ta, easiest review of the day :)
-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