| Message ID | 20240930031624.2116592-1-wojackbb@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: wwan: t7xx: add PM_AUTOSUSPEND_MS_BY_DW5933E for Dell DW5933e | expand |
On Mon, Sep 30, 2024 at 11:16:24AM +0800, wojackbb@gmail.com wrote: > From: Jack Wu <wojackbb@gmail.com> > > Because optimizing the power consumption of Dell DW5933e, > Add a new auto suspend time for Dell DW5933e. Please don't send new versions of a patch within 24 hours. https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq You commit message is not very good. What makes this machine special? What is wrong with the current code? Do you have any benchmark data to show the improvement? Andrew
On Tue, Oct 15, 2024 at 10:48:15AM +0800, 吳逼逼 wrote: > Hi Andrew, > > We have a power test that uses a small script to loop through the power_state > of Dell DW5933e.(/sys/bus/pci/devices/..../power_state) > > We expect that PCIE can enter the D3 state when Host don;t have data packet > transmission, > but the experimental result of the small script test is that it is only in the > D3 state about 5% of the time. > > We analyze logs to found that Dell DW5933e occasionally sends signal change > notifications, and ModemManager occasionally captures Modem status. > Although these situations are not very frequent, > However, since the default auto suspend time is 20 seconds, the chance of PCIE > being able to enter the D3 state is very small. > > After we changed auto suspend to 5 seconds, PCIE have 50% of the time was in D3 > state, which met Dell's requirements. So you answered some of my questions. But missed: What makes this machine special? It is maybe because this machine occasionally sends signal change notifications? There are modem status changes? Have you compared this to other machines with the same hardware? Do they do the same? Or is this Dell special? If it is special, why is it special? Andrew
Andrew Lunn <andrew@lunn.ch> 於 2024年10月15日 週二 下午8:09寫道： > > On Tue, Oct 15, 2024 at 10:48:15AM +0800, 吳逼逼 wrote: > > Hi Andrew, > > > > We have a power test that uses a small script to loop through the power_state > > of Dell DW5933e.(/sys/bus/pci/devices/..../power_state) > > > > We expect that PCIE can enter the D3 state when Host don;t have data packet > > transmission, > > but the experimental result of the small script test is that it is only in the > > D3 state about 5% of the time. > > > > We analyze logs to found that Dell DW5933e occasionally sends signal change > > notifications, and ModemManager occasionally captures Modem status. > > Although these situations are not very frequent, > > However, since the default auto suspend time is 20 seconds, the chance of PCIE > > being able to enter the D3 state is very small. > > > > After we changed auto suspend to 5 seconds, PCIE have 50% of the time was in D3 > > state, which met Dell's requirements. > > So you answered some of my questions. But missed: > > What makes this machine special? > > It is maybe because this machine occasionally sends signal change > notifications? There are modem status changes? > > Have you compared this to other machines with the same hardware? Do > they do the same? Or is this Dell special? If it is special, why is it > special? > > Andrew Hi Andrew, The chip of Fibocom FM350 is MTK T7XX, It is the same chip as our device. We tested the Fibocom FM350 and It had the same issue as our device. The following tests use the same environment and steps: a. Make data call to connect Internet b. No data is transferred to the Internet and wait one minute. c. use test script to capture and count power_state until one minute. Result: 1. When autosuspend_delay_ms is 20000, Our device's d3_cold time is 0% Fibocom FM350's d3_cold time 0% 2. When autosuspend_delay_ms is 5000, Our device's d3_cold time is 80% Fibocom FM350's d3_cold time 60% So this problem is a common problem. Should I remove PM_AUTOSUSPEND_MS_BY_DW5933E, and modify PM_AUTOSUSPEND_MS to 5000 at my patch? Thanks.
> Hi Andrew, > > The chip of Fibocom FM350 is MTK T7XX, > It is the same chip as our device. > > We tested the Fibocom FM350 and It had the same issue as our device. > The following tests use the same environment and steps: > a. Make data call to connect Internet > b. No data is transferred to the Internet and wait one minute. > c. use test script to capture and count power_state until one minute. > > Result: > 1. When autosuspend_delay_ms is 20000, > Our device's d3_cold time is 0% > Fibocom FM350's d3_cold time 0% > > 2. When autosuspend_delay_ms is 5000, > Our device's d3_cold time is 80% > Fibocom FM350's d3_cold time 60% > > So this problem is a common problem. > Should I remove PM_AUTOSUSPEND_MS_BY_DW5933E, > and modify PM_AUTOSUSPEND_MS to 5000 at my patch? A sample of two is not great, but does suggest there is nothing special about the DW5933E, and all users can benefit from this change. Please do change PM_AUTOSUSPEND_MS. Andrew
diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c index e556e5bd49ab..ec567153ea6e 100644 --- a/drivers/net/wwan/t7xx/t7xx_pci.c +++ b/drivers/net/wwan/t7xx/t7xx_pci.c @@ -49,6 +49,7 @@ #define PM_SLEEP_DIS_TIMEOUT_MS 20 #define PM_ACK_TIMEOUT_MS 1500 #define PM_AUTOSUSPEND_MS 20000 +#define PM_AUTOSUSPEND_MS_BY_DW5933E 5000 #define PM_RESOURCE_POLL_TIMEOUT_US 10000 #define PM_RESOURCE_POLL_STEP_US 100 @@ -174,7 +175,7 @@ static int t7xx_wait_pm_config(struct t7xx_pci_dev *t7xx_dev) return ret; } -static int t7xx_pci_pm_init(struct t7xx_pci_dev *t7xx_dev) +static int t7xx_pci_pm_init(struct t7xx_pci_dev *t7xx_dev, int pm_autosuspend_ms) { struct pci_dev *pdev = t7xx_dev->pdev; @@ -191,7 +192,7 @@ static int t7xx_pci_pm_init(struct t7xx_pci_dev *t7xx_dev) DPM_FLAG_NO_DIRECT_COMPLETE); iowrite32(T7XX_L1_BIT(0), IREG_BASE(t7xx_dev) + DISABLE_ASPM_LOWPWR); - pm_runtime_set_autosuspend_delay(&pdev->dev, PM_AUTOSUSPEND_MS); + pm_runtime_set_autosuspend_delay(&pdev->dev, pm_autosuspend_ms); pm_runtime_use_autosuspend(&pdev->dev); return 0; @@ -824,7 +825,13 @@ static int t7xx_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) IREG_BASE(t7xx_dev) = pcim_iomap_table(pdev)[T7XX_PCI_IREG_BASE]; t7xx_dev->base_addr.pcie_ext_reg_base = pcim_iomap_table(pdev)[T7XX_PCI_EREG_BASE]; - ret = t7xx_pci_pm_init(t7xx_dev); + if (id->vendor == 0x14c0 && id->device == 0x4d75) { + /* Dell DW5933e */ + ret = t7xx_pci_pm_init(t7xx_dev, PM_AUTOSUSPEND_MS_BY_DW5933E); + } else { + /* Other devices */ + ret = t7xx_pci_pm_init(t7xx_dev, PM_AUTOSUSPEND_MS); + } if (ret) return ret;