linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Sean V Kelley <sean.v.kelley@intel.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
	Kuppuswamy Sathyanarayanan 
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	Keith Busch <kbusch@kernel.org>,
	"open list:PCI SUBSYSTEM" <linux-pci@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: [PATCH] PCI/portdrv: Disallow runtime suspend when waekup is required but PME service isn't supported
Date: Tue, 10 Aug 2021 18:21:44 +0200	[thread overview]
Message-ID: <20210810162144.GA24713@wunner.de> (raw)
In-Reply-To: <CAAd53p7qm=K99xO1n0Pwmn020Q7_iDj2S6-QGjeRjP0CpSphTg@mail.gmail.com>

On Tue, Aug 10, 2021 at 11:37:12PM +0800, Kai-Heng Feng wrote:
> On Mon, Aug 9, 2021 at 11:00 PM Lukas Wunner <lukas@wunner.de> wrote:
> > If PME is not granted to the OS, the only consequence is that the PME
> > port service is not instantiated at the root port.  But PME is still
> > enabled for downstream devices.  Maybe that's a mistake?  I think the
> > ACPI spec is a little unclear what to do if PME control is *not* granted.
> > It only specifies what to do if PME control is *granted*:
> 
> So do you prefer to just disable runtime PM for the downstream device?

I honestly don't know.  I was just wondering whether it is okay
to enable PME on devices if control is not granted by the firmware.
The spec is fairly vague.  But I guess the idea is that enabling PME
on devices is correct, just handling the interrupts is done by firmware
instead of the OS.

In your case, the endpoint device claims it can signal PME from D3cold,
which is why we allow the root port above to runtime suspend to D3hot.
The lspci output you've attached to the bugzilla indicates that yes,
signaling PME in D3cold does work, but the PME interrupt is neither
handled by the OS (because it's not allowed to) nor by firmware.

So you would like to rely on PME polling instead, which only works if the
root port remains in D0.  Otherwise config space of the endpoint device
is inaccessible.

I think the proper solution is that firmware should handle the PME
interrupt.  You've said the vendor objects because they found PME
doesn't work reliably.  Well in that case the endpoint device shouldn't
indicate that it can signal PME, at least not from D3cold.  Perhaps
the vendor is able to change the endpoint device's config space so
that it doesn't claim to support PME?

If that doesn't work and thus a kernel patch is necessary, the next
question is whether changing core code is the right approach.

If you do want to change core code, I'd suggest modifying 
pci_dev_check_d3cold() so that it blocks runtime PM on upstream
bridges if PME is not handled natively AND firmware failed to enable
the PME interrupt at the root port.  The rationale is that upstream
bridges need to remain in D0 so that PME polling is possible.

An alternative would be a quirk for this specific laptop which clears
pdev->pme_support.

Thanks,

Lukas

  reply	other threads:[~2021-08-10 16:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-13  7:57 [PATCH 1/1] PCI: Coalesce host bridge contiguous apertures without sorting Kai-Heng Feng
2021-07-13  8:45 ` Guenter Roeck
2021-07-13  8:49   ` Kai-Heng Feng
2021-07-13  9:06     ` Guenter Roeck
2021-07-13 12:50 ` [PATCH v2] PCI: Reinstate "PCI: Coalesce host bridge contiguous apertures" Kai-Heng Feng
2021-09-29 21:25   ` Bjorn Helgaas
2021-11-17 14:12   ` Johannes Berg
2022-07-13  9:36     ` Geert Uytterhoeven
2021-08-09  4:24 ` [PATCH] PCI/portdrv: Disallow runtime suspend when waekup is required but PME service isn't supported Kai-Heng Feng
2021-08-09  9:47   ` Lukas Wunner
2021-08-09 10:40     ` Kai-Heng Feng
2021-08-09 15:00       ` Lukas Wunner
2021-08-10 15:37         ` Kai-Heng Feng
2021-08-10 16:21           ` Lukas Wunner [this message]
2021-08-11  5:06             ` Kai-Heng Feng
2021-08-11  7:11               ` Lukas Wunner
2021-08-12  5:20                 ` Kai-Heng Feng

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=20210810162144.GA24713@wunner.de \
    --to=lukas@wunner.de \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=bhelgaas@google.com \
    --cc=kai.heng.feng@canonical.com \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=qiuxu.zhuo@intel.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=sean.v.kelley@intel.com \
    /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;
as well as URLs for NNTP newsgroup(s).