[Intel-gfx] [PATCH 03/39] drm/i915: Avoid tainting i915_gem_park() with wakeref.lock

Chris Wilson chris at chris-wilson.co.uk
Fri Jun 14 14:51:16 UTC 2019


Quoting Mika Kuoppala (2019-06-14 15:47:46)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > While we need to flush the wakeref before parking, we do not need to
> > perform the i915_gem_park() itself underneath the wakeref lock, merely
> > the struct_mutex. If we rearrange the locks, we can avoid the unnecessary
> > tainting.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_pm.c | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > index 6e75702c5671..a33f69610d6f 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c
> > @@ -30,23 +30,22 @@ static void idle_work_handler(struct work_struct *work)
> >  {
> >       struct drm_i915_private *i915 =
> >               container_of(work, typeof(*i915), gem.idle_work);
> > -     bool restart = true;
> > +     bool park;
> >
> 
> can_park...meh.
> 
> > -     cancel_delayed_work(&i915->gem.retire_work);
> > +     cancel_delayed_work_sync(&i915->gem.retire_work);
> >       mutex_lock(&i915->drm.struct_mutex);
> >  
> >       intel_wakeref_lock(&i915->gt.wakeref);
> > -     if (!intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work)) {
> > -             i915_gem_park(i915);
> > -             restart = false;
> > -     }
> > +     park = !intel_wakeref_active(&i915->gt.wakeref) && !work_pending(work);
> >       intel_wakeref_unlock(&i915->gt.wakeref);
> > -
> > -     mutex_unlock(&i915->drm.struct_mutex);
> > -     if (restart)
> > +     if (park)
> > +             i915_gem_park(i915);
> 
> Did not find anything beneath gem park that would need wakeref.

In fact, mostly planning to move this to a timer and not use the global
idle. Certainly for i915_vma_parked() which is just supposed to be cache
of userspace vma and is causing me some pain with i915_vma.kref :)
-Chris


More information about the Intel-gfx mailing list