Quoting Mika Kuoppala (2019-06-14 15:47:46)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
