r8168 is needed to enter P-state: Package State 6 (pc6) on Haswell hardware

r8168 is needed to enter P-state: Package State 6 (pc6) on Haswell hardware

[Ceriel Jacobs]

With in-kernel r8169 module, only P-state package C3 (pc3) can be 
reached when enabling ASPM.

Only after installing r8168 driver (and enabling ASPM), a Haswell 
Celeron G1820/G1840 processer will enter package C6 state (pc6).

Tested with Ubuntu kernel 3.13 x86_64
and Ubuntu mainline
Linux ubuntu14 3.17.0-999-generic #201409240305 SMP Wed Sep 24 02:07:04 
UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

I hope this gets fixed: Haswell processor entering pc6 with ASPM enabled 
and r8169 in-kernel module.

R8111GR hardware:
# lspci -nnkvv -s 03:00.0
03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. 
RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] 
(rev 11)
	Subsystem: ASRock Incorporation Motherboard (one of many) [1849:8168]
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 27
	Region 0: I/O ports at e000 [size=256]
	Region 2: Memory at f0404000 (64-bit, non-prefetchable) [size=4K]
	Region 4: Memory at f0400000 (64-bit, prefetchable) [size=16K]
	Capabilities: [40] Power Management version 3
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA 
PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [50] MSI: Enable+ Count=1/1 Maskable- 64bit+
		Address: 00000000fee0100c  Data: 41e1
	Capabilities: [70] Express (v2) Endpoint, MSI 01
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 4096 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency 
L0s unlimited, L1 <64us
			ClockPM+ Surprise- LLActRep- BwNot-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk+ DLActive- 
BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR+, OBFF Via 
message/WAKE#
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR+, OBFF 
Disabled
		LnkCtl2: Target Link Speed: 2.5GT/s, EnterCompliance- SpeedDis-
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- 
ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, 
EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
		Vector table: BAR=4 offset=00000000
		PBA: BAR=4 offset=00000800
	Capabilities: [d0] Vital Product Data
		Unknown small resource type 00, will not decode more.
	Capabilities: [100 v1] Advanced Error Reporting
		UESta:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- 
MalfTLP- ECRC- UnsupReq- ACSViol-
		UEMsk:	DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- 
MalfTLP- ECRC- UnsupReq- ACSViol-
		UESvrt:	DLP+ SDES+ TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ 
MalfTLP+ ECRC- UnsupReq- ACSViol-
		CESta:	RxErr- BadTLP+ BadDLLP- Rollover- Timeout+ NonFatalErr-
		CEMsk:	RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr+
		AERCap:	First Error Pointer: 00, GenCap+ CGenEn- ChkCap+ ChkEn-
	Capabilities: [140 v1] Virtual Channel
		Caps:	LPEVC=0 RefClk=100ns PATEntryBits=1
		Arb:	Fixed- WRR32- WRR64- WRR128-
		Ctrl:	ArbSelect=Fixed
		Status:	InProgress-
		VC0:	Caps:	PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
			Arb:	Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
			Ctrl:	Enable+ ID=0 ArbSelect=Fixed TC/VC=01
			Status:	NegoPending- InProgress-
	Capabilities: [160 v1] Device Serial Number <cut>
	Capabilities: [170 v1] Latency Tolerance Reporting
		Max snoop latency: 71680ns
		Max no snoop latency: 71680ns

Re: r8168 is needed to enter P-state: Package State 6 (pc6) on Haswell hardware

[Francois Romieu]

Ceriel Jacobs <linux-ide@crashplan.pro> :
> With in-kernel r8169 module, only P-state package C3 (pc3) can be reached
> when enabling ASPM.
> 
> Only after installing r8168 driver (and enabling ASPM), a Haswell Celeron
> G1820/G1840 processer will enter package C6 state (pc6).

Vanilla kernel r8169 driver logs a message that contains an "XID" string.
Please grep for it in your dmesg so that I can narrow the search for 
meaningful differences in Realtek's r8168 driver.

