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

Aaron Ma aaron.ma at canonical.com
Thu Aug 25 01:38:34 UTC 2022



On 8/25/22 09:36, Jeremy Szu wrote:
> On Thu, Aug 25, 2022 at 9:13 AM Aaron Ma <aaron.ma at canonical.com> wrote:
>>
>> On 8/24/22 19:13, 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/
>>>
>>> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao at 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 at lists.ubuntu.com
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
>




More information about the kernel-team mailing list