On Thu, 2015-09-24 at 11:57 +0100, Chris Wilson wrote:
> When preallocating a stolen object during early initialisation, we
> may
> be running before we have setup the the global GTT VM state, in
> particular before we have initialised the range manager and
> associated
> lists. As this is the case, we defer binding the stolen object until
> we
> call i915_gem_setup_global_gtt(). Not only should we defer the
> binding,
> but we should also defer the VM list manipulation.
>
> Fixes regression uncovered by commit
> a2cad9dff4dd44d0244b966d980de9d602d87593
> Author: Michał Winiarski <michal.winiar...@intel.com>
> Date: Wed Sep 16 11:49:00 2015 +0200
>
> drm/i915/gtt: Do not initialize drm_mm twice.
>
> Whilst I am here remove the duplicate work leaving dangling pointers
> from the error path...
>
> v2: Typos galore before coffee.
>
> Reported-by: Jesse Barnes <jbar...@virtuousgeek.org>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92099
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Jesse Barnes <jbar...@virtuousgeek.org>
> Cc: Michel Thierry <michel.thie...@intel.com>
> Cc: Mika Kuoppala <mika.kuopp...@intel.com>
> Cc: Michał Winiarski <michal.winiar...@intel.com>
> Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
> Cc: Jani Nikula <jani.nik...@linux.intel.com>
Reviewed-by: Michał Winiarski <michal.winiar...@intel.com>
-Michał
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_stolen.c | 16 ++++++----------
> 2 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 01f3521e77d3..291305fb2f3c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2533,7 +2533,6 @@ static int ggtt_bind_vma(struct i915_vma *vma,
> * the bound flag ourselves.
> */
> vma->bound |= GLOBAL_BIND;
> -
> }
>
> if (dev_priv->mm.aliasing_ppgtt && flags & LOCAL_BIND) {
> @@ -2657,6 +2656,7 @@ static int i915_gem_setup_global_gtt(struct
> drm_device *dev,
> return ret;
> }
> vma->bound |= GLOBAL_BIND;
> + list_add_tail(&vma->mm_list, &ggtt_vm
> ->inactive_list);
> }
>
> /* Clear any non-preallocated blocks */
> diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c
> b/drivers/gpu/drm/i915/i915_gem_stolen.c
> index 55df6ce34751..15207796e1b3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
> @@ -584,7 +584,7 @@
> i915_gem_object_create_stolen_for_preallocated(struct drm_device
> *dev,
> vma = i915_gem_obj_lookup_or_create_vma(obj, ggtt);
> if (IS_ERR(vma)) {
> ret = PTR_ERR(vma);
> - goto err_out;
> + goto err;
> }
>
> /* To simplify the initialisation sequence between KMS and
> GTT,
> @@ -598,23 +598,19 @@
> i915_gem_object_create_stolen_for_preallocated(struct drm_device
> *dev,
> ret = drm_mm_reserve_node(&ggtt->mm, &vma->node);
> if (ret) {
> DRM_DEBUG_KMS("failed to allocate stolen GTT
> space\n");
> - goto err_vma;
> + goto err;
> }
> - }
>
> - vma->bound |= GLOBAL_BIND;
> + vma->bound |= GLOBAL_BIND;
> + list_add_tail(&vma->mm_list, &ggtt->inactive_list);
> + }
>
> list_add_tail(&obj->global_list, &dev_priv->mm.bound_list);
> - list_add_tail(&vma->mm_list, &ggtt->inactive_list);
> i915_gem_object_pin_pages(obj);
>
> return obj;
>
> -err_vma:
> - i915_gem_vma_destroy(vma);
> -err_out:
> - i915_gem_stolen_remove_node(dev_priv, stolen);
> - kfree(stolen);
> +err:
> drm_gem_object_unreference(&obj->base);
> return NULL;
> }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