Thanks.

-- 
Ueimor

Re: r8168 is needed to enter P-state: Package State 6 (pc6) on Haswell hardware

[Ceriel Jacobs]

This is the requested "XID" dmesg line is:

[    1.479298] r8169 0000:03:00.0 eth0: RTL8168g/8111g at 
0xffffc90004752000, d0:50:99:0b:09:90, XID 0c000800 IRQ 27

Francois Romieu schreef op 05-10-14 om 18:59:
> Ceriel Jacobs <linux-ide@crashplan.pro> :
>> With in-kernel r8169 module, only P-state package C3 (pc3) can be reached
>> when enabling ASPM.
>>
>> Only after installing r8168 driver (and enabling ASPM), a Haswell Celeron
>> G1820/G1840 processer will enter package C6 state (pc6).
>
> Vanilla kernel r8169 driver logs a message that contains an "XID" string.
> Please grep for it in your dmesg so that I can narrow the search for
> meaningful differences in Realtek's r8168 driver.
>
> Thanks.
>

RE: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware

[Hayes Wang]

 Francois Romieu [mailto:romieu@fr.zoreil.com] 
> Sent: Monday, October 06, 2014 12:59 AM
[...]
> > With in-kernel r8169 module, only P-state package C3 (pc3) 
> > can be reached
> > when enabling ASPM.
> > 
> > Only after installing r8168 driver (and enabling ASPM), a 
> > Haswell Celeron
> > G1820/G1840 processer will enter package C6 state (pc6).
> 
> Vanilla kernel r8169 driver logs a message that contains an 
> "XID" string.
> Please grep for it in your dmesg so that I can narrow the search for 
> meaningful differences in Realtek's r8168 driver.

I don't sure if the following information is helpful. Besides, I remember the rtl_init_one()
would disable it.

http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=d64ec841517a25f6d468bde9f67e5b4cffdc67c7

http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4521e1a94279ce610d3f9b7945c17d581f804242
 
Best Regards,
Hayes

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware

[Francois Romieu]

Hayes Wang <hayeswang@realtek.com> :
>  Francois Romieu [mailto:romieu@fr.zoreil.com] 
[...]
> I don't sure if the following information is helpful. Besides, I remember
> the rtl_init_one() would disable it.
> 
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=d64ec841517a25f6d468bde9f67e5b4cffdc67c7
> 
> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4521e1a94279ce610d3f9b7945c17d581f804242

Yes, I did not expect this stuff to stay in geostationary orbit for long :o/

Realtek's r8168 driver defaults to CONFIG_ASPM=1 but I guess some users
need to disable it and there's no known pattern / blacklist, right ?

Ceriel, does the patch below against current kernel make a difference ?

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 0921302..b4a3881 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -468,6 +468,7 @@ enum rtl8168_registers {
 #define PWM_EN				(1 << 22)
 #define RXDV_GATED_EN			(1 << 19)
 #define EARLY_TALLY_EN			(1 << 16)
+#define FORCE_CLK			(1 << 15) /* force clock request */
 };
 
 enum rtl_register_content {
@@ -5279,8 +5280,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
 	rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 0x1d8f, ERIAR_EXGMAC);
 
 	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
-	RTL_W32(MISC, RTL_R32(MISC) & ~RXDV_GATED_EN);
+	RTL_W32(MISC, (RTL_R32(MISC) | FORCE_CLK) & ~RXDV_GATED_EN);
 	RTL_W8(MaxTxPacketSize, EarlySize);
+	RTL_W8(Config5, RTL_R8(Config5) | ASPM_en);
+	RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
 
 	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
 	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);

RE: r8168 is needed to enter P-state: Package State 6(pc6)onHaswellhardware

[Hayes Wang]

 Francois Romieu [mailto:romieu@fr.zoreil.com] 
> Sent: Tuesday, October 07, 2014 6:13 AM
[...]
> Realtek's r8168 driver defaults to CONFIG_ASPM=1 but I guess 
> some users
> need to disable it and there's no known pattern / blacklist, right ?

