| Age | Commit message (Collapse) | Author |
|---|
|
Communications with a hardware vendor confirm that the expected behaviour
on systems that set the FADT ASPM disable bit but which still grant full
PCIe control is for the OS to leave any BIOS configuration intact and
refuse to touch the ASPM bits. This mimics the behaviour of Windows.
Signed-off-by: Matthew Garrett <mjg59@coreos.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
|
|
We had an inconsistent mix of using and omitting the "extern" keyword
on function declarations in header files. This removes them all.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
|
|
Right now we forcibly clear ASPM state on all devices if the BIOS indicates
that the feature isn't supported. Based on the Microsoft presentation
"PCI Express In Depth for Windows Vista and Beyond", I'm starting to think
that this may be an error. The implication is that unless the platform
grants full control via _OSC, Windows will not touch any PCIe features -
including ASPM. In that case clearing ASPM state would be an error unless
the platform has granted us that control.
This patch reworks the ASPM disabling code such that the actual clearing
of state is triggered by a successful handoff of PCIe control to the OS.
The general ASPM code undergoes some changes in order to ensure that the
ability to clear the bits isn't overridden by ASPM having already been
disabled. Further, this theoretically now allows for situations where
only a subset of PCIe roots hand over control, leaving the others in the
BIOS state.
It's difficult to know for sure that this is the right thing to do -
there's zero public documentation on the interaction between all of these
components. But enough vendors enable ASPM on platforms and then set this
bit that it seems likely that they're expecting the OS to leave them alone.
Measured to save around 5W on an idle Thinkpad X220.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
|
|
Need to use it in _e1000e_disable_aspm. This routine is used for error
recovery, where the pci_bus_sem is already held, and we don't want
pci_disable_link_state to try to take it again. So add a locked variant
for use in cases like this.
Found lock up:
[ 2374.654557] kworker/32:1 D ffff881027f6b0f0 0 6075 2 0x00000000
[ 2374.654816] ffff88503f099a68 0000000000000046 ffff88503f098000 0000000000004000
[ 2374.654837] 00000000001d1ec0 ffff88503f099fd8 00000000001d1ec0 ffff88503f099fd8
[ 2374.654860] 0000000000004000 00000000001d1ec0 ffff88503dcc8000 ffff88503f090000
[ 2374.654880] Call Trace:
[ 2374.654898] [<ffffffff810b1302>] ? __lock_acquired+0x3a/0x224
[ 2374.654914] [<ffffffff81c2b59c>] ? _raw_spin_unlock_irq+0x30/0x36
[ 2374.654925] [<ffffffff810b069d>] ? trace_hardirqs_on_caller+0x1f/0x178
[ 2374.654936] [<ffffffff81c2ab24>] rwsem_down_failed_common+0xd3/0x103
[ 2374.654945] [<ffffffff810b158f>] ? __lock_contended+0x3a/0x2a2
[ 2374.654955] [<ffffffff81c2ab7b>] rwsem_down_read_failed+0x12/0x14
[ 2374.654967] [<ffffffff813371e4>] call_rwsem_down_read_failed+0x14/0x30
[ 2374.654981] [<ffffffff8135df20>] ? pci_disable_link_state+0x5f/0xf5
[ 2374.654990] [<ffffffff81c2a0e6>] ? down_read+0x7e/0x91
[ 2374.654999] [<ffffffff8135df20>] ? pci_disable_link_state+0x5f/0xf5
[ 2374.655008] [<ffffffff8135df20>] pci_disable_link_state+0x5f/0xf5
[ 2374.655024] [<ffffffff81661796>] e1000e_disable_aspm+0x55/0x5a
[ 2374.655037] [<ffffffff816677eb>] e1000_io_slot_reset+0x59/0xea
[ 2374.655048] [<ffffffff8135fe0d>] ? report_mmio_enabled+0x5d/0x5d
[ 2374.655057] [<ffffffff8135fe3b>] report_slot_reset+0x2e/0x5d
[ 2374.655072] [<ffffffff8135369e>] pci_walk_bus+0x8a/0xb7
[ 2374.655081] [<ffffffff8135fe0d>] ? report_mmio_enabled+0x5d/0x5d
[ 2374.655091] [<ffffffff813603be>] broadcast_error_message+0xa4/0xb2
[ 2374.655101] [<ffffffff81352c71>] ? pci_bus_read_config_dword+0x72/0x80
[ 2374.655110] [<ffffffff813606df>] do_recovery+0x9e/0xf9
[ 2374.655120] [<ffffffff81360786>] handle_error_source+0x4c/0x51
[ 2374.655129] [<ffffffff81360974>] aer_isr_one_error+0x1e9/0x21a
[ 2374.655138] [<ffffffff81360a6c>] aer_isr+0xc7/0xcc
[ 2374.655147] [<ffffffff813609a5>] ? aer_isr_one_error+0x21a/0x21a
[ 2374.655159] [<ffffffff81096d9f>] process_one_work+0x237/0x3ec
[ 2374.655168] [<ffffffff81096d10>] ? process_one_work+0x1a8/0x3ec
[ 2374.655178] [<ffffffff8109728d>] worker_thread+0x17c/0x240
[ 2374.655186] [<ffffffff810b0803>] ? trace_hardirqs_on+0xd/0xf
[ 2374.655196] [<ffffffff81097111>] ? manage_workers+0xab/0xab
[ 2374.655209] [<ffffffff8109c8ed>] kthread+0xa0/0xa8
[ 2374.655223] [<ffffffff81c332d4>] kernel_thread_helper+0x4/0x10
[ 2374.655232] [<ffffffff81c2b880>] ? retint_restore_args+0xe/0xe
[ 2374.655243] [<ffffffff8109c84d>] ? __init_kthread_worker+0x5b/0x5b
[ 2374.655252] [<ffffffff81c332d0>] ? gs_change+0xb/0xb
when aer happens,
pci_walk_bus already have down_read(&pci_bus_sem)...
then report_slot_reset
==> e1000_io_slot_reset
==> e1000e_disable_aspm
==> pci_disable_link_state...
We can not use pci_disable_link_state, and it will try to hold pci_bus_sem again.
Try to have __pci_disable_link_state that will not need to hold pci_bus_sem.
-v2: change name to pci_disable_link_state_locked() according to Jesse.
[jbarnes: make sure new function is exported for modules]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
|
|
v3 -> v2: Moved ASPM enabling logic to pci_set_power_state()
v2 -> v1: Preserved the logic in pci_raw_set_power_state()
: Added ASPM enabling logic after scanning Root Bridge
: http://marc.info/?l=linux-pci&m=130046996216391&w=2
v1 : http://marc.info/?l=linux-pci&m=130013164703283&w=2
The assumption made in commit 41cd766b065970ff6f6c89dd1cf55fa706c84a3d
(PCI: Don't enable aspm before drivers have had a chance to veto it) that
pci_enable_device() will result in re-configuring ASPM when aspm_policy is
POWERSAVE is no longer valid. This is due to commit
97c145f7c87453cec90e91238fba5fe2c1561b32 (PCI: read current power state
at enable time) which resets dev->current_state to D0. Due to this the
call to pcie_aspm_pm_state_change() is never made. Note the equality check
(below) that returns early:
./drivers/pci/pci.c: pci_raw_set_pci_power_state()
546 /* Check if we're already there */
547 if (dev->current_state == state)
548 return 0;
Therefore OSPM never configures the PCIe links for ASPM to turn them "on".
Fix it by configuring ASPM from the pci_enable_device() code path. This
also allows a driver such as the e1000e networking driver a chance to
disable ASPM (L0s, L1), if need be, prior to enabling the device. A
driver may perform this action if the device is known to mis-behave
wrt ASPM.
Signed-off-by: Naga Chumbalkar <nagananda.chumbalkar@hp.com>
Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
|
|
We currently refuse to touch the ASPM registers if the BIOS tells us that
ASPM isn't supported. This can cause problems if the BIOS has (for any
reason) enabled ASPM on some devices anyway. Change the code such that we
explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
and make sure we tidy up appropriately on device removal in order to deal
with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
over control then we won't touch the registers.
Signed-off-by: Matthew Garrett <mjg@redhat.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
|
|
The ACPI FADT table includes an ASPM control bit. If the bit is set, do
not enable ASPM since it may indicate that the platform doesn't actually
support the feature.
Tested-by: Jack Howarth <howarth@bromo.msbb.uc.edu>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
|
|
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.
Note: some devices might not work well with aspm, either because chipset
issue or device issue. The patch provide API (pci_disable_link_state),
driver can disable ASPM for specific device.
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
|
