All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Mickler <florian@mickler.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>,
	Dave Airlie <airlied@gmail.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@elte.hu>,
	Keith Packard <keithp@keithp.com>
Subject: Re: "do_IRQ: 0.89 No irq handler for vector (irq -1)"
Date: Tue, 16 Nov 2010 20:46:45 +0100	[thread overview]
Message-ID: <20101116204645.2c105e9b@schatten.dmk.lab> (raw)
In-Reply-To: <20101012124938.47170457@jbarnes-desktop>

On Tue, 12 Oct 2010 12:49:38 -0700
Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> On Tue, 12 Oct 2010 21:01:17 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Tuesday, October 12, 2010, Jesse Barnes wrote:
> > > On Mon, 11 Oct 2010 15:48:26 -0700
> > > Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > 
> > > > On Fri, 8 Oct 2010 21:46:50 +1000
> > > > Dave Airlie <airlied@gmail.com> wrote:
> > > > > Not sure how best to fix, I can workaround by calling
> > > > > pci_set_power_state(PCI_D0) in the drm drivers, but I sorta thing the
> > > > > PCI layer should take care of this.
> > > > 
> > > > So I think we *should* be able to call pci_disable_device at remove
> > > > time.  But as you say, some platforms may not correctly re-route VGA
> > > > space to an existing device or disable it properly when we do that.
> > > > AFAICT x86 will be ok here though (seems to work ok locally too).
> > > 
> > > Just tested this some more, and I think it's the right thing to do in
> > > the KMS case at least.  When we load a KMS driver it takes over the gfx
> > > device and nothing can assume anything about VGA state unless using the
> > > VGA arbiter.  So calling pci_disable_device() in the shutdown path of a
> > > KMS driver shouldn't make things any worse, and will work around this
> > > issue.
> > > 
> > > Doing so in the non-KMS case violates some PC assumptions though, in
> > > that things like vgacon and the BIOS will assume VGA memory is still
> > > around, which on some platforms pci_disable_device() may affect (I only
> > > checked the x86 implementation).
> > > 
> > > > That said, it seems like we should update the current device state at
> > > > load time as well, once we've matched the driver it seems like there
> > > > should be no harm.
> > > > 
> > > > Rafael, what do you think?  Would having the correct power state at
> > > > load time cause any trouble with other PM code?  I know we've had
> > > > issues with setting it explicitly in the past...
> > > 
> > > So we should probably make pci_enable_device pick up the current state
> > > as well, instead of assuming it's unknown just because the enable count
> > > was non-zero (which as Dave points out, can be affected by sysfs writes
> > > too).
> > > 
> > > The only downside I can think of there is that if the device is already
> > > enabled, we generally have to assume another driver owns it, and who
> > > knows if the device is actually alive enough to read the current state
> > > from.  But I think we handle those errors ok too, so pulling it out
> > > should be safe.
> > 
> > I remember trying to do something like this and it didn't play well with the
> > initialization.  Still, I didn't do that in pci_enable_device(), so I can't say
> > for sure at the moment.  I _think_ it will be fine, though.
> 
> Seems to work ok for the buggy i915 reload case.  But looking at it
> again, I really don't like two things:
>   1) doing just the set_power_state seems wrong, what about the rest of
>   enable?
>   2) allowing nested enables at all
> 
> I know sysfs currently allows us to bump the enable count to arbitrary
> levels, but that's easy to fix; we can just check pci_is_enabled()
> before calling enable_device in the sysfs store routine.
> 
> If we did that, we could warn when pci_enable_device is called in a
> nested way, which is generally a bug (two drivers trying to take over a
> device?).
> 
> I don't think I buy that VGA is special anyway, at least not for KMS
> enabled kernels, where vgacon and the BIOS can't assume anything about
> graphics state anymore.  More generally, I don't think BIOSes have been
> able to assume anything about the current graphics state since Windows
> 3.1, when most platforms stopped using BIOS calls and/or VGA regs for
> mode setting.
> 
> Thoughts?
> 

does this need to go to 2.6.36.y? (is it already on it's way?)

commit 97c145f7c87453cec90e91238fba5fe2c1561b32
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri Nov 5 15:16:36 2010 -0400

    PCI: read current power state at enable time


  reply	other threads:[~2010-11-16 19:47 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-07  8:55 "do_IRQ: 0.89 No irq handler for vector (irq -1)" Dave Airlie
2010-10-07  9:05 ` Dave Airlie
2010-10-07 20:38 ` Thomas Gleixner
2010-10-07 21:35   ` Thomas Gleixner
2010-10-08  7:52     ` Thomas Gleixner
2010-10-08 11:46       ` Dave Airlie
2010-10-11 22:48         ` Jesse Barnes
2010-10-11 23:40           ` Jesse Barnes
2010-10-12 19:01             ` Rafael J. Wysocki
2010-10-12 19:49               ` Jesse Barnes
2010-11-16 19:46                 ` Florian Mickler [this message]
2010-11-16 20:11                   ` Jesse Barnes
2010-11-16 20:47                     ` Florian Mickler
2010-10-13 21:20               ` Jesse Barnes
2010-10-13 21:48                 ` Rafael J. Wysocki
2010-10-08 11:53     ` Chris Wilson
2010-10-08 12:15       ` Thomas Gleixner
2010-10-08 12:30         ` Chris Wilson
2010-10-10 17:46 ` Maciej Rutecki

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=20101116204645.2c105e9b@schatten.dmk.lab \
    --to=florian@mickler.org \
    --cc=airlied@gmail.com \
    --cc=jbarnes@virtuousgeek.org \
    --cc=keithp@keithp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rjw@sisk.pl \
    --cc=tglx@linutronix.de \
    /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.