On Fri, Jun 19, 2009 at 01:28:44PM +0800, Kenji Kaneshige wrote: > Shaohua Li wrote: > > On Fri, Jun 19, 2009 at 11:19:49AM +0800, Kenji Kaneshige wrote: > >> 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. > > Can you send me the output of lspci -vvvxxx please? > > > > Here it is. Please note that I'm using pcie_aspm=force option. Hi, can you please split the patch into two patches, then I will have more detail review/test. Also please don't do cleanup in the patches for new feature. I had a simple look at the patch, and found at least one issue: A link might have multiple child links, we need check all child links's latency to enable the parent's ASPM. In pcie_config_aspm_path(), the patch seems not check peer link. Thanks, Shaohua -- 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
