NACK/cmnt: [SRU] [A] [PATCH 0/7] Fix Runtime PM for r8169
kai.heng.feng at canonical.com
Wed Mar 28 10:08:55 UTC 2018
Stefan Bader <stefan.bader at canonical.com> wrote:
> On 21.03.2018 14:08, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1757422
>> r8169 stays in D0 even when no ethernet cable is plugged in. This drains
>> lots of power (~3W). The tested laptop uses 5.5W when r8169 is in D0,
>> 1.8W when r8169 is in D3. The power saved is substantial.
>> Improved rumtime PM logic to let the device gets suspended (D3) when the
>> port is not in used and the link is down.
>> [Test Case]
>> The chip version is 8168h/8111h.
>> Test when no ethernet gets plugged.
>> Powertop shows power consumption is roughly 5.5W.
>> lspci shows the device is in D0.
>> With the patch,
>> The power consumption is reduced to 1.8W.
>> lspci shows the device is in D3, PME# is correctly enabled.
>> Plug ethernet cable can corretly wake up the device.
>> Unplug the cable, the device gets suspended to D3 correctly.
>> [Regression Potential]
>> - r8169 is so ubiquitous, with lots of different chip versions. It's
>> hard to test all of them.
>> - PCI D3 needs system firmware (ACPI) support, this might hit some
>> plaform bugs.
>> - the code is still in v4.16-rc*, so it's not well tested by end users.
>> If it's too risky to merge this series into Artful, it should still get
>> merged for Bionic, the power consumption reduction is significant.
>> Daniel Drake (1):
>> r8169: only enable PCI wakeups when WOL is active
>> Heiner Kallweit (6):
>> PCI: Add pcim_set_mwi(), a device-managed pci_set_mwi()
>> r8169: switch to device-managed functions in probe
>> r8169: remove netif_napi_del in probe error path
>> r8169: remove unneeded rpm ops in rtl_shutdown
>> r8169: improve runtime pm in rtl8169_check_link_status
>> r8169: improve runtime pm in general and suspend unused ports
>> drivers/net/ethernet/realtek/r8169.c | 131 ++++++++++-------------------------
>> drivers/pci/pci.c | 25 +++++++
>> include/linux/pci.h | 1 +
>> 3 files changed, 62 insertions(+), 95 deletions(-)
> At least for now until this would have settled within Bionic. Just feels
> there could be too many possible problems.
Yea I guess it's too risky to include for Bionic.
That said, we still need this for Bionic based linux-oem.
When the time 18.10 is release, the patch should be well-tested.
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
More information about the kernel-team