When enabling the ASPM, it would influence the thrpughput. It is hard to
choose performance or power saving. Therefore, we reserve the config
to let the user determines it.

 Best Regards,
Hayes

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware

[Ceriel Jacobs]

Francois Romieu schreef op 07-10-14 om 00:13:
> Hayes Wang <hayeswang@realtek.com> :
>>   Francois Romieu [mailto:romieu@fr.zoreil.com]
> [...]
>> I don't sure if the following information is helpful. Besides, I remember
>> the rtl_init_one() would disable it.
>>
>> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=d64ec841517a25f6d468bde9f67e5b4cffdc67c7
>>
>> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4521e1a94279ce610d3f9b7945c17d581f804242
>
> Yes, I did not expect this stuff to stay in geostationary orbit for long :o/
>
> Realtek's r8168 driver defaults to CONFIG_ASPM=1

# modinfo r8168 suggests the opposite (ASPM is disabled by default):
version:        8.039.00-NAPI
parm:           aspm:Enable ASPM. (int)

When ASPM would be enabled by default, one would need a boot parameter like:
parm:		aspm:Disable ASPM. (int)

  but I guess some users
> need to disable it and there's no known pattern / blacklist, right ?

I don't want to disable ASPM. In fact the r8168 module I am even running 
with poot params like:
# cat /proc/cmdline
BOOT_IMAGE=/boot/vmlinuz-3.17.0-999-generic root=/dev/sda1 ro 
biosdevname=0 intel_pstate=enable ipv6.disabled=1 debug ignore_loglevel 
panic=10 pcie_aspm.policy=powersave pcie_aspm=force r8168.aspm=1 
r8168.eee_enable=1 oops=panic

>
> Ceriel, does the patch below against current kernel make a difference ?

Francois, what do you mean with "current kernel", the latest Ubuntu 
mainline kernel or something different?

>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 0921302..b4a3881 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -468,6 +468,7 @@ enum rtl8168_registers {
>   #define PWM_EN				(1 << 22)
>   #define RXDV_GATED_EN			(1 << 19)
>   #define EARLY_TALLY_EN			(1 << 16)
> +#define FORCE_CLK			(1 << 15) /* force clock request */
>   };
>
>   enum rtl_register_content {
> @@ -5279,8 +5280,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
>   	rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 0x1d8f, ERIAR_EXGMAC);
>
>   	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> -	RTL_W32(MISC, RTL_R32(MISC) & ~RXDV_GATED_EN);
> +	RTL_W32(MISC, (RTL_R32(MISC) | FORCE_CLK) & ~RXDV_GATED_EN);
>   	RTL_W8(MaxTxPacketSize, EarlySize);
> +	RTL_W8(Config5, RTL_R8(Config5) | ASPM_en);
> +	RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
>
>   	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
>   	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
>

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware

[Francois Romieu]

Ceriel Jacobs <linux-ide@crashplan.pro> :
> Francois Romieu schreef op 07-10-14 om 00:13:
[...]
> >Realtek's r8168 driver defaults to CONFIG_ASPM=1
> 
> # modinfo r8168 suggests the opposite (ASPM is disabled by default):
> version:        8.039.00-NAPI
> parm:           aspm:Enable ASPM. (int)

$ grep CONFIG_ASPM src/Makefile 
CONFIG_ASPM = y

$ less src/r8168_n.c
[...]
#ifdef CONFIG_ASPM
static int aspm = 1;
#else
static int aspm = 0;
#endif

[...]
> Francois, what do you mean with "current kernel", the latest Ubuntu mainline
> kernel or something different?

Sorry. I meant current mainline as seen from git but anything close like
v3.15.x, v3.16.y or v3.17.k should be fine (patch applies).

-- 
Ueimor

Re: r8168 is needed to enter P-state: Package State 6(pc6)onHaswellhardware

