public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Oliver Hartkopp <socketcan@hartkopp.net>
Subject: Re: [PATCH] drm/i915: Fix irq enable tracking in driver load
Date: Thu, 4 Sep 2014 15:59:36 +0200	[thread overview]
Message-ID: <20140904135936.GU15520@phenom.ffwll.local> (raw)
In-Reply-To: <87tx4nec37.fsf@intel.com>

On Thu, Sep 04, 2014 at 04:42:36PM +0300, Jani Nikula wrote:
> On Thu, 04 Sep 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Thu, Sep 04, 2014 at 02:12:10PM +0300, Jani Nikula wrote:
> >> On Wed, 27 Aug 2014, Daniel Vetter <daniel.vetter@ffwll.ch> 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@SHSMSX101.ccr.corp.intel.com
> >
> > Shrug it off as an unstable test result. Both mine and Jesse's patch
> > really only change the logic we use to WARN about interrupt state. We
> > don't use pm._irqs_disabled for anything else at all.
> 
> Okay, so this is a PITA to review, but at least
> ironlake_enable_display_irq will behave differently during
> drm_irq_install because of this patch.

Oops, I've somehow completely missed the early return in there. That means
we actually break the ironlake rps setup done in postinstall without any
of these patches. I've mixed this up with the pipestat check I've done
where I just WARN, but don't bail out.

tbh not sure why we bail out, at worst we'll get a few unclaimed register
warnings.

The only other place is if we get an interrupt right away, but that means
the preinstall hook has a bug somewhere. So the only place where behaviour
chances is still only ironlake, so still no explanation why hsw/bdw
suddenly start to fall over all together.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

  reply	other threads:[~2014-09-04 13:59 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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=20140904135936.GU15520@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox