From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: disable interrupts earlier in the driver unload code Date: Wed, 24 Apr 2013 11:51:31 +0200 Message-ID: <20130424095131.GP6169@phenom.ffwll.local> References: <1366795160-2427-1-git-send-email-daniel.vetter@ffwll.ch> <20130424093516.GC11689@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f170.google.com (mail-ea0-f170.google.com [209.85.215.170]) by gabe.freedesktop.org (Postfix) with ESMTP id 0233FE6341 for ; Wed, 24 Apr 2013 02:48:29 -0700 (PDT) Received: by mail-ea0-f170.google.com with SMTP id z7so622160eaf.1 for ; Wed, 24 Apr 2013 02:48:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130424093516.GC11689@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , Daniel Vetter , Intel Graphics Development , Jani Nikula List-Id: intel-gfx@lists.freedesktop.org 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