[net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[wojackbb]

From: Jack Wu <wojackbb@gmail.com>

Because optimizing the power consumption of t7XX,
change auto suspend time to 5000.

The Tests uses a script to loop through the power_state
of t7XX.
(for example: /sys/bus/pci/devices/0000\:72\:00.0/power_state)

* If Auto suspend is 20 seconds,
  test script show power_state have 0~5% of the time was in D3 state
  when host don't have data packet transmission.

* Changed auto suspend time to 5 seconds,
  test script show power_state have 50%~80% of the time was in D3 state
  when host don't have data packet transmission.

We tested Fibocom FM350 and our products using the t7xx and they all
benefited from this.

Signed-off-by: Jack Wu <wojackbb@gmail.com>
---
V2:
 * supplementary commit information
---
---
 drivers/net/wwan/t7xx/t7xx_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
index e556e5bd49ab..dcadd615a025 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.c
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -48,7 +48,7 @@
 #define T7XX_INIT_TIMEOUT		20
 #define PM_SLEEP_DIS_TIMEOUT_MS		20
 #define PM_ACK_TIMEOUT_MS		1500
-#define PM_AUTOSUSPEND_MS		20000
+#define PM_AUTOSUSPEND_MS		5000
 #define PM_RESOURCE_POLL_TIMEOUT_US	10000
 #define PM_RESOURCE_POLL_STEP_US	100
 
-- 
2.34.1

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Andrew Lunn]

On Thu, Nov 14, 2024 at 06:20:02PM +0800, wojackbb@gmail.com wrote:
> From: Jack Wu <wojackbb@gmail.com>
> 
> Because optimizing the power consumption of t7XX,
> change auto suspend time to 5000.
> 
> The Tests uses a script to loop through the power_state
> of t7XX.
> (for example: /sys/bus/pci/devices/0000\:72\:00.0/power_state)
> 
> * If Auto suspend is 20 seconds,
>   test script show power_state have 0~5% of the time was in D3 state
>   when host don't have data packet transmission.
> 
> * Changed auto suspend time to 5 seconds,
>   test script show power_state have 50%~80% of the time was in D3 state
>   when host don't have data packet transmission.
> 
> We tested Fibocom FM350 and our products using the t7xx and they all
> benefited from this.
> 
> Signed-off-by: Jack Wu <wojackbb@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Sergey Ryazanov]

Hello Jack,

On 14.11.2024 12:20, wojackbb@gmail.com wrote:
> From: Jack Wu <wojackbb@gmail.com>
> 
> Because optimizing the power consumption of t7XX,
> change auto suspend time to 5000.
> 
> The Tests uses a script to loop through the power_state
> of t7XX.
> (for example: /sys/bus/pci/devices/0000\:72\:00.0/power_state)
> 
> * If Auto suspend is 20 seconds,
>    test script show power_state have 0~5% of the time was in D3 state
>    when host don't have data packet transmission.
> 
> * Changed auto suspend time to 5 seconds,
>    test script show power_state have 50%~80% of the time was in D3 state
>    when host don't have data packet transmission.
> 
> We tested Fibocom FM350 and our products using the t7xx and they all
> benefited from this.

Possible negative outcomes for data transmission still need 
clarification. Let me repeat it here.

On 06.11.2024 13:10, 吳逼逼 wrote:
> Receiving or sending data will cause PCIE to change D3 Cold to D0 state.

Am I understand it correctly that receiving IP packets on downlink will 
cause PCIe link re-activation?


I am concerned about a TCP connection that can be idle for a long period 
of time. For example, an established SSH connection can stay idle for 
minutes. If I connected to a server and execute something like this:

user@host$ sleep 20 && echo "Done"

Will I eventually see the "Done" message or will the autosuspended modem 
effectively block any incoming traffic? And how long does it take for 
the modem to wake up and deliver a downlink packet to the host? Have you 
measured StDev change?

