diff mbox series

[V1] PCI/ASPM: Skip L1SS save/restore if not already enabled

Message ID 20230119094913.20536-1-vidyas@nvidia.com (mailing list archive)
State Superseded
Headers show
Series [V1] PCI/ASPM: Skip L1SS save/restore if not already enabled | expand

Commit Message

Vidya Sagar Jan. 19, 2023, 9:49 a.m. UTC
Skip save and restore of ASPM L1 Sub-States specific registers if they
are not already enabled in the system. This is to avoid issues observed
on certain platforms during restoration process, particularly when
restoring the L1SS registers contents.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216782
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 drivers/pci/pcie/aspm.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Comments

Kai-Heng Feng Jan. 19, 2023, 2:21 p.m. UTC | #1
Hi Vidya,

On Thu, Jan 19, 2023 at 5:49 PM Vidya Sagar <vidyas@nvidia.com> wrote:
>
> Skip save and restore of ASPM L1 Sub-States specific registers if they
> are not already enabled in the system. This is to avoid issues observed
> on certain platforms during restoration process, particularly when
> restoring the L1SS registers contents.
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216782
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>  drivers/pci/pcie/aspm.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 53a1fa306e1e..5d3f09b0a6a9 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -757,15 +757,29 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
>                                 PCI_L1SS_CTL1_L1SS_MASK, val);
>  }
>
> +static bool skip_l1ss_restore;

Maybe move it inside "struct pci_dev"?

Kai-Heng

> +
>  void pci_save_aspm_l1ss_state(struct pci_dev *dev)
>  {
>         struct pci_cap_saved_state *save_state;
>         u16 l1ss = dev->l1ss;
> -       u32 *cap;
> +       u32 *cap, val;
>
>         if (!l1ss)
>                 return;
>
> +       /*
> +        * Skip save and restore of L1 Sub-States registers if they are not
> +        * already enabled in the system
> +        */
> +       pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, &val);
> +       if (!(val & PCI_L1SS_CTL1_L1SS_MASK)) {
> +               skip_l1ss_restore = 1;
> +               return;
> +       }
> +
> +       skip_l1ss_restore = 0;
> +
>         save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
>         if (!save_state)
>                 return;
> @@ -784,6 +798,9 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *dev)
>         if (!l1ss)
>                 return;
>
> +       if (skip_l1ss_restore)
> +               return;
> +
>         save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
>         if (!save_state)
>                 return;
> --
> 2.17.1
>
Wysocki, Rafael J Jan. 19, 2023, 7:01 p.m. UTC | #2
On 1/19/2023 3:21 PM, Kai-Heng Feng wrote:
> Hi Vidya,
>
> On Thu, Jan 19, 2023 at 5:49 PM Vidya Sagar <vidyas@nvidia.com> wrote:
>> Skip save and restore of ASPM L1 Sub-States specific registers if they
>> are not already enabled in the system. This is to avoid issues observed
>> on certain platforms during restoration process, particularly when
>> restoring the L1SS registers contents.
>>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216782
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>>   drivers/pci/pcie/aspm.c | 19 ++++++++++++++++++-
>>   1 file changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 53a1fa306e1e..5d3f09b0a6a9 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -757,15 +757,29 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
>>                                  PCI_L1SS_CTL1_L1SS_MASK, val);
>>   }
>>
>> +static bool skip_l1ss_restore;
> Maybe move it inside "struct pci_dev"?

Yes, it can be different for different devices, so it cannot be static IMV.


>> +
>>   void pci_save_aspm_l1ss_state(struct pci_dev *dev)
>>   {
>>          struct pci_cap_saved_state *save_state;
>>          u16 l1ss = dev->l1ss;
>> -       u32 *cap;
>> +       u32 *cap, val;
>>
>>          if (!l1ss)
>>                  return;
>>
>> +       /*
>> +        * Skip save and restore of L1 Sub-States registers if they are not
>> +        * already enabled in the system
>> +        */
>> +       pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, &val);
>> +       if (!(val & PCI_L1SS_CTL1_L1SS_MASK)) {
>> +               skip_l1ss_restore = 1;
>> +               return;
>> +       }
>> +
>> +       skip_l1ss_restore = 0;
>> +
>>          save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
>>          if (!save_state)
>>                  return;
>> @@ -784,6 +798,9 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *dev)
>>          if (!l1ss)
>>                  return;
>>
>> +       if (skip_l1ss_restore)
>> +               return;
>> +
>>          save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
>>          if (!save_state)
>>                  return;
>> --
>> 2.17.1
>>
diff mbox series

Patch

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 53a1fa306e1e..5d3f09b0a6a9 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -757,15 +757,29 @@  static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
 				PCI_L1SS_CTL1_L1SS_MASK, val);
 }
 
+static bool skip_l1ss_restore;
+
 void pci_save_aspm_l1ss_state(struct pci_dev *dev)
 {
 	struct pci_cap_saved_state *save_state;
 	u16 l1ss = dev->l1ss;
-	u32 *cap;
+	u32 *cap, val;
 
 	if (!l1ss)
 		return;
 
+	/*
+	 * Skip save and restore of L1 Sub-States registers if they are not
+	 * already enabled in the system
+	 */
+	pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, &val);
+	if (!(val & PCI_L1SS_CTL1_L1SS_MASK)) {
+		skip_l1ss_restore = 1;
+		return;
+	}
+
+	skip_l1ss_restore = 0;
+
 	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
 	if (!save_state)
 		return;
@@ -784,6 +798,9 @@  void pci_restore_aspm_l1ss_state(struct pci_dev *dev)
 	if (!l1ss)
 		return;
 
+	if (skip_l1ss_restore)
+		return;
+
 	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_L1SS);
 	if (!save_state)
 		return;