| Message ID | 20200715115147.11866-5-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/66] drm/i915: Reduce i915_request.lock contention for i915_request_wait | expand |
On 15/07/2020 12:50, Chris Wilson wrote: > If no active callback is defined for i915_active, we do not need to > serialise its enabling with the mutex. We still do only want to call the > debug activate once, and must still serialise with a concurrent retire. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_active.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > index d960d0be5bd2..841b5c30950a 100644 > --- a/drivers/gpu/drm/i915/i915_active.c > +++ b/drivers/gpu/drm/i915/i915_active.c > @@ -416,6 +416,14 @@ bool i915_active_acquire_if_busy(struct i915_active *ref) > return atomic_add_unless(&ref->count, 1, 0); > } > > +static void __i915_active_activate(struct i915_active *ref) > +{ > + spin_lock_irq(&ref->tree_lock); /* __active_retire() */ > + if (!atomic_fetch_inc(&ref->count)) > + debug_active_activate(ref); > + spin_unlock_irq(&ref->tree_lock); > +} > + > int i915_active_acquire(struct i915_active *ref) > { > int err; > @@ -423,23 +431,22 @@ int i915_active_acquire(struct i915_active *ref) > if (i915_active_acquire_if_busy(ref)) > return 0; > > + if (!ref->active) { > + __i915_active_activate(ref); > + return 0; > + } > + > err = mutex_lock_interruptible(&ref->mutex); > if (err) > return err; > > if (likely(!i915_active_acquire_if_busy(ref))) { > - if (ref->active) > - err = ref->active(ref); > - if (!err) { > - spin_lock_irq(&ref->tree_lock); /* __active_retire() */ > - debug_active_activate(ref); > - atomic_inc(&ref->count); > - spin_unlock_irq(&ref->tree_lock); > - } > + err = ref->active(ref); > + if (!err) > + __i915_active_activate(ref); > } > > mutex_unlock(&ref->mutex); > - Blank line was nice there. > return err; > } > > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On 2020-07-15 13:50, Chris Wilson wrote: > If no active callback is defined for i915_active, we do not need to > serialise its enabling with the mutex. We still do only want to call the > debug activate once, and must still serialise with a concurrent retire. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Minor nit below, Otherwise Reviewed-by: Thomas Hellström <thomas.hellstrom@intel.com> > --- > drivers/gpu/drm/i915/i915_active.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > index d960d0be5bd2..841b5c30950a 100644 > --- a/drivers/gpu/drm/i915/i915_active.c > +++ b/drivers/gpu/drm/i915/i915_active.c > @@ -416,6 +416,14 @@ bool i915_active_acquire_if_busy(struct i915_active *ref) > return atomic_add_unless(&ref->count, 1, 0); > } > > +static void __i915_active_activate(struct i915_active *ref) > +{ > + spin_lock_irq(&ref->tree_lock); /* __active_retire() */ > + if (!atomic_fetch_inc(&ref->count)) > + debug_active_activate(ref); > + spin_unlock_irq(&ref->tree_lock); > +} > + > int i915_active_acquire(struct i915_active *ref) > { > int err; > @@ -423,23 +431,22 @@ int i915_active_acquire(struct i915_active *ref) > if (i915_active_acquire_if_busy(ref)) > return 0; > > + if (!ref->active) { > + __i915_active_activate(ref); > + return 0; > + } > + > err = mutex_lock_interruptible(&ref->mutex); > if (err) > return err; > > if (likely(!i915_active_acquire_if_busy(ref))) { > - if (ref->active) > - err = ref->active(ref); > - if (!err) { > - spin_lock_irq(&ref->tree_lock); /* __active_retire() */ > - debug_active_activate(ref); > - atomic_inc(&ref->count); > - spin_unlock_irq(&ref->tree_lock); > - } > + err = ref->active(ref); > + if (!err) > + __i915_active_activate(ref); > } > > mutex_unlock(&ref->mutex); > - Unrelated > return err; > } >
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index d960d0be5bd2..841b5c30950a 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -416,6 +416,14 @@ bool i915_active_acquire_if_busy(struct i915_active *ref) return atomic_add_unless(&ref->count, 1, 0); } +static void __i915_active_activate(struct i915_active *ref) +{ + spin_lock_irq(&ref->tree_lock); /* __active_retire() */ + if (!atomic_fetch_inc(&ref->count)) + debug_active_activate(ref); + spin_unlock_irq(&ref->tree_lock); +} + int i915_active_acquire(struct i915_active *ref) { int err; @@ -423,23 +431,22 @@ int i915_active_acquire(struct i915_active *ref) if (i915_active_acquire_if_busy(ref)) return 0; + if (!ref->active) { + __i915_active_activate(ref); + return 0; + } + err = mutex_lock_interruptible(&ref->mutex); if (err) return err; if (likely(!i915_active_acquire_if_busy(ref))) { - if (ref->active) - err = ref->active(ref); - if (!err) { - spin_lock_irq(&ref->tree_lock); /* __active_retire() */ - debug_active_activate(ref); - atomic_inc(&ref->count); - spin_unlock_irq(&ref->tree_lock); - } + err = ref->active(ref); + if (!err) + __i915_active_activate(ref); } mutex_unlock(&ref->mutex); - return err; }
If no active callback is defined for i915_active, we do not need to serialise its enabling with the mutex. We still do only want to call the debug activate once, and must still serialise with a concurrent retire. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_active.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)