| Message ID | 20190718070024.21781-13-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/20] drm/i915: Move aliasing_ppgtt underneath its i915_ggtt | expand |
On 18/07/2019 08:00, Chris Wilson wrote: > In the next patch, we would like to couple into the engine wakeref to > free the batch pool on idling. The caveat here is that we therefore want > to track the engine wakeref more precisely and to hold it instead of the > broader GT wakeref as we process the ioctl. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 36 ++++++++++++------- > drivers/gpu/drm/i915/gt/intel_context.h | 7 ++++ > 2 files changed, 31 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index cbd7c6e3a1f8..8768d436e54b 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -2138,13 +2138,35 @@ static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce) > if (err) > return err; > > + /* > + * Take a local wakeref for preparing to dispatch the execbuf as > + * we expect to access the hardware fairly frequently in the > + * process. Upon first dispatch, we acquire another prolonged > + * wakeref that we hold until the GPU has been idle for at least > + * 100ms. Old comment but slightly not business of this layer to talk about 100ms. > + */ > + err = intel_context_timeline_lock(ce); > + if (err) > + goto err_unpin; > + > + intel_context_enter(ce); > + intel_context_timeline_unlock(ce); > + > eb->engine = ce->engine; > eb->context = ce; > return 0; > + > +err_unpin: > + intel_context_unpin(ce); > + return err; > } > > static void eb_unpin_context(struct i915_execbuffer *eb) > { > + __intel_context_timeline_lock(eb->context); Slight misuse of established semantics for underscores. (I'd expect no locking, or something along those lines, not a difference between interruptible and non-interruptible.) > + intel_context_exit(eb->context); > + intel_context_timeline_unlock(eb->context); > + > intel_context_unpin(eb->context); > } > > @@ -2425,18 +2447,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, > if (unlikely(err)) > goto err_destroy; > > - /* > - * Take a local wakeref for preparing to dispatch the execbuf as > - * we expect to access the hardware fairly frequently in the > - * process. Upon first dispatch, we acquire another prolonged > - * wakeref that we hold until the GPU has been idle for at least > - * 100ms. > - */ > - intel_gt_pm_get(&eb.i915->gt); > - > err = i915_mutex_lock_interruptible(dev); > if (err) > - goto err_rpm; > + goto err_context; > > err = eb_select_engine(&eb, file, args); > if (unlikely(err)) > @@ -2601,8 +2614,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, > eb_unpin_context(&eb); > err_unlock: > mutex_unlock(&dev->struct_mutex); > -err_rpm: > - intel_gt_pm_put(&eb.i915->gt); > +err_context: > i915_gem_context_put(eb.gem_context); > err_destroy: > eb_destroy(&eb); > diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h > index 23c7e4c0ce7c..485886d84a56 100644 > --- a/drivers/gpu/drm/i915/gt/intel_context.h > +++ b/drivers/gpu/drm/i915/gt/intel_context.h > @@ -127,6 +127,13 @@ static inline void intel_context_put(struct intel_context *ce) > kref_put(&ce->ref, ce->ops->destroy); > } > > +static inline void > +__intel_context_timeline_lock(struct intel_context *ce) > + __acquires(&ce->ring->timeline->mutex) > +{ > + mutex_lock(&ce->ring->timeline->mutex); > +} > + > static inline int __must_check > intel_context_timeline_lock(struct intel_context *ce) > __acquires(&ce->ring->timeline->mutex) > But passable. Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index cbd7c6e3a1f8..8768d436e54b 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -2138,13 +2138,35 @@ static int eb_pin_context(struct i915_execbuffer *eb, struct intel_context *ce) if (err) return err; + /* + * Take a local wakeref for preparing to dispatch the execbuf as + * we expect to access the hardware fairly frequently in the + * process. Upon first dispatch, we acquire another prolonged + * wakeref that we hold until the GPU has been idle for at least + * 100ms. + */ + err = intel_context_timeline_lock(ce); + if (err) + goto err_unpin; + + intel_context_enter(ce); + intel_context_timeline_unlock(ce); + eb->engine = ce->engine; eb->context = ce; return 0; + +err_unpin: + intel_context_unpin(ce); + return err; } static void eb_unpin_context(struct i915_execbuffer *eb) { + __intel_context_timeline_lock(eb->context); + intel_context_exit(eb->context); + intel_context_timeline_unlock(eb->context); + intel_context_unpin(eb->context); } @@ -2425,18 +2447,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, if (unlikely(err)) goto err_destroy; - /* - * Take a local wakeref for preparing to dispatch the execbuf as - * we expect to access the hardware fairly frequently in the - * process. Upon first dispatch, we acquire another prolonged - * wakeref that we hold until the GPU has been idle for at least - * 100ms. - */ - intel_gt_pm_get(&eb.i915->gt); - err = i915_mutex_lock_interruptible(dev); if (err) - goto err_rpm; + goto err_context; err = eb_select_engine(&eb, file, args); if (unlikely(err)) @@ -2601,8 +2614,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, eb_unpin_context(&eb); err_unlock: mutex_unlock(&dev->struct_mutex); -err_rpm: - intel_gt_pm_put(&eb.i915->gt); +err_context: i915_gem_context_put(eb.gem_context); err_destroy: eb_destroy(&eb); diff --git a/drivers/gpu/drm/i915/gt/intel_context.h b/drivers/gpu/drm/i915/gt/intel_context.h index 23c7e4c0ce7c..485886d84a56 100644 --- a/drivers/gpu/drm/i915/gt/intel_context.h +++ b/drivers/gpu/drm/i915/gt/intel_context.h @@ -127,6 +127,13 @@ static inline void intel_context_put(struct intel_context *ce) kref_put(&ce->ref, ce->ops->destroy); } +static inline void +__intel_context_timeline_lock(struct intel_context *ce) + __acquires(&ce->ring->timeline->mutex) +{ + mutex_lock(&ce->ring->timeline->mutex); +} + static inline int __must_check intel_context_timeline_lock(struct intel_context *ce) __acquires(&ce->ring->timeline->mutex)
In the next patch, we would like to couple into the engine wakeref to free the batch pool on idling. The caveat here is that we therefore want to track the engine wakeref more precisely and to hold it instead of the broader GT wakeref as we process the ioctl. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 36 ++++++++++++------- drivers/gpu/drm/i915/gt/intel_context.h | 7 ++++ 2 files changed, 31 insertions(+), 12 deletions(-)