diff mbox series

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

Message ID 20220824111357.4062191-3-acelan.kao@canonical.com
State New
Headers show
Series System freeze after resuming from suspend due to PCI ASPM settings | expand

Commit Message

AceLan Kao Aug. 24, 2022, 11:13 a.m. UTC 
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(-)

Comments

Kai-Heng Feng Aug. 24, 2022, 11:33 a.m. UTC | #1 
On Wed, Aug 24, 2022 at 7:14 PM AceLan Kao <acelan.kao@canonical.com> 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"),
> +               },
> +       },

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@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Aaron Ma Aug. 25, 2022, 1:12 a.m. UTC | #2 
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.
Could you please try this change?
-			DMI_MATCH(DMI_PRODUCT_NAME, "Stealth"),
-			DMI_MATCH(DMI_PRODUCT_SERIAL, "P620SITC1"),
+			DMI_MATCH(DMI_BIOS_VERSION, "S07K"),

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);
>   }
Jeremy Szu Aug. 25, 2022, 1:36 a.m. UTC | #3 
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"),
>
> 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
Aaron Ma Aug. 25, 2022, 1:38 a.m. UTC | #4 
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
>
>
Jeremy Szu Aug. 25, 2022, 1:51 a.m. UTC | #5 
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
> >
> >
>
diff mbox series

Patch

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"),
+		},
+	},
+	{
+		.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);
 }