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

[Jeremy Szu]

got it, thanks for sharing!

On Thu, Aug 25, 2022 at 9:38 AM Aaron Ma <aaron.ma於canonical.com> wrote:
>
>
>
> On 8/25/22 09:36, Jeremy Szu wrote:
> > On Thu, Aug 25, 2022 at 9:13 AM Aaron Ma <aaron.ma於canonical.com> wrote:
> >>
> >> On 8/24/22 19:13, AceLan Kao wrote:
> >>> From: "Chia-Lin Kao (AceLan)" <acelan.kao於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/
> >>>
> >>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao於canonical.com>
> >>> ---
> >>>    drivers/pci/pci.c | 36 +++++++++++++++++++++++++++++-------
> >>>    1 file changed, 29 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> >>> index 5c7ef86db60b..2012e9f16e23 100644
> >>> --- a/drivers/pci/pci.c
> >>> +++ b/drivers/pci/pci.c
> >>> @@ -166,6 +166,24 @@ __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_SYS_VENDOR, "LENOVO"),
> >>> +                     DMI_MATCH(DMI_PRODUCT_NAME, "Stealth"),
> >>> +                     DMI_MATCH(DMI_PRODUCT_SERIAL, "P620SITC1"),
> >> This Product Name/Serial are not for product level.
> >> Usually BIOS version can be used to identify the system.
> > Yes, it's the same as what I know from Lenovo.
> > However, Lenovo only commit the first three characters for identifying
> > a platform.
> >> Could you please try this change?
> >> -                       DMI_MATCH(DMI_PRODUCT_NAME, "Stealth"),
> >> -                       DMI_MATCH(DMI_PRODUCT_SERIAL, "P620SITC1"),
> >> +                       DMI_MATCH(DMI_BIOS_VERSION, "S07K"),
> > Thus, it should be:
> > +                       DMI_MATCH(DMI_BIOS_VERSION, "S07"),
>
> S07 is a product, K means specific BIOS.
> I think S07K is more safe.
>
> Aaron
>
> >> Aaron
> >>
> >>
> >>> +             },
> >>> +     },
> >>> +     {
> >>> +             .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 +1161,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 +1641,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 +1749,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 +3457,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);
> >>>    }
> >> --
> >> kernel-team mailing list
> >> kernel-team於lists.ubuntu.com
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >
> >
>


-- 
Sincerely,
Jeremy Su