Re: [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP

2020-04-13 Thread Jeff Kirsher
On Fri, Apr 10, 2020 at 9:03 AM Rafael J. Wysocki  wrote:
>
> From: "Rafael J. Wysocki" 
>
> Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
> matches its purpose more closely.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki 

Acked-by: Jeff Kirsher 

For the driver changes to e1000e, igb and igc.

> ---
>  Documentation/driver-api/pm/devices.rst|  6 +++---
>  Documentation/power/pci.rst| 10 +-
>  drivers/base/power/main.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c|  2 +-
>  drivers/gpu/drm/radeon/radeon_kms.c|  2 +-
>  drivers/misc/mei/pci-me.c  |  2 +-
>  drivers/misc/mei/pci-txe.c |  2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
>  drivers/pci/pcie/portdrv_pci.c |  2 +-
>  include/linux/pm.h |  6 +++---
>  13 files changed, 21 insertions(+), 21 deletions(-)
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP

2020-04-10 Thread Bjorn Helgaas
On Fri, Apr 10, 2020 at 05:56:13PM +0200, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki" 
> 
> Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
> matches its purpose more closely.
> 
> No functional impact.
> 
> Signed-off-by: Rafael J. Wysocki 

Acked-by: Bjorn Helgaas  # for PCI parts

> ---
>  Documentation/driver-api/pm/devices.rst|  6 +++---
>  Documentation/power/pci.rst| 10 +-
>  drivers/base/power/main.c  |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  2 +-
>  drivers/gpu/drm/i915/intel_runtime_pm.c|  2 +-
>  drivers/gpu/drm/radeon/radeon_kms.c|  2 +-
>  drivers/misc/mei/pci-me.c  |  2 +-
>  drivers/misc/mei/pci-txe.c |  2 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
>  drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
>  drivers/pci/pcie/portdrv_pci.c |  2 +-
>  include/linux/pm.h |  6 +++---
>  13 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/driver-api/pm/devices.rst 
> b/Documentation/driver-api/pm/devices.rst
> index f66c7b9126ea..4ace0eba4506 100644
> --- a/Documentation/driver-api/pm/devices.rst
> +++ b/Documentation/driver-api/pm/devices.rst
> @@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, 
> ``suspend_late``, ``suspend_noirq``.
>   runtime PM disabled.

Minor question about a preceding paragraph that ends:

  In that case, the ``->complete`` callback will be invoked directly
  after the ``->prepare`` callback and is entirely responsible for
  putting the device into a consistent state as appropriate.

What does" a consistent state as appropriate" mean?  I know this is
generic documentation at a high level, so maybe there's no good
explanation for "consistent state," but I don't know what to imagine
there.

And what does "as appropriate" mean?  Would it change the meaning to
drop those two words, or are there situations where it's not
appropriate to put the device into a consistent state?  Or maybe it's
just that the type of device determines what the consistent state is?

>   This feature also can be controlled by device drivers by using the
> - ``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
> - management flags.  [Typically, they are set at the time the driver is
> - probed against the device in question by passing them to the
> + ``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
> + power management flags.  [Typically, they are set at the time the driver
> + is probed against the device in question by passing them to the
>   :c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
>   these flags is set, the PM core will not apply the direct-complete
>   procedure described above to the given device and, consequenty, to any

s/consequenty/consequently/

Drive-by comment: I looked for a definition of "direct-complete".  The
closest I found is a couple paragraphs above this, where it says "Note
that this direct-complete procedure ...," but that leaves me to try to
reconstruct the definition from the preceding text.

AFAICT, going to freeze, standby, or memory sleep includes these
callbacks:

  ->prepare
  ->suspend
  ->suspend_late
  ->suspend_noirq
  ->complete (not mentioned in the list of phases)

And "direct-complete" means we skip the suspend, suspend_late,
and suspend_noirq callbacks so we only use these:

  ->prepare
  ->complete

And apparently we skip those callbacks for device X if ->prepare() for
X and all its descendents returns a positive value AND they are all
runtime-suspended, except if a driver for X or a descendent sets
DPM_FLAG_NO_DIRECT_COMPLETE.

Bjorn
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 5/7] PM: sleep: core: Rename DPM_FLAG_NEVER_SKIP

2020-04-10 Thread Rafael J. Wysocki
From: "Rafael J. Wysocki" 

Rename DPM_FLAG_NEVER_SKIP to DPM_FLAG_NO_DIRECT_COMPLETE which
matches its purpose more closely.

No functional impact.

Signed-off-by: Rafael J. Wysocki 
---
 Documentation/driver-api/pm/devices.rst|  6 +++---
 Documentation/power/pci.rst| 10 +-
 drivers/base/power/main.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|  2 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c|  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c|  2 +-
 drivers/misc/mei/pci-me.c  |  2 +-
 drivers/misc/mei/pci-txe.c |  2 +-
 drivers/net/ethernet/intel/e1000e/netdev.c |  2 +-
 drivers/net/ethernet/intel/igb/igb_main.c  |  2 +-
 drivers/net/ethernet/intel/igc/igc_main.c  |  2 +-
 drivers/pci/pcie/portdrv_pci.c |  2 +-
 include/linux/pm.h |  6 +++---
 13 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/driver-api/pm/devices.rst 
b/Documentation/driver-api/pm/devices.rst
index f66c7b9126ea..4ace0eba4506 100644
--- a/Documentation/driver-api/pm/devices.rst
+++ b/Documentation/driver-api/pm/devices.rst
@@ -361,9 +361,9 @@ the phases are: ``prepare``, ``suspend``, ``suspend_late``, 
``suspend_noirq``.
runtime PM disabled.
 
This feature also can be controlled by device drivers by using the
-   ``DPM_FLAG_NEVER_SKIP`` and ``DPM_FLAG_SMART_PREPARE`` driver power
-   management flags.  [Typically, they are set at the time the driver is
-   probed against the device in question by passing them to the
+   ``DPM_FLAG_NO_DIRECT_COMPLETE`` and ``DPM_FLAG_SMART_PREPARE`` driver
+   power management flags.  [Typically, they are set at the time the driver
+   is probed against the device in question by passing them to the
:c:func:`dev_pm_set_driver_flags` helper function.]  If the first of
these flags is set, the PM core will not apply the direct-complete
procedure described above to the given device and, consequenty, to any
diff --git a/Documentation/power/pci.rst b/Documentation/power/pci.rst
index aa1c7fce6cd0..9e1408121bea 100644
--- a/Documentation/power/pci.rst
+++ b/Documentation/power/pci.rst
@@ -1004,11 +1004,11 @@ including the PCI bus type.  The flags should be set 
once at the driver probe
 time with the help of the dev_pm_set_driver_flags() function and they should 
not
 be updated directly afterwards.
 
-The DPM_FLAG_NEVER_SKIP flag prevents the PM core from using the 
direct-complete
-mechanism allowing device suspend/resume callbacks to be skipped if the device
-is in runtime suspend when the system suspend starts.  That also affects all of
-the ancestors of the device, so this flag should only be used if absolutely
-necessary.
+The DPM_FLAG_NO_DIRECT_COMPLETE flag prevents the PM core from using the
+direct-complete mechanism allowing device suspend/resume callbacks to be 
skipped
+if the device is in runtime suspend when the system suspend starts.  That also
+affects all of the ancestors of the device, so this flag should only be used if
+absolutely necessary.
 
 The DPM_FLAG_SMART_PREPARE flag instructs the PCI bus type to only return a
 positive value from pci_pm_prepare() if the ->prepare callback provided by the
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 21187ee37b22..aa9c8df9fc4b 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1850,7 +1850,7 @@ static int device_prepare(struct device *dev, 
pm_message_t state)
spin_lock_irq(>power.lock);
dev->power.direct_complete = state.event == PM_EVENT_SUSPEND &&
(ret > 0 || dev->power.no_pm_callbacks) &&
-   !dev_pm_test_driver_flags(dev, DPM_FLAG_NEVER_SKIP);
+   !dev_pm_test_driver_flags(dev, DPM_FLAG_NO_DIRECT_COMPLETE);
spin_unlock_irq(>power.lock);
return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fd1dc3236eca..a9086ea1ab60 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -191,7 +191,7 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
}
 
if (adev->runpm) {
-   dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NEVER_SKIP);
+   dev_pm_set_driver_flags(dev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
pm_runtime_use_autosuspend(dev->dev);
pm_runtime_set_autosuspend_delay(dev->dev, 5000);
pm_runtime_set_active(dev->dev);
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c 
b/drivers/gpu/drm/i915/intel_runtime_pm.c
index ad719c9602af..9cb2d7548daa 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -549,7 +549,7 @@ void intel_runtime_pm_enable(struct intel_runtime_pm *rpm)
 * becaue the HDA driver may require us to enable the audio power
 * domain