Re: [PATCH] drm/i915: Fix irq enable tracking in driver load

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, Aug 27, 2014 at 10:11:34AM +0200, Daniel Vetter wrote:
> A bunch of warnings fire on some ->irq_postinstall hooks since those
> can enable interrupts (e.g. rps interrupts). And then our ordering
> self-checks fire and complain.
> 
> To fix that set the tracking boolen before enabling the irqs with
> drm_irq_install. Quoting the discussion with Jesse why that's safe:
> 
> On Tue, Aug 26, 2014 at 11:18 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > Yes, it might work, but if you look through the history, we set this
> > field carefully; first to true in the irq_init code, then to false only
> > after the irq_install completes.  So I think your fragility arguments
> > apply to this change too.
> 
> Well we've done it in 4 commits or so, but currently we have:
> 
> - Set irqs_disabled to true early in driver load to make sure checks
> that. That's done in irq_init, which is totally not the function that
> enables interrupts, only the function that initializes all the vtables
> and similar things. We actually have a fairly sane naming scheme
> nowadays (not fully consistent ofc): _init is sw setup,
> _enable/_hw_init is the actual hw setup. That is done in
> 95f25beddba2ec9510b249740bacc11eca70cf75
> 
> - Set irqs_disabled to false right after the irqs are actually
> enabled. This is done in ed2e6df18935beb3d63613c50103bf9757b2aa85
> 
> So my change should only move the flag change over the ->preinstall
> and ->postinstall hooks. I've done a little audit and didn't spot
> anything amiss. Furthermore the runtime pm setup already clears
> irqs_disabled _before_ calling these two hooks.
> 
> This regression has been introduced in
> 
> commit ed2e6df18935beb3d63613c50103bf9757b2aa85
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date:   Fri Jun 20 09:39:36 2014 -0700
> 
>     drm/i915: clear pm._irqs_disabled field after installing IRQs
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
> Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Chris Wilson <chris@chris-wilson.co.uk> # gm45, ilk

gm45 because it has been one of my troublesome machines in the past wrt
to modeset init. ilk because it exhibits this bug.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre