[PATCH v2 2/2][SRU][J] UBUNTU: SAUCE: whitelist platforms that needs save/restore ASPM L1SS for suspend/resume

[AceLan Kao]

Stefan Bader <stefan.bader at canonical.com> 於 2022年8月25日 週四 下午4:15寫道：
>
> On 25.08.22 03:56, AceLan Kao wrote:
> > From: "Chia-Lin Kao (AceLan)" <acelan.kao at canonical.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1980829
> >
> > Add a DMI quirk for the previous commit
> > "PCI/ASPM: Save/restore L1SS Capability for suspend/resume"
> > The DMI quirk lists the platforms that needs this patch, and also
> > applied the concept of the below commit to not call
> > pcie_aspm_pm_state_change() if the platform is listed in the whitelist
> > https://patchwork.ozlabs.org/project/linux-pci/patch/20220509073639.2048236-1-kai.heng.feng@canonical.com/
>
> To limit the save/restore calls makes sense. However it is hard to say whether
> limiting the state change as well is good or bad. At least it feels like it
> might change behavior for other platforms... Why was this done?
Limit the state change is from this commit
https://patchwork.ozlabs.org/project/linux-pci/patch/20220509073639.2048236-1-kai.heng.feng@canonical.com/
We have applied this commit on the OEM kernels, so I want to apply
this on Jammy 5.15, too.
But I can't apply this commit, because it removes the function, so I
apply the dmi check on the if statement to archive the same thing.

>
> -Stefan
>
> >
> > v2.
> >     1. added the missing null terminator at the end of the quirk
> >     2. changed the DMI match for LENOVO to its DMI_BIOS_VERSION
> >
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao at canonical.com>
> > ---
> >   drivers/pci/pci.c | 35 ++++++++++++++++++++++++++++-------
> >   1 file changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 5c7ef86db60b..d8b71a7b8cd0 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -166,6 +166,23 @@ __setup("pcie_port_pm=", pcie_port_pm_setup);
> >   /* Time to wait after a reset for device to become responsive */
> >   #define PCIE_RESET_READY_POLL_MS 60000
> >
> > +static const struct dmi_system_id aspm_fix_whitelist[] = {
> > +     {
> > +             .ident = "LENOVO Stealth Thinkstation",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_BIOS_VERSION, "S07K"),
> > +             },
> > +     },
> > +     {
> > +             .ident = "Dell Inc. Precision 7960 Tower",
> > +             .matches = {
> > +                     DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> > +                     DMI_MATCH(DMI_PRODUCT_NAME, "Precision 7960 Tower"),
> > +             },
> > +     },
> > +     {}
> > +};
> > +
> >   /**
> >    * pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
> >    * @bus: pointer to PCI bus structure to search
> > @@ -1143,7 +1160,7 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state)
> >       if (need_restore)
> >               pci_restore_bars(dev);
> >
> > -     if (dev->bus->self)
> > +     if (dev->bus->self && !dmi_check_system(aspm_fix_whitelist))
> >               pcie_aspm_pm_state_change(dev->bus->self);
> >
> >       return 0;
> > @@ -1623,7 +1640,8 @@ int pci_save_state(struct pci_dev *dev)
> >               return i;
> >
> >       pci_save_ltr_state(dev);
> > -     pci_save_aspm_l1ss_state(dev);
> > +     if (dmi_check_system(aspm_fix_whitelist))
> > +             pci_save_aspm_l1ss_state(dev);
> >       pci_save_dpc_state(dev);
> >       pci_save_aer_state(dev);
> >       pci_save_ptm_state(dev);
> > @@ -1730,7 +1748,8 @@ void pci_restore_state(struct pci_dev *dev)
> >        * LTR itself (in the PCIe capability).
> >        */
> >       pci_restore_ltr_state(dev);
> > -     pci_restore_aspm_l1ss_state(dev);
> > +     if (dmi_check_system(aspm_fix_whitelist))
> > +             pci_restore_aspm_l1ss_state(dev);
> >
> >       pci_restore_pcie_state(dev);
> >       pci_restore_pasid_state(dev);
> > @@ -3437,10 +3456,12 @@ void pci_allocate_cap_save_buffers(struct pci_dev *dev)
> >       if (error)
> >               pci_err(dev, "unable to allocate suspend buffer for LTR\n");
> >
> > -     error = pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_L1SS,
> > -                                         2 * sizeof(u32));
> > -     if (error)
> > -             pci_err(dev, "unable to allocate suspend buffer for ASPM-L1SS\n");
> > +     if (dmi_check_system(aspm_fix_whitelist)) {
> > +             error = pci_add_ext_cap_save_buffer(dev, PCI_EXT_CAP_ID_L1SS,
> > +                                                 2 * sizeof(u32));
> > +             if (error)
> > +                     pci_err(dev, "unable to allocate suspend buffer for ASPM-L1SS\n");
> > +     }
> >
> >       pci_allocate_vc_save_buffers(dev);
> >   }
>