Re: [PATCH] drm/i915: disable interrupts earlier in the driver unload code

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 24, 2013 at 10:35:16AM +0100, Chris Wilson wrote:
> On Wed, Apr 24, 2013 at 11:19:20AM +0200, Daniel Vetter wrote:
> > Our rps code relies on the interrupts being off to prevent re-arming
> > of the work items at inopportune moments.
> > 
> > Also drop the redundant cancel_work for the main rps work,
> > disable_gt_powersave already takes care of that.
> > 
> > Finally add a WARN_ON to ensure we obey that piece of ordering
> > constraint. Long term I want to lock down the setup/teardown code in a
> > similar way to how we painstakingly check modeset sequence constraints
> > already.
> 
> Reminds me, did we every get around to flushing our deferred hw tasks
> upon suspend? I've a funny feeling that is still missing.

I think with the unification between suspend and driver unload we're
mostly there. Like I've said I think it's time to encode all these
constraints properly into debug asserts, but the problem I have is that I
don't have a good idea to encode things like "this work item is now
guaranteed to never get re-armed again".

There's the object debug code in the kernel, but that only fires when the
work/timer is actually armed. Since a lot of the work stuff happens rarely
or is not delayed that's not really good enough ...

Ideas for this very much welcome, since the lack of these self-checks
makes any attempt to solve this for real futile. devres.c might help a bit
in preventing resource leaks and tidy up the code, but it doesn't help at
all for correct ordering and cleanup of async workers/timers.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch