[Intel-gfx] [PATCH 07/14] drm/i915: s/pm._irqs_disabled/pm.irqs_enabled/

Chris Wilson chris at chris-wilson.co.uk
Fri Oct 3 11:27:26 CEST 2014


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 <daniel.vetter at ffwll.ch>:
> > > 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
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre



More information about the Intel-gfx mailing list