diff mbox series

rpm: pm: enable PM_RPM_EXCEPTION config flag

Message ID 20231031093921.755204-1-guanyulin@google.com (mailing list archive)
State Not Applicable, archived
Headers show
Series rpm: pm: enable PM_RPM_EXCEPTION config flag | expand

Commit Message

Guan-Yu Lin Oct. 31, 2023, 9:38 a.m. UTC
Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
between system power management and runtime power management. In
suspend-to-idle flow, PM core will suspend all devices to avoid device
interact with the system. However, chances are devices might be used by
other systems rather than a single system. In this case, PM core shouldn't
suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
such exception, and determine the power state of a device with runtime
power management rather than system power management.

Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
---
 drivers/usb/core/generic.c |  6 ++++++
 drivers/usb/core/usb.h     | 16 ++++++++++++++++
 kernel/power/Kconfig       |  8 ++++++++
 3 files changed, 30 insertions(+)

Comments

Greg Kroah-Hartman Oct. 31, 2023, 9:48 a.m. UTC | #1
On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> between system power management and runtime power management. In
> suspend-to-idle flow, PM core will suspend all devices to avoid device
> interact with the system. However, chances are devices might be used by
> other systems rather than a single system. In this case, PM core shouldn't
> suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> such exception, and determine the power state of a device with runtime
> power management rather than system power management.
> 
> Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
> ---
>  drivers/usb/core/generic.c |  6 ++++++
>  drivers/usb/core/usb.h     | 16 ++++++++++++++++
>  kernel/power/Kconfig       |  8 ++++++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> index 740342a2812a..bb0dfcfc9764 100644
> --- a/drivers/usb/core/generic.c
> +++ b/drivers/usb/core/generic.c
> @@ -266,6 +266,9 @@ int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
>  {
>  	int rc;
>  
> +	if (usb_runtime_pm_exception(udev))
> +		return 0;
> +
>  	/* Normal USB devices suspend through their upstream port.
>  	 * Root hubs don't have upstream ports to suspend,
>  	 * so we have to shut down their downstream HC-to-USB
> @@ -294,6 +297,9 @@ int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
>  {
>  	int rc;
>  
> +	if (usb_runtime_pm_exception(udev))
> +		return 0;
> +
>  	/* Normal USB devices resume/reset through their upstream port.
>  	 * Root hubs don't have upstream ports to resume or reset,
>  	 * so we have to start up their downstream HC-to-USB
> diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> index 60363153fc3f..14a054f814a2 100644
> --- a/drivers/usb/core/usb.h
> +++ b/drivers/usb/core/usb.h
> @@ -90,6 +90,22 @@ extern void usb_major_cleanup(void);
>  extern int usb_device_supports_lpm(struct usb_device *udev);
>  extern int usb_port_disable(struct usb_device *udev);
>  
> +#ifdef	CONFIG_PM_RPM_EXCEPTION
> +
> +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> +{
> +	return atomic_read(&udev->dev.power.usage_count);
> +}
> +
> +#else
> +
> +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> +{
> +	return 0;
> +}
> +
> +#endif
> +
>  #ifdef	CONFIG_PM
>  
>  extern int usb_suspend(struct device *dev, pm_message_t msg);
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4b31629c5be4..beba7a0f3947 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -193,6 +193,14 @@ config PM
>  	  responsible for the actual handling of device suspend requests and
>  	  wake-up events.
>  
> +config PM_RPM_EXCEPTION
> +	bool "Prioritize Runtime Power Management more than Power Management"
> +	default n

The default is always 'n' so no need to specify it.

> +	help
> +	Provides a way to prioritize Runtime Power Management more than Power
> +	Management. This way system can suspnd with maintaining specific
> +	components in operation.

This really doesn't give me a good description of why someone would ever
want to enable this at all.

And why does this have to be a build option?  That feels very heavy, why
not make it changable at runtime?

If this is a build option, how are you going to get all the distros and
all of the Android/ChromeOS systems in the world to enable it?

thanks,

greg k-h
Alan Stern Oct. 31, 2023, 2:39 p.m. UTC | #2
On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> between system power management and runtime power management. In
> suspend-to-idle flow, PM core will suspend all devices to avoid device

Your patch affects all forms of system suspend, not just 
suspend-to-idle.  What do you actually mean here?

> interact with the system. However, chances are devices might be used by
> other systems rather than a single system. In this case, PM core shouldn't
> suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> such exception, and determine the power state of a device with runtime
> power management rather than system power management.

This sort of arrangement -- a device shared between two different 
systems -- could happen with any sort of device.  Why does your patch 
affect only USB devices?

> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 4b31629c5be4..beba7a0f3947 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -193,6 +193,14 @@ config PM
>  	  responsible for the actual handling of device suspend requests and
>  	  wake-up events.
>  
> +config PM_RPM_EXCEPTION
> +	bool "Prioritize Runtime Power Management more than Power Management"

Runtime Power Management is a form of Power Management, so what you 
wrote doesn't make sense.  What you really meant is: Prioritize Runtime 
Power Management more than System Power Management.

> +	default n
> +	help
> +	Provides a way to prioritize Runtime Power Management more than Power
> +	Management. This way system can suspnd with maintaining specific

s/suspnd/suspend/
s/with/while/

> +	components in operation.

Your patch does not allow _specific_ components to be kept in operation.  
_All_ in-use components that support prioritized PM (with this patch, 
all USB components) will remain powered during system suspend, even if 
the user wants only _some_ of them to be kept powered.

Alan Stern

> +
>  config PM_DEBUG
>  	bool "Power Management Debug Support"
>  	depends on PM
> -- 
> 2.42.0.820.g83a721a137-goog
>
Guan-Yu Lin Nov. 8, 2023, 8:44 a.m. UTC | #3
On Tue, Oct 31, 2023 at 5:48 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> > Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> > between system power management and runtime power management. In
> > suspend-to-idle flow, PM core will suspend all devices to avoid device
> > interact with the system. However, chances are devices might be used by
> > other systems rather than a single system. In this case, PM core shouldn't
> > suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> > such exception, and determine the power state of a device with runtime
> > power management rather than system power management.
> >
> > Signed-off-by: Guan-Yu Lin <guanyulin@google.com>
> > ---
> >  drivers/usb/core/generic.c |  6 ++++++
> >  drivers/usb/core/usb.h     | 16 ++++++++++++++++
> >  kernel/power/Kconfig       |  8 ++++++++
> >  3 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> > index 740342a2812a..bb0dfcfc9764 100644
> > --- a/drivers/usb/core/generic.c
> > +++ b/drivers/usb/core/generic.c
> > @@ -266,6 +266,9 @@ int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
> >  {
> >       int rc;
> >
> > +     if (usb_runtime_pm_exception(udev))
> > +             return 0;
> > +
> >       /* Normal USB devices suspend through their upstream port.
> >        * Root hubs don't have upstream ports to suspend,
> >        * so we have to shut down their downstream HC-to-USB
> > @@ -294,6 +297,9 @@ int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
> >  {
> >       int rc;
> >
> > +     if (usb_runtime_pm_exception(udev))
> > +             return 0;
> > +
> >       /* Normal USB devices resume/reset through their upstream port.
> >        * Root hubs don't have upstream ports to resume or reset,
> >        * so we have to start up their downstream HC-to-USB
> > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> > index 60363153fc3f..14a054f814a2 100644
> > --- a/drivers/usb/core/usb.h
> > +++ b/drivers/usb/core/usb.h
> > @@ -90,6 +90,22 @@ extern void usb_major_cleanup(void);
> >  extern int usb_device_supports_lpm(struct usb_device *udev);
> >  extern int usb_port_disable(struct usb_device *udev);
> >
> > +#ifdef       CONFIG_PM_RPM_EXCEPTION
> > +
> > +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> > +{
> > +     return atomic_read(&udev->dev.power.usage_count);
> > +}
> > +
> > +#else
> > +
> > +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> > +{
> > +     return 0;
> > +}
> > +
> > +#endif
> > +
> >  #ifdef       CONFIG_PM
> >
> >  extern int usb_suspend(struct device *dev, pm_message_t msg);
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 4b31629c5be4..beba7a0f3947 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -193,6 +193,14 @@ config PM
> >         responsible for the actual handling of device suspend requests and
> >         wake-up events.
> >
> > +config PM_RPM_EXCEPTION
> > +     bool "Prioritize Runtime Power Management more than Power Management"
> > +     default n
>
> The default is always 'n' so no need to specify it.
>

Thanks, I will include this in the next version.

> > +     help
> > +     Provides a way to prioritize Runtime Power Management more than Power
> > +     Management. This way system can suspnd with maintaining specific
> > +     components in operation.
>
> This really doesn't give me a good description of why someone would ever
> want to enable this at all.
>
> And why does this have to be a build option?  That feels very heavy, why
> not make it changable at runtime?
>
> If this is a build option, how are you going to get all the distros and
> all of the Android/ChromeOS systems in the world to enable it?
>
> thanks,
>
> greg k-h

Let's reach a consensus on what this patch should do first. I'll then
change the description and implementation accordingly if needed. Please
see the next reply for an explanation of my idea.

Thanks,
Guan-Yu
Guan-Yu Lin Nov. 8, 2023, 8:45 a.m. UTC | #4
On Tue, Oct 31, 2023 at 10:39 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> > Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> > between system power management and runtime power management. In
> > suspend-to-idle flow, PM core will suspend all devices to avoid device
>
> Your patch affects all forms of system suspend, not just
> suspend-to-idle.  What do you actually mean here?
>
> > interact with the system. However, chances are devices might be used by
> > other systems rather than a single system. In this case, PM core shouldn't
> > suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> > such exception, and determine the power state of a device with runtime
> > power management rather than system power management.
>
> This sort of arrangement -- a device shared between two different
> systems -- could happen with any sort of device.  Why does your patch
> affect only USB devices?
>
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 4b31629c5be4..beba7a0f3947 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -193,6 +193,14 @@ config PM
> >         responsible for the actual handling of device suspend requests and
> >         wake-up events.
> >
> > +config PM_RPM_EXCEPTION
> > +     bool "Prioritize Runtime Power Management more than Power Management"
>
> Runtime Power Management is a form of Power Management, so what you
> wrote doesn't make sense.  What you really meant is: Prioritize Runtime
> Power Management more than System Power Management.
>
> > +     default n
> > +     help
> > +     Provides a way to prioritize Runtime Power Management more than Power
> > +     Management. This way system can suspnd with maintaining specific
>
> s/suspnd/suspend/
> s/with/while/
>
> > +     components in operation.
>
> Your patch does not allow _specific_ components to be kept in operation.
> _All_ in-use components that support prioritized PM (with this patch,
> all USB components) will remain powered during system suspend, even if
> the user wants only _some_ of them to be kept powered.
>
> Alan Stern
>
> > +
> >  config PM_DEBUG
> >       bool "Power Management Debug Support"
> >       depends on PM
> > --
> > 2.42.0.820.g83a721a137-goog
> >

Thanks for the questions. Let me first introduce my motivation for
proposing this feature. We can discuss the implementation details later.

Motivation:
Currently, system PM operations always override runtime PM operations.
As runtime PM reflects the power status of devices, there is a
possibility that runtime PM states that a device is in use, but system
PM decides to suspend it. Up to now, we have assumed that a device can't
function without resources from the system, so the device should acquire
a wakelock to prevent this from happening. However, what if the device
does not need the system's support to function? Or only needs limited
resources (e.g., only limited power source or clock) to function? In this
situation, we would like to keep the device on but allow the system to
suspend. This is an example where we would like devices to follow runtime
PM rather than system PM.

Feature Supported:
1. Devices could control the priority of system PM and runtime PM during
   runtime.
2. The control should be at the device level, meaning that different
   devices should control their own priorities.

Goal of This Patch:
1. Design a framework to support features above.
2. Apply it into usb for demonstration.

Thanks,
Guan-Yu
Alan Stern Nov. 8, 2023, 3:56 p.m. UTC | #5
On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> Thanks for the questions. Let me first introduce my motivation for
> proposing this feature. We can discuss the implementation details later.
> 
> Motivation:
> Currently, system PM operations always override runtime PM operations.
> As runtime PM reflects the power status of devices, there is a
> possibility that runtime PM states that a device is in use, but system
> PM decides to suspend it. Up to now, we have assumed that a device can't
> function without resources from the system, so the device should acquire
> a wakelock to prevent this from happening. However, what if the device

[From the fact that you mention wakelocks, I assume that you're trying 
to implement something for Android systems rather than Linux systems 
in general.]

> does not need the system's support to function? Or only needs limited
> resources (e.g., only limited power source or clock) to function? In this
> situation, we would like to keep the device on but allow the system to
> suspend. This is an example where we would like devices to follow runtime
> PM rather than system PM.

To put it more simply, you want a way to leave some devices in an active 
state while the rest of the system is suspended.  It's not clear why you 
have dragged runtime PM into the discussion (apart from the obvious fact 
that you won't want to keep a device active if it isn't active already).

This sounds like a major change, not something to be done with a simple 
override.  You should discuss it with Rafael Wysocki and the linux-pm 
mailing list before trying to implement anything.

> Feature Supported:
> 1. Devices could control the priority of system PM and runtime PM during
>    runtime.

This seems like a totally unnecessary side issue.  Forget about runtime 
PM for the time being and concentrate instead on which devices you want 
to keep active.

> 2. The control should be at the device level, meaning that different
>    devices should control their own priorities.
> 
> Goal of This Patch:
> 1. Design a framework to support features above.
> 2. Apply it into usb for demonstration.

You may find that it is easier (and less work in the long run) to design 
the general framework and get it working than to concentrate on one 
particular subsystem.

Alan Stern
Guan-Yu Lin Nov. 14, 2023, 9:08 a.m. UTC | #6
On Wed, Nov 8, 2023 at 11:56 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> > Thanks for the questions. Let me first introduce my motivation for
> > proposing this feature. We can discuss the implementation details later.
> >
> > Motivation:
> > Currently, system PM operations always override runtime PM operations.
> > As runtime PM reflects the power status of devices, there is a
> > possibility that runtime PM states that a device is in use, but system
> > PM decides to suspend it. Up to now, we have assumed that a device can't
> > function without resources from the system, so the device should acquire
> > a wakelock to prevent this from happening. However, what if the device
>
> [From the fact that you mention wakelocks, I assume that you're trying
> to implement something for Android systems rather than Linux systems
> in general.]
>

Apologies, I should use "wakeup source" here.

> > does not need the system's support to function? Or only needs limited
> > resources (e.g., only limited power source or clock) to function? In this
> > situation, we would like to keep the device on but allow the system to
> > suspend. This is an example where we would like devices to follow runtime
> > PM rather than system PM.
>
> To put it more simply, you want a way to leave some devices in an active
> state while the rest of the system is suspended.  It's not clear why you
> have dragged runtime PM into the discussion (apart from the obvious fact
> that you won't want to keep a device active if it isn't active already).
>
> This sounds like a major change, not something to be done with a simple
> override.  You should discuss it with Rafael Wysocki and the linux-pm
> mailing list before trying to implement anything.
>
> > Feature Supported:
> > 1. Devices could control the priority of system PM and runtime PM during
> >    runtime.
>
> This seems like a totally unnecessary side issue.  Forget about runtime
> PM for the time being and concentrate instead on which devices you want
> to keep active.
>
> > 2. The control should be at the device level, meaning that different
> >    devices should control their own priorities.
> >
> > Goal of This Patch:
> > 1. Design a framework to support features above.
> > 2. Apply it into usb for demonstration.
>
> You may find that it is easier (and less work in the long run) to design
> the general framework and get it working than to concentrate on one
> particular subsystem.
>
> Alan Stern

Hi Rafael,
We'd like to implement a feature to allow system suspend with several
devices still active. Do you have any consideration on this?

Thanks,
Guan-Yu
Rafael J. Wysocki Nov. 14, 2023, 1:31 p.m. UTC | #7
On Tue, Nov 14, 2023 at 10:08 AM Guan-Yu Lin <guanyulin@google.com> wrote:
>
> On Wed, Nov 8, 2023 at 11:56 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> > > Thanks for the questions. Let me first introduce my motivation for
> > > proposing this feature. We can discuss the implementation details later.
> > >
> > > Motivation:
> > > Currently, system PM operations always override runtime PM operations.
> > > As runtime PM reflects the power status of devices, there is a
> > > possibility that runtime PM states that a device is in use, but system
> > > PM decides to suspend it. Up to now, we have assumed that a device can't
> > > function without resources from the system, so the device should acquire
> > > a wakelock to prevent this from happening. However, what if the device
> >
> > [From the fact that you mention wakelocks, I assume that you're trying
> > to implement something for Android systems rather than Linux systems
> > in general.]
> >
>
> Apologies, I should use "wakeup source" here.
>
> > > does not need the system's support to function? Or only needs limited
> > > resources (e.g., only limited power source or clock) to function? In this
> > > situation, we would like to keep the device on but allow the system to
> > > suspend. This is an example where we would like devices to follow runtime
> > > PM rather than system PM.
> >
> > To put it more simply, you want a way to leave some devices in an active
> > state while the rest of the system is suspended.  It's not clear why you
> > have dragged runtime PM into the discussion (apart from the obvious fact
> > that you won't want to keep a device active if it isn't active already).
> >
> > This sounds like a major change, not something to be done with a simple
> > override.  You should discuss it with Rafael Wysocki and the linux-pm
> > mailing list before trying to implement anything.
> >
> > > Feature Supported:
> > > 1. Devices could control the priority of system PM and runtime PM during
> > >    runtime.
> >
> > This seems like a totally unnecessary side issue.  Forget about runtime
> > PM for the time being and concentrate instead on which devices you want
> > to keep active.
> >
> > > 2. The control should be at the device level, meaning that different
> > >    devices should control their own priorities.
> > >
> > > Goal of This Patch:
> > > 1. Design a framework to support features above.
> > > 2. Apply it into usb for demonstration.
> >
> > You may find that it is easier (and less work in the long run) to design
> > the general framework and get it working than to concentrate on one
> > particular subsystem.
> >
> > Alan Stern
>
> Hi Rafael,
> We'd like to implement a feature to allow system suspend with several
> devices still active. Do you have any consideration on this?

I will have more time to think about this when I'm back from the
conference I'm participating in now.

In the meantime, I would recommend you to address Alan's comments.
Guan-Yu Lin Nov. 15, 2023, 7:08 a.m. UTC | #8
On Wed, Nov 8, 2023 at 11:56 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> > Thanks for the questions. Let me first introduce my motivation for
> > proposing this feature. We can discuss the implementation details later.
> >
> > Motivation:
> > Currently, system PM operations always override runtime PM operations.
> > As runtime PM reflects the power status of devices, there is a
> > possibility that runtime PM states that a device is in use, but system
> > PM decides to suspend it. Up to now, we have assumed that a device can't
> > function without resources from the system, so the device should acquire
> > a wakelock to prevent this from happening. However, what if the device
>
> [From the fact that you mention wakelocks, I assume that you're trying
> to implement something for Android systems rather than Linux systems
> in general.]
>
> > does not need the system's support to function? Or only needs limited
> > resources (e.g., only limited power source or clock) to function? In this
> > situation, we would like to keep the device on but allow the system to
> > suspend. This is an example where we would like devices to follow runtime
> > PM rather than system PM.
>
> To put it more simply, you want a way to leave some devices in an active
> state while the rest of the system is suspended.  It's not clear why you
> have dragged runtime PM into the discussion (apart from the obvious fact
> that you won't want to keep a device active if it isn't active already).
>

The determination of which device should remain active when the system
suspends can be based on various factors. One straightforward approach
is to consider the device's runtime pm state. Alternatively, we could
explore more elaborate techniques that consider additional criteria.

> This sounds like a major change, not something to be done with a simple
> override.  You should discuss it with Rafael Wysocki and the linux-pm
> mailing list before trying to implement anything.
>
> > Feature Supported:
> > 1. Devices could control the priority of system PM and runtime PM during
> >    runtime.
>
> This seems like a totally unnecessary side issue.  Forget about runtime
> PM for the time being and concentrate instead on which devices you want
> to keep active.
>
> > 2. The control should be at the device level, meaning that different
> >    devices should control their own priorities.
> >
> > Goal of This Patch:
> > 1. Design a framework to support features above.
> > 2. Apply it into usb for demonstration.
>
> You may find that it is easier (and less work in the long run) to design
> the general framework and get it working than to concentrate on one
> particular subsystem.
>
> Alan Stern

The big picture is "a way to leave some devices in an active state
while the rest of the system is suspended", I think it could be
separated into:
(1) Each system should be able to choose which device(s) is included
    in this feature.
(2) For devices chosen in (1), each of them should have the flexibility
    to determine when it will not suspend with the system, not just
    always being active when the system suspends.

Regards,
Guan-Yu
Rafael J. Wysocki Nov. 15, 2023, 2:10 p.m. UTC | #9
On Wed, Nov 15, 2023 at 8:08 AM Guan-Yu Lin <guanyulin@google.com> wrote:
>
> On Wed, Nov 8, 2023 at 11:56 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> > > Thanks for the questions. Let me first introduce my motivation for
> > > proposing this feature. We can discuss the implementation details later.
> > >
> > > Motivation:
> > > Currently, system PM operations always override runtime PM operations.
> > > As runtime PM reflects the power status of devices, there is a
> > > possibility that runtime PM states that a device is in use, but system
> > > PM decides to suspend it. Up to now, we have assumed that a device can't
> > > function without resources from the system, so the device should acquire
> > > a wakelock to prevent this from happening. However, what if the device
> >
> > [From the fact that you mention wakelocks, I assume that you're trying
> > to implement something for Android systems rather than Linux systems
> > in general.]
> >
> > > does not need the system's support to function? Or only needs limited
> > > resources (e.g., only limited power source or clock) to function? In this
> > > situation, we would like to keep the device on but allow the system to
> > > suspend. This is an example where we would like devices to follow runtime
> > > PM rather than system PM.
> >
> > To put it more simply, you want a way to leave some devices in an active
> > state while the rest of the system is suspended.  It's not clear why you
> > have dragged runtime PM into the discussion (apart from the obvious fact
> > that you won't want to keep a device active if it isn't active already).
> >
>
> The determination of which device should remain active when the system
> suspends can be based on various factors. One straightforward approach
> is to consider the device's runtime pm state.

Not really.  The runtime PM status has no bearing on whether or not
the device should remain active over a system suspend/resume cycle.

> Alternatively, we could
> explore more elaborate techniques that consider additional criteria.

In fact, the device's driver decides what is going to happen to it
during the system suspend transition.  It very well may decide to
leave the device in the operational state, but it needs to take
dependencies between into account.

> > This sounds like a major change, not something to be done with a simple
> > override.  You should discuss it with Rafael Wysocki and the linux-pm
> > mailing list before trying to implement anything.
> >
> > > Feature Supported:
> > > 1. Devices could control the priority of system PM and runtime PM during
> > >    runtime.
> >
> > This seems like a totally unnecessary side issue.  Forget about runtime
> > PM for the time being and concentrate instead on which devices you want
> > to keep active.
> >
> > > 2. The control should be at the device level, meaning that different
> > >    devices should control their own priorities.
> > >
> > > Goal of This Patch:
> > > 1. Design a framework to support features above.
> > > 2. Apply it into usb for demonstration.
> >
> > You may find that it is easier (and less work in the long run) to design
> > the general framework and get it working than to concentrate on one
> > particular subsystem.
> >
> > Alan Stern
>
> The big picture is "a way to leave some devices in an active state
> while the rest of the system is suspended", I think it could be
> separated into:
> (1) Each system should be able to choose which device(s) is included
>     in this feature.
> (2) For devices chosen in (1), each of them should have the flexibility
>     to determine when it will not suspend with the system, not just
>     always being active when the system suspends.

A specific use case, please.
Guan-Yu Lin Nov. 16, 2023, 9:54 a.m. UTC | #10
On Wed, Nov 15, 2023 at 10:10 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Nov 15, 2023 at 8:08 AM Guan-Yu Lin <guanyulin@google.com> wrote:
> >
> > On Wed, Nov 8, 2023 at 11:56 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> > >
> > > On Wed, Nov 08, 2023 at 04:45:43PM +0800, Guan-Yu Lin wrote:
> > > > Thanks for the questions. Let me first introduce my motivation for
> > > > proposing this feature. We can discuss the implementation details later.
> > > >
> > > > Motivation:
> > > > Currently, system PM operations always override runtime PM operations.
> > > > As runtime PM reflects the power status of devices, there is a
> > > > possibility that runtime PM states that a device is in use, but system
> > > > PM decides to suspend it. Up to now, we have assumed that a device can't
> > > > function without resources from the system, so the device should acquire
> > > > a wakelock to prevent this from happening. However, what if the device
> > >
> > > [From the fact that you mention wakelocks, I assume that you're trying
> > > to implement something for Android systems rather than Linux systems
> > > in general.]
> > >
> > > > does not need the system's support to function? Or only needs limited
> > > > resources (e.g., only limited power source or clock) to function? In this
> > > > situation, we would like to keep the device on but allow the system to
> > > > suspend. This is an example where we would like devices to follow runtime
> > > > PM rather than system PM.
> > >
> > > To put it more simply, you want a way to leave some devices in an active
> > > state while the rest of the system is suspended.  It's not clear why you
> > > have dragged runtime PM into the discussion (apart from the obvious fact
> > > that you won't want to keep a device active if it isn't active already).
> > >
> >
> > The determination of which device should remain active when the system
> > suspends can be based on various factors. One straightforward approach
> > is to consider the device's runtime pm state.
>
> Not really.  The runtime PM status has no bearing on whether or not
> the device should remain active over a system suspend/resume cycle.
>

Thanks for the information.

> > Alternatively, we could
> > explore more elaborate techniques that consider additional criteria.
>
> In fact, the device's driver decides what is going to happen to it
> during the system suspend transition.  It very well may decide to
> leave the device in the operational state, but it needs to take
> dependencies between into account.

Seems like it would be better for each device to modify its suspend/
resume code rather than designing a generic framework. As the specific
use cases of each component are not yet fully understood, the device
driver provides ample flexibility for customization at this stage.

>
> > > This sounds like a major change, not something to be done with a simple
> > > override.  You should discuss it with Rafael Wysocki and the linux-pm
> > > mailing list before trying to implement anything.
> > >
> > > > Feature Supported:
> > > > 1. Devices could control the priority of system PM and runtime PM during
> > > >    runtime.
> > >
> > > This seems like a totally unnecessary side issue.  Forget about runtime
> > > PM for the time being and concentrate instead on which devices you want
> > > to keep active.
> > >
> > > > 2. The control should be at the device level, meaning that different
> > > >    devices should control their own priorities.
> > > >
> > > > Goal of This Patch:
> > > > 1. Design a framework to support features above.
> > > > 2. Apply it into usb for demonstration.
> > >
> > > You may find that it is easier (and less work in the long run) to design
> > > the general framework and get it working than to concentrate on one
> > > particular subsystem.
> > >
> > > Alan Stern
> >
> > The big picture is "a way to leave some devices in an active state
> > while the rest of the system is suspended", I think it could be
> > separated into:
> > (1) Each system should be able to choose which device(s) is included
> >     in this feature.
> > (2) For devices chosen in (1), each of them should have the flexibility
> >     to determine when it will not suspend with the system, not just
> >     always being active when the system suspends.
>
> A specific use case, please.

We have a sub-system sharing some devices (e.g., usb controller, host
controller) with the main system. In the current system power
management framework, when the main system suspends, the devices will
suspend, too. However, sometimes these devices are still used by the
sub-system, so we don't want the main system to always suspend the
devices.

Regards,
Guan-Yu
diff mbox series

Patch

diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index 740342a2812a..bb0dfcfc9764 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -266,6 +266,9 @@  int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
 {
 	int rc;
 
+	if (usb_runtime_pm_exception(udev))
+		return 0;
+
 	/* Normal USB devices suspend through their upstream port.
 	 * Root hubs don't have upstream ports to suspend,
 	 * so we have to shut down their downstream HC-to-USB
@@ -294,6 +297,9 @@  int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
 {
 	int rc;
 
+	if (usb_runtime_pm_exception(udev))
+		return 0;
+
 	/* Normal USB devices resume/reset through their upstream port.
 	 * Root hubs don't have upstream ports to resume or reset,
 	 * so we have to start up their downstream HC-to-USB
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 60363153fc3f..14a054f814a2 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -90,6 +90,22 @@  extern void usb_major_cleanup(void);
 extern int usb_device_supports_lpm(struct usb_device *udev);
 extern int usb_port_disable(struct usb_device *udev);
 
+#ifdef	CONFIG_PM_RPM_EXCEPTION
+
+static inline int usb_runtime_pm_exception(struct usb_device *udev)
+{
+	return atomic_read(&udev->dev.power.usage_count);
+}
+
+#else
+
+static inline int usb_runtime_pm_exception(struct usb_device *udev)
+{
+	return 0;
+}
+
+#endif
+
 #ifdef	CONFIG_PM
 
 extern int usb_suspend(struct device *dev, pm_message_t msg);
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 4b31629c5be4..beba7a0f3947 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -193,6 +193,14 @@  config PM
 	  responsible for the actual handling of device suspend requests and
 	  wake-up events.
 
+config PM_RPM_EXCEPTION
+	bool "Prioritize Runtime Power Management more than Power Management"
+	default n
+	help
+	Provides a way to prioritize Runtime Power Management more than Power
+	Management. This way system can suspnd with maintaining specific
+	components in operation.
+
 config PM_DEBUG
 	bool "Power Management Debug Support"
 	depends on PM