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. >> >> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
