All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix irq enable tracking in driver load
@ 2014-08-27  8:11 Daniel Vetter
  2014-08-27  9:01 ` Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Vetter @ 2014-08-27  8:11 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Oliver Hartkopp

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>
---
 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-09-08  7:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-27  8:11 [PATCH] drm/i915: Fix irq enable tracking in driver load Daniel Vetter
2014-08-27  9:01 ` Chris Wilson
2014-08-27 18:43 ` Chris Wilson
2014-09-05 16:06   ` Jesse Barnes
2014-09-04 11:12 ` Jani Nikula
2014-09-04 11:13   ` Jani Nikula
2014-09-04 13:05   ` Daniel Vetter
2014-09-04 13:42     ` Jani Nikula
2014-09-04 13:59       ` Daniel Vetter
2014-09-08  7:03         ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.