2022-09-09 20:54:06

by Bjorn Helgaas

[permalink] [raw]
Subject: [PATCH v4 0/9] PCI/PM: Always disable PTM for all devices during suspend

From: Bjorn Helgaas <[email protected]>

We currently disable PTM for Root Ports during suspend. Leaving PTM
enabled for downstream devices causes UR errors if they send PTM Requests
to upstream devices that have PTM disabled.

The intent of this series is to:

- Unconditionally disable PTM during suspend (even if the driver saves
its own state) by moving the disable from pci_prepare_to_sleep() to
pci_pm_suspend().

- Disable PTM for all devices by removing the Root Port condition and
doing it early in the suspend paths.

- Explicitly re-enable PTM during resume.

Changes between v3 and v4:
- Use u16 for ptm_cap
- Add kernel-doc for pci_enable_ptm() and pci_disable_ptm() (exported
functions)
- Drop "Preserve PTM Root Select" (unnecessary since enabling PTM sets
Root Select when needed)
- Squash these three patches into one because they make more sense that
way:
PCI/PTM: Add suspend/resume
PCI/PTM: Add pci_enable_ptm() wrapper
PCI/PTM: Add pci_disable_ptm() wrapper
- Add "PCI/PTM: Preserve RsvdP bits in PTM Control register"
- Add "PCI/PTM: Consolidate PTM interface declarations"

Bjorn Helgaas (9):
PCI/PTM: Cache PTM Capability offset
PCI/PTM: Add pci_upstream_ptm() helper
PCI/PTM: Separate configuration and enable
PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm()
PCI/PTM: Move pci_ptm_info() body into its only caller
PCI/PTM: Preserve RsvdP bits in PTM Control register
PCI/PTM: Reorder functions in logical order
PCI/PTM: Consolidate PTM interface declarations
PCI/PM: Always disable PTM for all devices during suspend

drivers/pci/pci-driver.c | 11 ++
drivers/pci/pci.c | 28 +--
drivers/pci/pci.h | 14 +-
drivers/pci/pcie/ptm.c | 384 +++++++++++++++++++++------------------
include/linux/pci.h | 3 +
5 files changed, 234 insertions(+), 206 deletions(-)

--
2.25.1


2022-09-12 04:33:53

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v4 0/9] PCI/PM: Always disable PTM for all devices during suspend

On Fri, Sep 09, 2022 at 03:24:56PM -0500, Bjorn Helgaas wrote:
> Bjorn Helgaas (9):
> PCI/PTM: Cache PTM Capability offset
> PCI/PTM: Add pci_upstream_ptm() helper
> PCI/PTM: Separate configuration and enable
> PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm()
> PCI/PTM: Move pci_ptm_info() body into its only caller
> PCI/PTM: Preserve RsvdP bits in PTM Control register
> PCI/PTM: Reorder functions in logical order
> PCI/PTM: Consolidate PTM interface declarations
> PCI/PM: Always disable PTM for all devices during suspend

For the whole series,

Reviewed-by: Mika Westerberg <[email protected]>

2022-09-12 21:08:41

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 0/9] PCI/PM: Always disable PTM for all devices during suspend