[Francois Romieu]

Hayes Wang <hayeswang@realtek.com> :
[...]
> When enabling the ASPM, it would influence the thrpughput. It is hard to
> choose performance or power saving. Therefore, we reserve the config
> to let the user determines it.

Mmmm...

How do Realtek's devices behave if Config{2, 3} + MISC registers are
configured for ASPM whereas PCI config space registers aren't ?

-- 
Ueimor

RE: r8168 is needed to enter P-state: Package State6(pc6)onHaswellhardware

[Hayes Wang]

 Francois Romieu [mailto:romieu@fr.zoreil.com] 
> Sent: Wednesday, October 08, 2014 4:17 AM
[...]
> > When enabling the ASPM, it would influence the thrpughput. 
> > It is hard to
> > choose performance or power saving. Therefore, we reserve the config
> > to let the user determines it.
> 
> Mmmm...
> 
> How do Realtek's devices behave if Config{2, 3} + MISC registers are
> configured for ASPM whereas PCI config space registers aren't ?

The feature of the ASPM for the nic would be disabled.
ASPM is enabled when both the internal settings and
the PCI config space registers are enabled.
 
Best Regards,
Hayes

Re: r8168 is needed to enter P-state: Package State6(pc6)onHaswellhardware

[Ceriel Jacobs]

Hayes Wang schreef op 08-10-14 om 04:35:
>   Francois Romieu [mailto:romieu@fr.zoreil.com]
>> Sent: Wednesday, October 08, 2014 4:17 AM
> [...]
>>> When enabling the ASPM, it would influence the thrpughput.
>>> It is hard to
>>> choose performance or power saving. Therefore, we reserve the config
>>> to let the user determines it.
>>
>> Mmmm...
>>
>> How do Realtek's devices behave if Config{2, 3} + MISC registers are
>> configured for ASPM whereas PCI config space registers aren't ?
>
> The feature of the ASPM for the nic would be disabled.
> ASPM is enabled when both the internal settings and
> the PCI config space registers are enabled.

Have in mind a situation where the PCI config space registers are set 
manually (after user login).

At which point in time will be determined whether ASPM for the nic will 
be disabled or enabled? Once, when loading the kernel module? Or more often?

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Ceriel Jacobs]

Sorry for the delay. As newbie I first needed to learn about depmod to 
restore loading the r8169 module instead of r8168.

Now, I am struggling with applying the patch. Even with fuzz 99 applying 
this patch fails for hunk #2:

~/linux-3.13.0# patch -Np1 -F99 -r - --dry-run <<'END'
 > index 0921302..b4a3881 100644
...
 >  rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
 > END
checking file drivers/net/ethernet/realtek/r8169.c
Hunk #1 succeeded at 468 with fuzz 3.
Hunk #2 FAILED at 5280.
1 out of 2 hunks FAILED

~/linux-3.13.0# grep -nA8 'rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 
0x1d8f, ERIAR_EXGMAC)' drivers/net/ethernet/realtek/r8169.c
5274:	rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 0x1d8f, ERIAR_EXGMAC);
5275-
5276-	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
5277-	RTL_W32(MISC, RTL_R32(MISC) & ~RXDV_GATED_EN);
5278-	RTL_W8(MaxTxPacketSize, EarlySize);
5279-
5280-	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
5281-	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
5282-

# ls -l $(locate drivers/net/ethernet/realtek/r8169.c)
-rw-r--r-- 1 root root 177933 Aug 12 23:42 
/home/user/src/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c
-rw-r--r-- 1 root root 177933 Aug 24 22:41 
/root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c

# md5sum /root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c
95056b56932b375f8b65a6379009f704 
/root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c


Francois, could you help me apply your patch?


