2020-12-08 08:29:33

by Kai-Heng Feng

Subject: [PATCH 2/2] PCI/ASPM: Use capability to override ASPM via sysfs

If we are to use sysfs to change ASPM settings, we may want to override
the default ASPM policy.

So use ASPM capability, instead of default policy, to be able to use all
possible ASPM states.

Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/pci/pcie/aspm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 2ea9fddadfad..326da7bbc84d 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1239,8 +1239,7 @@ static ssize_t aspm_attr_store_common(struct device *dev,

link->aspm_disable |= state;
}
-
- pcie_config_aspm_link(link, policy_to_aspm_state(link));
+ pcie_config_aspm_link(link, link->aspm_capable);

mutex_unlock(&aspm_lock);
up_read(&pci_bus_sem);
--
2.29.2

2020-12-08 17:21:35

by Heiner Kallweit

Subject: Re: [PATCH 2/2] PCI/ASPM: Use capability to override ASPM via sysfs

Am 08.12.2020 um 09:25 schrieb Kai-Heng Feng:
> If we are to use sysfs to change ASPM settings, we may want to override
> the default ASPM policy.
>
> So use ASPM capability, instead of default policy, to be able to use all
> possible ASPM states.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> drivers/pci/pcie/aspm.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 2ea9fddadfad..326da7bbc84d 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1239,8 +1239,7 @@ static ssize_t aspm_attr_store_common(struct device *dev,
>
> link->aspm_disable |= state;
> }
> -
> - pcie_config_aspm_link(link, policy_to_aspm_state(link));
> + pcie_config_aspm_link(link, link->aspm_capable);
>
I like the idea, because the policy can be changed by the user anyway.
Therefore I don't see it as a hard system limit.

However I think this change is not sufficient. Each call to
pcie_config_aspm_link(link, policy_to_aspm_state(link)), e.g. in path
pcie_aspm_pm_state_change -> pcie_config_aspm_path will reset the
enabled states to the policy.

> mutex_unlock(&aspm_lock);
> up_read(&pci_bus_sem);
>

2020-12-09 17:55:30

by Kai-Heng Feng

Subject: Re: [PATCH 2/2] PCI/ASPM: Use capability to override ASPM via sysfs

> On Dec 9, 2020, at 01:18, Heiner Kallweit <[email protected]> wrote:
>
> Am 08.12.2020 um 09:25 schrieb Kai-Heng Feng:
>> If we are to use sysfs to change ASPM settings, we may want to override
>> the default ASPM policy.
>>
>> So use ASPM capability, instead of default policy, to be able to use all
>> possible ASPM states.
>>
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>> ---
>> drivers/pci/pcie/aspm.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 2ea9fddadfad..326da7bbc84d 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -1239,8 +1239,7 @@ static ssize_t aspm_attr_store_common(struct device *dev,
>>
>> link->aspm_disable |= state;
>> }
>> -
>> - pcie_config_aspm_link(link, policy_to_aspm_state(link));
>> + pcie_config_aspm_link(link, link->aspm_capable);
>>
> I like the idea, because the policy can be changed by the user anyway.
> Therefore I don't see it as a hard system limit.
>
> However I think this change is not sufficient. Each call to
> pcie_config_aspm_link(link, policy_to_aspm_state(link)), e.g. in path
> pcie_aspm_pm_state_change -> pcie_config_aspm_path will reset the
> enabled states to the policy.

That's right, let me work this in v2.

Kai-Heng

>
>> mutex_unlock(&aspm_lock);
>> up_read(&pci_bus_sem);
>>
>