| Message ID | 1444315020-32033-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/8/2015 3:37 PM, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae > Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Date: Mon Oct 5 13:26:36 2015 +0100 > > drm/i915: Clean up associated VMAs on context destruction > > Introduced a wrong assumption that all contexts have a ppgtt > instance. This is not true when full PPGTT is not active so > remove the WARN_ON_ONCE from the context cleanup code. Aliasing ppgtt? (for the record, I tried with enable_ppgtt=1 while reviewing the patch and didn't see the warning... maybe I didn't try hard enough). > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 680b4c9f6b73..8c688a5f1589 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -138,7 +138,7 @@ static void i915_gem_context_clean(struct intel_context *ctx) > struct i915_hw_ppgtt *ppgtt = ctx->ppgtt; > struct i915_vma *vma, *next; > > - if (WARN_ON_ONCE(!ppgtt)) > + if (!ppgtt) > return; > > WARN_ON(!list_empty(&ppgtt->base.active_list)); > Reviewed-by: Michel Thierry <michel.thierry@intel.com>
On Thu, Oct 08, 2015 at 04:12:31PM +0100, Michel Thierry wrote: > On 10/8/2015 3:37 PM, Tvrtko Ursulin wrote: > >From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > >commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae > >Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >Date: Mon Oct 5 13:26:36 2015 +0100 > > > > drm/i915: Clean up associated VMAs on context destruction > > > >Introduced a wrong assumption that all contexts have a ppgtt > >instance. This is not true when full PPGTT is not active so > >remove the WARN_ON_ONCE from the context cleanup code. > > Aliasing ppgtt? (for the record, I tried with enable_ppgtt=1 while > reviewing the patch and didn't see the warning... maybe I didn't try > hard enough). We create a context per-fd on all platforms, as in it we store user specific information (such as hangstats, process identifiers etc). Think of a context as a view of driver state not just GPU state. -Chris
On Thu, Oct 08, 2015 at 04:12:31PM +0100, Michel Thierry wrote: > On 10/8/2015 3:37 PM, Tvrtko Ursulin wrote: > >From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > > >commit e9f24d5fb7cf3628b195b18ff3ac4e37937ceeae > >Author: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >Date: Mon Oct 5 13:26:36 2015 +0100 > > > > drm/i915: Clean up associated VMAs on context destruction > > > >Introduced a wrong assumption that all contexts have a ppgtt > >instance. This is not true when full PPGTT is not active so > >remove the WARN_ON_ONCE from the context cleanup code. > > Aliasing ppgtt? (for the record, I tried with enable_ppgtt=1 while reviewing > the patch and didn't see the warning... maybe I didn't try hard enough). Aliasing ppgtt isn't a real ppgtt attached to a context, but just a pile of pagetables attached to the global gtt in dev_priv->mm.aliasing_ppgtt. Originally I had the idea that there would be another structure for the real per-process stuff like struct i915_ppgtt { i915_address_space base; i915_hw_ppgtt ppgtt; }; but that kinda never got anywhere with the original per-process infrastructure merge. So now I get to have a broken record about "aliasing ppgtt isnt' a real ppgtt and will never show up in ctx->ppgtt". > > > > >Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > >Cc: Michel Thierry <michel.thierry@intel.com> > >--- > > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > >index 680b4c9f6b73..8c688a5f1589 100644 > >--- a/drivers/gpu/drm/i915/i915_gem_context.c > >+++ b/drivers/gpu/drm/i915/i915_gem_context.c > >@@ -138,7 +138,7 @@ static void i915_gem_context_clean(struct intel_context *ctx) > > struct i915_hw_ppgtt *ppgtt = ctx->ppgtt; > > struct i915_vma *vma, *next; > > > >- if (WARN_ON_ONCE(!ppgtt)) > >+ if (!ppgtt) > > return; > > > > WARN_ON(!list_empty(&ppgtt->base.active_list)); > > > > Reviewed-by: Michel Thierry <michel.thierry@intel.com> Queued for -next, thanks for the patch. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 680b4c9f6b73..8c688a5f1589 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -138,7 +138,7 @@ static void i915_gem_context_clean(struct intel_context *ctx) struct i915_hw_ppgtt *ppgtt = ctx->ppgtt; struct i915_vma *vma, *next; - if (WARN_ON_ONCE(!ppgtt)) + if (!ppgtt) return; WARN_ON(!list_empty(&ppgtt->base.active_list));