| Message ID | 20240223205851.114931-1-helgaas@kernel.org |
|---|---|
| Headers | show |
| Series | PCI/ASPM: Save/restore L1 PM Substates for suspend/resume | expand |
On Fri, Feb 23, 2024 at 02:58:46PM -0600, Bjorn Helgaas wrote: > From: Bjorn Helgaas <bhelgaas@google.com> > > This is some rework of David's series to preserve ASPM L1 substate > configuration across suspend/resume. > > We've had several attempts to make this work: > > (unlabeled): https://lore.kernel.org/r/20240128233212.1139663-1-david.e.box@linux.intel.com > v5: https://lore.kernel.org/r/20231221011250.191599-1-david.e.box@linux.intel.com > v4: https://lore.kernel.org/all/20231002070044.2299644-1-mika.westerberg@linux.intel.com/ > v3: https://lore.kernel.org/linux-pci/20230925074636.2893747-1-mika.westerberg@linux.intel.com/ > v2: https://lore.kernel.org/linux-pci/20230911073352.3472918-1-mika.westerberg@linux.intel.com/ > v1: https://lore.kernel.org/linux-pci/20230627062442.54008-1-mika.westerberg@linux.intel.com/ > > The most recent posting is the unlabeled one mentioned above, and I'm > calling it v6 and this rework v7. > > Changes since the unlabeled v6: > > - Rename pci_save_aspm_state() to pci_save_aspm_l1ss_state() (this > is the reason for opening this again, because Vidya's patch [1] > had to do some incidental renaming). > > - Rename pcie_restore_aspm_l1ss() to pci_restore_aspm_l1ss_state() > to match. > > - Move the PCI_EXP_LNKCTL_ASPMC from pci_restore_aspm_state() to > pci_restore_pcie_state() so both writes are in the same place. > > - Rename pci_aspm_get_l1ss() to pci_configure_aspm_l1ss() and add > the save_buffer there as well. > > - Split [1/5] into two patches: move pci_configure_ltr() and > pci_bridge_reconfigure_ltr() to aspm.c, and build aspm.c > unconditionally. > > - Squash [2/5] and [3/5] since [2/5] didn't add any functionality > itself so they seem like a single logical change. > > [1] https://lore.kernel.org/r/20230125133830.20620-1-vidyas@nvidia.com > > David E. Box (5): > PCI/ASPM: Move pci_configure_ltr() to aspm.c > PCI/ASPM: Always build aspm.c > PCI/ASPM: Move pci_save_ltr_state() to aspm.c > PCI/ASPM: Save L1 PM Substates Capability for suspend/resume > PCI/ASPM: Call pci_save_ltr_state() from pci_save_pcie_state() > > drivers/pci/pci.c | 89 ++++------------ > drivers/pci/pci.h | 13 ++- > drivers/pci/pcie/Makefile | 2 +- > drivers/pci/pcie/aspm.c | 215 ++++++++++++++++++++++++++++++++++++++ > drivers/pci/probe.c | 62 +---------- > include/linux/pci.h | 2 +- > 6 files changed, 252 insertions(+), 131 deletions(-) I applied these as pci/aspm for v6.9, replacing the original unlabeled v6 that has been in -next. I also added the patch I suggested at https://lore.kernel.org/r/20240223213733.GA115410@bhelgaas to disable L1 inside pci_restore_aspm_l1ss_state() where we actually depend on it being disabled. Bjorn
[+cc Koba Ko] On Tue, Mar 05, 2024 at 03:46:56PM -0600, Bjorn Helgaas wrote: > On Fri, Feb 23, 2024 at 02:58:46PM -0600, Bjorn Helgaas wrote: > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > This is some rework of David's series to preserve ASPM L1 substate > > configuration across suspend/resume. > > ... > > David E. Box (5): > > PCI/ASPM: Move pci_configure_ltr() to aspm.c > > PCI/ASPM: Always build aspm.c > > PCI/ASPM: Move pci_save_ltr_state() to aspm.c > > PCI/ASPM: Save L1 PM Substates Capability for suspend/resume > > PCI/ASPM: Call pci_save_ltr_state() from pci_save_pcie_state() > > > > drivers/pci/pci.c | 89 ++++------------ > > drivers/pci/pci.h | 13 ++- > > drivers/pci/pcie/Makefile | 2 +- > > drivers/pci/pcie/aspm.c | 215 ++++++++++++++++++++++++++++++++++++++ > > drivers/pci/probe.c | 62 +---------- > > include/linux/pci.h | 2 +- > > 6 files changed, 252 insertions(+), 131 deletions(-) > > I applied these as pci/aspm for v6.9, replacing the original unlabeled > v6 that has been in -next. Would anybody be able to test this, particularly to make sure it works for the bugs we're claiming to fix with this series? https://bugzilla.kernel.org/show_bug.cgi?id=217321 https://bugzilla.kernel.org/show_bug.cgi?id=216782 https://bugzilla.kernel.org/show_bug.cgi?id=216877 This series is headed for v6.9, and I hope we can finally claim victory over these issues. This is in -next as of the Mar 7 tree. Or if you want just the ASPM changes, based on v6.8-rc1, you can use the branch at https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm Bjorn
Le 7/03/24 à 23:25, Bjorn Helgaas a écrit : > [+cc Koba Ko] > > On Tue, Mar 05, 2024 at 03:46:56PM -0600, Bjorn Helgaas wrote: >> On Fri, Feb 23, 2024 at 02:58:46PM -0600, Bjorn Helgaas wrote: >>> From: Bjorn Helgaas <bhelgaas@google.com> >>> >>> This is some rework of David's series to preserve ASPM L1 substate >>> configuration across suspend/resume. >>> ... >>> David E. Box (5): >>> PCI/ASPM: Move pci_configure_ltr() to aspm.c >>> PCI/ASPM: Always build aspm.c >>> PCI/ASPM: Move pci_save_ltr_state() to aspm.c >>> PCI/ASPM: Save L1 PM Substates Capability for suspend/resume >>> PCI/ASPM: Call pci_save_ltr_state() from pci_save_pcie_state() >>> >>> drivers/pci/pci.c | 89 ++++------------ >>> drivers/pci/pci.h | 13 ++- >>> drivers/pci/pcie/Makefile | 2 +- >>> drivers/pci/pcie/aspm.c | 215 ++++++++++++++++++++++++++++++++++++++ >>> drivers/pci/probe.c | 62 +---------- >>> include/linux/pci.h | 2 +- >>> 6 files changed, 252 insertions(+), 131 deletions(-) >> I applied these as pci/aspm for v6.9, replacing the original unlabeled >> v6 that has been in -next. > Would anybody be able to test this, particularly to make sure it works > for the bugs we're claiming to fix with this series? > > https://bugzilla.kernel.org/show_bug.cgi?id=217321 > https://bugzilla.kernel.org/show_bug.cgi?id=216782 > https://bugzilla.kernel.org/show_bug.cgi?id=216877 > > This series is headed for v6.9, and I hope we can finally claim > victory over these issues. > > This is in -next as of the Mar 7 tree. Or if you want just the ASPM > changes, based on v6.8-rc1, you can use the branch at > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm > > Bjorn I just tested the patch v7 from Bjorn, it works on my Asus UX305FA. Tested on kernel v6.8-rc1. Tasev
On Tue, Mar 12, 2024 at 06:03:21PM +0100, tasev.stefanoska wrote: > Le 7/03/24 à 23:25, Bjorn Helgaas a écrit : > > On Tue, Mar 05, 2024 at 03:46:56PM -0600, Bjorn Helgaas wrote: > > > On Fri, Feb 23, 2024 at 02:58:46PM -0600, Bjorn Helgaas wrote: > > > > From: Bjorn Helgaas <bhelgaas@google.com> > > > > > > > > This is some rework of David's series to preserve ASPM L1 substate > > > > configuration across suspend/resume. > > > > ... > > > > David E. Box (5): > > > > PCI/ASPM: Move pci_configure_ltr() to aspm.c > > > > PCI/ASPM: Always build aspm.c > > > > PCI/ASPM: Move pci_save_ltr_state() to aspm.c > > > > PCI/ASPM: Save L1 PM Substates Capability for suspend/resume > > > > PCI/ASPM: Call pci_save_ltr_state() from pci_save_pcie_state() > > > > > > > > drivers/pci/pci.c | 89 ++++------------ > > > > drivers/pci/pci.h | 13 ++- > > > > drivers/pci/pcie/Makefile | 2 +- > > > > drivers/pci/pcie/aspm.c | 215 ++++++++++++++++++++++++++++++++++++++ > > > > drivers/pci/probe.c | 62 +---------- > > > > include/linux/pci.h | 2 +- > > > > 6 files changed, 252 insertions(+), 131 deletions(-) > > > > > > I applied these as pci/aspm for v6.9, replacing the original unlabeled > > > v6 that has been in -next. > > > > Would anybody be able to test this, particularly to make sure it works > > for the bugs we're claiming to fix with this series? > > > > https://bugzilla.kernel.org/show_bug.cgi?id=217321 > > https://bugzilla.kernel.org/show_bug.cgi?id=216782 > > https://bugzilla.kernel.org/show_bug.cgi?id=216877 > > > > This series is headed for v6.9, and I hope we can finally claim > > victory over these issues. > > > > This is in -next as of the Mar 7 tree. Or if you want just the ASPM > > changes, based on v6.8-rc1, you can use the branch at > > https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=aspm > > I just tested the patch v7 from Bjorn, it works on my Asus UX305FA. > Tested on kernel v6.8-rc1. Thank you very much! I added the following to the "PCI/ASPM: Save L1 PM Substates Capability for suspend/resume" patch: Tested-by: Tasev Nikola <tasev.stefanoska@skynet.be> # Asus UX305FA
From: Bjorn Helgaas <bhelgaas@google.com> This is some rework of David's series to preserve ASPM L1 substate configuration across suspend/resume. We've had several attempts to make this work: (unlabeled): https://lore.kernel.org/r/20240128233212.1139663-1-david.e.box@linux.intel.com v5: https://lore.kernel.org/r/20231221011250.191599-1-david.e.box@linux.intel.com v4: https://lore.kernel.org/all/20231002070044.2299644-1-mika.westerberg@linux.intel.com/ v3: https://lore.kernel.org/linux-pci/20230925074636.2893747-1-mika.westerberg@linux.intel.com/ v2: https://lore.kernel.org/linux-pci/20230911073352.3472918-1-mika.westerberg@linux.intel.com/ v1: https://lore.kernel.org/linux-pci/20230627062442.54008-1-mika.westerberg@linux.intel.com/ The most recent posting is the unlabeled one mentioned above, and I'm calling it v6 and this rework v7. Changes since the unlabeled v6: - Rename pci_save_aspm_state() to pci_save_aspm_l1ss_state() (this is the reason for opening this again, because Vidya's patch [1] had to do some incidental renaming). - Rename pcie_restore_aspm_l1ss() to pci_restore_aspm_l1ss_state() to match. - Move the PCI_EXP_LNKCTL_ASPMC from pci_restore_aspm_state() to pci_restore_pcie_state() so both writes are in the same place. - Rename pci_aspm_get_l1ss() to pci_configure_aspm_l1ss() and add the save_buffer there as well. - Split [1/5] into two patches: move pci_configure_ltr() and pci_bridge_reconfigure_ltr() to aspm.c, and build aspm.c unconditionally. - Squash [2/5] and [3/5] since [2/5] didn't add any functionality itself so they seem like a single logical change. [1] https://lore.kernel.org/r/20230125133830.20620-1-vidyas@nvidia.com David E. Box (5): PCI/ASPM: Move pci_configure_ltr() to aspm.c PCI/ASPM: Always build aspm.c PCI/ASPM: Move pci_save_ltr_state() to aspm.c PCI/ASPM: Save L1 PM Substates Capability for suspend/resume PCI/ASPM: Call pci_save_ltr_state() from pci_save_pcie_state() drivers/pci/pci.c | 89 ++++------------ drivers/pci/pci.h | 13 ++- drivers/pci/pcie/Makefile | 2 +- drivers/pci/pcie/aspm.c | 215 ++++++++++++++++++++++++++++++++++++++ drivers/pci/probe.c | 62 +---------- include/linux/pci.h | 2 +- 6 files changed, 252 insertions(+), 131 deletions(-)