Francois Romieu schreef op 07-10-14 om 00:13:
> Hayes Wang <hayeswang@realtek.com> :
>>   Francois Romieu [mailto:romieu@fr.zoreil.com]
> [...]
>> I don't sure if the following information is helpful. Besides, I remember
>> the rtl_init_one() would disable it.
>>
>> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=d64ec841517a25f6d468bde9f67e5b4cffdc67c7
>>
>> http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=4521e1a94279ce610d3f9b7945c17d581f804242
>
> Yes, I did not expect this stuff to stay in geostationary orbit for long :o/
>
> Realtek's r8168 driver defaults to CONFIG_ASPM=1 but I guess some users
> need to disable it and there's no known pattern / blacklist, right ?
>
> Ceriel, does the patch below against current kernel make a difference ?
>
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index 0921302..b4a3881 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -468,6 +468,7 @@ enum rtl8168_registers {
>   #define PWM_EN				(1 << 22)
>   #define RXDV_GATED_EN			(1 << 19)
>   #define EARLY_TALLY_EN			(1 << 16)
> +#define FORCE_CLK			(1 << 15) /* force clock request */
>   };
>
>   enum rtl_register_content {
> @@ -5279,8 +5280,10 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
>   	rtl_eri_write(tp, 0x2f8, ERIAR_MASK_0011, 0x1d8f, ERIAR_EXGMAC);
>
>   	RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
> -	RTL_W32(MISC, RTL_R32(MISC) & ~RXDV_GATED_EN);
> +	RTL_W32(MISC, (RTL_R32(MISC) | FORCE_CLK) & ~RXDV_GATED_EN);
>   	RTL_W8(MaxTxPacketSize, EarlySize);
> +	RTL_W8(Config5, RTL_R8(Config5) | ASPM_en);
> +	RTL_W8(Config2, RTL_R8(Config2) | ClkReqEn);
>
>   	rtl_eri_write(tp, 0xc0, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
>   	rtl_eri_write(tp, 0xb8, ERIAR_MASK_0011, 0x0000, ERIAR_EXGMAC);
>

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Francois Romieu]

Ceriel Jacobs <linux-ide@crashplan.pro> :
[...]
> Francois, could you help me apply your patch?

$ git describe
v3.13
$ patch -p1 --dry-run < /tmp/plop
checking file drivers/net/ethernet/realtek/r8169.c
Hunk #1 succeeded at 467 (offset -1 lines).
Hunk #2 succeeded at 5275 (offset -5 lines).

$ git rev-list v3.16 -- drivers/net/ethernet/realtek/r8169.c | while read x; do echo $x:$(git cat-file -p $x:drivers/net/ethernet/realtek/r8169.c | md5sum); done | grep 95056b56932b375f8b65a6379009f704

-> oualou

Where does your 3.13 source tree come from ?

-- 
Ueimor

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Ceriel Jacobs]

The 3.13 source was installed using "apt-get source linux-source-3.13.0"

The patch hunk#2 failing issue is something in the "heredoc" notation 
(me trying to apply the patch without storing the patch lines as 
temporary input file). I am now using the temp file without any issue.

The next newly blocking issue is this:

# make -C /lib/modules/$(uname -r)/build M=$(pwd) modules
make: Entering directory `/usr/src/linux-headers-3.17.0-031700rc7-generic'
   CC [M]  /root/linux-3.13.0/drivers/net/ethernet/realtek/8139cp.o
   CC [M]  /root/linux-3.13.0/drivers/net/ethernet/realtek/8139too.o
   CC [M]  /root/linux-3.13.0/drivers/net/ethernet/realtek/atp.o
   CC [M]  /root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.o
/root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c: In function 
'rtl_init_one':
/root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.c:7127:2: error: 
implicit declaration of function 'SET_ETHTOOL_OPS' 
[-Werror=implicit-function-declaration]
   SET_ETHTOOL_OPS(dev, &rtl8169_ethtool_ops);
   ^
cc1: some warnings being treated as errors
make[1]: *** [/root/linux-3.13.0/drivers/net/ethernet/realtek/r8169.o] 
Error 1
make: *** [_module_/root/linux-3.13.0/drivers/net/ethernet/realtek] Error 2
make: Leaving directory `/usr/src/linux-headers-3.17.0-031700rc7-generic'

