Re: [PATCH] e1000e: Assign DPM_FLAG_SMART_SUSPEND and DPM_FLAG_MAY_SKIP_RESUME to speed up s2ram

[Chen Yu]

On Fri, Nov 27, 2020 at 08:20:17PM +0800, Kai-Heng Feng wrote:
> Thanks for the confirmation. How about a different approach? 
> Simply use direct-complete to let PM core handle the rest:
>
Thanks for your suggestion and sorry about replying too late.
Yes, using direct-complete could leverage pm core to do that,
although this is a little different than my original thought
to mainly skip the resume process. But anyway, I'll use direct
complete and send a v3 out.
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index b30f00891c03..1d1424a20733 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -25,6 +25,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/aer.h>
>  #include <linux/prefetch.h>
> +#include <linux/suspend.h>
>  
>  #include "e1000.h"
>  
> @@ -6868,6 +6869,20 @@ static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state)
>  	__e1000e_disable_aspm(pdev, state, 1);
>  }
>  
> +static int e1000e_pm_prepare(struct device *dev)
> +{
> +	return pm_runtime_suspended(dev) &&
> +	       pm_suspend_via_firmware() &&
> +	       !device_may_wakeup(dev);
> +}
device_may_wakeup() is not needed as pm core will check it anyway.
> +
> +static void e1000e_pm_complete(struct device *dev)
> +{
> +	/* Detect link change */
> +	if (pm_runtime_suspended(dev))
> +		pm_request_resume(dev);
> +}
There is no need to force resume the device, just keep it
runtime suspended would be okay. Besides the pm core's complete()
will restore runtime usage_count which is increased by pm core's
prepare(), so there is no need to do that here again.
> +
>  static int e1000e_pm_thaw(struct device *dev)
>  {
>  	struct net_device *netdev = dev_get_drvdata(dev);
> @@ -7665,9 +7680,7 @@ 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);
> -
DPM_FLAG_SMART_PREPARE must be set otherwise pci subsystem will ignore the
.prepare() from the driver.
> -	if (pci_dev_run_wake(pdev) && hw->mac.type < e1000_pch_cnp)
> +	if (pci_dev_run_wake(pdev))
>  		pm_runtime_put_noidle(&pdev->dev);
>
I would prefer to only disable runtime for cnp, in case of
any user regressions.

thanks,
Chenyu