On Thu, Sep 24, 2015 at 02:24:36PM +0300, Jani Nikula wrote: > On Thu, 24 Sep 2015, "Winiarski, Michal" <michal.winiarski@xxxxxxxxx> wrote: > > 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.winiarski@xxxxxxxxx> > >> Date: Wed Sep 16 11:49:00 2015 +0200 > >> > >> drm/i915/gtt: Do not initialize drm_mm twice. > > I confirm reverting the above works. > > This patch is > > Tested-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > on a BWD GT2 ULT. Queued for -next, thanks for the patch. -Daniel > > >> > >> Whilst I am here remove the duplicate work leaving dangling pointers > >> from the error path... > >> > >> v2: Typos galore before coffee. > >> > >> Reported-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92099 > >> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > >> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > >> Cc: Michel Thierry <michel.thierry@xxxxxxxxx> > >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > >> Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx> > >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > >> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > > > > Reviewed-by: Michał Winiarski <michal.winiarski@xxxxxxxxx> > > > > -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; > >> } > > -- > Jani Nikula, Intel Open Source Technology Center -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
