| Message ID | AANLkTin3DNEPDA+vpY-WuSa8ia-sgEA8_sr9qOLpDHph@mail.gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Le lundi 16 août 2010 à 13:35 +0800, LionSky a écrit : > It is a very typical scenario that users of laptop/Netbook experience > Internet over wireless instead of ethernet. > An obvious hint about that is user unplug-in the ethernet cable which > implies that he does not want to use the ethernet. So for power > efficiency, it is best to place ethernet adapter into a low power > state, such as C3hot, when ethernet cable is disconnected. > Battery life is very important for Laptop/Netbook. Currently, the > power consumption of active idle on a typical Netbook is about 5-6 > Watt. If you placed the etherent adapter in C3hot state, about > 0.5~1.0Watt power is saved which depneds on your H/W. This can prolong > battery life about 10%~20% which is a quite big power saving. > Now, Linux kernel supports runtime power management. It is not a hard > work to enable this feature on each ethernet adapter driver. > > Here I attached a simple and prototype implementation which is > verified on a Netbook (ASUS E1005PE with Atheros AR8132 PCI-E Fast > Ethernet Controller). > 1. When ethernet cable is disconnected, driver places ethernet adapter > into D3hot low power state. Just as CPU go to deep C-State. > 2. When ethernet cable is connected again, driver places ethernet back > in D0 power state. > > To further improve the power behavior of Linux on laptop/Netbook, a > proposal is that all ethernet drivers must support Low Power on Media ... should support ... > Disconnect. > > Is it right? Welcome any comments. > I cannot say if this patch is right on the power side. (Would it be better to handle the ethernet device power switch from a user policy daemon, instead of forcing the policy from the driver ?) I dont know the implications of Low Power on Media disconnect. At least, let ethtool be able to switch on or off this automatic behavior... But your patch introduces a shared (static actually) variable, atl1c_d3. Are you sure a single state is enough to handle several ethernet ports ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks Eric, Yes, you are right. It is better to handle the ethernet device power switch from a user policy daemon. This is just a proposal about Low Power on Media Disconnect which means when media, such as ethernet cable, is disconnected, the related device should be placed in a low power state, such as D3hot. Maybe we needs a smart implementation. Thanks -Lionsky 2010/8/16 Eric Dumazet <eric.dumazet@gmail.com>: > Le lundi 16 août 2010 à 13:35 +0800, LionSky a écrit : >> It is a very typical scenario that users of laptop/Netbook experience >> Internet over wireless instead of ethernet. >> An obvious hint about that is user unplug-in the ethernet cable which >> implies that he does not want to use the ethernet. So for power >> efficiency, it is best to place ethernet adapter into a low power >> state, such as C3hot, when ethernet cable is disconnected. >> Battery life is very important for Laptop/Netbook. Currently, the >> power consumption of active idle on a typical Netbook is about 5-6 >> Watt. If you placed the etherent adapter in C3hot state, about >> 0.5~1.0Watt power is saved which depneds on your H/W. This can prolong >> battery life about 10%~20% which is a quite big power saving. >> Now, Linux kernel supports runtime power management. It is not a hard >> work to enable this feature on each ethernet adapter driver. >> >> Here I attached a simple and prototype implementation which is >> verified on a Netbook (ASUS E1005PE with Atheros AR8132 PCI-E Fast >> Ethernet Controller). >> 1. When ethernet cable is disconnected, driver places ethernet adapter >> into D3hot low power state. Just as CPU go to deep C-State. >> 2. When ethernet cable is connected again, driver places ethernet back >> in D0 power state. >> >> To further improve the power behavior of Linux on laptop/Netbook, a >> proposal is that all ethernet drivers must support Low Power on Media > > ... should support ... > >> Disconnect. >> >> Is it right? Welcome any comments. >> > > I cannot say if this patch is right on the power side. > (Would it be better to handle the ethernet device power switch from a > user policy daemon, instead of forcing the policy from the driver ?) > I dont know the implications of Low Power on Media disconnect. > At least, let ethtool be able to switch on or off this automatic > behavior... > > But your patch introduces a shared (static actually) variable, atl1c_d3. > Are you sure a single state is enough to handle several ethernet ports ? > > > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- atl1cold/atl1c_main.c 2010-08-04 16:21:59.863328895 +0800 +++ atl1cnew/atl1c_main.c 2010-08-16 11:06:42.217285358 +0800 @@ -20,6 +20,7 @@ */ #include "atl1c.h" +#include <linux/pm_runtime.h> #define ATL1C_DRV_VERSION "1.0.0.2-NAPI" char atl1c_driver_name[] = "atl1c"; @@ -66,6 +67,8 @@ static void atl1c_setup_mac_ctrl(struct atl1c_adapter *adapter); static void atl1c_clean_rx_irq(struct atl1c_adapter *adapter, u8 que, int *work_done, int work_to_do); +static int atl1c_runtime_suspend(struct device *device); +static int atl1c_runtime_resume(struct device *device); static const u16 atl1c_pay_load_size[] = { 128, 256, 512, 1024, 2048, 4096, @@ -98,6 +101,9 @@ static const u32 atl1c_default_msg = NETIF_MSG_DRV | NETIF_MSG_PROBE | NETIF_MSG_LINK | NETIF_MSG_TIMER | NETIF_MSG_IFDOWN | NETIF_MSG_IFUP; +/*a variable indicates current H/W power state*/ +static u16 atl1c_d3 = 1; + /* * atl1c_init_pcie - init PCIE module */ @@ -240,8 +246,13 @@ atl1c_set_aspm(hw, false); } netif_carrier_off(netdev); + + pm_runtime_put_noidle(&pdev->dev); + pm_schedule_suspend(&pdev->dev,0); } else { /* Link Up */ + pm_request_resume(&pdev->dev); + hw->hibernate = false; spin_lock_irqsave(&adapter->mdio_lock, flags); err = atl1c_get_speed_and_duplex(hw, &speed, &duplex); @@ -1576,6 +1587,21 @@ } /* + * checkPoweState - If current power state is D3hot, resume it to D0 + * @pdev: pointer to a pci network interface device structure + */ +static inline int checkPowerState(struct pci_dev * pdev) +{ + if (unlikely(atl1c_d3)) { + pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 0); + pdev->dev.power.runtime_status = 0; + atl1c_d3 = 0; + return 1; + } + return 0; +} + +/* * atl1c_intr - Interrupt Handler * @irq: interrupt number * @data: pointer to a network interface device structure @@ -1592,6 +1618,8 @@ u32 status; u32 reg_data; + checkPowerState(pdev); + do { AT_READ_REG(hw, REG_ISR, ®_data); status = reg_data & hw->intr_mask; @@ -2703,6 +2731,12 @@ if (netif_msg_probe(adapter)) dev_info(&pdev->dev, "version %s\n", ATL1C_DRV_VERSION); cards_found++; + + pm_runtime_set_active(&pdev->dev); + pm_runtime_enable(&pdev->dev); + + pm_runtime_idle(&pdev->dev); + return 0; err_reset: @@ -2832,6 +2866,57 @@ .resume = atl1c_io_resume, }; +/* Adding runtime power management support*/ +#ifdef CONFIG_PM + +//callback function definition +static int atl1c_runtime_suspend(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + + pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 3); + atl1c_d3 = 1; + dev_err(&pdev->dev, "[debug] runtime_suspend\n"); + + return 0; +} + +static int atl1c_runtime_resume(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + + pci_write_config_word(pdev, pdev->pm_cap + PCI_PM_CTRL, 0); + atl1c_d3 = 0; + dev_err(&pdev->dev, "[debug] runtime_resume\n"); + + return 0; +} + +static int atl1c_runtime_idle(struct device *device) +{ + struct pci_dev *pdev = to_pci_dev(device); + struct net_device *dev = pci_get_drvdata(pdev); + struct atl1c_adapter *adapter = netdev_priv(dev); + + atl1c_check_link_status(adapter); + + return 0; +} + +static const struct dev_pm_ops atl1c_pm_ops = { + .runtime_suspend = atl1c_runtime_suspend, + .runtime_resume = atl1c_runtime_resume, + .runtime_idle = atl1c_runtime_idle, +}; + +#define ATL1C_PM_OPS (&atl1c_pm_ops) + +#else /* !CONFIG_PM */ + +#define ATL1C_PM_OPS NULL + +#endif /* !CONFIG_PM */ + static struct pci_driver atl1c_driver = { .name = atl1c_driver_name, .id_table = atl1c_pci_tbl, @@ -2841,7 +2926,8 @@ .suspend = atl1c_suspend, .resume = atl1c_resume, .shutdown = atl1c_shutdown, - .err_handler = &atl1c_err_handler + .err_handler = &atl1c_err_handler, + .driver.pm = ATL1C_PM_OPS, }; /*