> Signed-off-by: Jack Wu <wojackbb@gmail.com>
> ---
> V2:
>   * supplementary commit information
> ---
> ---
>   drivers/net/wwan/t7xx/t7xx_pci.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
> index e556e5bd49ab..dcadd615a025 100644
> --- a/drivers/net/wwan/t7xx/t7xx_pci.c
> +++ b/drivers/net/wwan/t7xx/t7xx_pci.c
> @@ -48,7 +48,7 @@
>   #define T7XX_INIT_TIMEOUT		20
>   #define PM_SLEEP_DIS_TIMEOUT_MS		20
>   #define PM_ACK_TIMEOUT_MS		1500
> -#define PM_AUTOSUSPEND_MS		20000
> +#define PM_AUTOSUSPEND_MS		5000
>   #define PM_RESOURCE_POLL_TIMEOUT_US	10000
>   #define PM_RESOURCE_POLL_STEP_US	100
>   

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Jakub Kicinski]

On Thu, 14 Nov 2024 20:54:20 +0200 Sergey Ryazanov wrote:
> > We tested Fibocom FM350 and our products using the t7xx and they all
> > benefited from this.  
> 
> Possible negative outcomes for data transmission still need 
> clarification. Let me repeat it here.
> 
> On 06.11.2024 13:10, 吳逼逼 wrote:
> > Receiving or sending data will cause PCIE to change D3 Cold to D0 state.  
> 
> Am I understand it correctly that receiving IP packets on downlink will 
> cause PCIe link re-activation?
> 
> 
> I am concerned about a TCP connection that can be idle for a long period 
> of time. For example, an established SSH connection can stay idle for 
> minutes. If I connected to a server and execute something like this:
> 
> user@host$ sleep 20 && echo "Done"
> 
> Will I eventually see the "Done" message or will the autosuspended modem 
> effectively block any incoming traffic? And how long does it take for 
> the modem to wake up and deliver a downlink packet to the host? Have you 
> measured StDev change?

He's decreasing the sleep timer from 20 to 5 sec, both of which 
are very high for networking, anyway. You appear to be questioning
autosuspend itself but it seems to have been added 2 years ago already.

What am I missing?

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Sergey Ryazanov]

On 16.11.2024 01:21, Jakub Kicinski wrote:
> On Thu, 14 Nov 2024 20:54:20 +0200 Sergey Ryazanov wrote:
>>> We tested Fibocom FM350 and our products using the t7xx and they all
>>> benefited from this.
>>
>> Possible negative outcomes for data transmission still need
>> clarification. Let me repeat it here.
>>
>> On 06.11.2024 13:10, 吳逼逼 wrote:
>>> Receiving or sending data will cause PCIE to change D3 Cold to D0 state.
>>
>> Am I understand it correctly that receiving IP packets on downlink will
>> cause PCIe link re-activation?
>>
>>
>> I am concerned about a TCP connection that can be idle for a long period
>> of time. For example, an established SSH connection can stay idle for
>> minutes. If I connected to a server and execute something like this:
>>
>> user@host$ sleep 20 && echo "Done"
>>
>> Will I eventually see the "Done" message or will the autosuspended modem
>> effectively block any incoming traffic? And how long does it take for
>> the modem to wake up and deliver a downlink packet to the host? Have you
>> measured StDev change?
> 
> He's decreasing the sleep timer from 20 to 5 sec, both of which
> are very high for networking, anyway. You appear to be questioning
> autosuspend itself but it seems to have been added 2 years ago already.
> 
> What am I missing?

Some possible funny side-effect of sleeping with this chipset. Like 
loosing network connection and dropping TCP sessions. I hope that 20 
seconds was putted on purpose.

Suddenly, I don't have this modem at hand and want to be sure that we 
are not going to receive a stream of bug reports.

--
Sergey

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Jakub Kicinski]

