Hi Yu, > On Nov 24, 2020, at 23:32, Chen Yu <yu.c.c...@intel.com> wrote: > > The NIC is put in runtime suspend status when there is no wire connected. > As a result, it is safe to keep this NIC in runtime suspended during s2ram > because the system does not rely on the NIC plug event nor WOL to wake up > the system. Unlike the s2idle, s2ram does not need to manipulate S0ix settings > during suspend.
Please see below for the reason why I explicitly disable direct-complete in the driver. > > This patch assigns DPM_FLAG_SMART_SUSPEND and DPM_FLAG_MAY_SKIP_RESUME > to the e1000e driver so that the s2ram could skip the .suspend_late(), > .suspend_noirq() and .resume_noirq() .resume_early() when possible. > Also skip .suspend() and .resume() if dev_pm_skip_suspend() and > dev_pm_skip_resume() return true, so as to speed up the s2ram. If we really want direct-complete here, maybe always set current WoL setting in runtime suspend routine? > > Signed-off-by: Chen Yu <yu.c.c...@intel.com> > --- > drivers/base/power/main.c | 2 ++ > drivers/net/ethernet/intel/e1000e/netdev.c | 14 +++++++++++++- > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c > index c7ac49042cee..9cd8abba8612 100644 > --- a/drivers/base/power/main.c > +++ b/drivers/base/power/main.c > @@ -580,6 +580,7 @@ bool dev_pm_skip_resume(struct device *dev) > > return !dev->power.must_resume; > } > +EXPORT_SYMBOL_GPL(dev_pm_skip_resume); I don't think it's a good idea to use this predicate out side of PM core, must_resume may change during suspend process. > > /** > * device_resume_noirq - Execute a "noirq resume" callback for given device. > @@ -2010,3 +2011,4 @@ bool dev_pm_skip_suspend(struct device *dev) > return dev_pm_test_driver_flags(dev, DPM_FLAG_SMART_SUSPEND) && > pm_runtime_status_suspended(dev); > } > +EXPORT_SYMBOL_GPL(dev_pm_skip_suspend); > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c > b/drivers/net/ethernet/intel/e1000e/netdev.c > index b30f00891c03..d79fddabc553 100644 > --- a/drivers/net/ethernet/intel/e1000e/netdev.c > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c > @@ -6965,6 +6965,14 @@ static __maybe_unused int e1000e_pm_suspend(struct > device *dev) > struct e1000_hw *hw = &adapter->hw; > int rc; > > + /* Runtime suspended means that there is no wired connection > + * and it has nothing to do with WOL that, we don't need to > + * adjust the WOL settings. So it is safe to put NIC in > + * runtime suspend while doing system suspend. > + */ What about plugging ethernet cable and using WoL after system is suspended? Commit "e1000e: Exclude device from suspend direct complete optimization" was to address that scenario. > + if (dev_pm_skip_suspend(dev)) > + return 0; > + > e1000e_flush_lpic(pdev); > > e1000e_pm_freeze(dev); > @@ -6989,6 +6997,9 @@ static __maybe_unused int e1000e_pm_resume(struct > device *dev) > struct e1000_hw *hw = &adapter->hw; > int rc; > > + if (dev_pm_skip_resume(dev)) > + return 0; > + > /* Introduce S0ix implementation */ > if (hw->mac.type >= e1000_pch_cnp && > !e1000e_check_me(hw->adapter->pdev->device)) > @@ -7665,7 +7676,8 @@ static int e1000_probe(struct pci_dev *pdev, const > struct pci_device_id *ent) > > e1000_print_device_info(adapter); > > - dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE); > + dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NO_DIRECT_COMPLETE | > + DPM_FLAG_SMART_SUSPEND | > DPM_FLAG_MAY_SKIP_RESUME); > > if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp) > pm_runtime_put_noidle(&pdev->dev); Also, most e1000e device on modern platforms doesn't runtime suspend at all after commit "e1000e: Disable runtime PM on CNP+". Kai-Heng > -- > 2.25.1 >
