On Sun, Mar 02, 2014 at 03:58:09PM -0800, Ben Widawsky wrote:
> On Fri, Feb 28, 2014 at 08:06:50PM +0000, Chris Wilson wrote:
> > ctx->obj = i915_gem_object_create_stolen(dev, dev_priv->hw_context_size);
> How this working for you ^ ?
Fine. It helps detect cases where we try to load uninitialised contexts,
but other than that I have not noticed any difference.
> > + if (HAS_HW_CONTEXTS(dev)) {
> > + idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
> > + idr_destroy(&file_priv->context_idr);
> > }
> >
> > - idr_for_each(&file_priv->context_idr, context_idr_cleanup, NULL);
> > i915_gem_context_unreference(file_priv->private_default_ctx);
> > - idr_destroy(&file_priv->context_idr);
> > }
>
> I don't see too much harm in just initializing the idr regardless to
> keep the close path simpler. Then stick a WARN in context_idr_cleanup if
> it actually does anything, fake_context_idr_cleanup();
Agreed I didn't see any harm in making that change as well. I was just
trying to keep the set of changes small, and targetted towards removing
that ctx->obj dereference.
> I have some recollection of hitting a really tricky thing while
> developing PPGTT where the order of the unref in the above sequence
> really mattered. Looking again though, I don't see anything familiar
> - but my suggestion would put me completely at ease.
Sure, but I think you've squashed that bug already. :)
> In either case:
> Reviewed-by: Ben Widawsky <ben@xxxxxxxxxxxx>
Ok, let's go with a second patch to converge the fake context with the
hw context later.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