On Mon, Sep 12, 2022 at 01:15:59PM -0700, Rajvi Jingar wrote:
> On 9/9/22 13:24, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas<[email protected]>
> >
> > We currently disable PTM for Root Ports during suspend. Leaving PTM
> > enabled for downstream devices causes UR errors if they send PTM Requests
> > to upstream devices that have PTM disabled.
> >
> > The intent of this series is to:
> >
> > - Unconditionally disable PTM during suspend (even if the driver saves
> > its own state) by moving the disable from pci_prepare_to_sleep() to
> > pci_pm_suspend().
> >
> > - Disable PTM for all devices by removing the Root Port condition and
> > doing it early in the suspend paths.
> >
> > - Explicitly re-enable PTM during resume.
> >
> > Changes between v3 and v4:
> > - Use u16 for ptm_cap
> > - Add kernel-doc for pci_enable_ptm() and pci_disable_ptm() (exported
> > functions)
> > - Drop "Preserve PTM Root Select" (unnecessary since enabling PTM sets
> > Root Select when needed)
> > - Squash these three patches into one because they make more sense that
> > way:
> > PCI/PTM: Add suspend/resume
> > PCI/PTM: Add pci_enable_ptm() wrapper
> > PCI/PTM: Add pci_disable_ptm() wrapper
> > - Add "PCI/PTM: Preserve RsvdP bits in PTM Control register"
> > - Add "PCI/PTM: Consolidate PTM interface declarations"
> >
> > Bjorn Helgaas (9):
> > PCI/PTM: Cache PTM Capability offset
> > PCI/PTM: Add pci_upstream_ptm() helper
> > PCI/PTM: Separate configuration and enable
> > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm()
> > PCI/PTM: Move pci_ptm_info() body into its only caller
> > PCI/PTM: Preserve RsvdP bits in PTM Control register
> > PCI/PTM: Reorder functions in logical order
> > PCI/PTM: Consolidate PTM interface declarations
> > PCI/PM: Always disable PTM for all devices during suspend
>
> Looks good. It fixes the issue on Dell Precision 5530 with Coffee Lake
> CPU platforms to get improved residency in low power idle states.
>
> Tested-by:[email protected]

Thanks a lot for testing this!

It's best if you can send plain-text mail to the lists. This was a
multi-part message, which the vger mailing lists reject:

http://vger.kernel.org/majordomo-info.html

Bjorn

2022-09-12 21:28:29

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH v4 0/9] PCI/PM: Always disable PTM for all devices during suspend

On Mon, Sep 12, 2022 at 07:16:23AM +0300, Mika Westerberg wrote:
> On Fri, Sep 09, 2022 at 03:24:56PM -0500, Bjorn Helgaas wrote:
> > Bjorn Helgaas (9):
> > PCI/PTM: Cache PTM Capability offset
> > PCI/PTM: Add pci_upstream_ptm() helper
> > PCI/PTM: Separate configuration and enable
> > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm()
> > PCI/PTM: Move pci_ptm_info() body into its only caller
> > PCI/PTM: Preserve RsvdP bits in PTM Control register
> > PCI/PTM: Reorder functions in logical order
> > PCI/PTM: Consolidate PTM interface declarations
> > PCI/PM: Always disable PTM for all devices during suspend
>
> For the whole series,
>
> Reviewed-by: Mika Westerberg <[email protected]>

Thank you very much, Mika, Kuppuswamy, and Rajvi for your effort in
reviewing and testing these. I know that's a lot of work, and I
really appreciate it.

I put these on pci/pm together with Rajvi's pci_pm_suspend_noirq()
simplification for v6.1.

Bjorn

2022-09-13 08:26:19

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v4 0/9] PCI/PM: Always disable PTM for all devices during suspend

On Mon, Sep 12, 2022 at 10:36 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Mon, Sep 12, 2022 at 07:16:23AM +0300, Mika Westerberg wrote:
> > On Fri, Sep 09, 2022 at 03:24:56PM -0500, Bjorn Helgaas wrote:
> > > Bjorn Helgaas (9):
> > > PCI/PTM: Cache PTM Capability offset
> > > PCI/PTM: Add pci_upstream_ptm() helper
> > > PCI/PTM: Separate configuration and enable
> > > PCI/PTM: Add pci_suspend_ptm() and pci_resume_ptm()
> > > PCI/PTM: Move pci_ptm_info() body into its only caller
> > > PCI/PTM: Preserve RsvdP bits in PTM Control register
> > > PCI/PTM: Reorder functions in logical order
> > > PCI/PTM: Consolidate PTM interface declarations
> > > PCI/PM: Always disable PTM for all devices during suspend
> >
> > For the whole series,
> >
> > Reviewed-by: Mika Westerberg <[email protected]>
>
> Thank you very much, Mika, Kuppuswamy, and Rajvi for your effort in
> reviewing and testing these. I know that's a lot of work, and I
> really appreciate it.
>
> I put these on pci/pm together with Rajvi's pci_pm_suspend_noirq()
> simplification for v6.1.

Awesome, thanks!