Quoting Matthew Auld (2019-08-15 21:33:07)
> On Wed, 14 Aug 2019 at 10:28, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Forgo the struct_mutex requirement for request retirement as we have
> > been transitioning over to only using the timeline->mutex for
> > controlling the lifetime of a request on that timeline.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > .../gpu/drm/i915/gem/i915_gem_execbuffer.c | 183 ++++++++++--------
> > drivers/gpu/drm/i915/gt/intel_context.h | 18 +-
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 1 -
> > drivers/gpu/drm/i915/gt/intel_engine_types.h | 3 -
> > drivers/gpu/drm/i915/gt/intel_gt.c | 2 -
> > drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 -
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 1 +
> > drivers/gpu/drm/i915/gt/intel_ringbuffer.c | 19 +-
> > drivers/gpu/drm/i915/gt/mock_engine.c | 1 -
> > drivers/gpu/drm/i915/gt/selftest_context.c | 9 +-
> > drivers/gpu/drm/i915/i915_request.c | 156 +++++++--------
> > drivers/gpu/drm/i915/i915_request.h | 3 -
> > 12 files changed, 209 insertions(+), 189 deletions(-)
> >
>
> > bool i915_retire_requests(struct drm_i915_private *i915)
> > {
> > - struct intel_ring *ring, *tmp;
> > + struct intel_gt_timelines *timelines = &i915->gt.timelines;
> > + struct intel_timeline *tl, *tn;
> > + LIST_HEAD(free);
> > +
> > + spin_lock(&timelines->lock);
> > + list_for_each_entry_safe(tl, tn, &timelines->active_list, link) {
> > + if (!mutex_trylock(&tl->mutex))
> > + continue;
> >
> > - lockdep_assert_held(&i915->drm.struct_mutex);
> > + intel_timeline_get(tl);
> > + GEM_BUG_ON(!tl->active_count);
> > + tl->active_count++; /* pin the list element */
> > + spin_unlock(&timelines->lock);
> >
> > - list_for_each_entry_safe(ring, tmp,
> > - &i915->gt.active_rings, active_link) {
> > - intel_ring_get(ring); /* last rq holds reference! */
> > - ring_retire_requests(ring);
> > - intel_ring_put(ring);
> > + retire_requests(tl);
> > +
> > + spin_lock(&timelines->lock);
> > +
> > + /* Restart iteration after dropping lock */
> > + list_safe_reset_next(tl, tn, link);
>
> That's a new one.
I was quite chuffed with that loop, all the pinning across the lock
dropping to ensure the list stayed intact and we could resume from where
we left off.
And if all goes to plan, we should be rarely using this loop!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
