| Message ID | 20180625100604.22598-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25/06/2018 11:06, Chris Wilson wrote: > Due to how we only release the pining on the context state on > retirement and never track activity on the context vma itself, the > object can never be active at the point of release. Replace the > conditional transfer of ownership onto an active-reference with an > assert that the object is idle. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 4 +++- > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 97460ee25b7d..2b21a6596360 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1343,7 +1343,9 @@ static void execlists_context_destroy(struct intel_context *ce) > return; > > intel_ring_free(ce->ring); > - __i915_gem_object_release_unless_active(ce->state->obj); > + > + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); > + i915_gem_object_put(ce->state->obj); > } > > static void execlists_context_unpin(struct intel_context *ce) > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index e0448eff12bd..700f94c371b3 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -1169,8 +1169,11 @@ static void intel_ring_context_destroy(struct intel_context *ce) > { > GEM_BUG_ON(ce->pin_count); > > - if (ce->state) > - __i915_gem_object_release_unless_active(ce->state->obj); > + if (!ce->state) > + return; > + > + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); > + i915_gem_object_put(ce->state->obj); > } > > static int __context_pin_ppgtt(struct i915_gem_context *ctx) > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
Quoting Tvrtko Ursulin (2018-06-25 11:42:27) > > On 25/06/2018 11:06, Chris Wilson wrote: > > Due to how we only release the pining on the context state on > > retirement and never track activity on the context vma itself, the > > object can never be active at the point of release. Replace the > > conditional transfer of ownership onto an active-reference with an > > assert that the object is idle. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > --- > > drivers/gpu/drm/i915/intel_lrc.c | 4 +++- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++-- > > 2 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > index 97460ee25b7d..2b21a6596360 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -1343,7 +1343,9 @@ static void execlists_context_destroy(struct intel_context *ce) > > return; > > > > intel_ring_free(ce->ring); > > - __i915_gem_object_release_unless_active(ce->state->obj); > > + > > + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); > > + i915_gem_object_put(ce->state->obj); > > } > > > > static void execlists_context_unpin(struct intel_context *ce) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index e0448eff12bd..700f94c371b3 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -1169,8 +1169,11 @@ static void intel_ring_context_destroy(struct intel_context *ce) > > { > > GEM_BUG_ON(ce->pin_count); > > > > - if (ce->state) > > - __i915_gem_object_release_unless_active(ce->state->obj); > > + if (!ce->state) > > + return; > > + > > + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); > > + i915_gem_object_put(ce->state->obj); > > } > > > > static int __context_pin_ppgtt(struct i915_gem_context *ctx) > > > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Thanks for the review, pushed. If I remember, I should poke Zhao for an updated patch! -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 97460ee25b7d..2b21a6596360 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1343,7 +1343,9 @@ static void execlists_context_destroy(struct intel_context *ce) return; intel_ring_free(ce->ring); - __i915_gem_object_release_unless_active(ce->state->obj); + + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); + i915_gem_object_put(ce->state->obj); } static void execlists_context_unpin(struct intel_context *ce) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index e0448eff12bd..700f94c371b3 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1169,8 +1169,11 @@ static void intel_ring_context_destroy(struct intel_context *ce) { GEM_BUG_ON(ce->pin_count); - if (ce->state) - __i915_gem_object_release_unless_active(ce->state->obj); + if (!ce->state) + return; + + GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); + i915_gem_object_put(ce->state->obj); } static int __context_pin_ppgtt(struct i915_gem_context *ctx)
Due to how we only release the pining on the context state on retirement and never track activity on the context vma itself, the object can never be active at the point of release. Replace the conditional transfer of ownership onto an active-reference with an assert that the object is idle. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 4 +++- drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-)