Any new suggestion how to honour your patch/test request?

Francois Romieu schreef op 09-10-14 om 00:17:
> Ceriel Jacobs <linux-ide@crashplan.pro> :
> [...]
>> Francois, could you help me apply your patch?
>
> $ git describe
> v3.13
> $ patch -p1 --dry-run < /tmp/plop
> checking file drivers/net/ethernet/realtek/r8169.c
> Hunk #1 succeeded at 467 (offset -1 lines).
> Hunk #2 succeeded at 5275 (offset -5 lines).
>
> $ git rev-list v3.16 -- drivers/net/ethernet/realtek/r8169.c | while read x; do echo $x:$(git cat-file -p $x:drivers/net/ethernet/realtek/r8169.c | md5sum); done | grep 95056b56932b375f8b65a6379009f704
>
> -> oualou
>
> Where does your 3.13 source tree come from ?
>

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Francois Romieu]

Ceriel Jacobs <linux-ide@crashplan.pro> :
> The 3.13 source was installed using "apt-get source linux-source-3.13.0"
> 
> The patch hunk#2 failing issue is something in the "heredoc" notation (me
> trying to apply the patch without storing the patch lines as temporary input
> file). I am now using the temp file without any issue.
> 
> The next newly blocking issue is this:
> 
> # make -C /lib/modules/$(uname -r)/build M=$(pwd) modules
> make: Entering directory `/usr/src/linux-headers-3.17.0-031700rc7-generic'

You are mixing 3.13 and 3.17. Try 'uname -r' alone. Got it ?

Please get some pristine 3.17 sources - they're available on www.kernel.org -
and try again.

-- 
Ueimor

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Ceriel Jacobs]

Francois Romieu schreef op 09-10-14 om 01:26:
 > You are mixing 3.13 and 3.17. Try 'uname -r' alone. Got it ?
I think I have got it.

Francois Romieu schreef op 07-10-14 om 00:13:
 > Ceriel, does the patch below against current kernel make a difference?

Old r8169 "powertop --auto-tune && powertop" result:
C2 (pc2)   26.1%    |                     |
C3 (pc3)   73.6%    | C3 (cc3)    1.6%    | C3-HSW      1.6%  108.0 ms
C6 (pc6)    0.0%    | C6 (cc6)   98.3%    | C6-HSW     98.3%  194.9 ms
---
C2 (pc2)    0.0%    |                     |
C3 (pc3)   99.4%    | C3 (cc3)    0.0%    | C3-HSW      0.0%    0.0 ms
C6 (pc6)    0.0%    | C6 (cc6)   99.6%    | C6-HSW     99.6%  237.4 ms
---

New r8169 "powertop" result (even without --auto-tune):
C2 (pc2)    0.0%    |                     |
C3 (pc3)    9.9%    | C3 (cc3)    0.7%    | C3-HSW      0.7%   16.4 ms
C6 (pc6)   89.9%    | C6 (cc6)   99.2%    | C6-HSW     99.2%  223.2 ms
---

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference?

[Francois Romieu]

Ceriel Jacobs <linux-ide@crashplan.pro> :
> Francois Romieu schreef op 07-10-14 om 00:13:
> > Ceriel, does the patch below against current kernel make a difference?
[...]
> New r8169 "powertop" result (even without --auto-tune):
> C2 (pc2)    0.0%    |                     |
> C3 (pc3)    9.9%    | C3 (cc3)    0.7%    | C3-HSW      0.7%   16.4 ms
> C6 (pc6)   89.9%    | C6 (cc6)   99.2%    | C6-HSW     99.2%  223.2 ms
> ---

Fine (almost: I hope that ASPM was enabled from bios or during boot
behind your back).

Remember your "lspci -nnkvv -s 03:00.0" ?

03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 11)
[...]
        Capabilities: [70] Express (v2) Endpoint, MSI 01
                DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
                        ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
                DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
                        RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
                        MaxPayload 128 bytes, MaxReadReq 4096 bytes
                DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
                LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us
                   	ClockPM+ Surprise- LLActRep- BwNot-
                LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
                        ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-

It should now look like:
[...]
                LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+

Let's temporarily disable it and see if powertop notices a difference.

<full disclosure>

"Capabilities: [70]" above gives you the offset of the relevant registers:
# lspci -xxx -s 03:00.0
[...]
70: .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
^^ -> "[70]"

You are interested in the Link Control register, aka PCI_EXP_LNKCTL in
/usr/include/pci/header.h (devel part of pciutils) or kernel's
include/uapi/linux/pci_regs.h. It's 16 bytes further, thus
[...]
70: .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
80: 42 .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
    ^^

0x42 matches "LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+
ExtSynch-" built from above. There may be differences but the 3 lower
weight binary digits in 0x42 encode ASPM control (0=nada, 1=L0, 2=L1,
see PCI_EXP_LNKCTL_ASPxyz in include/uapi/linux/pci_regs.h). Mask these
out (0x42 & ~0x03) and feed the resulting value back into the Link
Control register:

# setpci -s 03:00.0 CAP_EXP+10.b=0x40

(CAP_EXP is pciutils's alias for the PCI Express capability block, see
PCI_CAP_ID_EXP in kernel's include/uapi/linux/pci_regs.h)

If you are not too sure about the 0x40 value, you can retrieve it with
lspci and an unpatched r8169 driver.

</full disclosure>

If I have understood Hayes correctly and he got my question right, lspci
should now tell that ASPM is disabled. C6 should not be reached anymore.

ASPM could thus be enabled unconditionally at the driver level, then
controled through the PCI config registers. Kernel r8169 driver would
thus protect polar bears as Realtek's own r8168 driver already does.

I can't exclude that it will fail miserably in a firework of smelly
smoke though.

-- 
Ueimor

Re: r8168 is needed to enter P-state: Package State 6 (pc6)onHaswell hardware: does the patch below against current kernel make a difference? Yes, it does

[Ceriel Jacobs]

ASPM is enabled in the BIOS as far as possible.
ASPM is also enabled using kernel parameters:
1. pcie_aspm.policy=powersave
2. pcie_aspm=force

Despite the result for 03:00.0 (and 2 other PCIe devices) is:
LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+


I have filed a bug report for that at:
https://bugzilla.kernel.org/show_bug.cgi?id=85621


In my testing before, I did manually enable L0s L1 ASPM after login 
prompt using:
# setpci -s 03:00.0 0x80.B=0x43 (also for 3 other ASPM PCIe devices)

=============================
Now back to your Realtek test request
=============================
For your information: I am running all testing remote via SSH over the 
Realtek NIc.

=======
L0s and L1 = PC6
=======
# setpci -s 03:00.0 0x80.B=0x43
# lspci -vvvv -s 03:00.0 | grep LnkCtl:
		LnkCtl:	ASPM L0s L1 Enabled; RCB 64 bytes Disabled- ...

PC6 reached in 1 powertop screen update (5 second interval)
C3 (pc3)   21.0%    | C3 (cc3)    0.0%    | C3-HSW      0.0%    0.0 ms
C6 (pc6)   76.0%    | C6 (cc6)   99.9%    | C6-HSW     99.9%  198.1 ms

=======
L1 only = no PC6
=======
# setpci -s 03:00.0 0x80.B=0x42
		LnkCtl:	ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+

C3 (pc3)   99.8%    | C3 (cc3)    0.0%    | C3-HSW      0.0%    0.0 ms
C6 (pc6)    0.0%    | C6 (cc6)   99.9%    | C6-HSW    100.0%  206.4 ms

=======
L0s only = PC6 !!
=======
# setpci -s 03:00.0 0x80.B=0x41
		LnkCtl:	ASPM L0s Enabled; RCB 64 bytes Disabled- ...

C3 (pc3)   10.6%    | C3 (cc3)    0.0%    | C3-HSW      0.0%    0.0 ms
C6 (pc6)   89.1%    | C6 (cc6)   99.9%    | C6-HSW     99.9%  210.7 ms

=======
Disabled = no PC6
=======
# setpci -s 03:00.0 0x80.B=0x40
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+

C3 (pc3)   99.8%    | C3 (cc3)    0.0%    | C3-HSW      0.0%    0.0 ms
C6 (pc6)    0.0%    | C6 (cc6)   99.9%    | C6-HSW     99.9%  230.1 ms

Now I am wondering whether it is by design or a bug that PC6 is not 
reached when only L1 ASPM is activated

An applause for:
1. saving "polar bears",
2. letting me test a little more,
3. realtime behavior changing (no module unloading and loading to active 
ASPM, just set the config space registers).

Francois Romieu schreef op 10-10-14 om 00:14:
> Ceriel Jacobs <linux-ide@crashplan.pro> :
>> Francois Romieu schreef op 07-10-14 om 00:13:
>>> Ceriel, does the patch below against current kernel make a difference?
> [...]
>> New r8169 "powertop" result (even without --auto-tune):
>> C2 (pc2)    0.0%    |                     |
>> C3 (pc3)    9.9%    | C3 (cc3)    0.7%    | C3-HSW      0.7%   16.4 ms
>> C6 (pc6)   89.9%    | C6 (cc6)   99.2%    | C6-HSW     99.2%  223.2 ms
>> ---
>
> Fine (almost: I hope that ASPM was enabled from bios or during boot
> behind your back).
>
> Remember your "lspci -nnkvv -s 03:00.0" ?
>
> 03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 11)
> [...]
>          Capabilities: [70] Express (v2) Endpoint, MSI 01
>                  DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
>                          ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
>                  DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
>                          RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
>                          MaxPayload 128 bytes, MaxReadReq 4096 bytes
>                  DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
>                  LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us
>                     	ClockPM+ Surprise- LLActRep- BwNot-
>                  LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk+
>                          ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
>
> It should now look like:
> [...]
>                  LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+
>
> Let's temporarily disable it and see if powertop notices a difference.
>
> <full disclosure>
>
> "Capabilities: [70]" above gives you the offset of the relevant registers:
> # lspci -xxx -s 03:00.0
> [...]
> 70: .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
> ^^ -> "[70]"
>
> You are interested in the Link Control register, aka PCI_EXP_LNKCTL in
> /usr/include/pci/header.h (devel part of pciutils) or kernel's
> include/uapi/linux/pci_regs.h. It's 16 bytes further, thus
> [...]
> 70: .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
> 80: 42 .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
>      ^^
>
> 0x42 matches "LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk+
> ExtSynch-" built from above. There may be differences but the 3 lower
> weight binary digits in 0x42 encode ASPM control (0=nada, 1=L0, 2=L1,
> see PCI_EXP_LNKCTL_ASPxyz in include/uapi/linux/pci_regs.h). Mask these
> out (0x42 & ~0x03) and feed the resulting value back into the Link
> Control register:
>
> # setpci -s 03:00.0 CAP_EXP+10.b=0x40
>
> (CAP_EXP is pciutils's alias for the PCI Express capability block, see
> PCI_CAP_ID_EXP in kernel's include/uapi/linux/pci_regs.h)
>
> If you are not too sure about the 0x40 value, you can retrieve it with
> lspci and an unpatched r8169 driver.
>
> </full disclosure>
>
> If I have understood Hayes correctly and he got my question right, lspci
> should now tell that ASPM is disabled. C6 should not be reached anymore.
>
> ASPM could thus be enabled unconditionally at the driver level, then
> controled through the PCI config registers. Kernel r8169 driver would
> thus protect polar bears as Realtek's own r8168 driver already does.
>
> I can't exclude that it will fail miserably in a firework of smelly
> smoke though.
>