Messages in this thread | | | Subject | Re: [RFC] PCIE ASPM support | From | Shaohua Li <> | Date | Tue, 08 Jan 2008 09:35:42 +0800 |
| |
On Tue, 2008-01-08 at 09:02 +1100, Michael Ellerman wrote: > On Thu, 2008-01-03 at 14:15 +0800, Shaohua Li wrote: > > PCI Express ASPM defines a protocol for PCI Express components in the D0 > > state to reduce Link power by placing their Links into a low power state > > and instructing the other end of the Link to do likewise. This > > capability allows hardware-autonomous, dynamic Link power reduction > > beyond what is achievable by software-only controlled power management. > > However, The device should be configured by software appropriately. > > Enabling ASPM will save power, but will introduce device latency. > > > > This patch adds ASPM support in Linux. It introduces a global policy for > > ASPM, a sysfs file /sys/module/pcie_aspm/parameters/policy can control > > it. The interface can be used as a boot option too. Currently we have > > below setting: > > -default, BIOS default setting > > -powersave, highest power saving mode, enable all available ASPM state > > and clock power management > > -performance, highest performance, disable ASPM and clock power > > management > > By default, the 'default' policy is used currently. > > > > In my test, power difference between powersave mode and performance mode > > is about 1.3w in a system with 3 PCIE links. > > > > please review, any comments will be appreciated. > > > > Signed-off-by: Shaohua Li <shaohua.li@intel.com> > > Minor nit, but you do a lot of these: > > + return ((struct link_state *)pdev->link_state)->bios_aspm_state; > + return ((struct link_state *)pdev->link_state)->bios_clk_state; > + ((struct link_state*)pdev->link_state)->clk_pm_enabled = !!enable; > + ((struct link_state*)pdev->link_state)->clk_pm_capable = capable; > + ((struct link_state*)pdev->link_state)->clk_pm_enabled = enabled; > + ((struct link_state*)pdev->link_state)->bios_clk_state = enabled; > + ((struct link_state *)pdev->link_state)->enabled_state = state; > + if (((struct link_state *)pdev->link_state)->support_state == 0) > + if (((struct link_state *)pdev->link_state)->enabled_state == state) > + ((struct link_state *)pdev->link_state)->enabled_state); Thanks for your time. I'll fix in next release.
Thanks, Shaohua
| |