[Intel-gfx] [PATCH 6/6] drm/i915: Migrate stolen objects before hibernation
Ville Syrjälä
ville.syrjala at linux.intel.com
Wed Dec 2 01:52:29 PST 2015
On Wed, Nov 11, 2015 at 04:06:13PM +0530, ankitprasad.r.sharma at intel.com wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
>
> Ville reminded us that stolen memory is not preserved across
> hibernation, and a result of this was that context objects now being
> allocated from stolen were being corrupted on S4 and promptly hanging
> the GPU on resume.
BTW we had a bug that says that the firmware can make a mess of
stolen even during S3 when rabidstart is enabled :(
https://bugs.freedesktop.org/show_bug.cgi?id=91295
>
> We want to utilise stolen for as much as possible (nothing else will use
> that wasted memory otherwise), so we need a strategy for handling
> general objects allocated from stolen and hibernation. A simple solution
> is to do a CPU copy through the GTT of the stolen object into a fresh
> shmemfs backing store and thenceforth treat it as a normal objects. This
> can be refined in future to either use a GPU copy to avoid the slow
> uncached reads (though it's hibernation!) and recreate stolen objects
> upon resume/first-use. For now, a simple approach should suffice for
> testing the object migration.
>
> v2:
> Swap PTE for pinned bindings over to the shmemfs. This adds a
> complicated dance, but is required as many stolen objects are likely to
> be pinned for use by the hardware. Swapping the PTEs should not result
> in externally visible behaviour, as each PTE update should be atomic and
> the two pages identical. (danvet)
>
> safe-by-default, or the principle of least surprise. We need a new flag
> to mark objects that we can wilfully discard and recreate across
> hibernation. (danvet)
>
> Just use the global_list rather than invent a new stolen_list. This is
> the slowpath hibernate and so adding a new list and the associated
> complexity isn't worth it.
>
> v3: Rebased on drm-intel-nightly (Ankit)
>
> v4: Use insert_page to map stolen memory backed pages for migration to
> shmem (Chris)
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 17 ++-
> drivers/gpu/drm/i915/i915_drv.h | 7 +
> drivers/gpu/drm/i915/i915_gem.c | 235 ++++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_display.c | 3 +
> drivers/gpu/drm/i915/intel_fbdev.c | 6 +
> drivers/gpu/drm/i915/intel_pm.c | 2 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +
> 7 files changed, 264 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9f55209..2bb9e9e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1036,6 +1036,21 @@ static int i915_pm_suspend(struct device *dev)
> return i915_drm_suspend(drm_dev);
> }
>
> +static int i915_pm_freeze(struct device *dev)
> +{
> + int ret;
> +
> + ret = i915_gem_freeze(pci_get_drvdata(to_pci_dev(dev)));
> + if (ret)
> + return ret;
> +
> + ret = i915_pm_suspend(dev);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int i915_pm_suspend_late(struct device *dev)
> {
> struct drm_device *drm_dev = dev_to_i915(dev)->dev;
> @@ -1700,7 +1715,7 @@ static const struct dev_pm_ops i915_pm_ops = {
> * @restore, @restore_early : called after rebooting and restoring the
> * hibernation image [PMSG_RESTORE]
> */
> - .freeze = i915_pm_suspend,
> + .freeze = i915_pm_freeze,
> .freeze_late = i915_pm_suspend_late,
> .thaw_early = i915_pm_resume_early,
> .thaw = i915_pm_resume,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2c75c32..a4e1bf7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2079,6 +2079,12 @@ struct drm_i915_gem_object {
> * Advice: are the backing pages purgeable?
> */
> unsigned int madv:2;
> + /**
> + * Whereas madv is for userspace, there are certain situations
> + * where we want I915_MADV_DONTNEED behaviour on internal objects
> + * without conflating the userspace setting.
> + */
> + unsigned int internal_volatile:1;
>
> /**
> * Current tiling mode for the object.
> @@ -3006,6 +3012,7 @@ int i915_gem_l3_remap(struct drm_i915_gem_request *req, int slice);
> void i915_gem_init_swizzling(struct drm_device *dev);
> void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
> int __must_check i915_gpu_idle(struct drm_device *dev);
> +int __must_check i915_gem_freeze(struct drm_device *dev);
> int __must_check i915_gem_suspend(struct drm_device *dev);
> void __i915_add_request(struct drm_i915_gem_request *req,
> struct drm_i915_gem_object *batch_obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e0b9502..dbe1173 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4506,12 +4506,27 @@ static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
> .put_pages = i915_gem_object_put_pages_gtt,
> };
>
> +static struct address_space *
> +i915_gem_set_inode_gfp(struct drm_device *dev, struct file *file)
> +{
> + struct address_space *mapping = file_inode(file)->i_mapping;
> + gfp_t mask;
> +
> + mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> + if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> + /* 965gm cannot relocate objects above 4GiB. */
> + mask &= ~__GFP_HIGHMEM;
> + mask |= __GFP_DMA32;
> + }
> + mapping_set_gfp_mask(mapping, mask);
> +
> + return mapping;
> +}
> +
> struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> size_t size)
> {
> struct drm_i915_gem_object *obj;
> - struct address_space *mapping;
> - gfp_t mask;
> int ret = 0;
>
> obj = i915_gem_object_alloc(dev);
> @@ -4523,15 +4538,7 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> return ERR_PTR(ret);
> }
>
> - mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> - if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> - /* 965gm cannot relocate objects above 4GiB. */
> - mask &= ~__GFP_HIGHMEM;
> - mask |= __GFP_DMA32;
> - }
> -
> - mapping = file_inode(obj->base.filp)->i_mapping;
> - mapping_set_gfp_mask(mapping, mask);
> + i915_gem_set_inode_gfp(dev, obj->base.filp);
>
> i915_gem_object_init(obj, &i915_gem_object_ops);
>
> @@ -4708,6 +4715,212 @@ i915_gem_stop_ringbuffers(struct drm_device *dev)
> dev_priv->gt.stop_ring(ring);
> }
>
> +static int
> +i915_gem_object_migrate_stolen_to_shmemfs(struct drm_i915_gem_object *obj)
> +{
> + struct drm_i915_private *i915 = to_i915(obj->base.dev);
> + struct i915_vma *vma, *vn;
> + struct drm_mm_node node;
> + struct file *file;
> + struct address_space *mapping;
> + struct sg_table *stolen_pages, *shmemfs_pages;
> + int ret, i;
> +
> + if (WARN_ON(i915_gem_object_needs_bit17_swizzle(obj)))
> + return -EINVAL;
> +
> + ret = i915_gem_object_set_to_gtt_domain(obj, false);
> + if (ret)
> + return ret;
> +
> + file = shmem_file_setup("drm mm object", obj->base.size, VM_NORESERVE);
> + if (IS_ERR(file))
> + return PTR_ERR(file);
> + mapping = i915_gem_set_inode_gfp(obj->base.dev, file);
> +
> + list_for_each_entry_safe(vma, vn, &obj->vma_list, vma_link)
> + if (i915_vma_unbind(vma))
> + continue;
> +
> + if (obj->madv != I915_MADV_WILLNEED && list_empty(&obj->vma_list)) {
> + /* Discard the stolen reservation, and replace with
> + * an unpopulated shmemfs object.
> + */
> + obj->madv = __I915_MADV_PURGED;
> + goto swap_pages;
> + }
> +
> + /* stolen objects are already pinned to prevent shrinkage */
> + memset(&node, 0, sizeof(node));
> + ret = drm_mm_insert_node_in_range_generic(&i915->gtt.base.mm,
> + &node,
> + 4096, 0, I915_CACHE_NONE,
> + 0, i915->gtt.mappable_end,
> + DRM_MM_SEARCH_DEFAULT,
> + DRM_MM_CREATE_DEFAULT);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < obj->base.size / PAGE_SIZE; i++) {
> + struct page *page;
> + void *__iomem src;
> + void *dst;
> +
> + wmb();
> + i915->gtt.base.insert_page(&i915->gtt.base,
> + i915_gem_object_get_dma_address(obj, i),
> + node.start,
> + I915_CACHE_NONE,
> + 0);
> + wmb();
> +
> + page = shmem_read_mapping_page(mapping, i);
> + if (IS_ERR(page)) {
> + ret = PTR_ERR(page);
> + goto err_node;
> + }
> +
> + src = io_mapping_map_atomic_wc(i915->gtt.mappable, node.start + PAGE_SIZE * i);
> + dst = kmap_atomic(page);
> + memcpy_fromio(dst, src, PAGE_SIZE);
> + kunmap_atomic(dst);
> + io_mapping_unmap_atomic(src);
> +
> + page_cache_release(page);
> + }
> +
> + wmb();
> + i915->gtt.base.clear_range(&i915->gtt.base,
> + node.start, node.size,
> + true);
> + drm_mm_remove_node(&node);
> +
> +swap_pages:
> + stolen_pages = obj->pages;
> + obj->pages = NULL;
> +
> + obj->base.filp = file;
> + obj->base.read_domains = I915_GEM_DOMAIN_CPU;
> + obj->base.write_domain = I915_GEM_DOMAIN_CPU;
> +
> + /* Recreate any pinned binding with pointers to the new storage */
> + if (!list_empty(&obj->vma_list)) {
> + ret = i915_gem_object_get_pages_gtt(obj);
> + if (ret) {
> + obj->pages = stolen_pages;
> + goto err_file;
> + }
> +
> + ret = i915_gem_gtt_prepare_object(obj);
> + if (ret) {
> + i915_gem_object_put_pages_gtt(obj);
> + obj->pages = stolen_pages;
> + goto err_file;
> + }
> +
> + ret = i915_gem_object_set_to_gtt_domain(obj, true);
> + if (ret) {
> + i915_gem_gtt_finish_object(obj);
> + i915_gem_object_put_pages_gtt(obj);
> + obj->pages = stolen_pages;
> + goto err_file;
> + }
> +
> + obj->get_page.sg = obj->pages->sgl;
> + obj->get_page.last = 0;
> +
> + list_for_each_entry(vma, &obj->vma_list, vma_link) {
> + if (!drm_mm_node_allocated(&vma->node))
> + continue;
> +
> + WARN_ON(i915_vma_bind(vma,
> + obj->cache_level,
> + PIN_UPDATE));
> + }
> + } else
> + list_del(&obj->global_list);
> +
> + /* drop the stolen pin and backing */
> + shmemfs_pages = obj->pages;
> + obj->pages = stolen_pages;
> +
> + i915_gem_object_unpin_pages(obj);
> + obj->ops->put_pages(obj);
> + if (obj->ops->release)
> + obj->ops->release(obj);
> +
> + obj->ops = &i915_gem_object_ops;
> + obj->pages = shmemfs_pages;
> +
> + return 0;
> +
> +err_node:
> + wmb();
> + i915->gtt.base.clear_range(&i915->gtt.base,
> + node.start, node.size,
> + true);
> + drm_mm_remove_node(&node);
> +err_file:
> + fput(file);
> + obj->base.filp = NULL;
> + return ret;
> +}
> +
> +int
> +i915_gem_freeze(struct drm_device *dev)
> +{
> + /* Called before i915_gem_suspend() when hibernating */
> + struct drm_i915_private *i915 = to_i915(dev);
> + struct drm_i915_gem_object *obj, *tmp;
> + struct list_head *phase[] = {
> + &i915->mm.unbound_list, &i915->mm.bound_list, NULL
> + }, **p;
> +
> + /* Across hibernation, the stolen area is not preserved.
> + * Anything inside stolen must copied back to normal
> + * memory if we wish to preserve it.
> + */
> + for (p = phase; *p; p++) {
> + struct list_head migrate;
> + int ret;
> +
> + INIT_LIST_HEAD(&migrate);
> + list_for_each_entry_safe(obj, tmp, *p, global_list) {
> + if (obj->stolen == NULL)
> + continue;
> +
> + if (obj->internal_volatile)
> + 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->global_list, &migrate);
> + }
> +
> + ret = 0;
> + list_for_each_entry_safe(obj, tmp, &migrate, global_list) {
> + if (ret == 0)
> + ret = i915_gem_object_migrate_stolen_to_shmemfs(obj);
> + drm_gem_object_unreference(&obj->base);
> + }
> + list_splice(&migrate, *p);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> int
> i915_gem_suspend(struct drm_device *dev)
> {
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f281e0b..0803922 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2549,6 +2549,9 @@ intel_alloc_initial_plane_obj(struct intel_crtc *crtc,
> if (IS_ERR(obj))
> return false;
>
> + /* Not to be preserved across hibernation */
> + obj->internal_volatile = true;
> +
> obj->tiling_mode = plane_config->tiling;
> if (obj->tiling_mode == I915_TILING_X)
> obj->stride = fb->pitches[0];
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index f43681e..1d89253 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -154,6 +154,12 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
> goto out;
> }
>
> + /* Discard the contents of the BIOS fb across hibernation.
> + * We really want to completely throwaway the earlier fbdev
> + * and reconfigure it anyway.
> + */
> + obj->internal_volatile = true;
> +
> fb = __intel_framebuffer_create(dev, &mode_cmd, obj);
> if (IS_ERR(fb)) {
> ret = PTR_ERR(fb);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 03ad276..6ddc20a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5181,6 +5181,8 @@ static void valleyview_setup_pctx(struct drm_device *dev)
> I915_WRITE(VLV_PCBR, pctx_paddr);
>
> out:
> + /* The power context need not be preserved across hibernation */
> + pctx->internal_volatile = true;
> DRM_DEBUG_DRIVER("PCBR: 0x%08x\n", I915_READ(VLV_PCBR));
> dev_priv->vlv_pctx = pctx;
> }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 5eabaf6..370d96a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2090,6 +2090,12 @@ static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
> if (IS_ERR(obj))
> return PTR_ERR(obj);
>
> + /* Ringbuffer objects are by definition volatile - only the commands
> + * between HEAD and TAIL need to be preserved and whilst there are
> + * any commands there, the ringbuffer is pinned by activity.
> + */
> + obj->internal_volatile = true;
> +
> /* mark ring buffers as read-only from GPU side by default */
> obj->gt_ro = 1;
>
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list