On 22/06/2018 11:57, Chris Wilson wrote:
As we may cancel the ce->state allocation during context pinning (but
crucially after we mark ce as operational), that means we may be asked
to destroy a nonexistent ce->state. Given the choice in handing a
complex error path on pinning, and just ignoring the lack of state in
destroy, choice the latter for simplicity.
Reported-by: Zhao Yakui <yakui.zhao@xxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_lrc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 33bc914c2ef5..02ee3b12507f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1337,11 +1337,15 @@ static void execlists_schedule(struct i915_request *request,
static void execlists_context_destroy(struct intel_context *ce)
{
- GEM_BUG_ON(!ce->state);
GEM_BUG_ON(ce->pin_count);
+ if (!ce->state)
+ return;
Or set ce->ops only after success in execlists_context_pin? Sounds
simpler and more logical unless I am missing something.
+
+ GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj));
+
intel_ring_free(ce->ring);
- __i915_gem_object_release_unless_active(ce->state->obj);
+ i915_gem_object_put(ce->state->obj);
Hm this bit is unexpected. I don't see an immediate intersect with the
commit message and previous change. Intuitively it makes sense that
ce->state->obj can/must never be active once here - but then doesn't
this second part belong in a separate patch?
Regards,
Tvrtko
}
static void execlists_context_unpin(struct intel_context *ce)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
