From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 07/14] drm/i915: s/pm._irqs_disabled/pm.irqs_enabled/ Date: Fri, 3 Oct 2014 13:49:25 +0200 Message-ID: <20141003114925.GF16117@phenom.ffwll.local> References: <1412067410-9346-1-git-send-email-daniel.vetter@ffwll.ch> <1412067410-9346-8-git-send-email-daniel.vetter@ffwll.ch> <20141003091926.GE16117@phenom.ffwll.local> <20141003092726.GY19278@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f47.google.com (mail-wg0-f47.google.com [74.125.82.47]) by gabe.freedesktop.org (Postfix) with ESMTP id 019A76E132 for ; Fri, 3 Oct 2014 04:49:30 -0700 (PDT) Received: by mail-wg0-f47.google.com with SMTP id x13so1251463wgg.18 for ; Fri, 03 Oct 2014 04:49:29 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20141003092726.GY19278@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , Paulo Zanoni , Daniel Vetter , Intel Graphics Development , Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org On Fri, Oct 03, 2014 at 10:27:26AM +0100, Chris Wilson wrote: > On Fri, Oct 03, 2014 at 11:19:26AM +0200, Daniel Vetter wrote: > > On Thu, Oct 02, 2014 at 05:36:11PM -0300, Paulo Zanoni wrote: > > > 2014-09-30 5:56 GMT-03:00 Daniel Vetter : > > > > Double negations just parse harder. Also this allows us to ditch some > > > > init code since clearing to 0 dtrt. Also ditch the assignment in > > > > intel_pm_setup, that's not redundant since we do the assignement now > > > > while setting up interrupts. > > > > > > > > While at it do engage in a bit of OCD and wrap up the few lines of > > > > setup/teardown code into little helper functions: intel_irq_fini for > > > > cleanup and intel_irq_init_hw for hw setup. > > > > > > So the werid thing is that we now have: > > > - intel_irq_init > > > - intel_irq_init_hw > > > - intel_irq_fini > > > > > > But the intel_irq_fini doesn't finish what intel_irq_init started, it > > > finishes what intel_irq_init_hw started. Since the functions you > > > introduced are basically wrappers to drm_irq_{un,}install, my bikeshed > > > would be to call the new functions simply intel_irq_install and > > > intel_irq_uninstall. > > > > I like this idea, so changed the names while merging. > > Is it worth the divergence? I think the right pattern for other areas of > the driver is: > > init > while : > resume > suspend > fini > > That becomes something like > > intel_irq_init > i915_gem_init > ... > while : > intel_irq_resume > i915_gem_resume (formerly i95_gem_init_hw) > ... > i915_gem_suspend > intel_irq_suspend > ... > i915_gem_fini > intel_irq_fini irq_install/uninstall is only done a driver load/unload time, so doesn't really fit into the pattern. At runtime (for system suspend/resume and anything else) we now disable/enable interrupts using the runtime pm functions. Unfoturnately that part is hand-rolled since runtime pm is still completely orthogonal to system s/r. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch