Chris Wilson <ch...@chris-wilson.co.uk> writes: > My original intention was for i915_sw_fence to be the base class and > provide the reference count for the container. This was from starting > with a design to handle async_work. In practice, for i915 we embed > fences into structs which have their own independent reference counting, > making the i915_sw_fence.kref duplicitous. If we remove the kref, we > remove the i915_sw_fence's ability to free itself and its independence, > it can only exist within a container and must be supplied with a > callback to handle its release. > > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuopp...@intel.com> > --- > drivers/gpu/drm/i915/i915_sw_fence.c | 55 > ++++++++---------------------------- > drivers/gpu/drm/i915/i915_sw_fence.h | 1 - > 2 files changed, 11 insertions(+), 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c > b/drivers/gpu/drm/i915/i915_sw_fence.c > index a277f8eb7beb..a0a690d6627e 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.c > +++ b/drivers/gpu/drm/i915/i915_sw_fence.c > @@ -120,34 +120,6 @@ void i915_sw_fence_fini(struct i915_sw_fence *fence) > } > #endif > > -static void i915_sw_fence_release(struct kref *kref) > -{ > - struct i915_sw_fence *fence = container_of(kref, typeof(*fence), kref); > - > - WARN_ON(atomic_read(&fence->pending) > 0); > - debug_fence_destroy(fence); > - > - if (fence->flags & I915_SW_FENCE_MASK) { > - __i915_sw_fence_notify(fence, FENCE_FREE); > - } else { > - i915_sw_fence_fini(fence); > - kfree(fence); > - } > -} > - > -static void i915_sw_fence_put(struct i915_sw_fence *fence) > -{ > - debug_fence_assert(fence); > - kref_put(&fence->kref, i915_sw_fence_release); > -} > - > -static struct i915_sw_fence *i915_sw_fence_get(struct i915_sw_fence *fence) > -{ > - debug_fence_assert(fence); > - kref_get(&fence->kref); > - return fence; > -} > - > static void __i915_sw_fence_wake_up_all(struct i915_sw_fence *fence, > struct list_head *continuation) > { > @@ -202,13 +174,15 @@ static void __i915_sw_fence_complete(struct > i915_sw_fence *fence, > > debug_fence_set_state(fence, DEBUG_FENCE_IDLE, DEBUG_FENCE_NOTIFY); > > - if (fence->flags & I915_SW_FENCE_MASK && > - __i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE) > + if (__i915_sw_fence_notify(fence, FENCE_COMPLETE) != NOTIFY_DONE) > return; > > debug_fence_set_state(fence, DEBUG_FENCE_NOTIFY, DEBUG_FENCE_IDLE); > > __i915_sw_fence_wake_up_all(fence, continuation); > + > + debug_fence_destroy(fence); > + __i915_sw_fence_notify(fence, FENCE_FREE); > } > > static void i915_sw_fence_complete(struct i915_sw_fence *fence) > @@ -232,33 +206,26 @@ void __i915_sw_fence_init(struct i915_sw_fence *fence, > const char *name, > struct lock_class_key *key) > { > - BUG_ON((unsigned long)fn & ~I915_SW_FENCE_MASK); > + BUG_ON(!fn || (unsigned long)fn & ~I915_SW_FENCE_MASK); > > debug_fence_init(fence); > > __init_waitqueue_head(&fence->wait, name, key); > - kref_init(&fence->kref); > atomic_set(&fence->pending, 1); > fence->flags = (unsigned long)fn; > } > > -static void __i915_sw_fence_commit(struct i915_sw_fence *fence) > -{ > - i915_sw_fence_complete(fence); > - i915_sw_fence_put(fence); > -} > - > void i915_sw_fence_commit(struct i915_sw_fence *fence) > { > debug_fence_activate(fence); > - __i915_sw_fence_commit(fence); > + i915_sw_fence_complete(fence); > } > > static int i915_sw_fence_wake(wait_queue_t *wq, unsigned mode, int flags, > void *key) > { > list_del(&wq->task_list); > __i915_sw_fence_complete(wq->private, key); > - i915_sw_fence_put(wq->private); > + > if (wq->flags & I915_SW_FENCE_FLAG_ALLOC) > kfree(wq); > return 0; > @@ -353,7 +320,7 @@ static int __i915_sw_fence_await_sw_fence(struct > i915_sw_fence *fence, > INIT_LIST_HEAD(&wq->task_list); > wq->flags = pending; > wq->func = i915_sw_fence_wake; > - wq->private = i915_sw_fence_get(fence); > + wq->private = fence; > > i915_sw_fence_await(fence); > > @@ -402,7 +369,7 @@ static void timer_i915_sw_fence_wake(unsigned long data) > dma_fence_put(cb->dma); > cb->dma = NULL; > > - __i915_sw_fence_commit(cb->fence); > + i915_sw_fence_complete(cb->fence); > cb->timer.function = NULL; > } > > @@ -413,7 +380,7 @@ static void dma_i915_sw_fence_wake(struct dma_fence *dma, > > del_timer_sync(&cb->timer); > if (cb->timer.function) > - __i915_sw_fence_commit(cb->fence); > + i915_sw_fence_complete(cb->fence); > dma_fence_put(cb->dma); > > kfree(cb); > @@ -440,7 +407,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence > *fence, > return dma_fence_wait(dma, false); > } > > - cb->fence = i915_sw_fence_get(fence); > + cb->fence = fence; > i915_sw_fence_await(fence); > > cb->dma = NULL; > diff --git a/drivers/gpu/drm/i915/i915_sw_fence.h > b/drivers/gpu/drm/i915/i915_sw_fence.h > index d31cefbbcc04..1d3b6051daaf 100644 > --- a/drivers/gpu/drm/i915/i915_sw_fence.h > +++ b/drivers/gpu/drm/i915/i915_sw_fence.h > @@ -23,7 +23,6 @@ struct reservation_object; > struct i915_sw_fence { > wait_queue_head_t wait; > unsigned long flags; > - struct kref kref; > atomic_t pending; > }; > > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx