All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
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: Mon, 08 Sep 2014 10:03:24 +0300	[thread overview]
Message-ID: <871trmzj9f.fsf@intel.com> (raw)
In-Reply-To: <20140904135936.GU15520@phenom.ffwll.local>

On Thu, 04 Sep 2014, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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.

Pushed to drm-intel-fixes. Thanks for the patch, testing and review.

BR,
Jani.



> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Jani Nikula, Intel Open Source Technology Center

      reply	other threads:[~2014-09-08  7:03 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
2014-09-08  7:03         ` Jani Nikula [this message]

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=871trmzj9f.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel@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.