All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Oliver Hartkopp <socketcan@hartkopp.net>
Subject: [PATCH] drm/i915: Fix irq enable tracking in driver load
Date: Wed, 27 Aug 2014 10:11:34 +0200	[thread overview]
Message-ID: <1409127094-5843-1-git-send-email-daniel.vetter@ffwll.ch> (raw)

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

             reply	other threads:[~2014-08-27  8:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-27  8:11 Daniel Vetter [this message]
2014-08-27  9:01 ` [PATCH] drm/i915: Fix irq enable tracking in driver load 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1409127094-5843-1-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=socketcan@hartkopp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.