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

[Kai-Heng Feng]

On Wed, Aug 24, 2022 at 7:14 PM AceLan Kao <acelan.kao at canonical.com> 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"),
> +               },
> +       },

I don't think the mass production SKUs have "Stealth" in their DMI.
Please ask Lenovo for proper DMI strings.

> +       {
> +               .ident = "Dell Inc. Precision 7960 Tower",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +                       DMI_MATCH(DMI_PRODUCT_NAME, "Precision 7960 Tower"),
> +               },
> +       },
> +};

This isn't quite right, a null terminator is missing.

Kai-Heng

> +
>  /**
>   * 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);
>  }
> --
> 2.25.1
>
>
> --
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team