| Message ID | 20241120061704.636295-2-en-wei.wu@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | System hang when running memory/memory_stress_ng test | expand |
On Wed, Nov 20, 2024 at 02:17:02PM +0800, En-Wei Wu wrote: > From: Jian-Hong Pan <jhp@endlessos.org> > > BugLink: https://bugs.launchpad.net/bugs/2085092 > > The remapped PCIe Root Port and the child device have PM L1 Substates > capability, but they are disabled originally. > > Here is a failed example on ASUS B1400CEAE: > > Capabilities: [900 v1] L1 PM Substates > L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- L1_PM_Substates+ > PortCommonModeRestoreTime=32us PortTPowerOnTime=10us > L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- > T_CommonMode=0us LTR1.2_Threshold=101376ns > L1SubCtl2: T_PwrOn=50us > > Enable PCI-PM L1 PM Substates for devices below VMD while they are in D0 > (see PCIe r6.0, sec 5.5.4). > > Link: https://lore.kernel.org/r/20241001083438.10070-4-jhp@endlessos.org > Link: https://bugzilla.kernel.org/show_bug.cgi?id=218394 > Signed-off-by: Jian-Hong Pan <jhp@endlessos.org> > Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > (backported from commit d66041063192497a4a97d21dbf86b79a03a7f4fb linux-next) > [En-Wei: We have a Kai-Heng's ASPM SAUCE patch so need to modify the code] > Signed-off-by: En-Wei Wu <en-wei.wu@canonical.com> > --- > drivers/pci/controller/vmd.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index e33d213a34fb..e5a0c6c57524 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -757,7 +757,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); The upstream patch moves the `pci_enable_link_state_locked` call to after out_state_change, though this backport leaves in the original call to `pci_enable_link_state_locked` and adds a new one after out_state_change. Since the purpose of this change is to prevent L1 link states being enabled before the device is in D0, wouldn't it be correct to match the original patch and only keep the new call to `pci_enable_link_state_locked`? > pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR); > if (!pos) > - return 0; > + goto out_state_change; > > /* > * Skip if the max snoop LTR is non-zero, indicating BIOS has set it > @@ -765,7 +765,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > */ > pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, <r_reg); > if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK))) > - return 0; > + goto out_state_change; > > /* > * Set the default values to the maximum required by the platform to > @@ -777,6 +777,13 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg); > pci_info(pdev, "VMD: Default LTR value set by driver\n"); > > +out_state_change: > + /* > + * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per > + * PCIe r6.0, sec 5.5.4. > + */ > + pci_set_power_state_locked(pdev, PCI_D0); > + pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); > return 0; > } > > -- > 2.43.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index e33d213a34fb..e5a0c6c57524 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -757,7 +757,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR); if (!pos) - return 0; + goto out_state_change; /* * Skip if the max snoop LTR is non-zero, indicating BIOS has set it @@ -765,7 +765,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) */ pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, <r_reg); if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK))) - return 0; + goto out_state_change; /* * Set the default values to the maximum required by the platform to @@ -777,6 +777,13 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg); pci_info(pdev, "VMD: Default LTR value set by driver\n"); +out_state_change: + /* + * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per + * PCIe r6.0, sec 5.5.4. + */ + pci_set_power_state_locked(pdev, PCI_D0); + pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); return 0; }