NACK: [SRU] [Bionic] [PULL] [PATCH 00/62] Fix Runtime PM for r8169

[Timo Aaltonen]

On 25.06.2018 12:52, Stefan Bader wrote:
> On 25.06.2018 09:52, Timo Aaltonen wrote:
>> On 25.06.2018 10:31, Kai-Heng Feng wrote:
>>> at 15:24, Stefan Bader <stefan.bader at canonical.com> wrote:
>>>
>>>> On 25.06.2018 08:56, Kai-Heng Feng wrote:
>>>>> BugLink: https://bugs.launchpad.net/bugs/1757422
>>>>>
>>>>> [Impact]
>>>>> 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.
>>>>>
>>>>> [Fix]
>>>>> Improved rumtime PM logic to let the device gets suspended (D3) when the
>>>>> port is not in used and the link is down.
>>>>>
>>>>> Sync r8169 driver with mainline, with one fix from net-next.
>>>>> Everything is cleanly cherry-picked.
>>>>> This means the fix for LP: #1752772 is dropped and re-cherry-picked.
>>>>>
>>>>> [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]
>>>>> Low.
>>>>> The code was in mainline for a while, it should be well-tested by now.
>>>>>
>>>>> The following changes since commit
>>>>> 123dad8e7f35b815fdf6d0647b056c096f14d052:
>>>>>
>>>>>   ixgbe/ixgbevf: Free IRQ when PCI error recovery removes the device
>>>>> (2018-06-18 14:57:01 -0400)
>>>>>
>>>>> are available in the Git repository at:
>>>>>
>>>>>   git://git.launchpad.net/~kaihengfeng/+git/linux sru-bionic-r8169-rpm
>>>>>
>>>>> for you to fetch changes up to e88347ecb4e961abebde8810973c8a62455f4ea6:
>>>>>
>>>>>   r8169: Fix netpoll oops (2018-06-25 12:32:25 +0800)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> Andy Shevchenko (2):
>>>>>       r8169: Dereference MMIO address immediately before use
>>>>>       r8169: switch to device-managed functions in probe (part 2)
>>>>>
>>>>> Heiner Kallweit (55):
>>>>>       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
>>>>>       r8169: remove some WOL-related dead code
>>>>>       r8169: remove not needed PHY soft reset in
>>>>> rtl8168e_2_hw_phy_config
>>>>>       r8169: disable WOL per default
>>>>>       r8169: simplify and improve check for dash
>>>>>       r8169: improve interrupt handling
>>>>>       r8169: convert remaining feature flag and remove enum features
>>>>>       r8169: fix interrupt number after adding support for MSI-X
>>>>> interrupts
>>>>>       r8169: simplify rtl_set_mac_address
>>>>>       r8169: change type of first argument in rtl_tx_performance_tweak
>>>>>       r8169: change type of argument in rtl_disable/enable_clock_request
>>>>>       r8169: add helper tp_to_dev
>>>>>       PCI: Add two more values for PCIe Max_Read_Request_Size
>>>>>       r8169: replace magic numbers with PCI MRRS constant
>>>>>       r8169: remove unused member features from struct
>>>>>       r8169: remove member align from struct rtl_cfg_info
>>>>>       r8169: use skb_copy_to_linear_data in rtl8169_try_rx_copy
>>>>>       r8169: use constant NAPI_POLL_WAIT
>>>>>       r8169: switch to napi_schedule_irqoff
>>>>>       r8169: simplify rtl8169_alloc_rx_data
>>>>>       r8169: improve rtl8169_init_ring
>>>>>       r8169: remove unneeded check in rtl8169_rx_fill
>>>>>       r8169: replace rx_buf_sz with a constant
>>>>>       r8169: remove rtl8169_map_to_asic
>>>>>       r8169: change hw_start argument type
>>>>>       r8169: change argument type of counters handling functions
>>>>>       r8169: change interrupt handler argument type
>>>>>       r8169: drop member opts1_mask from struct rtl8169_private
>>>>>       r8169: don't display tp->mmio_addr address
>>>>>       r8169: improve rtl8169_get_mac_version
>>>>>       r8169: drop member txd_version from struct rtl8169_private
>>>>>       r8169: improve pci region handling
>>>>>       r8169: remove jumbo_tx_csum from chip config struct
>>>>>       r8169: don't use netif_info et al before net_device has been
>>>>> registered
>>>>>       r8169: remove unneeded call to __rtl8169_set_features in rtl_open
>>>>>       r8169: improve rtl8169_set_features
>>>>>       r8169: replace magic number for INTT mask with a constant
>>>>>       r8169: improve CPlusCmd handling
>>>>>       r8169: improve handling of CPCMD quirk mask
>>>>>       r8169: simplify rtl_hw_start_8169
>>>>>       r8169: remove calls to rtl_set_rx_mode
>>>>>       r8169: move common initializations to tp->hw_start
>>>>>       r8169: remove unneeded check in r8168_pll_power_down
>>>>>       r8169: remove 810x_phy_power_up/down
>>>>>       r8169: merge r810x_pll_power_down/up into r8168_pll_power_down/up
>>>>>       r8169: drop member pll_power_ops from struct rtl8169_private
>>>>>       r8169: simplify code by using ranges in switch clauses
>>>>>       r8169: replace longer if statements with switch statements
>>>>>       r8169: drop rtl_generic_op
>>>>>       r8169: improve PCI config space access
>>>>>       r8169: avoid potentially misaligned access when getting mac
>>>>> address
>>>>>       r8169: replace get_protocol with vlan_get_protocol
>>>>>       r8169: fix network error on resume from suspend
>>>>>
>>>>> Kai-Heng Feng (4):
>>>>>       Revert "r8169: fix interrupt number after adding support for
>>>>> MSI-X interrupts"
>>>>>       Revert "r8169: improve interrupt handling"
>>>>>       Revert "r8169: disable WOL per default"
>>>>>       Revert "r8169: remove some WOL-related dead code"
>>>>>
>>>>> Ville Syrjälä (1):
>>>>>       r8169: Fix netpoll oops
>>>>>
>>>>>  drivers/net/ethernet/realtek/r8169.c | 2101
>>>>> +++++++++++-----------------------
>>>>>  include/uapi/linux/pci_regs.h        |    2 +
>>>>>  2 files changed, 649 insertions(+), 1454 deletions(-)
>>>> Though this is isolated to one driver, it is a driver that is in
>>>> wide-spread
>>>> use. The risk of regression is to high to justify pulling in that huge
>>>> delta.
>>>> Even more for PM reasons. This is what HWE kernels are for.
>>>
>>> Runtime PM is enabled for 4.13 based linux-oem, which will upgrade to
>>> Bionic's 4.15 soon.
>>> This transition will regress those who uses 4.13 based kernel.
>>>
>>> If this series can't get pulled in, then we should probably make it
>>> upgrade to 4.15 based linux-oem kernel instead of Bionic's kernel.
>>
>> There is no 4.15 based linux-oem in xenial..
> 
> Not yet, but I thought the currently 4.13 based one would be changed to a 4.15
> based one at some point.

Nope, because we don't want to support it that long (until 2021).
Instead, linux-oem will upgrade to hwe backport from bionic when the
time is right (after the next cycle or so).


-- 
t

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20180625/e8f7b459/attachment.sig>