Shaohua Li wrote:
> On Fri, Jun 19, 2009 at 05:28:41AM +0800, Jesse Barnes wrote:
>> On Thu, 21 May 2009 11:10:06 +0900
>> Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> wrote:
>>
>>> Hi,
>>>
>>> This patch changes ASPM driver to enable more detailed ASPM
>>> configuration. With this patch,
>>>
>>> - ASPM can be enabled partially in the hierarchy.
>>> - L0s can be managed for each direction (upstream direction and
>>> downstream direction) of the link.
> Well, these are nice features, but what kind of machine does you test?
> Does it have several level hierarchy? The most powerful system I saw
> just has two levels, which sounds not worth adding partial aspm.
Thank you very much for comments.
I think it is useful also for the system that doesn't have so
many levels of hierarchy, such as mobile systems. Here is "lspci -t"
output on my environment with some comments. My system also has
just two levels, but with the patch, upstream L0s is enabled on the
link 0000:15.00.0 (switch downstream port) to 0000:16:00.0 (endpoint)
in powersave mode.
[kanesige@localhost develop]$ /sbin/lspci -t
-[0000:00]-+-00.0
+-01.0-[0000:04-13]--
L0s(UP) enabled
L0s(DW) enabled(*)
+-02.0-[0000:14-29]--+-00.0-[0000:15-28]--+-00.0-[0000:16]----00.0
| | +-01.0-[0000:18-27]--
L0s(DW) enabled(*)
| | \-02.0-[0000:28]--+-00.0
| | \-00.1
| \-00.3-[0000:29]--
+-03.0-[0000:2a-2d]----00.0-[0000:2b-2d]--+-02.0-[0000:2c]--+-00.0
| | \-00.1
L0s(DW) enabled(*)
| \-04.0-[0000:2d]----00.0
+-04.0-[0000:2e-4f]----00.0-[0000:2f-4f]--+-02.0-[0000:30-3f]--+-00.0
| | \-00.1
| \-04.0-[0000:40-4f]--+-00.0
| \-00.1
+-06.0-[0000:51-73]----00.0-[0000:52-73]--+-02.0-[0000:54-63]--+-00.0
| | \-00.1
| \-04.0-[0000:64-73]----00.0-[0000:65]--+-08.0
| \-08.1
+-10.0
+-10.1
+-10.2
+-10.3
+-11.0
+-11.3
+-13.0
+-15.0
+-16.0
+-1c.0-[0000:75-83]--
+-1d.0
+-1d.1
+-1d.2
+-1d.3
+-1d.7
+-1e.0-[0000:84]--
+-1f.0
+-1f.2
\-1f.3
(*) Downstream direction L0s is enabled but it has no effect because
Downstream direction L0s is disabled on its upstream links.
> The second feature seems more reasonable.
I think I should have implement those features as separate patches.
I'll do it if needed.
> ASPM is still broken in a lot of devices, before we go far, we need to make
> sure the feature is really required.
I tested that pci_disable_link_state() first, actually :)
By the way, I've tested the patches with debugging patch that give us
the information like below through /proc. I'm attaching it for your
information.
LINK (addr: ffff88083c5fc840, pdev: ffff88083c604240, name: 0000:00:02.0)
root: ffff88083c5fc840, parent (null)
ASPM enabled: L0sUP-, L0sDW-, L1-
ASPM capable: L0sUP-, L0sDW+, L1-
ASPM support: L0sUP+, L0sDW+, L1-
ASPM disable: L0sUP+, L0sDW+, L1+
ASPM default: L0sUP-, L0sDW-, L1-
ASPM latency: L0sUP 5000, L0sDW 256, L1 65000
ClockPM: capable 0, enabled 0, default 0
LINK (addr: ffff88083c5fcb58, pdev: ffff88083c572968, name: 0000:15:02.0)
root: ffff88083c5fc840, parent ffff88083c5fc840
ASPM enabled: L0sUP-, L0sDW+, L1-
ASPM capable: L0sUP-, L0sDW+, L1-
ASPM support: L0sUP-, L0sDW+, L1-
ASPM disable: L0sUP-, L0sDW-, L1-
ASPM default: L0sUP-, L0sDW-, L1-
ASPM latency: L0sUP 256, L0sDW 128, L1 65000
ClockPM: capable 0, enabled 0, default 0
EP[0000:28:00.0] accept l0s 512, l1 64000
EP[0000:28:00.1] accept l0s 512, l1 64000
LINK (addr: ffff88083c5fca50, pdev: ffff88083c5718d8, name: 0000:15:00.0)
root: ffff88083c5fc840, parent ffff88083c5fc840
ASPM enabled: L0sUP+, L0sDW+, L1-
ASPM capable: L0sUP+, L0sDW+, L1-
ASPM support: L0sUP+, L0sDW+, L1-
ASPM disable: L0sUP-, L0sDW-, L1-
ASPM default: L0sUP-, L0sDW-, L1-
ASPM latency: L0sUP 5000, L0sDW 2048, L1 65000
ClockPM: capable 0, enabled 0, default 0
EP[0000:16:00.0] accept l0s 4294967295, l1 4294967295
Thanks,
Kenji Kaneshige
drivers/pci/pcie/aspm.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 113 insertions(+)
Index: 20090521/drivers/pci/pcie/aspm.c
===================================================================
--- 20090521.orig/drivers/pci/pcie/aspm.c
+++ 20090521/drivers/pci/pcie/aspm.c
@@ -19,6 +19,8 @@
#include <linux/jiffies.h>
#include <linux/delay.h>
#include <linux/pci-aspm.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
#include "../pci.h"
#ifdef MODULE_PARAM_PREFIX
@@ -859,6 +861,117 @@ void pcie_aspm_remove_sysfs_dev_files(st
sysfs_remove_file_from_group(&pdev->dev.kobj,
&dev_attr_clk_ctl.attr, power_group);
}
+
+static void show_link(struct seq_file *p,
+ struct pcie_link_state *link, int depth)
+{
+ struct pci_dev *child, *parent = link->pdev;
+ struct pci_bus *linkbus = parent->subordinate;
+ struct pcie_link_state *childlink;
+ int indent = depth * 4;
+
+ seq_printf(p,
+ "%*sLINK (addr: %p, pdev: %p, name: %s)\n"
+ "%*s root: %p, parent %p\n"
+ "%*s ASPM enabled: L0sUP%c, L0sDW%c, L1%c\n"
+ "%*s ASPM capable: L0sUP%c, L0sDW%c, L1%c\n"
+ "%*s ASPM support: L0sUP%c, L0sDW%c, L1%c\n"
+ "%*s ASPM disable: L0sUP%c, L0sDW%c, L1%c\n"
+ "%*s ASPM default: L0sUP%c, L0sDW%c, L1%c\n"
+ "%*s ASPM latency: L0sUP %u, L0sDW %u, L1 %u\n"
+ "%*s ClockPM: capable %u, enabled %u, default %u\n",
+ indent, "", link, parent, pci_name(parent),
+ indent, "", link->root, link->parent,
+ indent, "",
+ (link->aspm_enabled & ASPM_STATE_L0S_UP) ? '+' : '-',
+ (link->aspm_enabled & ASPM_STATE_L0S_DW) ? '+' : '-',
+ (link->aspm_enabled & ASPM_STATE_L1) ? '+' : '-',
+ indent, "",
+ (link->aspm_capable & ASPM_STATE_L0S_UP) ? '+' : '-',
+ (link->aspm_capable & ASPM_STATE_L0S_DW) ? '+' : '-',
+ (link->aspm_capable & ASPM_STATE_L1) ? '+' : '-',
+ indent, "",
+ (link->aspm_support & ASPM_STATE_L0S_UP) ? '+' : '-',
+ (link->aspm_support & ASPM_STATE_L0S_DW) ? '+' : '-',
+ (link->aspm_support & ASPM_STATE_L1) ? '+' : '-',
+ indent, "",
+ (link->aspm_disable & ASPM_STATE_L0S_UP) ? '+' : '-',
+ (link->aspm_disable & ASPM_STATE_L0S_DW) ? '+' : '-',
+ (link->aspm_disable & ASPM_STATE_L1) ? '+' : '-',
+ indent, "",
+ (link->aspm_default & ASPM_STATE_L0S_UP) ? '+' : '-',
+ (link->aspm_default & ASPM_STATE_L0S_DW) ? '+' : '-',
+ (link->aspm_default & ASPM_STATE_L1) ? '+' : '-',
+ indent, "",
+ link->latency_up.l0s, link->latency_dw.l0s,
+ max_t(u32, link->latency_up.l1, link->latency_dw.l1),
+ indent, "", link->clkpm_capable,
+ link->clkpm_enabled, link->clkpm_default);
+ list_for_each_entry(child, &linkbus->devices, bus_list) {
+ int func = PCI_FUNC(child->devfn);
+ if ((child->pcie_type != PCI_EXP_TYPE_ENDPOINT) &&
+ (child->pcie_type != PCI_EXP_TYPE_LEG_END))
+ continue;
+ seq_printf(p, "%*s EP[%s] accept l0s %u, l1 %u\n",
+ indent, "", pci_name(child),
+ link->acceptable[func].l0s,
+ link->acceptable[func].l1);
+ }
+ seq_printf(p, "\n");
+
+ list_for_each_entry(childlink, &link->children, link)
+ show_link(p, childlink, (depth + 1));
+}
+
+static int show_pcie_link_state(struct seq_file *p, void *v)
+{
+ struct pcie_link_state *link;
+
+ mutex_lock(&aspm_lock);
+ list_for_each_entry(link, &link_list, sibling) {
+ if (!link->parent)
+ show_link(p, link, 0);
+ }
+ mutex_unlock(&aspm_lock);
+ return 0;
+}
+
+static int pcie_link_state_open(struct inode *inode, struct file *file)
+{
+ size_t size = 128 * 1024;
+ char *buf;
+ int res;
+ struct seq_file *m;
+
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ res = single_open(file, show_pcie_link_state, NULL);
+ if (!res) {
+ m = file->private_data;
+ m->buf = buf;
+ m->size = size;
+ } else
+ kfree(buf);
+ return res;
+}
+
+static const struct file_operations pcie_link_state_operations = {
+ .open = pcie_link_state_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static int pcie_aspm_init_proc(void)
+{
+ if (aspm_disabled)
+ return 0;
+ proc_create("pcie_link_state", 0, NULL, &pcie_link_state_operations);
+ return 0;
+}
+late_initcall(pcie_aspm_init_proc);
#endif
static int __init pcie_aspm_disable(char *str)
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
