| Message ID | 20201208082534.2460215-2-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/2] PCI/ASPM: Store disabled ASPM states | expand |
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 <kai.heng.feng@canonical.com> > --- > 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); >
> On Dec 9, 2020, at 01:18, Heiner Kallweit <hkallweit1@gmail.com> 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 <kai.heng.feng@canonical.com> >> --- >> 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); >> >
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);
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 <kai.heng.feng@canonical.com> --- drivers/pci/pcie/aspm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)