On Tue, 19 Nov 2024 03:01:47 +0200 Sergey Ryazanov wrote:
> > He's decreasing the sleep timer from 20 to 5 sec, both of which
> > are very high for networking, anyway. You appear to be questioning
> > autosuspend itself but it seems to have been added 2 years ago already.
> > 
> > What am I missing?  
> 
> Some possible funny side-effect of sleeping with this chipset. Like 
> loosing network connection and dropping TCP sessions. I hope that 20 
> seconds was putted on purpose.
> 
> Suddenly, I don't have this modem at hand and want to be sure that we 
> are not going to receive a stream of bug reports.

Power saving is always tricky, but they say they tested. I think we
should give it a go, worst case - it will be an easy revert.

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[Sergey Ryazanov]

On 19.11.2024 03:44, Jakub Kicinski wrote:
> On Tue, 19 Nov 2024 03:01:47 +0200 Sergey Ryazanov wrote:
>>> He's decreasing the sleep timer from 20 to 5 sec, both of which
>>> are very high for networking, anyway. You appear to be questioning
>>> autosuspend itself but it seems to have been added 2 years ago already.
>>>
>>> What am I missing?
>>
>> Some possible funny side-effect of sleeping with this chipset. Like
>> loosing network connection and dropping TCP sessions. I hope that 20
>> seconds was putted on purpose.
>>
>> Suddenly, I don't have this modem at hand and want to be sure that we
>> are not going to receive a stream of bug reports.
> 
> Power saving is always tricky, but they say they tested. I think we
> should give it a go, worst case - it will be an easy revert.

Make sense. Then let's merge it.

Reviewed-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

Re: [net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 14 Nov 2024 18:20:02 +0800 you wrote:
> From: Jack Wu <wojackbb@gmail.com>
> 
> Because optimizing the power consumption of t7XX,
> change auto suspend time to 5000.
> 
> The Tests uses a script to loop through the power_state
> of t7XX.
> (for example: /sys/bus/pci/devices/0000\:72\:00.0/power_state)
> 
> [...]

Here is the summary with links:
  - [net-next,v2,net-next,v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000
    https://git.kernel.org/netdev/net-next/c/a0c80d5108ab

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[net-next,v2] [PATCH net-next v2] net: wwan: t7xx: Change PM_AUTOSUSPEND_MS to 5000

[jackbb_wu]

From: Jack Wu <wojackbb@gmail.com>

Because optimizing the power consumption of t7XX,
change auto suspend time to 5000.

The Tests uses a script to loop through the power_state
of t7XX.
(for example: /sys/bus/pci/devices/0000\:72\:00.0/power_state)

* If Auto suspend is 20 seconds,
  test script show power_state have 0~5% of the time was in D3 state
  when host don't have data packet transmission.

* Changed auto suspend time to 5 seconds,
  test script show power_state have 50%~80% of the time was in D3 state
  when host don't have data packet transmission.

We tested Fibocom FM350 and our products using the t7xx and they all
benefited from this.

Signed-off-by: Jack Wu <wojackbb@gmail.com>
---
V2:
 * supplementary commit information
---
---
 drivers/net/wwan/t7xx/t7xx_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c
index e556e5bd49ab..dcadd615a025 100644
--- a/drivers/net/wwan/t7xx/t7xx_pci.c
+++ b/drivers/net/wwan/t7xx/t7xx_pci.c
@@ -48,7 +48,7 @@
 #define T7XX_INIT_TIMEOUT              20
 #define PM_SLEEP_DIS_TIMEOUT_MS                20
 #define PM_ACK_TIMEOUT_MS              1500
-#define PM_AUTOSUSPEND_MS              20000
+#define PM_AUTOSUSPEND_MS              5000
 #define PM_RESOURCE_POLL_TIMEOUT_US    10000
 #define PM_RESOURCE_POLL_STEP_US       100

--
2.34.1



================================================================================================================================================================
This message may contain information which is private, privileged or confidential of Compal Electronics, Inc. If you are not the intended recipient of this message, please notify the sender and destroy/delete the message. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon this information, by persons or entities other than the intended recipient is prohibited.
================================================================================================================================================================