Re: [PATCH] drm/i915: Migrate stolen objects before hibernation

Chris Wilson
2015-06-30T10:31:48Z

On Tue, Jun 30, 2015 at 10:58:08AM +0100, Chris Wilson wrote:
> +int
> +i915_gem_freeze(struct drm_device *dev)
> +{
> + /* Called before freeze when hibernating */
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_object *obj, *tmp;
> + int ret;
> +
> + /* Across hibernation, the stolen area is not preserved.
> + * Anything inside stolen must copied back to normal
> + * memory if we wish to preserve it.
> + */

And I forgot to add the double pass + reference/unreference dance here

> + list_for_each_entry_safe(obj, tmp, &i915->mm.stolen_list, stolen_link) {
> + if (obj->madv != I915_MADV_WILLNEED) {
> + /* XXX any point in setting this? The objects for which
> + * we preserve never unset it afterwards.
> + */
> + obj->madv = __I915_MADV_PURGED;
> + continue;
> + }
> +
> + ret = i915_gem_object_migrate_stolen_to_shmemfs(obj);
> + if (ret)
> + return ret;
> + }

should be

+ INIT_LIST_HEAD(&migrate);
+ list_for_each_entry_safe(obj, tmp, &i915->mm.stolen_list, stolen_link) {
+ if (obj->madv != I915_MADV_WILLNEED)
+ continue;
+
+ /* In the general case, this object may only be alive due to
+ * an active reference, and that may disappear when we unbind
+ * any of the objects (and so wait upon the GPU and retire
+ * requests). To prevent one of the objects from disappearing
+ * beneath us, we need to take a reference to each as we
+ * build the migration list.
+ *
+ * This is similar to the strategy required whilst shrinking
+ * or evicting objects (for the same reason).
+ */
+ drm_gem_object_reference(&obj->base);
+ list_move(&obj->stolen_link, &migrate);
+ }
+
+ ret = 0;
+ list_for_each_entry_safe(obj, tmp, &migrate, stolen_link) {
+ if (ret == 0)
+ ret = i915_gem_object_migrate_stolen_to_shmemfs(obj);
+ drm_gem_object_unreference(&obj->base);
+ }
+ list_splice(&migrate, &i915->mm.stolen_list);