On Wed, 27 Aug 2014, Daniel Vetter <daniel.vetter@xxxxxxxx> 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 witho > drm_irq_install. Quoting the discussion with Jesse why that's safe: Yi Sun's testing result needs to be addressed one way or another before merging this: http://mid.gmane.org/D9F66AA509623343B6A9A3D4502D5A52112B0676@xxxxxxxxxxxxxxxxxxxxxxxxxxxx BR, Jani. > > On Tue, Aug 26, 2014 at 11:18 PM, Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> > Date: Fri Jun 20 09:39:36 2014 -0700 > > drm/i915: clear pm._irqs_disabled field after installing IRQs > > Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > Cc: Oliver Hartkopp <socketcan@xxxxxxxxxxxx> > Tested-by: Oliver Hartkopp <socketcan@xxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/i915_dma.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 3b24e3bb2106..498980661b2f 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1336,12 +1336,17 @@ static int i915_load_modeset_init(struct drm_device *dev) > > intel_power_domains_init_hw(dev_priv); > > + /* > + * We enable some interrupt sources in our postinstall hooks, so mark > + * interrupts as enabled _before_ actually enabling them to avoid > + * special cases in our ordering checks. > + */ > + dev_priv->pm._irqs_disabled = false; > + > ret = drm_irq_install(dev, dev->pdev->irq); > if (ret) > goto cleanup_gem_stolen; > > - dev_priv->pm._irqs_disabled = false; > - > /* Important: The output setup functions called by modeset_init need > * working irqs for e.g. gmbus and dp aux transfers. */ > intel_modeset_init(dev); > -- > 2.0.1 > -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
